8000 Token refresh can cause automatic authStyle detection to break · Issue #718 · golang/oauth2 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Token refresh can cause automatic authStyle detection to break #718

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.

Sign up for GitHub

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

Jump to bottom
Closed
codablock opened this issue Apr 5, 2024 · 5 comments
Closed

Token refresh can cause automatic authStyle detection to break #718

codablock opened this issue Apr 5, 2024 · 5 comments

Comments

@codablock
Copy link

retrieveToken is called in two situations:

  1. When the initial token is exchanged (https://github.com/golang/oauth2/blob/master/oauth2.go#L234)
  2. When the token is refreshed (https://github.com/golang/oauth2/blob/master/oauth2.go#L275)

In both cases, internal.RetrieveToken is called, which then performs auto-detection of authType when not explicitly configured.

The problem now is, that some providers allow to perform token refresh without additional authentication. They're perfectly happy when the refresh token is valid and thus they successfully return a valid token even when the wrong authType is used. This leads to caching the wrong authStyle when a token refresh is performed BEFORE the first token exchange.

This can happen after the application is restarted while users still have valid refresh tokens (e.g. in the browser session). If the application then issues a token refresh before a new signin is performed, the errornous caching will cause ALL future token exchanges to fail as it will keep using the wrong authStyle.

I encountered this with Azure AD and the v2.0 issuer.
codablock added a commit to kluctl/kluctl that referenced this issue Apr 5, 2024
@codablock
fix: Use a temporary copy of the oauth2 config while refreshing tokens
8e360db 
This is a workaround for golang/oauth2#718
@codablock codablock mentioned this issue Apr 5, 2024
Merged
1 task
@seankhliao
Copy link
Member

unfortunate, but i think if that's the case, it'd be better to explicitly specify the authstyle required

@codablock
Copy link
Author

@seankhliao Explicitly specifying the authStyle is only a viable solution when the provider is known in advance. In my case, the user specifies an OpenID discovery URL and expects the application to figure everything out on its own.

If specifying the authStyle is considered the proper solution, then the whole auto-detection of the authStyle should be re-considered.

@seankhliao
Copy link
Member
seankhliao commented Apr 22, 2025 •
edited
Loading

I think it's regrettable that it was added at all, though we aren't going to remove it.
In the time since the package was created, there's now a more standard oauth2 metadata endpoint https://datatracker.ietf.org/doc/html/rfc8414
and the associated key token_endpoint_auth_methods_supported

@seankhliao
Copy link
Member

Note that per rfc https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1

The authorization server MUST support the HTTP Basic authentication scheme for authenticating clients that were issued a client password.

Probing for any other support is a nice to have, but ultimately works around non compliant servers.

@codablock
Copy link
Author

Ah ok, thanks for the clarification, I was not aware of that development happening in the mean time 👍
I see that Azure AD v2 has the token_endpoint_auth_methods_supported field set, so I should be able to use it in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@codablock @seankhliao
0