Some checks failed
CI / storybook-a11y (push) Successful in 4m12s
CI / backend (push) Successful in 1m6s
CI / api-client-drift (push) Successful in 1m37s
CI / frontend (push) Successful in 1m31s
CI / codeql (csharp) (push) Failing after 1m51s
CI / codeql (javascript-typescript) (push) Failing after 1m24s
Gap analysis found the POC's designed-but-unbuilt strategic gaps: ABAC authorization (ADR-0002/PRD-0002 phase P1), no e2e coverage, unproven i18n second-locale seam, thin resilience seams (correlation-id, idempotency, retry), and in-memory-only persistence. Each WP is grounded in the current code (file paths + line numbers), not just the analysis. Also corrects PRD-0001's stale 'Proposed' status header — the Mijn aanvragen vertical is fully built. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
154 lines
9.1 KiB
Markdown
154 lines
9.1 KiB
Markdown
# WP-18 — ABAC capability spine (Principal + capabilities, phase P1)
|
||
|
||
Status: todo
|
||
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`
|
||
computes 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.
|
||
|
||
## 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)
|
||
|
||
## 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.
|
||
- **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.
|
||
|
||
## Files
|
||
|
||
- `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).
|
||
|
||
## Steps
|
||
|
||
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).
|
||
|
||
## Acceptance criteria
|
||
|
||
- [ ] `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).
|
||
|
||
## 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.
|
||
|
||
## 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`
|
||
`Principal` variant (ADR-0002 — no second actor exists yet, still YAGNI). Real
|
||
AD/OIDC integration (identity provider stays simulated).
|
||
|
||
## 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.
|