fix(auth): stop the /login redirect loop left by a dead Auth0 session - #2906
fix(auth): stop the /login redirect loop left by a dead Auth0 session#2906Astach wants to merge 1 commit into
Conversation
What:
When Auth0 cannot mint a token, the request interceptor now clears the local
session before redirecting, and it tags the login URL with
`reason=session-expired`. The login route skips its "already authenticated, go
back into the app" redirect when that reason is present, and shows the user why
they were signed out. Session teardown is single-flight across every axios
instance. The duplicate `useAuthInterceptor` registration on the Auth0 callback
route is removed. `getSafeRedirect` and the new login-URL contract move into
`@qovery/shared/routes` so both sides share one definition.
Why:
`Auth0Provider` runs with `cacheLocation="localstorage"`, and the SDK reads the
cached user back with no expiry check — `checkSession()` swallows the refresh
failure. So `isAuthenticated` stays true for a session that can no longer
produce a token, and nothing ever clears it. That closed a loop: at `/` a query
fires, the interceptor fails to get a token and does
`window.location.assign('/login?redirect=%2F')` (a full page reload), the reload
restores the same dead session, `/login` sees `isAuthenticated` and redirects to
`/`, and round it goes. The tab is unrecoverable without manually wiping
localStorage. Introduced by #2727, which added the redirect but no teardown.
Notes:
Teardown is awaited before navigating: a reload that outruns the cache wipe
restores the dead session and the loop survives. That ordering is covered by a
test.
A 401 response does NOT clear the session, only tags the URL. Because
`getAccessTokenSilently` refreshes proactively, an expired token never reaches
the API, so a 401 is as likely to be an endpoint using 401 where it means 403 —
clearing there would sign out a healthy user. The reason param is what breaks
the loop on that path.
Single-flight matters in practice, not just in theory: two axios instances plus
React Query's default three retries turn one dead session into a dozen
concurrent teardown attempts.
The reason param, not the teardown, is also the only guard that works for the
`qovery-e2e-auth-token` bypass, which forces `isAuthenticated` true and is
immune to `logout()`.
`libs/shared/routes` had a jest transform that could not parse the shared
TypeScript setup file, so its suite could never run; aligned it with
`shared-utils`.
Out of scope, worth a follow-up: `router.invalidate()` on auth change, moving
`/`'s component-level <Navigate> into a beforeLoad guard, and memoising the
Auth0 context value. None are required for this loop.
|
View your CI Pipeline Execution ↗ for commit 1a67501
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
2 issues found across 7 files
Confidence score: 3/5
- In
libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.tsx, the shared 401 promise lacks a teardown, so a silent-token failure joins it and never callsclearAuth0Session— the app navigates to login with a stale session. Add teardown logic to the shared promise so token failures always trigger session cleanup. - In
libs/shared/routes/src/lib/sub-router/login.router.ts, the redirect condition matches a raw/loginprefix, so valid routes like/login/fooincorrectly route to/. Use a segment-aware check (asisLoginPathdoes) after stripping the leading slash to avoid misfiring.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libs/shared/routes/src/lib/sub-router/login.router.ts">
<violation number="1" location="libs/shared/routes/src/lib/sub-router/login.router.ts:16">
P2: When a redirect targets a valid path beginning with `/login` but not the login route, this condition sends it to `/` because it tests a raw prefix. Use the same segment-aware check as `isLoginPath` after stripping the query and hash.</violation>
</file>
<file name="libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.tsx">
<violation number="1" location="libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.tsx:53">
P1: When a 401 reaches this handler before a silent-token failure, the shared promise is created without a teardown, so the token failure joins it and never calls `clearAuth0Session`. The app navigates to login while retaining the dead Auth0 cache, allowing the redirect loop to recur. Track navigation single-flight separately from session teardown, or upgrade an in-flight failure when a token-mint failure requires cleanup.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| // token request can reject while the session is still being created. | ||
| if (isLoginPath(window.location.pathname)) return Promise.resolve() | ||
|
|
||
| pendingAuthFailure ??= Promise.resolve() |
There was a problem hiding this comment.
P1: When a 401 reaches this handler before a silent-token failure, the shared promise is created without a teardown, so the token failure joins it and never calls clearAuth0Session. The app navigates to login while retaining the dead Auth0 cache, allowing the redirect loop to recur. Track navigation single-flight separately from session teardown, or upgrade an in-flight failure when a token-mint failure requires cleanup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.tsx, line 53:
<comment>When a 401 reaches this handler before a silent-token failure, the shared promise is created without a teardown, so the token failure joins it and never calls `clearAuth0Session`. The app navigates to login while retaining the dead Auth0 cache, allowing the redirect loop to recur. Track navigation single-flight separately from session teardown, or upgrade an in-flight failure when a token-mint failure requires cleanup.</comment>
<file context>
@@ -11,27 +12,70 @@ export interface SerializedError {
+ // token request can reject while the session is still being created.
+ if (isLoginPath(window.location.pathname)) return Promise.resolve()
+
+ pendingAuthFailure ??= Promise.resolve()
+ .then(() => clearSession?.())
+ .catch(() => undefined)
</file context>
| } | ||
|
|
||
| export function getSafeRedirect(redirectPath?: string) { | ||
| if (!redirectPath || redirectPath.startsWith(LOGIN_URL)) { |
There was a problem hiding this comment.
P2: When a redirect targets a valid path beginning with /login but not the login route, this condition sends it to / because it tests a raw prefix. Use the same segment-aware check as isLoginPath after stripping the query and hash.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/shared/routes/src/lib/sub-router/login.router.ts, line 16:
<comment>When a redirect targets a valid path beginning with `/login` but not the login route, this condition sends it to `/` because it tests a raw prefix. Use the same segment-aware check as `isLoginPath` after stripping the query and hash.</comment>
<file context>
@@ -1,3 +1,31 @@
+}
+
+export function getSafeRedirect(redirectPath?: string) {
+ if (!redirectPath || redirectPath.startsWith(LOGIN_URL)) {
+ return '/'
+ }
</file context>
| if (!redirectPath || redirectPath.startsWith(LOGIN_URL)) { | |
| if (!redirectPath || isLoginPath(redirectPath.split(/[?#]/)[0])) { |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## staging #2906 +/- ##
===========================================
+ Coverage 42.11% 48.18% +6.06%
===========================================
Files 248 1235 +987
Lines 7285 26650 +19365
Branches 2258 7912 +5654
===========================================
+ Hits 3068 12840 +9772
- Misses 3659 11643 +7984
- Partials 558 2167 +1609
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What:
When Auth0 cannot mint a token, the request interceptor now clears the local session before redirecting, and it tags the login URL with
reason=session-expired. The login route skips its "already authenticated, go back into the app" redirect when that reason is present, and shows the user why they were signed out. Session teardown is single-flight across every axios instance. The duplicateuseAuthInterceptorregistration on the Auth0 callback route is removed.getSafeRedirectand the new login-URL contract move into@qovery/shared/routesso both sides share one definition.Why:
Auth0Providerruns withcacheLocation="localstorage", and the SDK reads the cached user back with no expiry check —checkSession()swallows the refresh failure. SoisAuthenticatedstays true for a session that can no longer produce a token, and nothing ever clears it. That closed a loop: at/a query fires, the interceptor fails to get a token and doeswindow.location.assign('/login?redirect=%2F')(a full page reload), the reload restores the same dead session,/loginseesisAuthenticatedand redirects to/, and round it goes. The tab is unrecoverable without manually wiping localStorage. Introduced by #2727, which added the redirect but no teardown.Notes:
Teardown is awaited before navigating: a reload that outruns the cache wipe restores the dead session and the loop survives. That ordering is covered by a test.
A 401 response does NOT clear the session, only tags the URL. Because
getAccessTokenSilentlyrefreshes proactively, an expired token never reaches the API, so a 401 is as likely to be an endpoint using 401 where it means 403 — clearing there would sign out a healthy user. The reason param is what breaks the loop on that path.Single-flight matters in practice, not just in theory: two axios instances plus React Query's default three retries turn one dead session into a dozen concurrent teardown attempts.
The reason param, not the teardown, is also the only guard that works for the
qovery-e2e-auth-tokenbypass, which forcesisAuthenticatedtrue and is immune tologout().libs/shared/routeshad a jest transform that could not parse the shared TypeScript setup file, so its suite could never run; aligned it withshared-utils.Out of scope, worth a follow-up:
router.invalidate()on auth change, moving/'s component-level into a beforeLoad guard, and memoising the Auth0 context value. None are required for this loop.Summary by cubic
Fixes the
/loginredirect loop that traps users with a dead Auth0 session — one whereisAuthenticatedstays true but tokens can no longer be minted. The auth interceptor now clears the local Auth0 session before redirecting to/loginand tags the URL withreason=session-expired; the login page skips its "already authenticated" redirect when that reason is present and shows an expiry notice. Both sides now share the login-URL contract via@qovery/shared/routes.Behavior changes
qovery-e2e-auth-tokenbypass is guarded by the reason param, since it's immune tologout().useAuthInterceptorregistration on the Auth0 callback route.@qovery/shared/routes' jest transform, which had silently prevented its suite from running.Written for commit 1a67501. Summary will update on new commits.