From 5bbe2304ff2e55562635d67bf9e786c852447ce4 Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Wed, 13 May 2026 20:50:44 +0200 Subject: [PATCH] feat(portal-bff): helmet + env-driven CORS allowlist + double-submit CSRF (#122) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Phase-2 security baseline that the `main.ts` placeholder note has been advertising since the auth/session work began. Three independent middlewares + their SPA counterparts, all mounted in a single PR because they only become meaningful together. ### Helmet on the BFF `helmet()` with three overrides matching our specific shape: - **HSTS only in production** — dev runs on plain HTTP, HSTS is just noise. - **`crossOriginResourcePolicy: 'cross-origin'`** — the SPA on its own origin reads JSON from the BFF; the default `same-origin` would block it. - **CSP disabled in non-production** — the BFF doesn't render HTML, so CSP on JSON responses is mostly inert, but Helmet's default CSP triggers noisy `connect-src` violations in browser devtools that we don't need. Everything else is Helmet defaults: `X-Frame-Options=SAMEORIGIN`, `X-Content-Type-Options=nosniff`, `Referrer-Policy=no-referrer`, `X-Powered-By` removed, etc. ### CORS allowlist, env-driven `CORS_ALLOWED_ORIGINS` env (comma-separated) is now **mandatory** at boot. The BFF refuses to start without it via `readCorsAllowlist()` — same boot-time validator family as `assertSessionSecret` etc. The previous hardcoded `http://localhost:4200` fallback is gone; getting CORS wrong silently is the kind of "works in dev, breaks in prod" trap the validator is specifically designed to catch. `X-CSRF-Token` is now in the allowed headers. ### Double-submit CSRF - BFF mints a 256-bit `csrfToken` at session creation (`/auth/callback`), stored on `req.session.csrfToken` and mirrored to a JS-readable cookie (`__Host-portal_csrf` prod / `portal_csrf` dev). The cookie is the SPA's read-only view; the server-side session is the source of truth. - `createCsrfMiddleware` (mounted after the session middleware in `main.ts`) compares the `X-CSRF-Token` header with `req.session.csrfToken` using `crypto.timingSafeEqual`. Skips: - safe methods (`GET / HEAD / OPTIONS`), - anonymous requests (no `req.session.user`), - `/api/auth/login` and `/api/auth/callback` (those mint the token themselves). - Mismatch → `403 {"error":"csrf"}` with a structured Pino warn. - SPA's `csrfInterceptor` reads the cookie via `document.cookie` and copies its value into `X-CSRF-Token` on every mutating BFF request. The header is omitted on `GET / HEAD / OPTIONS` (BFF skips them anyway) and on non-BFF origins. - Logout and the absolute-timeout middleware both clear the CSRF cookie alongside the session cookie. ## Notable choices **Session-bound double-submit, not pure cookie-vs-header.** A naive "compare cookie with header" check is defeated when an attacker can plant a cookie (subdomain takeover, etc.). Comparing the header to the server-side session-stored token instead means the attacker would also need to be the authenticated user — which is what CSRF defense is supposed to prevent in the first place. **No CSRF for anonymous mutating routes (v1).** None exist today; we don't have an unauthenticated POST endpoint anywhere. Generating a CSRF token for anonymous sessions would conflict with `saveUninitialized: false` on express-session and add complexity we don't need yet. Anonymous public-form CSRF defenses (site-key, captcha) land if and when those routes ship. **`SameSite=Lax`, not `Strict`, on the CSRF cookie.** Matches the session cookie's policy so the two travel together on the SPA→BFF cross-origin same-site fetch (different ports = different origin, same registrable domain). The double-submit pattern is what gives the protection; `SameSite=Lax` is a belt-and-braces layer. **`csrfInterceptor` runs after `bffCredentialsInterceptor` and before `bffUnauthorizedInterceptor` in the chain.** Order: credentials first (set `withCredentials`), then CSRF (set the header), then unauthorized handling (catch 401s). Forward order, no surprises. **`CORS_ALLOWED_ORIGINS` has no localhost fallback.** I considered keeping the fallback for ergonomics but it makes the BFF silently misconfigured if someone forgets the env. The error message points straight at the file to edit. ## Out of scope (next PRs) - Rate limiting + structured error filter (still in the phase-2 to-do). - CSP fine-tuning when we have actual HTML pages (portal-shell + portal-admin static serving). - CSRF token rotation on idle-extension (today the token lives the session's lifetime; refreshing on each request would invalidate in-flight mutations). ## Test plan - [x] `pnpm nx run-many -t test --projects=portal-bff,feature-auth,portal-shell` clean env → **177 + 28 + 34 = 239/239 pass** (was 144 + 19 + 34 = 197 before; +42 specs across CSRF middleware, CSRF cookie helpers, CORS allowlist parser, csrfInterceptor, and extended auth.controller / absolute-timeout coverage). - [x] `pnpm nx run-many -t lint build --projects=portal-bff,feature-auth,portal-shell` → clean. - [x] **CI clean-env repro** (lesson from prior PRs): every env var unset (including new `CORS_ALLOWED_ORIGINS`) → tests still pass. The BFF refuses to boot without `CORS_ALLOWED_ORIGINS`, which is the intended behaviour. - [x] Prettier-clean. - [ ] Manual smoke against running BFF: - [ ] Sign in → `__Host-portal_csrf` (prod) / `portal_csrf` (dev) cookie set, value matches `audit.events.payload->>actorIdHash`-style traceability via `req.session.csrfToken` in Redis. - [ ] Hit a future POST route from the SPA → request carries `X-CSRF-Token`, BFF accepts. - [ ] Forge a POST without the header (curl) → 403 `{"error":"csrf"}`. - [ ] Sign out → both cookies cleared. --------- Co-authored-by: Julien Gautier Reviewed-on: https://git.unespace.com/julien/apf_portal/pulls/122 --- apps/portal-bff/.env.example | 10 + apps/portal-bff/src/app/app.module.ts | 2 + .../src/auth/auth.controller.spec.ts | 30 +++ apps/portal-bff/src/auth/auth.controller.ts | 16 +- .../src/config/check-cors-allowlist.spec.ts | 75 ++++++++ .../src/config/check-cors-allowlist.ts | 78 ++++++++ apps/portal-bff/src/main.ts | 67 +++++-- .../src/security/csrf-cookie.spec.ts | 49 +++++ apps/portal-bff/src/security/csrf-cookie.ts | 53 ++++++ .../src/security/csrf.middleware.spec.ts | 177 ++++++++++++++++++ .../src/security/csrf.middleware.ts | 102 ++++++++++ .../src/security/security.module.ts | 25 +++ .../portal-bff/src/security/security.token.ts | 11 ++ .../absolute-timeout.middleware.spec.ts | 1 + .../session/absolute-timeout.middleware.ts | 2 + apps/portal-bff/src/session/session.types.ts | 10 + apps/portal-shell/src/app/app.config.ts | 15 +- .../src/environments/environment.ts | 9 + libs/feature/auth/src/index.ts | 3 +- libs/feature/auth/src/lib/auth.config.ts | 9 + .../auth/src/lib/csrf.interceptor.spec.ts | 115 ++++++++++++ libs/feature/auth/src/lib/csrf.interceptor.ts | 56 ++++++ package.json | 1 + pnpm-lock.yaml | 9 + 24 files changed, 902 insertions(+), 23 deletions(-) create mode 100644 apps/portal-bff/src/config/check-cors-allowlist.spec.ts create mode 100644 apps/portal-bff/src/config/check-cors-allowlist.ts create mode 100644 apps/portal-bff/src/security/csrf-cookie.spec.ts create mode 100644 apps/portal-bff/src/security/csrf-cookie.ts create mode 100644 apps/portal-bff/src/security/csrf.middleware.spec.ts create mode 100644 apps/portal-bff/src/security/csrf.middleware.ts create mode 100644 apps/portal-bff/src/security/security.module.ts create mode 100644 apps/portal-bff/src/security/security.token.ts create mode 100644 libs/feature/auth/src/lib/csrf.interceptor.spec.ts create mode 100644 libs/feature/auth/src/lib/csrf.interceptor.ts diff --git a/apps/portal-bff/.env.example b/apps/portal-bff/.env.example index 943ac57..d4b9c1c 100644 --- a/apps/portal-bff/.env.example +++ b/apps/portal-bff/.env.example @@ -117,6 +117,16 @@ SESSION_ENCRYPTION_KEY=replace_with_32_random_bytes_base64url # node -e "console.log(require('crypto').randomBytes(32).toString('base64url'))" LOG_USER_ID_SALT=replace_with_32_random_bytes_base64url +# CORS allowlist (per ADR-0009 §"CORS"). Comma-separated list of +# origins (scheme://host[:port]) allowed to call the BFF with +# credentials. The BFF refuses to start without this — silently +# defaulting to localhost is the classic "works in dev, breaks in +# prod" trap. +# +# Local dev: the portal-shell dev server on :4200. Add :4201 once +# portal-admin grows its own dev server. +CORS_ALLOWED_ORIGINS=http://localhost:4200 + # Future env vars introduced by upcoming phases / ADRs: # # Auth flow (ADR-0009) — additional keys wired as the routes land: diff --git a/apps/portal-bff/src/app/app.module.ts b/apps/portal-bff/src/app/app.module.ts index 06d4a0d..626a5a3 100644 --- a/apps/portal-bff/src/app/app.module.ts +++ b/apps/portal-bff/src/app/app.module.ts @@ -7,6 +7,7 @@ import { HealthModule } from '../health/health.module'; import { AuditModule } from '../audit/audit.module'; import { AuthModule } from '../auth/auth.module'; import { RedisModule } from '../redis/redis.module'; +import { SecurityModule } from '../security/security.module'; import { SessionModule } from '../session/session.module'; @Module({ @@ -17,6 +18,7 @@ import { SessionModule } from '../session/session.module'; AuthModule, RedisModule, SessionModule, + SecurityModule, HealthModule, ], controllers: [AppController], diff --git a/apps/portal-bff/src/auth/auth.controller.spec.ts b/apps/portal-bff/src/auth/auth.controller.spec.ts index 799f3d5..73c4d87 100644 --- a/apps/portal-bff/src/auth/auth.controller.spec.ts +++ b/apps/portal-bff/src/auth/auth.controller.spec.ts @@ -55,6 +55,7 @@ interface SessionStub { user?: AuthenticatedUser; createdAt?: number; absoluteExpiresAt?: number; + csrfToken?: string; save: jest.Mock; destroy: jest.Mock; } @@ -261,6 +262,34 @@ describe('AuthController.callback', () => { } }); + it('mints a CSRF token, writes it to the session, and mirrors it to the JS-readable cookie', async () => { + 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); + + // Server-side source of truth lives on the session. + expect(typeof session.csrfToken).toBe('string'); + expect((session.csrfToken as string).length).toBeGreaterThan(20); + + // Cookie mirror — name picked from sessionCookieName's twin + // (NODE_ENV-conditional). In the test default (development), + // it's the unprefixed variant. + const csrfCookieCall = res.cookie.mock.calls.find( + (c) => c[0] === 'portal_csrf' || c[0] === '__Host-portal_csrf', + ); + expect(csrfCookieCall).toBeDefined(); + expect(csrfCookieCall?.[1]).toBe(session.csrfToken); + // Must NOT be HttpOnly — the SPA reads it to echo back via the + // X-CSRF-Token header. + expect(csrfCookieCall?.[2]).toMatchObject({ httpOnly: false }); + }); + it('registers the new session in the user_sessions index after save', async () => { const { controller, userSessionIndex } = makeController(); const res = makeResStub(); @@ -501,6 +530,7 @@ describe('AuthController.logout', () => { }); expect(session.destroy).toHaveBeenCalledTimes(1); expect(res.clearCookie).toHaveBeenCalledWith('portal_session', { path: '/' }); + expect(res.clearCookie).toHaveBeenCalledWith('portal_csrf', { path: '/' }); expect(logger.log).toHaveBeenCalledWith( { event: 'auth.signed_out', wasAuthenticated: true }, 'AuthLogout', diff --git a/apps/portal-bff/src/auth/auth.controller.ts b/apps/portal-bff/src/auth/auth.controller.ts index 0579e65..b2d7edb 100644 --- a/apps/portal-bff/src/auth/auth.controller.ts +++ b/apps/portal-bff/src/auth/auth.controller.ts @@ -1,7 +1,9 @@ +import { randomBytes } from 'node:crypto'; import { Controller, Get, Inject, Query, Req, Res } from '@nestjs/common'; import type { Request, Response } from 'express'; import { Logger } from 'nestjs-pino'; import { AuditWriter } from '../audit/audit.service'; +import { csrfCookieName, csrfCookieOptions } from '../security/csrf-cookie'; import { readSessionTimeouts, sessionCookieName } from '../session/session-cookie'; import { UserSessionIndexService } from '../session/user-session-index.service'; import { @@ -117,19 +119,30 @@ export class AuthController { try { const user = await this.authService.completeAuthCodeFlow(code, state, preAuth); const now = Date.now(); - const { absoluteSeconds } = readSessionTimeouts(); + const { idleSeconds, absoluteSeconds } = readSessionTimeouts(); + const csrfToken = randomBytes(32).toString('base64url'); req.session.user = user; req.session.createdAt = now; // Hard ceiling per ADR-0010 §"TTL policy" — checked on every // request by the absolute-timeout middleware, independent of // idle TTL. req.session.absoluteExpiresAt = now + absoluteSeconds * 1000; + // CSRF token per ADR-0009 §"Double-submit CSRF". Server-side + // source of truth lives on the session; the cookie below is + // the SPA's read-only mirror used to echo the value in the + // X-CSRF-Token header. + req.session.csrfToken = csrfToken; // Force the save before the redirect: express-session writes // on response end, but the 302 we're about to emit closes the // response before the async store-write would otherwise // complete. Without this, the browser hits the SPA before // Redis carries the new payload. await saveSession(req); + // Mirror the CSRF token to a JS-readable cookie. maxAge + // matches the session's idle TTL so the cookie expires at + // the same time as the session (rolling, refreshed on each + // request alongside express-session's own cookie). + res.cookie(csrfCookieName(), csrfToken, csrfCookieOptions(idleSeconds * 1000)); // Register the freshly-minted session id in the per-user // index so a future admin "logout everywhere" can enumerate // and revoke. Best-effort: a Redis hiccup here doesn't fail @@ -232,6 +245,7 @@ export class AuthController { } res.clearCookie(sessionCookieName(), { path: '/' }); + res.clearCookie(csrfCookieName(), { path: '/' }); this.logger.log({ event: 'auth.signed_out', wasAuthenticated }, 'AuthLogout'); res.redirect(302, logoutUrl); } diff --git a/apps/portal-bff/src/config/check-cors-allowlist.spec.ts b/apps/portal-bff/src/config/check-cors-allowlist.spec.ts new file mode 100644 index 0000000..c7f1ce9 --- /dev/null +++ b/apps/portal-bff/src/config/check-cors-allowlist.spec.ts @@ -0,0 +1,75 @@ +import { readCorsAllowlist } from './check-cors-allowlist'; + +describe('readCorsAllowlist', () => { + const original = process.env['CORS_ALLOWED_ORIGINS']; + + afterEach(() => { + if (original === undefined) { + delete process.env['CORS_ALLOWED_ORIGINS']; + } else { + process.env['CORS_ALLOWED_ORIGINS'] = original; + } + }); + + it('parses a single origin', () => { + process.env['CORS_ALLOWED_ORIGINS'] = 'http://localhost:4200'; + expect(readCorsAllowlist()).toEqual(['http://localhost:4200']); + }); + + it('parses a comma-separated list and trims whitespace', () => { + process.env['CORS_ALLOWED_ORIGINS'] = + 'http://localhost:4200, https://portal.apf.example ,https://admin.portal.apf.example'; + expect(readCorsAllowlist()).toEqual([ + 'http://localhost:4200', + 'https://portal.apf.example', + 'https://admin.portal.apf.example', + ]); + }); + + it('filters out empty entries (trailing commas tolerated)', () => { + process.env['CORS_ALLOWED_ORIGINS'] = 'http://localhost:4200,'; + expect(readCorsAllowlist()).toEqual(['http://localhost:4200']); + }); + + it('throws when the var is unset', () => { + delete process.env['CORS_ALLOWED_ORIGINS']; + expect(() => readCorsAllowlist()).toThrow(/CORS_ALLOWED_ORIGINS is not set/); + }); + + it('throws when the var is the empty string', () => { + process.env['CORS_ALLOWED_ORIGINS'] = ''; + expect(() => readCorsAllowlist()).toThrow(/CORS_ALLOWED_ORIGINS is not set/); + }); + + it('throws when the parsed list is empty (only commas / whitespace)', () => { + process.env['CORS_ALLOWED_ORIGINS'] = ' , , '; + expect(() => readCorsAllowlist()).toThrow(/empty after parsing/); + }); + + it('throws on a non-URL entry', () => { + process.env['CORS_ALLOWED_ORIGINS'] = 'not-a-url'; + expect(() => readCorsAllowlist()).toThrow(/not a valid URL/); + }); + + it('throws on a non-http(s) scheme', () => { + process.env['CORS_ALLOWED_ORIGINS'] = 'ftp://files.apf.example'; + expect(() => readCorsAllowlist()).toThrow(/must use http: or https:/); + }); + + it('throws when an entry has a path', () => { + process.env['CORS_ALLOWED_ORIGINS'] = 'https://portal.apf.example/foo'; + expect(() => readCorsAllowlist()).toThrow(/must be a bare origin/); + }); + + it('accepts a trailing slash by treating it as the root path', () => { + // `new URL('https://x.example/').pathname` is "/" — we accept + // that as equivalent to a bare origin. + process.env['CORS_ALLOWED_ORIGINS'] = 'https://portal.apf.example/'; + expect(readCorsAllowlist()).toEqual(['https://portal.apf.example/']); + }); + + it('throws when an entry has a query string', () => { + process.env['CORS_ALLOWED_ORIGINS'] = 'https://portal.apf.example?x=1'; + expect(() => readCorsAllowlist()).toThrow(/no query string or fragment/); + }); +}); diff --git a/apps/portal-bff/src/config/check-cors-allowlist.ts b/apps/portal-bff/src/config/check-cors-allowlist.ts new file mode 100644 index 0000000..470b3cb --- /dev/null +++ b/apps/portal-bff/src/config/check-cors-allowlist.ts @@ -0,0 +1,78 @@ +/** + * Parse and validate the `CORS_ALLOWED_ORIGINS` env var into a list + * of origin strings ready to be passed to `app.enableCors({ origin + * }) `. + * + * Wired in `main.ts` (no eager boot-time pre-flight check — the + * value is consumed at CORS configuration time, after `NestFactory. + * create`). The validator runs there to fail fast on a malformed + * URL. + * + * Format: comma-separated list of origins (`://[:port]`). + * Whitespace around each entry is trimmed. Empty entries are + * filtered out. Trailing slashes are not allowed (an "origin" is + * scheme+host+port; nothing after). + * + * No fallback to a dev default. The BFF refuses to start without + * an explicit allowlist — getting CORS wrong silently is exactly + * the kind of "works in dev, breaks in prod" issue this guard is + * supposed to catch. + * + * Production should set `CORS_ALLOWED_ORIGINS` to the SPA / admin + * SPA's deploy origins (e.g. + * `https://portal.apf.example,https://admin.portal.apf.example`). + * Dev `.env` lists `http://localhost:4200` (and 4201 once + * portal-admin grows a dev server). + */ +export function readCorsAllowlist(): readonly string[] { + const raw = process.env['CORS_ALLOWED_ORIGINS']; + if (raw === undefined || raw === '') { + throw new Error( + `CORS_ALLOWED_ORIGINS is not set. Add the SPA origin(s) to ` + + `apps/portal-bff/.env, e.g. CORS_ALLOWED_ORIGINS=http://localhost:4200`, + ); + } + + const origins = raw + .split(',') + .map((o) => o.trim()) + .filter((o) => o.length > 0); + + if (origins.length === 0) { + throw new Error(`CORS_ALLOWED_ORIGINS is empty after parsing: "${raw}"`); + } + + for (const origin of origins) { + assertOriginShape(origin); + } + + return origins; +} + +function assertOriginShape(origin: string): void { + let url: URL; + try { + url = new URL(origin); + } catch { + throw new Error(`CORS_ALLOWED_ORIGINS entry "${origin}" is not a valid URL`); + } + + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + throw new Error( + `CORS_ALLOWED_ORIGINS entry "${origin}" must use http: or https:, got "${url.protocol}"`, + ); + } + + if (url.pathname !== '/' && url.pathname !== '') { + throw new Error( + `CORS_ALLOWED_ORIGINS entry "${origin}" must be a bare origin ` + + `(scheme://host[:port]) with no path. Got pathname "${url.pathname}".`, + ); + } + + if (url.search !== '' || url.hash !== '') { + throw new Error( + `CORS_ALLOWED_ORIGINS entry "${origin}" must be a bare origin — no query string or fragment.`, + ); + } +} diff --git a/apps/portal-bff/src/main.ts b/apps/portal-bff/src/main.ts index 07d8eee..201c78a 100644 --- a/apps/portal-bff/src/main.ts +++ b/apps/portal-bff/src/main.ts @@ -6,14 +6,17 @@ import './observability/tracing'; import { ValidationPipe } from '@nestjs/common'; import { NestFactory } from '@nestjs/core'; import cookieParser from 'cookie-parser'; +import helmet from 'helmet'; import { Logger } from 'nestjs-pino'; import { AppModule } from './app/app.module'; +import { readCorsAllowlist } from './config/check-cors-allowlist'; import { assertDatabaseUrl } from './config/check-database-url'; import { assertEntraConfig } from './config/check-entra-config'; import { assertRedisConfig } from './config/check-redis-config'; import { assertLogUserIdSalt } from './config/check-log-user-id-salt'; import { assertSessionEncryptionKey } from './config/check-session-encryption-key'; import { assertSessionSecret } from './config/check-session-secret'; +import { CSRF_MIDDLEWARE } from './security/security.token'; import { SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE, SESSION_MIDDLEWARE, @@ -57,21 +60,43 @@ async function bootstrap() { const app = await NestFactory.create(AppModule, { bufferLogs: true }); app.useLogger(app.get(Logger)); - // CORS — minimal dev-time allowlist. The SPA running on - // http://localhost:4200 issues fetches to the BFF and must be - // able to send the W3C `traceparent` (and `tracestate`) headers - // that `@opentelemetry/instrumentation-fetch` injects, so the BFF - // can pick up the parent span id and emit child spans on the same - // trace. The full security-grade allowlist (per-environment - // origins, credentials policy, helmet stack, etc.) lands with the - // phase-2 security ADR — for now this is the minimum needed for - // end-to-end tracing. + // Security headers (phase-2). Defaults from `helmet()` are good + // for an API server returning JSON: X-Frame-Options=SAMEORIGIN, + // X-Content-Type-Options=nosniff, Referrer-Policy=no-referrer, + // X-Powered-By removed, etc. CSP defaults apply too but the BFF + // doesn't render HTML, so they're inert here. + // + // Three overrides for our specific shape: + // - HSTS only in production (dev runs on plain HTTP). + // - crossOriginResourcePolicy: 'cross-origin' so the SPA on its + // own origin can read JSON from the BFF without being blocked + // by Spectre-class CORP protections. + // - contentSecurityPolicy: false in dev — Helmet's default CSP + // blocks `connect-src` from anything but 'self', which is + // fine for HTML pages but irrelevant for JSON responses and + // noisy in browser devtools. + app.use( + helmet({ + hsts: process.env['NODE_ENV'] === 'production', + crossOriginResourcePolicy: { policy: 'cross-origin' }, + contentSecurityPolicy: process.env['NODE_ENV'] === 'production', + }), + ); + + // CORS allowlist — env-driven via `CORS_ALLOWED_ORIGINS`, parsed + // and validated at boot. No hardcoded localhost fallback: getting + // CORS wrong silently is exactly the kind of "works in dev, breaks + // in prod" issue this validator is meant to catch. app.enableCors({ - origin: (process.env['CORS_ALLOWED_ORIGINS'] ?? 'http://localhost:4200') - .split(',') - .map((o) => o.trim()) - .filter(Boolean), - allowedHeaders: ['Content-Type', 'Accept', 'Authorization', 'traceparent', 'tracestate'], + origin: [...readCorsAllowlist()], + allowedHeaders: [ + 'Content-Type', + 'Accept', + 'Authorization', + 'X-CSRF-Token', + 'traceparent', + 'tracestate', + ], credentials: true, }); @@ -104,9 +129,17 @@ async function bootstrap() { // needed). app.use(app.get(SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE)); - // Phase-2 security ADRs will harden the above: helmet, real CORS - // allowlist, CSRF protection, rate limiting, auth guards, - // structured error filter. + // Double-submit CSRF (ADR-0009 §"CSRF defense"). Mounted after + // the session middleware so `req.session.csrfToken` is available + // for comparison with the `X-CSRF-Token` request header. Skips + // safe methods (GET / HEAD / OPTIONS), anonymous requests, and + // the auth entry routes that *mint* the token (`/auth/login`, + // `/auth/callback`). + app.use(app.get(CSRF_MIDDLEWARE)); + + // Phase-2 hardening still pending: rate limiting + structured + // error filter (helmet, CORS allowlist, CSRF protection landed + // with this PR). const globalPrefix = 'api'; app.setGlobalPrefix(globalPrefix); diff --git a/apps/portal-bff/src/security/csrf-cookie.spec.ts b/apps/portal-bff/src/security/csrf-cookie.spec.ts new file mode 100644 index 0000000..781155e --- /dev/null +++ b/apps/portal-bff/src/security/csrf-cookie.spec.ts @@ -0,0 +1,49 @@ +import { csrfCookieName, csrfCookieOptions } from './csrf-cookie'; + +describe('csrf-cookie', () => { + const originalNodeEnv = process.env['NODE_ENV']; + afterEach(() => { + if (originalNodeEnv === undefined) { + delete process.env['NODE_ENV']; + } else { + process.env['NODE_ENV'] = originalNodeEnv; + } + }); + + describe('csrfCookieName', () => { + it('uses __Host-portal_csrf in production', () => { + process.env['NODE_ENV'] = 'production'; + expect(csrfCookieName()).toBe('__Host-portal_csrf'); + }); + + it('uses portal_csrf in development (HTTP, no Secure)', () => { + process.env['NODE_ENV'] = 'development'; + expect(csrfCookieName()).toBe('portal_csrf'); + }); + }); + + describe('csrfCookieOptions', () => { + it('is NOT HttpOnly (SPA must read the value to echo it back)', () => { + expect(csrfCookieOptions(1800_000).httpOnly).toBe(false); + }); + + it('uses SameSite=Lax to travel with the session cookie', () => { + expect(csrfCookieOptions(1800_000).sameSite).toBe('lax'); + }); + + it('sets Secure only in production', () => { + process.env['NODE_ENV'] = 'production'; + expect(csrfCookieOptions(1800_000).secure).toBe(true); + process.env['NODE_ENV'] = 'development'; + expect(csrfCookieOptions(1800_000).secure).toBe(false); + }); + + it('pins path=/', () => { + expect(csrfCookieOptions(1800_000).path).toBe('/'); + }); + + it('passes maxAge through unchanged', () => { + expect(csrfCookieOptions(123456).maxAge).toBe(123456); + }); + }); +}); diff --git a/apps/portal-bff/src/security/csrf-cookie.ts b/apps/portal-bff/src/security/csrf-cookie.ts new file mode 100644 index 0000000..915d3b6 --- /dev/null +++ b/apps/portal-bff/src/security/csrf-cookie.ts @@ -0,0 +1,53 @@ +import type { CookieOptions } from 'express'; + +/** + * Name of the CSRF cookie that pairs with the `__Host-portal_session` + * cookie. Per ADR-0009 §"Double-submit CSRF": the BFF generates a + * random token at session creation and writes it both to the session + * payload (server-side) and to this cookie (readable by the SPA). + * Every mutating request must echo the value back in an + * `X-CSRF-Token` header; the middleware compares the header against + * the *session-stored* token, not the cookie — the cookie is just + * the SPA's UX way of reading the value it's expected to send back. + * + * Same env-conditional naming as the session cookie: + * - production: `__Host-portal_csrf` (prefix mandates Secure + + * Path=/ + no Domain — defeats subdomain cookie injection) + * - development: `portal_csrf` (no Secure available on HTTP) + */ +const PRODUCTION_NAME = '__Host-portal_csrf'; +const DEVELOPMENT_NAME = 'portal_csrf'; + +export function csrfCookieName(): string { + return process.env['NODE_ENV'] === 'production' ? PRODUCTION_NAME : DEVELOPMENT_NAME; +} + +/** + * Cookie options for the CSRF cookie. + * + * Crucially **NOT** `HttpOnly`: the SPA needs JavaScript access to + * read the token and echo it back in the `X-CSRF-Token` header. The + * tradeoff is acceptable because the cookie is _not_ the auth + * credential — the session id cookie (`__Host-portal_session`, which + * IS HttpOnly) is. An XSS that reads this CSRF cookie still cannot + * impersonate the user without also stealing the session cookie. + * + * `SameSite=Lax` (not Strict): matches the session cookie so the + * two travel together on the SPA→BFF cross-origin same-site fetch + * (different ports = different origin but same registrable domain). + * Strict would drop on top-level POST navigations originating from + * a different origin, which is correct for raw CSRF defense but + * loses parity with the session cookie's policy. The double-submit + * pattern itself is what gives the protection here; SameSite=Lax is + * a belt-and-braces layer. + */ +export function csrfCookieOptions(maxAgeMs: number): CookieOptions { + const isProduction = process.env['NODE_ENV'] === 'production'; + return { + httpOnly: false, + sameSite: 'lax', + secure: isProduction, + path: '/', + maxAge: maxAgeMs, + }; +} diff --git a/apps/portal-bff/src/security/csrf.middleware.spec.ts b/apps/portal-bff/src/security/csrf.middleware.spec.ts new file mode 100644 index 0000000..887fbc1 --- /dev/null +++ b/apps/portal-bff/src/security/csrf.middleware.spec.ts @@ -0,0 +1,177 @@ +import type { NextFunction, Request, Response } from 'express'; +import type { Logger } from 'nestjs-pino'; +import { createCsrfMiddleware } from './csrf.middleware'; + +interface SessionStub { + user?: { oid: string }; + csrfToken?: string; +} + +function makeReq(opts: { + method: string; + path: string; + headers?: Record; + session?: SessionStub; +}): Request { + const headers = opts.headers ?? {}; + return { + method: opts.method, + path: opts.path, + session: opts.session, + get: (name: string) => headers[name], + } as unknown as Request; +} + +function makeRes() { + const res = { + status: jest.fn(), + json: jest.fn(), + }; + res.status.mockReturnValue(res); + res.json.mockReturnValue(res); + return res as unknown as Response & { status: jest.Mock; json: jest.Mock }; +} + +function makeLogger() { + return { + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), + } as unknown as Logger & { warn: jest.Mock }; +} + +describe('csrf middleware', () => { + const next: NextFunction = jest.fn(); + beforeEach(() => (next as jest.Mock).mockReset()); + + describe('skip cases', () => { + it.each(['GET', 'HEAD', 'OPTIONS'])('passes through safe method %s', (method) => { + const csrf = createCsrfMiddleware(makeLogger()); + const req = makeReq({ method, path: '/api/anything' }); + const res = makeRes(); + csrf(req, res, next); + expect(next).toHaveBeenCalledTimes(1); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('passes through anonymous mutating requests (no req.session.user)', () => { + const csrf = createCsrfMiddleware(makeLogger()); + const req = makeReq({ method: 'POST', path: '/api/public', session: {} }); + const res = makeRes(); + csrf(req, res, next); + expect(next).toHaveBeenCalledTimes(1); + }); + + it('passes through /api/auth/login (CSRF-exempt: session is born here)', () => { + const csrf = createCsrfMiddleware(makeLogger()); + const req = makeReq({ + method: 'POST', + path: '/api/auth/login', + session: { user: { oid: 'u' }, csrfToken: 'irrelevant' }, + }); + const res = makeRes(); + csrf(req, res, next); + expect(next).toHaveBeenCalledTimes(1); + }); + + it('passes through /api/auth/callback (CSRF-exempt)', () => { + const csrf = createCsrfMiddleware(makeLogger()); + const req = makeReq({ + method: 'POST', + path: '/api/auth/callback', + session: { user: { oid: 'u' }, csrfToken: 'irrelevant' }, + }); + const res = makeRes(); + csrf(req, res, next); + expect(next).toHaveBeenCalledTimes(1); + }); + }); + + describe('enforcement', () => { + it('rejects with 403 when an authenticated mutation lacks X-CSRF-Token', () => { + const logger = makeLogger(); + const csrf = createCsrfMiddleware(logger); + const req = makeReq({ + method: 'POST', + path: '/api/profile', + session: { user: { oid: 'u' }, csrfToken: 'expected-token' }, + }); + const res = makeRes(); + csrf(req, res, next); + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ error: 'csrf' }); + expect(next).not.toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ event: 'csrf.reject', hasHeaderToken: false }), + 'Csrf', + ); + }); + + it('rejects with 403 when the header value mismatches the session token', () => { + const csrf = createCsrfMiddleware(makeLogger()); + const req = makeReq({ + method: 'POST', + path: '/api/profile', + headers: { 'X-CSRF-Token': 'wrong-token' }, + session: { user: { oid: 'u' }, csrfToken: 'expected-token' }, + }); + const res = makeRes(); + csrf(req, res, next); + expect(res.status).toHaveBeenCalledWith(403); + }); + + it('rejects with 403 when the session has no csrfToken (auth without CSRF — should never happen)', () => { + const csrf = createCsrfMiddleware(makeLogger()); + const req = makeReq({ + method: 'POST', + path: '/api/profile', + headers: { 'X-CSRF-Token': 'anything' }, + session: { user: { oid: 'u' } }, + }); + const res = makeRes(); + csrf(req, res, next); + expect(res.status).toHaveBeenCalledWith(403); + }); + + it('passes when header matches the session token exactly', () => { + const csrf = createCsrfMiddleware(makeLogger()); + const req = makeReq({ + method: 'POST', + path: '/api/profile', + headers: { 'X-CSRF-Token': 'expected-token' }, + session: { user: { oid: 'u' }, csrfToken: 'expected-token' }, + }); + const res = makeRes(); + csrf(req, res, next); + expect(next).toHaveBeenCalledTimes(1); + expect(res.status).not.toHaveBeenCalled(); + }); + + it.each(['PUT', 'PATCH', 'DELETE'])('enforces on mutation method %s like POST', (method) => { + const csrf = createCsrfMiddleware(makeLogger()); + const req = makeReq({ + method, + path: '/api/profile', + headers: { 'X-CSRF-Token': 'tok' }, + session: { user: { oid: 'u' }, csrfToken: 'tok' }, + }); + const res = makeRes(); + csrf(req, res, next); + expect(next).toHaveBeenCalledTimes(1); + }); + + it('uses constant-time comparison (different lengths short-circuit as unequal)', () => { + const csrf = createCsrfMiddleware(makeLogger()); + const req = makeReq({ + method: 'POST', + path: '/api/profile', + headers: { 'X-CSRF-Token': 'short' }, + session: { user: { oid: 'u' }, csrfToken: 'much-longer-token-value' }, + }); + const res = makeRes(); + csrf(req, res, next); + expect(res.status).toHaveBeenCalledWith(403); + }); + }); +}); diff --git a/apps/portal-bff/src/security/csrf.middleware.ts b/apps/portal-bff/src/security/csrf.middleware.ts new file mode 100644 index 0000000..71d3199 --- /dev/null +++ b/apps/portal-bff/src/security/csrf.middleware.ts @@ -0,0 +1,102 @@ +import { timingSafeEqual } from 'node:crypto'; +import type { NextFunction, Request, RequestHandler, Response } from 'express'; +import { Logger } from 'nestjs-pino'; + +/** + * Methods that, per RFC 7231 / 9110, MUST NOT alter server state. + * The CSRF check skips them — there is nothing to forge for routes + * that don't change anything. + */ +const SAFE_METHODS = new Set(['GET', 'HEAD', 'OPTIONS']); + +/** + * Paths the CSRF middleware lets through unconditionally. These are + * the OIDC entry points that *create* the session (and the token + * along with it) — there is no token to verify yet. + * + * `GET /auth/logout` is a safe method so it's already covered by + * `SAFE_METHODS`; if a future PR moves logout to POST, the bypass + * here would need extending. + */ +const CSRF_EXEMPT_PREFIXES = ['/api/auth/login', '/api/auth/callback']; + +/** + * Double-submit CSRF middleware (per ADR-0009 §"CSRF defense"). + * + * Contract: + * - GET / HEAD / OPTIONS pass through. + * - Unauthenticated requests pass through. CSRF protects an + * authenticated identity from being abused by another origin; + * there is no identity to abuse on anonymous routes. (Anonymous + * POST endpoints, if they ever exist, will need a different + * defense — site-key, captcha, etc.) + * - `/api/auth/login` and `/api/auth/callback` are exempt: they + * are the routes that *establish* the session and thereby + * mint the CSRF token. + * - Any other request must carry `X-CSRF-Token` whose value + * equals `req.session.csrfToken`. Comparison is + * `crypto.timingSafeEqual` to avoid leaking the token via + * response-time differences. + * - Mismatch / missing → 403 `{ error: 'csrf' }`. No further + * processing. + * + * The middleware reads the token from the **session**, not the + * cookie. The cookie is the SPA's read-only mirror; an attacker + * who can plant a cookie (subdomain takeover, etc.) cannot also + * plant a matching session-stored token without already being the + * authenticated user. Tying the check to the server-side session + * is what distinguishes "session-bound double-submit" from the + * weaker pure cookie-vs-header comparison. + */ +export function createCsrfMiddleware(logger: Logger): RequestHandler { + return function csrf(req: Request, res: Response, next: NextFunction): void { + if (SAFE_METHODS.has(req.method.toUpperCase())) { + next(); + return; + } + + const session = req.session; + if (!session?.user) { + // Anonymous mutating request — out of scope for CSRF v1. + next(); + return; + } + + if (CSRF_EXEMPT_PREFIXES.some((p) => req.path.startsWith(p))) { + next(); + return; + } + + const expected = session.csrfToken; + const headerToken = req.get('X-CSRF-Token'); + + if (!expected || !headerToken || !equalsTimingSafe(headerToken, expected)) { + logger.warn( + { + event: 'csrf.reject', + path: req.path, + method: req.method, + hasHeaderToken: Boolean(headerToken), + hasSessionToken: Boolean(expected), + }, + 'Csrf', + ); + res.status(403).json({ error: 'csrf' }); + return; + } + + next(); + }; +} + +function equalsTimingSafe(a: string, b: string): boolean { + // `timingSafeEqual` requires equal-length buffers. Different + // lengths short-circuit as unequal — we accept the early return + // here because the length itself is not a secret. + const ba = Buffer.from(a, 'utf8'); + const bb = Buffer.from(b, 'utf8'); + if (ba.length !== bb.length) { + return false; + } + return timingSafeEqual(ba, bb); +} diff --git a/apps/portal-bff/src/security/security.module.ts b/apps/portal-bff/src/security/security.module.ts new file mode 100644 index 0000000..8b69837 --- /dev/null +++ b/apps/portal-bff/src/security/security.module.ts @@ -0,0 +1,25 @@ +import { Module } from '@nestjs/common'; +import { Logger } from 'nestjs-pino'; +import { createCsrfMiddleware } from './csrf.middleware'; +import { CSRF_MIDDLEWARE, type RequestHandler } from './security.token'; + +/** + * Phase-2 security primitives (per the `main.ts` placeholder note + * left in earlier PRs). For now: the CSRF middleware. Helmet and + * the CORS allowlist parser are wired directly in `main.ts` because + * they are pure stateless configuration — they don't need DI. + * + * Future phase-2 work that will fit here: rate limiting, structured + * error filter, request-size limits. + */ +@Module({ + providers: [ + { + provide: CSRF_MIDDLEWARE, + inject: [Logger], + useFactory: (logger: Logger): RequestHandler => createCsrfMiddleware(logger), + }, + ], + exports: [CSRF_MIDDLEWARE], +}) +export class SecurityModule {} diff --git a/apps/portal-bff/src/security/security.token.ts b/apps/portal-bff/src/security/security.token.ts new file mode 100644 index 0000000..b67affc --- /dev/null +++ b/apps/portal-bff/src/security/security.token.ts @@ -0,0 +1,11 @@ +import type { RequestHandler } from 'express'; + +/** + * DI token for the double-submit CSRF middleware (per ADR-0009). + * Resolved at bootstrap in `main.ts` and mounted with `app.use(...)` + * after the session middleware so `req.session.csrfToken` is + * available for comparison. + */ +export const CSRF_MIDDLEWARE = 'CSRF_MIDDLEWARE'; + +export type { RequestHandler }; diff --git a/apps/portal-bff/src/session/absolute-timeout.middleware.spec.ts b/apps/portal-bff/src/session/absolute-timeout.middleware.spec.ts index 5334b47..2aa27d4 100644 --- a/apps/portal-bff/src/session/absolute-timeout.middleware.spec.ts +++ b/apps/portal-bff/src/session/absolute-timeout.middleware.spec.ts @@ -162,6 +162,7 @@ describe('absoluteTimeout middleware', () => { expect(session.destroy).toHaveBeenCalledTimes(1); expect(index.remove).toHaveBeenCalledWith(USER.oid, 'session-abc'); expect(res.clearCookie).toHaveBeenCalledWith('portal_session', { path: '/' }); + expect(res.clearCookie).toHaveBeenCalledWith('portal_csrf', { path: '/' }); expect(audit.sessionExpired).toHaveBeenCalledWith({ actor: { oid: USER.oid }, sessionId: 'session-abc', diff --git a/apps/portal-bff/src/session/absolute-timeout.middleware.ts b/apps/portal-bff/src/session/absolute-timeout.middleware.ts index 78dfb70..a16a8fd 100644 --- a/apps/portal-bff/src/session/absolute-timeout.middleware.ts +++ b/apps/portal-bff/src/session/absolute-timeout.middleware.ts @@ -1,6 +1,7 @@ import type { NextFunction, Request, RequestHandler, Response } from 'express'; import { Logger } from 'nestjs-pino'; import { AuditWriter } from '../audit/audit.service'; +import { csrfCookieName } from '../security/csrf-cookie'; import { sessionCookieName } from './session-cookie'; import { UserSessionIndexService } from './user-session-index.service'; @@ -80,6 +81,7 @@ export function createAbsoluteTimeoutMiddleware( await index.remove(userId, sessionId); res.clearCookie(sessionCookieName(), { path: '/' }); + res.clearCookie(csrfCookieName(), { path: '/' }); // Audit the expiry — blocking per ADR-0013: if the audit row // can't be written, the request fails (the user sees a 5xx, diff --git a/apps/portal-bff/src/session/session.types.ts b/apps/portal-bff/src/session/session.types.ts index 81d1c16..7faf560 100644 --- a/apps/portal-bff/src/session/session.types.ts +++ b/apps/portal-bff/src/session/session.types.ts @@ -33,6 +33,16 @@ declare module 'express-session' { * the session. */ absoluteExpiresAt?: number; + /** + * Double-submit CSRF token (per ADR-0009). 32 random bytes + * base64url-encoded; minted at session creation alongside + * `user` and lasts the session lifetime. The same value is + * mirrored to the `__Host-portal_csrf` cookie (HttpOnly: false + * so the SPA can read it) and compared with the `X-CSRF-Token` + * request header by the CSRF middleware. The cookie is the + * SPA's read-only mirror; the source of truth lives here. + */ + csrfToken?: string; } } diff --git a/apps/portal-shell/src/app/app.config.ts b/apps/portal-shell/src/app/app.config.ts index 56fc460..7581b85 100644 --- a/apps/portal-shell/src/app/app.config.ts +++ b/apps/portal-shell/src/app/app.config.ts @@ -7,8 +7,10 @@ import { provideHttpClient, withFetch, withInterceptors } from '@angular/common/ import { provideRouter } from '@angular/router'; import { AUTH_BFF_BASE_URL, + AUTH_CSRF_COOKIE_NAME, bffCredentialsInterceptor, bffUnauthorizedInterceptor, + csrfInterceptor, } from 'feature-auth'; import { environment } from '../environments/environment'; import { appRoutes } from './app.routes'; @@ -33,13 +35,18 @@ export const appConfig: ApplicationConfig = { // when a BFF route (other than `/auth/me` itself) answers // 401, keeping the SPA's auth state in sync after server- // side session destruction (absolute-timeout, manual revoke). + // - `csrfInterceptor` reads the BFF's CSRF cookie and copies + // the value into the `X-CSRF-Token` header on mutating BFF + // requests (POST / PUT / PATCH / DELETE), per the + // double-submit pattern from ADR-0009. provideHttpClient( withFetch(), - withInterceptors([bffCredentialsInterceptor, bffUnauthorizedInterceptor]), + withInterceptors([bffCredentialsInterceptor, csrfInterceptor, bffUnauthorizedInterceptor]), ), - // `feature-auth` is environment-agnostic and reads the BFF base - // URL from this token — provided once per app from `environment.ts` - // (per ADR-0018). + // `feature-auth` is environment-agnostic and reads its BFF + // config from these tokens — provided once per app from + // `environment.ts` (per ADR-0018). { provide: AUTH_BFF_BASE_URL, useValue: environment.bffApiBaseUrl }, + { provide: AUTH_CSRF_COOKIE_NAME, useValue: environment.bffCsrfCookieName }, ], }; diff --git a/apps/portal-shell/src/environments/environment.ts b/apps/portal-shell/src/environments/environment.ts index cb13b92..121f3dd 100644 --- a/apps/portal-shell/src/environments/environment.ts +++ b/apps/portal-shell/src/environments/environment.ts @@ -25,6 +25,15 @@ export const environment = { */ bffApiBaseUrl: 'http://localhost:3000/api', + /** + * Name of the BFF's CSRF cookie. Mirrors the BFF's + * `csrfCookieName()` convention: `__Host-portal_csrf` in + * production (Secure-required, hence excluded from HTTP dev), + * `portal_csrf` otherwise. Read by the `csrfInterceptor` to + * echo the value back in the `X-CSRF-Token` request header. + */ + bffCsrfCookieName: 'portal_csrf', + /** * OTLP/HTTP traces endpoint. Targets the Collector in * `infra/local/dev.compose.yml`. CORS is enabled there — see diff --git a/libs/feature/auth/src/index.ts b/libs/feature/auth/src/index.ts index ce9cbb6..4d52ea0 100644 --- a/libs/feature/auth/src/index.ts +++ b/libs/feature/auth/src/index.ts @@ -1,6 +1,7 @@ -export { AUTH_BFF_BASE_URL, AUTH_NAVIGATOR } from './lib/auth.config'; +export { AUTH_BFF_BASE_URL, AUTH_CSRF_COOKIE_NAME, AUTH_NAVIGATOR } from './lib/auth.config'; export { authGuard } from './lib/auth.guard'; export { AuthService } from './lib/auth.service'; export type { AuthState, CurrentUser } from './lib/auth.types'; export { bffCredentialsInterceptor } from './lib/bff-credentials.interceptor'; export { bffUnauthorizedInterceptor } from './lib/bff-unauthorized.interceptor'; +export { csrfInterceptor } from './lib/csrf.interceptor'; diff --git a/libs/feature/auth/src/lib/auth.config.ts b/libs/feature/auth/src/lib/auth.config.ts index a8507e0..16c3920 100644 --- a/libs/feature/auth/src/lib/auth.config.ts +++ b/libs/feature/auth/src/lib/auth.config.ts @@ -26,3 +26,12 @@ export const AUTH_NAVIGATOR = new InjectionToken<(url: string) => void>('AUTH_NA providedIn: 'root', factory: () => (url: string) => window.location.assign(url), }); + +/** + * Name of the BFF's CSRF cookie that the `csrfInterceptor` reads + * to echo back in the `X-CSRF-Token` header. Provided by the host + * from its per-environment config (`environment.bffCsrfCookieName`) + * so the lib doesn't have to know the env-conditional naming + * (`portal_csrf` dev / `__Host-portal_csrf` prod) the BFF picks. + */ +export const AUTH_CSRF_COOKIE_NAME = new InjectionToken('AUTH_CSRF_COOKIE_NAME'); diff --git a/libs/feature/auth/src/lib/csrf.interceptor.spec.ts b/libs/feature/auth/src/lib/csrf.interceptor.spec.ts new file mode 100644 index 0000000..e095376 --- /dev/null +++ b/libs/feature/auth/src/lib/csrf.interceptor.spec.ts @@ -0,0 +1,115 @@ +import { HttpClient, provideHttpClient, withInterceptors } from '@angular/common/http'; +import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; +import { TestBed } from '@angular/core/testing'; +import { AUTH_BFF_BASE_URL, AUTH_CSRF_COOKIE_NAME } from './auth.config'; +import { csrfInterceptor } from './csrf.interceptor'; + +const BFF_BASE = 'http://bff.test/api'; +const CSRF_COOKIE = 'portal_csrf'; + +function setup() { + TestBed.configureTestingModule({ + providers: [ + provideHttpClient(withInterceptors([csrfInterceptor])), + provideHttpClientTesting(), + { provide: AUTH_BFF_BASE_URL, useValue: BFF_BASE }, + { provide: AUTH_CSRF_COOKIE_NAME, useValue: CSRF_COOKIE }, + ], + }); + return { + http: TestBed.inject(HttpClient), + httpCtrl: TestBed.inject(HttpTestingController), + }; +} + +/** + * jsdom keeps `document.cookie` as a single mutable string. Wipe it + * around each test so prior writes don't leak. + */ +function setCookie(name: string, value: string): void { + document.cookie = `${name}=${value}; path=/`; +} +function clearAllCookies(): void { + const all = document.cookie.split(';'); + for (const c of all) { + const eq = c.indexOf('='); + const name = (eq > -1 ? c.slice(0, eq) : c).trim(); + if (name) document.cookie = `${name}=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/`; + } +} + +describe('csrfInterceptor', () => { + beforeEach(() => clearAllCookies()); + afterEach(() => { + clearAllCookies(); + TestBed.resetTestingModule(); + }); + + it('echoes the cookie value into the X-CSRF-Token header on POST', () => { + setCookie(CSRF_COOKIE, 'tok-abc'); + const { http, httpCtrl } = setup(); + http.post(`${BFF_BASE}/profile`, { name: 'Jane' }).subscribe(); + const req = httpCtrl.expectOne(`${BFF_BASE}/profile`); + expect(req.request.headers.get('X-CSRF-Token')).toBe('tok-abc'); + req.flush({}); + }); + + it.each(['PUT', 'PATCH', 'DELETE'])('sets the header on %s too', (method) => { + setCookie(CSRF_COOKIE, 'tok-abc'); + const { http, httpCtrl } = setup(); + http.request(method, `${BFF_BASE}/profile`, { body: {} }).subscribe(); + const req = httpCtrl.expectOne(`${BFF_BASE}/profile`); + expect(req.request.headers.get('X-CSRF-Token')).toBe('tok-abc'); + req.flush({}); + }); + + it('does NOT set the header on GET (BFF skips CSRF on safe methods)', () => { + setCookie(CSRF_COOKIE, 'tok-abc'); + const { http, httpCtrl } = setup(); + http.get(`${BFF_BASE}/me`).subscribe(); + const req = httpCtrl.expectOne(`${BFF_BASE}/me`); + expect(req.request.headers.has('X-CSRF-Token')).toBe(false); + req.flush({}); + }); + + it('does NOT touch requests to other origins', () => { + setCookie(CSRF_COOKIE, 'tok-abc'); + const { http, httpCtrl } = setup(); + http.post('http://otel.example/v1/traces', {}).subscribe(); + const req = httpCtrl.expectOne('http://otel.example/v1/traces'); + expect(req.request.headers.has('X-CSRF-Token')).toBe(false); + req.flush({}); + }); + + it('omits the header when the cookie is missing (lets the BFF respond with its 403)', () => { + // No setCookie call — cookie isn't set. + const { http, httpCtrl } = setup(); + http.post(`${BFF_BASE}/profile`, {}).subscribe(); + const req = httpCtrl.expectOne(`${BFF_BASE}/profile`); + expect(req.request.headers.has('X-CSRF-Token')).toBe(false); + req.flush({}); + }); + + it('URL-decodes the cookie value (cookies are URI-encoded in transit)', () => { + // Cookies preserve their stored form; if anything URL-encodes + // the token (current BFF doesn't, base64url is safe), we still + // round-trip cleanly. + setCookie(CSRF_COOKIE, encodeURIComponent('A B+/=')); + const { http, httpCtrl } = setup(); + http.post(`${BFF_BASE}/profile`, {}).subscribe(); + const req = httpCtrl.expectOne(`${BFF_BASE}/profile`); + expect(req.request.headers.get('X-CSRF-Token')).toBe('A B+/='); + req.flush({}); + }); + + it('finds the right cookie when others are also set', () => { + setCookie('other_cookie', 'noise'); + setCookie(CSRF_COOKIE, 'tok-abc'); + setCookie('yet_another', 'noise'); + const { http, httpCtrl } = setup(); + http.post(`${BFF_BASE}/profile`, {}).subscribe(); + const req = httpCtrl.expectOne(`${BFF_BASE}/profile`); + expect(req.request.headers.get('X-CSRF-Token')).toBe('tok-abc'); + req.flush({}); + }); +}); diff --git a/libs/feature/auth/src/lib/csrf.interceptor.ts b/libs/feature/auth/src/lib/csrf.interceptor.ts new file mode 100644 index 0000000..65075d6 --- /dev/null +++ b/libs/feature/auth/src/lib/csrf.interceptor.ts @@ -0,0 +1,56 @@ +import type { HttpHandlerFn, HttpInterceptorFn, HttpRequest } from '@angular/common/http'; +import { inject } from '@angular/core'; +import { AUTH_BFF_BASE_URL, AUTH_CSRF_COOKIE_NAME } from './auth.config'; + +/** + * Double-submit CSRF helper interceptor (per ADR-0009). Reads the + * BFF-issued `__Host-portal_csrf` cookie (or its dev twin + * `portal_csrf`) and copies the value into an `X-CSRF-Token` header + * on every BFF request whose method mutates state. + * + * GET / HEAD / OPTIONS pass through without a header — the BFF + * skips CSRF on safe methods anyway, and adding the header would + * just be noise. + * + * Non-BFF origins pass through untouched. Same posture as the other + * BFF-only interceptors in this lib. + * + * If the cookie is missing (anonymous, freshly-cleared session, + * etc.), the request goes through without the header — the BFF + * skips CSRF on anonymous requests, and on the authenticated path + * the BFF will answer 403 which the SPA can surface as "please + * sign in again". + */ +const SAFE_METHODS = new Set(['GET', 'HEAD', 'OPTIONS']); + +export const csrfInterceptor: HttpInterceptorFn = ( + req: HttpRequest, + next: HttpHandlerFn, +) => { + const bffBaseUrl = inject(AUTH_BFF_BASE_URL); + const cookieName = inject(AUTH_CSRF_COOKIE_NAME); + + if (!req.url.startsWith(bffBaseUrl)) return next(req); + if (SAFE_METHODS.has(req.method.toUpperCase())) return next(req); + + const token = readCookie(cookieName); + if (!token) return next(req); + + return next(req.clone({ setHeaders: { 'X-CSRF-Token': token } })); +}; + +function readCookie(name: string): string | null { + // `document.cookie` is the only browser API to read non-HttpOnly + // cookies. The CSRF cookie is deliberately *not* HttpOnly so we + // can do exactly this; the session cookie remains HttpOnly. + const cookies = typeof document !== 'undefined' ? document.cookie : ''; + for (const raw of cookies.split(';')) { + const eq = raw.indexOf('='); + if (eq < 0) continue; + const key = raw.slice(0, eq).trim(); + if (key === name) { + return decodeURIComponent(raw.slice(eq + 1)); + } + } + return null; +} diff --git a/package.json b/package.json index 45a45bf..d662321 100644 --- a/package.json +++ b/package.json @@ -151,6 +151,7 @@ "connect-redis": "^9.0.0", "cookie-parser": "^1.4.7", "express-session": "^1.19.0", + "helmet": "^8.1.0", "ioredis": "^5.10.1", "lucide-angular": "^1.0.0", "nestjs-cls": "^6.2.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f957793..0d7c37c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -126,6 +126,9 @@ importers: express-session: specifier: ^1.19.0 version: 1.19.0 + helmet: + specifier: ^8.1.0 + version: 8.1.0 ioredis: specifier: ^5.10.1 version: 5.10.1 @@ -6510,6 +6513,10 @@ packages: resolution: {integrity: sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==} hasBin: true + helmet@8.1.0: + resolution: {integrity: sha512-jOiHyAZsmnr8LqoPGmCjYAaiuWwjAPLgY8ZX2XrmHawt99/u1y6RgrZMTeoPfpUbV96HOalYgz1qzkRbw54Pmg==} + engines: {node: '>=18.0.0'} + help-me@5.0.0: resolution: {integrity: sha512-7xgomUX6ADmcYzFik0HzAxh/73YlKR9bmFzf51CZwR+b6YtzU2m0u49hQCqV6SvlqIqsaxovfwdvbnsw3b/zpg==} @@ -17339,6 +17346,8 @@ snapshots: he@1.2.0: {} + helmet@8.1.0: {} + help-me@5.0.0: {} homedir-polyfill@1.0.3: