From f20101a8dd390df9a6ee3dffb08a36c08f8c57f6 Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 03:17:20 +0300 Subject: [PATCH 1/9] fix(server): authenticate logout via refresh cookie Logout previously required a valid access token, so an expired session could not log out (the exact moment logout matters). Reuse the validateRefreshToken middleware, which already populates req.user for the controller to revoke the stored token. --- server/src/routers/auth.router.ts | 4 ++-- server/test/auth.test.ts | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) 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..dc62894 100644 --- a/server/test/auth.test.ts +++ b/server/test/auth.test.ts @@ -155,6 +155,27 @@ 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 return 401 when accessing protected route without token', async () => { const res = await request(app).get('/api/user'); expect(res.status).toBe(StatusCodes.UNAUTHORIZED); From 7289f8f3ca84ce2171c7eb8228e84487b018348c Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 03:20:42 +0300 Subject: [PATCH 2/9] fix(server): stop setting unused accessToken cookie on refresh The refresh endpoint set an httpOnly accessToken cookie that no client ever reads; the access token already travels in the response body. --- server/src/controllers/auth.controller.ts | 7 ------- server/test/auth.test.ts | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/server/src/controllers/auth.controller.ts b/server/src/controllers/auth.controller.ts index 86abcff..7839314 100644 --- a/server/src/controllers/auth.controller.ts +++ b/server/src/controllers/auth.controller.ts @@ -317,13 +317,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/test/auth.test.ts b/server/test/auth.test.ts index dc62894..ea3a781 100644 --- a/server/test/auth.test.ts +++ b/server/test/auth.test.ts @@ -206,4 +206,25 @@ describe('Auth Routes', () => { 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'); + }); }); From 7c382b53067f809f7618727a7cd2969b1c5a20da Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 03:26:10 +0300 Subject: [PATCH 3/9] fix(server): set refresh cookie flags per environment SameSite=None + Secure is only valid for cross-site HTTPS deployments; in dev the client and API are same-site over plain http, where Secure cookies get dropped and None requires TLS. Use Lax/insecure outside production, None/Secure in production. --- server/src/controllers/auth.controller.ts | 9 +++++++-- server/test/auth.test.ts | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/server/src/controllers/auth.controller.ts b/server/src/controllers/auth.controller.ts index 7839314..c82b030 100644 --- a/server/src/controllers/auth.controller.ts +++ b/server/src/controllers/auth.controller.ts @@ -176,11 +176,16 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) = tokenExpiry: '15m', }); + // Cross-site deployments (prod) need SameSite=None + Secure; in dev the + // client and API are same-site over http, where Secure cookies are dropped + // by browsers that don't trust localhost and None is rejected without TLS. + 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({ diff --git a/server/test/auth.test.ts b/server/test/auth.test.ts index ea3a781..a01d534 100644 --- a/server/test/auth.test.ts +++ b/server/test/auth.test.ts @@ -201,6 +201,25 @@ 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 refreshCookie = res.headers['set-cookie'].find((c: string) => 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'); From 896a31d2d28154528863ad249398c4d46f6be715 Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 03:33:05 +0300 Subject: [PATCH 4/9] fix(client): auto-refresh session on 401 with single-flight refresh Add a response interceptor that refreshes the access token once and replays the failed request. Concurrent 401s share one in-flight refresh; a request whose token was already rotated by a concurrent request replays directly instead of triggering a second round-trip. When the refresh itself fails, the stored token is cleared and listeners are notified so the auth provider can reset state. Also add a node-env vitest unit project so shared client lib code has a test runner (the existing vitest setup only ran Storybook tests). --- client/src/lib/__tests__/api.test.ts | 138 +++++++++++++++++++++++++++ client/src/lib/api.ts | 99 ++++++++++++++++++- client/vite.config.ts | 12 +++ 3 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 client/src/lib/__tests__/api.test.ts diff --git a/client/src/lib/__tests__/api.test.ts b/client/src/lib/__tests__/api.test.ts new file mode 100644 index 0000000..ab375aa --- /dev/null +++ b/client/src/lib/__tests__/api.test.ts @@ -0,0 +1,138 @@ +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('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..233cfec 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -1,11 +1,25 @@ import axios from 'axios'; import { env } from '@/config/env'; -import { getAccessToken } from '@/utils/token'; +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 +30,87 @@ 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; + +/** + * Requests a fresh access token via the httpOnly refresh cookie; concurrent + * callers share the in-flight request so only one round-trip happens. + */ +const refreshAccessToken = (): Promise => { + if (!refreshPromise) { + refreshPromise = axios + .post(`${env.API_URL}/api/auth/refresh`, null, { withCredentials: true }) + .then((res) => { + const token: string = res.data.accessToken; + setAccessToken(token); + return token; + }) + .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 the token while this one was in + // flight; reuse it instead of refreshing again. + const token = + getAccessToken() !== original._tokenUsed + ? getAccessToken()! + : await refreshAccessToken(); + original.headers.Authorization = `Bearer ${token}`; + return api(original); + } catch { + clearAccessToken(); + sessionExpiredListeners.forEach((listener) => listener()); + 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: [ From fbd8e842e2ddaf7578d23c4ba4d4e3e6a7ea9eea Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 03:49:13 +0300 Subject: [PATCH 5/9] fix(client): rebuild session bootstrap without wasLoggedOut hack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bootstrap now restores the session via the shared single-flight refresh and simply marks the user signed out when it fails — it no longer calls the authenticated logout endpoint (which always 401'd and threw an unhandled rejection). Remove the wasLoggedOut localStorage flag that kept logging users out after a logout->login cycle, swallow logout errors when the session is already dead server-side, and reset auth state when the interceptor reports an unrecoverable session expiry. Add e2e coverage for logout->re-login->reload session restore and for not calling logout during an unauthenticated bootstrap; serialize auth specs since real logins overwrite the user's single stored refresh token. --- client/e2e/auth.spec.ts | 46 ++++++++++++++++++++++ client/src/context/auth/auth-provider.tsx | 47 ++++++++++++++--------- client/src/lib/api.ts | 27 +++++++++---- 3 files changed, 93 insertions(+), 27 deletions(-) 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/api.ts b/client/src/lib/api.ts index 233cfec..a4c39b8 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -1,6 +1,7 @@ import axios from 'axios'; import { env } from '@/config/env'; +import { type User } from '@/types/api'; import { clearAccessToken, getAccessToken, @@ -62,20 +63,30 @@ const AUTH_PATHS = [ '/auth/logout', ]; -let refreshPromise: Promise | null = null; +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. + * callers share the in-flight request so only one round-trip happens. Stores + * the new token before resolving with the full payload. */ -const refreshAccessToken = (): Promise => { +export const refreshAccessToken = (): Promise => { if (!refreshPromise) { refreshPromise = axios - .post(`${env.API_URL}/api/auth/refresh`, null, { withCredentials: true }) + .post(`${env.API_URL}/api/auth/refresh`, null, { + withCredentials: true, + }) .then((res) => { - const token: string = res.data.accessToken; - setAccessToken(token); - return token; + setAccessToken(res.data.accessToken); + return res.data; }) .finally(() => { refreshPromise = null; @@ -105,7 +116,7 @@ api.interceptors.response.use(undefined, async (error: unknown) => { const token = getAccessToken() !== original._tokenUsed ? getAccessToken()! - : await refreshAccessToken(); + : (await refreshAccessToken()).accessToken; original.headers.Authorization = `Bearer ${token}`; return api(original); } catch { From baea27927244274dd9999e75ee2b30d2ec352054 Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 03:51:56 +0300 Subject: [PATCH 6/9] test(server): fix set-cookie typing in cookie flags test --- server/test/auth.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/test/auth.test.ts b/server/test/auth.test.ts index a01d534..51e3d30 100644 --- a/server/test/auth.test.ts +++ b/server/test/auth.test.ts @@ -214,7 +214,10 @@ describe('Auth Routes', () => { }); expect(res.status).toBe(StatusCodes.OK); - const refreshCookie = res.headers['set-cookie'].find((c: string) => c.startsWith('refreshToken=')); + 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'); From 4f8cdf55f296d8aa6044d36dfcdae6e478812bee Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Fri, 28 Aug 2026 02:10:15 +0300 Subject: [PATCH 7/9] fix(server): clear refresh cookie with matching attributes clearCookie must mirror the cookie's Secure/SameSite/Path/HttpOnly attributes, otherwise browsers retain the prod SameSite=None; Secure cookie after logout and subsequent refresh still succeeds. --- server/src/controllers/auth.controller.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/server/src/controllers/auth.controller.ts b/server/src/controllers/auth.controller.ts index c82b030..12ef769 100644 --- a/server/src/controllers/auth.controller.ts +++ b/server/src/controllers/auth.controller.ts @@ -176,9 +176,7 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) = tokenExpiry: '15m', }); - // Cross-site deployments (prod) need SameSite=None + Secure; in dev the - // client and API are same-site over http, where Secure cookies are dropped - // by browsers that don't trust localhost and None is rejected without TLS. + // Production uses cross-site cookies, which require SameSite=None + Secure. const isProduction = process.env.NODE_ENV === 'production'; res.cookie('refreshToken', refreshToken, { @@ -241,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', { From 575598c149b1880a38252523aaae6fa1152aa7a9 Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Fri, 28 Aug 2026 02:20:44 +0300 Subject: [PATCH 8/9] test(server): cover logout cookie clearing with matching attributes Verifies that logout clears both refreshToken and legacy accessToken cookies with the same Path/HttpOnly/SameSite attributes used on set (otherwise prod SameSite=None; Secure cookies survive logout and refresh still succeeds), and that replaying the old cookie after logout is rejected. Would have failed before 4f8cdf5. --- server/test/auth.test.ts | 82 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/server/test/auth.test.ts b/server/test/auth.test.ts index 51e3d30..adc7d45 100644 --- a/server/test/auth.test.ts +++ b/server/test/auth.test.ts @@ -176,6 +176,88 @@ describe('Auth Routes', () => { 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); From 0556d9c5b89705f4adea13073c2ffae2a21af161 Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Fri, 28 Aug 2026 02:47:48 +0300 Subject: [PATCH 9/9] fix(client): dedupe session expiry and guard cleared token replay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concurrent 401s sharing one refreshPromise notified listeners per waiter (3×) and could replay with Bearer null after clearAccessToken. Move clear+notify into refreshPromise rejection (once) and guard the current !== _tokenUsed branch to throw when current is null instead of asserting non-null. --- client/src/lib/__tests__/api.test.ts | 19 ++++++++++++++++++ client/src/lib/api.ts | 29 +++++++++++++++++++--------- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/client/src/lib/__tests__/api.test.ts b/client/src/lib/__tests__/api.test.ts index ab375aa..3bca57c 100644 --- a/client/src/lib/__tests__/api.test.ts +++ b/client/src/lib/__tests__/api.test.ts @@ -121,6 +121,25 @@ describe('api response interceptor', () => { 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(); diff --git a/client/src/lib/api.ts b/client/src/lib/api.ts index a4c39b8..ee7d82f 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -76,7 +76,9 @@ interface RefreshPayload { /** * 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. + * 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) { @@ -88,6 +90,11 @@ export const refreshAccessToken = (): Promise => { setAccessToken(res.data.accessToken); return res.data; }) + .catch((err: unknown) => { + clearAccessToken(); + sessionExpiredListeners.forEach((listener) => listener()); + throw err; + }) .finally(() => { refreshPromise = null; }); @@ -111,17 +118,21 @@ api.interceptors.response.use(undefined, async (error: unknown) => { original._retry = true; try { - // A concurrent request may have refreshed the token while this one was in - // flight; reuse it instead of refreshing again. - const token = - getAccessToken() !== original._tokenUsed - ? getAccessToken()! - : (await refreshAccessToken()).accessToken; + // 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 { - clearAccessToken(); - sessionExpiredListeners.forEach((listener) => listener()); throw error; } });