Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions apps/console/src/routes/login/auth0-callback.tsx
Original file line number Diff line number Diff line change
@@ -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 = {
Expand Down Expand Up @@ -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) {
Expand Down
20 changes: 11 additions & 9 deletions apps/console/src/routes/login/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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<T>(values: T[]) {
const shuffled = [...values]
for (let i = shuffled.length - 1; i > 0; i -= 1) {
Expand Down Expand Up @@ -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) })
}
},
Expand Down Expand Up @@ -492,6 +487,13 @@ function RouteComponent() {
)}
</AnimatePresence>

{search.reason === SESSION_EXPIRED_REASON && (
<div className="mt-4 rounded-md border border-warning-component bg-surface-warning-subtle p-3">
<p className="text-sm font-medium text-warning">Your session has expired</p>
<p className="mt-1 text-sm text-neutral-subtle">Please log in again to continue.</p>
</div>
)}

{auth0Error && (
<div className="mt-4 rounded-md border border-negative-component bg-surface-negative-subtle p-3">
<p className="text-sm font-medium text-negative">{auth0Error.error}</p>
Expand Down
2 changes: 1 addition & 1 deletion libs/shared/routes/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
42 changes: 42 additions & 0 deletions libs/shared/routes/src/lib/sub-router/login.router.spec.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
})
28 changes: 28 additions & 0 deletions libs/shared/routes/src/lib/sub-router/login.router.ts
Original file line number Diff line number Diff line change
@@ -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)) {

@cubic-dev-ai cubic-dev-ai Bot Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
if (!redirectPath || redirectPath.startsWith(LOGIN_URL)) {
if (!redirectPath || isLoginPath(redirectPath.split(/[?#]/)[0])) {
Fix with cubic

return '/'
}

return redirectPath
}

export function shouldRedirectAuthenticatedUser({
isAuthenticated,
reason,
}: {
isAuthenticated: boolean
reason?: string
}) {
return isAuthenticated && reason !== SESSION_EXPIRED_REASON
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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<void>((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)
})

Expand All @@ -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 {
Expand Down
Loading
Loading