ansys / openapi-common

Common authentication components for OpenAPI client libraries
https://openapi.docs.pyansys.com/
MIT License
2 stars 0 forks source link

OIDC is broken with requests-auth 8.0.0 #639

Closed Andy-Grigg closed 2 months ago

Andy-Grigg commented 3 months ago

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

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

Andy-Grigg commented 3 months ago

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 commented 3 months ago

For reference, https://github.com/Colin-b/requests_auth/pull/81 is the PR that introduced this change

da1910 commented 3 months ago

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 commented 2 months ago

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