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

Not possible to include HTTP Basic Auth data in auto_refresh_kwargs #379

Open
GittCatt opened this issue Aug 21, 2019 · 6 comments
Open

Comments

@GittCatt
Copy link

GittCatt commented Aug 21, 2019

Hello.
I feel that it should be possible to add auth (as supported by requests, in tuple form) to auto_refresh_kwargs in OAuth2Session. Right now both possible situations are bad:

  • adding auth to auto_refresh_kwargs results in adding it to the body of the refresh request;
  • adding auth to OAuth2Session directly would result in it being passed to all subsequent requests made with that session, which is not what I want.

I would like to add it to OAuth2Session in the way that would ensure that any automatic future calls to refresh_token will have auth passed as well, so it would end up being passed to requests and handled as HTTP Basic Auth.

PS A workaround to do it would be, for example:

def wrap(func):
    def wrapper(*args, **kwargs):
        kwargs['auth'] = ('user', 'password')
        return func(*args, **kwargs)
    return wrapper

session.refresh_token = wrap(session.refresh_token)
@OrangeDog
Copy link

refresh_token should be using the same auth (client id+secret) as fetch_token.
There isn't really any reason for it to be a separate function in the first place.

The refresh mechanism is unusable right now, as there's no way to provide the required auth.

@OrangeDog
Copy link

You can hack around this by always passing client_id and client_secret to Session.request().
They should only ever get used for automatic refresh.

@GittCatt
Copy link
Author

GittCatt commented Sep 22, 2020

Is there anything wrong with my hack above?

By the way, I'm not really sure if you have the same issue in mind as I did.

@GittCatt GittCatt changed the title Not possible to include auth in auto_refresh_kwargs Not possible to include HTTP Basic Auth data in auto_refresh_kwargs Sep 22, 2020
@OrangeDog
Copy link

This also also part of #264

@JimHokanson
Copy link

What's the status on this? It seems like there may be solutions but I'm not smart enough to understand what is being proposed. I thought just setting the auth property to the basic authentication header would work but then of course subsequent API requests that need the access token as the authentication header fail. More details on my situation here:
https://stackoverflow.com/questions/65874797/include-authorization-in-a-oauth2session-for-requests-oauthlib

As is, this example in the docs (below) doesn't seem like it will actually work if the API server wants a basic authentication header for the refresh token.

https://requests-oauthlib.readthedocs.io/en/latest/oauth2_workflow.html#third-recommended-define-automatic-token-refresh-and-update

>>> from requests_oauthlib import OAuth2Session
>>> client = OAuth2Session(client_id, token=token, auto_refresh_url=refresh_url,
...     auto_refresh_kwargs=extra, token_updater=token_saver)
>>> r = client.get(protected_url)

@timdawborn
Copy link

I'm also desiring this functionality. The Xero API requires Basic Auth for the refresh token: https://developer.xero.com/documentation/oauth2/auth-flow#refresh . I'm currently having to work around auto-refresh not working by caching the token expired exceptions everywhere where the session is used. Not very elegant.

The PR #433 looks like a relatively clean solution that doesn't change the API or backwards compatibility of refresh_token.

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

No branches or pull requests

4 participants