Replace alphaRbac feature flag with backend-driven RBAC config#7877
Replace alphaRbac feature flag with backend-driven RBAC config#7877thabofletcher merged 6 commits intomainfrom
Conversation
The frontend RBAC behavior was controlled by a static alphaRbac feature flag that defaulted to true in development, while the backend RBAC was off by default. This split-brain caused the RolesForm to show empty roles for users assigned via the legacy system, and potentially prevented page access due to a race condition in permission resolution. Replace the static flag with a backend-driven check: the Plus health endpoint now exposes rbac.enabled, and the frontend reads this to determine RBAC mode. One flag (FIDESPLUS__RBAC__ENABLED) now controls both frontend and backend behavior.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
…cation # Conflicts: # clients/admin-ui/src/types/api/index.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: Replace alphaRbac flag with backend-driven RBAC config
This is a clean, focused migration. The approach — reading rbac.enabled from the Plus health endpoint instead of a client-side feature flag — is the right call: it makes RBAC gating a server-side concern and removes a flag that developers would have to remember to toggle. The implementation is consistent across the affected files.
Summary of findings
Must fix (1)
HealthCheck.ts:rbacis typed as required (rbac: RBACStatus) but every call site in this PR accesses it with optional chaining (?.rbac?.enabled). If an older Plus server omits the field, TypeScript won't catch the mismatch. Declare itrbac?: RBACStatusto match usage. (See inline comment.)
Nits / suggestions (2)
-
features.slice.ts: Minor style inconsistency —rbacuses optional chaining while adjacenttcfuses direct access inside the same ternary guard. Optional chaining is the right approach given the type should be optional; just worth aligningtcfandfides_cloudfor consistency. -
viewer-assigned-system.cy.ts: The inlineHealthCheckstub object is a duplicate ofPLUS_HEALTH_RBAC_ENABLEDfromrbac-ui-management.cy.ts. Consider lifting it to a shared Cypress support/fixtures file to prevent drift.
What looks good
- The
plus_health.jsonfixture now correctly defaultsrbac.enabledtofalse— tests that don't opt in to RBAC will behave correctly without extra setup. CommonSubscriptions.tsxsimplification is correct:useFeatures().rbacis already implicitly gated on Plus being available, so the old!isPlusEnabledguard was redundant.selectRbacEnabledinplus.slice.tsis a clean, reusable selector that properly encapsulates the Plus health derivation.- Nav config
requiresRbacis threaded consistently throughconfigureNavRoute→configureNavGroups→useNav. - The
alphaRbacremoval fromflags.jsonis complete with no apparent stale references left.
🔬 Codegraph: connected (46363 nodes)
💡 Write /code-review in a comment to re-run this review.
Match the canonical format used by sibling status types (TCFStatus, FidesCloudStatus). The generated schema correctly marks enabled as optional since the backend Pydantic field has a default value.
2 participants
Summary
alphaRbacfrontend feature flag that caused a split-brain with the backend RBAC configrbac.enabledfrom the Plus health endpoint (/api/v1/plus/health)FIDESPLUS__RBAC__ENABLED) controls both frontend and backend RBAC modeCompanion PR
rbac.enabledin Plus health endpoint)Test plan
FIDESPLUS__RBAC__ENABLED=false(default): verify legacy Permissions tab is shown, owner user can see all pages, permissions work correctlyFIDESPLUS__RBAC__ENABLED=true: verify RBAC Roles tab is shown, role assignments reflect correctlyalphaRbacin the codebase