From ad369d872ad26a915ad90b021a5c194b4f65d81b Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Fri, 15 May 2026 10:29:51 +0200 Subject: [PATCH] fix(portal-bff): align admin entra role name with Portal.Admin The AdminRoleGuard was matching on `admin`, but the Entra app registration declares the role with value `Portal.Admin`. Sessions were established with `roles: []`, so every authenticated user got a 403 on `/api/admin/audit` and `/api/admin/users`. Single source of truth: the `ADMIN_ROLE` constant in admin-role.guard.ts. The spec already imports the constant, so the behaviour rolls through unchanged. Doc-comment touches in admin-role.guard.ts, admin.controller.ts and audit.service.ts keep the inline references accurate; ADR-0020, docs/architecture.md and CLAUDE.md updated to the new name. --- CLAUDE.md | 2 +- apps/portal-bff/src/admin/admin-role.guard.ts | 10 +++++----- apps/portal-bff/src/admin/admin.controller.ts | 6 +++--- apps/portal-bff/src/audit/audit.service.ts | 4 ++-- docs/architecture.md | 2 +- docs/decisions/0020-portal-admin-app.md | 12 ++++++------ 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9a89e1e..954a524 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,7 +48,7 @@ The structural, security, observability, and quality choices are recorded as ADR - **Performance budgets:** Core Web Vitals at Google "Good" thresholds (LCP ≤ 2.5 s, INP ≤ 200 ms, CLS ≤ 0.1, TBT ≤ 200 ms, TTFB ≤ 800 ms), Lighthouse Performance ≥ 90 on critical routes. **Lighthouse CI** (`@lhci/cli`) runs in CI with median-of-3 mitigation, blocking on threshold breach. Angular bundle `budgets` (`type: "error"`): initial ≤ 300 KB gzip, lazy chunks ≤ 100 KB gzip. BFF p95/p99 SLOs per endpoint family observed via OTel (advisory in CI, alerting in prod). Weekly scheduled Lighthouse run on prod env. **a11y wins over perf** when they conflict — see [ADR-0017](docs/decisions/0017-performance-budgets-lighthouse-ci.md). - **Environment configuration:** SPA per-environment values via Angular `environment.ts` + `fileReplacements` at build time (no runtime config-fetch). BFF reads `process.env` directly with small per-key boot-time validators (no `@nestjs/config` overhead at this scale). The audit log uses a separate `AUDIT_DATABASE_URL` connection pool in production (`audit_writer`-only login, defense in depth) and falls back to the shared pool + `SET LOCAL ROLE` in dev — see [ADR-0018](docs/decisions/0018-environment-configuration-strategy.md). - **Internationalisation:** `@angular/localize` in build-time mode, two locales (`fr` default served at `/`, `en`), source locale = English (project English-only rule). Path-based URLs always prefixed (`/fr/...`, `/en/...`); `/` smart-redirects via cookie → `Accept-Language` → `fr`. UI strings live in XLIFF (`messages.fr.xlf`); editorial / CMS content is BFF-served already localised (see admin app). Footer hosts the locale switcher; switching writes a `__Host-portal_locale` cookie and hard-refreshes — see [ADR-0019](docs/decisions/0019-internationalisation-angular-localize.md). -- **Admin application (`portal-admin`):** dedicated Angular SPA alongside `portal-shell`, sharing the same `portal-bff` via `/api/admin/*` routes guarded by an Entra `admin` role + `@RequireMfa({ freshness: 600 })` at entry. Distinct origin / cookie / session from `portal-shell` (`__Host-portal_admin_session`). v1 modules: CMS for static pages (multilingual), menu management, user list (read-only), audit log viewer. Bundle budget relaxed to ≤ 500 KB gzip (vs 300 KB for `portal-shell`); same a11y + dark-mode baseline. Shared UI primitives (`Icon`, `LayoutStateService`, brand tokens) graduate to `libs/shared/*` as both apps need them — see [ADR-0020](docs/decisions/0020-portal-admin-app.md). +- **Admin application (`portal-admin`):** dedicated Angular SPA alongside `portal-shell`, sharing the same `portal-bff` via `/api/admin/*` routes guarded by an Entra `Portal.Admin` role + `@RequireMfa({ freshness: 600 })` at entry. Distinct origin / cookie / session from `portal-shell` (`__Host-portal_admin_session`). v1 modules: CMS for static pages (multilingual), menu management, user list (read-only), audit log viewer. Bundle budget relaxed to ≤ 500 KB gzip (vs 300 KB for `portal-shell`); same a11y + dark-mode baseline. Shared UI primitives (`Icon`, `LayoutStateService`, brand tokens) graduate to `libs/shared/*` as both apps need them — see [ADR-0020](docs/decisions/0020-portal-admin-app.md). - **Local quality gates:** Husky + lint-staged + commitlint with Conventional Commits — see [ADR-0007](docs/decisions/0007-pre-commit-hooks-and-conventional-commits.md). - **Runtime:** Node.js latest LTS major. diff --git a/apps/portal-bff/src/admin/admin-role.guard.ts b/apps/portal-bff/src/admin/admin-role.guard.ts index 71ac5c1..c77d096 100644 --- a/apps/portal-bff/src/admin/admin-role.guard.ts +++ b/apps/portal-bff/src/admin/admin-role.guard.ts @@ -10,12 +10,12 @@ import { AuditWriter } from '../audit/audit.service'; /** * Single Entra app role that gates the entire `/api/admin/*` surface - * per ADR-0020 §"Auth — `admin` role claim". The value matches the - * `value` field declared on the app role in the Entra app + * per ADR-0020 §"Auth — `Portal.Admin` role claim". The value matches + * the `value` field declared on the app role in the Entra app * registration manifest. Defined as a constant so spec assertions * can refer to the same source of truth. */ -export const ADMIN_ROLE = 'admin'; +export const ADMIN_ROLE = 'Portal.Admin'; /** * `AdminRoleGuard` — enforces ADR-0020's role-based admin gate. @@ -28,7 +28,7 @@ export const ADMIN_ROLE = 'admin'; * unauthenticated 401 is normal traffic the absolute-timeout * middleware would surface anyway. * - * - **Session but missing `admin` role → 403 + audit.** The user + * - **Session but missing `Portal.Admin` role → 403 + audit.** The user * *is* authenticated; they just are not authorised for admin. * This is the privilege-escalation attempt audit signal — every * denial lands in `audit.events` with `outcome=denied`, the @@ -38,7 +38,7 @@ export const ADMIN_ROLE = 'admin'; * (consistent with the existing audit call sites in * `AuthController`). * - * - **Session with `admin` role → pass through.** Downstream + * - **Session with `Portal.Admin` role → pass through.** Downstream * controllers see `req.session.user` populated and can rely on * the role check having happened. */ diff --git a/apps/portal-bff/src/admin/admin.controller.ts b/apps/portal-bff/src/admin/admin.controller.ts index f666ddd..d990863 100644 --- a/apps/portal-bff/src/admin/admin.controller.ts +++ b/apps/portal-bff/src/admin/admin.controller.ts @@ -5,8 +5,8 @@ import { RequireAdmin } from './require-admin.decorator'; /** * `/api/admin/*` controllers per ADR-0020. The `@RequireAdmin()` - * class-level decorator gates every route on the Entra `admin` role; - * downstream PRs will add per-method `@RequireMfa({ freshness: … })` + * class-level decorator gates every route on the Entra `Portal.Admin` + * role; downstream PRs will add per-method `@RequireMfa({ freshness: … })` * for the entry route. * * `GET /api/admin/me` is the self-test endpoint named in ADR-0020's @@ -27,7 +27,7 @@ export class AdminController { @Get('me') me(@Req() req: Request) { // `AdminRoleGuard` has already established that `req.session.user` - // is present and has the `admin` role — the handler can read the + // is present and has the `Portal.Admin` role — the handler can read the // field without re-checking. Using the non-null assertion keeps // the dead-branch noise out of the controller while leaving the // safety contract anchored in the guard's spec. diff --git a/apps/portal-bff/src/audit/audit.service.ts b/apps/portal-bff/src/audit/audit.service.ts index 5c62b6c..22df74e 100644 --- a/apps/portal-bff/src/audit/audit.service.ts +++ b/apps/portal-bff/src/audit/audit.service.ts @@ -196,8 +196,8 @@ export class AuditWriter { /** * Typed event: `/api/admin/*` request rejected by `AdminRoleGuard` - * because the session's `roles` claim does not include `admin`. - * Per ADR-0020 §"Auth — same Entra ID … `admin` role claim", every + * 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) diff --git a/docs/architecture.md b/docs/architecture.md index e18a7ab..f15ff4e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -84,7 +84,7 @@ flowchart TB Notes embedded in the diagram: -- **Two SPAs, one BFF.** `portal-shell` is the end-user surface; `portal-admin` is a separate Angular app on a distinct origin with its own bundle, sign-in flow, and session cookie (per ADR-0020). They share libs but never a runtime — an admin session is not silently authenticated to `portal-shell` and vice versa. The admin entry route is gated by an Entra `admin` app-role claim + `@RequireMfa({ freshness: 600 })` per ADR-0011. +- **Two SPAs, one BFF.** `portal-shell` is the end-user surface; `portal-admin` is a separate Angular app on a distinct origin with its own bundle, sign-in flow, and session cookie (per ADR-0020). They share libs but never a runtime — an admin session is not silently authenticated to `portal-shell` and vice versa. The admin entry route is gated by an Entra `Portal.Admin` app-role claim + `@RequireMfa({ freshness: 600 })` per ADR-0011. - The SPAs carry no token. Only the opaque session id cookie (`__Host-portal_session` / `__Host-portal_admin_session`) plus the CSRF cookie (`__Host-portal_csrf`, read by JS for double-submit). - `traceparent` (W3C) propagates from the browser through the BFF to the downstream APIs — same `trace_id` end-to-end. - The OTel Collector is the only piece coupled to the eventual on-prem observability backend. Choice of backend (Grafana Loki + Tempo, ELK, …) is deferred to the on-prem infrastructure ADR. diff --git a/docs/decisions/0020-portal-admin-app.md b/docs/decisions/0020-portal-admin-app.md index a4ef96e..e43f20e 100644 --- a/docs/decisions/0020-portal-admin-app.md +++ b/docs/decisions/0020-portal-admin-app.md @@ -44,7 +44,7 @@ This ADR records **where the admin lives**, **what ships in v1**, and **what sta ### How is admin access enforced -- **Entra ID role claim + BFF guard** _(chosen)_. The user's app role in Entra carries `admin` (assignment managed in Entra Admin Center). The BFF reads the role claim on every request to `/api/admin/*`; missing role → 403. The SPA does not own authorisation; it just decides what to render based on the claim it sees through the session payload. +- **Entra ID role claim + BFF guard** _(chosen)_. The user's app role in Entra carries `Portal.Admin` (assignment managed in Entra Admin Center). The BFF reads the role claim on every request to `/api/admin/*`; missing role → 403. The SPA does not own authorisation; it just decides what to render based on the claim it sees through the session payload. - **Static IP / VPN gate only**. Network-layer isolation but no identity-bound check. Not sufficient; if an internal user without admin role lands on the admin URL, they should not be able to act. - **Distinct OIDC client / scope per app**. Two app registrations in Entra, one per audience. Symmetric but operationally heavier without clear payoff at our scale. @@ -85,13 +85,13 @@ apps/ - `portal-admin` is a separate Angular workspace app per the Nx `apps` preset. Same Angular major (latest LTS), same standalone + zoneless + Signals + CSR-only configuration as `portal-shell` (ADR-0004). - Shared UI primitives that turn out to need promoting (the `Icon` façade, `LayoutStateService`, brand tokens, Tailwind dark-mode variant, button / form primitives) graduate to `libs/shared/ui` and `libs/shared/state` when the second consumer materialises — that is _now_. They stay app-local in `portal-shell` until then; the move is mechanical. -- The BFF gains `apps/portal-bff/src/admin/admin.module.ts`. Every admin controller is guarded by a Nest `@UseGuards(AdminRoleGuard)` that asserts the session's role claim contains `admin`. The guard returns 403 (not 401) on missing role — the user _is_ authenticated, just not authorised. Audit log captures every 403 with the actor and the attempted route. +- The BFF gains `apps/portal-bff/src/admin/admin.module.ts`. Every admin controller is guarded by a Nest `@UseGuards(AdminRoleGuard)` that asserts the session's role claim contains `Portal.Admin`. The guard returns 403 (not 401) on missing role — the user _is_ authenticated, just not authorised. Audit log captures every 403 with the actor and the attempted route. - `portal-admin` ships under a distinct origin / hostname (e.g. `admin.portal.apf.fr` or `portal.apf.fr/admin/` depending on the production hosting decision in the future infrastructure ADR). The exact URL is **not** locked here — it depends on TLS / reverse-proxy choices. Lock-in level: medium; either option lets us IP-restrict or VPN-gate the admin URL without affecting `portal-shell`. -### Auth — same Entra ID, same MSAL Node, `admin` role claim, fresh-MFA at entry +### Auth — same Entra ID, same MSAL Node, `Portal.Admin` role claim, fresh-MFA at entry -- Identity & auth flow per ADR-0008 / ADR-0009 are reused as-is. `portal-admin` is **not** a new Entra app registration in v1 — it uses the same registration with an additional `admin` app role assignable in Entra Admin Center. -- On every request to `/api/admin/*`, the BFF guard reads `session.user.roles` and requires `admin`. Missing role → 403 with an audit log entry (`action: 'admin.access_denied'`). +- Identity & auth flow per ADR-0008 / ADR-0009 are reused as-is. `portal-admin` is **not** a new Entra app registration in v1 — it uses the same registration with an additional `Portal.Admin` app role assignable in Entra Admin Center. +- On every request to `/api/admin/*`, the BFF guard reads `session.user.roles` and requires `Portal.Admin`. Missing role → 403 with an audit log entry (`action: 'admin.access_denied'`). - The admin SPA's entry route is decorated with `@RequireMfa({ freshness: 600 })` (ADR-0011) — even though the standard portal does not require fresh MFA in v1, the admin app always does. Concrete consequence: signing in to admin re-prompts MFA if the user's last MFA event is older than 10 minutes. ### Sessions — distinct from `portal-shell` @@ -145,7 +145,7 @@ apps/ - Good, because admin code is isolated from the end-user bundle — zero risk of accidentally shipping admin features to the public surface (e.g. an export button leaking through a feature flag misconfiguration). - Good, because the admin URL is a first-class concern operationally — VPN, IP allow-list, stricter Conditional Access policy are all local changes. - Good, because the existing security baseline (sessions, audit, OBO, observability) is reused — no duplicate code paths, no risk of "the admin has its own auth that drifted". -- Good, because the `admin` Entra app role becomes the single source of authority. Granting / revoking admin is one Entra Admin Center operation, takes effect on next sign-in. +- Good, because the `Portal.Admin` Entra app role becomes the single source of authority. Granting / revoking admin is one Entra Admin Center operation, takes effect on next sign-in. - Good, because the menu-management feature delivers the `requiredPermissions` story the `` already anticipates. - Bad, because Nx now manages two SPAs — wider CI matrix, larger workspace. Bounded; Nx is designed for this. - Bad, because shared UI primitives must graduate to `libs/shared/*` _now_ (a chunk of refactor) where they could have stayed app-local indefinitely otherwise. Acceptable; the move is mechanical and the libs are the right home anyway.