10 Commits

Author	SHA1	Message	Date
julien	b84b58068a	refactor(users): rename Prisma User model to UserDirectoryEntry (#230) ... ## Summary **Mechanical refactor only**, prerequisite to ADR-0026 PR 1. Renames the existing Prisma `User` model (the ADR-0020 user-directory cache, `oid` PK, written by `UserDirectoryService.recordSignIn`) to `UserDirectoryEntry`. Frees the `User` name for the upcoming ADR-0026 model (UUID PK, FK to `Person`, `lastSignInAt` — different semantics). Zero behavioural change. Same columns, same indexes, same constraints, same call sites — just a different identifier on the model and the table. ## What lands | File | Change | | --- | --- | | `apps/portal-bff/prisma/schema.prisma` | `model User` → `model UserDirectoryEntry`. `@@map("users")` → `@@map("user_directory_entries")`. Comment block extended to flag the upcoming distinction from ADR-0026's new `User`. | | `apps/portal-bff/prisma/migrations/20260526200000_rename_users_to_user_directory_entries/migration.sql` | **New**. `ALTER TABLE "users" RENAME TO "user_directory_entries"` + `ALTER INDEX` renames for the PK constraint and the two named indexes (Postgres doesn't auto-rename these on table rename). | | `apps/portal-bff/src/users/user-directory.service.ts` | Two renames: the **TS input interface** `UserDirectoryEntry` → `RecordSignInInput` (the existing name was a misnomer — it's the input to `recordSignIn`, not the entry itself, and would collide with the Prisma-generated `UserDirectoryEntry` type after the model rename). The **Prisma client ref** `this.prisma.user.upsert` → `this.prisma.userDirectoryEntry.upsert`. | | `apps/portal-bff/src/users/user-directory.service.spec.ts` | Imports / mock setup / fixture type updated to track the two renames. | | `apps/portal-bff/src/admin/admin-users-reader.service.ts` | `this.prisma.user.{count,findMany}` → `this.prisma.userDirectoryEntry.{count,findMany}`. `Prisma.UserWhereInput` → `Prisma.UserDirectoryEntryWhereInput`. Doc comments mentioning `public.users` updated to `public.user_directory_entries`. The class name `AdminUsersReader`, the endpoint URL `/api/admin/users`, the DTO `AdminUserDto`, and the local `interface UserRow` are unchanged — these are SPA/HTTP-facing identifiers, where the URL semantics ("admin user list") still hold regardless of the backing table name. | | `apps/portal-bff/src/admin/admin-users-reader.service.spec.ts` | Mock setup updated to track the Prisma client field rename. | ## Why two renames in one file `apps/portal-bff/src/users/user-directory.service.ts` had a TypeScript `interface UserDirectoryEntry` carrying the **input shape** of `recordSignIn(entry: UserDirectoryEntry)`. After the Prisma model rename to `UserDirectoryEntry`, that name would clash with the Prisma-generated row type. The fix is to rename the TS interface to its proper role — `RecordSignInInput` — at the same time. Net effect: clearer naming on both sides (the call-input name now describes the call, the persisted-row type name now describes the row). ## Recovery procedure (for anyone with the old migration applied locally) The new migration `20260526200000_rename_users_to_user_directory_entries` is a pure `ALTER TABLE ... RENAME` + index renames — Prisma's `migrate dev` runs it forward without prompting: ```bash cd apps/portal-bff && pnpm exec prisma migrate dev # Should report: "Applied migration `20260526200000_rename_users_to_user_directory_entries`" ``` No `down -v` needed — existing data carries over. ## Test plan - [x] `node scripts/check-catalogue-drift.mjs` — clean (4 / 24 / 7). - [x] `node --test scripts/check-catalogue-drift.spec.mjs` — 22 tests passing. - [x] `pnpm exec prettier --check` clean on the touched files. - [x] Sweep grep — zero leftover `model User`, `prisma.user.`, `Prisma.UserWhereInput`, `interface UserDirectoryEntry`, or `public.users` references anywhere under `apps/portal-bff/src/` or `apps/portal-bff/prisma/`. - [ ] **Locally**: `pnpm exec prisma migrate dev` applies the rename migration cleanly; `pnpm exec nx test portal-bff` runs the updated specs green. - [ ] **Review focus** — the two-rename rationale in `user-directory.service.ts`, the migration's `ALTER INDEX` clauses (don't forget those — `ALTER TABLE ... RENAME` does NOT cascade to index names in Postgres), the unchanged class/URL/DTO/local-interface identifiers in `admin-users-reader.service.ts`. ## Why ship as a separate PR ADR-0026 PR 1's nominal scope is `Person` + `User` + `UserScope` + provisioner + drift gate + PrincipalBuilder — already ~15+ files touched. Folding the rename into that PR would mix mechanical refactor with new design. Splitting keeps both PRs reviewable for what they actually do. ## What's next (post-merge) **ADR-0026 PR 1** — `Person` + new `User` + `UserScope` schema + `PersonAndUserProvisioner` wired into `SessionEstablisher` + `Person.source` catalogue + drift-gate extension + `PrincipalBuilder` populating `Principal.user.{id, personId}` from real rows. Now unblocked. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #230	2026-05-26 12:42:23 +02:00
julien	7d91da691b	feat(auth): @RequirePrivilege/@RequireRole/@RequireScope guards (ADR-0025) (#208) ... ## Summary Phase 2 of [ADR-0025](docs/decisions/0025-authorization-model-privileges-roles-scopes.md)'s implementation phasing (§"More Information"). The three new route decorators land — `@RequirePrivilege`, `@RequireRole`, `@RequireScope` — alongside the migration of the legacy `@RequireAdmin` to read from `principal.privileges`. Each guard consumes the session-resident `Principal` built by [#206](#206), evaluates its requirement, and either passes or emits a 403 + audit row. The drift-CI gate (phase 3) is **not** in this PR — it lands next, once the catalogue is being referenced from real route handlers. ## What lands ### Shared lib (`libs/shared/auth`) | File | Role | | --- | --- | | `src/lib/principal-matchers.ts` | Pure functions: `principalHasAnyPrivilege` / `principalHasAnyRole` / `principalCoversResource`. The matchers live in the shared lib so the SPA can render UI predicates with the same logic the BFF enforces server-side. | | Same file | `ScopableResource` type — optional FINESS / delegation / region / siege parentage. Callers populate the subset their route resource exposes; the matcher iterates and returns true on the first scope that covers a present field. | | `src/lib/principal-matchers.spec.ts` | 24 unit tests covering OR-composition, the empty-requirement degenerate case (treated as "no constraint" — pass), every scope kind, and the resource-side parentage chain. | ### BFF guards + decorators (`apps/portal-bff/src/auth`) | File | Role | | --- | --- | | `require-privilege.{decorator,guard}.ts` | `@RequirePrivilege('Portal.Admin', ...)` — type-locked to the closed `Privilege` catalogue; multiple values OR-combine. | | `require-role.{decorator,guard}.ts` | `@RequireRole('rh', ...)` — same shape, against `FunctionalRole`. | | `require-scope.{decorator,guard}.ts` | `@RequireScope(req => extractResource(req))` — extractor can be async (Prisma lookup pattern); returning `null` is treated as denial with empty `required[]`. | | `principal-extractor.ts` | Single read of `Principal` off the session, with a legacy-session bridge for principals minted before [#206](#206) landed — filters `user.roles` for `Portal.*` values and synthesises an `unrestricted` scope. After the 12 h absolute-TTL window post-deploy, the bridge becomes dead code. | | `principal-extractor.spec.ts` | 7 tests covering both code paths. | | `require-{privilege,role,scope}.guard.spec.ts` | Unit tests for each guard: 401 on anonymous, 403 + audit on denial, pass on match, generic `forbidden` response body (no role/privilege/resource hint leaks). | | `auth-guards.persona-matrix.spec.ts` | Integration tests: each of the 19 test-tenant personas walked through 5 representative privilege guards + 6 representative role guards. The matrix proves the contracts hold against the real provisioning, not just synthesised principals. | ### Audit module | File | Change | | --- | --- | | `audit.types.ts` | New `AuthorizationDeniedInput` discriminated by `kind: 'privilege' \| 'role' \| 'scope'`. Carries `required[]` and `held[]` arrays so an auditor can pivot on "tried admin while logged in as RH" patterns without joining anything. | | `audit.service.ts` | New `authorizationDenied()` method emitting the `auth.authorization_denied` event type. Distinct from the existing `admin.access_denied` so admin-surface signals stay clean. | | `audit.service.spec.ts` | 3 new tests covering the three `kind` values. | ### Legacy guard migration | File | Change | | --- | --- | | `admin/admin-role.guard.ts` | Reads `principal.privileges` instead of `user.roles`. Public `@RequireAdmin()` API unchanged; `admin.access_denied` event type unchanged. The audit row's `rolesHeld` field now carries `Portal.*` values rather than the raw legacy `roles` claim. | | `admin/admin-role.guard.spec.ts` | Rewritten to exercise `session.principal`-shaped sessions, with a dedicated legacy-bridge sub-suite that asserts pre-ADR-0025 sessions still work. | | `me/me.controller.ts` | `capabilities().canAccessAdmin` reads `principal.privileges` via the same extractor. | | `me/me.controller.spec.ts` | Rewritten against the principal shape. | ### AuthModule wiring | File | Change | | --- | --- | | `auth/auth.module.ts` | Three new guards registered as providers and re-exported. | ## Notes for the reviewer - **Composition semantics.** Within a single decorator, values OR-combine (`@RequireRole('rh', 'comptable')` matches anyone with either). Stacking decorators AND-combines at the Nest level — `@RequireRole('rh') @RequirePrivilege('Portal.Admin')` requires both, because each guard runs separately and a single denial stops the request. The empty-requirement case is rejected at the type level by the tuple `[Privilege, ...Privilege[]]` so a route can not opt out of authorization by passing zero values. - **Why `auth.authorization_denied` and not `admin.access_denied` for the new guards.** ADR-0025 §347 originally said "writes an `admin.access_denied` row", but reusing the `admin.*` namespace would have meant the future `@RequireRole('rh')` on a non-admin HR route ships an event whose type implies an admin surface. The new event type lives in the `auth.*` namespace where the guards live; a `kind` discriminator on the payload tells privilege / role / scope denials apart. The legacy `admin.access_denied` event continues unchanged for `AdminRoleGuard`. - **The legacy-session bridge is short-lived.** After the 12 h absolute-TTL window from this PR's deploy, every session in Redis has a `principal` field and the bridge's `else` branch becomes unreachable. The bridge keeps `@RequireAdmin` + `MeController` functional during that window without needing a forced re-auth campaign. - **Why a `ScopableResource` shape rather than a `Resource | Resource | ...` union per kind.** Routes protect heterogeneous resource types (`Etablissement`, `Dossier`, `Person`), each exposing a different subset of the parentage chain. The optional-field shape lets each route populate what its resource carries and lets the matcher iterate over the principal's scopes once. The cost is that a route author who forgets to populate `delegationCode` on an `Etablissement` resource locks delegation-scoped users out — a typing exercise that the next round of work (concrete consumers) will surface naturally. - **Why `principalCoversResource` deny on doubt.** When a route's extractor returns a resource that lacks the parentage `delegationCode` field, a `delegation:33` scope no longer matches — deny is the safe direction. An over-restrictive deny shows up in the audit log (operator can fix the extractor); an over-permissive pass would silently leak data. The non-transitivity of the parentage chain is an intentional contract. - **Why migrate `MeController` in the same PR.** The `canAccessAdmin` flag reads the same axis (`Portal.Admin` privilege). Leaving it on the legacy `user.roles` shape while everything else moves to `principal.privileges` would create internal inconsistency; a future reviewer would rightfully ask "why?". The migration is three lines plus a spec rewrite. - **The drift-CI gate is the next PR.** ADR-0025 §"More Information" step 3. ESLint custom rule (or a `pnpm run` script) grepping every `@RequirePrivilege('...')` / `@RequireRole('...')` / scope-kind literal in the codebase and asserting each one exists in the catalogue. Now there is something to grep for (the decorators and their tests reference catalogue values), so the gate is well-scoped to land standalone. - **No real consumers of `@RequireScope` yet.** The scope guard ships with a fully exercised contract (extractor signature, async support, audit shape, generic-forbidden body) but no live route. The first consumer surface lands with the `Person` + `User` schema (proposed ADR-0026) and the resource-loading routes that follow. ## Test plan - [x] `pnpm nx affected -t lint test build --base=main` — 3 projects green - `portal-bff`: 741 tests pass (was 497 in #206 — +244 new: matchers/guards/persona-matrix/audit/extractor). - `shared-auth`: 41 tests pass (was 17 in #206 — +24 new matcher tests). - `portal-bff-e2e`: lint green. - [x] `pnpm nx format:check` — clean after `pnpm nx format:write`. - [ ] **Review focus** — the 19-persona matrix (each persona walked through 5 privilege guards + 6 role guards); the legacy-session bridge in `principal-extractor.ts` and its tests; the `admin.access_denied` audit payload now carries `Portal.*` values rather than legacy `roles` (intentional, called out above); the `auth.authorization_denied` event-type rationale. ## What's next Per ADR-0025 §"More Information" phasing: 1. ✅ #206 — types + Principal builder + group-to-role mapping skeleton. 2. ✅ **This PR** — the three new decorators + guards, legacy `@RequireAdmin` migration. 3. **Next PR** — drift CI gate. ESLint custom rule (or `pnpm run` script) that asserts every `@RequireX('...')` literal in code is in the closed catalogue. 4. **Depends on ADR-0026** — `user_scopes` Prisma table + seed + `PrismaScopeResolver` replacing `StubScopeResolver`, then the first concrete consumers of `@RequireScope`. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #208	2026-05-23 21:49:34 +02:00
julien	2cdeb74341	feat(portal-bff): audit-stats endpoint — server-side aggregations with redis cache (#173) ... ## 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	2480d0dd6d	fix(portal-bff): align admin entra role name with Portal.Admin (#145) ... ## Summary The [`AdminRoleGuard`](apps/portal-bff/src/admin/admin-role.guard.ts) was matching on the literal `'admin'`, but the Entra app registration declares the admin app role with `value: "Portal.Admin"`. End result: an authenticated user with the role assigned in Entra still landed with `roles: []` in their session (claim simply not present in the id token), and every request to `/api/admin/audit` and `/api/admin/users` returned a **403**. Caught manually in the portal-admin SPA: login succeeded, sidebar links to "Audit log" / "User list" returned 403. The [`/api/admin/auth/me`](apps/portal-bff/src/admin/admin-auth.controller.ts) self-test confirmed the missing claim was the cause. ## What lands ### Constant value — single source of truth [`apps/portal-bff/src/admin/admin-role.guard.ts:18`](apps/portal-bff/src/admin/admin-role.guard.ts#L18): ```diff -export const ADMIN_ROLE = 'admin'; +export const ADMIN_ROLE = 'Portal.Admin'; ``` [`admin-role.guard.spec.ts`](apps/portal-bff/src/admin/admin-role.guard.spec.ts) already imports `ADMIN_ROLE` from the source rather than hardcoding the literal, so the guard contract spec rolls through unchanged. The fixtures elsewhere ([`auth.service.spec.ts`](apps/portal-bff/src/auth/auth.service.spec.ts), [`admin.controller.spec.ts`](apps/portal-bff/src/admin/admin.controller.spec.ts), [`admin-auth.controller.spec.ts`](apps/portal-bff/src/admin/admin-auth.controller.spec.ts), [`require-mfa.guard.spec.ts`](apps/portal-bff/src/auth/require-mfa.guard.spec.ts)) keep `roles: ['admin']` as fixture data — those tests exercise the extraction / serialization pipeline, which is role-value-agnostic; touching them would be incidental cleanup with no behaviour signal. ### Doc-comment refresh Inline references to the role name updated so future readers don't grep `'admin'` and find a phantom value: - [`admin-role.guard.ts`](apps/portal-bff/src/admin/admin-role.guard.ts) — class doc-block (3 mentions). - [`admin.controller.ts`](apps/portal-bff/src/admin/admin.controller.ts) — class doc-block + inline guard-contract comment. - [`audit.service.ts`](apps/portal-bff/src/audit/audit.service.ts) — `adminAccessDenied` doc-block (2 mentions). ### Documentation - [`docs/decisions/0020-portal-admin-app.md`](docs/decisions/0020-portal-admin-app.md) — 5 references to the role across §"How is admin access enforced", §"Auth — same Entra ID …", and the Consequences §. - [`docs/architecture.md`](docs/architecture.md) — note next to the C4 container diagram describing the admin entry gate. - [`CLAUDE.md`](CLAUDE.md) — "Admin application" project rule. ## Notes for the reviewer - **Why `Portal.Admin` rather than `admin`?** Operator's call on the Entra side. The `<Application>.<Role>` namespace is the conventional Entra App Role pattern when the directory may host roles for multiple applications, and `admin` alone is ambiguous in a directory shared across products. - **Why no migration / backfill?** The role value lives only in two places: Entra app-registration manifest (operator-managed) and the BFF constant (this PR). Existing Redis sessions captured `roles: []` (claim absent) — they'll naturally pick up the correct value on next sign-in. No persisted data references the old value. - **No ADR.** ADR-0020 §"How is admin access enforced" already commits to "Entra ID role claim + BFF guard"; the literal role string is an implementation detail the ADR happened to spell. Updated the ADR's prose to the new value to keep the doc honest, but the decision is unchanged. ## Test plan - [x] `pnpm nx test portal-bff` — **396 specs pass**, unchanged from `main`. The `AdminRoleGuard` contract spec (covers 401-on-no-session, 403-on-missing-role + audit emission, pass-through-on-role-present) imports `ADMIN_ROLE` and re-exercises with the new value. - [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean. - [ ] Manual verification — pending Entra-side App Role declaration with `value: "Portal.Admin"` + assignment to the test user. Once both exist: sign out + sign in on portal-admin, hit `/api/admin/auth/me` and confirm `roles: ["Portal.Admin"]`, then click "Audit log" + "User list" and confirm both render. An `admin.access_denied` row in `audit.events` is the negative-test signal (still emitted for any user without the role). --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #145	2026-05-15 10:33:54 +02:00
julien	1513ad327c	feat(portal-bff): openapi spec + scalar api reference UI (dev-only) (#143) ... ## Summary Adds an OpenAPI 3 spec + a [Scalar API Reference](https://scalar.com/) UI to `portal-bff`, dev-only. The BFF previously had no way to *see* its HTTP surface short of grepping for `@Get` / `@Post`; this PR generates the spec from the existing Nest controllers via [`@nestjs/swagger`](https://docs.nestjs.com/openapi/introduction) and renders it through Scalar — a modern alternative to the classic Swagger UI (single-page, fast, dark-mode native, better typography). ## What lands ### Two new dev-only routes | Route | What it serves | | --- | --- | | `GET /api/openapi.json` | Raw OpenAPI 3 document. External tools (Bruno / Insomnia / Postman) import from here. | | `GET /api/docs` | Scalar API Reference HTML page. Loads the JSON spec at render time and renders the full endpoint catalogue with a "Try it" panel. | Both routes are gated behind `process.env.NODE_ENV !== 'production'` in [`setupOpenApi`](apps/portal-bff/src/openapi/openapi.ts) — production deployments don't need the docs surface, and publishing it would hand an attacker a curated map of every authenticated endpoint + every DTO shape. If a future ops use-case wants the spec in prod (internal gateway, contract testing), the gate is one line away from an opt-in `OPENAPI_PUBLISH=true` env knob. ### Core implementation — [`apps/portal-bff/src/openapi/openapi.ts`](apps/portal-bff/src/openapi/openapi.ts) Two exported helpers: - **`buildOpenApiDocument(app)`** — wraps Nest's `DocumentBuilder` + `SwaggerModule.createDocument`. Sets title, description (mentions the CSRF caveat — see below), version, and registers **two** cookie security schemes: - `portal_session` for the user-portal surface ([ADR-0009](docs/decisions/0009-auth-flow-oidc-pkce-msal-node.md)). - `portal_admin_session` for the admin-portal surface ([ADR-0020](docs/decisions/0020-portal-admin-app.md)). No `@ApiBearerAuth` is declared — the BFF never exposes a bearer-auth surface (SPA never holds tokens per ADR-0009; downstream OBO tokens are server-side only per ADR-0014). - **`setupOpenApi(app, globalPrefix)`** — short-circuits in production, otherwise binds the two routes via the Express adapter directly (`app.getHttpAdapter().get(...)` and `app.use(...)`). The OpenAPI JSON is a static asset and Scalar is a vanilla Express middleware — wrapping either in a Nest controller would add zero value and an extra layer of indirection. Wired into bootstrap at [`apps/portal-bff/src/main.ts:220`](apps/portal-bff/src/main.ts#L220), immediately after the JWKS endpoint mount and before `app.listen()`. ### Controllers decorated with `@ApiTags` / `@ApiOperation` / `@ApiCookieAuth` Annotations are cosmetic but make the spec actually browsable. Tag taxonomy: | Controller | Tag | Security | | --- | --- | --- | | [`AppController`](apps/portal-bff/src/app/app.controller.ts) | `app (scaffolding)` | — | | [`HealthController`](apps/portal-bff/src/health/health.controller.ts) | `health` | — | | [`AuthController`](apps/portal-bff/src/auth/auth.controller.ts) | `auth (user portal)` | `portal_session` on `/me` + `/logout` | | [`AdminAuthController`](apps/portal-bff/src/admin/admin-auth.controller.ts) | `auth (admin portal)` | `portal_admin_session` on `/me` + `/logout` | | [`AdminController`](apps/portal-bff/src/admin/admin.controller.ts) | `admin (self-test)` | class-level `portal_admin_session` | | [`AdminAuditController`](apps/portal-bff/src/admin/admin-audit.controller.ts) | `admin (audit log)` | class-level `portal_admin_session` | | [`AdminUsersController`](apps/portal-bff/src/admin/admin-users.controller.ts) | `admin (user directory)` | class-level `portal_admin_session` | `@ApiOperation({ summary: … })` added on every route — populates the one-line description Scalar shows in its left-rail TOC. ### Deps + Jest - `@nestjs/swagger ^11` (matches the Nest 11 major already pinned) and `@scalar/nestjs-api-reference` added to the workspace root. - [`jest.config.cts`](apps/portal-bff/jest.config.cts) — widened `transformIgnorePatterns` from `/node_modules/(?!.*jose)/` to `/node_modules/(?!.*(jose|@scalar/))/`. `@scalar/client-side-rendering` (a transitive dep) ships ESM-only; without this widening the spec suite fails to load the module under ts-jest. ## Notes for the reviewer - **Why two cookie schemes rather than one?** Scalar renders a per-endpoint lock icon driven by the security scheme name. Splitting `portal_session` / `portal_admin_session` keeps the indicator semantically truthful — `/api/auth/me` and `/api/admin/auth/me` look identical otherwise. - **CSRF caveat.** Mutating routes (`POST` / `PUT` / `PATCH` / `DELETE`) require `X-CSRF-Token` per [ADR-0009](docs/decisions/0009-auth-flow-oidc-pkce-msal-node.md). The header must be set manually in Scalar's "Try it" panel to the value of the `portal_csrf` cookie when exercising those routes. The spec description mentions it; auto-injecting the header from the cookie is a future polish. - **No ADR for this.** `@nestjs/swagger` is the framework's own first-party tooling; Scalar is a thin UI on top of a standard OpenAPI 3 document. Both replaceable without touching the controllers (the `@Api*` annotations are spec-standard). Dev-only, no prod surface — doesn't cross any of the bars that warrant an ADR per [CLAUDE.md](CLAUDE.md). - **Express-layer routing.** Same pattern as the JWKS endpoint (#139): the OpenAPI JSON is a static asset and Scalar a vanilla Express handler, so wiring through Nest's router adds no value. ## Test plan - [x] **5 new specs** in [`apps/portal-bff/src/openapi/openapi.spec.ts`](apps/portal-bff/src/openapi/openapi.spec.ts) — document shape (openapi version, title, version), both cookie schemes declared, smoke controller route captured in `paths`, production short-circuit (no routes mounted, no `app.use` called), dev mount (JSON at `/api/openapi.json` via the HTTP adapter, Scalar UI at `/api/docs` via `app.use`). - [x] `pnpm nx test portal-bff` — **396 specs pass** (was 391). - [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean. - [x] Manual dev smoke: `pnpm nx serve portal-bff`, `curl /api/openapi.json | jq .info` returns title + version, open `/api/docs` in a browser, every controller's routes visible under their tag, lock icons match the cookie scheme on guarded routes. ## What's next — light follow-ups Not blocking this PR; mentioned so they're not lost: - Auto-inject the `X-CSRF-Token` header in Scalar from the `portal_csrf` cookie (custom Scalar config preset). - Promote `@ApiOperation` summaries with multi-line `description`s on the more involved routes (`/api/admin/audit`, `/api/admin/users`). - Annotate DTOs with `@ApiProperty` once the first contract-test consumer arrives — Nest can also pick them up automatically with the `@nestjs/swagger` ts-plugin if we wire it into the Nx build target. Deferred until the spec is consumed by tooling that benefits from the precision. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #143	2026-05-14 20:52:03 +02:00
julien	1df99cd800	feat(portal-bff): admin user-directory read endpoint (#141) ... ## 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) ... ## 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	77343e3113	fix(portal-bff): use the real portal-admin dev port (4300) in admin-flow references (#131) ... ## Summary PR #129 (`feat(portal-bff): distinct admin session + /api/admin/auth flow`) baked `4201` into a handful of comments, test fixtures, and the `.env.example` as the portal-admin dev port. The actual port wired in [apps/portal-admin/project.json](apps/portal-admin/project.json#L87) `serve.options.port` is **4300** — that's what `pnpm nx serve portal-admin` listens on. This PR aligns the references so a contributor copying values from `.env.example` (or reading the test fixtures) sees the same port their browser is going to hit. It also drops `http://localhost:4300` into `CORS_ALLOWED_ORIGINS` — the portal-admin SPA will hit the BFF with credentials as soon as the admin auth flow is exercised end-to-end, and without the origin in the allowlist the browser blocks the call. Better to set the right example now than have the next contributor chase a CORS error. ## Touched - [apps/portal-bff/.env.example](apps/portal-bff/.env.example): - `ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI` default + the surrounding comment now point at `http://localhost:4300/`. - `CORS_ALLOWED_ORIGINS` example lists both `:4200` (portal-shell) and `:4300` (portal-admin). - Both sections cite `apps/<app>/project.json` `serve.options.port` as the source of truth so a future reader doesn't have to grep. - [apps/portal-bff/src/config/check-cors-allowlist.ts](apps/portal-bff/src/config/check-cors-allowlist.ts) — stale doc-comment that pre-dated the portal-admin scaffolding, now matches reality. - Test-fixture `adminPostLogoutRedirectUri` values in `auth.module.spec.ts`, `auth.controller.spec.ts`, `auth.service.spec.ts`, `admin-auth.controller.spec.ts`, `check-entra-config.spec.ts` — tests don't depend on the port; aligned for clarity only. ## Test plan - [x] `grep -rn 4201 apps/ libs/` → empty. - [x] `pnpm nx test portal-bff` — **278 specs pass** (unchanged from #129; this PR only touches strings). - [x] No behaviour change in the BFF; only the example values shift. Developers must update their local `.env` to pick up the new port + origin. ## Notes for the reviewer The two new env vars from #129 (`ENTRA_ADMIN_REDIRECT_URI`, `ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI`) plus the existing `CORS_ALLOWED_ORIGINS` are mandatory at boot. If your local `apps/portal-bff/.env` still has the `4201` value, the BFF will still start (any valid URL passes the validators) — but admin logout will 302 you to a port nothing is listening on, and the admin SPA's BFF calls will fail CORS. Update to `4300` to match the actual portal-admin dev server. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #131	2026-05-14 15:40:07 +02:00
julien	fed905edc5	feat(portal-bff): distinct admin session + /api/admin/auth flow (#129) ... ## 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) ... ## 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