blocker fix phases added
This commit is contained in:
@@ -0,0 +1,78 @@
|
||||
# Phase 01 — Admin can't do anything
|
||||
|
||||
**Blocker:** blockers.md § "Admin can't do anything" (the #1 leverage item — a large share of the other
|
||||
phases in this folder are gated behind it).
|
||||
**Depends on:** nothing.
|
||||
**Unlocks:** [02-reviews-moderation.md](02-reviews-moderation.md) outright; makes every other admin-facing
|
||||
phase (09, 10, 11, 14) independently testable via the seeded `super_admin`/`finance` demo accounts.
|
||||
|
||||
---
|
||||
|
||||
**Root cause.** `server/src/Infrastructure/Baya.Infrastructure.Identity/Identity/PermissionManager/DynamicPermissionService.cs:9` only bypasses for the literal role `"admin"`:
|
||||
```csharp
|
||||
if (user.IsInRole("admin")) { return true; }
|
||||
```
|
||||
`RoleNames` (`server/src/Core/Baya.Domain/Entities/User/RoleNames.cs`) defines `Admin`, `Support`, `Finance`,
|
||||
`Moderation`, `SuperAdmin` as **five sibling roles**, not a hierarchy — `super_admin` doesn't imply `admin`.
|
||||
The seeded demo accounts (`DemoWorldDefinitions.cs:142-146`) hold `super_admin` and `finance` — **never**
|
||||
the literal `admin` — so neither passes this check. The only path that ever grants the literal `admin` role
|
||||
is the config-gated bootstrap-admin (`SeedDataBase.cs:50-71`, needs `Seed:AdminUsername`/`Seed:AdminPassword`,
|
||||
neither set anywhere), and it's a username/password account that can't sign in through the phone-OTP web UI
|
||||
anyway.
|
||||
|
||||
The fallback branch (a `DynamicPermission` claim matching `"{area}:{controller}:"`) is **dead code** — nothing
|
||||
anywhere writes that claim to any role. The commands that could (`Features/Role/*`: `AddRoleCommand`,
|
||||
`UpdateRoleClaimsCommand`, `GetAllRolesQuery`, `GetAuthorizableRoutesQuery`) are fully implemented but never
|
||||
wired to a controller — confirmed zero references under `Controllers/V1/`.
|
||||
|
||||
**Why manual QA doesn't catch it:** `client/src/services/admin/constants.ts:9` — `USE_ADMIN_MOCK = true` —
|
||||
19 of 21 admin consoles are served from an in-browser mock and never call the API. Only `/fa/admin/tickets`
|
||||
and `/fa/admin/reviews` are wired to the real seam, and both visibly 403 today.
|
||||
|
||||
## The fix, in two parts
|
||||
|
||||
1. **Broaden the bypass** in `DynamicPermissionService.CanAccess` to also accept `RoleNames.SuperAdmin`
|
||||
(`super_admin` is definitionally the top role — this isn't a guess, it mirrors what the client's own
|
||||
permission matrix already assumes, see below):
|
||||
```csharp
|
||||
if (user.IsInRole(RoleNames.Admin) || user.IsInRole(RoleNames.SuperAdmin)) { return true; }
|
||||
```
|
||||
This alone closes BL-002 (the seeded `super_admin` demo account, `09120000020`, becomes fully functional)
|
||||
and fixes every admin action that only needs `super_admin`.
|
||||
|
||||
2. **Grant the `finance` role (and, for completeness, `support`/`moderation`, even though no demo account
|
||||
holds them yet) real `DynamicPermission` claims**, scoped to the consoles they own. Don't guess this
|
||||
mapping — it's already designed and reviewed, just not backed by anything: `client/src/hooks/capabilities.ts`
|
||||
encodes the exact intended matrix (`useAdminCapabilities`), and reading the *code* (not just its stale
|
||||
doc-comment) gives:
|
||||
- `finance` → `canRefund`, `canPayout`, `canConfig` → controllers **`AdminRefunds`**, **`AdminPayouts`**,
|
||||
**`PlatformConfig`**.
|
||||
- `support` → `canVerify`, `canManageAlerts`, `canManageTickets` → **`AdminVerifications`**,
|
||||
**`AdminVerificationStepTypes`**, **`SupportAlerts`**, **`AdminTickets`**.
|
||||
- `moderation` → `canModerate` → **`AdminReviews`**, **`Reviews`** (the latter only for its
|
||||
`PATCH {id}/status` action, which is separately method-gated).
|
||||
(Everything else — `AdminInvoices`, `AdminClawbacks`, `AdminCancellationPolicies`, `AdminBnpl`, `AdminGeo`,
|
||||
`Holidays`, `AdminCatalog`, `AdminBookingRequests`, `AdminEvv`, `AdminSearch`, `AdminPartnerCenters`,
|
||||
`Audit`, `InternalCenters` — stays `super_admin`/`admin`-only, matching "admin — everything except role
|
||||
management" from the same matrix.)
|
||||
|
||||
Concretely: add a step to `SeedDataBase.Seed()` (same file that already idempotently ensures all 7 roles
|
||||
exist) that, for each of those three roles, calls the already-built
|
||||
`IRoleManagerService.ChangeRolePermissionsAsync(new EditRolePermissionsDto { RoleId = role.Id, Permissions = [...] })`
|
||||
with the claim values `$":{controller}:"` (area is always empty today — no controller uses `[Area]`) for
|
||||
its owned controllers. This reuses existing, tested plumbing rather than hand-rolling `RoleClaim` inserts.
|
||||
`SeedDataBase` will need `IRoleManagerService` injected (it's already registered in DI for the identity
|
||||
assembly).
|
||||
|
||||
## Deliberately out of scope
|
||||
|
||||
Wiring the `Features/Role/*` commands to a real `AdminRolesController` so `super_admin` can grant/revoke
|
||||
roles through the UI. The client's own `/fa/admin/roles` screen is explicitly marked "DEFERRED-IF-MISSING"
|
||||
with its own banner (`admin/roles/page.tsx:2-9`) and served by the mock on purpose — leave it deferred; the
|
||||
seeding approach above makes the two blockers real without needing that screen built.
|
||||
|
||||
## Bonus, required by forgotten-features.md
|
||||
|
||||
"The nurse-verification approve/reject buttons point at server actions that don't exist yet" is real and
|
||||
separate from the RBAC bug — see [09-nurse-verification-badge.md](09-nurse-verification-badge.md) for the
|
||||
two missing endpoints.
|
||||
Reference in New Issue
Block a user