Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add SessionAuthHandler #599

Merged
merged 13 commits into from
Feb 15, 2024

Conversation

mishaschwartz
Copy link
Contributor

@mishaschwartz mishaschwartz commented Feb 7, 2024

Add weaver.cli.SessionAuthHandler class which uses the cookies stored in a pre-existing requests.Session instance to authenticate.

Alternative to #597 as discussed in #597 (comment)

Closes #597

@github-actions github-actions bot added ci/doc Issue related to documentation of the package feature/cli Issues or features related to CLI operations. labels Feb 7, 2024
@mishaschwartz
Copy link
Contributor Author

Ok, here's the new usage:

CLI:

weaver processes -aC weaver.cli.CookieAuthHandler -aT 'auth=thecookievaluethattshouldbeincluded' --url https://example.com/weaver

Client

s = requests.Session()
# add cookies to the session
client = WeaverClient("https://example.com/weaver", auth=CookieAuthHandler(token=s.cookies.get_dict()))

@mishaschwartz
Copy link
Contributor Author

Note that because I made the changes in the RequestAuthHandler as discussed:

It would be fine with adding a pre-check to RequestAuthHandler that skips request_auth call when the token/cookie is predefined in the handler

it is possible to send a token to the BearerAuthHandler as well in a similar way (if you have the bearer token already)

fmigneault
fmigneault previously approved these changes Feb 12, 2024
Copy link
Collaborator

@fmigneault fmigneault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the new feature (waiting for the tests to run/succeed before merge).

@fmigneault
Copy link
Collaborator

@mishaschwartz You can add yourself to the contributors list in https://github.com/crim-ca/weaver/blob/master/AUTHORS.rst if you desire.

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d234e11) 85.65% compared to head (e7e3670) 85.66%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #599      +/-   ##
==========================================
+ Coverage   85.65%   85.66%   +0.01%     
==========================================
  Files          79       79              
  Lines       18559    18580      +21     
  Branches     2844     2847       +3     
==========================================
+ Hits        15896    15917      +21     
  Misses       1926     1926              
  Partials      737      737              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fmigneault fmigneault merged commit d911c16 into crim-ca:master Feb 15, 2024
26 of 27 checks passed
@mishaschwartz mishaschwartz deleted the session-authentication-alt branch February 15, 2024 16:29
@fmigneault
Copy link
Collaborator

@mishaschwartz FYI https://github.com/crim-ca/weaver/tree/5.1.0

@fmigneault
Copy link
Collaborator

@mishaschwartz Due to a CI issue with https://github.com/crim-ca/weaver/tree/5.1.0, I pushed another https://github.com/crim-ca/weaver/tree/5.1.1. The docker image for it should be ready soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/doc Issue related to documentation of the package feature/cli Issues or features related to CLI operations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants