Files
baya-monorepo/dev/post-phase/hardening/hardening-phase-1-admin-rbac.md
T
2026-07-17 13:22:04 +03:30

113 lines
7.3 KiB
Markdown

# Hardening Phase 1 — Admin RBAC & staff role scopes (make role checks real)
> Resurrect the dead `DynamicPermission` policy so the seeded admin personas can actually operate the
> backoffice, expose the already-built role-management handlers over HTTP (delivering REQ-031), and
> narrow the over-broad `BookingRoles.Admin` bucket that hands Support/Moderation staff clinical and
> financial access. This is the server half of *"roles aren't checked."*
> **Track:** backend · **Depends on:** — (parallel-safe with Phase 0) · **Unlocks:** Phase 4
> **Before you start, read [_shared/agent-operating-rules.md](../../phases/_shared/agent-operating-rules.md).**
## 1. Context — where this sits
Fixes **H-04, H-05** from [issues.md](issues.md). Every admin endpoint is gated by
`[Authorize(ConstantPolicies.DynamicPermission)]`, whose handler passes only for the literal role
`"admin"` or a matching RoleClaim — but no user holds `"admin"`, no RoleClaim is ever seeded, and the
only claim-writing service has no controller. Net effect: the seeded `super_admin` (09120000020) and
`finance` (09120000021) personas — which RUNBOOK.md documents as the sanctioned admin path — get 403 on
**every** admin console action. Separately, handler-level role sets are the entire authz boundary for
bookings (controllers carry blanket `[Authorize]`), and `BookingRoles.Admin` bundles five staff roles
into clinical-notes/nurse-balance/forced-transition access, violating the codebase's own
"narrowest fitting scope" convention.
**What already exists (do not rebuild):** the `DynamicPermissionHandler`/`DynamicPermissionService`
wiring (`ServiceCollectionExtension.cs:49-50,100-103`), `RoleManagerService.ChangeRolePermissionsAsync`
(`RoleManagerService.cs:145-208` — the claim writer), the Application-layer
`Features/{Role,Admin}` commands/queries (handlers exist, no HTTP surface), the demo seeder
(`DemoWorldSeeder.cs:279-308`), tenancy checks in handlers (correct — leave alone), and
`Baya.Application/Common/StaffRoles.cs` + `Features/PatientCareRecords/PatientAccess.cs:17` (the
narrow-scope pattern to mirror).
## 2. Required reading (do this first)
- [issues.md](issues.md) H-04/H-05 — evidence with file:line; don't re-audit.
- `DynamicPermissionService.cs`, `RoleNames.cs`, `RoleManagerService.cs`,
`DemoWorldDefinitions.cs`/`DemoWorldSeeder.cs`, `Features/Bookings/BookingRoles.cs` and the three
handlers listed in H-05, `Common/StaffRoles.cs`, `PatientAccess.cs`.
- `dev/post-phase/refinement/RUNBOOK.md:115-130` and
`dev/shared-working-context/reports/refinement-phase-2-report.md` — the docs that currently
overstate what works (you will correct them).
- `server/CLAUDE.md` + `server/CONVENTIONS.md` (patterns, zero-new-warnings gate).
- REQ-031 in `dev/shared-working-context/frontend/requests/for-backend.md` (the RBAC endpoints the
frontend's `/admin/roles` grid expects).
## 3. Scope — build this
1. **Make DynamicPermission passable.**
- `DynamicPermissionService.CanAccess`: treat `RoleNames.SuperAdmin` (and `RoleNames.Admin`) as a
full bypass — mirroring how the rest of the codebase treats super_admin — instead of the single
literal `"admin"`.
- **Seed RoleClaims** for the fine-grained roles (`finance`, `support`, `moderation`) mapping each
to the controller/action set it should reach (finance → payouts/refunds/invoices/BNPL admin;
support → tickets/alerts; moderation → reviews queue). Seed via the same idempotent mechanism the
demo seeder uses (or a migration `HasData` if that's the house style — check how roles themselves
are seeded). A fresh clone must have a working admin console out of the box.
2. **Expose role management over HTTP (REQ-031).** Add a `RolesController` (or `AdminRolesController`)
under `Controllers/V1` wiring the existing `Features/Role` handlers: list roles (+ their claims),
grant/revoke a user role, update role permissions. Gate it to `super_admin` only. Follow the house
controller conventions (snake_case routes, ApiResult envelope, versioning). Mark REQ-031 delivered
in `for-backend.md`.
3. **Split `BookingRoles.Admin`.** Introduce purpose-specific sets (suggested:
`BookingRoles.ClinicalAccess = [Admin, SuperAdmin]`,
`BookingRoles.Financial = [Admin, SuperAdmin, Finance]`, keep a `Staff` set where genuinely all
staff belong, e.g. read-only booking lookups for ticket context). Update the users:
`GetCareInstructionsQuery` (clinical), `TransitionBookingStatusCommand` (financial — it arms the
payout trigger), `GetNursePayableBalanceQuery` (financial), and review the remaining
`BookingRoles.Admin` consumers (`CancelSessionCommand`, `CancelBookingCommand`,
`SubmitCareInstructionsCommand`, `GetVisitVerificationQuery`, `ListBookingsQuery`,
`GetBookingDetailQuery`, `GetBookingRequestQuery`) — assign each the narrowest set that matches
what it exposes. Justify each choice in the report.
4. **Docs honesty.** Correct `RUNBOOK.md` + the refinement-phase-2 report claims about the admin
personas; document the finance/support/moderation scopes somewhere durable (server/CLAUDE.md or a
product note).
## 4. Mocks & seams in this phase
None. This is authorization wiring only — no vendor seams, no schema beyond possible RoleClaim seed
rows.
## 5. Critical rules you must not get wrong
- **Do not weaken anything:** endpoints currently gated by DynamicPermission must stay gated; you are
making the policy *satisfiable*, not optional. No `[AllowAnonymous]` anywhere in this phase.
- Tenancy behavior (404-not-403 on cross-tenant) is correct today — don't touch those handlers except
for the role-set swap.
- The role-claim seed must be **idempotent** (the seeder runs on every Dev boot) and must not touch
production posture (respect the refinement-phase-5 config guard patterns).
- Support/Moderation must lose clinical + financial access, but must NOT lose what tickets genuinely
need (booking summary context for a thread). Check the ticket flows before narrowing a shared query.
- Zero new build warnings; enum/role names are stable wire contracts — don't rename `super_admin` etc.
## 6. Definition of Done
- Integration tests (Testing env, house `WebApplicationFactory` pattern): super_admin 200s on a
representative endpoint of every admin console family; finance 200s on payouts + 403s on
verification decide; support 403s on `GET bookings/{id}/care_instructions` and on
`TransitionBookingStatus`; moderation 200s on the review queue only. Grant/revoke via the new
controller round-trips.
- `dotnet build Baya.sln` 0 new warnings · `dotnet test Baya.sln` green (all prior tests + new).
- REQ-031 marked delivered; RUNBOOK/report corrections committed.
## 7. How to test (human)
1. Boot per RUNBOOK; log in as 09120000020 (super_admin) via dev OTP.
2. Call `GET api/v1/platform_configs`, the payout batches list, the moderation queue — all 200.
3. Log in as 09120000021 (finance): payouts 200; verification decide → 403.
4. Grant `moderation` to a fresh user via the new RolesController; verify the queue opens for them.
## 8. Hand off & document
- Tick H-04/H-05 in [issues.md](issues.md) with the commit hash.
- Update `for-backend.md` (REQ-031), `server/CLAUDE.md` (authz model paragraph), RUNBOOK.
- Write `dev/shared-working-context/reports/hardening-phase-1-report.md` + a backend handoff note if
Phase 4 runs as a separate agent.