From 9ba5815f1b02f0760d4adc2e3d545630318f77e0 Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Thu, 14 May 2026 01:29:37 +0200 Subject: [PATCH] feat(portal-bff): @RequireMfa decorator + freshness guard (ADR-0011) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Designed-in, dormant per ADR-0011 §"Step-up MFA". This PR ships the guard, the decorator, and the audit integration — no v1 route uses the decorator yet. First consumer will be the admin entry route per ADR-0020 (`@RequireMfa({ freshness: 600 })`) once the distinct admin session is in place. Mechanics: - `auth/mfa.ts` exports the documented `MFA_AMR_VALUES` allow-list (mfa, otp, fido, wia, phr) and `wasMultiFactor(amr)`. Adding a value is an ADR-recorded decision; the spec pins the list. - `config/check-mfa-config.ts` reads `MFA_FRESHNESS_SECONDS` with default 600 s + minimum 60 s. Anything below the floor fails the validator (the floor catches "MFA on every navigation" misconfiguration). - `RequireMfaGuard` (`auth/require-mfa.guard.ts`): - No session → 401 `unauthenticated`, no audit. - Session with no MFA-class `amr` → 401 `mfa_required` + audit `auth.mfa_required reason=no-mfa-in-amr`. - Session with no `mfaVerifiedAt` → 401 + audit `reason=no-mfa-verified-at`. - Session with stale `mfaVerifiedAt` → 401 + audit `reason=mfa-stale` (includes `mfaAgeMs` payload field). - Same audit-write propagation posture as `AdminRoleGuard`. - The 401 carries `code: 'mfa_required'` in the structured error envelope. The `reason` discriminator is NOT surfaced over the wire — only the audit row carries it, so an attacker can't fingerprint sessions by probing. - `RequireMfaOptions.freshness` overrides the env default per-route. Read via `Reflector.getAllAndOverride` with method-level metadata winning over class-level — Nest's standard merge. - `WWW-Authenticate` header + MSAL claims-challenge blob (ADR-0011 §"Step-up MFA — designed-in" step 2) defer to a later PR — they need MSAL Node integration AND the SPA interceptor to consume them. The structured `code: 'mfa_required'` is sufficient for the SPA to pivot on once the interceptor lands. Session payload: - `session.mfaVerifiedAt` added to the express-session augmentation in `session.types.ts`. Set to `Date.now()` at sign-in by the callback — Entra's CA policy is the authority on whether MFA actually happened; the BFF just stamps "now" when persisting a session whose `amr` reflects MFA. Refreshed by future step-up re-auth flows. Tests: +37 specs (mfa helpers 9, config reader 9, guard 12, audit typed method 3, callback assertion +1, +3 parametric expansions). --- .../src/audit/audit.service.spec.ts | 51 ++++ apps/portal-bff/src/audit/audit.service.ts | 27 +++ apps/portal-bff/src/audit/audit.types.ts | 18 ++ .../src/auth/auth.controller.spec.ts | 22 ++ apps/portal-bff/src/auth/auth.controller.ts | 7 + apps/portal-bff/src/auth/auth.module.ts | 4 +- apps/portal-bff/src/auth/mfa.spec.ts | 35 +++ apps/portal-bff/src/auth/mfa.ts | 35 +++ .../src/auth/require-mfa.decorator.ts | 47 ++++ .../src/auth/require-mfa.guard.spec.ts | 223 ++++++++++++++++++ apps/portal-bff/src/auth/require-mfa.guard.ts | 144 +++++++++++ .../src/config/check-mfa-config.spec.ts | 58 +++++ .../portal-bff/src/config/check-mfa-config.ts | 41 ++++ apps/portal-bff/src/session/session.types.ts | 15 ++ 14 files changed, 726 insertions(+), 1 deletion(-) create mode 100644 apps/portal-bff/src/auth/mfa.spec.ts create mode 100644 apps/portal-bff/src/auth/mfa.ts create mode 100644 apps/portal-bff/src/auth/require-mfa.decorator.ts create mode 100644 apps/portal-bff/src/auth/require-mfa.guard.spec.ts create mode 100644 apps/portal-bff/src/auth/require-mfa.guard.ts create mode 100644 apps/portal-bff/src/config/check-mfa-config.spec.ts create mode 100644 apps/portal-bff/src/config/check-mfa-config.ts diff --git a/apps/portal-bff/src/audit/audit.service.spec.ts b/apps/portal-bff/src/audit/audit.service.spec.ts index 9082ced..a6e39ca 100644 --- a/apps/portal-bff/src/audit/audit.service.spec.ts +++ b/apps/portal-bff/src/audit/audit.service.spec.ts @@ -316,6 +316,57 @@ describe('AuditWriter — typed event methods', () => { }); }); + describe('mfaRequired()', () => { + it('records auth.mfa_required (denied) with reason + freshnessSeconds in payload', async () => { + const { writer, prisma, hashUserId } = await createSubject(); + await writer.mfaRequired({ + actor: { oid: 'user-oid' }, + attemptedRoute: 'GET /api/admin/me', + reason: 'no-mfa-in-amr', + freshnessSeconds: 600, + }); + expect(hashUserId.hash).toHaveBeenCalledWith('user-oid'); + const row = extractInsertedRow(prisma); + expect(row.eventType).toBe('auth.mfa_required'); + expect(row.audience).toBe('workforce'); + expect(row.outcome).toBe('denied'); + expect(row.actorIdHash).toBe('hash(user-oid)'); + expect(row.subject).toBe('GET /api/admin/me'); + expect(row.payloadJson).toBe( + JSON.stringify({ reason: 'no-mfa-in-amr', freshnessSeconds: 600 }), + ); + }); + + it('includes mfaAgeMs in the payload when supplied (stale-session path)', async () => { + const { writer, prisma } = await createSubject(); + await writer.mfaRequired({ + actor: { oid: 'user-oid' }, + attemptedRoute: 'POST /api/admin/cms/pages', + reason: 'mfa-stale', + freshnessSeconds: 600, + mfaAgeMs: 720_000, + }); + expect(extractInsertedRow(prisma).payloadJson).toBe( + JSON.stringify({ reason: 'mfa-stale', freshnessSeconds: 600, mfaAgeMs: 720_000 }), + ); + }); + + it('omits mfaAgeMs from the payload when undefined (no-mfa-verified-at path)', async () => { + const { writer, prisma } = await createSubject(); + await writer.mfaRequired({ + actor: { oid: 'user-oid' }, + attemptedRoute: 'GET /api/admin/me', + reason: 'no-mfa-verified-at', + freshnessSeconds: 600, + }); + const payload = JSON.parse(extractInsertedRow(prisma).payloadJson as string) as Record< + string, + unknown + >; + expect(payload).not.toHaveProperty('mfaAgeMs'); + }); + }); + describe('adminAccessDenied()', () => { it('records admin.access_denied with outcome=denied, attempted route as subject, and rolesHeld payload', async () => { const { writer, prisma, hashUserId } = await createSubject(); diff --git a/apps/portal-bff/src/audit/audit.service.ts b/apps/portal-bff/src/audit/audit.service.ts index 6ed8046..f04550c 100644 --- a/apps/portal-bff/src/audit/audit.service.ts +++ b/apps/portal-bff/src/audit/audit.service.ts @@ -5,6 +5,7 @@ import { PrismaService } from 'nestjs-prisma'; import type { AdminAccessDeniedInput, AuditEventInput, + MfaRequiredInput, SignInActor, SignInFailedInput, SignOutInput, @@ -114,6 +115,32 @@ export class AuditWriter { }); } + /** + * Typed event: a request was rejected by `RequireMfaGuard` because + * the session did not satisfy the route's MFA freshness gate. + * Per ADR-0011 §"Step-up MFA — designed-in, dormant", every + * step-up challenge emitted by the BFF lands here so auditors can + * track the distribution of step-up prompts (and the rate of stale + * `mfaVerifiedAt` rejections that would suggest the default + * freshness is too tight). `outcome=denied` matches the + * `AdminRoleGuard` posture: the user *is* authenticated, the + * action is just refused. + */ + async mfaRequired(input: MfaRequiredInput): Promise { + await this.recordEvent({ + eventType: 'auth.mfa_required', + audience: 'workforce', + outcome: 'denied', + actorIdHash: this.hashUserId.hash(input.actor.oid), + subject: input.attemptedRoute, + payload: { + reason: input.reason, + freshnessSeconds: input.freshnessSeconds, + ...(input.mfaAgeMs !== undefined ? { mfaAgeMs: input.mfaAgeMs } : {}), + }, + }); + } + /** * Typed event: `/api/admin/*` request rejected by `AdminRoleGuard` * because the session's `roles` claim does not include `admin`. diff --git a/apps/portal-bff/src/audit/audit.types.ts b/apps/portal-bff/src/audit/audit.types.ts index 3f9fefa..31d54ae 100644 --- a/apps/portal-bff/src/audit/audit.types.ts +++ b/apps/portal-bff/src/audit/audit.types.ts @@ -100,3 +100,21 @@ export interface AdminAccessDeniedInput { */ rolesHeld: readonly string[]; } + +/** Reason `RequireMfaGuard` rejected a request — fed into the audit payload. */ +export type MfaRequiredReason = 'no-mfa-in-amr' | 'no-mfa-verified-at' | 'mfa-stale'; + +export interface MfaRequiredInput { + actor: { oid: string }; + /** Canonical "{METHOD} {originalUrl}" of the rejected request. */ + attemptedRoute: string; + reason: MfaRequiredReason; + /** + * Freshness window (seconds) the guard checked against. Captured + * so an auditor can tell apart a 10-min default rejection from a + * tighter per-route override. + */ + freshnessSeconds: number; + /** Age (ms) of the session's `mfaVerifiedAt` at the moment of rejection — undefined when the session had none. */ + mfaAgeMs?: number; +} diff --git a/apps/portal-bff/src/auth/auth.controller.spec.ts b/apps/portal-bff/src/auth/auth.controller.spec.ts index e7591ad..4e2a338 100644 --- a/apps/portal-bff/src/auth/auth.controller.spec.ts +++ b/apps/portal-bff/src/auth/auth.controller.spec.ts @@ -57,6 +57,7 @@ interface SessionStub { createdAt?: number; absoluteExpiresAt?: number; csrfToken?: string; + mfaVerifiedAt?: number; save: jest.Mock; destroy: jest.Mock; } @@ -230,6 +231,27 @@ describe('AuthController.callback', () => { expect(res.redirect).toHaveBeenCalledWith(302, ENTRA.postLogoutRedirectUri); }); + it('records mfaVerifiedAt on the session (ADR-0011 §"Confirmation")', async () => { + const before = Date.now(); + const { controller } = makeController(); + const res = makeResStub(); + const session = makeSessionStub(); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + session, + }); + + await controller.callback(req, res, 'auth-code', PRE_AUTH.state); + + const after = Date.now(); + expect(session.mfaVerifiedAt).toBeGreaterThanOrEqual(before); + expect(session.mfaVerifiedAt).toBeLessThanOrEqual(after); + // Posted at the same moment as createdAt — the callback grabs + // `now` once and writes both fields, so the freshness anchor + // and the absolute-timeout anchor share a clock reading. + expect(session.mfaVerifiedAt).toBe(session.createdAt); + }); + it('records createdAt + absoluteExpiresAt on the session (ADR-0010 §"TTL policy")', async () => { // Force the ADR-0010 default for this test — apps/portal-bff/.env // may have a custom SESSION_ABSOLUTE_TIMEOUT_SECONDS for local diff --git a/apps/portal-bff/src/auth/auth.controller.ts b/apps/portal-bff/src/auth/auth.controller.ts index 602c36e..8e8f5d0 100644 --- a/apps/portal-bff/src/auth/auth.controller.ts +++ b/apps/portal-bff/src/auth/auth.controller.ts @@ -132,6 +132,13 @@ export class AuthController { // the SPA's read-only mirror used to echo the value in the // X-CSRF-Token header. req.session.csrfToken = csrfToken; + // MFA freshness anchor per ADR-0011 §"Confirmation". Entra's + // CA policy decides whether MFA actually happened — the BFF + // does not re-validate factors. The stamp reflects "the + // session was just established with whatever assurance Entra + // returned"; `RequireMfaGuard` measures freshness against it. + // Refreshed by future step-up re-auth flows. + req.session.mfaVerifiedAt = now; // Force the save before the redirect: express-session writes // on response end, but the 302 we're about to emit closes the // response before the async store-write would otherwise diff --git a/apps/portal-bff/src/auth/auth.module.ts b/apps/portal-bff/src/auth/auth.module.ts index 6cc5da1..fdb5a6e 100644 --- a/apps/portal-bff/src/auth/auth.module.ts +++ b/apps/portal-bff/src/auth/auth.module.ts @@ -7,6 +7,7 @@ import { AuthController } from './auth.controller'; import { AuthService } from './auth.service'; import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token'; import { MSAL_CLIENT } from './msal-client.token'; +import { RequireMfaGuard } from './require-mfa.guard'; /** * Auth module — owns the Entra ID configuration, the MSAL Node @@ -43,6 +44,7 @@ import { MSAL_CLIENT } from './msal-client.token'; controllers: [AuthController], providers: [ AuthService, + RequireMfaGuard, { provide: ENTRA_CONFIG, useFactory: () => assertEntraConfig(), @@ -88,6 +90,6 @@ import { MSAL_CLIENT } from './msal-client.token'; }), }, ], - exports: [ENTRA_CONFIG, MSAL_CLIENT], + exports: [ENTRA_CONFIG, MSAL_CLIENT, RequireMfaGuard], }) export class AuthModule {} diff --git a/apps/portal-bff/src/auth/mfa.spec.ts b/apps/portal-bff/src/auth/mfa.spec.ts new file mode 100644 index 0000000..126830b --- /dev/null +++ b/apps/portal-bff/src/auth/mfa.spec.ts @@ -0,0 +1,35 @@ +import { MFA_AMR_VALUES, wasMultiFactor } from './mfa'; + +describe('MFA_AMR_VALUES', () => { + it('covers the documented Entra MFA emission tokens', () => { + // Spec assertion against the list per ADR-0011 §"BFF + // verification". Changing this list is a deliberate ADR-recorded + // decision; the test fails loudly when someone edits the const + // without updating the ADR's confirmation block. + expect([...MFA_AMR_VALUES]).toEqual(['mfa', 'otp', 'fido', 'wia', 'phr']); + }); +}); + +describe('wasMultiFactor', () => { + it.each([ + [['mfa']], + [['otp']], + [['fido']], + [['wia']], + [['phr']], + [['pwd', 'mfa']], + [['mfa', 'pwd']], + [['totp', 'pwd', 'fido']], // unknown token mixed in, fido still counts + ])('returns true when amr=%j contains an MFA-class token', (amr) => { + expect(wasMultiFactor(amr)).toBe(true); + }); + + it.each([ + [[]], + [['pwd']], + [['pwd', 'sms']], // 'sms' is not in our allow-list (Entra emits 'otp' instead) + [['unknown']], + ])('returns false when amr=%j carries no MFA-class token', (amr) => { + expect(wasMultiFactor(amr)).toBe(false); + }); +}); diff --git a/apps/portal-bff/src/auth/mfa.ts b/apps/portal-bff/src/auth/mfa.ts new file mode 100644 index 0000000..950e173 --- /dev/null +++ b/apps/portal-bff/src/auth/mfa.ts @@ -0,0 +1,35 @@ +/** + * `amr` claim values that the BFF treats as evidence of multi-factor + * authentication having occurred at sign-in (or step-up). Per + * [ADR-0011](../../../../docs/decisions/0011-mfa-enforcement-entra-conditional-access.md) + * §"BFF verification" the list is the BFF-side allow-list used by + * `@RequireMfa()` to decide whether a session is multi-factored. + * + * - `mfa` — generic "multi-factor" indicator Entra emits when a + * Conditional-Access policy enforced MFA, regardless + * of factor. + * - `otp` — one-time-password (SMS / authenticator app code). + * - `fido` — FIDO2 / WebAuthn (phishing-resistant; the target). + * - `wia` — Windows Integrated Auth (Hello, Kerberos-bound). + * - `phr` — phishing-resistant authenticator (Entra's umbrella + * token for fido + cert-bound flows). + * + * Not exhaustive — the list is reviewed against Microsoft's + * documentation on each security-review cadence. Adding a value here + * is a deliberate ADR-recorded decision; do not add ad-hoc. + * + * Sign-in via password alone surfaces as `["pwd"]` (no MFA value) + * and is rejected by `@RequireMfa()` even when the session itself + * is otherwise valid. + */ +export const MFA_AMR_VALUES = ['mfa', 'otp', 'fido', 'wia', 'phr'] as const; + +/** + * True when at least one entry in the session user's `amr` array is + * in {@link MFA_AMR_VALUES}. Used by `RequireMfaGuard` and by the + * (deferred) callback-time sanity-check per ADR-0011 §"Confirmation". + */ +export function wasMultiFactor(amr: readonly string[]): boolean { + const allowed = new Set(MFA_AMR_VALUES); + return amr.some((v) => allowed.has(v)); +} diff --git a/apps/portal-bff/src/auth/require-mfa.decorator.ts b/apps/portal-bff/src/auth/require-mfa.decorator.ts new file mode 100644 index 0000000..ac40379 --- /dev/null +++ b/apps/portal-bff/src/auth/require-mfa.decorator.ts @@ -0,0 +1,47 @@ +import { SetMetadata, UseGuards, applyDecorators } from '@nestjs/common'; +import { REQUIRE_MFA_METADATA, RequireMfaGuard } from './require-mfa.guard'; + +export interface RequireMfaOptions { + /** + * Override of `MFA_FRESHNESS_SECONDS` for this specific route. + * Useful for surfaces that demand a tighter window than the + * global default (e.g. admin entry uses 10 min explicitly per + * ADR-0020). When omitted, the guard reads the env value at + * resolution time. + */ + freshness?: number; +} + +/** + * `@RequireMfa()` — gates a route on a recent MFA assertion per + * [ADR-0011](../../../../docs/decisions/0011-mfa-enforcement-entra-conditional-access.md). + * + * Behaviour at the guard: + * + * 1. No session → 401 `unauthenticated`. + * 2. `session.user.amr` carries no MFA-class value → 401 + * `mfa_required` (SPA interceptor triggers re-auth). + * 3. `session.mfaVerifiedAt` missing or older than the freshness + * window → 401 `mfa_required` (step-up). + * 4. Otherwise → pass through. + * + * Composable with `@RequireAdmin()`: when both are applied (admin + * entry route per ADR-0020), Nest runs them in registration order. + * Always put `@RequireMfa()` *outside* `@RequireAdmin()` so the + * freshness check fires only after the user is established as an + * admin — keeping the audit log noise-free. + * + * Per-route override: + * + * ```ts + * @RequireMfa({ freshness: 600 }) // 10 min, admin entry + * @Get('me') + * me() { … } + * ``` + */ +export function RequireMfa(options?: RequireMfaOptions): ClassDecorator & MethodDecorator { + return applyDecorators( + SetMetadata(REQUIRE_MFA_METADATA, options ?? {}), + UseGuards(RequireMfaGuard), + ); +} diff --git a/apps/portal-bff/src/auth/require-mfa.guard.spec.ts b/apps/portal-bff/src/auth/require-mfa.guard.spec.ts new file mode 100644 index 0000000..1233347 --- /dev/null +++ b/apps/portal-bff/src/auth/require-mfa.guard.spec.ts @@ -0,0 +1,223 @@ +import { ExecutionContext, UnauthorizedException } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import type { Request } from 'express'; +import { AuditWriter } from '../audit/audit.service'; +import { REQUIRE_MFA_METADATA, RequireMfaGuard } from './require-mfa.guard'; + +interface SessionStub { + user?: { + oid: string; + tid: string; + username: string; + displayName: string; + amr: readonly string[]; + roles: readonly string[]; + }; + mfaVerifiedAt?: number; +} + +function makeRequest(opts: { + session?: SessionStub; + method?: string; + originalUrl?: string; +}): Request { + return { + session: opts.session, + method: opts.method ?? 'GET', + originalUrl: opts.originalUrl ?? '/api/admin/me', + } as unknown as Request; +} + +function makeContext(req: Request): ExecutionContext { + return { + switchToHttp: () => ({ getRequest: () => req as T }), + getHandler: () => () => undefined, + getClass: () => class {}, + } as unknown as ExecutionContext; +} + +function makeAuditStub(): jest.Mocked> { + return { mfaRequired: jest.fn().mockResolvedValue(undefined) }; +} + +function makeReflectorStub(metadata?: { freshness?: number }): Reflector { + return { + getAllAndOverride: jest.fn().mockReturnValue(metadata), + } as unknown as Reflector; +} + +const USER = { + oid: 'user-oid', + tid: 't', + username: 'jane@example', + displayName: 'Jane', + amr: ['pwd', 'mfa'], + roles: ['admin'], +}; + +describe('RequireMfaGuard', () => { + const originalFreshness = process.env['MFA_FRESHNESS_SECONDS']; + + afterEach(() => { + if (originalFreshness === undefined) { + delete process.env['MFA_FRESHNESS_SECONDS']; + } else { + process.env['MFA_FRESHNESS_SECONDS'] = originalFreshness; + } + jest.useRealTimers(); + }); + + it('throws 401 unauthenticated when there is no session', async () => { + const audit = makeAuditStub(); + const guard = new RequireMfaGuard(makeReflectorStub(), audit as unknown as AuditWriter); + const ctx = makeContext(makeRequest({ session: undefined })); + await expect(guard.canActivate(ctx)).rejects.toMatchObject({ + response: { code: 'unauthenticated' }, + }); + expect(audit.mfaRequired).not.toHaveBeenCalled(); + }); + + it('throws 401 unauthenticated when the session has no user', async () => { + const audit = makeAuditStub(); + const guard = new RequireMfaGuard(makeReflectorStub(), audit as unknown as AuditWriter); + const ctx = makeContext(makeRequest({ session: {} })); + await expect(guard.canActivate(ctx)).rejects.toBeInstanceOf(UnauthorizedException); + expect(audit.mfaRequired).not.toHaveBeenCalled(); + }); + + it('throws 401 mfa_required + audits when amr carries no MFA-class token', async () => { + const audit = makeAuditStub(); + const guard = new RequireMfaGuard(makeReflectorStub(), audit as unknown as AuditWriter); + const ctx = makeContext( + makeRequest({ + session: { user: { ...USER, amr: ['pwd'] }, mfaVerifiedAt: Date.now() }, + method: 'POST', + originalUrl: '/api/admin/audit/query', + }), + ); + await expect(guard.canActivate(ctx)).rejects.toMatchObject({ + response: { code: 'mfa_required' }, + }); + expect(audit.mfaRequired).toHaveBeenCalledWith({ + actor: { oid: 'user-oid' }, + attemptedRoute: 'POST /api/admin/audit/query', + reason: 'no-mfa-in-amr', + freshnessSeconds: 600, + }); + }); + + it('throws 401 mfa_required + audits when mfaVerifiedAt is missing from the session', async () => { + const audit = makeAuditStub(); + const guard = new RequireMfaGuard(makeReflectorStub(), audit as unknown as AuditWriter); + const ctx = makeContext(makeRequest({ session: { user: USER /* no mfaVerifiedAt */ } })); + await expect(guard.canActivate(ctx)).rejects.toMatchObject({ + response: { code: 'mfa_required' }, + }); + expect(audit.mfaRequired).toHaveBeenCalledWith({ + actor: { oid: 'user-oid' }, + attemptedRoute: 'GET /api/admin/me', + reason: 'no-mfa-verified-at', + freshnessSeconds: 600, + }); + }); + + it('throws 401 mfa_required + audits with mfaAgeMs when assertion is stale', async () => { + const now = 10_000_000_000; + jest.useFakeTimers(); + jest.setSystemTime(now); + const audit = makeAuditStub(); + const guard = new RequireMfaGuard(makeReflectorStub(), audit as unknown as AuditWriter); + const stale = now - 700_000; // 700 s old, default freshness is 600 s + const ctx = makeContext(makeRequest({ session: { user: USER, mfaVerifiedAt: stale } })); + await expect(guard.canActivate(ctx)).rejects.toMatchObject({ + response: { code: 'mfa_required' }, + }); + expect(audit.mfaRequired).toHaveBeenCalledWith({ + actor: { oid: 'user-oid' }, + attemptedRoute: 'GET /api/admin/me', + reason: 'mfa-stale', + freshnessSeconds: 600, + mfaAgeMs: 700_000, + }); + }); + + it('returns true when MFA assertion is within the freshness window', async () => { + const now = 10_000_000_000; + jest.useFakeTimers(); + jest.setSystemTime(now); + const audit = makeAuditStub(); + const guard = new RequireMfaGuard(makeReflectorStub(), audit as unknown as AuditWriter); + const ctx = makeContext(makeRequest({ session: { user: USER, mfaVerifiedAt: now - 60_000 } })); + await expect(guard.canActivate(ctx)).resolves.toBe(true); + expect(audit.mfaRequired).not.toHaveBeenCalled(); + }); + + it('returns true at the exact freshness boundary (age == freshness)', async () => { + const now = 10_000_000_000; + jest.useFakeTimers(); + jest.setSystemTime(now); + const audit = makeAuditStub(); + const guard = new RequireMfaGuard(makeReflectorStub(), audit as unknown as AuditWriter); + const ctx = makeContext(makeRequest({ session: { user: USER, mfaVerifiedAt: now - 600_000 } })); + await expect(guard.canActivate(ctx)).resolves.toBe(true); + }); + + it('honours a per-route freshness override (tighter than env default)', async () => { + const now = 10_000_000_000; + jest.useFakeTimers(); + jest.setSystemTime(now); + const audit = makeAuditStub(); + // Default env (600 s) would pass; route asks for 60 s, so a + // 5-min-old assertion must be rejected. + const guard = new RequireMfaGuard( + makeReflectorStub({ freshness: 60 }), + audit as unknown as AuditWriter, + ); + const ctx = makeContext( + makeRequest({ session: { user: USER, mfaVerifiedAt: now - 5 * 60_000 } }), + ); + await expect(guard.canActivate(ctx)).rejects.toMatchObject({ + response: { code: 'mfa_required' }, + }); + expect(audit.mfaRequired).toHaveBeenCalledWith( + expect.objectContaining({ reason: 'mfa-stale', freshnessSeconds: 60 }), + ); + }); + + it('honours MFA_FRESHNESS_SECONDS when set and no per-route override', async () => { + process.env['MFA_FRESHNESS_SECONDS'] = '120'; + const now = 10_000_000_000; + jest.useFakeTimers(); + jest.setSystemTime(now); + const audit = makeAuditStub(); + const guard = new RequireMfaGuard(makeReflectorStub(), audit as unknown as AuditWriter); + const ctx = makeContext(makeRequest({ session: { user: USER, mfaVerifiedAt: now - 180_000 } })); + await expect(guard.canActivate(ctx)).rejects.toMatchObject({ + response: { code: 'mfa_required' }, + }); + expect(audit.mfaRequired).toHaveBeenCalledWith( + expect.objectContaining({ freshnessSeconds: 120 }), + ); + }); + + it('propagates audit-write failures (no audit ⇒ no action, per ADR-0013)', async () => { + const audit = { + mfaRequired: jest.fn().mockRejectedValue(new Error('audit_writer denied')), + }; + const guard = new RequireMfaGuard(makeReflectorStub(), audit as unknown as AuditWriter); + const ctx = makeContext(makeRequest({ session: { user: { ...USER, amr: ['pwd'] } } })); + await expect(guard.canActivate(ctx)).rejects.toThrow('audit_writer denied'); + }); + + it('reads the per-route options via the Reflector under the documented metadata key', async () => { + const audit = makeAuditStub(); + const reflector = makeReflectorStub({ freshness: 60 }); + const guard = new RequireMfaGuard(reflector, audit as unknown as AuditWriter); + const ctx = makeContext(makeRequest({ session: { user: USER, mfaVerifiedAt: Date.now() } })); + await guard.canActivate(ctx); + expect(reflector.getAllAndOverride).toHaveBeenCalledWith( + REQUIRE_MFA_METADATA, + expect.any(Array), + ); + }); +}); diff --git a/apps/portal-bff/src/auth/require-mfa.guard.ts b/apps/portal-bff/src/auth/require-mfa.guard.ts new file mode 100644 index 0000000..f8d20fd --- /dev/null +++ b/apps/portal-bff/src/auth/require-mfa.guard.ts @@ -0,0 +1,144 @@ +import { CanActivate, ExecutionContext, Injectable, UnauthorizedException } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import type { Request } from 'express'; +import { AuditWriter } from '../audit/audit.service'; +import type { MfaRequiredReason } from '../audit/audit.types'; +import { readMfaConfig } from '../config/check-mfa-config'; +import { wasMultiFactor } from './mfa'; +import type { RequireMfaOptions } from './require-mfa.decorator'; + +/** + * Reflector metadata key carrying the per-route `RequireMfaOptions`. + * Re-exported by the decorator module so the two stay in lockstep. + */ +export const REQUIRE_MFA_METADATA = 'auth:require-mfa'; + +/** + * 401 response code emitted on every MFA-step-up rejection. The SPA + * interceptor (per ADR-0011 §"Step-up MFA — SPA cooperation") will + * pivot on this exact value to distinguish a step-up challenge from + * a plain "no session" 401 — and trigger an Entra re-auth round + * trip with a claims challenge in a later PR. v1 surfaces the code + * without the `WWW-Authenticate` header / MSAL claims blob; both + * land when the SPA interceptor is built. + */ +const MFA_REQUIRED_CODE = 'mfa_required'; + +/** + * `RequireMfaGuard` — enforces ADR-0011's step-up MFA freshness + * window. + * + * Contract + * -------- + * - **No session → 401 `unauthenticated`.** Same shape as the + * `AdminRoleGuard` 401 branch: no audit row (unauthenticated + * traffic is noise). The user just needs to sign in. + * + * - **Session with no MFA-class `amr` → 401 `mfa_required`.** The + * session is authenticated but does not reflect an MFA assertion + * at all (password-only sign-in, CA policy mis-skipped MFA, etc.). + * Emits `auth.mfa_required` with `reason=no-mfa-in-amr`. + * + * - **Session with stale `mfaVerifiedAt` → 401 `mfa_required`.** + * The user did MFA at some point but the assertion is older than + * the freshness window. Emits `auth.mfa_required` with + * `reason=mfa-stale`. The `mfaAgeMs` payload field lets auditors + * see how far past freshness the rejection landed. + * + * - **Session with no `mfaVerifiedAt` at all → 401 `mfa_required`.** + * Defensive — sessions created before this field was introduced + * would otherwise leak past the guard. Emits + * `auth.mfa_required` with `reason=no-mfa-verified-at`. + * + * - **Audit-write failures propagate.** Same posture as + * `AdminRoleGuard`: no audit ⇒ no action per ADR-0013. + * + * Freshness resolution: per-route `RequireMfaOptions.freshness` + * wins; otherwise the env-driven `MFA_FRESHNESS_SECONDS` (default + * 600 s, min 60 s — validated by `readMfaConfig`). + */ +@Injectable() +export class RequireMfaGuard implements CanActivate { + constructor( + private readonly reflector: Reflector, + private readonly audit: AuditWriter, + ) {} + + async canActivate(context: ExecutionContext): Promise { + const req = context.switchToHttp().getRequest(); + const user = req.session?.user; + + if (!user) { + throw new UnauthorizedException({ + code: 'unauthenticated', + message: 'Unauthenticated', + }); + } + + const options = this.resolveOptions(context); + const freshnessSeconds = options.freshness ?? readMfaConfig().freshnessSeconds; + const route = `${req.method} ${req.originalUrl}`; + const now = Date.now(); + + if (!wasMultiFactor(user.amr)) { + await this.audit.mfaRequired({ + actor: { oid: user.oid }, + attemptedRoute: route, + reason: 'no-mfa-in-amr', + freshnessSeconds, + }); + throw this.mfaChallenge('no-mfa-in-amr'); + } + + const verifiedAt = req.session?.mfaVerifiedAt; + if (verifiedAt === undefined) { + await this.audit.mfaRequired({ + actor: { oid: user.oid }, + attemptedRoute: route, + reason: 'no-mfa-verified-at', + freshnessSeconds, + }); + throw this.mfaChallenge('no-mfa-verified-at'); + } + + const ageMs = now - verifiedAt; + if (ageMs > freshnessSeconds * 1000) { + await this.audit.mfaRequired({ + actor: { oid: user.oid }, + attemptedRoute: route, + reason: 'mfa-stale', + freshnessSeconds, + mfaAgeMs: ageMs, + }); + throw this.mfaChallenge('mfa-stale'); + } + + return true; + } + + private resolveOptions(context: ExecutionContext): RequireMfaOptions { + // Method-level metadata wins over class-level — same merge + // strategy Nest's built-in guards use. `getAllAndOverride` + // returns the first non-undefined match in the lookup order. + return ( + this.reflector.getAllAndOverride(REQUIRE_MFA_METADATA, [ + context.getHandler(), + context.getClass(), + ]) ?? {} + ); + } + + private mfaChallenge(reason: MfaRequiredReason): UnauthorizedException { + // Structured-error envelope per ADR-0021 — the + // StructuredErrorFilter expects `code` + `message` on the + // exception response body. The `reason` is intentionally NOT + // surfaced over the wire: an attacker shouldn't be able to + // distinguish a stale session from a no-MFA session by probing. + // The audit row carries the reason for forensic use. + void reason; + return new UnauthorizedException({ + code: MFA_REQUIRED_CODE, + message: 'Fresh multi-factor authentication required', + }); + } +} diff --git a/apps/portal-bff/src/config/check-mfa-config.spec.ts b/apps/portal-bff/src/config/check-mfa-config.spec.ts new file mode 100644 index 0000000..b8c5154 --- /dev/null +++ b/apps/portal-bff/src/config/check-mfa-config.spec.ts @@ -0,0 +1,58 @@ +import { readMfaConfig } from './check-mfa-config'; + +describe('readMfaConfig', () => { + const original = process.env['MFA_FRESHNESS_SECONDS']; + + afterEach(() => { + if (original === undefined) { + delete process.env['MFA_FRESHNESS_SECONDS']; + } else { + process.env['MFA_FRESHNESS_SECONDS'] = original; + } + }); + + it('defaults to 600 seconds when MFA_FRESHNESS_SECONDS is unset', () => { + delete process.env['MFA_FRESHNESS_SECONDS']; + expect(readMfaConfig()).toEqual({ freshnessSeconds: 600 }); + }); + + it('defaults to 600 seconds when MFA_FRESHNESS_SECONDS is the empty string', () => { + process.env['MFA_FRESHNESS_SECONDS'] = ''; + expect(readMfaConfig()).toEqual({ freshnessSeconds: 600 }); + }); + + it('accepts a positive integer above the minimum', () => { + process.env['MFA_FRESHNESS_SECONDS'] = '1200'; + expect(readMfaConfig()).toEqual({ freshnessSeconds: 1200 }); + }); + + it('accepts the minimum (60) exactly', () => { + process.env['MFA_FRESHNESS_SECONDS'] = '60'; + expect(readMfaConfig()).toEqual({ freshnessSeconds: 60 }); + }); + + it('throws when MFA_FRESHNESS_SECONDS is below the 60 s floor', () => { + process.env['MFA_FRESHNESS_SECONDS'] = '30'; + expect(() => readMfaConfig()).toThrow(/must be ≥ 60/); + }); + + it('throws when MFA_FRESHNESS_SECONDS is not a positive integer', () => { + process.env['MFA_FRESHNESS_SECONDS'] = '0'; + expect(() => readMfaConfig()).toThrow(/positive integer/); + }); + + it('throws when MFA_FRESHNESS_SECONDS is negative', () => { + process.env['MFA_FRESHNESS_SECONDS'] = '-600'; + expect(() => readMfaConfig()).toThrow(/positive integer/); + }); + + it('throws when MFA_FRESHNESS_SECONDS is non-numeric', () => { + process.env['MFA_FRESHNESS_SECONDS'] = 'soon'; + expect(() => readMfaConfig()).toThrow(/positive integer/); + }); + + it('throws when MFA_FRESHNESS_SECONDS is a decimal', () => { + process.env['MFA_FRESHNESS_SECONDS'] = '600.5'; + expect(() => readMfaConfig()).toThrow(/positive integer/); + }); +}); diff --git a/apps/portal-bff/src/config/check-mfa-config.ts b/apps/portal-bff/src/config/check-mfa-config.ts new file mode 100644 index 0000000..97ef897 --- /dev/null +++ b/apps/portal-bff/src/config/check-mfa-config.ts @@ -0,0 +1,41 @@ +/** + * Reads + validates the MFA-related env vars at boot. + * + * MFA_FRESHNESS_SECONDS — default 600 (10 min). Window during + * which a session's `mfaVerifiedAt` is considered "fresh enough" + * for routes decorated with `@RequireMfa()`. The decorator can + * override it per-route; the env value is the global default. + * + * Minimum: 60 s. Anything below would mean the user has to re-MFA + * basically on every navigation; the floor catches a misconfigured + * `MFA_FRESHNESS_SECONDS=30` before the BFF starts. ADR-0011's + * confirmation list explicitly calls out this floor. + * + * No upper bound — operators can pick a long window for low-risk + * surfaces if they want; the trade-off is theirs to record in their + * runbook, not the BFF's to refuse. + */ +const DEFAULT_FRESHNESS_SECONDS = 600; +const MIN_FRESHNESS_SECONDS = 60; + +export interface MfaConfig { + readonly freshnessSeconds: number; +} + +export function readMfaConfig(): MfaConfig { + const raw = process.env['MFA_FRESHNESS_SECONDS']; + if (raw === undefined || raw === '') { + return { freshnessSeconds: DEFAULT_FRESHNESS_SECONDS }; + } + const value = Number(raw); + if (!Number.isInteger(value) || value <= 0) { + throw new Error(`MFA_FRESHNESS_SECONDS must be a positive integer (got "${raw}").`); + } + if (value < MIN_FRESHNESS_SECONDS) { + throw new Error( + `MFA_FRESHNESS_SECONDS must be ≥ ${MIN_FRESHNESS_SECONDS} (got ${value}). ` + + `Anything shorter forces re-MFA on every navigation and is almost certainly a misconfiguration.`, + ); + } + return { freshnessSeconds: value }; +} diff --git a/apps/portal-bff/src/session/session.types.ts b/apps/portal-bff/src/session/session.types.ts index 7faf560..3c222c6 100644 --- a/apps/portal-bff/src/session/session.types.ts +++ b/apps/portal-bff/src/session/session.types.ts @@ -43,6 +43,21 @@ declare module 'express-session' { * SPA's read-only mirror; the source of truth lives here. */ csrfToken?: string; + /** + * Epoch ms of the last MFA assertion captured on this session. + * Set at sign-in by the callback (Entra's CA policy is the + * authority on whether MFA actually occurred — the BFF just + * stamps "now" when it persists a session whose `amr` reflects + * MFA). Will be refreshed by future step-up re-auth flows per + * ADR-0011. + * + * Consumed by `RequireMfaGuard` to decide whether the session + * is "fresh enough" for routes decorated with `@RequireMfa()`. + * Undefined when the session was created before this field + * existed — the guard treats undefined as "no recent MFA + * assertion" and triggers step-up. + */ + mfaVerifiedAt?: number; } } -- 2.30.2