frontend: warn user before token expiry and auto-logout on session end#5371
frontend: warn user before token expiry and auto-logout on session end#5371prabindersinghh wants to merge 8 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: prabindersinghh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @prabindersinghh! |
6ae57b7 to
ed52048
Compare
There was a problem hiding this comment.
Pull request overview
Adds session-expiry awareness across the auth flow so the frontend can learn token lifetime from /clusters/:cluster/me, warn before expiry, and react when the session ends.
Changes:
- Extended backend
/meresponses to includetokenExpiryderived from the JWTexpclaim. - Added frontend
fetchClusterMetypes/helper for reading/clusters/:cluster/meand distinguishing 401 expiry from other failures. - Introduced
TokenExpiryNotificationin the main layout to poll session state, show an expiry banner, and trigger logout on confirmed expiry.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/lib/auth.ts |
Adds /me fetch helper and response/result types for token-expiry-aware frontend auth checks. |
frontend/src/components/App/TokenExpiryNotification.tsx |
New banner/poller component for warning on impending expiry and logging out after expiry. |
frontend/src/components/App/Layout.tsx |
Mounts the new token-expiry notification in the app shell. |
backend/pkg/auth/auth.go |
Includes token expiry in /me responses after validating the token is still valid. |
backend/pkg/auth/auth_test.go |
Adds backend test coverage for tokenExpiry being returned by /me. |
illume
left a comment
There was a problem hiding this comment.
Thanks for working on this.
Would you mind addressing the open Copilot review comments? Please mark each comment as resolved after addressing it.
|
Thanks for the review! Working on addressing all three Copilot comments now, countdown timer, test coverage, and the logout delay gap. Will push shortly. |
|
Addressed all three points, added a per-second setInterval for the live countdown, fixed immediate local-expiry logout without waiting for the next /me poll, and added 4 Vitest tests covering the warning threshold, future token, tokenExpired logout, and route suppression. Resolved all Copilot conversations. |
illume
left a comment
There was a problem hiding this comment.
Thanks for these changes.
Can you please have a look at the git commits to see if they meet the contribution guidelines? We use a Linux kernel style of git commits. See the contributing guide for general context, and please see previous git commits with git log for examples.
Commits that need attention
frontend: fix countdown timer, add tests, fix immediate logout on expiry— Description must start with a capital letter — e.g.frontend: HomeButton: Fix the buttonnotfrontend: HomeButton: fix the button.
Commit guidelines
- Use atomic commits focused on a single change.
- Use the title format
<area>: <Description of changes>— description must start with a capital letter. - Keep the title under 72 characters (soft requirement).
- Explain the intention and why the change is needed.
- Make commit titles meaningful and describe what changed.
- Do not add code that a later commit rewrites; squash or reorder commits instead.
- Do not include
Fixes #NNin commit messages.
Good examples:
frontend: HomeButton: Fix so it navigates to homebackend: config: Add enable-dynamic-clusters flag
illume
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
There are some open Copilot review comments — could you take a look at them? Please mark each one as resolved once you've addressed it.
The GitHub CI test job has snapshot failures. Run cd frontend && npm run test -- -u to regenerate the snapshots.
How to update snapshots
Run cd frontend && npm run test -- -u to regenerate all snapshots. Review the diff to make sure the visual changes are intentional, then commit the updated snapshot files.
The frontend lint job in CI is failing. Run cd frontend && npm run lint to see the errors.
How to fix lint errors
Run cd frontend && npm run lint to see all ESLint errors. Many can be fixed automatically with cd frontend && npm run lint -- --fix. Remaining errors need manual attention.
The backend test job in CI is failing. Run cd backend && go test ./... to reproduce the errors locally.
How to run the backend tests
Run cd backend && go test ./... to see all failures. Fix the failing tests and commit the result.
64c4ca1 to
62eca1c
Compare
|
Thanks for the detailed feedback. Fixing the commit message casing, addressing the setInterval early-start issue, correcting the mock route paths, running lint, fixing backend tests, and regenerating snapshots. Will push a clean rebase shortly. |
62eca1c to
76794ca
Compare
|
Pushed updated commits, fixed commit message casing to match headlamp style (capital after colon), addressed the setInterval early-start with a setTimeout delay, corrected mock route paths to match the real router, fixed lint (eqeqeq, import order), regenerated the pluginLib snapshot which now includes fetchClusterMe, and backend tests pass. Resolved all Copilot conversations. |
illume
left a comment
There was a problem hiding this comment.
Thanks for these changes.
Can you please have a look at the git commits to see if they meet the contribution guidelines? We use a Linux kernel style of git commits. See the contributing guide for general context, and please see previous git commits with git log for examples.
Commits that need attention
frontend: TokenExpiryNotification: key polling on cluster name not pathname— Description must start with a capital letter — e.g.frontend: HomeButton: Fix the buttonnotfrontend: HomeButton: fix the button.
Commit guidelines
- Use atomic commits focused on a single change.
- Use the title format
<area>: <Description of changes>— description must start with a capital letter. - Keep the title under 72 characters (soft requirement).
- Explain the intention and why the change is needed.
- Make commit titles meaningful and describe what changed.
- Do not add code that a later commit rewrites; squash or reorder commits instead.
- Do not include
Fixes #NNin commit messages.
Good examples:
frontend: HomeButton: Fix so it navigates to homebackend: config: Add enable-dynamic-clusters flag
I noticed the GitHub CI frontend i18n check is failing. It looks like the translation files need to be updated. You can run cd frontend && npm run i18n locally to update them and then commit the result.
How to update translation files
Run cd frontend && npm run i18n to regenerate the translation files. Commit the updated files alongside your other changes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
frontend/src/components/App/TokenExpiryNotification.tsx:84
- If a subsequent poll returns a successful response that omits
tokenExpiry(e.g., after a silent token refresh that switches the backend to a non-JWT/proxy auth path, or for an auth provider that stops returning the claim), the existingtokenExpirystate value is never cleared. The component will continue to count down toward the old expiry and may auto-logout based on stale data. Consider explicitly resettingtokenExpiryto null when the new response is successful buttokenExpiryis missing.
if (result.tokenExpired) {
setTokenExpired(true);
} else if (result.data?.tokenExpiry !== null && result.data?.tokenExpiry !== undefined) {
setTokenExpiry(result.data.tokenExpiry);
}
frontend/src/components/App/TokenExpiryNotification.tsx:138
- Auto-logout fires on the first poll if the backend returns 401, including the case where the user navigated to a Headlamp route with a cluster context but has never authenticated to that cluster (e.g., cold load on a route under
/c/<cluster>/...before the auth flow has run). In that scenario the user gets immediately bounced throughlogout(), which can produce a confusing UX vs. the normal "you are not authenticated" flow. Consider only triggering auto-logout when the user previously had a valid session (i.e., we've successfully observedtokenExpired: falseat least once during this mount), rather than on the very first response.
React.useEffect(() => {
if (!tokenExpired) return;
if (clusterName) {
logout(clusterName);
}
}, [tokenExpired, clusterName]);
frontend/src/components/App/TokenExpiryNotification.tsx:82
- The two
null/undefinedchecks onresult.data?.tokenExpiryare redundant — optional chaining on a nulldataalready yieldsundefined. This can be simplified to a single!= null(loose equality) ortypeof result.data?.tokenExpiry === 'number'check, which also defensively guards against unexpected non-numeric values from the backend.
} else if (result.data?.tokenExpiry !== null && result.data?.tokenExpiry !== undefined) {
Add tokenExpiry Unix timestamp to writeMeResponse so the frontend can track session lifetime without reading httpOnly cookies.
2ab10e6 to
d9dbdde
Compare
|
Addressed all Copilot feedback in the latest push (bed0c63): -Reverted time.Now().Before → time.Now().After for correct expiry boundary semantics. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
frontend/src/components/App/TokenExpiryNotification.tsx:84
- When a successful
/meresponse is returned without atokenExpiryfield (for example after a silent token refresh that produces a non-JWT credential, or when the auth provider stops returning anexpclaim), the existingtokenExpirystate is not cleared. The countdown and the local clock-based check at lines 125–128 will continue to operate against the old expiry value, and may fire a false-positive warning banner — or even auto-logout — even though the current token has no known expiry. Consider resettingtokenExpirytonullwhen the field is absent from a successful response.
if (result.tokenExpired) {
setTokenExpired(true);
} else if (result.data?.tokenExpiry !== null && result.data?.tokenExpiry !== undefined) {
setTokenExpiry(result.data.tokenExpiry);
}
frontend/src/components/App/TokenExpiryNotification.tsx:149
- At line 149 the render path calls
getCluster()again instead of reusing theclusterNamevariable already computed at line 55 (and used as the effect dependency). Re-reading the cluster here can desynchronize from the effect (which keyed its setup on the earlierclusterName) ifgetCluster()ever returns a different value between the two reads on the same render. For consistency and to avoid surprising behavior, useclusterNamehere.
if (!showOnRoute || !getCluster()) {
|
Addressed latest Copilot feedback in efb150d: -Polling interval now clears immediately on token expiry via tokenExpiredRef, no further /me requests after logout is triggered |
illume
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
There are some open Copilot review comments — could you take a look at them? Please mark each one as resolved once you've addressed it.
5387837 to
122e781
Compare
|
All Copilot comments addressed and resolved: -JSDoc @internal tag merged into main docstring block Ready for re-review. |
Summary
Adds a session expiry warning banner that counts down the last 2 minutes
of a token's lifetime and automatically logs the user out when the token
expires. The backend
/meendpoint now exposestokenExpiryso thefrontend can track session lifetime without reading httpOnly cookies.
Related Issue
Related #5112
Changes
tokenExpiryfield towriteMeResponseinbackend/pkg/auth/auth.goTestHandleMe_IncludesTokenExpiryinbackend/pkg/auth/auth_test.gofetchClusterMe,ClusterMeResponse, andClusterMeResultdiscriminated union in
frontend/src/lib/auth.tsTokenExpiryNotification.tsxcomponent — polls/meevery60s, shows countdown warning banner when <2 min remain, auto-logouts
on confirmed expiry
<TokenExpiryNotification />inLayout.tsxalongside<AlertNotification />Steps to Test
kubectl create token default --duration=180scountdown ("Session expires in 1:45")
and redirected to auth page with "Session expired. Logging out…"
Screenshots (if applicable)
Warning State (< 2 minutes remaining)
Expired State (auto logout)
Behavior with automatically refreshed tokens
For auth providers that silently refresh tokens, the experience is
transparent — the updated expiry is reflected on the next poll and the
banner either never appears or dismisses immediately. The countdown
resets on every successful poll.
Known limitation — OIDC users
For OIDC clusters, the backend middleware refreshes tokens reactively
within 10s of expiry. This creates a ~110 second window where the warning
banner may appear for OIDC users even though they will not actually be
logged out.
This can be addressed by detecting the auth type from the cluster config
on the frontend and suppressing the banner entirely when the provider is
known to support silent refresh. Happy to include this in the current PR
or keep it as a follow-up, whichever you prefer.
Notes for the Reviewer
or tighter (30s) based on preference
PureTokenExpiryNotificationaccepts an injectablefetchClusterMeFnfor unit testing
support left as a follow-up