feat(portal-bff): @RequireMfa decorator + freshness guard (ADR-0011)
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).
This commit is contained in:
@@ -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();
|
||||
|
||||
@@ -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<void> {
|
||||
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`.
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 {}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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<string>(MFA_AMR_VALUES);
|
||||
return amr.some((v) => allowed.has(v));
|
||||
}
|
||||
@@ -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),
|
||||
);
|
||||
}
|
||||
@@ -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: <T>() => req as T }),
|
||||
getHandler: () => () => undefined,
|
||||
getClass: () => class {},
|
||||
} as unknown as ExecutionContext;
|
||||
}
|
||||
|
||||
function makeAuditStub(): jest.Mocked<Pick<AuditWriter, 'mfaRequired'>> {
|
||||
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),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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<boolean> {
|
||||
const req = context.switchToHttp().getRequest<Request>();
|
||||
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<RequireMfaOptions>(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',
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -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/);
|
||||
});
|
||||
});
|
||||
@@ -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 };
|
||||
}
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user