From 1a675016c266a81f48e5ee829822518be052b7f4 Mon Sep 17 00:00:00 2001 From: Antoine Date: Wed, 26 Aug 2026 16:03:46 +0200 Subject: [PATCH] fix(auth): stop the /login redirect loop left by a dead Auth0 session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 into a beforeLoad guard, and memoising the Auth0 context value. None are required for this loop. --- .../src/routes/login/auth0-callback.tsx | 4 - apps/console/src/routes/login/index.tsx | 20 +-- libs/shared/routes/jest.config.ts | 2 +- .../src/lib/sub-router/login.router.spec.ts | 42 +++++ .../routes/src/lib/sub-router/login.router.ts | 28 ++++ .../auth-interceptor/auth-interceptor.spec.ts | 148 +++++++++++++++++- .../auth-interceptor/auth-interceptor.tsx | 74 +++++++-- 7 files changed, 290 insertions(+), 28 deletions(-) create mode 100644 libs/shared/routes/src/lib/sub-router/login.router.spec.ts diff --git a/apps/console/src/routes/login/auth0-callback.tsx b/apps/console/src/routes/login/auth0-callback.tsx index 984fe70a306..528c5f921b0 100644 --- a/apps/console/src/routes/login/auth0-callback.tsx +++ b/apps/console/src/routes/login/auth0-callback.tsx @@ -1,14 +1,11 @@ import { useAuth0 } from '@auth0/auth0-react' import { Navigate, createFileRoute, useNavigate } from '@tanstack/react-router' -import axios from 'axios' import { useEffect } from 'react' import { useOrganizations } from '@qovery/domains/organizations/feature' import { useUserSignUp } from '@qovery/domains/users-sign-up/feature' import { useAuth } from '@qovery/shared/auth' import { getOnboardingEntryUrl } from '@qovery/shared/routes' import { LoadingScreen } from '@qovery/shared/ui' -import { QOVERY_API } from '@qovery/shared/util-node-env' -import { useAuthInterceptor } from '@qovery/shared/utils' import { consumePendingReturnTo } from '../../auth/auth0' type Auth0CallbackSearch = { @@ -84,7 +81,6 @@ function useRedirectIfLogged(connection?: string) { function PageRedirectLogin() { const { connection, error, error_description } = Route.useSearch() - useAuthInterceptor(axios, QOVERY_API) useRedirectIfLogged(connection) if (error != null) { diff --git a/apps/console/src/routes/login/index.tsx b/apps/console/src/routes/login/index.tsx index 55bf2a37bf3..06908b65ade 100644 --- a/apps/console/src/routes/login/index.tsx +++ b/apps/console/src/routes/login/index.tsx @@ -6,6 +6,7 @@ import { Controller, FormProvider, useForm } from 'react-hook-form' import { z } from 'zod' import { AuthEnum, useAuth } from '@qovery/shared/auth' import { IconEnum } from '@qovery/shared/enums' +import { SESSION_EXPIRED_REASON, getSafeRedirect, shouldRedirectAuthenticatedUser } from '@qovery/shared/routes' import { Badge, Button, Icon, InputTextSmall, Link } from '@qovery/shared/ui' import { useLocalStorage } from '@qovery/shared/util-hooks' @@ -120,16 +121,10 @@ const TESTIMONIALS = [ const loginSearchParamsSchema = z.object({ redirect: z.string().optional(), + // .catch keeps an unrecognised reason from failing search validation and breaking the page + reason: z.literal(SESSION_EXPIRED_REASON).optional().catch(undefined), }) -function getSafeRedirect(redirectPath?: string) { - if (!redirectPath || redirectPath.startsWith('/login')) { - return '/' - } - - return redirectPath -} - function shuffleArray(values: T[]) { const shuffled = [...values] for (let i = shuffled.length - 1; i > 0; i -= 1) { @@ -180,7 +175,7 @@ function useAuth0Error() { export const Route = createFileRoute('/login/')({ validateSearch: loginSearchParamsSchema, beforeLoad: ({ context, search }) => { - if (context.auth.isAuthenticated) { + if (shouldRedirectAuthenticatedUser({ isAuthenticated: context.auth.isAuthenticated, reason: search.reason })) { throw redirect({ to: getSafeRedirect(search.redirect) }) } }, @@ -492,6 +487,13 @@ function RouteComponent() { )} + {search.reason === SESSION_EXPIRED_REASON && ( +
+

Your session has expired

+

Please log in again to continue.

+
+ )} + {auth0Error && (

{auth0Error.error}

diff --git a/libs/shared/routes/jest.config.ts b/libs/shared/routes/jest.config.ts index 5c858e700fd..f4a8616264a 100644 --- a/libs/shared/routes/jest.config.ts +++ b/libs/shared/routes/jest.config.ts @@ -4,7 +4,7 @@ export default { preset: '../../../jest.preset.js', transform: { '^(?!.*\\.(js|jsx|ts|tsx|css|json)$)': '@nx/react/plugins/jest', - '^.+\\.[tj]sx?$': 'babel-jest', + '^.+\\.[tj]sx?$': ['babel-jest', { presets: ['@nx/react/babel'] }], }, moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx'], coverageDirectory: '../../../coverage/libs/shared/routes', diff --git a/libs/shared/routes/src/lib/sub-router/login.router.spec.ts b/libs/shared/routes/src/lib/sub-router/login.router.spec.ts new file mode 100644 index 00000000000..67c13ecc413 --- /dev/null +++ b/libs/shared/routes/src/lib/sub-router/login.router.spec.ts @@ -0,0 +1,42 @@ +import { SESSION_EXPIRED_REASON, getSafeRedirect, isLoginPath, shouldRedirectAuthenticatedUser } from './login.router' + +describe('login router', () => { + describe('isLoginPath', () => { + it.each(['/login', '/login/auth0-callback'])('should match %s', (pathname) => { + expect(isLoginPath(pathname)).toBe(true) + }) + + it.each(['/', '/organization/123', '/loginish'])('should not match %s', (pathname) => { + expect(isLoginPath(pathname)).toBe(false) + }) + }) + + describe('getSafeRedirect', () => { + it('should fall back to the root when there is no redirect', () => { + expect(getSafeRedirect(undefined)).toBe('/') + expect(getSafeRedirect('')).toBe('/') + }) + + it('should refuse a redirect that points back at the login page', () => { + expect(getSafeRedirect('/login?redirect=%2F')).toBe('/') + }) + + it('should keep an in-app redirect', () => { + expect(getSafeRedirect('/organization/123/overview')).toBe('/organization/123/overview') + }) + }) + + describe('shouldRedirectAuthenticatedUser', () => { + it('should send an authenticated visitor back into the app', () => { + expect(shouldRedirectAuthenticatedUser({ isAuthenticated: true })).toBe(true) + }) + + it('should keep an anonymous visitor on the login page', () => { + expect(shouldRedirectAuthenticatedUser({ isAuthenticated: false })).toBe(false) + }) + + it('should keep a visitor whose session expired on the login page even though Auth0 still reports them as authenticated', () => { + expect(shouldRedirectAuthenticatedUser({ isAuthenticated: true, reason: SESSION_EXPIRED_REASON })).toBe(false) + }) + }) +}) diff --git a/libs/shared/routes/src/lib/sub-router/login.router.ts b/libs/shared/routes/src/lib/sub-router/login.router.ts index 6cbc7dda60b..d6988e57c13 100644 --- a/libs/shared/routes/src/lib/sub-router/login.router.ts +++ b/libs/shared/routes/src/lib/sub-router/login.router.ts @@ -1,3 +1,31 @@ export const LOGIN_URL = '/login' export const LOGIN_AUTH_REDIRECT_URL = '/auth0-callback' export const LOGOUT_URL = '/logout' + +// Marks a redirect to /login that was forced by an unusable session. The login page must not +// bounce such a visit back into the app: Auth0 caches the user in localStorage without an expiry +// check, so `isAuthenticated` can stay true for a session that can no longer mint a token, and +// the two redirects chase each other forever. +export const SESSION_EXPIRED_REASON = 'session-expired' + +export function isLoginPath(pathname: string) { + return pathname === LOGIN_URL || pathname.startsWith(`${LOGIN_URL}/`) +} + +export function getSafeRedirect(redirectPath?: string) { + if (!redirectPath || redirectPath.startsWith(LOGIN_URL)) { + return '/' + } + + return redirectPath +} + +export function shouldRedirectAuthenticatedUser({ + isAuthenticated, + reason, +}: { + isAuthenticated: boolean + reason?: string +}) { + return isAuthenticated && reason !== SESSION_EXPIRED_REASON +} diff --git a/libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.spec.ts b/libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.spec.ts index d87de5bf51c..8502937b44b 100644 --- a/libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.spec.ts +++ b/libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.spec.ts @@ -3,18 +3,32 @@ import axios, { AxiosHeaders, type AxiosInstance } from 'axios' import { buildLoginRedirectUrl, useAuthInterceptor } from './auth-interceptor' const mockGetAccessTokenSilently = jest.fn() +const mockLogout = jest.fn() jest.mock('@auth0/auth0-react', () => ({ useAuth0: () => { return { getAccessTokenSilently: mockGetAccessTokenSilently, + logout: mockLogout, } }, })) +function navigateTo(pathname: string) { + window.history.pushState({}, '', pathname) +} + +// The auth failure handler is deliberately async (it awaits the session teardown), so the +// assertions have to let the microtask queue drain before inspecting the spies. +function flushAsyncWork() { + return new Promise((resolve) => setTimeout(resolve, 0)) +} + describe('UseAuthInterceptor', () => { beforeEach(() => { mockGetAccessTokenSilently.mockResolvedValue('someAuthToken') + mockLogout.mockResolvedValue(undefined) + navigateTo('/') }) afterEach(() => { @@ -49,13 +63,93 @@ describe('UseAuthInterceptor', () => { const navigateToLogin = jest.fn() mockGetAccessTokenSilently.mockRejectedValue(authError) - renderHook(() => useAuthInterceptor(axiosInstance, 'https://api.qovery.com', navigateToLogin)) + renderHook(() => useAuthInterceptor(axiosInstance, 'https://api.qovery.com', { navigateToLogin })) + + const requestHandler = requestUse.mock.calls[0][0] + + await expect(requestHandler({ url: '/organizations', headers: new AxiosHeaders() })).rejects.toThrow( + 'login_required' + ) + expect(navigateToLogin).toHaveBeenCalledTimes(1) + }) + + it('should clear the Auth0 session when silent token renewal fails', async () => { + const requestUse = jest.fn().mockReturnValue(1) + const responseUse = jest.fn().mockReturnValue(2) + const axiosInstance = createAxiosInstanceMock(requestUse, responseUse) + const navigateToLogin = jest.fn() + mockGetAccessTokenSilently.mockRejectedValue(new Error('login_required')) + + renderHook(() => useAuthInterceptor(axiosInstance, 'https://api.qovery.com', { navigateToLogin })) const requestHandler = requestUse.mock.calls[0][0] await expect(requestHandler({ url: '/organizations', headers: new AxiosHeaders() })).rejects.toThrow( 'login_required' ) + expect(mockLogout).toHaveBeenCalledWith({ openUrl: false }) + }) + + it('should clear the session before navigating away', async () => { + let releaseLogout = () => { + /* replaced below */ + } + mockLogout.mockReturnValue( + new Promise((resolve) => { + releaseLogout = resolve + }) + ) + const requestUse = jest.fn().mockReturnValue(1) + const responseUse = jest.fn().mockReturnValue(2) + const axiosInstance = createAxiosInstanceMock(requestUse, responseUse) + const navigateToLogin = jest.fn() + mockGetAccessTokenSilently.mockRejectedValue(new Error('login_required')) + + renderHook(() => useAuthInterceptor(axiosInstance, 'https://api.qovery.com', { navigateToLogin })) + + const requestHandler = requestUse.mock.calls[0][0] + const pending = requestHandler({ url: '/organizations', headers: new AxiosHeaders() }).catch(() => undefined) + + await flushAsyncWork() + // A reload that outruns the cache wipe restores the dead session and the loop survives + expect(navigateToLogin).not.toHaveBeenCalled() + + releaseLogout() + await pending + + expect(navigateToLogin).toHaveBeenCalledTimes(1) + }) + + it('should clear the session only once when several requests fail concurrently', async () => { + const requestUseA = jest.fn().mockReturnValue(1) + const requestUseB = jest.fn().mockReturnValue(1) + const navigateToLogin = jest.fn() + mockGetAccessTokenSilently.mockRejectedValue(new Error('login_required')) + + // main.tsx registers the interceptor on two axios instances, and React Query retries each + // failed query three times, so a single dead session produces a burst of failures + renderHook(() => + useAuthInterceptor(createAxiosInstanceMock(requestUseA, jest.fn().mockReturnValue(2)), 'https://api.qovery.com', { + navigateToLogin, + }) + ) + renderHook(() => + useAuthInterceptor( + createAxiosInstanceMock(requestUseB, jest.fn().mockReturnValue(2)), + 'https://copilot.qovery.com', + { + navigateToLogin, + } + ) + ) + + await Promise.allSettled([ + requestUseA.mock.calls[0][0]({ url: '/organizations', headers: new AxiosHeaders() }), + requestUseB.mock.calls[0][0]({ url: '/messages', headers: new AxiosHeaders() }), + ]) + await flushAsyncWork() + + expect(mockLogout).toHaveBeenCalledTimes(1) expect(navigateToLogin).toHaveBeenCalledTimes(1) }) @@ -66,22 +160,72 @@ describe('UseAuthInterceptor', () => { const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation() const navigateToLogin = jest.fn() - renderHook(() => useAuthInterceptor(axiosInstance, 'https://api.qovery.com', navigateToLogin)) + renderHook(() => useAuthInterceptor(axiosInstance, 'https://api.qovery.com', { navigateToLogin })) const responseErrorHandler = responseUse.mock.calls[0][1] await expect(responseErrorHandler({ response: { status: 401, data: { status: 401 } } })).rejects.toMatchObject({ code: '401', }) + await flushAsyncWork() + expect(consoleErrorSpy).toHaveBeenCalledWith('Error', undefined) expect(navigateToLogin).toHaveBeenCalledTimes(1) }) + it('should not clear the session when the API returns unauthorized', async () => { + const requestUse = jest.fn().mockReturnValue(1) + const responseUse = jest.fn().mockReturnValue(2) + const axiosInstance = createAxiosInstanceMock(requestUse, responseUse) + jest.spyOn(console, 'error').mockImplementation() + const navigateToLogin = jest.fn() + + renderHook(() => useAuthInterceptor(axiosInstance, 'https://api.qovery.com', { navigateToLogin })) + + const responseErrorHandler = responseUse.mock.calls[0][1] + + await expect(responseErrorHandler({ response: { status: 401, data: { status: 401 } } })).rejects.toMatchObject({ + code: '401', + }) + await flushAsyncWork() + + // A single endpoint answering 401 is not proof the session is dead, so signing the user out + // here would log out a healthy user. The session-expired reason breaks the loop instead. + expect(mockLogout).not.toHaveBeenCalled() + expect(navigateToLogin).toHaveBeenCalledTimes(1) + }) + + it('should not clear the session nor navigate while already on a login page', async () => { + // getAccessTokenSilently can reject transiently while the Auth0 callback is still in flight + navigateTo('/login/auth0-callback') + const requestUse = jest.fn().mockReturnValue(1) + const responseUse = jest.fn().mockReturnValue(2) + const axiosInstance = createAxiosInstanceMock(requestUse, responseUse) + const navigateToLogin = jest.fn() + mockGetAccessTokenSilently.mockRejectedValue(new Error('login_required')) + + renderHook(() => useAuthInterceptor(axiosInstance, 'https://api.qovery.com', { navigateToLogin })) + + const requestHandler = requestUse.mock.calls[0][0] + + await expect(requestHandler({ url: '/organizations', headers: new AxiosHeaders() })).rejects.toThrow( + 'login_required' + ) + await flushAsyncWork() + + expect(mockLogout).not.toHaveBeenCalled() + expect(navigateToLogin).not.toHaveBeenCalled() + }) + it('should build the login redirect url from the current location', () => { expect(buildLoginRedirectUrl('/organization/123', '?tab=clusters', '#overview')).toBe( '/login?redirect=%2Forganization%2F123%3Ftab%3Dclusters%23overview' ) }) + + it('should tag the login redirect url with the reason the session ended', () => { + expect(buildLoginRedirectUrl('/', '', '', 'session-expired')).toBe('/login?redirect=%2F&reason=session-expired') + }) }) function createAxiosInstanceMock(requestUse: jest.Mock, responseUse: jest.Mock): AxiosInstance { diff --git a/libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.tsx b/libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.tsx index fa9ce7bd2b0..db42b3b308e 100644 --- a/libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.tsx +++ b/libs/shared/utils/src/lib/http/interceptors/auth-interceptor/auth-interceptor.tsx @@ -1,6 +1,7 @@ import { useAuth0 } from '@auth0/auth0-react' import { type AxiosInstance, type AxiosResponse } from 'axios' -import { useEffect } from 'react' +import { useCallback, useEffect } from 'react' +import { SESSION_EXPIRED_REASON, isLoginPath } from '@qovery/shared/routes' import { NODE_ENV } from '@qovery/shared/util-node-env' export interface SerializedError { @@ -11,27 +12,70 @@ export interface SerializedError { response?: AxiosResponse } +export interface AuthInterceptorOptions { + navigateToLogin?: () => void + clearSession?: () => Promise +} + const E2E_AUTH_TOKEN_STORAGE_KEY = 'qovery-e2e-auth-token' -export function buildLoginRedirectUrl(pathname: string, search: string, hash: string) { +export function buildLoginRedirectUrl(pathname: string, search: string, hash: string, reason?: string) { const redirect = `${pathname}${search}${hash}` const searchParams = new URLSearchParams({ redirect }) + if (reason) { + searchParams.set('reason', reason) + } + return `/login?${searchParams.toString()}` } function redirectToLogin() { - if (window.location.pathname.startsWith('/login')) return + window.location.assign( + buildLoginRedirectUrl( + window.location.pathname, + window.location.search, + window.location.hash, + SESSION_EXPIRED_REASON + ) + ) +} + +// A dead session produces a burst of failures — two axios instances, three React Query retries per +// query — and each one would otherwise clear the session and reload the page on its own. +let pendingAuthFailure: Promise | null = null - window.location.assign(buildLoginRedirectUrl(window.location.pathname, window.location.search, window.location.hash)) +function handleAuthFailure(clearSession: (() => Promise) | undefined, navigateToLogin: () => void) { + // Bailing out here rather than inside navigateToLogin also protects the Auth0 callback, where a + // 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) + .then(() => { + navigateToLogin() + }) + .finally(() => { + pendingAuthFailure = null + }) + + return pendingAuthFailure } -export function useAuthInterceptor( - axiosInstance: AxiosInstance, - apiUrl: string, - navigateToLogin: () => void = redirectToLogin -) { - const { getAccessTokenSilently } = useAuth0() +export function useAuthInterceptor(axiosInstance: AxiosInstance, apiUrl: string, options: AuthInterceptorOptions = {}) { + const { getAccessTokenSilently, logout } = useAuth0() + // Destructured out of `options` so callers that pass no options don't re-register on every render + const { navigateToLogin = redirectToLogin, clearSession } = options + + const clearAuth0Session = useCallback(async () => { + if (clearSession) { + await clearSession() + return + } + // Wipes the cached user Auth0 keeps in localStorage, without a round-trip to the IdP + await logout({ openUrl: false }) + }, [clearSession, logout]) useEffect(() => { const requestInterceptor = axiosInstance.interceptors.request.use(async (config) => { @@ -44,7 +88,9 @@ export function useAuthInterceptor( try { token = token || (await getAccessTokenSilently()) } catch (e) { - navigateToLogin() + // Auth0 refusing to mint a token is definitive, so the session goes. Awaiting matters: a + // reload that outruns the teardown restores the dead session and the loop survives. + await handleAuthFailure(clearAuth0Session, navigateToLogin) return Promise.reject(e) } @@ -67,7 +113,10 @@ export function useAuthInterceptor( } if (error.response?.status === 401) { - navigateToLogin() + // No session teardown here: getAccessTokenSilently already refreshes on expiry, so a 401 + // is as likely to be one endpoint using 401 for 403 as it is a dead credential. The + // session-expired reason on the login URL is what keeps this from looping. + void handleAuthFailure(undefined, navigateToLogin) } // we reformat the error output to improve the dev experience @@ -93,6 +142,7 @@ export function useAuthInterceptor( apiUrl, getAccessTokenSilently, navigateToLogin, + clearAuth0Session, ]) const removeBaseUrl = (url = '') => {