2cdeb74341
## Summary
PR 1 of the tabs + full-result-charts chantier. New BFF endpoint `GET /api/admin/audit/stats` that computes the three chart aggregations server-side over the **full filtered set** (not the paginated slice the SPA currently feeds the charts with).
```
PR 1 (this one) — BFF endpoint + Redis cache + audit event + ADR-0013 amendment.
PR 2 — SPA: Tabs UX (Table / Charts) + replace the per-page computeds
with calls to this endpoint.
```
## What lands
### New route — `GET /api/admin/audit/stats`
```ts
GET /api/admin/audit/stats?eventType=...&audience=...&outcome=...
&subjectPrefix=...&createdAtFrom=...&createdAtTo=...
&actorIdHash=...
→ {
dailyVolume: [{ day: 'YYYY-MM-DD', count }],
outcomeBreakdown: [{ outcome, count }],
eventTypeByDay: [{ day, eventType, count }],
total // sum of dailyVolume.count, drives the donut centre
}
```
Same filter shape as the existing `GET /api/admin/audit` minus pagination — the stats endpoint always aggregates the whole filtered set. `@RequireAdmin` gated (per [ADR-0020](docs/decisions/0020-portal-admin-app.md)). Time bound respects the filters strictly per the chantier brief: no filter → aggregates across the full audit retention (365 days per [ADR-0013](docs/decisions/0013-audit-trail-separated-postgres-append-only.md)). The Redis cache below absorbs repeated heavy queries.
### New service — [`AuditStatsReader`](apps/portal-bff/src/admin/audit-stats.service.ts)
Mirrors `AuditReader`'s posture:
- Every query inside a transaction that opens with `SET LOCAL ROLE audit_reader`. SELECT-only on `audit.events` even if the BFF's connection is otherwise privileged.
- Parameterised SQL only. Filter values flow through positional parameters, never concatenated.
- Three `GROUP BY` queries scoped by the same `WHERE` clause:
- `date_trunc('day', created_at)::date AS day, COUNT(*) GROUP BY day`
- `outcome::text, COUNT(*) GROUP BY outcome`
- `date_trunc('day', created_at)::date AS day, event_type, COUNT(*) GROUP BY day, event_type`
### Redis cache — 5-minute TTL per filter-hash
- Cache key: `audit:stats:<sha256(canonical-JSON of filters), 16 hex chars>`. Sorted-keys canonicalisation so the same filters in different argument orders map to the same key.
- TTL: 300 s. Audit rows are append-only so past aggregations are stable; new events are continuously inserted, so admins see at most 5-minute-stale aggregations — acceptable for "approximate dashboard" usage, not for "did the last event just land" debugging (use the list endpoint for that).
- Cache writes are best-effort — a Redis-write failure does not fail the response. The DB read already happened; the next call rebuilds the cache.
- The cache *write* path is covered by spec; the cache-hit shortcut path is covered too (skips the DB transaction entirely).
### New audit event — `admin.audit.stats.query`
Mirrors `admin.audit.query` in posture (every admin read is auditable per ADR-0020 §"Read actions ... to deter fishing expeditions") with two differences:
- Distinct `event_type` so an auditor can spot "scanned aggregations" vs "paged through rows" — different observation signals (the stats endpoint can sweep millions of rows in one call; the list endpoint is bounded by `MAX_LIMIT=200`).
- Payload carries `total` (size of the aggregated set) instead of `resultCount` — stats responses don't paginate, the value carries more "size of scan" signal.
### Light amendment — [ADR-0013](docs/decisions/0013-audit-trail-separated-postgres-append-only.md)
Two additions:
- New **"Reader endpoints"** subsection that enumerates the two-endpoint reader surface (list + stats), documents the Redis-cache caveat, and points at the new `admin.audit.stats.query` event family.
- The "events emitted in v1" table grows four rows it was previously missing on `main`: `admin.access_denied`, `admin.audit.query`, `admin.audit.stats.query`, `admin.users.query`.
No supersession, no new ADR. The decision shape (server-side aggregation + Redis cache + new audit event family) was settled in chat via `AskUserQuestion` before the implementation started; recording it here keeps the ADR honest without spawning a full ADR-0024 for what's essentially an extension of ADR-0013's reader surface.
## Notes for the reviewer
- **Why not factor `buildWhere` into a shared helper between `AuditReader` and `AuditStatsReader`?** Considered. The two readers' shapes diverge in non-trivial ways: `AuditReader` adds `LIMIT/OFFSET` parameters appended to the same parameter array, `AuditStatsReader` runs three queries that all share the same `WHERE` (no further params). A shared helper would have to either expose both shapes or hand back the raw clauses + params for callers to assemble — at which point the abstraction earns its weight back. Two ~50 LOC copies today, extraction when a third reader lands or when the shape diverges further.
- **Why not cap the time window when no filter is provided?** Honest disclosure beats clever defaults. The list endpoint also returns "everything matching the filters" with no protective cap; the stats endpoint follows the same posture. The Redis cache absorbs the cost when the same heavy query lands repeatedly; an admin running unfiltered queries at high rate will see flat latency after the first call. If we later observe a real perf issue, a `windowDays` parameter is a smaller change than retrofitting one across the API.
- **Why a `text` cast on `outcome` in the SQL?** Prisma's Postgres enum types come back as JS strings already, but the `outcome` column carries a Postgres enum (`audit.AuditOutcome`). The explicit `::text` is defensive — `$queryRawUnsafe`'s typing isn't enum-aware, and the cast keeps the projection unambiguous regardless of the driver's row-shape inference.
- **Why does the date round-trip through `Date.toISOString().slice(0, 10)`?** `date_trunc('day', ...)::date` returns a Postgres `date` that node-postgres surfaces as a JS `Date` at UTC midnight. The default `toJSON` serialises the full ISO timestamp with the timezone offset — which is not what the chart x-axis wants. Slicing to `YYYY-MM-DD` matches the SPA's chart bucket convention exactly.
- **No mention of the `actorIdHash` audit row for the stats endpoint?** It's the same hash flow as `adminAuditQuery` — the `actor.oid` from the session goes through `HashUserIdService` per ADR-0012's salt-based pseudonymisation. The same flow is exercised by the existing `adminAuditQuery` tests; the new `adminAuditStatsQuery` method just routes to `recordEvent` with a different `eventType`.
## Test plan
- [x] `pnpm nx test portal-bff` — **414 specs pass** (was 401; +13 new: 8 `AuditStatsReader` service + 5 controller `stats` endpoint).
- [x] `pnpm nx run portal-bff:lint` — clean.
- [x] `pnpm nx build portal-bff` — clean (webpack).
- [ ] **Manual smoke** — `pnpm nx serve portal-bff`, sign in to portal-admin with `Portal.Admin`:
- `curl http://localhost:3000/api/admin/audit/stats --cookie-jar /tmp/admin` returns the three projections.
- Verify the `admin.audit.stats.query` row in `audit.events` after the call (`SELECT * FROM audit.events WHERE event_type = 'admin.audit.stats.query' ORDER BY occurred_at DESC LIMIT 1`).
- Hit the endpoint twice in quick succession with the same filters → second call shows < 5 ms latency (cache hit, no DB transaction).
- Hit it with different filters → first call hits DB, second cache, third with same filters → cache hit.
- Stop Redis (`./infra/local/dev.sh stop redis`), hit the endpoint → still succeeds (cache miss + write swallowed), comes back live from DB.
## What's next
PR 2 — SPA Tabs UX (Table / Charts) + replace `dailyVolume() / outcomeBreakdown() / dailyByEventType()` (currently computed from `page().items`) with calls to this endpoint. The three computeds become signals filled by the HTTP call; the chart components on the Charts tab consume them unchanged.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #173
291 lines
11 KiB
TypeScript
291 lines
11 KiB
TypeScript
import { Injectable } from '@nestjs/common';
|
|
import { trace } from '@opentelemetry/api';
|
|
import { ClsService } from 'nestjs-cls';
|
|
import { PrismaService } from 'nestjs-prisma';
|
|
import type {
|
|
AdminAccessDeniedInput,
|
|
AdminAuditQueryInput,
|
|
AdminAuditStatsQueryInput,
|
|
AdminUsersQueryInput,
|
|
AuditEventInput,
|
|
MfaRequiredInput,
|
|
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 },
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Typed event: a request was rejected by `RequireMfaGuard` because
|
|
* the session did not satisfy the route's MFA freshness gate.
|
|
* Per ADR-0011 §"Step-up MFA — designed-in, dormant", every
|
|
* step-up challenge emitted by the BFF lands here so auditors can
|
|
* track the distribution of step-up prompts (and the rate of stale
|
|
* `mfaVerifiedAt` rejections that would suggest the default
|
|
* freshness is too tight). `outcome=denied` matches the
|
|
* `AdminRoleGuard` posture: the user *is* authenticated, the
|
|
* action is just refused.
|
|
*/
|
|
async mfaRequired(input: MfaRequiredInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'auth.mfa_required',
|
|
audience: 'workforce',
|
|
outcome: 'denied',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
subject: input.attemptedRoute,
|
|
payload: {
|
|
reason: input.reason,
|
|
freshnessSeconds: input.freshnessSeconds,
|
|
...(input.mfaAgeMs !== undefined ? { mfaAgeMs: input.mfaAgeMs } : {}),
|
|
},
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Typed event: an admin queried the audit-log viewer. Recorded at
|
|
* **coarser granularity** per ADR-0020 §"Audit log captures every
|
|
* admin write action … Read actions (audit log viewing, user list
|
|
* browsing) are also captured … to deter fishing expeditions".
|
|
*
|
|
* The filter object the admin used is serialised into the payload
|
|
* so a reviewer can see what was searched (event type, time range,
|
|
* actor hash, etc.). `resultCount` is the number of rows the SPA
|
|
* received in this trip — paged calls each emit one event; an
|
|
* auditor spotting a sequence of `admin.audit.query` with growing
|
|
* offsets is the v1 detection signal for a sweep.
|
|
*
|
|
* `outcome=success` regardless of whether the query returned any
|
|
* rows: the event captures "the admin ran a query", not "the
|
|
* query matched something". An `outcome=failure` audit row from
|
|
* this surface is reserved for the day we add per-row authZ.
|
|
*/
|
|
async adminAuditQuery(input: AdminAuditQueryInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'admin.audit.query',
|
|
audience: 'workforce',
|
|
outcome: 'success',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
payload: {
|
|
filters: input.filters,
|
|
resultCount: input.resultCount,
|
|
},
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Typed event: an admin queried the audit-log STATISTICS endpoint
|
|
* (`GET /api/admin/audit/stats`). Same deterrent posture as
|
|
* {@link adminAuditQuery} but separates the event type so an
|
|
* auditor can spot "the admin scanned aggregations" vs "the admin
|
|
* paged through rows" — different observation signals.
|
|
*
|
|
* Payload carries `total` (the size of the aggregated dataset)
|
|
* rather than `resultCount` — stats responses don't paginate, and
|
|
* `total` is what tells a reviewer how wide the scan was.
|
|
*/
|
|
async adminAuditStatsQuery(input: AdminAuditStatsQueryInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'admin.audit.stats.query',
|
|
audience: 'workforce',
|
|
outcome: 'success',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
payload: {
|
|
filters: input.filters,
|
|
total: input.total,
|
|
},
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Typed event: an admin queried the user-directory viewer. Same
|
|
* deterrent posture as {@link adminAuditQuery} per ADR-0020
|
|
* §"Read actions ... to deter fishing expeditions" — captures the
|
|
* filter and the result count so a reviewer can spot a sweep
|
|
* (admin paging through everyone) without joining anything.
|
|
*/
|
|
async adminUsersQuery(input: AdminUsersQueryInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'admin.users.query',
|
|
audience: 'workforce',
|
|
outcome: 'success',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
payload: {
|
|
filters: input.filters,
|
|
resultCount: input.resultCount,
|
|
},
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Typed event: `/api/admin/*` request rejected by `AdminRoleGuard`
|
|
* because the session's `roles` claim does not include `Portal.Admin`.
|
|
* Per ADR-0020 §"Auth — same Entra ID … `Portal.Admin` role claim", every
|
|
* 403 from the admin surface is captured here with the attempted
|
|
* route and the roles the user actually held — auditors looking
|
|
* for privilege-escalation attempts pivot on `subject` (the route)
|
|
* and `outcome=denied`.
|
|
*/
|
|
async adminAccessDenied(input: AdminAccessDeniedInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'admin.access_denied',
|
|
audience: 'workforce',
|
|
outcome: 'denied',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
subject: input.attemptedRoute,
|
|
payload: { rolesHeld: input.rolesHeld },
|
|
});
|
|
}
|
|
|
|
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,
|
|
);
|
|
});
|
|
}
|
|
}
|