113 lines
7.3 KiB
Markdown
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.
|