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

Move auth calculation into refresh_token method #340

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

joelugh
Copy link

@joelugh joelugh commented Dec 3, 2018

I was using an endpoint with no client_secret. I couldn't pass client_id to the endpoint as Basic auth unless I provided it as an auth kwarg to each request. However if the TokenExpiredError was not raised, then this auth was passed through to the original request endpoint as it was not popped from kwargs (?overwriting the intended token auth).

Based on the docs it seems like I should be able to pass client_id through auto_refresh_kwargs (https://requests-oauthlib.readthedocs.io/en/latest/oauth2_workflow.html). However currently auth does not consider auto_refresh_kwargs at all.

Changes assume that the intended behaviour is that the token refresh auth can be provided in auto_refresh_kwargs else in the request kwargs. And that a client_secret is not mandatory.

Note that client_id and client_secret would still be passed down from the request method as kwargs.

More generally,

  • I don't think a client_secret should be required for token refresh by client_id
  • I don't think client_id and client_secret should need to be provided in every request but rather be sourced at a session level (like auto_refresh_kwargs) or from the client itself.

@singingwolfboy
Copy link
Member

This pull request can't be reviewed until all the automated tests pass on Travis CI.

fodinabor added a commit to fodinabor/requests-oauthlib that referenced this pull request Apr 21, 2020
fodinabor added a commit to fodinabor/requests-oauthlib that referenced this pull request Apr 21, 2020
fodinabor added a commit to fodinabor/requests-oauthlib that referenced this pull request Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants