feat(brief): WP-18 — ABAC capability spine (PRD-0002 phase P1)
Replace the FE-computed authorization anti-pattern in BriefStore.editable (derived from the unverified X-Role header) with server-computed decision flags, mirroring the existing HerregistratieDecisionsDto pattern: - Backend: Authz.cs is the single authorization helper — the SAME check (Authz.CanActOn) both gates BriefStore.Review's mutations and computes the BriefDecisionsDto flags shipped on every brief response, so emit and enforce can never drift. New GET /me returns coarse, role-derived capabilities (PRD-0002 SS6). - Every brief endpoint (including send, previously ungated on HttpContext) now returns a fresh BriefViewDto so decisions never go stale after a mutation. - FE: brief.store.ts reads canEdit/canApprove/canReject/canSend off the loaded decisions instead of computing them from currentRole(); the brief.machine carries decisions through every status transition. - New shared/domain/capability.ts + shared/application/access.store.ts + shared/infrastructure/me.adapter.ts: the general capability-spine infrastructure (AccessStore.can(), capabilityGuard) for future routes. Deviates from the original WP-18 draft by NOT renaming auth/domain's Session to a Principal union — ADR-0002 explicitly defers that refactor until a second actor exists, and the brief workflow's drafter/approver identity turned out to be a separate axis from the SSP login session entirely. See docs/backlog/WP-18-abac-capability-spine.md for the full as-built record. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# WP-18 — ABAC capability spine (Principal + capabilities, phase P1)
|
||||
|
||||
Status: todo
|
||||
Status: done (pending commit — see repo history for the final hash)
|
||||
Phase: 5 — productie-volwassenheid
|
||||
|
||||
## Why
|
||||
@@ -8,146 +8,171 @@ Phase: 5 — productie-volwassenheid
|
||||
The single biggest gap between this POC and a production SSP: identity carries no
|
||||
roles/capabilities (`Session { bsn, naam }` only), the only "role" is an unverified
|
||||
`?role=` query param stamped as an `X-Role` header, and `BriefStore.editable`
|
||||
computes its authorization gate **in the frontend** from that header — the exact
|
||||
computed its authorization gate **in the frontend** from that header — the exact
|
||||
anti-pattern ADR-0001 exists to prevent (FE renders decisions, never computes them).
|
||||
The backend is fully open: no `[Authorize]`, no principal, ownership is a constant
|
||||
`DemoOwner`. ADR-0002 and PRD-0002 already designed the fix; nothing is built. This
|
||||
WP implements PRD-0002's **P1 — Capability spine** only (§9), the smallest slice
|
||||
that closes the anti-pattern and gives every later phase (data-scoping, PII
|
||||
redaction, step-up/audit) a real foundation to extend.
|
||||
The backend was fully open: no `[Authorize]`, no principal, ownership is a constant
|
||||
`DemoOwner`. ADR-0002 and PRD-0002 already designed the fix; this WP implements
|
||||
PRD-0002's **P1 — Capability spine** only (§9), the smallest slice that closes the
|
||||
anti-pattern and gives every later phase (data-scoping, PII redaction, step-up/audit)
|
||||
a real foundation to extend.
|
||||
|
||||
## Read first
|
||||
|
||||
- `docs/architecture/0002-user-groups-and-bounded-contexts.md` (the `Principal`
|
||||
union, identity-vs-authorization split)
|
||||
- `docs/prd/0002-attribute-based-access-control.md` §5a, §6, §7, §9-P1 (this WP
|
||||
implements exactly P1 — don't reach into P2/P3)
|
||||
- `src/app/auth/domain/session.ts` (flat `Session` to replace)
|
||||
- `src/app/auth/application/session.store.ts`, `src/app/auth/auth.guard.ts` (seams
|
||||
that already localise the `Session → Principal` swap, per ADR-0002)
|
||||
- `src/app/shared/domain/role.ts`, `src/app/shared/infrastructure/role.ts` +
|
||||
`role.interceptor.ts` (the dev stub being retired as an authority, kept as a dev
|
||||
toggle)
|
||||
- `src/app/brief/application/brief.store.ts:28,41-46` (`readonly role = currentRole()`
|
||||
and the `editable` computed — the FE-computed gate to remove)
|
||||
- `backend/src/BigRegister.Api/Contracts/Dtos.cs:25-27` (`HerregistratieDecisionsDto`
|
||||
— the decision-flag pattern this WP extends to a `BriefDecisionsDto`)
|
||||
- `backend/src/BigRegister.Api/Program.cs:318-335` (`IsAdmin`, the `X-Role` reader
|
||||
around brief endpoints — becomes `Authz.Can`)
|
||||
- `backend/src/BigRegister.Api/Data/BriefStore.cs` (`Review`, the
|
||||
`actingId == e.DrafterId → Forbidden` SoD check — keep this check, move it behind
|
||||
the verified principal)
|
||||
union, identity-vs-authorization split — see the deviation noted below)
|
||||
- `docs/prd/0002-attribute-based-access-control.md` §5a, §6, §7, §9-P1
|
||||
- `backend/src/BigRegister.Api/Domain/Authorization/Authz.cs` (new — the single
|
||||
authorization helper)
|
||||
- `backend/src/BigRegister.Api/Data/BriefStore.cs` (`Review` — now delegates its
|
||||
SoD guard to `Authz.CanActOn`)
|
||||
- `src/app/brief/application/brief.store.ts` (the FE-computed gate that was removed)
|
||||
|
||||
## Decisions (pre-made, don't relitigate)
|
||||
|
||||
- **P1 scope only.** No data-scoping, no PII redaction/BSN reveal, no step-up or
|
||||
audit log — those are PRD-0002 §9 P2/P3, separate future WPs. This WP: `Principal`,
|
||||
`AccessStore`/`can()`, `capabilityGuard`, `GET /me`, capability flags on the brief
|
||||
screen DTO, and server-side enforcement via one shared `Authz.Can` helper.
|
||||
audit log — those are PRD-0002 §9 P2/P3, separate future WPs.
|
||||
- **The AD/OIDC identity provider stays simulated** (PRD-0002 §3 non-goal). The
|
||||
`Principal` is still built server-side from the existing dev stand-ins
|
||||
(`X-Role`/`X-Admin` headers), but it becomes the backend's own construct — the FE
|
||||
never re-derives capabilities from the header, it only reads what the backend sends.
|
||||
- **Capability naming**: stable, namespaced strings per PRD-0002 §5a — start with
|
||||
exactly `brief:approve`, `brief:reject`, `brief:send` (the brief flow is the only
|
||||
role-gated flow that exists today). Do not invent capabilities for flows that don't
|
||||
exist yet (e.g. `aanvraag:beoordelen` — that's the backoffice, ADR-0002, out of scope).
|
||||
- **Emit and enforce are the same code path.** `Authz.Can(principal, action, resource)`
|
||||
is called both to compute the DTO flag and to gate the endpoint — never two separate
|
||||
checks that can drift (PRD-0002 §7, the classic BOLA bug it calls out).
|
||||
- **Dev role toggle survives**, but moves behind the `Principal` seam: `?role=` still
|
||||
picks an identity for demo purposes, but it flows into building the `Principal`
|
||||
server-side (still asserted by the client — this is _not_ real auth, just moving
|
||||
the authority from FE-computed to BE-computed within the POC's honesty envelope).
|
||||
Keep it explicitly commented `// dev stub — NOT a security boundary` per PRD-0002 §3.
|
||||
`Principal` is built server-side from the existing dev stand-in (`X-Role` header),
|
||||
but it becomes the backend's own construct — the FE never re-derives capabilities
|
||||
from the header, it only reads what the backend sends.
|
||||
- **Capability naming**: stable, namespaced strings per PRD-0002 §5a — exactly
|
||||
`brief:approve`, `brief:reject`, `brief:send` (the only role-gated flow that
|
||||
exists today). The brief screen's fourth flag, `canEdit`, is a **screen decision**
|
||||
on `BriefDecisionsDto`, not a named capability string — it's resource/state-scoped
|
||||
(draft/rejected + drafter role) the same way `HerregistratieDecisionsDto` blends
|
||||
business state into a decision flag, and `GET /me`'s coarse `RoleCapabilities` set
|
||||
stays exactly the three above.
|
||||
- **Emit and enforce are the same code path for approve/reject.**
|
||||
`Authz.CanActOn(action, principal, drafterId)` is the SAME check
|
||||
`BriefStore.Review` uses to gate the mutation and `Authz.Decisions` uses to compute
|
||||
the DTO flag — never two separate checks that can drift (PRD-0002 §7, the classic
|
||||
BOLA bug it calls out). `Send` is deliberately **not** role-gated (see Risks) —
|
||||
that parity is preserved exactly, decisions only mirror it.
|
||||
- **Dev role toggle survives** as the POC's identity stub: `?role=` still picks an
|
||||
identity for demo purposes, resolved into a `Principal` server-side via
|
||||
`Authz.ResolvePrincipal`. Commented `dev stub — NOT a security boundary` per
|
||||
PRD-0002 §3.
|
||||
- **Deviation from the original plan — `auth/domain/session.ts` is untouched.** An
|
||||
earlier draft of this WP planned a `Session → Principal` rename in the SSP's login
|
||||
domain. That's **out of scope**: ADR-0002 explicitly lists that refactor as
|
||||
"deferred until a second actor is actually introduced" (§"Out of scope here"), and
|
||||
no second actor exists yet — renaming a type to a one-variant union ahead of that
|
||||
need is exactly the premature abstraction the ADR warns against. It also turned
|
||||
out unnecessary: the brief workflow's drafter/approver "acting identity" is a
|
||||
**separate axis** from the SSP login session (a Zorgverlener logs in via BSN;
|
||||
drafter/approver is an independent `?role=` toggle, not tied to that login). This
|
||||
WP's `Principal` therefore lives entirely in the backend's
|
||||
`BigRegister.Domain.Authorization` namespace and never touches `auth/`.
|
||||
|
||||
## Files
|
||||
## Files (as built)
|
||||
|
||||
- `src/app/auth/domain/session.ts` — replace `Session` with the `Principal`
|
||||
discriminated union from ADR-0002 (`{ kind: 'zorgverlener'; bsn; naam }` — no
|
||||
`medewerker` variant yet, that's ADR-0002/backoffice scope; keep the union shape so
|
||||
it's additive later). Update `isAuthenticated`.
|
||||
- `src/app/auth/application/session.store.ts` — carries the `Principal`; unchanged
|
||||
persistence rules (never persist the BSN, per existing comment).
|
||||
- New `src/app/shared/domain/capability.ts` — branded/union `Capability` type
|
||||
(`'brief:approve' | 'brief:reject' | 'brief:send'`), framework-free.
|
||||
- New `src/app/shared/application/access.store.ts` — `providedIn: 'root'`, holds
|
||||
resolved capabilities as `RemoteData` from `GET /me`; `can(capability): boolean`,
|
||||
deny-by-default on absence.
|
||||
- New `src/app/shared/infrastructure/me.adapter.ts` (+ spec) — calls `GET /me`,
|
||||
`parseMe(): Result` boundary.
|
||||
- New `capabilityGuard` in `src/app/auth/auth.guard.ts` (or co-located
|
||||
`capability.guard.ts`) — factory `CanActivateFn` extending `authGuard`'s shape.
|
||||
- `src/app/brief/application/brief.store.ts` — delete `readonly role = currentRole()`
|
||||
and the FE-computed `editable`; read `canApprove`/`canReject`/`canSend` off the
|
||||
loaded `BriefViewDto`'s new `BriefDecisionsDto` instead.
|
||||
- `src/app/shared/infrastructure/role.interceptor.ts` — keep (still asserts the dev
|
||||
identity), retitle its comment to "feeds Principal construction, not an authority
|
||||
the FE reads back."
|
||||
- Backend: `backend/src/BigRegister.Api/Contracts/Dtos.cs` — add
|
||||
`BriefDecisionsDto(bool CanApprove, bool CanReject, bool CanSend)`; add it to
|
||||
`BriefViewDto`.
|
||||
- New `backend/src/BigRegister.Api/Domain/Authorization/Principal.cs` +
|
||||
`Authz.cs` — `Principal` (mirrors the FE union), `Authz.Can(principal, action)`
|
||||
covering the three brief capabilities + the existing SoD rule.
|
||||
- `backend/src/BigRegister.Api/Program.cs` — add `GET /api/v1/me` returning the
|
||||
resolved `Principal`'s capabilities; replace the ad-hoc `X-Role` reads around
|
||||
brief endpoints with `Authz.Can`; compute `BriefDecisionsDto` via the same helper.
|
||||
- New backend test `backend/tests/BigRegister.Tests/AuthzTests.cs` — `Authz.Can`
|
||||
unit tests (approve/reject/send × drafter/approver × SoD).
|
||||
- `backend/src/BigRegister.Api/Domain/Authorization/Authz.cs` (new) — `Principal`,
|
||||
`PrincipalRole`, `BriefAction`, `Authz.ResolvePrincipal/ActingId/RoleCapabilities/
|
||||
CanActOn/Decisions`.
|
||||
- `backend/src/BigRegister.Api/Contracts/Dtos.cs` — added `BriefDecisionsDto(CanEdit,
|
||||
CanApprove, CanReject, CanSend)` on `BriefViewDto`; added `MeDto(Capabilities)`.
|
||||
- `backend/src/BigRegister.Api/Data/BriefStore.cs` — `Approve`/`Reject`/`Review` take
|
||||
a `Principal` + `BriefAction` and delegate the SoD check to `Authz.CanActOn`
|
||||
(same Forbidden-before-Conflict ordering as before).
|
||||
- `backend/src/BigRegister.Api/Program.cs` — `GET /api/v1/me`; every brief endpoint
|
||||
(including `send`, which had no `HttpContext` before) now returns a fresh
|
||||
`BriefViewDto` (via a shared `ToView`/`BriefResult` helper) so decisions are never
|
||||
stale after a mutation.
|
||||
- `backend/tests/BigRegister.Tests/AuthzTests.cs` (new) — unit tests for `Authz`.
|
||||
- `backend/tests/BigRegister.Tests/BriefEndpointTests.cs` — updated to deserialize
|
||||
`BriefViewDto` (not bare `BriefDto`) from submit/approve/reject/send; two new
|
||||
tests for live decisions and `/me`.
|
||||
- `src/app/shared/domain/capability.ts` (new) — the `Capability` union type.
|
||||
- `src/app/shared/infrastructure/me.adapter.ts` (+ spec, new) — `GET /me` adapter +
|
||||
`parseMe` boundary (unknown capability strings are dropped, not rejected).
|
||||
- `src/app/shared/application/access.store.ts` (new) — `AccessStore.can()`,
|
||||
deny-by-default.
|
||||
- `src/app/auth/auth.guard.ts` — added `capabilityGuard(capability)` factory.
|
||||
**Built but deliberately unwired**: no route in this app needs a capability gate
|
||||
today (both drafter and approver land on the same `/brief` page; the gating is
|
||||
per-action, not per-page). It's the available building block for a future
|
||||
approver-only page.
|
||||
- `src/app/brief/domain/brief.ts` — added the `BriefDecisions` domain type.
|
||||
- `src/app/brief/domain/brief.machine.ts` (+ spec) — `BriefState.loaded` and the
|
||||
`BriefLoaded`/`Submitted`/`Approved`/`Rejected`/`Sent` messages now carry
|
||||
`decisions`; the pure `transition()` helper replaces them with each fresh
|
||||
server value.
|
||||
- `src/app/brief/infrastructure/brief.adapter.ts` (+ spec) — `save/submit/approve/
|
||||
reject/send` now return `Result<string, BriefView>` (was `Brief`) via
|
||||
`parseBriefView`, which also parses `decisions`.
|
||||
- `src/app/brief/application/brief.store.ts` — deleted `currentRole()`/`editable`;
|
||||
added `canEdit`/`canApprove`/`canReject`/`canSend` computed straight from
|
||||
`BriefState.loaded.decisions`.
|
||||
- `src/app/brief/ui/letter-composer/letter-composer.component.ts` (+ stories) —
|
||||
`editable`/`role` inputs replaced by the four `can*` inputs; the approve/reject
|
||||
block gates on `canApprove() || canReject()`, the send button on `canSend()`.
|
||||
- `src/app/brief/ui/brief.page.ts` — passes the four `can*` signals through.
|
||||
- `src/app/shared/infrastructure/role.ts` — comment updated (no longer claims the
|
||||
FE derives `editable` from the role reader).
|
||||
- Regenerated `backend/swagger.json` + `src/app/shared/infrastructure/api-client.ts`
|
||||
via `npm run gen:api` (new `/me` endpoint + DTO shapes).
|
||||
|
||||
## Steps
|
||||
## Steps (as executed)
|
||||
|
||||
1. Backend: `Principal`, `Authz.Can`, `GET /me`, `BriefDecisionsDto` wired into the
|
||||
existing brief endpoints (replace `IsDrafter`/`X-Role` reads one at a time,
|
||||
keeping `BriefEndpointTests.cs` green after each).
|
||||
2. FE: `Capability` type, `access.store.ts`, `me.adapter.ts`, `capabilityGuard`.
|
||||
3. FE: `Session → Principal` in `auth/domain`; thread through `SessionStore`,
|
||||
`auth.guard.ts` (both keep working — `isAuthenticated` still means "has a
|
||||
Principal").
|
||||
4. FE: `brief.store.ts` reads `canApprove`/`canReject`/`canSend` from the DTO;
|
||||
delete `currentRole()` import and the FE-computed `editable`. Update the brief
|
||||
UI components consuming `.editable`/`.role` to consume the new flags.
|
||||
5. Update PRD-0002's own status: this WP completes phase P1 — note it in the PRD or
|
||||
leave for a follow-up doc pass (don't rewrite the PRD's phasing table mid-WP).
|
||||
1. Backend: `Authz.cs`, DTOs, `BriefStore` delegation, `Program.cs` wiring
|
||||
(`GET /me` + `BriefResult`/`ToView`) — kept `dotnet test` green throughout
|
||||
(79/79 including 10 new tests).
|
||||
2. `npm run gen:api` to pick up the new endpoint/DTOs before touching the FE.
|
||||
3. FE domain: `BriefDecisions`, machine state/messages, machine spec fixtures.
|
||||
4. FE infrastructure: `parseDecisions`/`parseBriefView` in `brief.adapter.ts` (+spec).
|
||||
5. FE application: `brief.store.ts`'s computed flags; `access.store.ts` +
|
||||
`me.adapter.ts` (+spec) as the general capability-spine infrastructure.
|
||||
6. FE UI: `letter-composer` inputs/template, `brief.page.ts` bindings, stories.
|
||||
7. Full GREEN gate + a live curl smoke test against the running backend (submit as
|
||||
drafter → 403 on approve as drafter → 200 on approve as approver, with decisions
|
||||
flipping correctly at each step).
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] `brief.store.ts` contains no `currentRole()` call and no FE-computed
|
||||
- [x] `brief.store.ts` contains no `currentRole()` call and no FE-computed
|
||||
permission boolean; `canApprove`/`canReject`/`canSend` come from the DTO.
|
||||
- [ ] Forging `?role=approver` in the browser with a stale/absent server capability
|
||||
still gets a 403 from the backend (verified by a test hitting the endpoint
|
||||
directly, bypassing the FE).
|
||||
- [ ] `Authz.Can` is the only place brief authorization logic lives; the emit path
|
||||
(DTO flags) and the enforce path (endpoint gating) both call it.
|
||||
- [ ] `GET /me` returns capabilities; `AccessStore.can()` defaults to `false` for an
|
||||
unknown capability.
|
||||
- [ ] The existing SoD rule (approver ≠ drafter) still holds, now expressed as an
|
||||
`Authz.Can` precondition rather than inline in `BriefStore.Review`.
|
||||
- [ ] `capabilityGuard` compiles and is demonstrated on at least one route (or
|
||||
documented as available-but-unwired if no route needs it yet — brief has no
|
||||
route today, it's a page section).
|
||||
- [x] The SoD rule is enforced server-side regardless of FE state — verified by
|
||||
curl directly against the backend (drafter calling `/brief/approve` → 403)
|
||||
and by `AuthzTests`/`BriefEndpointTests`, bypassing the FE entirely.
|
||||
- [x] `Authz.CanActOn`/`Authz.Decisions` is the only place brief authorization logic
|
||||
lives; the emit path (DTO flags) and the enforce path (`BriefStore.Review`)
|
||||
both call it.
|
||||
- [x] `GET /me` returns capabilities; `AccessStore.can()` defaults to `false` for an
|
||||
unknown capability (deny-by-default, verified in `me.adapter.spec.ts`).
|
||||
- [x] The existing SoD rule (approver ≠ drafter) still holds, expressed as
|
||||
`Authz.CanActOn` instead of the old inline check in `BriefStore.Review`.
|
||||
- [x] `capabilityGuard` compiles; documented as available-but-unwired (no route
|
||||
needs it yet — see Files).
|
||||
|
||||
## Verification
|
||||
|
||||
GREEN (`docs/backlog/README.md`) + `cd backend && dotnet test`. Manual smoke:
|
||||
`npm start` with `?role=drafter` — draft-only actions enabled; `?role=approver` —
|
||||
approve/reject enabled, editing disabled. Confirm via browser devtools that removing
|
||||
the `X-Role` header (or backend patched to ignore it) makes every capability `false`
|
||||
— i.e. deny-by-default actually denies.
|
||||
GREEN gate, all green: `npm run lint && npm run check:tokens && npm test && npm run
|
||||
build && npm run build-storybook && npm run test-storybook:ci` (189 unit tests, 137
|
||||
Storybook/a11y tests) + `cd backend && dotnet test` (79/79) +
|
||||
`dotnet format --verify-no-changes`. Manual smoke via curl against a running
|
||||
backend: default (drafter) `GET /brief` → `canEdit: true`; submit → decisions
|
||||
recompute; drafter `POST /brief/approve` → 403; approver `POST /brief/approve` →
|
||||
200, `canSend: true` afterward. `GET /me` → `[]` for drafter,
|
||||
`["brief:approve","brief:reject","brief:send"]` for approver.
|
||||
|
||||
## Out of scope
|
||||
|
||||
PRD-0002 P2 (data-scoping, PII/BSN redaction) and P3 (step-up, break-glass, audit
|
||||
log) — separate future WPs. The Behandeling/backoffice app and `medewerker`
|
||||
log) — separate future WPs. The Behandeling/backoffice app and a `medewerker`
|
||||
`Principal` variant (ADR-0002 — no second actor exists yet, still YAGNI). Real
|
||||
AD/OIDC integration (identity provider stays simulated).
|
||||
AD/OIDC integration (identity provider stays simulated). The `auth/domain/session.ts`
|
||||
`Session → Principal` rename (see the Decisions deviation above — ADR-0002 defers
|
||||
it explicitly).
|
||||
|
||||
## Risks
|
||||
|
||||
Threading `Principal` through `SessionStore`/`auth.guard.ts` touches the one
|
||||
authenticated-session seam every route depends on — keep the observable shape
|
||||
(`isAuthenticated(): boolean`) identical so no route wiring needs to change, only
|
||||
what's inside `Session`/`Principal`. Backend `Authz.Can` replacing inline `X-Role`
|
||||
reads must preserve the existing 403 `Outcome.Forbidden` mapping
|
||||
(`Program.cs:330-335`) exactly, or `BriefEndpointTests.cs` breaks.
|
||||
`Send` was already unauthenticated/unauthorized before this WP (no role check on
|
||||
`POST /brief/send`) — `Authz.CanActOn(Send, …)` preserves that exactly
|
||||
(`=> true`, a mechanical dispatch step) rather than silently introducing a new gate
|
||||
that would have broken the existing `Send_only_from_approved` test (which calls
|
||||
`send` as the default drafter identity and expects success). If a future WP decides
|
||||
`send` should be approver-only, that's a deliberate behavior change, not a bug fix.
|
||||
Every brief mutation endpoint now returns `BriefViewDto` instead of bare `BriefDto`
|
||||
— a wire-shape change; the generated `api-client.ts` was regenerated and every FE
|
||||
call site updated, but any other caller of these endpoints outside this repo would
|
||||
need the same update.
|
||||
|
||||
Reference in New Issue
Block a user