a97be121e6
## Summary #120 shipped the audit pipeline but the end-to-end path was never smoke-tested against a running Postgres. First click on `/auth/logout` returned 500 with the Pino log: ``` PostgresError code 42501 — permission denied for table events ``` Despite: - ACL on `audit.events` showing `audit_writer=a/audit_owner` (INSERT granted). - `has_table_privilege('audit_writer', 'audit.events', 'INSERT')` returning `t`. - `has_schema_privilege` / `has_type_privilege` all `t`. - A direct psql `INSERT INTO audit.events ...` after `SET LOCAL ROLE audit_writer` **succeeding**. - A psql `INSERT ... RETURNING id` after the same `SET LOCAL ROLE` **failing** with the exact same error. Root cause: Prisma's ORM `tx.auditEvent.create(...)` issues `INSERT ... RETURNING *` to hydrate the returned entity. Postgres requires **SELECT** on every column listed in `RETURNING`. `audit_writer` has INSERT only by ADR-0013 design — RETURNING fails with `code 42501` and the error message reads "permission denied for table events" (no mention of SELECT or RETURNING, which is what made it deeply non-obvious to diagnose). ## Fix `AuditWriter.recordEvent` now issues a parameterised raw INSERT via `tx.$executeRawUnsafe` instead of the ORM `create()`: ```ts await tx.$executeRawUnsafe( `INSERT INTO "audit"."events" (id, event_type, audience, outcome, subject, actor_id_hash, trace_id, payload) VALUES (gen_random_uuid(), $1, $2::"audit"."AuditAudience", $3::"audit"."AuditOutcome", $4, $5, $6, $7::jsonb)`, input.eventType, input.audience, input.outcome, input.subject ?? null, actorIdHash, traceId, payloadJson, ); ``` The role contract per ADR-0013 stays strict: `audit_writer` keeps INSERT only, no SELECT/UPDATE/DELETE/TRUNCATE. The other natural fix (`GRANT SELECT` to `audit_writer`) would have weakened the writer/reader role separation, so we deliberately went the other way. ## Notable choices **`gen_random_uuid()` server-side instead of Prisma's `@default(uuid())` client-side.** The model still declares `@default(uuid())` for any future ORM read or `audit_reader`-side query, but the write path uses the built-in Postgres function. No extension required (Postgres 13+). **Explicit enum and jsonb casts.** Parameters travel as TEXT over the wire; the SQL casts (`$2::"audit"."AuditAudience"`, `$7::jsonb`) ensure Postgres parses them as the right type. Without the casts, the type system rejects the INSERT before privilege check even fires. **Parameterised, not interpolated.** `$executeRawUnsafe` accepts a SQL template with `$1, $2, …` placeholders and a vararg of values — same wire-level parameter binding as a prepared statement, so SQL injection isn't possible even on caller-controlled inputs like `eventType`. The spec pins this with a malicious-input test. **Also fixes an env-sensitivity bug in `auth.controller.spec.ts`.** The test that asserts `session.absoluteExpiresAt == createdAt + 43200000` was reading the default via `readSessionTimeouts()` but didn't override `SESSION_ABSOLUTE_TIMEOUT_SECONDS`. If `apps/portal-bff/.env` has a custom value (as it did during the manual audit debugging), the test failed non-deterministically. Now the test deletes the env var before running and restores it after — same pattern as the other env-touching tests in this file. ## ADR amendment [ADR-0013](docs/decisions/0013-audit-trail-separated-postgres-append-only.md) §"Writer" now carries an **Implementation trap** callout explaining why Prisma's ORM `create()` cannot be used for audit writes (RETURNING requires SELECT, audit_writer has INSERT only). The corresponding Confirmation entry cross-references the callout. Two-commit shape on this PR (code + docs) — the squash-merge will fold them. ## Test plan - [x] `pnpm nx test portal-bff` (clean env) → **144/144 pass**. - [x] `pnpm nx lint portal-bff` → clean. - [x] `pnpm nx build portal-bff` → clean. - [x] Prettier-clean. - [ ] Manual smoke against running BFF + Postgres: - [ ] Sign in → row in `audit.events` with `event_type = 'auth.sign_in'`. - [ ] Sign out → row with `event_type = 'auth.sign_out'`. **The 500 from before is gone.** - [ ] Verify the role contract is still strict : ```sql SET ROLE audit_writer; SELECT * FROM audit.events LIMIT 1; -- should fail "permission denied" UPDATE audit.events SET event_type = 'x'; -- should fail DELETE FROM audit.events; -- should fail ``` --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #121
165 lines
6.1 KiB
TypeScript
165 lines
6.1 KiB
TypeScript
import { Injectable } from '@nestjs/common';
|
|
import { trace } from '@opentelemetry/api';
|
|
import { ClsService } from 'nestjs-cls';
|
|
import { PrismaService } from 'nestjs-prisma';
|
|
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.
|
|
*
|
|
* Contract
|
|
* --------
|
|
* - **Append-only at the database level.** Every write runs inside a
|
|
* transaction whose first statement is `SET LOCAL ROLE
|
|
* audit_writer`. That role only has `INSERT` on `audit.events`
|
|
* (per the migration that created the table); `UPDATE`, `DELETE`,
|
|
* `TRUNCATE` all fail at the Postgres level even if the BFF
|
|
* connection is otherwise privileged. The role is reset
|
|
* automatically at transaction end.
|
|
*
|
|
* - **Fail loud, never swallow.** Per ADR-0013 §"Blocking writes":
|
|
* no audit ⇒ no action. Callers must propagate the rejection up
|
|
* so the requested action does not proceed when its audit trail
|
|
* cannot be written. The service throws the underlying Prisma
|
|
* error unchanged; do not wrap it in a catch-and-log block.
|
|
*
|
|
* - **trace_id and actor_id_hash are auto-resolved.** trace_id is
|
|
* read from the active OTel span context (so the audit row joins
|
|
* with the BFF request span and the Pino log lines on the same
|
|
* request). actor_id_hash is read from the CLS context populated
|
|
* by future auth guards (ADR-0009 / ADR-0010); v1 stores `null`
|
|
* when no actor is established. Callers can override either by
|
|
* passing them on `AuditEventInput`.
|
|
*/
|
|
@Injectable()
|
|
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 =
|
|
input.actorIdHash ?? this.cls.get<string | undefined>('actorIdHash') ?? null;
|
|
const payloadJson = input.payload === undefined ? null : JSON.stringify(input.payload);
|
|
|
|
await this.prisma.$transaction(async (tx) => {
|
|
// Lock the connection to audit_writer for the duration of this
|
|
// transaction. SET LOCAL is reset at COMMIT/ROLLBACK so the
|
|
// pool's next consumer sees the original role.
|
|
await tx.$executeRawUnsafe(`SET LOCAL ROLE audit_writer`);
|
|
|
|
// Deliberately NOT `tx.auditEvent.create(...)`. The Prisma ORM
|
|
// create() emits `INSERT … RETURNING *` to hydrate the entity
|
|
// it returns, and Postgres requires SELECT on every column
|
|
// listed in RETURNING. `audit_writer` is granted INSERT only
|
|
// (ADR-0013 §"Append-only by role grants"); RETURNING fails
|
|
// with the deeply misleading "permission denied for table
|
|
// events" error code 42501. Raw parameterised INSERT keeps
|
|
// the role contract strict — audit_writer never needs SELECT.
|
|
//
|
|
// `gen_random_uuid()` is built into Postgres 13+ (the dev /
|
|
// prod target is 17). The enum + jsonb casts are needed
|
|
// because the parameter values are sent as TEXT over the
|
|
// wire.
|
|
await tx.$executeRawUnsafe(
|
|
`INSERT INTO "audit"."events"
|
|
(id, event_type, audience, outcome, subject, actor_id_hash, trace_id, payload)
|
|
VALUES (
|
|
gen_random_uuid(),
|
|
$1,
|
|
$2::"audit"."AuditAudience",
|
|
$3::"audit"."AuditOutcome",
|
|
$4,
|
|
$5,
|
|
$6,
|
|
$7::jsonb
|
|
)`,
|
|
input.eventType,
|
|
input.audience,
|
|
input.outcome,
|
|
input.subject ?? null,
|
|
actorIdHash,
|
|
traceId,
|
|
payloadJson,
|
|
);
|
|
});
|
|
}
|
|
}
|