fix(ui): silent-refresh cold-load 401 and post-refresh reauth - #31597
Conversation
Two user-visible bugs in the SPA silent-refresh path, fixed with
surgical changes to AuthProvider.tsx. No architecture changes.
Bug 1 — cold-load 401 doesn't trigger silent refresh:
On tab reopen after id-token expiry, /loggedInUser returns
401 {"code":401,"message":"Expired token!"}, but the axios
interceptor's refresh path races the lazy authenticator's renewer
registration and TokenService.fetchNewToken returns null before
the network call fires. The interceptor then force-logs the user
out — no /token call ever happens, and the user lands on /signin.
Fix: proactive gate in getLoggedInUserDetails — check the stored
token's expiry BEFORE calling /loggedInUser. If expired, wait up
to 2s for the renewer to become a function on TokenService, then
call refreshToken() and only after success issue /loggedInUser.
Additional defense: in the catch block, if the error is still a
refreshable 401 (e.g. the interceptor refresh happened but the
retry failed), give the renewer one more chance and retry once.
Falls through to resetUserDetails on any failure.
Bug 2 — visibility-triggered silent refresh doesn't reauth the UI:
visibilitychange handler successfully refreshes the token after
Bug 1 has already flipped isAuthenticated=false, but nothing
runs getLoggedInUserDetails afterward. Storage is fresh, UI stays
on the sign-in page.
Fix: in the visibility handler, await the refresh, then read
useApplicationStore.getState().isAuthenticated (avoids the mount-
only useEffect's stale closure) and run getLoggedInUserDetails
if false — that flips isAuthenticated back to true and drops the
user into the authenticated app.
Both fixes are self-contained helpers (waitForRenewerReady,
ensureFreshTokenBeforeUserFetch, retryLoggedInUserAfterRenewer,
applyLoggedInUser, isRefreshableAuthError) inside AuthProvider.tsx.
No storage schema, SDK, or interceptor architecture changes; no
coordinator, no cross-tab primitive additions.
Tests: 5 new unit tests pin down the contracts the fixes rely on —
TokenService.refreshToken return-shape normalization, REFRESHABLE_
AUTH_ERRORS classification, visibility handler sequencing, and the
skip-reauth branches. All 14 pre-existing tests continue to pass
unchanged (19/19 total).
Follow-up (separate PR after release): full AuthCoordinator refactor
that owns silent-refresh, cross-tab lock, 401 queue, and focus
handling as a single module.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
✅ Playwright Results — workflow succeededValidated commit ✅ 797 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 52m 57s ⏱️ Max setup 3m 30s · max shard execution 21m 10s · max shard-job elapsed before upload 24m 44s · reporting 5s 🌐 222.43 requests/attempt · 2.67 app boots/UI scenario · 28.82% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
…l unit coverage Addresses two PR-review findings: Greptile P1 — "Renewer timeout destroys session": Previously ensureFreshTokenBeforeUserFetch returned false on renewer- registration timeout, and getLoggedInUserDetails called resetUserDetails() on that false — which clears the valid refresh credential and forces the user to /signin on merely-slow lazy authenticator load. Fix: change the helper to Promise<void> with best-effort semantics — if the renewer never registers, or the refresh throws, we resolve silently. The caller then issues /loggedInUser normally and lets the axios interceptor + catch-block retry handle a real 401 if one arrives. A hard failure here is strictly worse than the original bug it was trying to fix. Gitar + Greptile P2 — "Tests duplicate production behavior": Removed the five tautological tests that asserted against local inline closures. Extracted the pure helpers — isRefreshableAuthError, waitForRenewerReady, ensureFreshTokenBeforeUserFetch — from AuthProvider.tsx into silentRefreshHelpers.ts, taking their dependencies as parameters. Added silentRefreshHelpers.test.ts with 14 unit tests that exercise the actual exported functions, covering every branch (including the best-effort no-throw contract that guards against P1). Total: 42/42 tests pass (14 pre-existing AuthProvider + 14 pre- existing OktaAuthProvider + 14 new silentRefreshHelpers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to Greptile's P1 restatement: even with the best-effort proactive gate in getLoggedInUserDetails, a lazy authenticator load that takes longer than the caller's wait would fall through to the /loggedInUser request, get a 401, and the response interceptor's refreshToken() would still return null (because renewToken isn't registered yet) — triggering resetUserDetails(true), which clears storage before the caller's catch could retry. Real fix: move the renewer wait into TokenService.fetchNewToken itself. The interceptor's refresh call now blocks briefly (up to 10s) for the lazy authenticator to register its renewer, then performs the refresh normally. resetUserDetails(true) only fires when the renewer truly never registers within the wait window. This makes the fix work for every caller of tokenService.refreshToken (interceptor, visibility handler, proactive timer, SSE stream reconnect) — not just getLoggedInUserDetails. Cleanup in AuthProvider.tsx: - Removed ensureFreshTokenBeforeUserFetch call and the retryLoggedInUserAfterRenewer catch-block retry — both were racing the same failure mode that TokenService now handles at the source. - Removed the silentRefreshHelpers module (no longer used in production). - getLoggedInUserDetails is back to a straightforward try/catch; refresh reliability now lives in TokenService where it belongs. Bug 2 fix (visibility-triggered reauth) is unchanged. Tests: 4 new TokenServiceUtil.test.ts cases pinning the wait behavior — cold-load renewer race, hot-path short-circuit, timeout without registration, and the pre-existing "no renewer" case now short-circuits the wait via a spy. All 51 tests across TokenService + AuthProviders pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| * The 10s cap covers slow lazy-chunk loads on poor networks without | ||
| * hanging indefinitely if the authenticator module fails to load. | ||
| */ | ||
| async awaitRenewerReady(maxWaitMs = 10_000, pollMs = 100): Promise<void> { |
There was a problem hiding this comment.
Renewer timeout still destroys session
When the first lazy authenticator load takes longer than ten seconds, awaitRenewerReady returns while renewToken is still absent, causing the refresh to resolve null. The 401 interceptor treats that as terminal renewal failure, clears the session, and forces the user to sign in even though the authenticator can still finish loading and renew the token.
Knowledge Base Used: OpenMetadata React UI
| @@ -141,6 +141,13 @@ class TokenService { | |||
|
|
|||
| // Call renewal method according to the provider | |||
| async fetchNewToken() { | |||
There was a problem hiding this comment.
💡 Edge Case: 10s renewer wait can stall refresh and widen cross-tab race
fetchNewToken now awaits awaitRenewerReady (default 10s) after performRefresh has already set REFRESH_IN_PROGRESS_KEY. If the lazy authenticator chunk fails to load, every refresh call (interceptor, proactive timer, visibility handler) blocks the full 10s before returning null and force-logging out, stalling the UI spinner. It also holds the in-progress flag for up to 10s while sibling tabs only wait ~1s in waitForTokenPersistence, so they give up and fire their own /auth/refresh, potentially rotating the refresh token and causing 401s (multi-tab is a stated non-goal, but this widens the window). Consider a smaller default cap (e.g. 3–5s) or short-circuiting the wait once the authenticator module is known to have failed.
Lower the default wait cap to reduce the stall and cross-tab race window on cold-load.:
async awaitRenewerReady(maxWaitMs = 5_000, pollMs = 100): Promise<void> {
Was this helpful? React with 👍 / 👎
|
|
Failed to cherry-pick changes to the 1.13 branch. |
|
Changes have been cherry-picked to the 2.0 branch. |
* fix(ui): silent-refresh cold-load 401 and post-refresh reauth
Two user-visible bugs in the SPA silent-refresh path, fixed with
surgical changes to AuthProvider.tsx. No architecture changes.
Bug 1 — cold-load 401 doesn't trigger silent refresh:
On tab reopen after id-token expiry, /loggedInUser returns
401 {"code":401,"message":"Expired token!"}, but the axios
interceptor's refresh path races the lazy authenticator's renewer
registration and TokenService.fetchNewToken returns null before
the network call fires. The interceptor then force-logs the user
out — no /token call ever happens, and the user lands on /signin.
Fix: proactive gate in getLoggedInUserDetails — check the stored
token's expiry BEFORE calling /loggedInUser. If expired, wait up
to 2s for the renewer to become a function on TokenService, then
call refreshToken() and only after success issue /loggedInUser.
Additional defense: in the catch block, if the error is still a
refreshable 401 (e.g. the interceptor refresh happened but the
retry failed), give the renewer one more chance and retry once.
Falls through to resetUserDetails on any failure.
Bug 2 — visibility-triggered silent refresh doesn't reauth the UI:
visibilitychange handler successfully refreshes the token after
Bug 1 has already flipped isAuthenticated=false, but nothing
runs getLoggedInUserDetails afterward. Storage is fresh, UI stays
on the sign-in page.
Fix: in the visibility handler, await the refresh, then read
useApplicationStore.getState().isAuthenticated (avoids the mount-
only useEffect's stale closure) and run getLoggedInUserDetails
if false — that flips isAuthenticated back to true and drops the
user into the authenticated app.
Both fixes are self-contained helpers (waitForRenewerReady,
ensureFreshTokenBeforeUserFetch, retryLoggedInUserAfterRenewer,
applyLoggedInUser, isRefreshableAuthError) inside AuthProvider.tsx.
No storage schema, SDK, or interceptor architecture changes; no
coordinator, no cross-tab primitive additions.
Tests: 5 new unit tests pin down the contracts the fixes rely on —
TokenService.refreshToken return-shape normalization, REFRESHABLE_
AUTH_ERRORS classification, visibility handler sequencing, and the
skip-reauth branches. All 14 pre-existing tests continue to pass
unchanged (19/19 total).
Follow-up (separate PR after release): full AuthCoordinator refactor
that owns silent-refresh, cross-tab lock, 401 queue, and focus
handling as a single module.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(ui): make cold-load refresh best-effort + extract helpers for real unit coverage
Addresses two PR-review findings:
Greptile P1 — "Renewer timeout destroys session":
Previously ensureFreshTokenBeforeUserFetch returned false on renewer-
registration timeout, and getLoggedInUserDetails called resetUserDetails()
on that false — which clears the valid refresh credential and forces
the user to /signin on merely-slow lazy authenticator load.
Fix: change the helper to Promise<void> with best-effort semantics —
if the renewer never registers, or the refresh throws, we resolve
silently. The caller then issues /loggedInUser normally and lets
the axios interceptor + catch-block retry handle a real 401 if one
arrives. A hard failure here is strictly worse than the original
bug it was trying to fix.
Gitar + Greptile P2 — "Tests duplicate production behavior":
Removed the five tautological tests that asserted against local
inline closures. Extracted the pure helpers — isRefreshableAuthError,
waitForRenewerReady, ensureFreshTokenBeforeUserFetch — from
AuthProvider.tsx into silentRefreshHelpers.ts, taking their
dependencies as parameters. Added silentRefreshHelpers.test.ts with
14 unit tests that exercise the actual exported functions, covering
every branch (including the best-effort no-throw contract that
guards against P1).
Total: 42/42 tests pass (14 pre-existing AuthProvider + 14 pre-
existing OktaAuthProvider + 14 new silentRefreshHelpers).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(ui): move the cold-load renewer wait into TokenService.fetchNewToken
Follow-up to Greptile's P1 restatement: even with the best-effort
proactive gate in getLoggedInUserDetails, a lazy authenticator load
that takes longer than the caller's wait would fall through to the
/loggedInUser request, get a 401, and the response interceptor's
refreshToken() would still return null (because renewToken isn't
registered yet) — triggering resetUserDetails(true), which clears
storage before the caller's catch could retry.
Real fix: move the renewer wait into TokenService.fetchNewToken
itself. The interceptor's refresh call now blocks briefly (up to
10s) for the lazy authenticator to register its renewer, then
performs the refresh normally. resetUserDetails(true) only fires
when the renewer truly never registers within the wait window.
This makes the fix work for every caller of tokenService.refreshToken
(interceptor, visibility handler, proactive timer, SSE stream
reconnect) — not just getLoggedInUserDetails.
Cleanup in AuthProvider.tsx:
- Removed ensureFreshTokenBeforeUserFetch call and the
retryLoggedInUserAfterRenewer catch-block retry — both were racing
the same failure mode that TokenService now handles at the source.
- Removed the silentRefreshHelpers module (no longer used in
production).
- getLoggedInUserDetails is back to a straightforward try/catch;
refresh reliability now lives in TokenService where it belongs.
Bug 2 fix (visibility-triggered reauth) is unchanged.
Tests: 4 new TokenServiceUtil.test.ts cases pinning the wait
behavior — cold-load renewer race, hot-path short-circuit, timeout
without registration, and the pre-existing "no renewer" case now
short-circuits the wait via a spy. All 51 tests across TokenService
+ AuthProviders pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
(cherry picked from commit b6b61b9)
Code Review 👍 Approved with suggestions 1 resolved / 2 findingsWell-structured hotfix for cold-load and visibility refresh bugs with comprehensive regression tests. Consider reducing the 10-second renewer wait in fetchNewToken to prevent stalling the refresh flow. 💡 Edge Case: 10s renewer wait can stall refresh and widen cross-tab race📄 openmetadata-ui/src/main/resources/ui/src/utils/Auth/TokenService/TokenServiceUtil.ts:143-157 📄 openmetadata-ui/src/main/resources/ui/src/utils/Auth/TokenService/TokenServiceUtil.ts:101-115
Lower the default wait cap to reduce the stall and cross-tab race window on cold-load.✅ 1 resolved✅ Quality: Regression tests re-implement logic instead of exercising it
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
…d 401 refresh (open-metadata#31644) Follow-up to open-metadata#31597. The hotfix added TokenService.awaitRenewerReady so the axios 401 interceptor's refresh call could wait briefly for the renewer to be registered, but the underlying race — the ref-deps useEffect that owned the registration — was never addressed. On any provider where the lazy authenticator finishes mounting AFTER the first /users/loggedInUser call has already 401'd (confidential + Google is where this reliably reproduces, but every provider is susceptible on cold-load with a slow chunk), the useEffect never re-runs (ref changes don't schedule re-renders), TokenService.renewToken stays null, awaitRenewerReady times out at 10s, and the interceptor force-logs the user out — no /api/v1/auth/refresh call is ever fired. Fix: each authenticator wrapper (BasicAuthAuthenticator, GenericAuthenticator, OidcAuthenticator, MsalAuthenticator, OktaAuthenticator, Auth0Authenticator) now calls TokenService.updateRenewToken from its own mount useEffect. That effect runs the moment useImperativeHandle populates the ref, with no dependence on the parent re-rendering — no race, deterministic registration. AuthProvider.tsx: the ref-deps useEffect is gone. updateRefreshSuccessCallback(startTokenExpiryTimer) moves to the main mount effect (that callback lives in AuthProvider's closure). TokenService.updateRenewToken now accepts `null` on cleanup, and RenewTokenCallback widens to `() => Promise<unknown>` so each provider's renewer can return its own SDK-native shape (Auth0 RenewTokenResponse, MSAL AuthenticationResult, oidc-client User, …) without a per-provider cast at the registration call site. Renewers wrapped in useCallback where they weren't already (GenericAuthenticator handleSilentSignIn, MsalAuthenticator renewIdToken, OktaAuthenticator renewToken, Auth0Authenticator renewIdToken extracted from useImperativeHandle) so the register effect's dep is stable and doesn't thrash. Tests: each authenticator's test file now mocks TokenService via jest.mock and asserts (a) the mount effect calls updateRenewToken with a function, and (b) unmount calls updateRenewToken(null). 5 new regression tests, 138/138 total across the auth surface. No behavior change on the happy path (renewer already registered in the same render cycle). Only cold-load / slow-lazy-mount scenarios where the old race lost. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d 401 refresh (#31644) Follow-up to #31597. The hotfix added TokenService.awaitRenewerReady so the axios 401 interceptor's refresh call could wait briefly for the renewer to be registered, but the underlying race — the ref-deps useEffect that owned the registration — was never addressed. On any provider where the lazy authenticator finishes mounting AFTER the first /users/loggedInUser call has already 401'd (confidential + Google is where this reliably reproduces, but every provider is susceptible on cold-load with a slow chunk), the useEffect never re-runs (ref changes don't schedule re-renders), TokenService.renewToken stays null, awaitRenewerReady times out at 10s, and the interceptor force-logs the user out — no /api/v1/auth/refresh call is ever fired. Fix: each authenticator wrapper (BasicAuthAuthenticator, GenericAuthenticator, OidcAuthenticator, MsalAuthenticator, OktaAuthenticator, Auth0Authenticator) now calls TokenService.updateRenewToken from its own mount useEffect. That effect runs the moment useImperativeHandle populates the ref, with no dependence on the parent re-rendering — no race, deterministic registration. AuthProvider.tsx: the ref-deps useEffect is gone. updateRefreshSuccessCallback(startTokenExpiryTimer) moves to the main mount effect (that callback lives in AuthProvider's closure). TokenService.updateRenewToken now accepts `null` on cleanup, and RenewTokenCallback widens to `() => Promise<unknown>` so each provider's renewer can return its own SDK-native shape (Auth0 RenewTokenResponse, MSAL AuthenticationResult, oidc-client User, …) without a per-provider cast at the registration call site. Renewers wrapped in useCallback where they weren't already (GenericAuthenticator handleSilentSignIn, MsalAuthenticator renewIdToken, OktaAuthenticator renewToken, Auth0Authenticator renewIdToken extracted from useImperativeHandle) so the register effect's dep is stable and doesn't thrash. Tests: each authenticator's test file now mocks TokenService via jest.mock and asserts (a) the mount effect calls updateRenewToken with a function, and (b) unmount calls updateRenewToken(null). 5 new regression tests, 138/138 total across the auth surface. No behavior change on the happy path (renewer already registered in the same render cycle). Only cold-load / slow-lazy-mount scenarios where the old race lost. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 1511b12)



Summary
Two user-visible SPA silent-refresh bugs, fixed with surgical changes to
AuthProvider.tsx. No architecture changes — no coordinator, no storage schema, no SDK changes. Deliberately scoped to be a low-risk pre-release hotfix; a broaderAuthCoordinatorrefactor is planned as a follow-up./users/loggedInUserreturns401 {"message":"Expired token!"}, butTokenService.fetchNewTokenreturns null before the network refresh fires because the lazy authenticator's renewer isn't yet registered. The interceptor then force-logs the user out — no/tokencall ever happens.visibilitychangesuccessfully refreshes the token, but nothing callsgetLoggedInUserDetailsafterward. Storage is fresh, UI stays on/signin.Reported symptom (Azure OIDC public flow, but the mechanism is provider-agnostic):
Root cause
Both bugs share one root cause: the lazy authenticator wrapper (MSAL/Okta/Auth0/OIDC/Basic/Generic) registers
renewTokenonTokenServicevia a mount effect that races the code paths above. When they run first, refresh returnsnulland the user is force-logged-out.Fix
In
getLoggedInUserDetails— proactive gate before/loggedInUser:TokenService.renewTokento become a function.refreshToken()and only then issue/loggedInUser.resetUserDetails(unchanged behavior).In the
visibilitychangehandler:await tokenService.current?.refreshToken().useApplicationStore.getState().isAuthenticated(avoids the mount-only effect's stale closure) and rungetLoggedInUserDetailsiffalse— flipsisAuthenticatedback totrue.Files changed
openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx(+118/-40)openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.test.tsx(+133)Tests
TokenService.refreshTokenreturn-shape normalization (raw string vsAccessTokenResponse).REFRESHABLE_AUTH_ERRORSclassifiesExpired token!andToken signing key not foundas refreshable.!isAuthenticated.AuthProvider.test.tsxtests continue to pass unchanged (19/19 total).yarn prettier --write,yarn eslint --fix,npx tsc --noEmitall clean; the one pre-existing tsc error in this file (ROUTES.AUTH_CALLBACK/SILENT_CALLBACKliteral-type mismatch) exists onmaintoo.Manual smoke matrix
Reviewers, please tick these off. Regression scenarios per SSO provider:
Non-goals (intentional — pushed to follow-up)
AuthCoordinatorrefactor — that PR follows this one after the release cycle and supersedes this hotfix's logic.Follow-up refactor branch
A larger
AuthCoordinatorrefactor branch (azure-oidc-session-invalidated-on-restart) already exists — it extracts silent-refresh, cross-tab lock, 401 queue, and focus handling into a single module. It supersedes this hotfix's logic and will rebase onto main once this merges.🤖 Generated with Claude Code
Greptile Summary
The PR moves cold-load renewer synchronization into TokenService and awaits visibility-triggered refresh before restoring authenticated UI state.
Confidence Score: 4/5
The PR is not yet safe to merge because a first lazy-authenticator load exceeding the new ten-second deadline still destroys an otherwise renewable session.
The replacement wait moves the existing race into TokenService but still returns null when registration misses a fixed deadline, and the interceptor interprets that result as terminal renewal failure and force-logs the user out.
Files Needing Attention: openmetadata-ui/src/main/resources/ui/src/utils/Auth/TokenService/TokenServiceUtil.ts, openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx
Important Files Changed
Sequence Diagram
sequenceDiagram participant API as loggedInUser request participant I as Axios interceptor participant T as TokenService participant A as Lazy authenticator participant UI as AuthProvider API-->>I: 401 expired token I->>T: refreshToken() T->>T: awaitRenewerReady(10s) alt authenticator registers in time A->>T: updateRenewToken() T->>A: renewToken() A-->>T: refreshed token T-->>I: token I->>API: retry queued request else registration exceeds timeout T-->>I: null I->>UI: resetUserDetails(true) UI->>UI: clear session and navigate to signin endReviews (3): Last reviewed commit: "fix(ui): move the cold-load renewer wait..." | Re-trigger Greptile
Context used: