julien/apf_portal
1
0
Fork 0
Code Issues 1 Pull Requests Actions 1 Packages Projects Releases Wiki Activity
233 Commits 1 Branch 0 Tags
12136f7a8a6e59f075d71afefd99f51c5226c96b
Commit Graph

3 Commits

Author SHA1 Message Date
julien 2480d0dd6d fix(portal-bff): align admin entra role name with Portal.Admin (#145)
CI / check (push) Successful in 2m47s
Details
CI / commits (push) Has been skipped
Details
CI / scan (push) Successful in 1m47s
Details
CI / a11y (push) Successful in 1m30s
Details
CI / perf (push) Successful in 3m57s
Details 
## 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 6aee56abe9 docs(architecture): refresh containers + nx boundaries diagrams, add local-infra diagram (#119)
CI / check (push) Successful in 2m8s
Details
CI / commits (push) Has been skipped
Details
CI / scan (push) Successful in 2m14s
Details
CI / a11y (push) Successful in 1m12s
Details
CI / perf (push) Successful in 4m3s
Details 
## Summary

Dépoussiérage of `docs/architecture.md` after the auth/session track and a new local-infra diagram.

### Fixed

- **§2 Containers** — `portal-admin` was missing despite shipping in ADR-0020 and being scaffolded in `apps/portal-admin`. Added as a sibling SPA with its own session cookie (`__Host-portal_admin_session`), and the BFF box now distinguishes `/api/*` (end-user) from `/api/admin/*` (RBAC + `@RequireMfa({ freshness: 600 })`).
- **§3 Nx module boundaries** — three small lies:
  - `shared-ui` was drawn under `scope:portal-shell`. The actual tag in [libs/shared/ui/project.json:7](libs/shared/ui/project.json#L7) is `scope:shared`. Same for `shared-state` (which wasn't on the diagram at all). Both now live in the `scope:shared` bubble.
  - `portal-admin` was absent. Added with its planned (dashed) edges to the shared libs and a note that no `scope:portal-admin` row exists in `eslint.config.mjs` yet — that lands when admin modules grow real lib deps (follow-up).
  - The "forbidden" examples included `shared-tokens ⟶ shared-ui` framed as "narrower scope", which doesn't apply (both are `scope:shared` / `type:shared`). Replaced with examples actually enforced by `depConstraints`.

### Added

- **§5 Local dev infrastructure** — visualises what `docker compose up` actually starts: postgres / redis / otel-collector always up, plus opt-in profiles `dbtools` (pgweb), `observability` (Jaeger), `serve-static` (Caddy). Shows ports, named volumes, the bring-up cheat sheet, and the separate CI-runners stack (`apf-portal-ci-runners`, distinct network). Sources point straight at the compose files so a contributor can chase any detail in one click.

### Misc

- Updated the "Trace context propagation" row in the "To be added" table — its trigger ("first observability instrumentation lands") was already met. Now flagged as overdue / pending a follow-up PR (deferred from this scope).
- Carried the pre-existing one-line fix on the §4 `squash[…]` node (mermaid was rejecting unquoted parens inside the label).

## Test plan

- [x] `pnpm exec prettier --check docs/architecture.md` → clean.
- [x] Re-read all five mermaid blocks for syntax (no unquoted parens / `:` / `(` inside `[]` labels). Compose-side `--profile X` only appears inside `"…"` so it's safe.
- [ ] Render in Gitea / IDE markdown preview — the five mermaid blocks should display without errors.
- [ ] Eyeball §5 against [infra/local/dev.compose.yml](../infra/local/dev.compose.yml): every service + profile present, ports match.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #119
 2026-05-13 11:15:03 +02:00
Julien Gautier 312791b74e docs: add docs/architecture.md with C4 contexts, Nx boundaries, and CI/CD pipeline 
Cross-cutting visual reference for the architecture, written in
Mermaid (text in markdown, rendered natively by Gitea / GitHub / IDE
viewers, diffable in PR). Four diagrams that summarise decisions
spread across multiple ADRs:

1. C4 level 1 - System Context. The portal as a black box with its
   workforce/customer/IT/RSSI actors and Entra ID + downstream APIs
   as external systems. Customer audience and Entra External ID are
   shown dashed (future scope per ADR-0008's dual-audience design).

2. C4 level 2 - Containers. Browser-side portal-shell, on-prem BFF,
   Postgres (public + audit schemas), Redis, local OTel Collector,
   plus external Entra ID and downstream APIs. Annotates the wire
   protocols (HTTPS + __Host- cookies, OIDC, OBO/signed assertion,
   OTLP, ioredis with AES-GCM at rest, traceparent end-to-end).

3. Nx module boundaries. The Project graph rendered with the
   depConstraints from ADR-0003 (scope axis: portal-shell /
   portal-bff / shared, plus type axis: app / feature / shared).
   Forbidden directions called out below the diagram.

4. CI/CD pipeline. Local hooks → push → PR → 5 parallel CI jobs
   (check / scan / commits / perf / a11y) → branch protection →
   squash-merge → tag → release. Includes the weekly scheduled
   security-scheduled.yml workflow (full-tree scan + prod
   Lighthouse).

Convention adopted at the top of architecture.md: cross-cutting
diagrams live here; single-ADR diagrams live inline in the ADR
itself (sequence flows, ERDs, lifecycle diagrams, etc.). The 'To be
added' section at the bottom maps each future diagram to where it
will land and what triggers its addition.

docs/README.md index updated with a new 'Architecture' section
linking to architecture.md, and the previous empty 'Architecture'
placeholder removed (the placeholder was a tick-the-box section
that violated the doc convention 'documentation when genuinely
useful, not just to tick a box').
 2026-04-30 20:55:21 +02:00