feat(portal-bff): @RequireMfa decorator + freshness guard (ADR-0011) #128
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: julien/apf_portal#128
Reference in New Issue
Block a user
Delete Branch "feat/portal-bff-require-mfa-guard"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Third step in the
portal-adminaudit-log-viewer workstream — ships the@RequireMfa({ freshness })decorator + guard called out in ADR-0011 and referenced as the gate on the admin entry route in ADR-0020. 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—MFA_AMR_VALUES = ['mfa', 'otp', 'fido', 'wia', 'phr']allow-list andwasMultiFactor(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—readMfaConfig()readsMFA_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— four branches:unauthenticatedamrmfa_requiredauth.mfa_required reason=no-mfa-in-amrmfaVerifiedAtmfa_requiredauth.mfa_required reason=no-mfa-verified-atmfaVerifiedAtmfa_requiredauth.mfa_required reason=mfa-stale, mfaAgeMs=…The
reasondiscriminator 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—@RequireMfa({ freshness? })built viaapplyDecorators(SetMetadata, UseGuards). The per-routefreshnessoverride wins over the env default. Designed to compose with@RequireAdmin()— apply@RequireMfaoutside@RequireAdminso the freshness gate runs only after role is established.AuditWriter.mfaRequired()— new typed method usingoutcome=denied, capturesreason,freshnessSeconds, andmfaAgeMs(when applicable) in the JSONB payload.session.mfaVerifiedAt: number— augmented ontoexpress-session'sSessionDatainsession.types.ts. Set toDate.now()at sign-in by the callback (auth.controller.ts). Entra's CA policy is the authority on whether MFA actually happened; the BFF stamps "now" when persisting a session whoseamrreflects MFA.Deferred — for the SPA-interceptor PR
ADR-0011 §"Step-up MFA — designed-in" step 2 calls for a
WWW-Authenticateheader carrying a claims challenge (MSAL-produced blob) on the 401. That requires:/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 theWWW-Authenticateheader and the MSAL claims blob without changing the guard's audit contract.Composability with
@RequireAdminThe admin entry route (next-PR consumer) will read:
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 fromAdminRoleGuardwithout a spuriousauth.mfa_requiredaudit row. The decorator's JSDoc spells this out for future consumers.Notes for the reviewer
RequireMfaGuardis registered as a provider inAuthModuleand re-exported. Per the existing convention ("AuthModulestays non-global; modules state 'I depend on auth' by importing it"), any future module using@RequireMfa()will need toimports: [AuthModule]. TheAdminModulealready does this transitively via sharedAuditWriter; 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.MFA_FRESHNESS_SECONDSis optional (default 600). No production env change is required to ship this PR.Test plan
pnpm nx test portal-bff— 251 specs pass (was 214; +37 covering helpers, config reader, guard branches, audit typed method, callback stamp).pnpm exec nx affected -t format:check lint test build --base=origin/main— clean (the pre-existing_res/_nextwarnings inrate-limit.middleware.tsare unrelated).MFA_FRESHNESS_SECONDSboot validator: default + valid + below-floor + non-integer + decimal + non-numeric all covered.amrcarrying an MFA token. Will be exercised when the admin entry route applies the decorator.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).