Repository navigation
F27: Roles + scoped permissions — role model, flock-scoped workers, RBAC UI - #104
Conversation
…BAC UI (#103) Spec §5.1–5.3, growing #73's binary Admin/Worker split into the real role model. #84 rides along: role names live once in Roles (Domain), policies map tiers not raw strings, the SPA gets a typed Role union. Roles shipped: Admin (owner — stored name kept for existing DBs and tokens), Manager, Sales, ReadOnly, and Worker (= no role row, the #73 convention). Vet/Consultant deferred (no health module to gate); house-level scoping deferred (houses aren't entities) — the assignment schema carries the nullable house/farm columns per §5.2 so nothing churns later. Policy tiers (AuthPolicies.AddCluckworkPolicies): - AdminOnly (historic name) = Owner + Manager — every pre-#103 corrective/config/money gate now admits Managers - OwnerOnly = user management - SalesAccess = Owner/Manager/Sales — payments move here from admin-only (spec §5.1: Sales handles payments); expenses, money reports, audit, and export stay Owner/Manager - SalesFlow = everyone but ReadOnly — workers keep selling (#73 principle); applied to customers + the draft→confirm order flow - ProductionWrite = Owner/Manager/Worker — Sales and ReadOnly are fenced off production capture (entries, water, feed, purchases, flock create) Flock scoping (spec §5.3): UserRoleAssignment rows narrow a worker to assigned flocks — no rows = account-wide (grandfathering every existing worker), the first row restricts, farm/house-wide rows grant all in the single-farm MVP. IFlockScopeGuard enforces in the record daily entry / submit / water / feed paths (elevated roles and non-HTTP system callers skip). Owner-only assignment CRUD under /users/{id}/flock-assignments, audited (User.FlockAssign/Unassign). Identity: CreateUserAsync takes any assignable role; ListUsers reports the actual role; the seeder creates all four roles up front. ICurrentUser carries the token's roles. SPA: role picker (5 roles), worker flock-assignment drill-down on Users, nav tiered per role (Sales/ReadOnly lose production links, ReadOnly loses sales links, Users is Owner-only). GLOSSARY roles entry + Help "Who can do what" rewritten. Tests (+5 → 291): Manager corrective-tier-but-no-user-mgmt; Sales sells/settles but 403s production+expenses+audit; ReadOnly reads everything writes nothing; worker flock scoping full round-trip (unscoped → assign narrows + audit row + duplicate 409 → unassigned flock 422 FlockScope.NotAssigned on entry AND water → elevated roles unscoped → removal restores); assignment tenant isolation. Closes #103. Closes #84.
Both reviewers attacked the authz rewiring; every actionable finding applied: - Multi-role precedence (codex High / panel Medium): the negative- role assertions inverted under principals holding several roles (Owner+ReadOnly locked out of the sales flow; Manager+Sales inside production). Policies now resolve ONE effective role — highest wins (Owner > Manager > Sales > ReadOnly) — and use it consistently. A principal carrying only unrecognized role names is denied outright, never treated as a worker (Worker = zero role claims); both policies also RequireAuthenticatedUser so a future route gated only by them can't be anonymous-writable. This also defuses the seeder's promote-to-Admin multi-role landmine. - Sales could VOID payments (panel Medium, best catch of the round): voiding is the corrective tier everywhere else — a Sales user could record a cash payment, pocket the cash, and void their own record with no Owner/Manager in the loop. VoidPayment now carries route-level Owner/Manager; record/list/balances stay Sales-tier. SPA matches: Sales sees and records payments, the void button is Owner/Manager. - Unresolved-user guard bypass (codex Medium): an authenticated principal whose token lacks a parseable sub now gets 401 in the middleware instead of flowing on unresolved — the flock-scope guard's system-caller pass-through is provably unreachable over HTTP. - Mismatched unassign (codex Medium): DELETE now requires the assignment to belong to the route's user — a wrong pairing can no longer delete another worker's last assignment and silently widen them to account-wide. - Test gaps (codex Low): +3 tests — multi-role precedence (Owner+ReadOnly sells and records; Sales+ReadOnly still fenced; unknown "Contractor" role denied everywhere), Sales records-but- cannot-void a payment, and the remaining scope surfaces (feed usage guarded, out-of-scope DRAFT cannot be submitted after narrowing, mismatched unassign 404s and deletes nothing). 294 tests green.
|
Review round: codex + internal adversarial-authz agent (pi endpoint down, 6th consecutive PR). An authz rewiring got the strongest gauntlet of the session — the internal agent ran an exhaustive write-route sweep (every MapPost/MapPut/MapDelete vs its effective policy) and cleared tenant isolation, token/claims handling, idempotency-vs-authz ordering, and the flock-scope mechanics. What survived both reviewers, all applied (
Declined with rationale: Sales/ReadOnly production reads stay open (repo-wide reads-open convention; the §5.1 matrix governs writes) and workers keep flock creation (pre-existing behavior; nothing to scope a new flock to — a scoped worker still can't record against it without an assignment). |
Four reviewers on the post-fix head. The fix-regression pass found no regressions; the rest converged on one real hole, a SPA bug, and a coverage/doc batch. - Unknown-role read hole (codex High): my EffectiveRole comment said unknown roles are "denied outright", but that only held for the write policies — a principal whose only role is unrecognized could still READ through the bare-auth production groups. The default authorization policy now also requires EffectiveRole != Denied, so an unknown-role principal is refused everywhere, reads included. Worker (no role claims) and every known role still pass. claims.ts mirrors it (unknown → "Denied", not "Worker"). - Assign-flock preselect bug (conventions red): the Users page initialized the assignment dropdown from the UNFILTERED flock list, so an inactive first flock preselected an id the dropdown didn't show and Assign 404'd. Initialize from the active list. - ListUsers precedence (codex + conventions): multi-role users were collapsed by insertion order (g.First()); now highest-role-wins, matching EffectiveRole — an Owner+ReadOnly lists as Owner. - AssignFlock validation (codex): empty flock id returns a 400 validation problem instead of a downstream 404. - Docs (codex): the payment glossary/help text still said "admin-only end to end" and the old two-role model — updated to the Sales-records / Owner-Manager-voids reality and the five-role Users screen. Tests (+2 → 296) closing the negative-space gaps both external reviewers flagged: Manager CAN void a sale and a payment (the widening's whole point); Sales is refused VoidSale (not just VoidPayment); a plain Worker is refused across the corrective + money-config surface (users/audit/export/expense-categories/reports /both voids/grade+expense writes); ReadOnly refused the money routes (balances + record payment); Manager+Sales resolves to Manager (production allowed) and Sales+ReadOnly still sells (SalesFlow success side); unknown-role refused the open production reads too. Verified-false and declined: pi's "duplicate-assign surfaces as 500" (the global handler maps DbUpdateException → 409) and "reports money routes may be ungated" (sales/expenses/profit each carry route-level AdminOnly).
|
Second review round — four reviewers on the post-fix head ( Applied (
Tests (+2 → 296) closing the negative-space gaps both external reviewers flagged: Manager can void a sale and a payment (the widening's whole point, previously unverified); Sales refused Verified-false / declined: pi's "duplicate-assign → 500" (the global handler maps |
Closes #103. Closes #84 (batched — role/claim constants, typed SPA role union, tier-named policies). Spec §5.1–5.3, growing #73's Admin/Worker split into the real role model. This is the LAST feature slice of epic #14 before #45 (i18n).
The matrix
Deferred with rationale (in GLOSSARY): Vet/Consultant (no health module to gate — it would be Read-only with a misleading name) and house-level scoping (houses aren't entities; the §5.2 assignment schema carries the nullable columns so nothing churns later).
One deliberate money-rule evolution to flag: payments move from admin-only to Owner/Manager/Sales per spec §5.1 ("Sales: customers, sales, payments"). Customer balances move with payments (they are the settlement view the Sales role works from); expenses, money reports, audit, and export stay Owner/Manager. If you want payments back behind Owner/Manager until a real Sales user exists, it's a one-line policy change.
Flock scoping (spec §5.3)
UserRoleAssignmentrows narrow a worker: no rows = account-wide (grandfathering every existing #73 worker), the first row restricts to listed flocks, farm/house-wide rows grant all in the single-farm MVP.IFlockScopeGuardenforces inside the record-entry / submit / water / feed handlers (elevated roles skip; non-HTTP system callers like the demo seeder are account-level by definition). Owner-only CRUD under/users/{id}/flock-assignments, audited (User.FlockAssign/Unassign— audit filter updated).Policy tiers (#84)
AuthPolicies.AddCluckworkPolicies()—AdminOnly(Owner+Manager, historic name kept so 30+ existing gates didn't churn),OwnerOnly,SalesAccess,SalesFlow(everyone but ReadOnly — workers keep selling, the #73 principle),ProductionWrite(Owner/Manager/Worker). Role names live once inDomain.Accounts.Roles; the SPA gets a typedRoleunion and role-tiered nav.Tests (+5 → 291)
Manager corrective-tier-but-403-on-users; Sales sells/settles but 403s production+expenses+audit; ReadOnly reads-everything-writes-nothing; the full worker flock-scoping round-trip (unscoped records anywhere → assignment narrows with audit row + duplicate 409 → unassigned flock 422
FlockScope.NotAssignedon entries AND water → elevated roles unscoped → removal restores account-wide); assignment tenant isolation (cross-tenant user/flock both 404).Runtime drill (Playwright, 8/8)
Owner: Users page shows the 5-role picker → worker row expands to assignments ("account-wide access") → assign flock → listed → remove → restored, no error state. Worker: Daily entry + Sales nav present, Users absent.