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

OIDC is broken with requests-auth 8.0.0 #639

Closed
Andy-Grigg opened this issue Aug 15, 2024 · 4 comments · Fixed by #640
Closed

OIDC is broken with requests-auth 8.0.0 #639

Andy-Grigg opened this issue Aug 15, 2024 · 4 comments · Fixed by #640
Labels
bug Something isn't working

Comments

@Andy-Grigg
Copy link
Contributor

OIDC is broken in two different ways when using requests-auth 8.0.0:

  • The import path of the OAuth2 class has changed from requests_auth.authentication to requests_auth
  • TokenMemoryCache.forbid_concurrent_missing_token_function_call is now a private attribute

The lock file currently pins the package at a version <8, presumably because this version increase broke the tests.

@Andy-Grigg Andy-Grigg added the bug Something isn't working label Aug 15, 2024
@Andy-Grigg
Copy link
Contributor Author

Andy-Grigg commented Aug 15, 2024

The first fix is trivial, we can just wrap this in a try/except and everything works. However, the second one is more problematic. We could of course just use the private attribute, and I see we're using the private _add_acces_token method anyway, so maybe this isn't too bad.

I'm guessing we're using these private methods/attributes because we're trying to do something the package doesn't do, from context it seems like we want to force a refresh, but requests-auth doesn't support this. @da1910 do you remember why we have this in here? Is it worth trying to contribute this upstream?

@Andy-Grigg
Copy link
Contributor Author

For reference, Colin-b/requests_auth#81 is the PR that introduced this change

@da1910
Copy link
Collaborator

da1910 commented Aug 16, 2024

The issue, as I recall, was if you provide a refresh token to the builder you can get into a situation where the initial request to get an access token also triggers the refresh token to be rotated. I don't think this was properly handled in requests-auth, and we ended up with an out of date refresh token.

It might be worth seeing if it's still and issue and if so contributing a fix. This was an expedient fix at the time.

@Andy-Grigg
Copy link
Contributor Author

In that case, I'll create a separate issue to improve fundamentally how we do creating a session with a refresh token, which will probably involve seeing if we can contribute an improvement to requests_auth.

This ticket will address the immediate failure with requests_auth 8.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants