From 60183d51ae2772bad3c5eb0ef9d41a923fb21f60 Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Wed, 13 May 2026 14:08:30 +0200 Subject: [PATCH] feat(portal-bff): wire ADR-0013 audit pipeline to the auth lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the audit foundations were already in place from earlier work (prisma AuditEvent model, postgres roles + grants in infra/local/init/postgres/01-init.sql, AuditWriter.recordEvent with SET LOCAL ROLE audit_writer). this pr layers typed methods on top and emits the first four events from real code paths. typed methods on AuditWriter: signIn, signInFailed, signOut, sessionExpired. callers pass the raw entra oid; hashing happens inside the writer so the salt never leaves the audit module. adr-0013 explicitly defers adding typed methods "as the matching feature ships" — auth has shipped, so we add the four events tied to today's code paths. the other four catalogue entries (session.revoked, token.validation.failed, mfa.assertion.failed, authz.deny) wait for their triggering paths (admin "logout everywhere", obo, requiremfa guard, etc.). HashUserIdService: reads LOG_USER_ID_SALT once at injection, exposes hash(userId) → 16-hex-char digest. same salt + same input ⇒ same output across audit_events.actor_id_hash (adr-0013) and the future pino user_id_hash (adr-0012) — that stability is the join key investigators use to correlate the two streams. LOG_USER_ID_SALT env var: promoted from .env.example's "future vars" block to the active section. mandatory at boot; new assertLogUserIdSalt() validator follows the same pattern as session-secret / session-encryption-key (base64url, ≥32 bytes decoded, placeholder rejected). wired in main.ts. AuditModule is now @Global(): the previous in-line comment claimed it was "imported globally by AppModule" but the decorator was missing — without it, AuthController and the absolute-timeout middleware couldn't inject AuditWriter without re-importing AuditModule. AuditModule now also provides HashUserIdService. emission points: - /auth/callback success → audit.signIn after session.save() (blocking per adr-0013: a failed audit fails the sign-in, the user sees a 5xx, ops gets a pino line). - /auth/callback failure → audit.signInFailed with a discriminator failureKind. covers all six rejection paths: entra-error, missing-code-or-state, no-pre-auth-cookie, and the three AuthCodeFlowError kinds (state-mismatch, flow-expired, token-exchange-failed). the malformed-cookie branch collapses into no-pre-auth-cookie since the controller can't distinguish. - /auth/logout (authenticated only) → audit.signOut before session.destroy() — once destroy runs the actor id is gone. anonymous logout emits no audit row (would be noise). - absolute-timeout middleware → audit.sessionExpired with reason: 'absolute' and ageMs for forensic granularity. idle-ttl expiry stays silent (no bff observation point — redis drops the key on its own). specs: - auth.module.spec + session.module.spec now bootstrap their own @Global() stub for PrismaService + ClsService so auditwriter's transitive resolution works in the slice-of-graph test without booting prisma. lesson learned the hard way on prs #115/#116: nx loads apps/portal-bff/.env locally and masks env-graph bugs that ci's clean shell surfaces. all 142 specs pass under env -u REDIS_URL -u SESSION_SECRET -u SESSION_ENCRYPTION_KEY -u LOG_USER_ID_SALT -u DATABASE_URL -u ENTRA_*. - 19 new specs across check-log-user-id-salt, hash-user-id.service, audit.service typed-methods, auth.controller, and absolute-timeout.middleware. out of scope, landing in follow-ups: - the other four adr-0013 catalogue events. - separate audit_database_url pool with audit_writer-only creds (production hardening, deferred behind SET LOCAL ROLE in v1). - retention purge job + startup self-test probe (on-prem infra adr). - cls-populating middleware that puts actorIdHash on the request context — every emission path in this pr has the user in hand and passes it explicitly; the middleware can come when more routes need it. --- apps/portal-bff/.env.example | 16 +++- apps/portal-bff/src/audit/audit.module.ts | 18 ++-- .../src/audit/audit.service.spec.ts | 92 ++++++++++++++++++- apps/portal-bff/src/audit/audit.service.ts | 77 +++++++++++++++- apps/portal-bff/src/audit/audit.types.ts | 41 +++++++++ .../src/audit/hash-user-id.service.spec.ts | 62 +++++++++++++ .../src/audit/hash-user-id.service.ts | 42 +++++++++ .../src/auth/auth.controller.spec.ts | 90 +++++++++++++++++- apps/portal-bff/src/auth/auth.controller.ts | 21 +++++ apps/portal-bff/src/auth/auth.module.spec.ts | 54 ++++++++--- .../src/config/check-log-user-id-salt.spec.ts | 36 ++++++++ .../src/config/check-log-user-id-salt.ts | 63 +++++++++++++ apps/portal-bff/src/main.ts | 6 ++ .../absolute-timeout.middleware.spec.ts | 52 +++++++++-- .../session/absolute-timeout.middleware.ts | 14 +++ .../src/session/session.module.spec.ts | 29 +++++- apps/portal-bff/src/session/session.module.ts | 10 +- 17 files changed, 682 insertions(+), 41 deletions(-) create mode 100644 apps/portal-bff/src/audit/hash-user-id.service.spec.ts create mode 100644 apps/portal-bff/src/audit/hash-user-id.service.ts create mode 100644 apps/portal-bff/src/config/check-log-user-id-salt.spec.ts create mode 100644 apps/portal-bff/src/config/check-log-user-id-salt.ts diff --git a/apps/portal-bff/.env.example b/apps/portal-bff/.env.example index db2d790..943ac57 100644 --- a/apps/portal-bff/.env.example +++ b/apps/portal-bff/.env.example @@ -105,6 +105,18 @@ SESSION_ENCRYPTION_KEY=replace_with_32_random_bytes_base64url # SESSION_IDLE_TIMEOUT_SECONDS=1800 # SESSION_ABSOLUTE_TIMEOUT_SECONDS=43200 +# Per-environment salt used to pseudonymise the user id before it +# lands in audit rows (per ADR-0013 §"Schema") and in Pino app log +# lines (per ADR-0012 §"User id hashing"). Same value must be used +# on both sides so audit and app logs join on `actor_id_hash`. +# +# Rotation invalidates the join key — old rows / log lines can no +# longer be correlated with the new hash. Treat as long-lived per +# environment. Mandatory at boot. +# +# node -e "console.log(require('crypto').randomBytes(32).toString('base64url'))" +LOG_USER_ID_SALT=replace_with_32_random_bytes_base64url + # Future env vars introduced by upcoming phases / ADRs: # # Auth flow (ADR-0009) — additional keys wired as the routes land: @@ -121,10 +133,6 @@ SESSION_ENCRYPTION_KEY=replace_with_32_random_bytes_base64url # MFA (ADR-0011): # MFA_FRESHNESS_SECONDS (default 600) # -# Observability — additional keys to be wired as features land: -# LOG_USER_ID_SALT (per-environment salt for hashing user_id -# in CLS context — needed once auth lands) -# # Audit trail (ADR-0013): # AUDIT_DATABASE_URL (separate creds, role 'audit_writer') # AUDIT_ARCHIVER_DATABASE_URL (role 'audit_archiver', for the retention purge job) diff --git a/apps/portal-bff/src/audit/audit.module.ts b/apps/portal-bff/src/audit/audit.module.ts index 4653b54..2e705b8 100644 --- a/apps/portal-bff/src/audit/audit.module.ts +++ b/apps/portal-bff/src/audit/audit.module.ts @@ -1,15 +1,19 @@ -import { Module } from '@nestjs/common'; +import { Global, Module } from '@nestjs/common'; import { AuditWriter } from './audit.service'; +import { HashUserIdService } from './hash-user-id.service'; /** - * Provides the AuditWriter to the rest of the BFF. Imported globally - * by AppModule so any feature module can inject it without an extra - * import. The actual append-only contract is enforced by the - * Postgres role grants set up in the audit-schema migration — see + * Provides the AuditWriter (and the salt-bound HashUserIdService it + * depends on) to the rest of the BFF. Marked `@Global()` so a feature + * module can inject `AuditWriter` by writing it in a constructor — + * no need to re-import AuditModule in every feature. The append-only + * contract is enforced by the Postgres role grants set up in the + * audit-schema migration — see * apps/portal-bff/prisma/migrations/*_init_audit_schema. */ +@Global() @Module({ - providers: [AuditWriter], - exports: [AuditWriter], + providers: [HashUserIdService, AuditWriter], + exports: [HashUserIdService, AuditWriter], }) export class AuditModule {} diff --git a/apps/portal-bff/src/audit/audit.service.spec.ts b/apps/portal-bff/src/audit/audit.service.spec.ts index c861395..960686f 100644 --- a/apps/portal-bff/src/audit/audit.service.spec.ts +++ b/apps/portal-bff/src/audit/audit.service.spec.ts @@ -5,6 +5,7 @@ import { PrismaService } from 'nestjs-prisma'; import { Prisma } from '@prisma/client'; import { AuditWriter } from './audit.service'; import type { AuditEventInput } from './audit.types'; +import { HashUserIdService } from './hash-user-id.service'; interface AuditEventCreateCall { data: { @@ -45,17 +46,20 @@ async function createSubject(): Promise<{ writer: AuditWriter; prisma: MockPrisma; cls: { get: jest.Mock }; + hashUserId: { hash: jest.Mock }; }> { const { prisma, cls } = buildMocks(); + const hashUserId = { hash: jest.fn((id: string) => `hash(${id})`) }; const moduleRef = await Test.createTestingModule({ providers: [ AuditWriter, { provide: PrismaService, useValue: prisma }, { provide: ClsService, useValue: cls }, + { provide: HashUserIdService, useValue: hashUserId }, ], }).compile(); - return { writer: moduleRef.get(AuditWriter), prisma, cls }; + return { writer: moduleRef.get(AuditWriter), prisma, cls, hashUserId }; } const baseInput: AuditEventInput = { @@ -172,3 +176,89 @@ describe('AuditWriter', () => { ); }); }); + +describe('AuditWriter — typed event methods', () => { + describe('signIn()', () => { + it('hashes the actor oid and records auth.sign_in (success / workforce)', async () => { + const { writer, prisma, hashUserId } = await createSubject(); + await writer.signIn({ + actor: { oid: 'user-oid', amr: ['pwd', 'mfa'] }, + sessionId: 'sid-1', + }); + expect(hashUserId.hash).toHaveBeenCalledWith('user-oid'); + const call = prisma.tx.auditEvent.create.mock.calls[0]?.[0] as AuditEventCreateCall; + expect(call.data.eventType).toBe('auth.sign_in'); + expect(call.data.audience).toBe('workforce'); + expect(call.data.outcome).toBe('success'); + expect(call.data.actorIdHash).toBe('hash(user-oid)'); + expect(call.data.subject).toBe('session:sid-1'); + expect(call.data.payload).toEqual({ amr: ['pwd', 'mfa'] }); + }); + }); + + describe('signInFailed()', () => { + it('records auth.sign_in.failed with the failureKind and no actor by default', async () => { + const { writer, prisma, hashUserId } = await createSubject(); + await writer.signInFailed({ failureKind: 'state-mismatch' }); + expect(hashUserId.hash).not.toHaveBeenCalled(); + const call = prisma.tx.auditEvent.create.mock.calls[0]?.[0] as AuditEventCreateCall; + expect(call.data.eventType).toBe('auth.sign_in.failed'); + expect(call.data.outcome).toBe('failure'); + expect(call.data.actorIdHash).toBeNull(); + expect(call.data.payload).toEqual({ failureKind: 'state-mismatch' }); + }); + + it('merges payload with failureKind under the same key', async () => { + const { writer, prisma } = await createSubject(); + await writer.signInFailed({ + failureKind: 'entra-error', + payload: { entraError: 'access_denied', entraErrorDescription: 'user cancelled' }, + }); + const call = prisma.tx.auditEvent.create.mock.calls[0]?.[0] as AuditEventCreateCall; + expect(call.data.payload).toEqual({ + failureKind: 'entra-error', + entraError: 'access_denied', + entraErrorDescription: 'user cancelled', + }); + }); + + it('hashes the actor oid when one is provided (rare — identity-after-rejection path)', async () => { + const { writer, prisma, hashUserId } = await createSubject(); + await writer.signInFailed({ failureKind: 'amr-missing', actor: { oid: 'user-oid' } }); + expect(hashUserId.hash).toHaveBeenCalledWith('user-oid'); + const call = prisma.tx.auditEvent.create.mock.calls[0]?.[0] as AuditEventCreateCall; + expect(call.data.actorIdHash).toBe('hash(user-oid)'); + }); + }); + + describe('signOut()', () => { + it('records auth.sign_out (success) with the hashed actor + session subject', async () => { + const { writer, prisma, hashUserId } = await createSubject(); + await writer.signOut({ actor: { oid: 'user-oid' }, sessionId: 'sid-2' }); + expect(hashUserId.hash).toHaveBeenCalledWith('user-oid'); + const call = prisma.tx.auditEvent.create.mock.calls[0]?.[0] as AuditEventCreateCall; + expect(call.data.eventType).toBe('auth.sign_out'); + expect(call.data.outcome).toBe('success'); + expect(call.data.actorIdHash).toBe('hash(user-oid)'); + expect(call.data.subject).toBe('session:sid-2'); + }); + }); + + describe('sessionExpired()', () => { + it('records auth.session.expired with reason + ageMs', async () => { + const { writer, prisma, hashUserId } = await createSubject(); + await writer.sessionExpired({ + actor: { oid: 'user-oid' }, + sessionId: 'sid-3', + reason: 'absolute', + ageMs: 13 * 60 * 60 * 1000, + }); + expect(hashUserId.hash).toHaveBeenCalledWith('user-oid'); + const call = prisma.tx.auditEvent.create.mock.calls[0]?.[0] as AuditEventCreateCall; + expect(call.data.eventType).toBe('auth.session.expired'); + expect(call.data.outcome).toBe('success'); + expect(call.data.subject).toBe('session:sid-3'); + expect(call.data.payload).toEqual({ reason: 'absolute', ageMs: 13 * 60 * 60 * 1000 }); + }); + }); +}); diff --git a/apps/portal-bff/src/audit/audit.service.ts b/apps/portal-bff/src/audit/audit.service.ts index aeb5762..a8d83be 100644 --- a/apps/portal-bff/src/audit/audit.service.ts +++ b/apps/portal-bff/src/audit/audit.service.ts @@ -3,7 +3,14 @@ import { trace } from '@opentelemetry/api'; import { ClsService } from 'nestjs-cls'; import { PrismaService } from 'nestjs-prisma'; import { Prisma } from '@prisma/client'; -import type { AuditEventInput } from './audit.types'; +import type { + AuditEventInput, + SignInActor, + SignInFailedInput, + SignOutInput, + SessionExpiredInput, +} from './audit.types'; +import { HashUserIdService } from './hash-user-id.service'; /** * AuditWriter — single entry point for ADR-0013 audit-log writes. @@ -37,8 +44,76 @@ export class AuditWriter { constructor( private readonly prisma: PrismaService, private readonly cls: ClsService, + private readonly hashUserId: HashUserIdService, ) {} + /** + * Typed event: successful sign-in via the OIDC callback. Per + * ADR-0013's v1 catalogue (`auth.sign_in`). + * + * Hashes the user id internally — callers pass the raw Entra + * `oid`, never the hash, so the salt stays inside the audit + * module. + */ + async signIn(input: { actor: SignInActor; sessionId: string }): Promise { + await this.recordEvent({ + eventType: 'auth.sign_in', + audience: 'workforce', + outcome: 'success', + actorIdHash: this.hashUserId.hash(input.actor.oid), + subject: `session:${input.sessionId}`, + payload: { amr: input.actor.amr }, + }); + } + + /** + * Typed event: failed sign-in at the callback. The `failureKind` + * mirrors the discriminator on `AuthCodeFlowError` so the audit + * row is self-describing without joining anything. + * + * `actorIdHash` is left null on purpose: at the moment of + * failure we may not have resolved an identity yet (state + * mismatch, expired flow, token-exchange error before any user + * claim was parsed). Callers can pass an explicit hash when the + * identity *was* resolved before rejection. + */ + async signInFailed(input: SignInFailedInput): Promise { + await this.recordEvent({ + eventType: 'auth.sign_in.failed', + audience: 'workforce', + outcome: 'failure', + ...(input.actor !== undefined ? { actorIdHash: this.hashUserId.hash(input.actor.oid) } : {}), + payload: { failureKind: input.failureKind, ...(input.payload ?? {}) }, + }); + } + + async signOut(input: SignOutInput): Promise { + await this.recordEvent({ + eventType: 'auth.sign_out', + audience: 'workforce', + outcome: 'success', + actorIdHash: this.hashUserId.hash(input.actor.oid), + subject: `session:${input.sessionId}`, + }); + } + + /** + * Typed event: session destroyed by the absolute-timeout + * middleware (12 h hard ceiling, ADR-0010 §"TTL policy"). The + * idle-TTL expiry is *not* surfaced through this method — it + * happens silently inside Redis with no BFF observation point. + */ + async sessionExpired(input: SessionExpiredInput): Promise { + await this.recordEvent({ + eventType: 'auth.session.expired', + audience: 'workforce', + outcome: 'success', + actorIdHash: this.hashUserId.hash(input.actor.oid), + subject: `session:${input.sessionId}`, + payload: { reason: input.reason, ageMs: input.ageMs }, + }); + } + async recordEvent(input: AuditEventInput): Promise { const traceId = trace.getActiveSpan()?.spanContext().traceId ?? null; const actorIdHash = diff --git a/apps/portal-bff/src/audit/audit.types.ts b/apps/portal-bff/src/audit/audit.types.ts index bc2302a..78bab7f 100644 --- a/apps/portal-bff/src/audit/audit.types.ts +++ b/apps/portal-bff/src/audit/audit.types.ts @@ -41,3 +41,44 @@ export interface AuditEventInput { */ actorIdHash?: string; } + +/** + * Identity payload accepted by the typed audit methods. The raw + * Entra `oid` is hashed by `AuditWriter` itself so the salt stays + * inside the module. `amr` is captured for the sign-in record. + */ +export interface SignInActor { + oid: string; + amr: readonly string[]; +} + +export interface SignInFailedInput { + failureKind: string; + /** + * Optional — pass only when the identity was actually resolved + * before the rejection (rare). Most failure paths (state + * mismatch, expired flow, token-exchange error) reject before any + * user claim is parsed and should leave `actor` undefined. + */ + actor?: { oid: string }; + /** + * Extra fields to merge into the audit row's payload alongside + * `failureKind`. PII is the caller's responsibility, same posture + * as `AuditEventInput.payload`. + */ + payload?: Record; +} + +export interface SignOutInput { + actor: { oid: string }; + sessionId: string; +} + +export interface SessionExpiredInput { + actor: { oid: string }; + sessionId: string; + /** `absolute` is the only reason emitted in v1; `idle` is silent. */ + reason: 'absolute'; + /** Age of the session at the moment of expiry, in milliseconds. */ + ageMs: number; +} diff --git a/apps/portal-bff/src/audit/hash-user-id.service.spec.ts b/apps/portal-bff/src/audit/hash-user-id.service.spec.ts new file mode 100644 index 0000000..26de611 --- /dev/null +++ b/apps/portal-bff/src/audit/hash-user-id.service.spec.ts @@ -0,0 +1,62 @@ +import { randomBytes } from 'node:crypto'; +import { HashUserIdService } from './hash-user-id.service'; + +const STRONG_SALT = randomBytes(32).toString('base64url'); + +function withSalt(salt: string, fn: () => T): T { + const original = process.env['LOG_USER_ID_SALT']; + process.env['LOG_USER_ID_SALT'] = salt; + try { + return fn(); + } finally { + if (original === undefined) { + delete process.env['LOG_USER_ID_SALT']; + } else { + process.env['LOG_USER_ID_SALT'] = original; + } + } +} + +describe('HashUserIdService', () => { + it('hashes the same input to the same output (stable join key)', () => { + withSalt(STRONG_SALT, () => { + const service = new HashUserIdService(); + expect(service.hash('user-oid-42')).toBe(service.hash('user-oid-42')); + }); + }); + + it('returns a 16-hex-char digest', () => { + withSalt(STRONG_SALT, () => { + const service = new HashUserIdService(); + const hash = service.hash('user-oid-42'); + expect(hash).toMatch(/^[0-9a-f]{16}$/); + }); + }); + + it('produces different outputs for different inputs', () => { + withSalt(STRONG_SALT, () => { + const service = new HashUserIdService(); + expect(service.hash('alice')).not.toBe(service.hash('bob')); + }); + }); + + it('produces different outputs across salts (rotation invalidates the join key)', () => { + const a = withSalt(STRONG_SALT, () => new HashUserIdService().hash('alice')); + const b = withSalt(randomBytes(32).toString('base64url'), () => + new HashUserIdService().hash('alice'), + ); + expect(a).not.toBe(b); + }); + + it('refuses to construct when LOG_USER_ID_SALT is unset', () => { + const original = process.env['LOG_USER_ID_SALT']; + delete process.env['LOG_USER_ID_SALT']; + try { + expect(() => new HashUserIdService()).toThrow(/LOG_USER_ID_SALT is not set/); + } finally { + if (original !== undefined) { + process.env['LOG_USER_ID_SALT'] = original; + } + } + }); +}); diff --git a/apps/portal-bff/src/audit/hash-user-id.service.ts b/apps/portal-bff/src/audit/hash-user-id.service.ts new file mode 100644 index 0000000..b06c9ad --- /dev/null +++ b/apps/portal-bff/src/audit/hash-user-id.service.ts @@ -0,0 +1,42 @@ +import { createHash } from 'node:crypto'; +import { Injectable } from '@nestjs/common'; +import { assertLogUserIdSalt } from '../config/check-log-user-id-salt'; + +/** + * Pseudonymises a user id (Entra `oid`) into the 16-hex-char hash + * that both ADR-0013 (`audit_events.actor_id_hash`) and ADR-0012 + * (Pino `user_id_hash`) store. The salt is read once at injection + * time so the per-request hot path runs purely on `node:crypto`. + * + * Same salt + same input ⇒ same output, on both writers, across + * application restarts within a given environment. That stability + * is the *whole point*: an investigator joins audit rows with app + * log lines on this hash, then re-hydrates the join key to the + * cleartext user id only inside the live operational DB. + * + * Rotating the salt invalidates the join key for everything that + * was written before the rotation. Treat as long-lived per + * environment. + * + * Hash construction: + * sha256(`${salt}:${userId}`).hex.slice(0, 16) + * + * SHA-256 is overkill for collision resistance at 16 hex chars + * (64 bits of output), but truncation is intentional — log lines + * and audit rows want a compact identifier, and 64 bits of + * randomness is well above the birthday bound for any realistic + * user count. The salt prevents rainbow attacks even on the + * truncated form. + */ +@Injectable() +export class HashUserIdService { + private readonly salt: string; + + constructor() { + this.salt = assertLogUserIdSalt(); + } + + hash(userId: string): string { + return createHash('sha256').update(`${this.salt}:${userId}`).digest('hex').slice(0, 16); + } +} diff --git a/apps/portal-bff/src/auth/auth.controller.spec.ts b/apps/portal-bff/src/auth/auth.controller.spec.ts index 6ee0628..aea05c6 100644 --- a/apps/portal-bff/src/auth/auth.controller.spec.ts +++ b/apps/portal-bff/src/auth/auth.controller.spec.ts @@ -1,5 +1,6 @@ import type { Request, Response } from 'express'; import type { Logger } from 'nestjs-pino'; +import type { AuditWriter } from '../audit/audit.service'; import type { UserSessionIndexService } from '../session/user-session-index.service'; import { AuthController } from './auth.controller'; import { PRE_AUTH_COOKIE_NAME, PRE_AUTH_COOKIE_TTL_MS } from './auth.cookie'; @@ -102,6 +103,12 @@ interface ControllerFixture { completeAuthCodeFlow: jest.Mock; logger: ReturnType; userSessionIndex: { add: jest.Mock; remove: jest.Mock; list: jest.Mock }; + audit: { + signIn: jest.Mock; + signInFailed: jest.Mock; + signOut: jest.Mock; + sessionExpired: jest.Mock; + }; } const LOGOUT_URL = `${ENTRA.authority}/oauth2/v2.0/logout?post_logout_redirect_uri=${encodeURIComponent(ENTRA.postLogoutRedirectUri)}`; @@ -123,6 +130,12 @@ function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): Controller remove: jest.fn().mockResolvedValue(undefined), list: jest.fn().mockResolvedValue([]), }; + const audit = { + signIn: jest.fn().mockResolvedValue(undefined), + signInFailed: jest.fn().mockResolvedValue(undefined), + signOut: jest.fn().mockResolvedValue(undefined), + sessionExpired: jest.fn().mockResolvedValue(undefined), + }; const logger = makeLoggerStub(); return { controller: new AuthController( @@ -130,11 +143,13 @@ function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): Controller logger as unknown as Logger, ENTRA, userSessionIndex as unknown as UserSessionIndexService, + audit as unknown as AuditWriter, ), beginAuthCodeFlow, completeAuthCodeFlow, logger, userSessionIndex, + audit, }; } @@ -252,6 +267,65 @@ describe('AuthController.callback', () => { expect(session.save).toHaveBeenCalled(); }); + it('emits an auth.sign_in audit event on the happy path', async () => { + const { controller, audit } = makeController(); + const res = makeResStub(); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + session: makeSessionStub(), + sessionID: 'fresh-sid', + }); + + await controller.callback(req, res, 'auth-code', PRE_AUTH.state); + + expect(audit.signIn).toHaveBeenCalledWith({ + actor: USER, + sessionId: 'fresh-sid', + }); + expect(audit.signInFailed).not.toHaveBeenCalled(); + }); + + it('emits auth.sign_in.failed on the Entra-error branch', async () => { + const { controller, audit } = makeController(); + const res = makeResStub(); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + }); + + await controller.callback(req, res, undefined, undefined, 'access_denied', 'user cancelled'); + + expect(audit.signInFailed).toHaveBeenCalledWith({ + failureKind: 'entra-error', + payload: { entraError: 'access_denied', entraErrorDescription: 'user cancelled' }, + }); + expect(audit.signIn).not.toHaveBeenCalled(); + }); + + it('emits auth.sign_in.failed (no-pre-auth-cookie) when the cookie is missing', async () => { + const { controller, audit } = makeController(); + const res = makeResStub(); + const req = makeReqStub(); + + await controller.callback(req, res, 'code', 'state'); + + expect(audit.signInFailed).toHaveBeenCalledWith({ failureKind: 'no-pre-auth-cookie' }); + }); + + it('emits auth.sign_in.failed with the discriminator kind on AuthCodeFlowException', async () => { + const completeAuthCodeFlow = jest + .fn() + .mockRejectedValue(new AuthCodeFlowException({ kind: 'state-mismatch' })); + const { controller, audit } = makeController({ completeAuthCodeFlow }); + const res = makeResStub(); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + }); + + await controller.callback(req, res, 'code', 'state'); + + expect(audit.signInFailed).toHaveBeenCalledWith({ failureKind: 'state-mismatch' }); + }); + it('propagates a session.save() error as an exception (so the SPA never sees a successful redirect with no session)', async () => { const { controller } = makeController(); const res = makeResStub(); @@ -398,8 +472,8 @@ describe('AuthController.me', () => { }); describe('AuthController.logout', () => { - it('destroys the session, clears the session cookie, logs success, redirects to Entra logout', async () => { - const { controller, logger, userSessionIndex } = makeController(); + it('destroys the session, clears the session cookie, logs + audits sign-out, redirects to Entra logout', async () => { + const { controller, logger, userSessionIndex, audit } = makeController(); const res = makeResStub(); const session = makeSessionStub({ user: USER }); const req = makeReqStub({ session, sessionID: 'logged-out-sid' }); @@ -407,6 +481,10 @@ describe('AuthController.logout', () => { await controller.logout(req, res); expect(userSessionIndex.remove).toHaveBeenCalledWith(USER.oid, 'logged-out-sid'); + expect(audit.signOut).toHaveBeenCalledWith({ + actor: USER, + sessionId: 'logged-out-sid', + }); expect(session.destroy).toHaveBeenCalledTimes(1); expect(res.clearCookie).toHaveBeenCalledWith('portal_session', { path: '/' }); expect(logger.log).toHaveBeenCalledWith( @@ -446,8 +524,8 @@ describe('AuthController.logout', () => { } }); - it('still clears the cookie and redirects when the user was already anonymous', async () => { - const { controller, logger, userSessionIndex } = makeController(); + it('still clears the cookie and redirects when the user was already anonymous (no audit row either)', async () => { + const { controller, logger, userSessionIndex, audit } = makeController(); const res = makeResStub(); const session = makeSessionStub(); const req = makeReqStub({ session }); @@ -456,8 +534,10 @@ describe('AuthController.logout', () => { // Nothing was ever added to the index for an anonymous request — // skip the SREM (cheaper + avoids racing the cleanup of a - // future user's session id collision). + // future user's session id collision). The audit row is also + // skipped: an "anonymous user logged out" event is noise. expect(userSessionIndex.remove).not.toHaveBeenCalled(); + expect(audit.signOut).not.toHaveBeenCalled(); expect(session.destroy).toHaveBeenCalledTimes(1); expect(logger.log).toHaveBeenCalledWith( { event: 'auth.signed_out', wasAuthenticated: false }, diff --git a/apps/portal-bff/src/auth/auth.controller.ts b/apps/portal-bff/src/auth/auth.controller.ts index 156d02e..0579e65 100644 --- a/apps/portal-bff/src/auth/auth.controller.ts +++ b/apps/portal-bff/src/auth/auth.controller.ts @@ -1,6 +1,7 @@ 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 { readSessionTimeouts, sessionCookieName } from '../session/session-cookie'; import { UserSessionIndexService } from '../session/user-session-index.service'; import { @@ -29,6 +30,7 @@ export class AuthController { private readonly logger: Logger, @Inject(ENTRA_CONFIG) private readonly entra: EntraConfig, private readonly userSessionIndex: UserSessionIndexService, + private readonly audit: AuditWriter, ) {} /** @@ -90,10 +92,15 @@ export class AuthController { }, 'AuthCallback', ); + await this.audit.signInFailed({ + failureKind: 'entra-error', + payload: { entraError, entraErrorDescription }, + }); return this.redirectWithError(res, 'token-exchange-failed'); } if (typeof code !== 'string' || typeof state !== 'string') { + await this.audit.signInFailed({ failureKind: 'missing-code-or-state' }); return this.redirectWithError(res, 'token-exchange-failed'); } @@ -103,6 +110,7 @@ export class AuthController { // their browser dropped the cookie (TTL elapsed in // a 3rd-party-cookie blocker, etc.). Treat as flow-expired. this.logger.warn({ event: 'auth.no_pre_auth_cookie' }, 'AuthCallback'); + await this.audit.signInFailed({ failureKind: 'no-pre-auth-cookie' }); return this.redirectWithError(res, 'flow-expired'); } @@ -127,6 +135,12 @@ export class AuthController { // and revoke. Best-effort: a Redis hiccup here doesn't fail // sign-in (the service swallows + logs). await this.userSessionIndex.add(user.oid, req.sessionID); + // First write to the audit trail — blocking, per ADR-0013. + // If this throws the user does NOT see a successful sign-in: + // the exception propagates and the controller emits a 5xx via + // Nest's default exception filter. Same posture as the + // session.save() above. + await this.audit.signIn({ actor: user, sessionId: req.sessionID }); this.logger.log( { event: 'auth.signed_in', @@ -141,6 +155,7 @@ export class AuthController { } catch (err) { if (err instanceof AuthCodeFlowException) { this.logger.warn({ event: 'auth.flow_error', failure: err.failure }, 'AuthCallback'); + await this.audit.signInFailed({ failureKind: err.failure.kind }); return this.redirectWithError(res, err.failure.kind); } throw err; @@ -192,6 +207,12 @@ export class AuthController { // requests (nothing was ever added). if (user) { await this.userSessionIndex.remove(user.oid, sessionId); + // Audit the sign-out before tearing the session down — once + // destroy() runs we lose the actor id. Blocking per ADR-0013: + // if the audit row can't be written, the user does NOT get a + // "you're logged out" experience, because we can't certify + // the sign-out happened. + await this.audit.signOut({ actor: user, sessionId }); } try { diff --git a/apps/portal-bff/src/auth/auth.module.spec.ts b/apps/portal-bff/src/auth/auth.module.spec.ts index 64226c9..4a24c6b 100644 --- a/apps/portal-bff/src/auth/auth.module.spec.ts +++ b/apps/portal-bff/src/auth/auth.module.spec.ts @@ -1,18 +1,40 @@ import { randomBytes } from 'node:crypto'; import { ConfidentialClientApplication } from '@azure/msal-node'; +import { Global, Module } from '@nestjs/common'; import { Test } from '@nestjs/testing'; +import { ClsService } from 'nestjs-cls'; import { LoggerModule } from 'nestjs-pino'; +import { PrismaService } from 'nestjs-prisma'; +import { AuditModule } from '../audit/audit.module'; import { AuthModule } from './auth.module'; import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token'; import { MSAL_CLIENT } from './msal-client.token'; -// AuthModule imports SessionModule (so AuthController can inject -// `UserSessionIndexService`), which transitively pulls in -// RedisModule + the session-secret / session-encryption-key -// validators. The spec has to satisfy all of those env vars or the -// test fails as soon as `compile()` walks the import graph — which -// is exactly what bit the CI on PR #115 (CI starts with a clean env; -// locally `nx test` was loading apps/portal-bff/.env and masking it). +// Production app makes PrismaService + ClsService globally available +// via `PrismaModule.forRoot({ isGlobal: true })` and ObservabilityModule. +// In a slice-of-graph spec we stub both via a tiny @Global() module +// so AuditModule's transitive resolution of AuditWriter succeeds +// without booting Prisma or nestjs-cls for real. +@Global() +@Module({ + providers: [ + { provide: PrismaService, useValue: {} }, + { provide: ClsService, useValue: { get: () => undefined } }, + ], + exports: [PrismaService, ClsService], +}) +class TestStubsModule {} + +// AuthModule imports SessionModule + (transitively, via AuditModule +// being @Global) needs the AuditWriter providers. compile() walks +// the full import graph so the spec has to satisfy every validator +// at boot time, including: +// - SessionModule's RedisModule (REDIS_URL) +// - SessionModule's middleware factory (SESSION_SECRET + +// SESSION_ENCRYPTION_KEY) +// - AuditModule's HashUserIdService (LOG_USER_ID_SALT) +// Lesson from PR #115/#116: CI starts with a clean env, masking the +// failure that nx-loaded apps/portal-bff/.env hides locally. const VALID = { ENTRA_INSTANCE_URL: 'https://login.microsoftonline.com/', ENTRA_TENANT_ID: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', @@ -25,16 +47,26 @@ const VALID = { REDIS_URL: 'redis://default:test-pass@127.0.0.1:65535/0', SESSION_SECRET: randomBytes(32).toString('base64url'), SESSION_ENCRYPTION_KEY: randomBytes(32).toString('base64url'), + LOG_USER_ID_SALT: randomBytes(32).toString('base64url'), + // PrismaModule reads this at boot; an unreachable URL is fine in + // tests because we never issue queries. + DATABASE_URL: 'postgresql://test:test@127.0.0.1:65535/test', }; // AuthModule's MSAL_CLIENT factory injects the Pino-backed `Logger` // (which the production app supplies through `ObservabilityModule`). -// Importing `LoggerModule.forRoot({ pinoHttp: { level: 'silent' } })` -// here registers the same provider for the testing module without -// flooding test stdout. +// `AuditModule` is `@Global()` in production via AppModule; here we +// import it explicitly because we compile a slice of the graph. +// AuditWriter's deps (PrismaService, ClsService) are stubbed — the +// spec doesn't need a working DB, only that the wiring resolves. async function compile() { return Test.createTestingModule({ - imports: [LoggerModule.forRoot({ pinoHttp: { level: 'silent' } }), AuthModule], + imports: [ + LoggerModule.forRoot({ pinoHttp: { level: 'silent' } }), + TestStubsModule, + AuditModule, + AuthModule, + ], }).compile(); } diff --git a/apps/portal-bff/src/config/check-log-user-id-salt.spec.ts b/apps/portal-bff/src/config/check-log-user-id-salt.spec.ts new file mode 100644 index 0000000..6508395 --- /dev/null +++ b/apps/portal-bff/src/config/check-log-user-id-salt.spec.ts @@ -0,0 +1,36 @@ +import { randomBytes } from 'node:crypto'; +import { assertLogUserIdSalt } from './check-log-user-id-salt'; + +const STRONG_SALT = randomBytes(32).toString('base64url'); + +describe('assertLogUserIdSalt', () => { + const original = process.env['LOG_USER_ID_SALT']; + + afterEach(() => { + if (original === undefined) { + delete process.env['LOG_USER_ID_SALT']; + } else { + process.env['LOG_USER_ID_SALT'] = original; + } + }); + + it('returns the value when LOG_USER_ID_SALT decodes to ≥ 32 bytes', () => { + process.env['LOG_USER_ID_SALT'] = STRONG_SALT; + expect(assertLogUserIdSalt()).toBe(STRONG_SALT); + }); + + it('throws when LOG_USER_ID_SALT is unset', () => { + delete process.env['LOG_USER_ID_SALT']; + expect(() => assertLogUserIdSalt()).toThrow(/LOG_USER_ID_SALT is not set/); + }); + + it('throws when LOG_USER_ID_SALT is the .env.example placeholder', () => { + process.env['LOG_USER_ID_SALT'] = 'replace_with_32_random_bytes_base64url'; + expect(() => assertLogUserIdSalt()).toThrow(/placeholder/); + }); + + it('throws when LOG_USER_ID_SALT decodes below 32 bytes', () => { + process.env['LOG_USER_ID_SALT'] = randomBytes(16).toString('base64url'); + expect(() => assertLogUserIdSalt()).toThrow(/decodes to 16 bytes/); + }); +}); diff --git a/apps/portal-bff/src/config/check-log-user-id-salt.ts b/apps/portal-bff/src/config/check-log-user-id-salt.ts new file mode 100644 index 0000000..2916855 --- /dev/null +++ b/apps/portal-bff/src/config/check-log-user-id-salt.ts @@ -0,0 +1,63 @@ +/** + * Sanity-check the `LOG_USER_ID_SALT` env var early in bootstrap so + * a missing or obviously weak value fails fast instead of producing + * predictable hashes at runtime. + * + * Wired in `main.ts` alongside the other `assertX()` validators — + * same pre-flight family per ADR-0018 §"BFF env-var loading". + * + * `LOG_USER_ID_SALT` is the per-environment salt fed into SHA-256 + * to pseudonymise user ids before they land in audit rows + * (`actor_id_hash`, per ADR-0013 §"Schema") and in Pino app log + * lines (`user_id_hash`, per ADR-0012 §"User id hashing"). The + * same value must be in scope on both writers so the two streams + * join on the hash. + * + * Returns the salt as a string so callers (`HashUserIdService`) + * can fold it into the hash without re-reading the env. + */ + +const PLACEHOLDER = 'replace_with_32_random_bytes_base64url'; +const MIN_ENTROPY_BYTES = 32; + +export function assertLogUserIdSalt(): string { + const raw = process.env['LOG_USER_ID_SALT']; + if (!raw || raw === '') { + throw new Error( + `LOG_USER_ID_SALT is not set. Generate one with ` + + `"node -e \\"console.log(require('crypto').randomBytes(32).toString('base64url'))\\"" ` + + `and put it in apps/portal-bff/.env.`, + ); + } + + if (raw === PLACEHOLDER) { + throw new Error( + `LOG_USER_ID_SALT is still set to the .env.example placeholder ` + + `("${PLACEHOLDER}"). Replace with a real random value.`, + ); + } + + // Accept base64url-encoded input (the documented generation + // recipe). Anything below 32 bytes of decoded entropy is weaker + // than the project's AES-256 key family — reject early. + let decoded: Buffer; + try { + decoded = Buffer.from(raw, 'base64url'); + } catch { + throw new Error(`LOG_USER_ID_SALT must be a base64url-encoded string. Got: ${truncate(raw)}`); + } + + if (decoded.length < MIN_ENTROPY_BYTES) { + throw new Error( + `LOG_USER_ID_SALT decodes to ${decoded.length} bytes, ` + + `below the ${MIN_ENTROPY_BYTES}-byte minimum (≈ 256 bits of entropy). ` + + `Generate a longer value.`, + ); + } + + return raw; +} + +function truncate(s: string): string { + return s.length > 16 ? `${s.slice(0, 16)}…` : s; +} diff --git a/apps/portal-bff/src/main.ts b/apps/portal-bff/src/main.ts index a2049f6..07d8eee 100644 --- a/apps/portal-bff/src/main.ts +++ b/apps/portal-bff/src/main.ts @@ -11,6 +11,7 @@ import { AppModule } from './app/app.module'; 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 { @@ -44,6 +45,11 @@ assertRedisConfig(); // surface on the first authenticated request otherwise. assertSessionEncryptionKey(); +// LOG_USER_ID_SALT — per-environment SHA-256 salt for the actor-id +// hash that joins audit rows (ADR-0013) and Pino log lines +// (ADR-0012). Mandatory at boot. +assertLogUserIdSalt(); + async function bootstrap() { // `bufferLogs: true` holds early-bootstrap log lines until the // Pino-based Logger is wired in below, so we don't lose anything 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 1265078..5334b47 100644 --- a/apps/portal-bff/src/session/absolute-timeout.middleware.spec.ts +++ b/apps/portal-bff/src/session/absolute-timeout.middleware.spec.ts @@ -1,5 +1,6 @@ import type { NextFunction, Request, Response } from 'express'; import type { Logger } from 'nestjs-pino'; +import type { AuditWriter } from '../audit/audit.service'; import { createAbsoluteTimeoutMiddleware } from './absolute-timeout.middleware'; import type { UserSessionIndexService } from './user-session-index.service'; @@ -42,6 +43,12 @@ function makeIndex(): UserSessionIndexService & { add: jest.Mock; remove: jest.M } as unknown as UserSessionIndexService & { add: jest.Mock; remove: jest.Mock }; } +function makeAudit(): AuditWriter & { sessionExpired: jest.Mock } { + return { + sessionExpired: jest.fn().mockResolvedValue(undefined), + } as unknown as AuditWriter & { sessionExpired: jest.Mock }; +} + function makeLogger() { return { log: jest.fn(), @@ -65,7 +72,12 @@ const USER = { describe('absoluteTimeout middleware', () => { it('passes through when there is no session at all', async () => { - const middleware = createAbsoluteTimeoutMiddleware(makeIndex(), makeLogger(), () => NOW); + const middleware = createAbsoluteTimeoutMiddleware( + makeIndex(), + makeLogger(), + makeAudit(), + () => NOW, + ); const req = makeReq(undefined); const res = makeRes(); const next: NextFunction = jest.fn(); @@ -78,7 +90,12 @@ describe('absoluteTimeout middleware', () => { it('passes through when the session has no user (anonymous request hit a session-touching route)', async () => { const session = makeSession(); - const middleware = createAbsoluteTimeoutMiddleware(makeIndex(), makeLogger(), () => NOW); + const middleware = createAbsoluteTimeoutMiddleware( + makeIndex(), + makeLogger(), + makeAudit(), + () => NOW, + ); const res = makeRes(); const next: NextFunction = jest.fn(); @@ -90,7 +107,12 @@ describe('absoluteTimeout middleware', () => { it('passes through when absoluteExpiresAt is unset (pre-hardening session)', async () => { const session = makeSession({ user: USER }); - const middleware = createAbsoluteTimeoutMiddleware(makeIndex(), makeLogger(), () => NOW); + const middleware = createAbsoluteTimeoutMiddleware( + makeIndex(), + makeLogger(), + makeAudit(), + () => NOW, + ); const res = makeRes(); const next: NextFunction = jest.fn(); @@ -106,7 +128,12 @@ describe('absoluteTimeout middleware', () => { createdAt: NOW - 60_000, absoluteExpiresAt: NOW + 60_000, }); - const middleware = createAbsoluteTimeoutMiddleware(makeIndex(), makeLogger(), () => NOW); + const middleware = createAbsoluteTimeoutMiddleware( + makeIndex(), + makeLogger(), + makeAudit(), + () => NOW, + ); const res = makeRes(); const next: NextFunction = jest.fn(); @@ -117,7 +144,7 @@ describe('absoluteTimeout middleware', () => { expect(res.clearCookie).not.toHaveBeenCalled(); }); - it('destroys the session, removes from the index, clears the cookie, and still calls next() when past the ceiling', async () => { + it('destroys the session, removes from the index, clears the cookie, audits expiry, and still calls next() when past the ceiling', async () => { const session = makeSession({ user: USER, createdAt: NOW - 13 * 60 * 60 * 1000, @@ -125,7 +152,8 @@ describe('absoluteTimeout middleware', () => { }); const index = makeIndex(); const logger = makeLogger(); - const middleware = createAbsoluteTimeoutMiddleware(index, logger, () => NOW); + const audit = makeAudit(); + const middleware = createAbsoluteTimeoutMiddleware(index, logger, audit, () => NOW); const res = makeRes(); const next: NextFunction = jest.fn(); @@ -134,6 +162,12 @@ describe('absoluteTimeout middleware', () => { expect(session.destroy).toHaveBeenCalledTimes(1); expect(index.remove).toHaveBeenCalledWith(USER.oid, 'session-abc'); expect(res.clearCookie).toHaveBeenCalledWith('portal_session', { path: '/' }); + expect(audit.sessionExpired).toHaveBeenCalledWith({ + actor: { oid: USER.oid }, + sessionId: 'session-abc', + reason: 'absolute', + ageMs: 13 * 60 * 60 * 1000, + }); expect(logger.log).toHaveBeenCalledWith( expect.objectContaining({ event: 'session.absolute_timeout', @@ -145,7 +179,7 @@ describe('absoluteTimeout middleware', () => { expect(next).toHaveBeenCalledTimes(1); }); - it('logs but does not throw when destroy() fails — still clears cookie + index + calls next()', async () => { + it('logs but does not throw when destroy() fails — still clears cookie + index + audits + calls next()', async () => { const session = makeSession({ user: USER, createdAt: NOW - 13 * 60 * 60 * 1000, @@ -156,7 +190,8 @@ describe('absoluteTimeout middleware', () => { ); const index = makeIndex(); const logger = makeLogger(); - const middleware = createAbsoluteTimeoutMiddleware(index, logger, () => NOW); + const audit = makeAudit(); + const middleware = createAbsoluteTimeoutMiddleware(index, logger, audit, () => NOW); const res = makeRes(); const next: NextFunction = jest.fn(); @@ -170,6 +205,7 @@ describe('absoluteTimeout middleware', () => { 'AbsoluteTimeout', ); expect(index.remove).toHaveBeenCalledWith(USER.oid, 'session-abc'); + expect(audit.sessionExpired).toHaveBeenCalled(); expect(res.clearCookie).toHaveBeenCalled(); expect(next).toHaveBeenCalledTimes(1); }); diff --git a/apps/portal-bff/src/session/absolute-timeout.middleware.ts b/apps/portal-bff/src/session/absolute-timeout.middleware.ts index 96bd2a5..78dfb70 100644 --- a/apps/portal-bff/src/session/absolute-timeout.middleware.ts +++ b/apps/portal-bff/src/session/absolute-timeout.middleware.ts @@ -1,5 +1,6 @@ import type { NextFunction, Request, RequestHandler, Response } from 'express'; import { Logger } from 'nestjs-pino'; +import { AuditWriter } from '../audit/audit.service'; import { sessionCookieName } from './session-cookie'; import { UserSessionIndexService } from './user-session-index.service'; @@ -29,6 +30,7 @@ import { UserSessionIndexService } from './user-session-index.service'; export function createAbsoluteTimeoutMiddleware( index: UserSessionIndexService, logger: Logger, + audit: AuditWriter, now: () => number = Date.now, ): RequestHandler { return async function absoluteTimeout( @@ -79,6 +81,18 @@ export function createAbsoluteTimeoutMiddleware( await index.remove(userId, sessionId); res.clearCookie(sessionCookieName(), { path: '/' }); + // Audit the expiry — blocking per ADR-0013: if the audit row + // can't be written, the request fails (the user sees a 5xx, + // ops gets a critical Pino line). The session was already + // destroyed above, so the user is signed out regardless — the + // failure surfaces in the response, not the security posture. + await audit.sessionExpired({ + actor: { oid: userId }, + sessionId, + reason: 'absolute', + ageMs, + }); + logger.log( { event: 'session.absolute_timeout', diff --git a/apps/portal-bff/src/session/session.module.spec.ts b/apps/portal-bff/src/session/session.module.spec.ts index 110d02d..94ccfe9 100644 --- a/apps/portal-bff/src/session/session.module.spec.ts +++ b/apps/portal-bff/src/session/session.module.spec.ts @@ -1,9 +1,23 @@ import { randomBytes } from 'node:crypto'; +import { Global, Module } from '@nestjs/common'; import { Test } from '@nestjs/testing'; +import { ClsService } from 'nestjs-cls'; import { LoggerModule } from 'nestjs-pino'; +import { PrismaService } from 'nestjs-prisma'; +import { AuditModule } from '../audit/audit.module'; import { SessionModule } from './session.module'; import { SESSION_MIDDLEWARE, type RequestHandler } from './session.token'; +@Global() +@Module({ + providers: [ + { provide: PrismaService, useValue: {} }, + { provide: ClsService, useValue: { get: () => undefined } }, + ], + exports: [PrismaService, ClsService], +}) +class TestStubsModule {} + const STRONG_KEY = randomBytes(32).toString('base64url'); const STRONG_SECRET = randomBytes(32).toString('base64url'); @@ -11,12 +25,22 @@ interface OriginalEnv { REDIS_URL: string | undefined; SESSION_SECRET: string | undefined; SESSION_ENCRYPTION_KEY: string | undefined; + LOG_USER_ID_SALT: string | undefined; NODE_ENV: string | undefined; } +// SessionModule's absolute-timeout middleware factory now depends +// on `AuditWriter` (provided by AuditModule via @Global() in +// production). The spec compiles a slice of the graph so we import +// AuditModule explicitly and stub its DB-touching deps. async function compile() { return Test.createTestingModule({ - imports: [LoggerModule.forRoot({ pinoHttp: { level: 'silent' } }), SessionModule], + imports: [ + LoggerModule.forRoot({ pinoHttp: { level: 'silent' } }), + TestStubsModule, + AuditModule, + SessionModule, + ], }).compile(); } @@ -25,6 +49,7 @@ describe('SessionModule', () => { REDIS_URL: process.env['REDIS_URL'], SESSION_SECRET: process.env['SESSION_SECRET'], SESSION_ENCRYPTION_KEY: process.env['SESSION_ENCRYPTION_KEY'], + LOG_USER_ID_SALT: process.env['LOG_USER_ID_SALT'], NODE_ENV: process.env['NODE_ENV'], }; @@ -34,6 +59,7 @@ describe('SessionModule', () => { process.env['REDIS_URL'] = 'redis://default:test-pass@127.0.0.1:65535/0'; process.env['SESSION_SECRET'] = STRONG_SECRET; process.env['SESSION_ENCRYPTION_KEY'] = STRONG_KEY; + process.env['LOG_USER_ID_SALT'] = randomBytes(32).toString('base64url'); process.env['NODE_ENV'] = 'development'; }); @@ -41,6 +67,7 @@ describe('SessionModule', () => { restore('REDIS_URL', original.REDIS_URL); restore('SESSION_SECRET', original.SESSION_SECRET); restore('SESSION_ENCRYPTION_KEY', original.SESSION_ENCRYPTION_KEY); + restore('LOG_USER_ID_SALT', original.LOG_USER_ID_SALT); restore('NODE_ENV', original.NODE_ENV); }); diff --git a/apps/portal-bff/src/session/session.module.ts b/apps/portal-bff/src/session/session.module.ts index 92cce80..2e7343a 100644 --- a/apps/portal-bff/src/session/session.module.ts +++ b/apps/portal-bff/src/session/session.module.ts @@ -7,6 +7,7 @@ import { assertSessionEncryptionKey } from '../config/check-session-encryption-k import { assertSessionSecret } from '../config/check-session-secret'; import { RedisModule } from '../redis/redis.module'; import { REDIS_CLIENT, type Redis } from '../redis/redis.token'; +import { AuditWriter } from '../audit/audit.service'; import { createAbsoluteTimeoutMiddleware } from './absolute-timeout.middleware'; import { adaptIoredisForConnectRedis } from './ioredis-connect-redis-adapter'; import { SessionDecryptError, decrypt, encrypt } from './session-crypto'; @@ -57,9 +58,12 @@ import { UserSessionIndexService } from './user-session-index.service'; UserSessionIndexService, { provide: SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE, - inject: [UserSessionIndexService, Logger], - useFactory: (index: UserSessionIndexService, logger: Logger): RequestHandler => - createAbsoluteTimeoutMiddleware(index, logger), + inject: [UserSessionIndexService, Logger, AuditWriter], + useFactory: ( + index: UserSessionIndexService, + logger: Logger, + audit: AuditWriter, + ): RequestHandler => createAbsoluteTimeoutMiddleware(index, logger, audit), }, { provide: SESSION_MIDDLEWARE,