-
Notifications
You must be signed in to change notification settings - Fork 167
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
feat(ui): Refresh token exchange #1171
Conversation
✅ Deploy Preview for docs-kargo-akuity-io ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1171 +/- ##
=======================================
Coverage 45.82% 45.82%
=======================================
Files 135 135
Lines 11556 11556
=======================================
Hits 5296 5296
Misses 6069 6069
Partials 191 191 ☔ View full report in Codecov by Sentry. |
if (refreshToken) { | ||
localStorage.setItem(refreshTokenKey, refreshToken); | ||
} |
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 is never getting set for me, but I haven't quite figured out why.
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.
And my dex is definitely supporting it:
{
"issuer": "https://localhost:30081/dex",
"authorization_endpoint": "https://localhost:30081/dex/auth",
"token_endpoint": "https://localhost:30081/dex/token",
"jwks_uri": "https://localhost:30081/dex/keys",
"userinfo_endpoint": "https://localhost:30081/dex/userinfo",
"device_authorization_endpoint": "https://localhost:30081/dex/device/code",
"grant_types_supported": [
"authorization_code",
"refresh_token",
"urn:ietf:params:oauth:grant-type:device_code"
],
"response_types_supported": [
"code"
],
"subject_types_supported": [
"public"
],
"id_token_signing_alg_values_supported": [
"RS256"
],
"code_challenge_methods_supported": [
"S256",
"plain"
],
"scopes_supported": [
"openid",
"email",
"groups",
"profile",
"offline_access"
],
"token_endpoint_auth_methods_supported": [
"client_secret_basic",
"client_secret_post"
],
"claims_supported": [
"iss",
"sub",
"aud",
"iat",
"exp",
"email",
"email_verified",
"locale",
"name",
"preferred_username",
"at_hash"
]
}
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.
idk if it's a factor, but I'm using a self-signed cert. In the CLI, I ignore refresh tokens if the user picked --insecure-skip-tls-verify
. This is because I want to force them to re-authenticate in order to re-evaluate that decision.
idk if this is a factor here as well and possibly the source of my difficulties?
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.
@krancour Can you check the response from the server? Is there a refreshToken?
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.
@rpelczar I made a short video.
Note to an nefarious evildoers: The fragments of tokens you see in the video are for an IDP running locally and are completely useless. 😁
Screen.Recording.2023-12-11.at.7.54.12.PM.mov
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.
@krancour I found a bug, and fixed it.
Signed-off-by: Rafal Pelczar <rafal@akuity.io>
94727e3
to
4e2cbcc
Compare
Fixes #877
In this PR, I implemented a refresh token exchange.
How it works: when the token is expired before any API call, the user is redirected to the token renewal page, where the loader is visible for a second, and after the token exchange process, the user is redirected to the previous page.
The perfect solution is a request in the background, but unfortunately, we can't use React hooks inside the interceptor for now (library limitations), which is needed for our auth flow.