diff --git a/CLAUDE.md b/CLAUDE.md index 06d9b70..7512019 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,6 +37,7 @@ The structural choices are recorded as ADRs and summarized below. Any change to - **Backend (`portal-bff`):** NestJS at the latest stable major, mounted on the Express adapter (Fastify adapter swappable later) — see [ADR-0005](decisions/0005-backend-stack-nestjs.md). - **Persistence:** PostgreSQL (latest stable major) via Prisma — see [ADR-0006](decisions/0006-persistence-postgresql-prisma.md). - **Sessions:** opaque session id in `__Host-portal_session`, payload in self-hosted Redis (Sentinel HA in prod, single node in dev), tokens encrypted at rest with AES-256-GCM, idle 30 min sliding + absolute 12 h — see [ADR-0010](decisions/0010-session-management-redis.md). +- **MFA:** enforced by Entra ID Conditional Access (org-side policy, P1 licensing required); BFF sanity-checks the `amr` claim at session creation; `@RequireMfa()` decorator and freshness-based step-up are designed-in for future sensitive routes (no v1 consumer) — see [ADR-0011](decisions/0011-mfa-enforcement-entra-conditional-access.md). - **Identity:** multi-tenant Microsoft Entra ID with B2B invitation for workforce in v1, dual-audience design ready for future External ID activation — see [ADR-0008](decisions/0008-identity-model-entra-workforce-dual-audience.md). - **Authentication flow:** OIDC Authorization Code + PKCE via `@azure/msal-node`, executed entirely on the BFF; SPA never holds tokens; `__Host-` prefixed cookies, double-submit CSRF, RP-initiated logout — see [ADR-0009](decisions/0009-auth-flow-oidc-pkce-msal-node.md). - **Observability:** Pino structured logs + OpenTelemetry traces with W3C Trace Context propagation — to be locked-in in phase 2. diff --git a/decisions/0011-mfa-enforcement-entra-conditional-access.md b/decisions/0011-mfa-enforcement-entra-conditional-access.md new file mode 100644 index 0000000..e284197 --- /dev/null +++ b/decisions/0011-mfa-enforcement-entra-conditional-access.md @@ -0,0 +1,153 @@ +--- +status: accepted +date: 2026-04-29 +decision-makers: R&D Lead +tags: [security] +--- + +# MFA enforcement — Entra ID Conditional Access baseline, BFF claim sanity-check, step-up hooks designed-in + +## Context and Problem Statement + +Multi-factor authentication is mandatory for the portal's workforce audience. The question is *where* MFA logic lives, *how* the BFF gains assurance that an authenticated session has actually been multi-factored, and *how* the application is prepared for future step-up MFA on sensitive operations — without writing any of it in v1. + +Identity is Microsoft Entra ID ([ADR-0008](0008-identity-model-entra-workforce-dual-audience.md)) and the auth flow is OIDC Authorization Code + PKCE via MSAL Node ([ADR-0009](0009-auth-flow-oidc-pkce-msal-node.md)). The v1 portal has no admin UI and no defined "sensitive" operations beyond authentication itself, so step-up MFA is a design concern, not a v1 implementation concern. + +## Decision Drivers + +* MFA is a non-negotiable security baseline for workforce. +* Application code must not implement MFA mechanics — that belongs in the IdP. "Anti-bricolage" applies particularly here. +* The mechanism chosen at sign-in must be auditable in the session (was MFA actually performed?). +* The architecture must support future step-up MFA (a sensitive operation requires *fresh* MFA, beyond what was done at sign-in) without retrofitting authorization. +* Configuration responsibility must be clearly separated: the IdP (org IT) owns the policy; the BFF owns the verification. + +## Considered Options + +### Where MFA is enforced +* **Microsoft Entra Conditional Access policy at the tenant level.** (Chosen.) +* MFA logic in the BFF (e.g., a second-factor prompt after password). + +### How the BFF validates that MFA happened +* **Sanity-check the `amr` claim of the id_token at session creation.** (Chosen.) +* Require an explicit Authentication Context Class Reference (ACR) value via `acr_values`/`claims` request parameter. +* Trust Entra unconditionally without verification. + +### Step-up MFA in v1 +* **Designed-in via a `@RequireMfa()` decorator and a claims-challenge mechanism, but not applied to any v1 route.** (Chosen.) +* Built and exercised in v1 — premature, no consumer. +* Deferred entirely (no decorator, no hook) — costly to retrofit. + +## Decision Outcome + +**MFA enforcement** lives in **Microsoft Entra Conditional Access**. The org's IT/identity contact configures a Conditional Access policy at the tenant level that requires MFA for any sign-in to the BFF's app registration, for both the prod tenant and the M365 Developer tenant used for non-prod. Recommended settings (org-side, not in our code): + +- *Assignment.* Cloud apps → the BFF app. Users → All users (or "All workforce", depending on how the org structures groups). Excluded: break-glass accounts, by Microsoft's own recommendation. +- *Conditions* (recommended, not strictly required): device compliance, location, sign-in risk (Entra ID P2 for risk-based — opt-in). +- *Grant.* Require multi-factor authentication. Prefer phishing-resistant methods (FIDO2 / passkeys / Windows Hello for Business). Fall back to authenticator-app push or TOTP. SMS/voice deprecated, to be avoided. + +This policy is **out of scope for application code**. Our app does not implement MFA — it consumes the MFA outcome. + +**BFF verification.** At session creation (after the OIDC callback exchange and id_token validation), the BFF performs a sanity-check on the `amr` claim of the id_token. The session is rejected with a 401 and an explicit error if the claim is absent, empty, or contains only password-class values without an MFA-class indicator. The expected normal value emitted by Entra after a CA-enforced MFA sign-in is `["pwd","mfa"]` (or equivalent depending on the factor). + +This is *defense in depth*, not the primary control: the primary control is the CA policy. The sanity-check exists so that a misconfigured or disabled CA policy does not silently regress the security posture without anyone noticing — the BFF will simply refuse to grant sessions if MFA assurance is missing. + +A small validation table maintained in the BFF (in source, reviewed in PRs) lists the `amr` values accepted as multi-factor: + +```ts +// Values Entra emits in 'amr' that we treat as evidence of MFA having occurred. +// Not exhaustive — extended on review when Entra emits new values. +const MFA_AMR_VALUES = ['mfa', 'otp', 'fido', 'wia', 'phr'] as const; +``` + +This list is reviewed against Microsoft's documentation as part of the security review cadence. + +**Step-up MFA — designed-in, dormant.** A `@RequireMfa()` controller-method decorator and an associated guard are implemented in v1, but no v1 route uses them. The guard, when active, will: + +1. Inspect the session for a recent MFA assertion (a `mfaVerifiedAt` timestamp added to the session payload, set to `lastSeenAt` of the auth callback). +2. If the timestamp is older than a configurable freshness window (`MFA_FRESHNESS_SECONDS`, default 600 = 10 min), respond with 401 + a `WWW-Authenticate` header carrying a *claims challenge* (an opaque blob produced by MSAL Node). +3. The Angular SPA HTTP interceptor catches the 401 + claims challenge, redirects the browser to `/auth/login?claims=`, which forwards the challenge to Entra. Entra re-prompts MFA, returns a fresher `amr`/`mfaVerifiedAt`, and the original request is retried. + +Authentication Context Classes (`acr_values`) are *not* used in v1. The freshness-timestamp approach is sufficient for the baseline. ACR-based step-up (e.g., "this action requires CA policy `c2`") will be considered later, gated behind a follow-up ADR if specific operations demand it. + +**Service accounts / app-only tokens.** Out of scope. The BFF authenticates end-users via OIDC; service-to-service tokens (if needed for downstream APIs in a future ADR) follow a different flow — On-Behalf-Of or app-only — and have their own MFA story (i.e., none, because no human is involved). + +### Consequences + +* Good, because MFA mechanics live where they belong (the IdP). The application is not in the business of implementing OTP delivery, push notifications, or hardware-key challenges. +* Good, because the IdP configuration can evolve independently — adding device compliance, sign-in risk, location-based rules, or moving to phishing-resistant-only — without code changes. +* Good, because the `amr` sanity-check in the BFF catches a category of misconfiguration that would otherwise silently regress security. +* Good, because step-up MFA is designed-in via a single decorator. When a sensitive operation appears in a future feature, marking it requires only `@RequireMfa()` — no architectural change. +* Good, because separating freshness-based step-up (default) from ACR-based step-up (advanced) keeps v1 simple while leaving a clean upgrade path. +* Bad, because the `amr` value list is implicitly tied to Entra's emitted values; if Microsoft introduces a new factor with a new `amr` token, the BFF rejects sessions until the list is updated. Mitigated by a documented review cadence. +* Bad, because step-up MFA needs SPA cooperation (the HTTP interceptor that handles 401 + claims challenge); if a future SPA feature bypasses the interceptor, step-up will silently degrade to plain 401. Mitigated by integration tests on the interceptor. +* Bad, because Conditional Access policies — especially advanced ones — require Entra ID P1 (and P2 for risk-based). Already flagged in [ADR-0008](0008-identity-model-entra-workforce-dual-audience.md), restated here. +* Neutral, because v1 does not exercise the step-up path. Hooks must be kept alive by automated tests, not by being used in production traffic. + +### Confirmation + +* `apps/portal-bff/src/auth/mfa.ts` exports the `MFA_AMR_VALUES` constant and a `wasMultiFactor(claims): boolean` helper. +* The OIDC callback path rejects sessions for which `wasMultiFactor(idTokenClaims)` is false, with a structured 401 (logged as an audit event in the future audit-log ADR). +* Each session payload carries `mfaVerifiedAt: number` (epoch ms), set at the callback and refreshed on any subsequent step-up auth completion. +* `apps/portal-bff/src/auth/require-mfa.guard.ts` provides the `RequireMfaGuard` and the `@RequireMfa()` decorator (on top of the global `AuthGuard` from ADR-0009). +* `MFA_FRESHNESS_SECONDS` is read from env, defaulting to 600. The BFF refuses to start with a value below 60 (to catch misconfiguration). +* The Angular HTTP interceptor in `portal-shell` handles `401` + `WWW-Authenticate: Bearer error="insufficient_user_authentication"` (or the claims-challenge equivalent) and redirects to `/auth/login` with the challenge propagated. +* Integration tests cover: a sign-in without MFA assertion is rejected; a sign-in with valid MFA assertion succeeds; a route protected by `@RequireMfa()` accepts a fresh session, rejects with claims challenge after `MFA_FRESHNESS_SECONDS`, and accepts again after re-auth. +* The org-side Conditional Access policy is documented in an operational runbook (out of repo, owned by IT) and reviewed during onboarding of each tenant (prod + dev). + +## Pros and Cons of the Options + +### Where MFA is enforced + +#### Conditional Access (chosen) + +* Good, because it is the Microsoft-native, audited, IdP-managed mechanism — exactly the kind of "recognised, battle-tested" choice the project values. +* Good, because changes to MFA policy are a tenant-level config, not a deployment. +* Good, because the IdP catalogue of factors evolves without us — passkeys, FIDO2, certificate-based, etc. +* Bad, because it requires Entra ID P1 minimum at the tenant level (cost flagged in ADR-0008). + +#### MFA logic in the BFF + +* Bad, because it would re-implement OTP/push/key flows the IdP already provides — exactly the bricolage we are forbidden. +* Bad, because secret material (TOTP shared secrets, etc.) ends up in our Postgres, expanding the attack surface. + +### How the BFF validates MFA + +#### `amr` sanity-check (chosen) + +* Good, because cheap, immediate, no extra round trip. +* Good, because catches a real failure mode (CA policy disabled, scope misconfigured). +* Bad, because dependent on Entra's `amr` emission values — list maintenance required. + +#### Required ACR via `claims` parameter + +* Good, because explicit and machine-verifiable per request. +* Bad, because requires Authentication Context Classes to be configured tenant-side and exchanged on every request — heavy for a baseline "MFA at sign-in" need. Re-evaluate when fine-grained step-up arrives. + +#### No verification + +* Bad, because a CA policy disabled by mistake would silently regress security with no audit signal until an incident. + +### Step-up MFA in v1 + +#### Designed-in via decorator, dormant (chosen) + +* Good, because the cost is minimal: one guard, one decorator, one timestamp in the session. +* Good, because step-up is a one-line annotation on any future sensitive route. +* Bad, because a dormant code path drifts unless covered by tests — addressed in the Confirmation section. + +#### Built and exercised in v1 + +* Bad, because no v1 route is sensitive enough to require it. Building a feature with no consumer is bricolage by another name. + +#### Deferred entirely + +* Bad, because retrofitting step-up after the fact means refactoring guards, sessions, and the SPA interceptor. Cost asymmetric vs. the dormant-hook option. + +## More Information + +* Microsoft Entra Conditional Access overview: https://learn.microsoft.com/entra/identity/conditional-access/overview +* `amr` claim values (OpenID Connect): https://www.iana.org/assignments/authentication-method-reference-values/ +* Microsoft `amr` documentation: https://learn.microsoft.com/azure/active-directory/develop/access-tokens +* Authentication Context Classes (ACR) and claims challenges: https://learn.microsoft.com/entra/identity-platform/v2-conditional-access-dev-guide +* Phishing-resistant MFA (passkeys / FIDO2): https://learn.microsoft.com/entra/identity/authentication/concept-authentication-passwordless +* Related ADRs: [ADR-0008](0008-identity-model-entra-workforce-dual-audience.md) (identity model — P1 licensing flagged), [ADR-0009](0009-auth-flow-oidc-pkce-msal-node.md) (auth flow), [ADR-0010](0010-session-management-redis.md) (session — `mfaVerifiedAt` lives here), and the future ADRs for downstream APIs (On-Behalf-Of), audit trail (rejected-MFA logged events), and the operational runbook for tenant Conditional Access policies. diff --git a/decisions/README.md b/decisions/README.md index eea363f..3f01a17 100644 --- a/decisions/README.md +++ b/decisions/README.md @@ -54,3 +54,4 @@ ADRs are listed in numerical order. To slice by topic, filter on the `Tags` colu | [0008](0008-identity-model-entra-workforce-dual-audience.md) | Identity model — multi-tenant Entra ID for workforce, dual-audience design for future External ID | accepted | `security`, `data` | 2026-04-29 | | [0009](0009-auth-flow-oidc-pkce-msal-node.md) | Authentication flow — OIDC Authorization Code + PKCE via MSAL Node, BFF session pattern | accepted | `security`, `backend` | 2026-04-29 | | [0010](0010-session-management-redis.md) | Session management — opaque session IDs in cookies, payload in self-hosted Redis with AES-GCM at rest | accepted | `security`, `backend`, `infrastructure` | 2026-04-29 | +| [0011](0011-mfa-enforcement-entra-conditional-access.md) | MFA enforcement — Entra ID Conditional Access baseline, BFF claim sanity-check, step-up hooks designed-in | accepted | `security` | 2026-04-29 |