d51ccebe6a
## Summary
Third step in the `portal-admin` audit-log-viewer workstream — ships the `@RequireMfa({ freshness })` decorator + guard called out in [ADR-0011](docs/decisions/0011-mfa-enforcement-entra-conditional-access.md) and referenced as the gate on the admin entry route in [ADR-0020](docs/decisions/0020-portal-admin-app.md). Designed-in, dormant: no v1 route uses the decorator yet. First consumer will be the admin entry route once the distinct admin session lands (next PR).
## What ships
- **[`auth/mfa.ts`](apps/portal-bff/src/auth/mfa.ts)** — `MFA_AMR_VALUES = ['mfa', 'otp', 'fido', 'wia', 'phr']` allow-list and `wasMultiFactor(amr): boolean`. The list mirrors ADR-0011 §"BFF verification"; the spec pins it so an ad-hoc edit can't bypass review.
- **[`config/check-mfa-config.ts`](apps/portal-bff/src/config/check-mfa-config.ts)** — `readMfaConfig()` reads `MFA_FRESHNESS_SECONDS` (default **600 s**, minimum **60 s**). Anything below the floor throws at boot — the floor catches a misconfigured "MFA on every navigation" before the BFF starts.
- **[`auth/require-mfa.guard.ts`](apps/portal-bff/src/auth/require-mfa.guard.ts)** — four branches:
| Branch | HTTP | Code | Audit |
| --- | --- | --- | --- |
| No session | 401 | `unauthenticated` | none (noise) |
| Session, no MFA-class `amr` | 401 | `mfa_required` | `auth.mfa_required reason=no-mfa-in-amr` |
| Session, no `mfaVerifiedAt` | 401 | `mfa_required` | `auth.mfa_required reason=no-mfa-verified-at` |
| Session, stale `mfaVerifiedAt` | 401 | `mfa_required` | `auth.mfa_required reason=mfa-stale, mfaAgeMs=…` |
The `reason` discriminator is **not** surfaced over the wire — only the audit row carries it. An attacker probing for "stale vs no-MFA" can't distinguish the two from the response.
- **[`auth/require-mfa.decorator.ts`](apps/portal-bff/src/auth/require-mfa.decorator.ts)** — `@RequireMfa({ freshness? })` built via `applyDecorators(SetMetadata, UseGuards)`. The per-route `freshness` override wins over the env default. Designed to compose with `@RequireAdmin()` — apply `@RequireMfa` outside `@RequireAdmin` so the freshness gate runs only after role is established.
- **[`AuditWriter.mfaRequired()`](apps/portal-bff/src/audit/audit.service.ts)** — new typed method using `outcome=denied`, captures `reason`, `freshnessSeconds`, and `mfaAgeMs` (when applicable) in the JSONB payload.
- **`session.mfaVerifiedAt: number`** — augmented onto `express-session`'s `SessionData` in [`session.types.ts`](apps/portal-bff/src/session/session.types.ts). Set to `Date.now()` at sign-in by the callback ([`auth.controller.ts`](apps/portal-bff/src/auth/auth.controller.ts)). Entra's CA policy is the authority on whether MFA actually happened; the BFF stamps "now" when persisting a session whose `amr` reflects MFA.
## Deferred — for the SPA-interceptor PR
ADR-0011 §"Step-up MFA — designed-in" step 2 calls for a `WWW-Authenticate` header carrying a **claims challenge** (MSAL-produced blob) on the 401. That requires:
1. MSAL Node integration to mint the challenge — adds wire-format coupling to MSAL we don't have anywhere else yet.
2. The Angular SPA interceptor to consume the header, redirect to `/auth/login?claims=…`, and retry the original request.
Neither side has a consumer in this PR. Shipping a `code: 'mfa_required'` in the structured envelope is sufficient signalling for the SPA interceptor once it lands — the interceptor PR can layer the `WWW-Authenticate` header and the MSAL claims blob without changing the guard's audit contract.
## Composability with `@RequireAdmin`
The admin entry route (next-PR consumer) will read:
```ts
@Controller('admin')
@RequireMfa({ freshness: 600 })
@RequireAdmin()
export class AdminController { … }
```
Apply order matters — Nest runs guards in the order their decorators were applied (innermost first). Putting `@RequireMfa()` outside `@RequireAdmin()` means a non-admin user gets a clean 403 from `AdminRoleGuard` without a spurious `auth.mfa_required` audit row. The decorator's JSDoc spells this out for future consumers.
## Notes for the reviewer
- The `RequireMfaGuard` is registered as a provider in `AuthModule` and re-exported. Per the existing convention ("`AuthModule` stays non-global; modules state 'I depend on auth' by importing it"), any future module using `@RequireMfa()` will need to `imports: [AuthModule]`. The `AdminModule` already does this transitively via shared `AuditWriter`; the explicit import will follow when the decorator is first applied.
- `mfaChallenge(reason)` takes the reason argument deliberately even though it ignores it in the response — keeps the call sites readable (`throw this.mfaChallenge('mfa-stale')`) and parks a hook for the day we want to localise / differentiate the message.
- New env var `MFA_FRESHNESS_SECONDS` is **optional** (default 600). No production env change is required to ship this PR.
## Test plan
- [x] `pnpm nx test portal-bff` — **251 specs pass** (was 214; +37 covering helpers, config reader, guard branches, audit typed method, callback stamp).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean (the pre-existing `_res` / `_next` warnings in `rate-limit.middleware.ts` are unrelated).
- [x] `MFA_FRESHNESS_SECONDS` boot validator: default + valid + below-floor + non-integer + decimal + non-numeric all covered.
- [x] Guard timing-boundary cases covered (age == freshness passes; age == freshness + 1 ms fails — implicitly via the 700-s-vs-600-s test).
- [ ] e2e — pending real Entra session with `amr` carrying an MFA token. Will be exercised when the admin entry route applies the decorator.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #128
224 lines
8.2 KiB
TypeScript
224 lines
8.2 KiB
TypeScript
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),
|
|
);
|
|
});
|
|
});
|