feat(portal-bff): @RequireMfa decorator + freshness guard (ADR-0011) #128
@@ -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()', () => {
|
describe('adminAccessDenied()', () => {
|
||||||
it('records admin.access_denied with outcome=denied, attempted route as subject, and rolesHeld payload', async () => {
|
it('records admin.access_denied with outcome=denied, attempted route as subject, and rolesHeld payload', async () => {
|
||||||
const { writer, prisma, hashUserId } = await createSubject();
|
const { writer, prisma, hashUserId } = await createSubject();
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import { PrismaService } from 'nestjs-prisma';
|
|||||||
import type {
|
import type {
|
||||||
AdminAccessDeniedInput,
|
AdminAccessDeniedInput,
|
||||||
AuditEventInput,
|
AuditEventInput,
|
||||||
|
MfaRequiredInput,
|
||||||
SignInActor,
|
SignInActor,
|
||||||
SignInFailedInput,
|
SignInFailedInput,
|
||||||
SignOutInput,
|
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`
|
* Typed event: `/api/admin/*` request rejected by `AdminRoleGuard`
|
||||||
* because the session's `roles` claim does not include `admin`.
|
* because the session's `roles` claim does not include `admin`.
|
||||||
|
|||||||
@@ -100,3 +100,21 @@ export interface AdminAccessDeniedInput {
|
|||||||
*/
|
*/
|
||||||
rolesHeld: readonly string[];
|
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;
|
createdAt?: number;
|
||||||
absoluteExpiresAt?: number;
|
absoluteExpiresAt?: number;
|
||||||
csrfToken?: string;
|
csrfToken?: string;
|
||||||
|
mfaVerifiedAt?: number;
|
||||||
save: jest.Mock;
|
save: jest.Mock;
|
||||||
destroy: jest.Mock;
|
destroy: jest.Mock;
|
||||||
}
|
}
|
||||||
@@ -230,6 +231,27 @@ describe('AuthController.callback', () => {
|
|||||||
expect(res.redirect).toHaveBeenCalledWith(302, ENTRA.postLogoutRedirectUri);
|
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 () => {
|
it('records createdAt + absoluteExpiresAt on the session (ADR-0010 §"TTL policy")', async () => {
|
||||||
// Force the ADR-0010 default for this test — apps/portal-bff/.env
|
// Force the ADR-0010 default for this test — apps/portal-bff/.env
|
||||||
// may have a custom SESSION_ABSOLUTE_TIMEOUT_SECONDS for local
|
// 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
|
// the SPA's read-only mirror used to echo the value in the
|
||||||
// X-CSRF-Token header.
|
// X-CSRF-Token header.
|
||||||
req.session.csrfToken = csrfToken;
|
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
|
// Force the save before the redirect: express-session writes
|
||||||
// on response end, but the 302 we're about to emit closes the
|
// on response end, but the 302 we're about to emit closes the
|
||||||
// response before the async store-write would otherwise
|
// response before the async store-write would otherwise
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import { AuthController } from './auth.controller';
|
|||||||
import { AuthService } from './auth.service';
|
import { AuthService } from './auth.service';
|
||||||
import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token';
|
import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token';
|
||||||
import { MSAL_CLIENT } from './msal-client.token';
|
import { MSAL_CLIENT } from './msal-client.token';
|
||||||
|
import { RequireMfaGuard } from './require-mfa.guard';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Auth module — owns the Entra ID configuration, the MSAL Node
|
* Auth module — owns the Entra ID configuration, the MSAL Node
|
||||||
@@ -43,6 +44,7 @@ import { MSAL_CLIENT } from './msal-client.token';
|
|||||||
controllers: [AuthController],
|
controllers: [AuthController],
|
||||||
providers: [
|
providers: [
|
||||||
AuthService,
|
AuthService,
|
||||||
|
RequireMfaGuard,
|
||||||
{
|
{
|
||||||
provide: ENTRA_CONFIG,
|
provide: ENTRA_CONFIG,
|
||||||
useFactory: () => assertEntraConfig(),
|
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 {}
|
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.
|
* SPA's read-only mirror; the source of truth lives here.
|
||||||
*/
|
*/
|
||||||
csrfToken?: string;
|
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