diff --git a/client/e2e/auth.spec.ts b/client/e2e/auth.spec.ts index cd2c0ab..41a9c60 100644 --- a/client/e2e/auth.spec.ts +++ b/client/e2e/auth.spec.ts @@ -2,6 +2,10 @@ import { expect, test } from '@playwright/test'; // These tests cover the unauthenticated flows; they run without a saved // session (see the 'auth-specs' project in playwright.config.ts). +// Several of them perform real logins, which overwrite the user's single +// stored refresh token server-side — so they must not overlap. +test.describe.configure({ mode: 'serial' }); + test.describe('Authentication Flow', () => { test.beforeEach(async ({ page }) => { await page.goto('/'); @@ -68,6 +72,48 @@ test.describe('Authentication Flow', () => { await expect(page.getByRole('menuitem', { name: /logout/i })).toBeVisible(); }); + test('should keep the session when logging back in and reloading', async ({ + page, + }) => { + const login = async () => { + await page.goto('/login'); + await page.getByLabel(/email/i).fill('test@example.com'); + await page.getByLabel(/password/i).fill('testpassword'); + await page.getByRole('button', { name: /login|sign in/i }).click(); + await expect(page).toHaveURL(/.*\/app/); + }; + + await login(); + + // Log out via the UI, then log back in — all within the same SPA session. + await page.getByRole('button', { name: /user menu/i }).click(); + await page.getByRole('menuitem', { name: /logout/i }).click(); + await login(); + + // A reload must restore the session from the refresh cookie. + await page.reload(); + await expect(page).toHaveURL(/.*\/app/, { timeout: 10_000 }); + await expect(page.getByRole('button', { name: /user menu/i })).toBeVisible({ + timeout: 10_000, + }); + }); + + test('should not call authenticated logout when bootstrap has no session', async ({ + page, + }) => { + const logoutCalls: string[] = []; + page.on('request', (req) => { + if (req.url().includes('/api/auth/logout')) { + logoutCalls.push(req.url()); + } + }); + + await page.goto('/login'); + await expect(page.getByLabel(/email/i)).toBeVisible(); + + expect(logoutCalls).toEqual([]); + }); + test('should register a new account and redirect to login', async ({ page, }) => { diff --git a/client/src/context/auth/auth-provider.tsx b/client/src/context/auth/auth-provider.tsx index 55d618e..c09d908 100644 --- a/client/src/context/auth/auth-provider.tsx +++ b/client/src/context/auth/auth-provider.tsx @@ -1,19 +1,29 @@ -import { useState, useEffect } from 'react'; +import { useEffect, useState } from 'react'; -import { api } from '@/lib/api'; +import { api, onSessionExpired, refreshAccessToken } from '@/lib/api'; import { type User } from '@/types/api'; import { setAccessToken as storeToken, clearAccessToken } from '@/utils/token'; import { AuthContext } from './auth-context'; /** - * Session bootstrap: refreshes the token cookie on mount, exposes login/logout and mirrors the token into api defaults and module storage. + * Session bootstrap: restores the session from the refresh cookie on mount, + * exposes login/logout and mirrors the token into api defaults and module + * storage. A failed bootstrap simply means signed out — it never calls the + * authenticated logout endpoint. */ export const AuthProvider = ({ children }: { children: React.ReactNode }) => { const [user, setUser] = useState(null); const [accessToken, setAccessToken] = useState(null); const [loading, setLoading] = useState(true); + const markSignedOut = () => { + setAccessToken(null); + setUser(null); + delete api.defaults.headers.common['Authorization']; + clearAccessToken(); + }; + const login = (token: string, user: User) => { setAccessToken(token); setUser(user); @@ -22,33 +32,32 @@ export const AuthProvider = ({ children }: { children: React.ReactNode }) => { }; const logout = async () => { - await api.post('/auth/logout'); - setAccessToken(null); - setUser(null); - localStorage.setItem('wasLoggedOut', 'true'); - delete api.defaults.headers.common['Authorization']; - clearAccessToken(); + // The server may already be unreachable or the session expired; local + // cleanup happens either way. + try { + await api.post('/auth/logout'); + } catch { + // session already dead server-side + } + markSignedOut(); }; useEffect(() => { - const refresh = async () => { - if (localStorage.getItem('wasLoggedOut') === 'true') { - localStorage.removeItem('wasLoggedOut'); - setLoading(false); - return; - } + const bootstrap = async () => { try { - const res = await api.post('/auth/refresh'); - const { accessToken, user } = res.data; + // Shares the single-flight with the response interceptor, so a + // bootstrap racing an in-flight refresh triggers only one request. + const { accessToken, user } = await refreshAccessToken(); login(accessToken, user); } catch { - logout(); + markSignedOut(); } finally { setLoading(false); } }; - refresh(); + bootstrap(); + return onSessionExpired(markSignedOut); }, []); if (loading) return null; diff --git a/client/src/lib/__tests__/api.test.ts b/client/src/lib/__tests__/api.test.ts new file mode 100644 index 0000000..3bca57c --- /dev/null +++ b/client/src/lib/__tests__/api.test.ts @@ -0,0 +1,157 @@ +import { createServer, type Server } from 'node:http'; + +import axios from 'axios'; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest'; + +import { clearAccessToken, setAccessToken } from '@/utils/token'; + +import { api, onSessionExpired } from '../api'; + +const PORT = 4599; +const BASE = `http://localhost:${PORT}`; + +let server: Server; +let refreshCallCount = 0; +let refreshShouldFail = false; +const protectedAuthHeaders: string[] = []; + +/** + * Minimal API stub: /protected requires the *new* token, /api/auth/refresh + * issues it once. Behaves like the real server for the paths under test. + */ +async function startStub(): Promise { + server = createServer((req, res) => { + let body = ''; + req.on('data', (chunk) => (body += chunk)); + req.on('end', () => { + if (req.url === '/api/auth/refresh') { + refreshCallCount += 1; + res.setHeader('Content-Type', 'application/json'); + if (refreshShouldFail) { + res.statusCode = 401; + res.end(JSON.stringify({ error: 'Unauthorized' })); + } else { + res.end( + JSON.stringify({ accessToken: 'new-token', user: { id: 'u1' } }), + ); + } + return; + } + if (req.url === '/protected') { + protectedAuthHeaders.push(req.headers.authorization ?? ''); + if (req.headers.authorization === 'Bearer new-token') { + res.end(JSON.stringify({ ok: true })); + } else { + res.statusCode = 401; + res.end(JSON.stringify({ error: 'Invalid or expired token' })); + } + return; + } + if (req.url === '/always-401') { + res.statusCode = 401; + res.end(JSON.stringify({ error: 'Invalid or expired token' })); + return; + } + res.statusCode = 404; + res.end(); + }); + }); + await new Promise((resolve) => server.listen(PORT, resolve)); +} + +beforeAll(startStub); +afterAll(() => new Promise((resolve) => server.close(() => resolve()))); + +beforeEach(() => { + vi.restoreAllMocks(); +}); + +describe('api response interceptor', () => { + it('retries a request once after refreshing on 401', async () => { + refreshCallCount = 0; + protectedAuthHeaders.length = 0; + setAccessToken('old-token'); + + const res = await api.get(`${BASE}/protected`); + + expect(res.data).toEqual({ ok: true }); + expect(refreshCallCount).toBe(1); + // first attempt used stale token, replay used the refreshed one + expect(protectedAuthHeaders).toEqual([ + 'Bearer old-token', + 'Bearer new-token', + ]); + }); + + it('issues only one refresh for concurrent 401 responses', async () => { + refreshCallCount = 0; + protectedAuthHeaders.length = 0; + setAccessToken('old-token'); + + const [a, b, c] = await Promise.all([ + api.get(`${BASE}/protected`), + api.get(`${BASE}/protected`), + api.get(`${BASE}/protected`), + ]); + + expect(a.data).toEqual({ ok: true }); + expect(b.data).toEqual({ ok: true }); + expect(c.data).toEqual({ ok: true }); + expect(refreshCallCount).toBe(1); + }); + + it('clears the token and notifies listeners when refresh fails', async () => { + refreshShouldFail = true; + setAccessToken('expired-token'); + const expired = vi.fn(); + const unsubscribe = onSessionExpired(expired); + + await expect(api.get(`${BASE}/always-401`)).rejects.toThrow(); + + expect(expired).toHaveBeenCalledTimes(1); + unsubscribe(); + refreshShouldFail = false; + }); + + it('notifies sessionExpired listeners only once for concurrent refresh failures', async () => { + refreshShouldFail = true; + refreshCallCount = 0; + setAccessToken('expired-token'); + const expired = vi.fn(); + const unsubscribe = onSessionExpired(expired); + + await Promise.allSettled([ + api.get(`${BASE}/always-401`), + api.get(`${BASE}/always-401`), + api.get(`${BASE}/always-401`), + ]); + + expect(expired).toHaveBeenCalledTimes(1); + expect(refreshCallCount).toBe(1); + unsubscribe(); + refreshShouldFail = false; + }); + + it('does not attempt a refresh when no access token is stored', async () => { + refreshCallCount = 0; + clearAccessToken(); + + await expect(api.get(`${BASE}/always-401`)).rejects.toThrow(); + expect(refreshCallCount).toBe(0); + }); + + it('uses the shared instance against the configured base URL without manual base joining', async () => { + // sanity check that the exported api is an axios instance wired to env config + expect( + axios.isAxiosError(await api.get(`${BASE}/missing`).catch((e) => e)), + ).toBe(true); + }); +}); diff --git a/client/src/lib/api.ts b/client/src/lib/api.ts index eb50be2..ee7d82f 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -1,11 +1,26 @@ import axios from 'axios'; import { env } from '@/config/env'; -import { getAccessToken } from '@/utils/token'; +import { type User } from '@/types/api'; +import { + clearAccessToken, + getAccessToken, + setAccessToken, +} from '@/utils/token'; + +declare module 'axios' { + export interface InternalAxiosRequestConfig { + /** Access token attached when the request was originally sent. */ + _tokenUsed?: string; + /** Marks a request already replayed after a refresh. */ + _retry?: boolean; + } +} /** * Shared axios instance for all API calls. - * Attaches the stored access token to every request. + * Attaches the stored access token to every request and transparently + * refreshes an expired session once on 401 before replaying the request. */ export const api = axios.create({ baseURL: `${env.API_URL}/api`, @@ -16,6 +31,108 @@ api.interceptors.request.use((config) => { const token = getAccessToken(); if (token) { config.headers.Authorization = `Bearer ${token}`; + config._tokenUsed = token; } return config; }); + +/** + * Callback invoked when the session cannot be recovered (refresh failed). + */ +type SessionExpiredListener = () => void; + +const sessionExpiredListeners = new Set(); + +/** + * Registers a callback invoked when the session expires and cannot be + * refreshed; returns a function that unsubscribes the callback. + */ +export const onSessionExpired = ( + listener: SessionExpiredListener, +): (() => void) => { + sessionExpiredListeners.add(listener); + return () => sessionExpiredListeners.delete(listener); +}; + +// Auth endpoints manage their own credentials; refreshing from their own 401s +// would loop. +const AUTH_PATHS = [ + '/auth/login', + '/auth/register', + '/auth/refresh', + '/auth/logout', +]; + +let refreshPromise: Promise | null = null; + +/** + * Payload returned by the refresh endpoint. + */ +interface RefreshPayload { + accessToken: string; + user: User; +} + +/** + * Requests a fresh access token via the httpOnly refresh cookie; concurrent + * callers share the in-flight request so only one round-trip happens. Stores + * the new token before resolving with the full payload. On failure the + * session is cleared and listeners notified once — concurrent waiters share + * the same rejection. + */ +export const refreshAccessToken = (): Promise => { + if (!refreshPromise) { + refreshPromise = axios + .post(`${env.API_URL}/api/auth/refresh`, null, { + withCredentials: true, + }) + .then((res) => { + setAccessToken(res.data.accessToken); + return res.data; + }) + .catch((err: unknown) => { + clearAccessToken(); + sessionExpiredListeners.forEach((listener) => listener()); + throw err; + }) + .finally(() => { + refreshPromise = null; + }); + } + return refreshPromise; +}; + +api.interceptors.response.use(undefined, async (error: unknown) => { + if (!axios.isAxiosError(error) || !error.config) throw error; + const original = error.config; + const isAuthCall = AUTH_PATHS.some((path) => original.url?.includes(path)); + + if ( + error.response?.status !== 401 || + original._retry || + isAuthCall || + !original._tokenUsed + ) { + throw error; + } + + original._retry = true; + try { + // A concurrent request may have refreshed (or cleared) the token while + // this one was in flight; reuse it instead of refreshing again. If the + // token was cleared (null) the session is already expired — don't replay + // with `Bearer null` or trigger a second refresh, just fail. + const current = getAccessToken(); + let token: string; + if (current !== original._tokenUsed) { + if (!current) throw error; + token = current; + } else { + token = (await refreshAccessToken()).accessToken; + } + original.headers.Authorization = `Bearer ${token}`; + return api(original); + } catch { + throw error; + } +}); diff --git a/client/vite.config.ts b/client/vite.config.ts index 58a6bc2..357123d 100644 --- a/client/vite.config.ts +++ b/client/vite.config.ts @@ -39,6 +39,18 @@ export default defineConfig({ }, test: { projects: [ + { + extends: true, + test: { + name: 'unit', + include: ['src/**/*.test.{ts,tsx}'], + environment: 'node', + env: { + VITE_APP_API_URL: 'http://localhost:4599', + VITE_APP_SOCKET_URL: 'ws://localhost:5000/collaboration', + }, + }, + }, { extends: true, plugins: [ diff --git a/server/src/controllers/auth.controller.ts b/server/src/controllers/auth.controller.ts index 86abcff..12ef769 100644 --- a/server/src/controllers/auth.controller.ts +++ b/server/src/controllers/auth.controller.ts @@ -176,11 +176,14 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) = tokenExpiry: '15m', }); + // Production uses cross-site cookies, which require SameSite=None + Secure. + const isProduction = process.env.NODE_ENV === 'production'; + res.cookie('refreshToken', refreshToken, { httpOnly: true, maxAge: 24 * 60 * 60 * 1000, - sameSite: 'none', // ✅ allow cross-site cookies - secure: true, // ✅ must be secure for SameSite=None + sameSite: isProduction ? 'none' : 'lax', + secure: isProduction, }); res.status(StatusCodes.OK).json({ @@ -236,8 +239,18 @@ export const logoutUser = asyncErrorWrapper(async (req: AuthenticatedRequest, re userId, }); - res.clearCookie('refreshToken'); - res.clearCookie('accessToken'); + // Must match the attributes used when setting the cookie, otherwise + // browsers keep the SameSite=None; Secure cookie (prod) alive and a + // subsequent refresh still succeeds after logout. + const isProduction = process.env.NODE_ENV === 'production'; + const clearOpts = { + httpOnly: true, + secure: isProduction, + sameSite: (isProduction ? 'none' : 'lax') as 'none' | 'lax', + path: '/', + }; + res.clearCookie('refreshToken', clearOpts); + res.clearCookie('accessToken', clearOpts); res.status(StatusCodes.OK).json({ message: 'Logged out successfully' }); } catch (error) { logger.error('Logout failed - database error', { @@ -317,13 +330,6 @@ export const refreshToken = asyncErrorWrapper(async (req: Request, res: Response username: user.username, }); - res.cookie('accessToken', newAccessToken, { - httpOnly: true, - maxAge: 15 * 60 * 1000, - sameSite: 'none', // ✅ - secure: true, // ✅ - }); - res.status(StatusCodes.OK).json({ accessToken: newAccessToken, user: { diff --git a/server/src/routers/auth.router.ts b/server/src/routers/auth.router.ts index 40b8c5f..5043791 100644 --- a/server/src/routers/auth.router.ts +++ b/server/src/routers/auth.router.ts @@ -1,7 +1,7 @@ import express from 'express'; import { loginUser, logoutUser, refreshToken, registerUser } from '@/controllers/auth.controller'; -import { authenticate, validateRefreshToken } from '@/middlewares/auth.middleware'; +import { validateRefreshToken } from '@/middlewares/auth.middleware'; import { authLimiter } from '@/middlewares/rate-limit.middleware'; import { validate } from '@/middlewares/validation.middleware'; import { LoginUserSchema } from '@/validations/login.schema'; @@ -11,5 +11,5 @@ export const authRouter = express.Router(); authRouter.post('/register', authLimiter, validate({ body: RegisterUserSchema }), registerUser); authRouter.post('/login', authLimiter, validate({ body: LoginUserSchema }), loginUser); -authRouter.post('/logout', authenticate, logoutUser); +authRouter.post('/logout', validateRefreshToken, logoutUser); authRouter.post('/refresh', authLimiter, validateRefreshToken, refreshToken); diff --git a/server/test/auth.test.ts b/server/test/auth.test.ts index 2b1847d..adc7d45 100644 --- a/server/test/auth.test.ts +++ b/server/test/auth.test.ts @@ -155,6 +155,109 @@ describe('Auth Routes', () => { expect(logoutRes.status).toBe(StatusCodes.OK); }); + it('should logout with only the refresh cookie when no access token is sent', async () => { + await request(app).post('/api/auth/register').send({ + email: 'cookie-logout@test.dev', + username: 'cookieLogoutUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'cookie-logout@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const logoutRes = await request(app).post('/api/auth/logout').set('Cookie', cookieHeader); + + expect(logoutRes.status).toBe(StatusCodes.OK); + + const refreshRes = await request(app).post('/api/auth/refresh').set('Cookie', cookieHeader); + expect(refreshRes.status).toBe(StatusCodes.UNAUTHORIZED); + }); + + it('should clear refreshToken cookie with matching attributes on logout', async () => { + await request(app).post('/api/auth/register').send({ + email: 'clear-attrs@test.dev', + username: 'clearAttrsUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'clear-attrs@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const logoutRes = await request(app).post('/api/auth/logout').set('Cookie', cookieHeader); + + expect(logoutRes.status).toBe(StatusCodes.OK); + const setCookies = (logoutRes.headers['set-cookie'] ?? []) as string[]; + const cookiesArray = Array.isArray(setCookies) ? setCookies : [setCookies]; + // Express clearCookie sets `name=; Path=/; Expires=Thu, 01 Jan 1970 ...` + const refreshClear = cookiesArray.find(c => c.startsWith('refreshToken=;')); + expect(refreshClear).toBeDefined(); + // Must mirror login attributes or browsers (prod SameSite=None; Secure) won't clear + expect(refreshClear).toContain('Path=/'); + expect(refreshClear).toContain('HttpOnly'); + expect(refreshClear).toContain('SameSite=Lax'); + expect(refreshClear).not.toContain('Secure'); + expect(refreshClear).toMatch(/Expires=Thu, 01 Jan 1970|Max-Age=0/); + }); + + it('should clear both refreshToken and legacy accessToken cookies on logout', async () => { + await request(app).post('/api/auth/register').send({ + email: 'clear-both@test.dev', + username: 'clearBothUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'clear-both@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const logoutRes = await request(app).post('/api/auth/logout').set('Cookie', cookieHeader); + + expect(logoutRes.status).toBe(StatusCodes.OK); + const setCookies = (logoutRes.headers['set-cookie'] ?? []) as string[]; + const cookiesArray = Array.isArray(setCookies) ? setCookies : [setCookies]; + const names = cookiesArray.map(c => c.split('=')[0]); + expect(names).toContain('refreshToken'); + expect(names).toContain('accessToken'); + // Both clearing cookies must carry the same path/sameSite so they actually overwrite + for (const c of cookiesArray) { + if (c.startsWith('refreshToken=;') || c.startsWith('accessToken=;')) { + expect(c).toContain('Path=/'); + expect(c).toContain('SameSite=Lax'); + } + } + }); + + it('should not allow refresh with the old cookie after logout', async () => { + await request(app).post('/api/auth/register').send({ + email: 'refresh-after-logout@test.dev', + username: 'refreshAfterLogoutUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'refresh-after-logout@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const logoutRes = await request(app).post('/api/auth/logout').set('Cookie', cookieHeader); + expect(logoutRes.status).toBe(StatusCodes.OK); + + // Even though the client still holds the old cookie string, the server has + // nulled the stored refreshToken and the browser should have received a + // clearing Set-Cookie (verified above). Replaying the old cookie must fail. + const refreshRes = await request(app).post('/api/auth/refresh').set('Cookie', cookieHeader); + expect(refreshRes.status).toBe(StatusCodes.UNAUTHORIZED); + }); + it('should return 401 when accessing protected route without token', async () => { const res = await request(app).get('/api/user'); expect(res.status).toBe(StatusCodes.UNAUTHORIZED); @@ -180,9 +283,52 @@ describe('Auth Routes', () => { expect(res.body.email).toBe('me@test.dev'); }); + it('should set the refresh cookie without Secure/SameSite=None outside production', async () => { + await request(app).post('/api/auth/register').send({ + email: 'cookie-flags@test.dev', + username: 'cookieFlagsUser', + password: 'secure123', + }); + + const res = await request(app).post('/api/auth/login').send({ + email: 'cookie-flags@test.dev', + password: 'secure123', + }); + + expect(res.status).toBe(StatusCodes.OK); + const setCookies = Array.isArray(res.headers['set-cookie']) + ? res.headers['set-cookie'] + : [res.headers['set-cookie'] ?? '']; + const refreshCookie = setCookies.find(c => c.startsWith('refreshToken=')); + expect(refreshCookie).toBeDefined(); + expect(refreshCookie).not.toContain('Secure'); + expect(refreshCookie).toContain('SameSite=Lax'); + }); + it('should reject refresh with invalid refresh token', async () => { const res = await request(app).post('/api/auth/refresh').set('Cookie', 'refreshToken=invalid.token.here'); expect(res.status).toBe(StatusCodes.UNAUTHORIZED); }); + + it('should not set an accessToken cookie on refresh', async () => { + await request(app).post('/api/auth/register').send({ + email: 'refresh@test.dev', + username: 'refreshUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'refresh@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const res = await request(app).post('/api/auth/refresh').set('Cookie', cookieHeader); + + expect(res.status).toBe(StatusCodes.OK); + const cookies = res.headers['set-cookie'] ?? []; + const names = (Array.isArray(cookies) ? cookies : [cookies]).map(c => c.split('=')[0]); + expect(names).not.toContain('accessToken'); + }); });