diff --git a/server/src/middleware/globalMiddleware.ts b/server/src/middleware/globalMiddleware.ts index 5c3264c9e..0555dec3e 100644 --- a/server/src/middleware/globalMiddleware.ts +++ b/server/src/middleware/globalMiddleware.ts @@ -5,7 +5,6 @@ import helmet from 'helmet'; import cookieParser from 'cookie-parser'; import { readEnv, type AppEnv } from '../app-config'; import { logDebug, logWarn, logError } from '../nest/audit/audit-log.logger'; -import { enforceGlobalMfaPolicy } from './mfaPolicy'; /** * The global request pipeline shared by the legacy Express app and the NestJS @@ -175,7 +174,6 @@ export function applyGlobalMiddleware( app.use(express.urlencoded({ extended: true })); } app.use(cookieParser()); - app.use(enforceGlobalMfaPolicy); // Request logging with sensitive field redaction const SENSITIVE_KEYS = new Set(['password', 'new_password', 'current_password', 'token', 'jwt', 'authorization', 'cookie', 'client_secret', 'mfa_token', 'code', 'smtp_pass']); diff --git a/server/src/middleware/mfaPolicy.ts b/server/src/middleware/mfaPolicy.ts deleted file mode 100644 index e0fc0483d..000000000 --- a/server/src/middleware/mfaPolicy.ts +++ /dev/null @@ -1,112 +0,0 @@ -import { Request, Response, NextFunction } from 'express'; -import { readEnv } from '../app-config'; -import { db } from '../db/database'; -import { extractToken, verifyJwtAndLoadUser } from '../nest/auth/jwt-verify'; -import { DEMO_EMAILS } from '../nest/common/demo'; - -/** Paths that never require MFA (public or pre-auth). */ -export function isPublicApiPath(method: string, pathNoQuery: string): boolean { - if (method === 'GET' && pathNoQuery === '/api/health') return true; - if (method === 'GET' && pathNoQuery === '/api/auth/app-config') return true; - if (method === 'POST' && pathNoQuery === '/api/auth/login') return true; - if (method === 'POST' && pathNoQuery === '/api/auth/register') return true; - if (method === 'POST' && pathNoQuery === '/api/auth/demo-login') return true; - if (method === 'GET' && pathNoQuery.startsWith('/api/auth/invite/')) return true; - if (method === 'POST' && pathNoQuery === '/api/auth/mfa/verify-login') return true; - // Unauthenticated passkey (primary) login ceremony. - if (method === 'POST' && pathNoQuery === '/api/auth/passkey/login/options') return true; - if (method === 'POST' && pathNoQuery === '/api/auth/passkey/login/verify') return true; - if (pathNoQuery.startsWith('/api/auth/oidc/')) return true; - return false; -} - -/** Authenticated paths allowed while MFA is not yet enabled (setup + lockout recovery). */ -export function isMfaSetupExemptPath(method: string, pathNoQuery: string): boolean { - if (method === 'GET' && pathNoQuery === '/api/auth/me') return true; - if (method === 'POST' && pathNoQuery === '/api/auth/mfa/setup') return true; - if (method === 'POST' && pathNoQuery === '/api/auth/mfa/enable') return true; - // Allow enrolling a passkey as the second factor (a user-verified passkey - // satisfies require_mfa), so a fresh user under the policy isn't stuck. - if (method === 'POST' && pathNoQuery === '/api/auth/passkey/register/options') return true; - if (method === 'POST' && pathNoQuery === '/api/auth/passkey/register/verify') return true; - if (method === 'GET' && pathNoQuery === '/api/auth/passkey/credentials') return true; - if ((method === 'GET' || method === 'PUT') && pathNoQuery === '/api/auth/app-settings') return true; - return false; -} - -/** - * When app_settings.require_mfa is true, block API access for users without MFA enabled, - * except for public routes and MFA setup endpoints. - */ -export function enforceGlobalMfaPolicy(req: Request, res: Response, next: NextFunction): void { - const pathNoQuery = (req.originalUrl || req.url || '').split('?')[0]; - - if (!pathNoQuery.startsWith('/api')) { - next(); - return; - } - - if (isPublicApiPath(req.method, pathNoQuery)) { - next(); - return; - } - - // Accept both the httpOnly session cookie (regular SPA users) and the - // Authorization header (MCP / API clients). Previously this only looked - // at the header so every normal cookie-authenticated session sailed - // past `require_mfa` unchecked. - const token = extractToken(req); - if (!token) { - next(); - return; - } - - // Use the shared verify helper so the `password_version` gate applies - // here too — a JWT stolen before a password reset would otherwise - // continue to satisfy this middleware until its natural 24h expiry. - const verified = verifyJwtAndLoadUser(token); - if (!verified) { - next(); - return; - } - const userId = verified.id; - - const requireRow = db.prepare("SELECT value FROM app_settings WHERE key = 'require_mfa'").get() as { value: string } | undefined; - if (requireRow?.value !== 'true') { - next(); - return; - } - - if (readEnv().demo.enabled && verified.email && DEMO_EMAILS.has(verified.email)) { - next(); - return; - } - - const row = db.prepare('SELECT mfa_enabled FROM users WHERE id = ?').get(userId) as - | { mfa_enabled: number | boolean } - | undefined; - if (!row) { - next(); - return; - } - - // A user-verified passkey is phishing-resistant and inherently two-factor, so - // owning at least one satisfies the require_mfa policy exactly like TOTP does. - // (All stored passkeys were registered with userVerification required.) - const mfaOk = row.mfa_enabled === 1 || row.mfa_enabled === true; - const passkeyOk = !!db.prepare('SELECT 1 FROM webauthn_credentials WHERE user_id = ? LIMIT 1').get(userId); - if (mfaOk || passkeyOk) { - next(); - return; - } - - if (isMfaSetupExemptPath(req.method, pathNoQuery)) { - next(); - return; - } - - res.status(403).json({ - error: 'Two-factor authentication is required. Complete setup in Settings.', - code: 'MFA_REQUIRED', - }); -} diff --git a/server/src/nest/app.module.ts b/server/src/nest/app.module.ts index 776834846..5e1c8ff06 100644 --- a/server/src/nest/app.module.ts +++ b/server/src/nest/app.module.ts @@ -6,6 +6,7 @@ import { DatabaseModule } from './database/database.module'; import { RealtimeModule } from './realtime/realtime.module'; import { HealthModule } from './health/health.module'; import { GlobalAuthGuard } from './auth/global-auth.guard'; +import { MfaPolicyGuard } from './auth/mfa-policy.guard'; import { WeatherModule } from './weather/weather.module'; import { HelpModule } from './help/help.module'; import { AirportsModule } from './airports/airports.module'; @@ -69,6 +70,10 @@ import { IdempotencyInterceptor } from './common/idempotency.interceptor'; // @OptionalAuth(), or declares its own @UseGuards chain. Protection used to // be opt-in, which made a forgotten guard a silent bypass instead of an error. { provide: APP_GUARD, useClass: GlobalAuthGuard }, + // Second, and only second: it reads the user the guard above resolved + // instead of verifying the token a second time, which is what the Express + // middleware it replaces did on every /api request. + { provide: APP_GUARD, useClass: MfaPolicyGuard }, // Global error-envelope normaliser (DI-registered so it also catches // framework-level exceptions like the not-found handler). { provide: APP_FILTER, useClass: TrekExceptionFilter }, diff --git a/server/src/nest/auth/auth-public.controller.ts b/server/src/nest/auth/auth-public.controller.ts index 09c98d1b0..a107711c4 100644 --- a/server/src/nest/auth/auth-public.controller.ts +++ b/server/src/nest/auth/auth-public.controller.ts @@ -9,6 +9,7 @@ import { AuditService } from '../audit/audit.service'; import { willDropSecureCookie } from '../common/cookie'; import type { User } from '../../types'; import { Public } from './public.decorator'; +import { MfaExempt } from './mfa-policy.guard'; const WINDOW = 15 * 60 * 1000; const LOGIN_MIN_LATENCY_MS = 350; @@ -35,6 +36,7 @@ export class AuthPublicController { } @Get('app-config') + @MfaExempt('bootstrap config, read before the client knows whether it must set up MFA') @UseGuards(OptionalJwtGuard) appConfig(@Req() req: Request) { return this.auth.getAppConfig((req.user as User | undefined) ?? undefined); diff --git a/server/src/nest/auth/auth.controller.ts b/server/src/nest/auth/auth.controller.ts index 2ab443b23..96a183611 100644 --- a/server/src/nest/auth/auth.controller.ts +++ b/server/src/nest/auth/auth.controller.ts @@ -43,6 +43,7 @@ import { CurrentUser } from './current-user.decorator'; import { getClientIp } from '../audit/client-ip'; import { AuditService } from '../audit/audit.service'; import type { User } from '../../types'; +import { MfaExempt } from './mfa-policy.guard'; const WINDOW = 15 * 60 * 1000; const ALLOWED_AVATAR_EXTS = ['.jpg', '.jpeg', '.png', '.gif', '.webp']; @@ -83,6 +84,7 @@ export class AuthController { } @Get('me') + @MfaExempt('the client needs to know who it is to render the setup screen') me(@CurrentUser() user: User) { const loaded = this.auth.getCurrentUser(user.id); if (!loaded) { @@ -176,6 +178,7 @@ export class AuthController { } @Get('app-settings') + @MfaExempt('the setup screen reads the policy it is asking the user to satisfy') getAppSettings(@CurrentUser() user: User) { const result = this.auth.getAppSettings(user.id); if (result.error) { @@ -185,6 +188,7 @@ export class AuthController { } @Put('app-settings') + @MfaExempt('an admin locked out by their own policy must still be able to lift it') updateAppSettings(@CurrentUser() user: User, @Body() body: AppSettingsUpdateDto, @Req() req: Request) { const result = this.auth.updateAppSettings(user.id, body); if (result.error) { @@ -199,6 +203,7 @@ export class AuthController { // AtlasModule import. @Post('mfa/setup') + @MfaExempt('completing setup is the way out of the policy') @HttpCode(200) async mfaSetup(@CurrentUser() user: User) { const result = this.auth.setupMfa(user.id, user.email); @@ -215,6 +220,7 @@ export class AuthController { } @Post('mfa/enable') + @MfaExempt('completing setup is the way out of the policy') @HttpCode(200) mfaEnable(@CurrentUser() user: User, @Body() body: MfaEnableDto, @Req() req: Request) { this.limit('mfa', req, 5); diff --git a/server/src/nest/auth/global-auth.guard.ts b/server/src/nest/auth/global-auth.guard.ts index 39b6d149b..eea4d1dd5 100644 --- a/server/src/nest/auth/global-auth.guard.ts +++ b/server/src/nest/auth/global-auth.guard.ts @@ -44,12 +44,22 @@ export class GlobalAuthGuard implements CanActivate { return true; } - // A declared chain decides for itself, in the order it declared. + // A declared chain decides for itself, in the order it declared — but the + // user still gets resolved here, without refusing. Global guards run before + // route guards, so standing down silently would leave req.user unset for the + // MFA guard behind us, and it would wave every request through. Resolving + // without throwing keeps the declared chain in charge of the 401 while the + // rest of the chain can still see who is calling. Absent or invalid token + // leaves req.user null, which is what it was before. const declared = [ ...(this.reflector.get(GUARDS_METADATA, handler) ?? []), ...(this.reflector.get(GUARDS_METADATA, controller) ?? []), ]; - if (declared.length > 0) return true; + if (declared.length > 0) { + const declaredToken = extractToken(req); + (req as { user: unknown }).user = (declaredToken ? verifyJwtAndLoadUser(declaredToken) : null) || null; + return true; + } const token = extractToken(req); if (!token) { diff --git a/server/src/nest/auth/mfa-policy.guard.ts b/server/src/nest/auth/mfa-policy.guard.ts new file mode 100644 index 000000000..cf66f9b16 --- /dev/null +++ b/server/src/nest/auth/mfa-policy.guard.ts @@ -0,0 +1,93 @@ +import { CanActivate, ExecutionContext, HttpException, Injectable, SetMetadata } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import type { Request } from 'express'; +import { DatabaseService } from '../database/database.service'; +import { RuntimeEnvService } from '../app-config/runtime-env.service'; +import { DEMO_EMAILS } from '../common/demo'; +import { IS_PUBLIC } from './public.decorator'; +import type { User } from '../../types'; + +/** Metadata key `@MfaExempt()` writes. */ +export const MFA_EXEMPT = 'trek:mfa-exempt'; + +/** + * Reachable by an authenticated user who has not set up MFA yet, while the + * require_mfa policy is on. + * + * Two kinds qualify and both need the reason spelled out: the routes that let + * somebody *complete* setup (otherwise the policy locks out every new account), + * and the pre-auth ceremonies that carry a session but are not yet a session the + * policy should judge. + */ +export const MfaExempt = (reason: string) => SetMetadata(MFA_EXEMPT, { reason }); + +type MfaRequest = Request & { user?: User }; + +/** + * Enforces app_settings.require_mfa, as a guard rather than an Express layer. + * + * The middleware it replaces re-verified the JWT and re-read the users row on + * every single /api request, then threw the loaded user away — two jwt.verify + * calls and two SELECTs per authenticated request, because it ran before Nest + * and could not see what the auth guard had already resolved. This one reads + * `req.user`, which the global auth guard put there. + * + * It also corrects an asymmetry that had grown quietly. The middleware decided + * on two literal path lists, and every public /api endpoint added after those + * lists were written — /api/config, /api/help/*, the public journey and share + * routes, /api/health/features — answered a logged-in user without MFA with a + * 403 while answering the same request from a stranger perfectly well. Nothing + * intended that; the lists simply stopped being maintained. Keying on @Public() + * instead of a path list means the exemption cannot drift from the route again. + * That is a deliberate behaviour change, and the only one here. + */ +@Injectable() +export class MfaPolicyGuard implements CanActivate { + constructor( + private readonly db: DatabaseService, + private readonly env: RuntimeEnvService, + private readonly reflector: Reflector, + ) {} + + canActivate(context: ExecutionContext): boolean { + const handler = context.getHandler(); + const controller = context.getClass(); + + // A route a stranger may call is a route the policy has no business judging. + if (this.reflector.getAllAndOverride(IS_PUBLIC, [handler, controller])) return true; + + const user = context.switchToHttp().getRequest().user; + // No session: whoever answers next decides, exactly as before. + if (!user) return true; + + const requireRow = this.db.get<{ value: string }>( + "SELECT value FROM app_settings WHERE key = 'require_mfa'", + ); + if (requireRow?.value !== 'true') return true; + + if (this.env.isDemoMode() && user.email && DEMO_EMAILS.has(user.email)) return true; + + const row = this.db.get<{ mfa_enabled: number | boolean }>( + 'SELECT mfa_enabled FROM users WHERE id = ?', + user.id, + ); + if (!row) return true; + + // A user-verified passkey is phishing-resistant and inherently two-factor, so + // owning at least one satisfies require_mfa exactly like TOTP does. + // (All stored passkeys were registered with userVerification required.) + const mfaOk = row.mfa_enabled === 1 || row.mfa_enabled === true; + const passkeyOk = !!this.db.get('SELECT 1 FROM webauthn_credentials WHERE user_id = ? LIMIT 1', user.id); + if (mfaOk || passkeyOk) return true; + + if (this.reflector.getAllAndOverride(MFA_EXEMPT, [handler, controller])) return true; + + throw new HttpException( + { + error: 'Two-factor authentication is required. Complete setup in Settings.', + code: 'MFA_REQUIRED', + }, + 403, + ); + } +} diff --git a/server/src/nest/auth/passkey.controller.ts b/server/src/nest/auth/passkey.controller.ts index f2b1982dd..1b2a18b52 100644 --- a/server/src/nest/auth/passkey.controller.ts +++ b/server/src/nest/auth/passkey.controller.ts @@ -10,6 +10,7 @@ import { AuditService } from '../audit/audit.service'; import { PasskeyService } from './passkey.service'; import { PasskeyRegisterOptionsDto, PasskeyRegisterVerifyDto, PasskeyLoginVerifyDto, PasskeyRenameDto, PasskeyDeleteDto } from './auth.dto'; import type { User } from '../../types'; +import { MfaExempt } from './mfa-policy.guard'; const WINDOW = 15 * 60 * 1000; const LOGIN_MIN_LATENCY_MS = 350; @@ -43,6 +44,7 @@ export class PasskeyController { // ── Registration (authenticated) ── @Post('register/options') + @MfaExempt('a user-verified passkey satisfies require_mfa, so enrolling one is a way out') @HttpCode(200) @UseGuards(PasskeyEnabledGuard, JwtAuthGuard) async registerOptions(@CurrentUser() user: User, @Body() body: PasskeyRegisterOptionsDto, @Req() req: Request) { @@ -53,6 +55,7 @@ export class PasskeyController { } @Post('register/verify') + @MfaExempt('a user-verified passkey satisfies require_mfa, so enrolling one is a way out') @HttpCode(200) @UseGuards(PasskeyEnabledGuard, JwtAuthGuard) async registerVerify(@CurrentUser() user: User, @Body() body: PasskeyRegisterVerifyDto, @Req() req: Request) { @@ -64,6 +67,7 @@ export class PasskeyController { // ── Authentication (public — primary login) ── @Post('login/options') + @MfaExempt('unauthenticated passkey login ceremony') @HttpCode(200) @UseGuards(PasskeyEnabledGuard) async loginOptions(@Req() req: Request) { @@ -74,6 +78,7 @@ export class PasskeyController { } @Post('login/verify') + @MfaExempt('unauthenticated passkey login ceremony') @HttpCode(200) @UseGuards(PasskeyEnabledGuard) async loginVerify(@Body() body: PasskeyLoginVerifyDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { @@ -95,6 +100,7 @@ export class PasskeyController { // ── Management (authenticated, owner-scoped — NOT toggle-gated) ── @Get('credentials') + @MfaExempt('the setup screen lists what the user already has') @UseGuards(JwtAuthGuard) list(@CurrentUser() user: User) { return { credentials: this.passkeys.listPasskeys(user.id) }; diff --git a/server/tests/unit/middleware/mfaPolicy.test.ts b/server/tests/unit/middleware/mfaPolicy.test.ts deleted file mode 100644 index cc3d8a7c1..000000000 --- a/server/tests/unit/middleware/mfaPolicy.test.ts +++ /dev/null @@ -1,100 +0,0 @@ -import { describe, it, expect, vi } from 'vitest'; - -vi.mock('../../../src/db/database', () => ({ - db: { prepare: () => ({ get: vi.fn(), all: vi.fn() }) }, -})); -vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret' })); - -import { isPublicApiPath, isMfaSetupExemptPath } from '../../../src/middleware/mfaPolicy'; - -// ── isPublicApiPath ────────────────────────────────────────────────────────── - -describe('isPublicApiPath', () => { - // AUTH-001 — Public paths must bypass MFA - it('AUTH-001: GET /api/health is public', () => { - expect(isPublicApiPath('GET', '/api/health')).toBe(true); - }); - - it('GET /api/auth/app-config is public', () => { - expect(isPublicApiPath('GET', '/api/auth/app-config')).toBe(true); - }); - - it('POST /api/auth/login is public', () => { - expect(isPublicApiPath('POST', '/api/auth/login')).toBe(true); - }); - - it('POST /api/auth/register is public', () => { - expect(isPublicApiPath('POST', '/api/auth/register')).toBe(true); - }); - - it('POST /api/auth/demo-login is public', () => { - expect(isPublicApiPath('POST', '/api/auth/demo-login')).toBe(true); - }); - - it('GET /api/auth/invite/ is public', () => { - expect(isPublicApiPath('GET', '/api/auth/invite/abc123')).toBe(true); - expect(isPublicApiPath('GET', '/api/auth/invite/xyz-789')).toBe(true); - }); - - it('POST /api/auth/mfa/verify-login is public', () => { - expect(isPublicApiPath('POST', '/api/auth/mfa/verify-login')).toBe(true); - }); - - it('OIDC paths are public (any method)', () => { - expect(isPublicApiPath('GET', '/api/auth/oidc/callback')).toBe(true); - expect(isPublicApiPath('POST', '/api/auth/oidc/login')).toBe(true); - expect(isPublicApiPath('GET', '/api/auth/oidc/discovery')).toBe(true); - }); - - it('GET /api/trips is not public', () => { - expect(isPublicApiPath('GET', '/api/trips')).toBe(false); - }); - - it('POST /api/auth/login with wrong method (GET) is not public', () => { - expect(isPublicApiPath('GET', '/api/auth/login')).toBe(false); - }); - - it('GET /api/auth/me is not public', () => { - expect(isPublicApiPath('GET', '/api/auth/me')).toBe(false); - }); - - it('DELETE /api/auth/logout is not public', () => { - expect(isPublicApiPath('DELETE', '/api/auth/logout')).toBe(false); - }); -}); - -// ── isMfaSetupExemptPath ───────────────────────────────────────────────────── - -describe('isMfaSetupExemptPath', () => { - it('GET /api/auth/me is MFA-setup exempt', () => { - expect(isMfaSetupExemptPath('GET', '/api/auth/me')).toBe(true); - }); - - it('POST /api/auth/mfa/setup is MFA-setup exempt', () => { - expect(isMfaSetupExemptPath('POST', '/api/auth/mfa/setup')).toBe(true); - }); - - it('POST /api/auth/mfa/enable is MFA-setup exempt', () => { - expect(isMfaSetupExemptPath('POST', '/api/auth/mfa/enable')).toBe(true); - }); - - it('GET /api/auth/app-settings is MFA-setup exempt', () => { - expect(isMfaSetupExemptPath('GET', '/api/auth/app-settings')).toBe(true); - }); - - it('PUT /api/auth/app-settings is MFA-setup exempt', () => { - expect(isMfaSetupExemptPath('PUT', '/api/auth/app-settings')).toBe(true); - }); - - it('POST /api/auth/app-settings is NOT exempt (wrong method)', () => { - expect(isMfaSetupExemptPath('POST', '/api/auth/app-settings')).toBe(false); - }); - - it('GET /api/trips is NOT exempt', () => { - expect(isMfaSetupExemptPath('GET', '/api/trips')).toBe(false); - }); - - it('GET /api/auth/logout is NOT exempt', () => { - expect(isMfaSetupExemptPath('GET', '/api/auth/logout')).toBe(false); - }); -}); diff --git a/server/tests/unit/nest/mfa-policy.guard.test.ts b/server/tests/unit/nest/mfa-policy.guard.test.ts new file mode 100644 index 000000000..d555ace87 --- /dev/null +++ b/server/tests/unit/nest/mfa-policy.guard.test.ts @@ -0,0 +1,157 @@ +/** + * MfaPolicyGuard — the require_mfa policy, moved off the Express layer. + * + * The cases that matter here are the exemptions, because that is where the + * middleware had drifted: it decided on two literal path lists, and every public + * /api endpoint added after those lists were written answered a logged-in user + * without MFA with a 403 while answering a stranger fine. MFA-011 pins the + * correction. + */ +import { describe, it, expect, vi } from 'vitest'; +import { HttpException } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { MfaExempt, MFA_EXEMPT, MfaPolicyGuard } from '../../../src/nest/auth/mfa-policy.guard'; +import { Public, IS_PUBLIC } from '../../../src/nest/auth/public.decorator'; +import type { DatabaseService } from '../../../src/nest/database/database.service'; +import type { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; +import { DEMO_EMAIL_PRIMARY } from '../../../src/nest/common/demo'; +import type { User } from '../../../src/types'; + +const user = { id: 7, email: 'u@example.test', role: 'user' } as User; + +interface Rows { + requireMfa?: string; + mfaEnabled?: number | boolean | undefined; + hasPasskey?: boolean; + userMissing?: boolean; +} + +function makeGuard(rows: Rows = {}, meta: { public?: boolean; exempt?: boolean } = {}, demo = false) { + const get = vi.fn((sql: string) => { + if (sql.includes('app_settings')) return rows.requireMfa === undefined ? undefined : { value: rows.requireMfa }; + if (sql.includes('FROM users')) return rows.userMissing ? undefined : { mfa_enabled: rows.mfaEnabled ?? 0 }; + if (sql.includes('webauthn_credentials')) return rows.hasPasskey ? { 1: 1 } : undefined; + return undefined; + }); + const reflector = { + getAllAndOverride: vi.fn((key: string) => + (key === IS_PUBLIC && meta.public) || (key === MFA_EXEMPT && meta.exempt) ? { reason: 'x' } : undefined, + ), + } as unknown as Reflector; + const guard = new MfaPolicyGuard( + { get } as unknown as DatabaseService, + { isDemoMode: () => demo } as unknown as RuntimeEnvService, + reflector, + ); + return { guard, get }; +} + +function ctx(request: Record) { + return { + switchToHttp: () => ({ getRequest: () => request }), + getHandler: () => function handler() {}, + getClass: () => class Controller {}, + } as never; +} + +const thrown = (run: () => unknown) => { + try { + run(); + } catch (err) { + expect(err).toBeInstanceOf(HttpException); + const e = err as HttpException; + return { status: e.getStatus(), body: e.getResponse() }; + } + throw new Error('expected throw'); +}; + +const ENFORCED: Rows = { requireMfa: 'true', mfaEnabled: 0 }; + +describe('MfaPolicyGuard', () => { + it('MFA-001: refuses a user without MFA while the policy is on, with the legacy body', () => { + const { guard } = makeGuard(ENFORCED); + expect(thrown(() => guard.canActivate(ctx({ user })))).toEqual({ + status: 403, + body: { + error: 'Two-factor authentication is required. Complete setup in Settings.', + code: 'MFA_REQUIRED', + }, + }); + }); + + it('MFA-002: lets a user with TOTP through', () => { + const { guard } = makeGuard({ requireMfa: 'true', mfaEnabled: 1 }); + expect(guard.canActivate(ctx({ user }))).toBe(true); + }); + + it('MFA-003: a user-verified passkey satisfies the policy like TOTP does', () => { + const { guard } = makeGuard({ requireMfa: 'true', mfaEnabled: 0, hasPasskey: true }); + expect(guard.canActivate(ctx({ user }))).toBe(true); + }); + + it('MFA-004: does nothing when the policy is off, and never reads the users row', () => { + const { guard, get } = makeGuard({ requireMfa: 'false', mfaEnabled: 0 }); + expect(guard.canActivate(ctx({ user }))).toBe(true); + expect(get.mock.calls.some((c) => String(c[0]).includes('FROM users'))).toBe(false); + }); + + it('MFA-005: does nothing when the setting row is absent at all', () => { + const { guard } = makeGuard({ mfaEnabled: 0 }); + expect(guard.canActivate(ctx({ user }))).toBe(true); + }); + + it('MFA-006: an anonymous request is not the policy business', () => { + const { guard, get } = makeGuard(ENFORCED); + expect(guard.canActivate(ctx({}))).toBe(true); + expect(get).not.toHaveBeenCalled(); + }); + + it('MFA-007: the demo account is exempt while demo mode is on', () => { + const demoUser = { id: 1, email: DEMO_EMAIL_PRIMARY, role: 'user' } as User; + expect(makeGuard(ENFORCED, {}, true).guard.canActivate(ctx({ user: demoUser }))).toBe(true); + // …and not otherwise. + expect(thrown(() => makeGuard(ENFORCED, {}, false).guard.canActivate(ctx({ user: demoUser })))).toEqual({ + status: 403, + body: expect.objectContaining({ code: 'MFA_REQUIRED' }), + }); + }); + + it('MFA-008: a user row that vanished mid-request is let through, not 403d', () => { + const { guard } = makeGuard({ requireMfa: 'true', userMissing: true }); + expect(guard.canActivate(ctx({ user }))).toBe(true); + }); + + it('MFA-009: @MfaExempt is the way out — setup would be unreachable otherwise', () => { + const { guard } = makeGuard(ENFORCED, { exempt: true }); + expect(guard.canActivate(ctx({ user }))).toBe(true); + }); + + it('MFA-010: it reads req.user rather than verifying the token again', () => { + // The middleware this replaces called jwt.verify and SELECTed the users row a + // second time on every /api request, then discarded the result. + const { guard, get } = makeGuard({ requireMfa: 'true', mfaEnabled: 1 }); + guard.canActivate(ctx({ user, headers: { authorization: 'Bearer nonsense' } })); + expect(get.mock.calls.map((c) => c[0]).every((sql) => !String(sql).includes('password_version'))).toBe(true); + }); + + it('MFA-011: a @Public route no longer 403s a logged-in user without MFA', () => { + // The correction. /api/config, /api/help/*, the public journey and share + // routes and /api/health/features all answered a stranger fine and answered + // this user with a 403, because they were added after the middleware path + // lists were written and nobody noticed. + const { guard, get } = makeGuard(ENFORCED, { public: true }); + expect(guard.canActivate(ctx({ user }))).toBe(true); + expect(get).not.toHaveBeenCalled(); + }); + + it('MFA-012: the decorators write what the guard reads', () => { + class Probe { + @MfaExempt('setup would be unreachable') + exempt() {} + @Public('anonymous') + open() {} + } + expect(Reflect.getMetadata(MFA_EXEMPT, Probe.prototype.exempt)).toEqual({ reason: 'setup would be unreachable' }); + expect(Reflect.getMetadata(IS_PUBLIC, Probe.prototype.open)).toEqual({ reason: 'anonymous' }); + }); +});