From 90bca95fceb9c8081ec823866248fc627e699d61 Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Wed, 29 Apr 2026 22:59:33 +0200 Subject: [PATCH] docs: add ADR-0009 for the authentication flow (OIDC Auth Code + PKCE via MSAL Node) Pin the BFF authentication mechanics: OAuth 2.0 Authorization Code Flow with PKCE, executed server-side via @azure/msal-node's ConfidentialClientApplication. Tokens are held in the BFF session and never reach the browser; the SPA only ever sees the opaque __Host-portal_session cookie. Token validation enforces the tenant allowlist from ADR-0008 (iss check) and maps the audience claim to our Audience enum at the validation step. Refresh-token rotation is enabled via MSAL acquireTokenSilent. Cookies use the __Host- prefix (forces Secure/Path=/, no Domain) with HttpOnly/SameSite=Lax. CSRF uses the double-submit pattern with a matching X-CSRF-Token header on every state-changing request, enforced by a NestJS interceptor and injected client-side by an Angular HTTP interceptor. Logout is RP-initiated against Entra's end_session_endpoint. Routes are pinned: GET /auth/login, GET /auth/callback, POST /auth/logout, GET /auth/me. AuthGuard is registered globally - public routes must be explicitly opted in. Local dev runs over HTTPS via mkcert to keep cookie behaviour identical to prod. All Entra-specific values come from environment variables; the BFF refuses to start without them. decisions/README.md index updated. CLAUDE.md gains an explicit 'Authentication flow' line pointing to ADR-0009. --- CLAUDE.md | 3 +- .../0009-auth-flow-oidc-pkce-msal-node.md | 215 ++++++++++++++++++ decisions/README.md | 1 + 3 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 decisions/0009-auth-flow-oidc-pkce-msal-node.md diff --git a/CLAUDE.md b/CLAUDE.md index d8b6c61..57e866c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,8 @@ 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 / cache:** Redis self-hosted — to be locked-in in phase 2. -- **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). The OIDC Auth Code + PKCE flow via `@azure/msal-node` will be locked-in by a follow-up phase-2 ADR. +- **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. - **Local quality gates:** Husky + lint-staged + commitlint with Conventional Commits — see [ADR-0007](decisions/0007-pre-commit-hooks-and-conventional-commits.md). - **Runtime:** Node.js latest LTS major. diff --git a/decisions/0009-auth-flow-oidc-pkce-msal-node.md b/decisions/0009-auth-flow-oidc-pkce-msal-node.md new file mode 100644 index 0000000..c065d92 --- /dev/null +++ b/decisions/0009-auth-flow-oidc-pkce-msal-node.md @@ -0,0 +1,215 @@ +--- +status: accepted +date: 2026-04-29 +decision-makers: R&D Lead +tags: [security, backend] +--- + +# Authentication flow — OIDC Authorization Code + PKCE via MSAL Node, BFF session pattern + +## Context and Problem Statement + +[ADR-0008](0008-identity-model-entra-workforce-dual-audience.md) fixed the identity model: multi-tenant Entra ID for workforce, dual-audience design, M365 Developer tenant for non-prod. We now need to fix the *technical flow*: which OAuth/OIDC mechanism, which library, how tokens are obtained and held, how the session cookie is shaped, how CSRF is handled, how logout works. + +The SPA must never hold tokens — that is the BFF security pattern, recommended by Microsoft and by the OAuth 2.0 BCP for browser-based apps (`draft-ietf-oauth-browser-based-apps`). The BFF holds the tokens server-side; the browser only carries an opaque session cookie. + +## Decision Drivers + +* Conform to the current IETF best current practice for browser-based apps (BFF pattern). +* Conform to Microsoft's recommended path for multi-tenant Entra workforce apps. +* Tokens never leave the server — the SPA cannot leak them via XSS. +* Support a future On-Behalf-Of flow when the BFF needs to call downstream Entra-protected APIs (covered by a later ADR). +* Defense in depth: hardened cookies, CSRF protection, refresh-token rotation. +* Consistent route shape so frontend, backend, and operators speak the same vocabulary. + +## Considered Options + +### Library +* **`@azure/msal-node`** (`ConfidentialClientApplication`). (Chosen.) +* `openid-client` — generic OIDC client. +* `passport-azure-ad` — deprecated, in maintenance only. + +### OAuth flow +* **Authorization Code Flow with PKCE.** (Chosen.) +* Implicit Flow — deprecated by IETF, rejected. +* Hybrid Flow — legacy, rejected. +* Resource Owner Password Credentials — forbidden by Microsoft for production scenarios. + +### Token storage +* **Tokens held in the BFF session, never sent to the browser.** (Chosen — the BFF pattern itself.) +* Tokens in browser memory or localStorage / sessionStorage — XSS-exfiltrable, rejected. + +### Session cookie shape +* **`__Host-portal_session`** with `HttpOnly`, `Secure`, `SameSite=Lax`. (Chosen.) +* Same attributes without the `__Host-` prefix — weaker (subdomain bleed, downgrade attacks). + +### CSRF +* **Double-submit cookie pattern.** (Chosen.) `__Host-portal_csrf` cookie (readable by the SPA) + matching `X-CSRF-Token` header on every state-changing request. +* Synchronizer token (server-side store) — heavier, requires a dedicated CSRF token store. +* No CSRF protection — unacceptable. + +### Logout +* **RP-initiated logout.** (Chosen.) BFF invalidates the session, then redirects the browser to Entra's `end_session_endpoint`, which clears the Entra SSO session and redirects back. +* Local-only logout — the Entra SSO session stays live, the user can be silently re-authenticated without intent. + +## Decision Outcome + +**Library.** `@azure/msal-node`, instance of `ConfidentialClientApplication`, configured with the multi-tenant authority and the tenant allowlist from ADR-0008. PKCE is used despite the confidential-client setup, per IETF current BCP. + +**Flow.** OAuth 2.0 Authorization Code Flow with PKCE, executed entirely on the BFF. The SPA never sees `code`, `code_verifier`, or any token. + +**Tokens.** ID/access/refresh tokens are stored in the server-side session (storage backend covered by the next ADR). The browser only holds an opaque session identifier in `__Host-portal_session`. + +**Token validation.** On callback, MSAL Node validates the `id_token` (signature against JWKS, `iss`, `aud`, `exp`, `nbf`). The BFF additionally enforces: +- `iss` belongs to the tenant allowlist (workforce tenants accepted by ADR-0008); +- `aud` matches our app's `client_id`; +- the audience claim is mapped to our `Audience` enum (workforce tokens → `audience: 'workforce'`; any other classification fails). + +**Refresh.** Refresh-token rotation is enabled. MSAL Node's `acquireTokenSilent` is used to refresh access tokens transparently. When the refresh token is itself expired or revoked, the BFF returns 401 to the SPA, which redirects to `/auth/login`. + +**Cookies.** + +| Cookie | Purpose | Attributes | +| --- | --- | --- | +| `__Host-portal_session` | opaque session id | `HttpOnly`, `Secure`, `SameSite=Lax`, `Path=/` (forced by the `__Host-` prefix), no `Domain` | +| `__Host-portal_csrf` | CSRF token (readable by JS) | `Secure`, `SameSite=Lax`, `Path=/`, no `HttpOnly` (the SPA must read it) | + +`SameSite=Lax` (not `Strict`) is required because the Entra → BFF callback is a cross-site top-level redirect; `Lax` allows the cookie on top-level navigation, which is exactly what we need. The threat model trade-off is acceptable. + +In **local development** (HTTP `localhost`), browsers reject the `__Host-` prefix because it requires `Secure`. The dev environment runs over **HTTPS via mkcert**-issued local certificates so cookie names and attributes match production exactly. There is no fallback to non-prefixed cookies in dev — keeping a single behaviour avoids "works in dev, breaks in prod" surprises. + +**CSRF.** Every non-`GET` request to the BFF must carry an `X-CSRF-Token` header whose value equals the value of the `__Host-portal_csrf` cookie. A NestJS interceptor enforces this on every state-changing endpoint and rejects with 403 otherwise. The Angular SPA ships an `HttpInterceptor` that reads the cookie and injects the header on outgoing requests. + +**Routes.** All authentication endpoints live under the `/auth` prefix on the BFF: + +| Method | Path | Purpose | +| --- | --- | --- | +| `GET` | `/auth/login` | starts the flow; accepts an optional `returnTo` query parameter validated against an in-app path allowlist; redirects the browser to Entra's `authorize` endpoint with `state`, `nonce`, and PKCE parameters | +| `GET` | `/auth/callback` | receives `code` + `state` from Entra; verifies state, exchanges via MSAL Node, validates id_token, creates the session, redirects to `returnTo` (or `/`) | +| `POST` | `/auth/logout` | requires `X-CSRF-Token`; invalidates the BFF session; returns 200 with the Entra `end_session_endpoint` URL in JSON; the SPA navigates the browser to that URL | +| `GET` | `/auth/me` | returns the current user's id, audience, and a curated subset of claims for the SPA to display (no tokens) | + +**Authorization layer.** A NestJS `AuthGuard` checks for a valid session and rejects with 401 otherwise. A `@CurrentUser()` decorator extracts `{ id, audience, claims }` from the request scope. Every controller (other than `/auth/*` itself, `/health`, etc.) is protected by `AuthGuard` by default — the framework default is "denied", explicit allow-listing is required. + +**Configuration.** All Entra-specific values come from environment variables — no tenant ID, client ID, or secret in source. The BFF refuses to start if any required variable is missing. + +| Variable | Purpose | +| --- | --- | +| `ENTRA_TENANT_ID` | home tenant id of the BFF (the tenant where the app is registered as the home org) | +| `ENTRA_CLIENT_ID` | app's client id | +| `ENTRA_CLIENT_SECRET` *or* `ENTRA_CLIENT_CERT_PATH` | confidential-client credential (certificate preferred for production) | +| `ENTRA_ACCEPTED_TENANT_IDS` | comma-separated allowlist for `iss` validation | +| `ENTRA_REDIRECT_URI` | absolute URL of the `/auth/callback` endpoint, registered in the app registration | +| `ENTRA_POST_LOGOUT_REDIRECT_URI` | absolute URL the user lands on after Entra logout | +| `SESSION_SECRET` | symmetric secret for cookie signing (rotated procedure covered in a future ops ADR) | + +### Consequences + +* Good, because the SPA can never leak tokens — the worst an XSS can do is hijack the session cookie, and `HttpOnly` blocks even that. +* Good, because PKCE protects the code-exchange step against authorization-code interception even on the BFF side. +* Good, because the route shape and configuration surface are simple, predictable, and entirely documented. +* Good, because MSAL Node will let us add the On-Behalf-Of flow in the downstream-API ADR with minimal extra work. +* Good, because RP-initiated logout produces a clean cross-app state; users are not silently re-authenticated against their will. +* Good, because the dual-audience claim mapping is enforced at the validation step — workforce vs customer is a server-side decision, not a client-side hint. +* Bad, because the codebase becomes tightly coupled to MSAL Node and Entra. Switching IdP later means more than swapping a library — but the trade-off is accepted given identity is already locked to Entra. +* Bad, because `SameSite=Lax` (instead of `Strict`) is necessary for the callback to work; this is a known and accepted trade-off, mitigated by HTTPS, HSTS, the `__Host-` prefix, and CSRF. +* Bad, because every state-changing call from the SPA must carry `X-CSRF-Token` — small DX overhead, mitigated by a single Angular HTTP interceptor. +* Bad, because mkcert is required in dev to keep cookie behaviour identical to prod — slightly higher onboarding cost; documented in the dev setup guide. +* Bad, because the tenant allowlist (`ENTRA_ACCEPTED_TENANT_IDS`) must be maintained operationally as new B2B partners are onboarded — this is an operational item, not a one-shot. + +### Confirmation + +* `apps/portal-bff/src/auth/auth.module.ts` provides a single `ConfidentialClientApplication` configured from env, with PKCE enabled. +* `apps/portal-bff/src/auth/auth.controller.ts` exposes the four routes above and no others. +* `AuthGuard` is registered globally; routes that must be public are explicitly marked with a `@Public()` decorator. +* `__Host-portal_session` and `__Host-portal_csrf` are the only cookies set by the BFF (other than infrastructure cookies, none of which carry session data). +* A NestJS interceptor enforces double-submit CSRF on every non-`GET` request. +* The Angular HTTP interceptor in `portal-shell` injects `X-CSRF-Token` on every outgoing request to the BFF. +* `iss` validation rejects any token whose issuer is not in the tenant allowlist; tested with a token from a non-allowlisted tenant. +* Refresh-token rotation is asserted via integration tests (one expired access token + a still-valid refresh token must succeed silently; an expired refresh token must produce 401 and trigger re-authentication). +* RP-initiated logout is asserted: `POST /auth/logout` invalidates the session, returns the `end_session_endpoint` URL, and a subsequent request without re-authentication is denied. + +## Pros and Cons of the Options + +### Library + +#### `@azure/msal-node` (chosen) + +* Good, because Microsoft-official, actively maintained, aligned with Entra's quirks and roadmap (B2B, External ID later, OBO). +* Good, because handles refresh-token rotation, multi-tenant authority routing, and JWKS caching out of the box. +* Bad, because Microsoft-specific — couples the BFF to Entra at the protocol-handler layer. + +#### `openid-client` + +* Good, because a generic, RFC-faithful OIDC client; would work with any IdP. +* Bad, because lacks Microsoft-specific helpers, especially the OBO flow we'll need in the downstream-API ADR. +* Bad, because we'd be writing the Microsoft-specific glue ourselves — bricolage on a security-critical surface. + +#### `passport-azure-ad` + +* Bad, because deprecated by Microsoft; no new features, only critical fixes. Rejected. + +### OAuth flow + +#### Authorization Code with PKCE (chosen) + +* Good, because IETF BCP for browser-based apps, recommended by Microsoft for confidential clients on the BFF. +* Good, because PKCE adds a binding between the auth-request and the code-exchange that defeats code-interception attacks. + +#### Implicit Flow + +* Bad, because deprecated; tokens were returned in the URL fragment, exposed to browser history and the SPA. Rejected. + +#### Hybrid Flow + +* Bad, because legacy; combines Code and Implicit. Rejected. + +#### Resource Owner Password Credentials + +* Bad, because the BFF would handle the user's credentials directly — forbidden by Microsoft for production. Rejected. + +### Cookie shape + +#### `__Host-` prefix (chosen) + +* Good, because browsers enforce `Secure`, `Path=/`, and absence of `Domain` — no subdomain bleed, no downgrade. +* Bad, because requires HTTPS in dev (mkcert). + +#### Without the prefix + +* Bad, because subdomain cookies and HTTP-downgrade scenarios become attack surface. + +### CSRF + +#### Double-submit cookie (chosen) + +* Good, because stateless server-side — no token store to maintain. +* Good, because the cookie is `SameSite=Lax`, so it isn't sent on cross-site requests; combined with the header check, an attacker on another origin cannot synthesise a valid request. + +#### Synchronizer token + +* Good, because slightly stronger isolation (the token is server-only). +* Bad, because requires a server-side store and synchronisation — heavier for marginal gain in our threat model. + +#### No protection + +* Bad, because cookies are sent on cross-site form submissions; state-changing endpoints would be trivially exploitable. Rejected. + +### Logout + +#### RP-initiated (chosen) + +* Good, because the user's Entra SSO state is cleared too — explicit, no silent re-auth. + +#### Local-only + +* Bad, because the user remains signed in to Entra; "log out" feels reversible to the attacker who got their hands on the session for a moment, not to the user. + +## More Information + +* IETF "OAuth 2.0 for Browser-Based Apps" BCP draft: https://datatracker.ietf.org/doc/draft-ietf-oauth-browser-based-apps/ +* Microsoft BFF guidance: https://learn.microsoft.com/azure/architecture/patterns/backends-for-frontends +* MSAL Node overview: https://learn.microsoft.com/azure/active-directory/develop/msal-node-overview +* Cookie `__Host-` prefix: https://developer.mozilla.org/docs/Web/HTTP/Cookies#cookie_prefixes +* mkcert: https://github.com/FiloSottile/mkcert +* Related ADRs: [ADR-0005](0005-backend-stack-nestjs.md) (NestJS), [ADR-0008](0008-identity-model-entra-workforce-dual-audience.md) (identity model), and the future ADRs for session storage (Redis), MFA enforcement, downstream On-Behalf-Of flow, and audit trail. diff --git a/decisions/README.md b/decisions/README.md index 2025f27..283d521 100644 --- a/decisions/README.md +++ b/decisions/README.md @@ -52,3 +52,4 @@ ADRs are listed in numerical order. To slice by topic, filter on the `Tags` colu | [0006](0006-persistence-postgresql-prisma.md) | Persistence — PostgreSQL with Prisma | accepted | `data`, `backend` | 2026-04-29 | | [0007](0007-pre-commit-hooks-and-conventional-commits.md) | Pre-commit hooks and Conventional Commits | accepted | `process` | 2026-04-29 | | [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 |