Skip to content

T 107


uid: T-107 title: RBAC cleanup β€” drop the legacy role half from the badge dual-gate after T-022 step 7 bakes status: dropped area: auth-rbac created: 2026-06-24 updated: 2026-07-02 succeeds: T-022 folded_into: T-025


DROPPED 2026-07-02 β€” folded into T-025 (owner-confirmed). Not abandoned: the prerequisite (onboarding assigns badges) already shipped in PR #835, and the terminal removal (retire ROLE_PERMISSIONS/canPerform, strip hasAnyRole from the four firestore.*.rules + the page-rule role fields, drop canPerformActionOrLegacy) becomes T-025 phase 4 β€” because it's blocked on migrating the 11 live canPerform action-gate callers, which is T-025's work. Keeping it a separate task would have meant a confusing half-removed interim state + a second pass over the same files. See the 2026-07-02 decision-log entry below and the T-025 scope for the sequencing. Owner (verbatim, 2026-07-02): "Confirm folding T-107 into T-025 (document it please)". RBAC sequence: succeeds T-022 β€” only opens AFTER step 7's strict Security Rules have been live in prod long enough that no role-only access path is observed (default soak: 1-2 weeks of normal operation, zero PERMISSION_DENIED spikes attributable to a missing badge). Holds the badge-model migration as the single source of truth.

Why this exists

T-022 shipped the badge model as a dual-gate: every page rule, every requireRbac call site, every Firestore Security Rule capability admits the user via EITHER a matching badge OR a matching legacy role. That bridge was deliberate β€” it kept role-only users working through the migration window so a runtime mistake (missed claim, stale token, mis-targeted seed) couldn't lock anyone out.

Once step 7's strict rules have been live without incident for 1-2 weeks AND a dry-run of scripts/migrate-users-to-badges.ts reports zero diffs against every active user, the legacy half no longer carries any traffic. Leaving it in place is dead weight that hides what each capability actually depends on β€” and gives an opening to a future contributor to add a NEW role check "alongside the existing one" instead of reaching for a badge.

This task removes the legacy half completely:

  1. firestore.{aote-system,tebs-erl,tebs-mel,tebs-epl}.rules β€” drop every hasAnyRole([...]) / hasRole(...) clause from the capability functions (canReadProjects, canWriteAccountingErl, canReadCoaching, …). Each capability becomes "badge match only." The isAdmin() helper goes away too (its sole consumer was the catch-all admin gate, replaced by hasBadge('super-admin')).
  2. lib/rbac/pageRules.ts β€” strip requireAnyPermission / requireAllPermissions / allowedRoles / requiredSubsidiaries / requiredAnySubsidiary from DEFAULT_PAGE_RULES, keeping requireAnyBadgeGrant as the sole gate. Update evaluatePageRule to remove the case-B fallback (no legacy gates left to fall through to).
  3. lib/rbac/serverAuthApp.ts β€” same: drop the legacy-checks block; requireRbac becomes badge-only via canPerformAction semantics.
  4. lib/rbac/badges.ts β€” drop canPerformActionOrLegacy (no caller left); canPerformAction stays as the single API.
  5. lib/rbac/types.ts β€” drop Permission / ROLE_PERMISSIONS. The UserRole field itself stays on UserProfile for now (audit / display only β€” admins still want to see "what was this person's role"). A future cleanup can collapse it to a free-form position string once the reorg in T-094 / the owner's role+clearance discussion is fully digested.
  6. pages/api/auth/[...nextauth].ts β€” role keeps riding alongside badges in the session for audit, but no longer gates anything.
  7. Tests β€” drop canPerformAction.test.ts's canPerformActionOrLegacy suite + the pageRulesBadgeGate.test.ts fall-through cases. The drift guard (strictRulesBadgeDrift.test.ts) stays.
  8. Runbook β€” append the cleanup outcome to docs/runbooks/t022-step7-strict-rules-flip.md so the historical context is preserved next to the flip.

Why NOT do this in T-022 itself

Two reasons: - Safety: the dual-gate is the rollback path during the migration window. Dropping it before step 7 has baked means a runtime mistake locks people out with no fallback. The two-PR pattern (steps 1-6, then step 7) is good; the three-PR pattern (… then T-107) is better. - Reviewer attention: the T-022 PR is already big (6 steps in one). A focused delete-only-PR for the cleanup gets clearer scrutiny than "tail end of a big migration."

Entry criteria

This task only moves from todo to doing when ALL of these hold:

  • T-022 PR #797 (steps 1-6) merged.
  • T-022 PR #804 (step 7 β€” strict rules flip) merged.
  • scripts/migrate-users-to-badges.ts (dry-run on prod) reports zero diffs across every active user.
  • At least one calendar week of normal operation with the strict rules live, zero PERMISSION_DENIED spikes attributable to a missing badge in Sentry / Vercel logs.
  • RBAC_ENABLED=true set in Vercel and a redeploy has gone through.

Out of scope (deliberate)

  • The full retirement of UserRole as a profile field. The role + clearance question the owner has been thinking through (badges aren't the whole answer β€” there's still a "what's this person's job title" dimension) is a separate design call.
  • lib/directors/registry.ts consolidation into aote-system/users (called out as a follow-up in T-094's design entry β€” has WOPC pipeline blast radius and deserves its own task).

Decision log

2026-06-24 β€” task opened (cleanup placeholder for after step 7 bakes)

  • βœ… Attestation (User Management): read AGENTS.md; checked the board β€” no existing task covers the legacy-role-half cleanup; T-022 itself stays open through step 7's bake, then closes; T-094 is roster/management, T-095 is dropped, neither overlaps.
  • Source: User Management Β· https://claude.ai/code/session_01GGT5n9vCxKWoUQSRAfMSiW
  • Proposed by: User Management. Approved by: the owner (verbatim: "Proceed" β€” approving the explicit follow-up offer made in this session: "After step 7 lands and bakes, the natural follow-up is a separate T-NNN to drop the legacy role half from every capability helper and retire ROLE_PERMISSIONS β€” happy to open that task whenever you'd like.").
  • Why open it now, not at flip-time: the bridge's exit criteria can shift as the migration window unfolds; capturing them today (while the design context is fresh) is cheaper than re-deriving them weeks later when the agent doing the work won't have the same head state. The entry-criteria checklist above is the operator's gate; this file holds the design intent so whoever picks it up has the full picture.

2026-07-02 β€” readiness check + prerequisite fix (NOT the removal yet)

  • βœ… Attestation (User Management): read AGENTS.md; scope-checked β€” this is the T-107 prerequisite work, not the legacy-half removal itself. The removal stays gated on two criteria I can't verify from the cloud sandbox (see below).
  • Source: User Management Β· https://claude.ai/code/session_01GGT5n9vCxKWoUQSRAfMSiW
  • Trigger: a reviewing agent (EOP Local Assistance fork), relayed by the owner, flagged T-107 as unblocked ("soak is up, strict rules live since 06-25") and asked me to do the removal. Before removing the fallback I re-checked the entry criteria against live prod data β€” and found the removal is NOT yet safe.
  • What the live migration dry-run (2026-07-02, prod aote-system) showed β€” 5 users:
  • me@jefferochan.com (super_admin) β†’ ['super-admin'] βœ…
  • alisonhytang@gmail.com (admin) β†’ ['super-admin'] βœ…
  • jeffero.c@gmail.com (auditing_accountant) β†’ correct 3-badge bundle βœ…
  • jake@establishrecords.com (admin) β†’ holds all 7 badges (manually assigned via the picker; redundant since admin already carries super-admin, but harmless β€” NOT a lockout risk). The dry-run wants to strip the extras; do not --apply blanket, it would undo the owner's deliberate state.
  • waion1985@gmail.com (auditing_accountant) β†’ badges: [] ❗ A real user onboarded AFTER the 06-25 migration, with role-but-no-badge, passing strict rules only via the legacy role half. This is the exact user T-107's removal would lock out.
  • Root cause of the badge-less user: the invitation/onboarding path in pages/api/auth/[...nextauth].ts set role + subsidiaryAccess but never badges. The migration script was a one-time backfill; nothing taught onboarding to assign badges, so the badge model was not self-sustaining. Every post-migration user was riding the fallback T-107 removes.
  • Prerequisite fix shipped (this PR): wired mapUserToBadges(invitation.role, invitation.subsidiaryAccess) into all three onboarding write sites (returning-user invitation-activate, new-user invitation-activate, bootstrap super-admin). Purely additive β€” assigns badges, removes nothing, cannot lock anyone out. Now every newly onboarded/activated user is self-sustaining on badges. tsc clean; 89/89 RBAC tests.
  • Prerequisite data backfill (done): ran migrate-users-to-badges.ts --apply --uid=<waion1985> (targeted, so Jake's manual badges were untouched). waion1985 now holds ['auditing-accountant-erl','bookkeeper-epl','bookkeeper-mel'] + claims synced. Re-run dry-run confirms no active user is badge-less β€” Jake's extra-badges diff is the only remaining delta and is safe.
  • T-107 removal STILL BLOCKED on two owner-only gates (can't be checked from the cloud sandbox):
  • RBAC_ENABLED=true confirmed in the prod NAS container env (+ redeploy done). If it is still false, the legacy canPerform path is currently permissive (returns true for everyone), so removing canPerformActionOrLegacy's fallback would change server-side action gating in a way that should follow a deliberate flag flip, not precede it.
  • Zero PERMISSION_DENIED spikes in prod logs over the soak window attributable to a missing badge. Plus a soft consideration: badge claims only reach a user's active session token on the next ~60s JWT refresh (or re-sign-in) after the profile write. waion1985 was just backfilled, so give their session a refresh cycle before the fallback is pulled.
  • Recommendation: merge this prerequisite; then, once the owner confirms gates (1) + (2), the actual removal (canPerformActionOrLegacy β†’ canPerformAction, requireAnyPermission/ allowedRoles/hasAnyRole out of pageRules + serverAuthApp + the four firestore.*.rules, ROLE_PERMISSIONS retired) is a focused delete-only follow-up PR.

2026-07-02 β€” gates RESOLVED with data + code analysis; but removal is COUPLED to T-025

  • βœ… Attestation (User Management): read AGENTS.md; the owner asked me to "find my own answer" on the two gates rather than wait for confirmation. Did so; findings below reshape the plan.
  • Source: User Management Β· https://claude.ai/code/session_01GGT5n9vCxKWoUQSRAfMSiW
  • Owner evidence (verbatim, 2026-07-02): "the web app is now moved back to Vercel … please utilize the keys that you have … use them to go to places and find your own answer regarding RBAC_ENABLED and PERMISSION_DENIED."
  • Gate 2 (PERMISSION_DENIED) β€” RESOLVED, data-backed. Queried GCP Cloud Monitoring (firestore.googleapis.com/api/request_count by response_code, project aote-pms, minted a fresh token from the GOOGLE_* service-account creds) over 2026-06-23 β†’ 07-01, spanning the 06-25 strict-rules flip: 616,118 OK Β· 140 FAILED_PRECONDITION Β· 18 NOT_FOUND Β· ZERO PERMISSION_DENIED. The strict rules denied nothing. (The 140 FAILED_PRECONDITION are missing-composite-index errors β€” the IR56M/coaching index items other agents fixed β€” not RBAC.)
  • Gate 1 (RBAC_ENABLED) β€” can't read the raw flag, but proved it's NOT a hard dependency. The app is on Vercel now with no Vercel API token in the sandbox and no endpoint that exposes the flag, so its literal value is unreadable from here. But the code structure makes the value moot for T-107 safety:
  • Firestore rules enforce regardless of RBAC_ENABLED; the badge gate in serverAuthApp and the subsidiary gate are outside if (RBAC_ENABLED); the allowedRoles/requireAnyPermission page checks are inside it AND guarded by !badgeOk.
  • So with universal badge coverage (verified), page + data access already flows through badges irrespective of the flag; the role/permission checks T-107 removes are either not running (flag false) or skipped for badge-holders (flag true). Removing them is safe either way.
  • NEW finding β€” the removal is only PARTIALLY shippable; the rest is coupled to T-025. ROLE_PERMISSIONS / canPerform / roleHasPermission are NOT dormant β€” 11 API routes + several client components still call them for server-side action gating (accounting settings, coaching invoices/payments, projects/invoices CRUD). Retiring ROLE_PERMISSIONS would break them. canPerformActionOrLegacy, by contrast, has zero live callers (safe to drop).
  • Shippable now (safe, verified): hasAnyRole out of the four firestore.*.rules; allowedRoles/requireAnyPermission out of DEFAULT_PAGE_RULES + serverAuthApp; drop canPerformActionOrLegacy.
  • Blocked on T-025 phase 3: retiring ROLE_PERMISSIONS / canPerform β€” needs the 11 action-gate callers migrated to canPerformAction first.
  • Revised recommendation: do NOT do a partial legacy-removal now β€” it would leave a confusing mixed state (rules badge-only, but server action gates still role-based) and require a second pass. Instead fold T-107's terminal removal into T-025 as its final phase (phase 4 in the T-025 scope). Same cluster, one coherent migration, no interim ambiguity. T-107 stays todo as the named end-state; its prerequisite (onboarding badges) is already merged (PR #835).