fix(portal-bff): align admin entra role name with Portal.Admin
CI / scan (pull_request) Successful in 2m47s
CI / commits (pull_request) Successful in 2m47s
CI / check (pull_request) Successful in 2m56s
CI / a11y (pull_request) Successful in 2m40s
CI / perf (pull_request) Successful in 5m51s

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.
This commit is contained in:
Julien Gautier
2026-05-15 10:29:51 +02:00
parent 0335d992d9
commit ad369d872a
6 changed files with 18 additions and 18 deletions
@@ -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.
*/
@@ -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.
+2 -2
View File
@@ -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)