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 = '') => {