feat(portal-bff): wire ADR-0013 audit pipeline to the auth lifecycle
CI / commits (pull_request) Successful in 2m7s
CI / scan (pull_request) Successful in 2m20s
CI / check (pull_request) Successful in 2m28s
CI / a11y (pull_request) Successful in 1m31s
CI / perf (pull_request) Successful in 4m52s

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.
This commit is contained in:
Julien Gautier
2026-05-13 14:08:30 +02:00
parent 4d4791a807
commit 60183d51ae
17 changed files with 682 additions and 41 deletions
+12 -4
View File
@@ -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)
+11 -7
View File
@@ -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 {}
@@ -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 });
});
});
});
+76 -1
View File
@@ -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<void> {
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<void> {
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<void> {
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<void> {
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<void> {
const traceId = trace.getActiveSpan()?.spanContext().traceId ?? null;
const actorIdHash =
+41
View File
@@ -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<string, unknown>;
}
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;
}
@@ -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<T>(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;
}
}
});
});
@@ -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);
}
}
@@ -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<typeof makeLoggerStub>;
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 },
@@ -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 {
+43 -11
View File
@@ -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();
}
@@ -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/);
});
});
@@ -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;
}
+6
View File
@@ -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
@@ -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);
});
@@ -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',
@@ -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);
});
@@ -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,