Some checks failed
CI / frontend (push) Successful in 1m22s
CI / backend (push) Successful in 1m3s
CI / api-client-drift (push) Successful in 1m36s
CI / storybook-a11y (push) Successful in 4m4s
CI / codeql (csharp) (push) Failing after 1m45s
CI / codeql (javascript-typescript) (push) Failing after 1m25s
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
179 lines
11 KiB
Markdown
179 lines
11 KiB
Markdown
# WP-18 — ABAC capability spine (Principal + capabilities, phase P1)
|
|
|
|
Status: done (7ec13d8)
|
|
Phase: 5 — productie-volwassenheid
|
|
|
|
## Why
|
|
|
|
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`
|
|
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 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 — 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.
|
|
- **The AD/OIDC identity provider stays simulated** (PRD-0002 §3 non-goal). The
|
|
`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 (as built)
|
|
|
|
- `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 (as executed)
|
|
|
|
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
|
|
|
|
- [x] `brief.store.ts` contains no `currentRole()` call and no FE-computed
|
|
permission boolean; `canApprove`/`canReject`/`canSend` come from the DTO.
|
|
- [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 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 a `medewerker`
|
|
`Principal` variant (ADR-0002 — no second actor exists yet, still YAGNI). Real
|
|
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
|
|
|
|
`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.
|