From add0b17e041efbad95c5f35dc480613dd239b8bc Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 1 Apr 2026 11:02:45 +0200 Subject: [PATCH] feat(auth): migrate JWT storage from localStorage to httpOnly cookies Eliminates XSS token theft risk by storing session JWTs in an httpOnly cookie (trek_session) instead of localStorage, making them inaccessible to JavaScript entirely. - Add cookie-parser middleware and setAuthCookie/clearAuthCookie helpers - Set trek_session cookie on login, register, demo-login, MFA verify, OIDC exchange - Auth middleware reads cookie first, falls back to Authorization: Bearer (MCP unchanged) - Add POST /api/auth/logout to clear the cookie server-side - Remove all localStorage auth_token reads/writes from client - Axios uses withCredentials; raw fetch calls use credentials: include - WebSocket ws-token exchange uses credentials: include (no JWT param) - authStore initialises isLoading: true so ProtectedRoute waits for /api/auth/me Co-Authored-By: Claude Sonnet 4.6 --- client/src/App.tsx | 6 ++-- client/src/api/authUrl.ts | 9 ++--- client/src/api/client.ts | 11 ++---- client/src/api/websocket.ts | 31 ++++++++-------- .../src/components/Planner/DayPlanSidebar.tsx | 2 +- client/src/pages/LoginPage.tsx | 6 ++-- client/src/store/authStore.ts | 35 +++++-------------- server/package-lock.json | 31 ++++++++++++++++ server/package.json | 2 ++ server/src/index.ts | 2 ++ server/src/middleware/auth.ts | 14 +++++--- server/src/routes/auth.ts | 10 ++++++ server/src/routes/oidc.ts | 2 ++ server/src/services/cookie.ts | 22 ++++++++++++ 14 files changed, 115 insertions(+), 68 deletions(-) create mode 100644 server/src/services/cookie.ts diff --git a/client/src/App.tsx b/client/src/App.tsx index fa31363..0235276 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -75,13 +75,11 @@ function RootRedirect() { } export default function App() { - const { loadUser, token, isAuthenticated, demoMode, setDemoMode, setHasMapsKey, setServerTimezone, setAppRequireMfa, setTripRemindersEnabled } = useAuthStore() + const { loadUser, isAuthenticated, demoMode, setDemoMode, setHasMapsKey, setServerTimezone, setAppRequireMfa, setTripRemindersEnabled } = useAuthStore() const { loadSettings } = useSettingsStore() useEffect(() => { - if (token) { - loadUser() - } + loadUser() authApi.getAppConfig().then(async (config: { demo_mode?: boolean; has_maps_key?: boolean; version?: string; timezone?: string; require_mfa?: boolean; trip_reminders_enabled?: boolean; permissions?: Record }) => { if (config?.demo_mode) setDemoMode(true) if (config?.has_maps_key !== undefined) setHasMapsKey(config.has_maps_key) diff --git a/client/src/api/authUrl.ts b/client/src/api/authUrl.ts index 433f09d..203ceb3 100644 --- a/client/src/api/authUrl.ts +++ b/client/src/api/authUrl.ts @@ -1,13 +1,10 @@ export async function getAuthUrl(url: string, purpose: 'download' | 'immich'): Promise { - const jwt = localStorage.getItem('auth_token') - if (!jwt || !url) return url + if (!url) return url try { const resp = await fetch('/api/auth/resource-token', { method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'Authorization': `Bearer ${jwt}`, - }, + headers: { 'Content-Type': 'application/json' }, + credentials: 'include', body: JSON.stringify({ purpose }), }) if (!resp.ok) return url diff --git a/client/src/api/client.ts b/client/src/api/client.ts index f440469..7992005 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -3,18 +3,15 @@ import { getSocketId } from './websocket' const apiClient: AxiosInstance = axios.create({ baseURL: '/api', + withCredentials: true, headers: { 'Content-Type': 'application/json', }, }) -// Request interceptor - add auth token and socket ID +// Request interceptor - add socket ID apiClient.interceptors.request.use( (config) => { - const token = localStorage.getItem('auth_token') - if (token) { - config.headers.Authorization = `Bearer ${token}` - } const sid = getSocketId() if (sid) { config.headers['X-Socket-Id'] = sid @@ -29,7 +26,6 @@ apiClient.interceptors.response.use( (response) => response, (error) => { if (error.response?.status === 401) { - localStorage.removeItem('auth_token') if (!window.location.pathname.includes('/login') && !window.location.pathname.includes('/register')) { window.location.href = '/login' } @@ -285,9 +281,8 @@ export const backupApi = { list: () => apiClient.get('/backup/list').then(r => r.data), create: () => apiClient.post('/backup/create').then(r => r.data), download: async (filename: string): Promise => { - const token = localStorage.getItem('auth_token') const res = await fetch(`/api/backup/download/${filename}`, { - headers: { Authorization: `Bearer ${token}` }, + credentials: 'include', }) if (!res.ok) throw new Error('Download failed') const blob = await res.blob() diff --git a/client/src/api/websocket.ts b/client/src/api/websocket.ts index 757f953..2b4a520 100644 --- a/client/src/api/websocket.ts +++ b/client/src/api/websocket.ts @@ -9,7 +9,7 @@ let reconnectDelay = 1000 const MAX_RECONNECT_DELAY = 30000 const listeners = new Set() const activeTrips = new Set() -let currentToken: string | null = null +let shouldReconnect = false let refetchCallback: RefetchCallback | null = null let mySocketId: string | null = null let connecting = false @@ -27,15 +27,15 @@ function getWsUrl(wsToken: string): string { return `${protocol}://${location.host}/ws?token=${wsToken}` } -async function fetchWsToken(jwt: string): Promise { +async function fetchWsToken(): Promise { try { const resp = await fetch('/api/auth/ws-token', { method: 'POST', - headers: { 'Authorization': `Bearer ${jwt}` }, + credentials: 'include', }) if (resp.status === 401) { - // JWT expired — stop reconnecting - currentToken = null + // Session expired — stop reconnecting + shouldReconnect = false return null } if (!resp.ok) return null @@ -65,26 +65,25 @@ function scheduleReconnect(): void { if (reconnectTimer) return reconnectTimer = setTimeout(() => { reconnectTimer = null - if (currentToken) { - connectInternal(currentToken, true) + if (shouldReconnect) { + connectInternal(true) } }, reconnectDelay) reconnectDelay = Math.min(reconnectDelay * 2, MAX_RECONNECT_DELAY) } -async function connectInternal(token: string, _isReconnect = false): Promise { +async function connectInternal(_isReconnect = false): Promise { if (connecting) return if (socket && (socket.readyState === WebSocket.OPEN || socket.readyState === WebSocket.CONNECTING)) { return } connecting = true - const wsToken = await fetchWsToken(token) + const wsToken = await fetchWsToken() connecting = false if (!wsToken) { - // currentToken may have been cleared on 401; only schedule reconnect if still active - if (currentToken) scheduleReconnect() + if (shouldReconnect) scheduleReconnect() return } @@ -113,7 +112,7 @@ async function connectInternal(token: string, _isReconnect = false): Promise { socket = null - if (currentToken) { + if (shouldReconnect) { scheduleReconnect() } } @@ -123,18 +122,18 @@ async function connectInternal(token: string, _isReconnect = false): Promise { try { const res = await fetch(`/api/trips/${tripId}/export.ics`, { - headers: { 'Authorization': `Bearer ${localStorage.getItem('auth_token')}` }, + credentials: 'include', }) if (!res.ok) throw new Error() const blob = await res.blob() diff --git a/client/src/pages/LoginPage.tsx b/client/src/pages/LoginPage.tsx index 1f8283b..1474838 100644 --- a/client/src/pages/LoginPage.tsx +++ b/client/src/pages/LoginPage.tsx @@ -55,11 +55,11 @@ export default function LoginPage(): React.ReactElement { if (oidcCode) { setIsLoading(true) window.history.replaceState({}, '', '/login') - fetch('/api/auth/oidc/exchange?code=' + encodeURIComponent(oidcCode)) + fetch('/api/auth/oidc/exchange?code=' + encodeURIComponent(oidcCode), { credentials: 'include' }) .then(r => r.json()) - .then(data => { + .then(async data => { if (data.token) { - localStorage.setItem('auth_token', data.token) + await loadUser() navigate('/dashboard', { replace: true }) } else { setError(data.error || 'OIDC login failed') diff --git a/client/src/store/authStore.ts b/client/src/store/authStore.ts index 9fbad53..a98bd65 100644 --- a/client/src/store/authStore.ts +++ b/client/src/store/authStore.ts @@ -17,7 +17,6 @@ interface AvatarResponse { interface AuthState { user: User | null - token: string | null isAuthenticated: boolean isLoading: boolean error: string | null @@ -49,9 +48,8 @@ interface AuthState { export const useAuthStore = create((set, get) => ({ user: null, - token: localStorage.getItem('auth_token') || null, - isAuthenticated: !!localStorage.getItem('auth_token'), - isLoading: false, + isAuthenticated: false, + isLoading: true, error: null, demoMode: localStorage.getItem('demo_mode') === 'true', hasMapsKey: false, @@ -67,15 +65,13 @@ export const useAuthStore = create((set, get) => ({ set({ isLoading: false, error: null }) return { mfa_required: true as const, mfa_token: data.mfa_token } } - localStorage.setItem('auth_token', data.token) set({ user: data.user, - token: data.token, isAuthenticated: true, isLoading: false, error: null, }) - connect(data.token) + connect() return data as AuthResponse } catch (err: unknown) { const error = getApiErrorMessage(err, 'Login failed') @@ -88,15 +84,13 @@ export const useAuthStore = create((set, get) => ({ set({ isLoading: true, error: null }) try { const data = await authApi.verifyMfaLogin({ mfa_token: mfaToken, code: code.replace(/\s/g, '') }) - localStorage.setItem('auth_token', data.token) set({ user: data.user, - token: data.token, isAuthenticated: true, isLoading: false, error: null, }) - connect(data.token) + connect() return data as AuthResponse } catch (err: unknown) { const error = getApiErrorMessage(err, 'Verification failed') @@ -109,15 +103,13 @@ export const useAuthStore = create((set, get) => ({ set({ isLoading: true, error: null }) try { const data = await authApi.register({ username, email, password, invite_token }) - localStorage.setItem('auth_token', data.token) set({ user: data.user, - token: data.token, isAuthenticated: true, isLoading: false, error: null, }) - connect(data.token) + connect() return data } catch (err: unknown) { const error = getApiErrorMessage(err, 'Registration failed') @@ -128,7 +120,8 @@ export const useAuthStore = create((set, get) => ({ logout: () => { disconnect() - localStorage.removeItem('auth_token') + // Tell server to clear the httpOnly cookie + fetch('/api/auth/logout', { method: 'POST', credentials: 'include' }).catch(() => {}) // Clear service worker caches containing sensitive data if ('caches' in window) { caches.delete('api-data').catch(() => {}) @@ -136,7 +129,6 @@ export const useAuthStore = create((set, get) => ({ } set({ user: null, - token: null, isAuthenticated: false, error: null, }) @@ -144,11 +136,6 @@ export const useAuthStore = create((set, get) => ({ loadUser: async (opts?: { silent?: boolean }) => { const silent = !!opts?.silent - const token = get().token - if (!token) { - if (!silent) set({ isLoading: false }) - return - } if (!silent) set({ isLoading: true }) try { const data = await authApi.me() @@ -157,16 +144,14 @@ export const useAuthStore = create((set, get) => ({ isAuthenticated: true, isLoading: false, }) - connect(token) + connect() } catch (err: unknown) { // Only clear auth state on 401 (invalid/expired token), not on network errors const isAuthError = err && typeof err === 'object' && 'response' in err && (err as { response?: { status?: number } }).response?.status === 401 if (isAuthError) { - localStorage.removeItem('auth_token') set({ user: null, - token: null, isAuthenticated: false, isLoading: false, }) @@ -233,16 +218,14 @@ export const useAuthStore = create((set, get) => ({ set({ isLoading: true, error: null }) try { const data = await authApi.demoLogin() - localStorage.setItem('auth_token', data.token) set({ user: data.user, - token: data.token, isAuthenticated: true, isLoading: false, demoMode: true, error: null, }) - connect(data.token) + connect() return data } catch (err: unknown) { const error = getApiErrorMessage(err, 'Demo login failed') diff --git a/server/package-lock.json b/server/package-lock.json index f4af0a6..fb46ba6 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -12,6 +12,7 @@ "archiver": "^6.0.1", "bcryptjs": "^2.4.3", "better-sqlite3": "^12.8.0", + "cookie-parser": "^1.4.7", "cors": "^2.8.5", "dotenv": "^16.4.1", "express": "^4.18.3", @@ -34,6 +35,7 @@ "@types/archiver": "^7.0.0", "@types/bcryptjs": "^2.4.6", "@types/better-sqlite3": "^7.6.13", + "@types/cookie-parser": "^1.4.10", "@types/cors": "^2.8.19", "@types/express": "^4.17.25", "@types/jsonwebtoken": "^9.0.10", @@ -914,6 +916,16 @@ "@types/node": "*" } }, + "node_modules/@types/cookie-parser": { + "version": "1.4.10", + "resolved": "https://registry.npmjs.org/@types/cookie-parser/-/cookie-parser-1.4.10.tgz", + "integrity": "sha512-B4xqkqfZ8Wek+rCOeRxsjMS9OgvzebEzzLYw7NHYuvzb7IdxOkI0ZHGgeEBX4PUM7QGVvNSK60T3OvWj3YfBRg==", + "dev": true, + "license": "MIT", + "peerDependencies": { + "@types/express": "*" + } + }, "node_modules/@types/cors": { "version": "2.8.19", "resolved": "https://registry.npmjs.org/@types/cors/-/cors-2.8.19.tgz", @@ -1713,6 +1725,25 @@ "node": ">= 0.6" } }, + "node_modules/cookie-parser": { + "version": "1.4.7", + "resolved": "https://registry.npmjs.org/cookie-parser/-/cookie-parser-1.4.7.tgz", + "integrity": "sha512-nGUvgXnotP3BsjiLX2ypbQnWoGUPIIfHQNZkkC668ntrzGWEZVW70HDEB1qnNGMicPje6EttlIgzo51YSwNQGw==", + "license": "MIT", + "dependencies": { + "cookie": "0.7.2", + "cookie-signature": "1.0.6" + }, + "engines": { + "node": ">= 0.8.0" + } + }, + "node_modules/cookie-parser/node_modules/cookie-signature": { + "version": "1.0.6", + "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.6.tgz", + "integrity": "sha512-QADzlaHc8icV8I7vbaJXJwod9HWYp8uCqf1xa4OfNu1T7JVxQIrUgOWtHdNDtPiywmFbiS12VjotIXLrKM3orQ==", + "license": "MIT" + }, "node_modules/cookie-signature": { "version": "1.0.7", "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.7.tgz", diff --git a/server/package.json b/server/package.json index 27b8210..20030bc 100644 --- a/server/package.json +++ b/server/package.json @@ -11,6 +11,7 @@ "archiver": "^6.0.1", "bcryptjs": "^2.4.3", "better-sqlite3": "^12.8.0", + "cookie-parser": "^1.4.7", "cors": "^2.8.5", "dotenv": "^16.4.1", "express": "^4.18.3", @@ -33,6 +34,7 @@ "@types/archiver": "^7.0.0", "@types/bcryptjs": "^2.4.6", "@types/better-sqlite3": "^7.6.13", + "@types/cookie-parser": "^1.4.10", "@types/cors": "^2.8.19", "@types/express": "^4.17.25", "@types/jsonwebtoken": "^9.0.10", diff --git a/server/src/index.ts b/server/src/index.ts index c79bcc8..6c81750 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -3,6 +3,7 @@ import express, { Request, Response, NextFunction } from 'express'; import { enforceGlobalMfaPolicy } from './middleware/mfaPolicy'; import cors from 'cors'; import helmet from 'helmet'; +import cookieParser from 'cookie-parser'; import path from 'path'; import fs from 'fs'; @@ -86,6 +87,7 @@ if (shouldForceHttps) { } app.use(express.json({ limit: '100kb' })); app.use(express.urlencoded({ extended: true })); +app.use(cookieParser()); app.use(enforceGlobalMfaPolicy); diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index 9b0c040..b2a7807 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -4,9 +4,16 @@ import { db } from '../db/database'; import { JWT_SECRET } from '../config'; import { AuthRequest, OptionalAuthRequest, User } from '../types'; -const authenticate = (req: Request, res: Response, next: NextFunction): void => { +function extractToken(req: Request): string | null { + // Prefer httpOnly cookie; fall back to Authorization: Bearer (MCP, API clients) + const cookieToken = (req as any).cookies?.trek_session; + if (cookieToken) return cookieToken; const authHeader = req.headers['authorization']; - const token = authHeader && authHeader.split(' ')[1]; + return (authHeader && authHeader.split(' ')[1]) || null; +} + +const authenticate = (req: Request, res: Response, next: NextFunction): void => { + const token = extractToken(req); if (!token) { res.status(401).json({ error: 'Access token required' }); @@ -30,8 +37,7 @@ const authenticate = (req: Request, res: Response, next: NextFunction): void => }; const optionalAuth = (req: Request, res: Response, next: NextFunction): void => { - const authHeader = req.headers['authorization']; - const token = authHeader && authHeader.split(' ')[1]; + const token = extractToken(req); if (!token) { (req as OptionalAuthRequest).user = null; diff --git a/server/src/routes/auth.ts b/server/src/routes/auth.ts index f86357e..eb96998 100644 --- a/server/src/routes/auth.ts +++ b/server/src/routes/auth.ts @@ -22,6 +22,7 @@ import { writeAudit, getClientIp } from '../services/auditLog'; import { decrypt_api_key, maybe_encrypt_api_key, encrypt_api_key } from '../services/apiKeyCrypto'; import { startTripReminders } from '../scheduler'; import { createEphemeralToken } from '../services/ephemeralTokens'; +import { setAuthCookie, clearAuthCookie } from '../services/cookie'; authenticator.options = { window: 1 }; @@ -229,6 +230,7 @@ router.post('/demo-login', (_req: Request, res: Response) => { if (!user) return res.status(500).json({ error: 'Demo user not found' }); const token = generateToken(user); const safe = stripUserForClient(user) as Record; + setAuthCookie(res, token); res.json({ token, user: { ...safe, avatar_url: avatarUrl(user) } }); }); @@ -307,6 +309,7 @@ router.post('/register', authLimiter, (req: Request, res: Response) => { } writeAudit({ userId: Number(result.lastInsertRowid), action: 'user.register', ip: getClientIp(req), details: { username, email, role } }); + setAuthCookie(res, token); res.status(201).json({ token, user: { ...user, avatar_url: null } }); } catch (err: unknown) { res.status(500).json({ error: 'Error creating user' }); @@ -350,6 +353,7 @@ router.post('/login', authLimiter, (req: Request, res: Response) => { const userSafe = stripUserForClient(user) as Record; writeAudit({ userId: Number(user.id), action: 'user.login', ip: getClientIp(req), details: { email } }); + setAuthCookie(res, token); res.json({ token, user: { ...userSafe, avatar_url: avatarUrl(user) } }); }); @@ -367,6 +371,11 @@ router.get('/me', authenticate, (req: Request, res: Response) => { res.json({ user: { ...base, avatar_url: avatarUrl(user) } }); }); +router.post('/logout', (req: Request, res: Response) => { + clearAuthCookie(res); + res.json({ success: true }); +}); + router.put('/me/password', authenticate, rateLimiter(5, RATE_LIMIT_WINDOW), (req: Request, res: Response) => { const authReq = req as AuthRequest; if (isOidcOnlyMode()) { @@ -810,6 +819,7 @@ router.post('/mfa/verify-login', mfaLimiter, (req: Request, res: Response) => { const sessionToken = generateToken(user); const userSafe = stripUserForClient(user) as Record; writeAudit({ userId: Number(user.id), action: 'user.login', ip: getClientIp(req), details: { mfa: true } }); + setAuthCookie(res, sessionToken); res.json({ token: sessionToken, user: { ...userSafe, avatar_url: avatarUrl(user) } }); } catch { return res.status(401).json({ error: 'Invalid or expired verification token' }); diff --git a/server/src/routes/oidc.ts b/server/src/routes/oidc.ts index a62d306..8ba8c23 100644 --- a/server/src/routes/oidc.ts +++ b/server/src/routes/oidc.ts @@ -6,6 +6,7 @@ import { db } from '../db/database'; import { JWT_SECRET } from '../config'; import { User } from '../types'; import { decrypt_api_key } from '../services/apiKeyCrypto'; +import { setAuthCookie } from '../services/cookie'; interface OidcDiscoveryDoc { authorization_endpoint: string; @@ -289,6 +290,7 @@ router.get('/exchange', (req: Request, res: Response) => { if (!entry) return res.status(400).json({ error: 'Invalid or expired code' }); authCodes.delete(code); if (Date.now() - entry.created > AUTH_CODE_TTL) return res.status(400).json({ error: 'Code expired' }); + setAuthCookie(res, entry.token); res.json({ token: entry.token }); }); diff --git a/server/src/services/cookie.ts b/server/src/services/cookie.ts new file mode 100644 index 0000000..448e25c --- /dev/null +++ b/server/src/services/cookie.ts @@ -0,0 +1,22 @@ +import { Response } from 'express'; + +const COOKIE_NAME = 'trek_session'; + +function cookieOptions(clear = false) { + const secure = process.env.NODE_ENV === 'production' || process.env.FORCE_HTTPS === 'true'; + return { + httpOnly: true, + secure, + sameSite: 'strict' as const, + path: '/', + ...(clear ? {} : { maxAge: 24 * 60 * 60 * 1000 }), // 24h — matches JWT expiry + }; +} + +export function setAuthCookie(res: Response, token: string): void { + res.cookie(COOKIE_NAME, token, cookieOptions()); +} + +export function clearAuthCookie(res: Response): void { + res.clearCookie(COOKIE_NAME, cookieOptions(true)); +}