-
Notifications
You must be signed in to change notification settings - Fork 887
fix: stop extending API key access if OIDC refresh is available #17878
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
Conversation
473d5a4
to
c430f42
Compare
// Checking if the key is expired. | ||
// NOTE: The `RequireAuth` React component depends on this `Detail` to detect when | ||
// the users token has expired. If you change the text here, make sure to update it | ||
// in site/src/components/RequireAuth/RequireAuth.tsx as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that RequireAuth.tsx
was modified to not have this string match dependency in #9442
Also includes some extra logging in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small suggestions but the change makes sense to me. I can approve if need be but I'd feel better if @Emyrk also took a look, so deferring approval for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all makes sense to me 👍
c430f42
to
e6f157c
Compare
05131e2
to
45df24d
Compare
Merge activity
|
fixes #17070
Cleans up our handling of APIKey expiration and OIDC to keep them separate concepts. For an OIDC-login APIKey, both the APIKey and OIDC link must be valid to login. If the OIDC link is expired and we have a refresh token, we will attempt to refresh.
OIDC refreshes do not have any effect on APIKey expiry.
#17070 (comment) explains why this is the correct behavior.