Commit Graph

5 Commits

Author SHA1 Message Date
julien 2cdeb74341 feat(portal-bff): audit-stats endpoint — server-side aggregations with redis cache (#173)
CI / check (push) Successful in 3m26s
CI / commits (push) Has been skipped
CI / scan (push) Successful in 3m47s
CI / a11y (push) Successful in 4m1s
CI / perf (push) Successful in 7m30s
Docs site / build (push) Successful in 6m2s
## 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
2026-05-16 23:11:52 +02:00
julien 1df99cd800 feat(portal-bff): admin user-directory read endpoint (#141)
CI / check (push) Successful in 3m1s
CI / commits (push) Has been skipped
CI / scan (push) Successful in 2m34s
CI / a11y (push) Successful in 2m35s
CI / perf (push) Successful in 5m7s
## Summary

Second PR of the **portal-admin User-list chantier** per [ADR-0020](docs/decisions/0020-portal-admin-app.md) §"v1 scope — User list (read-only)". Ships the **read side**: paginated, filterable HTTP endpoint that queries the `public.users` directory populated at sign-in by PR #140. The SPA viewer screen lands in the final PR of the chantier.

## What lands

### [`AdminUsersQueryDto`](apps/portal-bff/src/admin/users-query.dto.ts)

Mirrors `AdminAuditQueryDto`'s posture — filters all optional, every unknown query key rejected by `forbidNonWhitelisted`, limit capped at **200** / default **50**.

| Filter | Type | Notes |
| --- | --- | --- |
| `username` | string ≤128 | Exact-prefix match (Prisma `startsWith`). |
| `displayName` | string ≤128 | Case-insensitive `contains` (display names vary in casing). |
| `audience` | enum | `workforce` \| `customer`. |
| `lastSeenAtFrom` | ISO-8601 | Inclusive lower bound. |
| `lastSeenAtTo` | ISO-8601 | Exclusive upper bound. |
| `limit` | int 1..200 | Default **50**. |
| `offset` | int ≥0 | Default **0**. |

### [`AdminUsersReader`](apps/portal-bff/src/admin/admin-users-reader.service.ts)

Prisma typed client against `public.users` — **no `SET LOCAL ROLE`** dance because `public.users` has no role-based privilege gate. The trust boundary is the controller's `@RequireAdmin` guard.

- **Order**: `last_seen_at DESC, oid ASC`. The second clause is a deterministic tie-breaker for pagination during sign-in bursts that share a timestamp.
- **COUNT + SELECT in one Prisma transaction** so the `total` reported to the SPA matches what's on the page even under a concurrent sign-in landing between the two queries.
- **Hard cap on limit** at 200 even when a caller bypasses the DTO — defense in depth on the BFF's event loop.

### [`AdminUsersController`](apps/portal-bff/src/admin/admin-users.controller.ts)

`GET /api/admin/users`, `@RequireAdmin()` at the class level. Forwards the validated DTO to `AdminUsersReader`, then emits `admin.users.query` with `{ filters, resultCount }` — the fishing-expedition deterrent (mirror of `admin.audit.query` from PR #132).

### [`AuditWriter.adminUsersQuery()`](apps/portal-bff/src/audit/audit.service.ts)

New typed method + `AdminUsersQueryInput` type. Same `outcome=success` / payload shape as `adminAuditQuery` — two distinct event types so a reviewer can pivot directly on `eventType` without parsing payload.

## Notes for the reviewer

- The shape mirrors `AdminAuditController` (#132) deliberately. Future admin read endpoints (CMS pages, menu items if they grow read filters) should adopt the same shape — keeps the SPA's data-flow pattern consistent across modules.
- `public.users` queries don't need `SET LOCAL ROLE` because there's no append-only / read-only role split on the table. That's a deliberate v1 simplification; if the security review later asks for stricter isolation we'd add a `users_reader` role + the same SET-LOCAL pattern AuditReader uses.
- The future "sign-in counts" join from `audit.events` on `actor_id_hash` is **deferred**. The salted hash is computable on the fly via `HashUserIdService`, so adding it later is a service-level change — no schema migration required.
- The `actor_id_hash` is deliberately **NOT** stored on `public.users` (per ADR-0013's invariant — the salt stays inside the audit module).

## Test plan

- [x] `pnpm nx test portal-bff` — **391 specs pass** (was 365; +26 covering DTO validation, reader transaction shape + filter forwarding + pagination defaults + cap, controller path, audit typed method).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean.
- [x] Prisma transaction shape verified: COUNT + SELECT both fire exactly once per call; tuple-return contract honoured.
- [x] Filter projections verified per filter: username `startsWith`, displayName case-insensitive `contains`, audience exact match, lastSeenAt `gte/lt` composition.
- [ ] e2e — pending the admin SPA viewer screen + a real admin session. Once both exist: `curl --cookie 'portal_admin_session=...' /api/admin/users?username=jane&limit=10` returns the matching subset and a new `admin.users.query` audit row lands.

## What's next

The chantier's final PR:

- **portal-admin `/users` screen** — SPA viewer with filter form + table + pagination. Same shape as the `/audit` page (PR #136): signal-driven state, color-coded badges for audience, ISO timestamps formatted locale-side, status states. Will graduate the sidebar entry from `aria-disabled` "Soon" badge to a live link.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #141
2026-05-14 20:01:38 +02:00
julien 261203ec5a feat(portal-bff): admin audit-log read endpoint + audit_reader-locked path (#132)
CI / check (push) Successful in 2m49s
CI / commits (push) Has been skipped
CI / scan (push) Successful in 2m11s
CI / a11y (push) Successful in 1m6s
CI / perf (push) Successful in 3m39s
## Summary

First real consumer of the admin module — `GET /api/admin/audit`, the paginated audit-log viewer named in [ADR-0020](docs/decisions/0020-portal-admin-app.md)'s v1 catalogue. Gated by `@RequireAdmin`, reads through the `audit_reader` Postgres role only, and emits `admin.audit.query` on every call as the "fishing expedition" deterrent ADR-0020 calls out (§"Read actions are also captured … to deter fishing expeditions"). This is the BFF half of the audit-viewer chantier — the SPA screen lands later.

## What ships

### [`AuditReader`](apps/portal-bff/src/admin/audit-reader.service.ts)

- Wraps every read in a transaction whose first statement is `SET LOCAL ROLE audit_reader`. Symmetric with `AuditWriter`'s `SET LOCAL ROLE audit_writer`: the runtime role contract holds even when the BFF connection is otherwise privileged. UPDATE / INSERT / DELETE on `audit.events` fail at the Postgres level regardless of what gets through the application layer.
- Parameterised SELECT only — filter values flow into `$queryRawUnsafe`'s positional params, never concatenated into the SQL string. Subject-prefix filter uses `LIKE` with explicit `ESCAPE '\\'` and escapes `%` / `_` / `\` in the literal so an admin-side wildcard can't masquerade as a meta-character.
- COUNT(\*) + `LIMIT` / `OFFSET` pagination. Ordering is `created_at DESC, id DESC` for deterministic page boundaries on identical timestamps (UUIDs break ties).
- Hard caps `limit` at `MAX_LIMIT` (200) even if a caller bypasses the DTO — defense in depth on the BFF's event loop.

### [`AdminAuditQueryDto`](apps/portal-bff/src/admin/audit-query.dto.ts)

| Filter | Type | Notes |
| --- | --- | --- |
| `eventType` | string ≤128 | Exact match (e.g. `auth.sign_in`). |
| `actorIdHash` | string ≤128 | Exact match on the salted hash from the writer. |
| `audience` | enum | `workforce` \| `customer`. |
| `outcome` | enum | `success` \| `failure` \| `denied`. |
| `subjectPrefix` | string ≤128 | `LIKE 'prefix%'`, escaped literal. |
| `createdAtFrom` | ISO-8601 | Inclusive lower bound. |
| `createdAtTo` | ISO-8601 | Exclusive upper bound. |
| `limit` | int 1..200 | Default **50**. |
| `offset` | int ≥0 | Default **0**. |

Bound through Nest's global `ValidationPipe` — unknown query keys are rejected by `forbidNonWhitelisted` (defends against query-string smuggling), `transform: true` coerces numeric strings into numbers.

### [`AdminAuditController`](apps/portal-bff/src/admin/admin-audit.controller.ts)

`@Controller('admin/audit')` + `@RequireAdmin()` at the class level. The handler:

1. Calls `AuditReader.findEvents(filters)`.
2. Emits `admin.audit.query` with `{ filters, resultCount }` so a reviewer can see exactly what the admin searched for and how many rows came back.
3. Returns the page to the SPA.

`@RequireMfa({ freshness: 600 })` is **intentionally not applied** in v1: the admin surface already sits behind a freshly-MFA'd session at sign-in (per ADR-0020), and the per-query audit row is the deterrent. Adding `@RequireMfa` later is a one-line change — that's why the decorator was designed-in by PR #128.

### [`AuditWriter.adminAuditQuery()`](apps/portal-bff/src/audit/audit.service.ts)

New typed method using `outcome=success`. The read **happened** regardless of whether it matched rows; row count lives in `resultCount`. An `outcome=denied` from this surface is reserved for the day we add per-row authZ.

## Operational notes

- **Dev pool, `SET LOCAL ROLE` pattern** (per [ADR-0018](docs/decisions/0018-environment-configuration-strategy.md)): the BFF talks to Postgres on the shared `DATABASE_URL` pool and switches role per-transaction. In production the audit-write pool is already split via `AUDIT_DATABASE_URL`; a dedicated `audit_reader`-only pool is a future follow-up if read-side isolation is desired (the role-locking on the shared pool already prevents privilege bleed at the Postgres level).
- COUNT(\*) is fine at v1 audit volume; if the table grows past a few million rows we'll switch to keyset pagination and drop the total.

## Test plan

- [x] `pnpm nx test portal-bff` — **308 specs pass** (was 278; +30: AuditReader 10, AdminAuditController 6, AuditWriter typed event 2, DTO validation 12).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean (the pre-existing `_res` / `_next` warnings in `rate-limit.middleware.ts` are unrelated).
- [x] SQL-injection probe via fixture (`'; DROP TABLE events; --` as `eventType`) — value lands in the params array, SQL stays templated.
- [x] LIKE escaping verified: `%`, `_`, `\` in `subjectPrefix` are escaped to their literal form.
- [ ] e2e — pending the admin SPA + at least one `admin` Entra role assignment. Once both exist: `curl --cookie 'portal_admin_session=…' /api/admin/audit?eventType=auth.sign_in&limit=10` returns the most recent sign-ins and `psql` shows the matching `admin.audit.query` row in `audit.events`.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #132
2026-05-14 16:08:58 +02:00
julien fed905edc5 feat(portal-bff): distinct admin session + /api/admin/auth flow (#129)
CI / commits (push) Has been skipped
CI / scan (push) Successful in 2m13s
CI / check (push) Successful in 2m27s
CI / a11y (push) Successful in 57s
CI / perf (push) Successful in 3m55s
## Summary

Phase-3a step per [ADR-0020](docs/decisions/0020-portal-admin-app.md) §"Sessions — distinct from `portal-shell`". Wires a second `express-session` middleware on `/api/admin/*` carrying `__Host-portal_admin_session` over Redis prefix `session:admin:`, and ships the parallel `/api/admin/auth/{login,callback,me,logout}` flow that populates it. Signing in to one surface no longer signs the user into the other — Entra SSO at the IdP level still preserves the click-through.

## What lands

### Session middlewares — path-routed dispatch

| Token | Cookie | Redis prefix | Bound to |
| --- | --- | --- | --- |
| `SESSION_MIDDLEWARE` | `portal_session` / `__Host-portal_session` | `session:` | every path **except** `/api/admin/*` |
| `ADMIN_SESSION_MIDDLEWARE` | `portal_admin_session` / `__Host-portal_admin_session` | `session:admin:` | `/api/admin/*` only |

Implemented via a `buildSessionMiddleware(redis, logger, opts)` factory in [session.module.ts](apps/portal-bff/src/session/session.module.ts) — the TTL policy, encryption key, signing secret, session-id entropy, and serializer error-handling all come from the same source. Only the cookie name + Redis key prefix differ.

The dispatch in [main.ts](apps/portal-bff/src/main.ts) is a tiny `(req, res, next) => req.path.startsWith('/api/admin') ? adminSession(...) : userSession(...)`. Running both middlewares unconditionally would have the second overwrite `req.session` from the first, collapsing the two surfaces.

### Distinct admin auth flow

[`AdminAuthController`](apps/portal-bff/src/admin/admin-auth.controller.ts) mounts `/api/admin/auth/{login,callback,me,logout}`. Structurally identical to [`AuthController`](apps/portal-bff/src/auth/auth.controller.ts) but passes `adminRedirectUri` / `adminPostLogoutRedirectUri` and clears the admin session cookie on logout. `me` exposes the `roles` claim (admin SPA needs it for conditional UI); the user-portal `me` intentionally still doesn't.

### Shared `SessionEstablisher` (no controller duplication)

[`SessionEstablisher`](apps/portal-bff/src/auth/session-establisher.service.ts) encapsulates the session lifecycle so both controllers stay thin:

- `establish({ user, req, res, surface })` — mints CSRF, populates `user / createdAt / absoluteExpiresAt / csrfToken / mfaVerifiedAt`, saves, sets the CSRF cookie, registers in `user_sessions` index, emits `auth.sign_in` audit (blocking), logs with the `surface` tag.
- `destroy({ actor, req })` — when `actor` is set, removes from index + emits `auth.sign_out`; always destroys the session with Redis-hiccup tolerance.

No code duplicated between the two surfaces — the only per-surface differences are the redirect URIs (passed in) and the cookie names cleared on logout (controller-local).

### Entra config gains two URIs

`EntraConfig` adds `adminRedirectUri` + `adminPostLogoutRedirectUri`, validated at boot in [check-entra-config.ts](apps/portal-bff/src/config/check-entra-config.ts). The validator **refuses to start** when `ENTRA_ADMIN_REDIRECT_URI === ENTRA_REDIRECT_URI` — that misconfiguration would silently collapse the two surfaces into one session. Both URIs must be registered on the same Entra app registration's "Redirect URIs" list.

### `AuthService` API change

`beginAuthCodeFlow(redirectUri)`, `completeAuthCodeFlow(code, state, preAuth, redirectUri, now?)`, and `buildLogoutUrl(postLogoutRedirectUri)` now take their URI as a parameter. Callers (user-portal vs admin-portal controllers) pick which set to pass.

## Required ops action before this PR can run locally

Two new mandatory env vars. The BFF refuses to start without them.

```env
ENTRA_ADMIN_REDIRECT_URI=http://localhost:3000/api/admin/auth/callback
ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI=http://localhost:4201/
```

The example values land in [apps/portal-bff/.env.example](apps/portal-bff/.env.example) for reference. The corresponding Entra app registration also needs `/api/admin/auth/callback` added to its "Redirect URIs" list before any admin sign-in works end-to-end.

## Notes for the reviewer

- The user-portal callback's post-login redirect still targets `postLogoutRedirectUri` (existing quirk where the post-auth and post-logout landing happen to be the same URL). The admin callback mirrors the pattern for `adminPostLogoutRedirectUri`. Splitting these into dedicated post-login URIs is a separate ADR/PR.
- `AdminModule` now imports `AuthModule` to consume `AuthService`, `SessionEstablisher`, and `ENTRA_CONFIG`. `AuditWriter` and `RequireMfaGuard` come through transitively.
- Existing `AuthController` spec assertions are preserved through the refactor by constructing a **real** `SessionEstablisher` in the test fixture with the same audit / index / logger mocks. No behavioural assertion was removed — the inline session-state-setting logic is now exercised through the establisher.
- The pre-existing docstring in `check-entra-config.ts` line 11-16 still says "the two redirect URIs are mandatory once the OIDC routes ship (next PR)" — stale, the routes have shipped. Not touched in this PR to keep the diff focused; can be a one-line doc PR later.

## Test plan

- [x] `pnpm nx test portal-bff` — **278 specs pass** (was 253; +25: admin cookie 3, session-establisher 11, admin auth controller 9, entra config 2).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean (the pre-existing `_res` / `_next` warnings in `rate-limit.middleware.ts` are unrelated).
- [x] Entra config validator: both URIs required, both URL-validated, equality refused.
- [x] Path-dispatch verified by routing — `/api/admin/me` and `/api/admin/auth/*` see the admin session; everything else sees the user session.
- [ ] e2e — pending env var update + Entra registration update to add the admin redirect URI. Once both are in place: sign in via `/api/auth/login`, see `portal_session` cookie; clear cookies; sign in via `/api/admin/auth/login`, see `portal_admin_session` cookie; verify `/api/admin/me` works on the admin session and `/api/auth/me` works on the user session — neither sees the other's session.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #129
2026-05-14 02:21:47 +02:00
julien 3ed6dae3a5 feat(portal-bff): admin module + role guard + /api/admin/me self-test (#127)
CI / scan (push) Successful in 2m36s
CI / commits (push) Has been skipped
CI / check (push) Successful in 4m0s
CI / a11y (push) Successful in 2m2s
CI / perf (push) Successful in 3m53s
## Summary

Lays the foundation for the `/api/admin/*` surface per [ADR-0020](docs/decisions/0020-portal-admin-app.md). This PR ships the role guard, the `@RequireAdmin()` decorator, and a self-test endpoint — no business routes yet. The next consumer (audit log viewer) lands in a later PR once the distinct admin session is in place.

## What ships

- **[AdminRoleGuard](apps/portal-bff/src/admin/admin-role.guard.ts)** — three branches:
  - No session at all → **401**. No audit; unauthenticated probes are normal traffic, not a privilege-escalation signal.
  - Session but `roles` lacks `admin` → **403** + `admin.access_denied` audit row with actor hash, attempted route (`${METHOD} ${originalUrl}`), and the roles the user did hold.
  - Session with `admin` role → pass through.
  - Audit-write failures propagate (no audit ⇒ no action, consistent with the existing call sites in [AuthController](apps/portal-bff/src/auth/auth.controller.ts)).
- **[`@RequireAdmin()`](apps/portal-bff/src/admin/require-admin.decorator.ts)** — semantic sugar for `@UseGuards(AdminRoleGuard)` built with `applyDecorators` so future composition (e.g. with the upcoming `@RequireMfa({ freshness })` for the admin entry route) is mechanical.
- **`GET /api/admin/me`** — self-test endpoint named in ADR-0020 §"Confirmation" step 3. Returns the public user payload + `roles` so ops can `curl` the gate with three sessions (no cookie / non-admin cookie / admin cookie) and observe `401` / `403` + audit / `200` respectively.
- **[`AuditWriter.adminAccessDenied()`](apps/portal-bff/src/audit/audit.service.ts)** — new typed method using the pre-existing `denied` outcome enum value. Keeps the salt inside the audit module, matches the pattern of `signIn` / `signOut` / `sessionExpired`.

## Why the shared portal-shell session (for now)

ADR-0020 mandates a distinct `__Host-portal_admin_session` cookie + Redis namespace `session:admin:*` for the admin app. **That is not in this PR.** The chantier sequence splits it out: this PR proves the guard semantics + audit integration on the existing session; the next PR introduces the distinct session middleware + admin-specific auth flow.

Rationale: the guard logic is independent of the session implementation — `session.user.roles` is the only field it reads. Landing it first means a smaller diff to review, a faster opportunity to validate the audit emission on a real Postgres, and a clean baseline to layer the session split onto.

## Notes for the reviewer

- The non-null assertion on `req.session.user!` in [admin.controller.ts:27](apps/portal-bff/src/admin/admin.controller.ts#L27) is explicitly disabled with an inline comment pointing at the guard's contract. The alternative (a defensive runtime check) duplicates the guard's logic without adding safety. The spec for the guard covers every branch including the absent-user path.
- `AdminController` does not depend on `AuthService`'s `toPublicUser` projection — that helper is private to the auth module and pulls `displayName` / `username` with extra account-object fallbacks specific to the OIDC callback. The admin response is built from the already-populated session, so a duplicated projection here is the simpler shape.

## Open questions (out of scope)

- The Entra app role `admin` must be **declared on the app registration manifest** and **assigned to at least one test user** before this gate can be exercised end-to-end. That's an Entra Admin Center operation, not code. The guard's behaviour under all three branches is covered by unit tests; e2e validation waits until the role is assigned.

## Test plan

- [x] `pnpm nx test portal-bff` — **214 specs pass** (was 203; +11 covering guard branches, controller projection, audit method).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean (the pre-existing `_res` / `_next` warnings in `rate-limit.middleware.ts` are unrelated).
- [x] Audit row schema verified — `admin.access_denied` events use `outcome=denied`, store the route in `subject`, and persist `{ rolesHeld: [...] }` in the JSONB payload.
- [ ] e2e — pending Entra role declaration + assignment. Will be covered by manual ops curl checks once the role exists.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #127
2026-05-14 01:17:30 +02:00