From 9814480a9629e81587cc340d8a352b9b2f20d841 Mon Sep 17 00:00:00 2001 From: Maurice <61554723+mauriceboe@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:01:24 +0200 Subject: [PATCH] refactor(server): move the JWT verify into nest/auth, delete four dead middlewares (#1828) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit middleware/auth.ts exported seven things. Two of them — extractToken and verifyJwtAndLoadUser — are the canonical session check every auth surface goes through. The other five (authenticate, requireCookieAuth, optionalAuth, adminOnly, demoUploadBlock) are Express middlewares with no production caller left; the guards in nest/auth replaced them and only comments still mention them. The two live functions move to nest/auth/jwt-verify.ts and the file goes, along with middleware/validate.ts (nothing but its own test imported it) and middleware/idempotency.ts (its only caller was `authenticate`, and IdempotencyInterceptor has replaced it). AuthRequest and OptionalAuthRequest go with them — src/typings/express.d.ts is what types req.user now. Eight importers repoint: the three guards, auth/files/plugins-proxy services, platform.routes and the global MFA policy. Two deliberate departures from the plan for this package. JWT_SECRET does not move to app-config. It is not an env value: it is generated into data/.jwt_secret, and the admin panel rotates it at runtime through updateJwtSecret. `export let` is precisely what makes a rotation take effect in-process — a registerAs token would freeze the boot value and silently break rotation. The verify also keeps reading the db singleton rather than DatabaseService. That change is not a one-line swap: it gives the three guards a constructor dependency, which means registering them as providers everywhere they are used. Measured: 38 directories use those guards and 21 of them do not import AuthModule, on top of 34 e2e tests that each build a container around a single domain module. That is its own PR, not a rider on one that deletes dead files. tests/unit/middleware/auth.test.ts covered real behaviour, so it is not simply deleted. Its extractToken cases and the five verify cases it drove through `authenticate` come back as tests/unit/nest/auth/jwt-verify.test.ts, now hitting verifyJwtAndLoadUser directly. Four cases are new: the password_version gate — the whole reason this function exists — had no direct coverage at all. Its three adminOnly cases are dropped because AdminGuard, their successor, is covered by three e2e suites. --- server/src/middleware/auth.ts | 120 ------------ server/src/middleware/idempotency.ts | 80 -------- server/src/middleware/mfaPolicy.ts | 2 +- server/src/middleware/validate.ts | 26 --- server/src/nest/app.module.ts | 4 +- server/src/nest/auth/auth.service.ts | 4 +- server/src/nest/auth/cookie-auth.guard.ts | 2 +- server/src/nest/auth/jwt-auth.guard.ts | 2 +- server/src/nest/auth/jwt-verify.ts | 59 ++++++ server/src/nest/auth/optional-jwt.guard.ts | 2 +- server/src/nest/common/demo.ts | 3 +- .../nest/common/idempotency.interceptor.ts | 5 +- server/src/nest/files/files.service.ts | 2 +- server/src/nest/platform/platform.routes.ts | 2 +- .../nest/plugins/plugins-proxy.controller.ts | 2 +- server/src/types.ts | 9 - server/tests/unit/middleware/auth.test.ts | 184 ------------------ .../tests/unit/middleware/idempotency.test.ts | 176 ----------------- server/tests/unit/middleware/validate.test.ts | 109 ----------- server/tests/unit/nest/auth-guard.test.ts | 4 +- server/tests/unit/nest/auth.service.test.ts | 2 +- .../tests/unit/nest/auth/jwt-verify.test.ts | 135 +++++++++++++ server/tests/unit/nest/files.service.test.ts | 2 +- .../unit/nest/platform.controller.test.ts | 2 +- .../tests/unit/plugins/plugins-proxy.test.ts | 2 +- 25 files changed, 215 insertions(+), 725 deletions(-) delete mode 100644 server/src/middleware/auth.ts delete mode 100644 server/src/middleware/idempotency.ts delete mode 100644 server/src/middleware/validate.ts create mode 100644 server/src/nest/auth/jwt-verify.ts delete mode 100644 server/tests/unit/middleware/auth.test.ts delete mode 100644 server/tests/unit/middleware/idempotency.test.ts delete mode 100644 server/tests/unit/middleware/validate.test.ts create mode 100644 server/tests/unit/nest/auth/jwt-verify.test.ts diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts deleted file mode 100644 index d485f4e80..000000000 --- a/server/src/middleware/auth.ts +++ /dev/null @@ -1,120 +0,0 @@ -import { Request, Response, NextFunction } from 'express'; -import jwt from 'jsonwebtoken'; -import { readEnv } from '../app-config'; -import { db } from '../db/database'; -import { JWT_SECRET } from '../config'; -import { AuthRequest, OptionalAuthRequest, User } from '../types'; -import { applyIdempotency } from './idempotency'; -import { isDemoEmail } from '../nest/common/demo'; - -export 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']; - return (authHeader && authHeader.split(' ')[1]) || null; -} - -/** - * Verify a JWT and load its user, enforcing the password_version gate. - * - * Exported so every auth surface in the codebase (MCP bearer tokens, - * file download query tokens, the photo-serving route) goes through the - * same check. A password reset bumps `users.password_version`, which - * invalidates every JWT that embedded the prior value — but only if - * every verify path actually compares the claim. Previously several - * paths called `jwt.verify` directly and skipped the DB lookup, so a - * stolen token kept working after the victim reset. - */ -export function verifyJwtAndLoadUser(token: string): User | null { - try { - const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number; pv?: number; purpose?: string }; - // Purpose-scoped tokens (e.g. the short-lived mfa_login token) share this - // secret but are not full session tokens — only their dedicated endpoint - // may accept them, so reject any token carrying a purpose claim here. - if (decoded.purpose) return null; - const row = db.prepare( - 'SELECT id, username, email, role, password_version FROM users WHERE id = ?' - ).get(decoded.id) as (User & { password_version?: number }) | undefined; - if (!row) return null; - // Session invalidation: any token whose embedded password_version - // predates the user's current one is rejected. Tokens issued before - // the `pv` claim existed (decoded.pv === undefined) are treated as - // version 0 so legacy sessions keep working until the user resets. - const tokenPv = typeof decoded.pv === 'number' ? decoded.pv : 0; - const currentPv = typeof row.password_version === 'number' ? row.password_version : 0; - if (tokenPv !== currentPv) return null; - // Don't leak password_version beyond the middleware. - const { password_version: _pv, ...user } = row; - return user as User; - } catch { - return null; - } -} - -const authenticate = (req: Request, res: Response, next: NextFunction): void => { - const token = extractToken(req); - - if (!token) { - res.status(401).json({ error: 'Access token required', code: 'AUTH_REQUIRED' }); - return; - } - - const user = verifyJwtAndLoadUser(token); - if (!user) { - res.status(401).json({ error: 'Invalid or expired token', code: 'AUTH_REQUIRED' }); - return; - } - (req as AuthRequest).user = user; - applyIdempotency(req, res, next, user.id); -}; - -/** Like `authenticate` but rejects requests that don't carry an httpOnly session cookie. - * Used on state-mutating OAuth endpoints (consent POST, client CRUD, session revoke) - * to prevent Bearer JWT tokens obtained by other means from managing OAuth clients. */ -const requireCookieAuth = (req: Request, res: Response, next: NextFunction): void => { - const cookieToken = (req as any).cookies?.trek_session; - if (!cookieToken) { - res.status(401).json({ error: 'Cookie session required for this endpoint', code: 'COOKIE_AUTH_REQUIRED' }); - return; - } - const user = verifyJwtAndLoadUser(cookieToken); - if (!user) { - res.status(401).json({ error: 'Invalid or expired session', code: 'AUTH_REQUIRED' }); - return; - } - (req as AuthRequest).user = user; - next(); -}; - -const optionalAuth = (req: Request, res: Response, next: NextFunction): void => { - const token = extractToken(req); - - if (!token) { - (req as OptionalAuthRequest).user = null; - return next(); - } - - (req as OptionalAuthRequest).user = verifyJwtAndLoadUser(token) || null; - next(); -}; - -const adminOnly = (req: Request, res: Response, next: NextFunction): void => { - const authReq = req as AuthRequest; - if (!authReq.user || authReq.user.role !== 'admin') { - res.status(403).json({ error: 'Admin access required' }); - return; - } - next(); -}; - -const demoUploadBlock = (req: Request, res: Response, next: NextFunction): void => { - const authReq = req as AuthRequest; - if (readEnv().demo.enabled && isDemoEmail(authReq.user?.email)) { - res.status(403).json({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }); - return; - } - next(); -}; - -export { authenticate, requireCookieAuth, optionalAuth, adminOnly, demoUploadBlock }; diff --git a/server/src/middleware/idempotency.ts b/server/src/middleware/idempotency.ts deleted file mode 100644 index 0e6acfd91..000000000 --- a/server/src/middleware/idempotency.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { Request, Response, NextFunction } from 'express'; -import { db } from '../db/database'; - -const MUTATING_METHODS = new Set(['POST', 'PUT', 'PATCH', 'DELETE']); -// Reject pathological client-supplied keys outright instead of hashing -// everything — 128 chars is plenty for any realistic UUID / ULID / nonce. -const MAX_KEY_LENGTH = 128; -// Responses larger than this are not worth caching — a backup-restore -// endpoint could otherwise store a megabyte-sized JSON body per request -// key and, with mass key creation, blow up idempotency_keys. -const MAX_CACHED_BODY_BYTES = 256 * 1024; - -interface IdempotencyRow { - status_code: number; - response_body: string; -} - -/** - * Called from within `authenticate` after req.user is set. - * - * For mutating requests carrying X-Idempotency-Key: - * - If (key, userId, method, path) already stored: replays the cached response. - * - Otherwise: wraps res.json to capture and store a successful response. - * - * The lookup is scoped by method + path as well as user so the same key - * replayed against a different endpoint doesn't return the cached body - * of an unrelated request. Key length is capped and the cached body is - * skipped when it exceeds `MAX_CACHED_BODY_BYTES`. - * - * Storing happens in idempotency_keys (24h TTL, cleaned by scheduler). - */ -export function applyIdempotency(req: Request, res: Response, next: NextFunction, userId: number): void { - if (!MUTATING_METHODS.has(req.method)) { - next(); - return; - } - - const key = req.headers['x-idempotency-key'] as string | undefined; - if (!key) { - next(); - return; - } - if (key.length > MAX_KEY_LENGTH) { - res.status(400).json({ error: 'X-Idempotency-Key exceeds maximum length of 128 characters' }); - return; - } - - // Return cached response only if the same key was seen for the same - // user AND the same method+path — avoids a POST's cached body leaking - // into an unrelated PATCH that reused the idempotency-key string. - const existing = db.prepare( - 'SELECT status_code, response_body FROM idempotency_keys WHERE key = ? AND user_id = ? AND method = ? AND path = ?' - ).get(key, userId, req.method, req.path) as IdempotencyRow | undefined; - - if (existing) { - res.status(existing.status_code).json(JSON.parse(existing.response_body)); - return; - } - - // Wrap res.json to capture the response on first successful execution - const originalJson = res.json.bind(res); - res.json = function (body: unknown): Response { - if (res.statusCode >= 200 && res.statusCode < 300) { - try { - const serialized = JSON.stringify(body); - if (serialized.length <= MAX_CACHED_BODY_BYTES) { - db.prepare( - `INSERT OR IGNORE INTO idempotency_keys (key, user_id, method, path, status_code, response_body, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?)` - ).run(key, userId, req.method, req.path, res.statusCode, serialized, Math.floor(Date.now() / 1000)); - } - } catch { - // Non-fatal: if storage fails, the request still succeeds - } - } - return originalJson(body); - }; - - next(); -} diff --git a/server/src/middleware/mfaPolicy.ts b/server/src/middleware/mfaPolicy.ts index 2d09f9b90..e0fc0483d 100644 --- a/server/src/middleware/mfaPolicy.ts +++ b/server/src/middleware/mfaPolicy.ts @@ -1,7 +1,7 @@ import { Request, Response, NextFunction } from 'express'; import { readEnv } from '../app-config'; import { db } from '../db/database'; -import { extractToken, verifyJwtAndLoadUser } from './auth'; +import { extractToken, verifyJwtAndLoadUser } from '../nest/auth/jwt-verify'; import { DEMO_EMAILS } from '../nest/common/demo'; /** Paths that never require MFA (public or pre-auth). */ diff --git a/server/src/middleware/validate.ts b/server/src/middleware/validate.ts deleted file mode 100644 index 90101e7b3..000000000 --- a/server/src/middleware/validate.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { Request, Response, NextFunction } from 'express'; - -function maxLength(field: string, max: number) { - return (req: Request, res: Response, next: NextFunction): void => { - if (req.body[field] && typeof req.body[field] === 'string' && req.body[field].length > max) { - res.status(400).json({ error: `${field} must be ${max} characters or less` }); - return; - } - next(); - }; -} - -function validateStringLengths(maxLengths: Record) { - return (req: Request, res: Response, next: NextFunction): void => { - for (const [field, max] of Object.entries(maxLengths)) { - const value = req.body[field]; - if (value && typeof value === 'string' && value.length > max) { - res.status(400).json({ error: `${field} must be ${max} characters or less` }); - return; - } - } - next(); - }; -} - -export { maxLength, validateStringLengths }; diff --git a/server/src/nest/app.module.ts b/server/src/nest/app.module.ts index 54511383c..366c2636e 100644 --- a/server/src/nest/app.module.ts +++ b/server/src/nest/app.module.ts @@ -71,8 +71,8 @@ import { IdempotencyInterceptor } from './common/idempotency.interceptor'; // equivalent of the legacy Express app.get('*') catch-all). @Catch(NotFoundException) // is more specific than TrekExceptionFilter, so Nest routes 404s here. { provide: APP_FILTER, useClass: SpaFallbackFilter }, - // Replays the X-Idempotency-Key the client sends on every write, matching - // the legacy applyIdempotency middleware so retried mutations don't double-apply. + // Replays the X-Idempotency-Key the client sends on every write, so retried + // mutations don't double-apply. { provide: APP_INTERCEPTOR, useClass: IdempotencyInterceptor }, // Global Zod validation: any parameter typed with a createZodDto class // (the .dto.ts wrappers over @trek/shared schemas) is validated; diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index 4485bbc6b..e60343206 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -25,7 +25,7 @@ import { startTripReminders } from '../../scheduler'; import { deleteUserCompletely } from '../../services/userCleanupService'; import { emitUserDeleted } from '../../plugin-user-lifecycle'; import { getFlightDistanceKm } from '../../services/distanceService'; -import { verifyJwtAndLoadUser } from '../../middleware/auth'; +import { verifyJwtAndLoadUser } from './jwt-verify'; import { User } from '../../types'; import { DEMO_EMAIL_PRIMARY, isDemoEmail } from '../common/demo'; import { avatarUrl } from '../common/avatarUrl'; @@ -1410,7 +1410,7 @@ export class AuthService { } /** - * Verify a JWT the same way `middleware/auth.ts#verifyJwtAndLoadUser` + * Verify a JWT the same way `auth/jwt-verify.ts#verifyJwtAndLoadUser` * does — including the `password_version` check — so that stolen tokens * lose access the moment the victim resets their password. * diff --git a/server/src/nest/auth/cookie-auth.guard.ts b/server/src/nest/auth/cookie-auth.guard.ts index c7c0dea30..6c93bdc19 100644 --- a/server/src/nest/auth/cookie-auth.guard.ts +++ b/server/src/nest/auth/cookie-auth.guard.ts @@ -1,6 +1,6 @@ import { CanActivate, ExecutionContext, HttpException, Injectable } from '@nestjs/common'; import type { Request } from 'express'; -import { verifyJwtAndLoadUser } from '../../middleware/auth'; +import { verifyJwtAndLoadUser } from './jwt-verify'; /** * Mirrors the legacy `requireCookieAuth` middleware: accepts ONLY the httpOnly diff --git a/server/src/nest/auth/jwt-auth.guard.ts b/server/src/nest/auth/jwt-auth.guard.ts index 1e1f6c6fb..e622826d8 100644 --- a/server/src/nest/auth/jwt-auth.guard.ts +++ b/server/src/nest/auth/jwt-auth.guard.ts @@ -1,6 +1,6 @@ import { CanActivate, ExecutionContext, HttpException, Injectable } from '@nestjs/common'; import type { Request } from 'express'; -import { extractToken, verifyJwtAndLoadUser } from '../../middleware/auth'; +import { extractToken, verifyJwtAndLoadUser } from './jwt-verify'; /** * Validates TREK's existing JWT session — the same httpOnly `trek_session` diff --git a/server/src/nest/auth/jwt-verify.ts b/server/src/nest/auth/jwt-verify.ts new file mode 100644 index 000000000..24280e326 --- /dev/null +++ b/server/src/nest/auth/jwt-verify.ts @@ -0,0 +1,59 @@ +import type { Request } from 'express'; +import jwt from 'jsonwebtoken'; +import { db } from '../../db/database'; +import { JWT_SECRET } from '../../config'; +import type { User } from '../../types'; + +/** + * The canonical JWT session check. Every auth surface goes through here — the + * three guards, MCP bearer tokens, the file-download query token, the + * photo-serving route and the global MFA policy — so the secret, the + * password_version gate and the loaded user cannot drift between them. + * + * Free functions rather than a provider: the MFA policy in + * middleware/mfaPolicy.ts and the platform routes run outside the container and + * would have no way to inject one. Note JWT_SECRET is deliberately a live + * binding from src/config, not an app-config value: the admin panel rotates it + * at runtime and `export let` is what makes a rotation take effect in-process. + */ +export 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']; + return (authHeader && authHeader.split(' ')[1]) || null; +} + +/** + * Verify a JWT and load its user, enforcing the password_version gate. + * + * A password reset bumps `users.password_version`, which invalidates every JWT + * that embedded the prior value — but only if every verify path actually + * compares the claim. Several paths used to call `jwt.verify` directly and skip + * the DB lookup, so a stolen token kept working after the victim reset. + */ +export function verifyJwtAndLoadUser(token: string): User | null { + try { + const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number; pv?: number; purpose?: string }; + // Purpose-scoped tokens (e.g. the short-lived mfa_login token) share this + // secret but are not full session tokens — only their dedicated endpoint + // may accept them, so reject any token carrying a purpose claim here. + if (decoded.purpose) return null; + const row = db.prepare( + 'SELECT id, username, email, role, password_version FROM users WHERE id = ?' + ).get(decoded.id) as (User & { password_version?: number }) | undefined; + if (!row) return null; + // Session invalidation: any token whose embedded password_version + // predates the user's current one is rejected. Tokens issued before + // the `pv` claim existed (decoded.pv === undefined) are treated as + // version 0 so legacy sessions keep working until the user resets. + const tokenPv = typeof decoded.pv === 'number' ? decoded.pv : 0; + const currentPv = typeof row.password_version === 'number' ? row.password_version : 0; + if (tokenPv !== currentPv) return null; + // Don't leak password_version beyond the verify. + const { password_version: _pv, ...user } = row; + return user as User; + } catch { + return null; + } +} diff --git a/server/src/nest/auth/optional-jwt.guard.ts b/server/src/nest/auth/optional-jwt.guard.ts index 4c1ade6db..58588f4ca 100644 --- a/server/src/nest/auth/optional-jwt.guard.ts +++ b/server/src/nest/auth/optional-jwt.guard.ts @@ -1,6 +1,6 @@ import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common'; import type { Request } from 'express'; -import { extractToken, verifyJwtAndLoadUser } from '../../middleware/auth'; +import { extractToken, verifyJwtAndLoadUser } from './jwt-verify'; /** * Mirrors the legacy `optionalAuth` middleware: populates req.user with the diff --git a/server/src/nest/common/demo.ts b/server/src/nest/common/demo.ts index 7cbf6a3de..8e30cd3b3 100644 --- a/server/src/nest/common/demo.ts +++ b/server/src/nest/common/demo.ts @@ -2,7 +2,8 @@ // // Historical: the demo account was seeded as "demo@trek.app" (see // authService.demoLogin), but several guards — demoUploadBlock in -// middleware/auth.ts, the MFA/backup-code bypasses in authService — +// the upload block that used to live in middleware/auth.ts, the MFA/backup-code +// bypasses in authService — // were still checking the pre-rename "demo@nomad.app" string, so they // either never fired or silently diverged between call sites. Routing // every check through this constant keeps them aligned. diff --git a/server/src/nest/common/idempotency.interceptor.ts b/server/src/nest/common/idempotency.interceptor.ts index d7a91c279..7908f65e3 100644 --- a/server/src/nest/common/idempotency.interceptor.ts +++ b/server/src/nest/common/idempotency.interceptor.ts @@ -4,9 +4,8 @@ import { Observable, of } from 'rxjs'; import { DatabaseService } from '../database/database.service'; /** - * Nest counterpart of the legacy `applyIdempotency` middleware - * (server/src/middleware/idempotency.ts), which the Express `authenticate` - * middleware runs on every authenticated request. + * Replaces the `applyIdempotency` middleware the Express `authenticate` ran on + * every authenticated request. Both are gone; this is the only implementation. * * The TREK client attaches an `X-Idempotency-Key` to ALL write operations (see * client/src/api/client.ts) and the offline sync queue replays mutations with diff --git a/server/src/nest/files/files.service.ts b/server/src/nest/files/files.service.ts index f58cc71a2..32fd14e88 100644 --- a/server/src/nest/files/files.service.ts +++ b/server/src/nest/files/files.service.ts @@ -7,7 +7,7 @@ import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; import { avatarUrl } from '../common/avatarUrl'; import { consumeEphemeralToken } from '../../services/ephemeralTokens'; -import { verifyJwtAndLoadUser } from '../../middleware/auth'; +import { verifyJwtAndLoadUser } from '../auth/jwt-verify'; import type { User, TripFile } from '../../types'; import { DatabaseService, type TripAccess } from '../database/database.service'; import { DEFAULT_ALLOWED_EXTENSIONS, filesDir } from './files.constants'; diff --git a/server/src/nest/platform/platform.routes.ts b/server/src/nest/platform/platform.routes.ts index e373ce662..16b78fa7e 100644 --- a/server/src/nest/platform/platform.routes.ts +++ b/server/src/nest/platform/platform.routes.ts @@ -3,7 +3,7 @@ import path from 'node:path'; import fs from 'node:fs'; import { readEnv } from '../../app-config'; -import { verifyJwtAndLoadUser } from '../../middleware/auth'; +import { verifyJwtAndLoadUser } from '../auth/jwt-verify'; import { db } from '../../db/database'; import { mcpHandler } from '../../mcp'; import { trekOAuthProvider, trekClientsStore } from '../../mcp/oauthProvider'; diff --git a/server/src/nest/plugins/plugins-proxy.controller.ts b/server/src/nest/plugins/plugins-proxy.controller.ts index 16a127d8b..b8f447142 100644 --- a/server/src/nest/plugins/plugins-proxy.controller.ts +++ b/server/src/nest/plugins/plugins-proxy.controller.ts @@ -1,6 +1,6 @@ import { All, Controller, Param, Req, Res } from '@nestjs/common'; import type { Request, Response } from 'express'; -import { extractToken, verifyJwtAndLoadUser } from '../../middleware/auth'; +import { extractToken, verifyJwtAndLoadUser } from '../auth/jwt-verify'; import { pluginsEnabled } from './kill-switch'; import { PluginRuntimeService } from './plugin-runtime.service'; diff --git a/server/src/types.ts b/server/src/types.ts index d5adc6549..0822199fd 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -307,15 +307,6 @@ export interface Setting { value?: string | null; } -export interface AuthRequest extends Request { - user: User; - trip?: { id: number; user_id: number }; -} - -export interface OptionalAuthRequest extends Request { - user: User | null; -} - export interface AssignmentRow extends DayAssignment { place_name: string; place_description: string | null; diff --git a/server/tests/unit/middleware/auth.test.ts b/server/tests/unit/middleware/auth.test.ts deleted file mode 100644 index 78bdda9ec..000000000 --- a/server/tests/unit/middleware/auth.test.ts +++ /dev/null @@ -1,184 +0,0 @@ -import { describe, it, expect, vi, afterEach } from 'vitest'; -import jwt from 'jsonwebtoken'; - -vi.mock('../../../src/db/database', () => ({ - db: { prepare: vi.fn(() => ({ get: vi.fn(), all: vi.fn() })) }, -})); -vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret' })); - -import { extractToken, authenticate, adminOnly } from '../../../src/middleware/auth'; -import { db } from '../../../src/db/database'; -import type { Request, Response, NextFunction } from 'express'; - -function makeReq(overrides: { - cookies?: Record; - headers?: Record; -} = {}): Request { - return { - cookies: overrides.cookies || {}, - headers: overrides.headers || {}, - } as unknown as Request; -} - -function makeRes(): { res: Response; status: ReturnType; json: ReturnType } { - const json = vi.fn(); - const status = vi.fn(() => ({ json })); - const res = { status } as unknown as Response; - return { res, status, json }; -} - -// ── extractToken ───────────────────────────────────────────────────────────── - -describe('extractToken', () => { - it('returns cookie value when trek_session cookie is set', () => { - const req = makeReq({ cookies: { trek_session: 'cookie-token' } }); - expect(extractToken(req)).toBe('cookie-token'); - }); - - it('returns Bearer token from Authorization header when no cookie', () => { - const req = makeReq({ headers: { authorization: 'Bearer header-token' } }); - expect(extractToken(req)).toBe('header-token'); - }); - - it('prefers cookie over Authorization header when both are present', () => { - const req = makeReq({ - cookies: { trek_session: 'cookie-token' }, - headers: { authorization: 'Bearer header-token' }, - }); - expect(extractToken(req)).toBe('cookie-token'); - }); - - it('returns null when neither cookie nor header are present', () => { - expect(extractToken(makeReq())).toBeNull(); - }); - - it('returns null for Authorization header without a token (empty Bearer)', () => { - const req = makeReq({ headers: { authorization: 'Bearer ' } }); - expect(extractToken(req)).toBeNull(); - }); - - it('returns null for Authorization header without Bearer prefix', () => { - const req = makeReq({ headers: { authorization: 'Basic sometoken' } }); - // split(' ')[1] returns 'sometoken' — this IS returned (not a null case) - // The function simply splits on space and takes index 1 - expect(extractToken(req)).toBe('sometoken'); - }); -}); - -// ── authenticate ───────────────────────────────────────────────────────────── - -describe('authenticate', () => { - it('returns 401 when no token is present', () => { - const next = vi.fn() as unknown as NextFunction; - const { res, status, json } = makeRes(); - authenticate(makeReq(), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(401); - expect(json).toHaveBeenCalledWith(expect.objectContaining({ code: 'AUTH_REQUIRED' })); - }); - - it('returns 401 when JWT is invalid', () => { - const next = vi.fn() as unknown as NextFunction; - const { res, status } = makeRes(); - authenticate(makeReq({ cookies: { trek_session: 'invalid.jwt.token' } }), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(401); - }); - - it('AUTH-MW-003: calls next() and sets req.user for a valid JWT', () => { - const mockUser = { id: 1, username: 'alice', email: 'alice@example.com', role: 'user' }; - vi.mocked(db.prepare).mockReturnValue({ get: vi.fn(() => mockUser), all: vi.fn() } as any); - - const token = jwt.sign({ id: 1 }, 'test-secret', { algorithm: 'HS256' }); - const req = makeReq({ cookies: { trek_session: token } }); - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - - authenticate(req, res, next); - - expect(next).toHaveBeenCalledOnce(); - expect((req as any).user).toEqual(mockUser); - }); - - it('AUTH-MW-004: returns 401 for a valid JWT when user does not exist in DB', () => { - vi.mocked(db.prepare).mockReturnValue({ get: vi.fn(() => undefined), all: vi.fn() } as any); - - const token = jwt.sign({ id: 99999 }, 'test-secret', { algorithm: 'HS256' }); - const next = vi.fn() as unknown as NextFunction; - const { res, status } = makeRes(); - - authenticate(makeReq({ cookies: { trek_session: token } }), res, next); - - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(401); - }); - - it('AUTH-MW-005: returns 401 for an expired JWT', () => { - const expiredToken = jwt.sign( - { id: 1, exp: Math.floor(Date.now() / 1000) - 3600 }, - 'test-secret', - { algorithm: 'HS256' } - ); - const next = vi.fn() as unknown as NextFunction; - const { res, status } = makeRes(); - authenticate(makeReq({ cookies: { trek_session: expiredToken } }), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(401); - }); - - it('AUTH-MW-006: returns 401 for a JWT signed with the wrong secret', () => { - const tamperedToken = jwt.sign({ id: 1 }, 'wrong-secret', { algorithm: 'HS256' }); - const next = vi.fn() as unknown as NextFunction; - const { res, status } = makeRes(); - authenticate(makeReq({ cookies: { trek_session: tamperedToken } }), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(401); - }); - - it('AUTH-MW-007: rejects a purpose-scoped mfa_login token even when the user is valid', () => { - // The token issued after the password check but before TOTP is signed with - // the same secret. It must never authenticate a normal request, otherwise - // password alone grants full access and MFA is bypassed. - const mockUser = { id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 0 }; - vi.mocked(db.prepare).mockReturnValue({ get: vi.fn(() => mockUser), all: vi.fn() } as any); - - const mfaToken = jwt.sign({ id: 1, purpose: 'mfa_login', pv: 0 }, 'test-secret', { algorithm: 'HS256' }); - const next = vi.fn() as unknown as NextFunction; - const { res, status } = makeRes(); - - authenticate(makeReq({ headers: { authorization: `Bearer ${mfaToken}` } }), res, next); - - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(401); - }); -}); - -// ── adminOnly ───────────────────────────────────────────────────────────────── - -describe('adminOnly', () => { - it('returns 403 when user role is not admin', () => { - const next = vi.fn() as unknown as NextFunction; - const { res, status, json } = makeRes(); - const req = { ...makeReq(), user: { id: 1, role: 'user' } } as unknown as Request; - adminOnly(req, res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(403); - expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringContaining('Admin') })); - }); - - it('calls next() when user role is admin', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - const req = { ...makeReq(), user: { id: 1, role: 'admin' } } as unknown as Request; - adminOnly(req, res, next); - expect(next).toHaveBeenCalled(); - }); - - it('returns 403 when req.user is undefined', () => { - const next = vi.fn() as unknown as NextFunction; - const { res, status } = makeRes(); - adminOnly(makeReq() as unknown as Request, res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(403); - }); -}); diff --git a/server/tests/unit/middleware/idempotency.test.ts b/server/tests/unit/middleware/idempotency.test.ts deleted file mode 100644 index 28bebadac..000000000 --- a/server/tests/unit/middleware/idempotency.test.ts +++ /dev/null @@ -1,176 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; - -// ── In-memory store + DB mock using vi.hoisted ──────────────────────────────── -const { rows, dbMock } = vi.hoisted(() => { - const rows: Record = {}; - - const dbMock = { - db: { - prepare: vi.fn((sql: string) => ({ - get: vi.fn((...args: unknown[]) => { - const [key, userId, method, path] = args; - return rows[`${key}:${userId}:${method}:${path}`] ?? undefined; - }), - run: vi.fn((...args: unknown[]) => { - const [key, userId, method, path, status_code, response_body] = args as [string, number, string, string, number, string]; - const k = `${key}:${userId}:${method}:${path}`; - if (!rows[k]) rows[k] = { status_code, response_body }; - }), - })), - }, - }; - - return { rows, dbMock }; -}); - -vi.mock('../../../src/db/database', () => dbMock); - -import { applyIdempotency } from '../../../src/middleware/idempotency'; -import type { Request, Response, NextFunction } from 'express'; - -function makeReq(method = 'POST', headers: Record = {}, path = '/api/test'): Request { - return { method, path, headers } as unknown as Request; -} - -function makeRes(statusCode = 200): Response { - const ctx = { status: statusCode }; - const res = { - get statusCode() { return ctx.status; }, - status(code: number) { ctx.status = code; return res; }, - json: vi.fn((_body: unknown) => res), - } as unknown as Response; - return res; -} - -beforeEach(() => { - Object.keys(rows).forEach(k => delete rows[k]); - vi.clearAllMocks(); -}); - -describe('applyIdempotency', () => { - it('calls next() for GET requests', () => { - const req = makeReq('GET', { 'x-idempotency-key': 'key1' }); - const res = makeRes(); - const next = vi.fn(); - applyIdempotency(req, res, next, 1); - expect(next).toHaveBeenCalledOnce(); - }); - - it('calls next() when header is absent for POST', () => { - const req = makeReq('POST', {}); - const res = makeRes(); - const next = vi.fn(); - applyIdempotency(req, res, next, 1); - expect(next).toHaveBeenCalledOnce(); - }); - - it('replays cached response when key+user+method+path already stored', () => { - rows['cached-key:42:POST:/api/test'] = { status_code: 201, response_body: JSON.stringify({ id: 99 }) }; - const req = makeReq('POST', { 'x-idempotency-key': 'cached-key' }); - const res = makeRes(); - const next = vi.fn(); - applyIdempotency(req, res, next, 42); - expect(next).not.toHaveBeenCalled(); - expect(res.json as ReturnType).toHaveBeenCalledWith({ id: 99 }); - }); - - it('different user same key does NOT replay', () => { - rows['cached-key:1:POST:/api/test'] = { status_code: 200, response_body: JSON.stringify({ ok: true }) }; - const req = makeReq('POST', { 'x-idempotency-key': 'cached-key' }); - const res = makeRes(); - const next = vi.fn(); - applyIdempotency(req, res, next, 99); // different user - expect(next).toHaveBeenCalledOnce(); - }); - - it('same key+user on different path does NOT replay (scoped cache)', () => { - // Key 'dual-key' is cached under /api/a but reused against /api/b. - // Without the (key, user_id, method, path) scoping, /api/b would - // have replayed /api/a's body — a silent cross-endpoint leak. - rows['dual-key:7:POST:/api/a'] = { status_code: 200, response_body: JSON.stringify({ from: 'a' }) }; - const req = makeReq('POST', { 'x-idempotency-key': 'dual-key' }, '/api/b'); - const res = makeRes(); - const next = vi.fn(() => { - (res.json as ReturnType)({ from: 'b' }); - }); - applyIdempotency(req, res, next, 7); - expect(next).toHaveBeenCalledOnce(); - expect(rows['dual-key:7:POST:/api/b']).toBeDefined(); - expect(JSON.parse(rows['dual-key:7:POST:/api/b'].response_body)).toEqual({ from: 'b' }); - // /api/a's row is untouched. - expect(JSON.parse(rows['dual-key:7:POST:/api/a'].response_body)).toEqual({ from: 'a' }); - }); - - it('same key+user+path but different method does NOT replay', () => { - rows['m-key:3:POST:/api/x'] = { status_code: 201, response_body: JSON.stringify({ m: 'post' }) }; - const req = makeReq('PATCH', { 'x-idempotency-key': 'm-key' }, '/api/x'); - const res = makeRes(); - const next = vi.fn(); - applyIdempotency(req, res, next, 3); - expect(next).toHaveBeenCalledOnce(); - }); - - it('stores 2xx response on first execution via wrapped res.json', () => { - const req = makeReq('POST', { 'x-idempotency-key': 'new-key' }); - const res = makeRes(201); - const next = vi.fn(() => { - // Simulate handler calling res.json - (res.json as ReturnType)({ id: 5 }); - }); - applyIdempotency(req, res, next, 7); - expect(next).toHaveBeenCalledOnce(); - expect(rows['new-key:7:POST:/api/test']).toBeDefined(); - expect(rows['new-key:7:POST:/api/test'].status_code).toBe(201); - expect(JSON.parse(rows['new-key:7:POST:/api/test'].response_body)).toEqual({ id: 5 }); - }); - - it('does NOT store 4xx responses', () => { - const req = makeReq('POST', { 'x-idempotency-key': 'fail-key' }); - const res = makeRes(422); - const next = vi.fn(() => { - (res.json as ReturnType)({ error: 'Invalid' }); - }); - applyIdempotency(req, res, next, 3); - expect(rows['fail-key:3:POST:/api/test']).toBeUndefined(); - }); - - it('returns 400 when X-Idempotency-Key exceeds 128 characters', () => { - const longKey = 'a'.repeat(129); - const req = makeReq('POST', { 'x-idempotency-key': longKey }); - const res = makeRes(); - const next = vi.fn(); - applyIdempotency(req, res, next, 1); - expect(next).not.toHaveBeenCalled(); - expect(res.json as ReturnType).toHaveBeenCalledWith( - expect.objectContaining({ error: expect.stringContaining('128') }), - ); - }); - - it('does NOT cache response body exceeding 256 KiB', () => { - const req = makeReq('POST', { 'x-idempotency-key': 'big-key' }); - const res = makeRes(200); - const originalJsonSpy = res.json as ReturnType; - const largePayload = { data: 'x'.repeat(256 * 1024 + 1) }; - const next = vi.fn(() => { - // res.json is now the wrapper; calling it exercises the size-cap branch - res.json(largePayload); - }); - applyIdempotency(req, res, next, 5); - expect(next).toHaveBeenCalledOnce(); - // Underlying spy was called (response reached the client) - expect(originalJsonSpy).toHaveBeenCalledWith(largePayload); - // But NOT stored in the idempotency store - expect(rows['big-key:5:POST:/api/test']).toBeUndefined(); - }); - - it('handles PUT, PATCH, and DELETE the same as POST', () => { - for (const method of ['PUT', 'PATCH', 'DELETE'] as const) { - const req = makeReq(method, { 'x-idempotency-key': `key-${method}` }); - const res = makeRes(200); - const next = vi.fn(); - applyIdempotency(req, res, next, 1); - expect(next).toHaveBeenCalled(); - vi.clearAllMocks(); - } - }); -}); diff --git a/server/tests/unit/middleware/validate.test.ts b/server/tests/unit/middleware/validate.test.ts deleted file mode 100644 index ea5ee56da..000000000 --- a/server/tests/unit/middleware/validate.test.ts +++ /dev/null @@ -1,109 +0,0 @@ -import { describe, it, expect, vi } from 'vitest'; -import { maxLength, validateStringLengths } from '../../../src/middleware/validate'; -import type { Request, Response, NextFunction } from 'express'; - -function makeReq(body: Record = {}): Request { - return { body } as Request; -} - -function makeRes(): { res: Response; status: ReturnType; json: ReturnType } { - const json = vi.fn(); - const status = vi.fn(() => ({ json })); - const res = { status } as unknown as Response; - return { res, status, json }; -} - -// ── maxLength ──────────────────────────────────────────────────────────────── - -describe('maxLength', () => { - it('calls next() when field is absent from body', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - maxLength('name', 10)(makeReq({}), res, next); - expect(next).toHaveBeenCalled(); - }); - - it('calls next() when field is not a string (number)', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - maxLength('count', 5)(makeReq({ count: 999 }), res, next); - expect(next).toHaveBeenCalled(); - }); - - it('calls next() when string length is within limit', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - maxLength('name', 10)(makeReq({ name: 'hello' }), res, next); - expect(next).toHaveBeenCalled(); - }); - - it('calls next() when string length equals max exactly', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - maxLength('name', 5)(makeReq({ name: 'hello' }), res, next); - expect(next).toHaveBeenCalled(); - }); - - it('returns 400 when field exceeds max', () => { - const next = vi.fn() as unknown as NextFunction; - const { res, status, json } = makeRes(); - maxLength('name', 4)(makeReq({ name: 'hello' }), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(400); - expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringContaining('name') })); - }); - - it('error message includes field name and max length', () => { - const next = vi.fn() as unknown as NextFunction; - const { res, json } = makeRes(); - maxLength('title', 3)(makeReq({ title: 'toolong' }), res, next); - expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringMatching(/title.*3|3.*title/i) })); - }); -}); - -// ── validateStringLengths ──────────────────────────────────────────────────── - -describe('validateStringLengths', () => { - it('calls next() when all fields are within limits', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - validateStringLengths({ name: 10, bio: 100 })(makeReq({ name: 'Alice', bio: 'A short bio' }), res, next); - expect(next).toHaveBeenCalled(); - }); - - it('returns 400 on first field that exceeds its limit', () => { - const next = vi.fn() as unknown as NextFunction; - const { res, status } = makeRes(); - validateStringLengths({ name: 3 })(makeReq({ name: 'toolong' }), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(400); - }); - - it('skips fields not present in body', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - validateStringLengths({ name: 10, missing: 5 })(makeReq({ name: 'Alice' }), res, next); - expect(next).toHaveBeenCalled(); - }); - - it('skips non-string fields', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - validateStringLengths({ count: 5 })(makeReq({ count: 999999 }), res, next); - expect(next).toHaveBeenCalled(); - }); - - it('handles empty maxLengths object — calls next()', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - validateStringLengths({})(makeReq({ anything: 'value' }), res, next); - expect(next).toHaveBeenCalled(); - }); - - it('calls next() only once even if multiple fields are valid', () => { - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - validateStringLengths({ a: 10, b: 10 })(makeReq({ a: 'ok', b: 'ok' }), res, next); - expect(next).toHaveBeenCalledOnce(); - }); -}); diff --git a/server/tests/unit/nest/auth-guard.test.ts b/server/tests/unit/nest/auth-guard.test.ts index 646b2f665..2743adf5c 100644 --- a/server/tests/unit/nest/auth-guard.test.ts +++ b/server/tests/unit/nest/auth-guard.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { HttpException } from '@nestjs/common'; import type { Request } from 'express'; -vi.mock('../../../src/middleware/auth', () => ({ extractToken: vi.fn(), verifyJwtAndLoadUser: vi.fn() })); +vi.mock('../../../src/nest/auth/jwt-verify', () => ({ extractToken: vi.fn(), verifyJwtAndLoadUser: vi.fn() })); vi.mock('../../../src/nest/common/cookie', () => ({ setAuthCookie: vi.fn() })); vi.mock('../../../src/nest/audit/client-ip', () => ({ getClientIp: vi.fn(() => '1.2.3.4') })); vi.mock('../../../src/nest/audit/audit-log.logger', () => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logDebug: vi.fn(), logError: vi.fn(), logWarn: vi.fn() })); @@ -32,7 +32,7 @@ const passkey = { }; const pc = (limiter: RateLimitService) => new PasskeyController(limiter, audit, passkey as unknown as PasskeyService); import { CurrentUser } from '../../../src/nest/auth/current-user.decorator'; -import { extractToken, verifyJwtAndLoadUser } from '../../../src/middleware/auth'; +import { extractToken, verifyJwtAndLoadUser } from '../../../src/nest/auth/jwt-verify'; import type { AuthService } from '../../../src/nest/auth/auth.service'; import type { PasskeyService } from '../../../src/nest/auth/passkey.service'; import { setAuthCookie } from '../../../src/nest/common/cookie'; diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 80d7e9509..258a5419c 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -77,7 +77,7 @@ import * as authBridge from '../../../src/nest/auth/auth.bridge'; import { AtlasService } from '../../../src/nest/atlas/atlas.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { DatabaseService } from '../../../src/nest/database/database.service'; -import { verifyJwtAndLoadUser } from '../../../src/middleware/auth'; +import { verifyJwtAndLoadUser } from '../../../src/nest/auth/jwt-verify'; import { authenticator } from 'otplib'; import { hashBackupCode } from '../../../src/nest/auth/auth.helpers'; import { createEphemeralToken } from '../../../src/services/ephemeralTokens'; diff --git a/server/tests/unit/nest/auth/jwt-verify.test.ts b/server/tests/unit/nest/auth/jwt-verify.test.ts new file mode 100644 index 000000000..0017fdf40 --- /dev/null +++ b/server/tests/unit/nest/auth/jwt-verify.test.ts @@ -0,0 +1,135 @@ +/** + * The canonical JWT session check, moved here with the verify itself. + * + * These assertions come from tests/unit/middleware/auth.test.ts, which exercised + * them through the Express `authenticate` middleware — a wrapper that no longer + * exists. They now hit verifyJwtAndLoadUser directly, so a 401 is expressed as + * "returns null" rather than "sets status 401"; the guards own the status codes + * and assert them in auth-guard.test.ts. + * + * The password_version gate gets direct cases here for the first time. It is the + * reason this function exists at all: a reset bumps users.password_version and + * every JWT carrying the prior value has to stop working. + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import jwt from 'jsonwebtoken'; + +vi.mock('../../../../src/db/database', () => ({ + db: { prepare: vi.fn(() => ({ get: vi.fn(), all: vi.fn() })) }, +})); +vi.mock('../../../../src/config', () => ({ JWT_SECRET: 'test-secret' })); + +import { extractToken, verifyJwtAndLoadUser } from '../../../../src/nest/auth/jwt-verify'; +import { db } from '../../../../src/db/database'; +import type { Request } from 'express'; + +function makeReq(overrides: { + cookies?: Record; + headers?: Record; +} = {}): Request { + return { + cookies: overrides.cookies || {}, + headers: overrides.headers || {}, + } as unknown as Request; +} + +/** Stub the single users row the verify reads. */ +function userRow(row: Record | undefined): void { + vi.mocked(db.prepare).mockReturnValue({ get: vi.fn(() => row), all: vi.fn() } as never); +} + +afterEach(() => vi.clearAllMocks()); + +describe('extractToken', () => { + it('returns cookie value when trek_session cookie is set', () => { + expect(extractToken(makeReq({ cookies: { trek_session: 'cookie-token' } }))).toBe('cookie-token'); + }); + + it('returns Bearer token from Authorization header when no cookie', () => { + expect(extractToken(makeReq({ headers: { authorization: 'Bearer header-token' } }))).toBe('header-token'); + }); + + it('prefers cookie over Authorization header when both are present', () => { + const req = makeReq({ + cookies: { trek_session: 'cookie-token' }, + headers: { authorization: 'Bearer header-token' }, + }); + expect(extractToken(req)).toBe('cookie-token'); + }); + + it('returns null when neither cookie nor header are present', () => { + expect(extractToken(makeReq())).toBeNull(); + }); + + it('returns null for Authorization header without a token (empty Bearer)', () => { + expect(extractToken(makeReq({ headers: { authorization: 'Bearer ' } }))).toBeNull(); + }); + + it('returns the second word for a non-Bearer scheme — it splits on space, it does not parse', () => { + expect(extractToken(makeReq({ headers: { authorization: 'Basic sometoken' } }))).toBe('sometoken'); + }); +}); + +describe('verifyJwtAndLoadUser', () => { + it('AUTH-JWT-001: returns the user for a valid token, without password_version', () => { + userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 0 }); + const token = jwt.sign({ id: 1 }, 'test-secret', { algorithm: 'HS256' }); + + expect(verifyJwtAndLoadUser(token)).toEqual({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user' }); + }); + + it('AUTH-JWT-002: returns null for a malformed token', () => { + expect(verifyJwtAndLoadUser('invalid.jwt.token')).toBeNull(); + }); + + it('AUTH-JWT-003: returns null when the user no longer exists', () => { + userRow(undefined); + expect(verifyJwtAndLoadUser(jwt.sign({ id: 99999 }, 'test-secret', { algorithm: 'HS256' }))).toBeNull(); + }); + + it('AUTH-JWT-004: returns null for an expired token', () => { + const expired = jwt.sign({ id: 1, exp: Math.floor(Date.now() / 1000) - 3600 }, 'test-secret', { algorithm: 'HS256' }); + expect(verifyJwtAndLoadUser(expired)).toBeNull(); + }); + + it('AUTH-JWT-005: returns null for a token signed with the wrong secret', () => { + expect(verifyJwtAndLoadUser(jwt.sign({ id: 1 }, 'wrong-secret', { algorithm: 'HS256' }))).toBeNull(); + }); + + it('AUTH-JWT-006: rejects a purpose-scoped mfa_login token even when the user is valid', () => { + // Issued after the password check but before TOTP, signed with the same + // secret. It must never authenticate an ordinary request. + userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 0 }); + const mfaToken = jwt.sign({ id: 1, purpose: 'mfa_login' }, 'test-secret', { algorithm: 'HS256' }); + + expect(verifyJwtAndLoadUser(mfaToken)).toBeNull(); + }); + + it('AUTH-JWT-007: rejects a token whose password_version predates the user row', () => { + userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 2 }); + const stale = jwt.sign({ id: 1, pv: 1 }, 'test-secret', { algorithm: 'HS256' }); + + expect(verifyJwtAndLoadUser(stale)).toBeNull(); + }); + + it('AUTH-JWT-008: accepts a token whose password_version matches', () => { + userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 2 }); + const current = jwt.sign({ id: 1, pv: 2 }, 'test-secret', { algorithm: 'HS256' }); + + expect(verifyJwtAndLoadUser(current)).not.toBeNull(); + }); + + it('AUTH-JWT-009: a pre-pv token still works against a never-reset user (both read as 0)', () => { + userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: null }); + const legacy = jwt.sign({ id: 1 }, 'test-secret', { algorithm: 'HS256' }); + + expect(verifyJwtAndLoadUser(legacy)).not.toBeNull(); + }); + + it('AUTH-JWT-010: but a pre-pv token stops working once the user has reset', () => { + userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 1 }); + const legacy = jwt.sign({ id: 1 }, 'test-secret', { algorithm: 'HS256' }); + + expect(verifyJwtAndLoadUser(legacy)).toBeNull(); + }); +}); diff --git a/server/tests/unit/nest/files.service.test.ts b/server/tests/unit/nest/files.service.test.ts index ea8102fa2..8eb85ec03 100644 --- a/server/tests/unit/nest/files.service.test.ts +++ b/server/tests/unit/nest/files.service.test.ts @@ -49,7 +49,7 @@ const checkPermission = vi.fn(() => true); const permissionsStub = { checkPermission } as unknown as PermissionsService; const { verifyJwtAndLoadUser } = vi.hoisted(() => ({ verifyJwtAndLoadUser: vi.fn() })); -vi.mock('../../../src/middleware/auth', () => ({ verifyJwtAndLoadUser })); +vi.mock('../../../src/nest/auth/jwt-verify', () => ({ verifyJwtAndLoadUser })); const { consumeEphemeralToken } = vi.hoisted(() => ({ consumeEphemeralToken: vi.fn() })); vi.mock('../../../src/services/ephemeralTokens', () => ({ consumeEphemeralToken })); diff --git a/server/tests/unit/nest/platform.controller.test.ts b/server/tests/unit/nest/platform.controller.test.ts index 801259248..8eb16b854 100644 --- a/server/tests/unit/nest/platform.controller.test.ts +++ b/server/tests/unit/nest/platform.controller.test.ts @@ -16,7 +16,7 @@ const h = vi.hoisted(() => ({ mcpHandler: vi.fn(), })); -vi.mock('../../../src/middleware/auth', () => ({ verifyJwtAndLoadUser: h.verifyJwtAndLoadUser })); +vi.mock('../../../src/nest/auth/jwt-verify', () => ({ verifyJwtAndLoadUser: h.verifyJwtAndLoadUser })); vi.mock('../../../src/db/database', () => ({ db: { prepare: h.dbPrepare } })); vi.mock('../../../src/mcp', () => ({ mcpHandler: h.mcpHandler })); vi.mock('../../../src/mcp/oauthProvider', () => ({ trekOAuthProvider: {}, trekClientsStore: {} })); diff --git a/server/tests/unit/plugins/plugins-proxy.test.ts b/server/tests/unit/plugins/plugins-proxy.test.ts index 08449dc1b..14c619c4b 100644 --- a/server/tests/unit/plugins/plugins-proxy.test.ts +++ b/server/tests/unit/plugins/plugins-proxy.test.ts @@ -11,7 +11,7 @@ const { pluginsEnabledMock, extractTokenMock, verifyMock } = vi.hoisted(() => ({ verifyMock: vi.fn(() => ({ id: 5, username: 'ada', role: 'user' })), })); vi.mock('../../../src/nest/plugins/kill-switch', () => ({ pluginsEnabled: pluginsEnabledMock })); -vi.mock('../../../src/middleware/auth', () => ({ extractToken: extractTokenMock, verifyJwtAndLoadUser: verifyMock })); +vi.mock('../../../src/nest/auth/jwt-verify', () => ({ extractToken: extractTokenMock, verifyJwtAndLoadUser: verifyMock })); import { PluginsProxyController } from '../../../src/nest/plugins/plugins-proxy.controller'; import type { PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.service';