ENG-3137: Fix viewer users unable to edit assigned systems

[thabofletcher]

Ticket ENG-3137

Description Of Changes

The RBAC management UI commit (8e9788e) introduced a read-only permission check in SystemInformationForm that was not gated behind the alphaRbac feature flag and only checked for the global SYSTEM_UPDATE scope. This caused two issues:

1. Ungated behavior change: The read-only check applied unconditionally, affecting all users regardless of whether RBAC was enabled — breaking the feature flag contract
2. Missing scope check: Even with RBAC enabled, viewers with assigned systems were locked out because the check didn't account for the per-system SYSTEM_MANAGER_UPDATE scope

Code Changes

• Gate the isReadOnly check behind flags.alphaRbac so it has no effect when RBAC is disabled
• Check both SYSTEM_UPDATE (global) and SYSTEM_MANAGER_UPDATE (per-system) scopes so viewers with assigned systems can still edit them
• Add Cypress regression test (viewer-assigned-system.cy.ts) with alphaRbac flag enabled, covering both scenarios:
  • Viewer with system_manager:update can edit assigned system (was broken)
  • Viewer without update scopes sees read-only form (still works)

Steps to Confirm

1. Disable the alphaRbac feature flag → system edit form should behave exactly as before (no read-only restrictions)
2. Enable the alphaRbac feature flag → viewer with system_manager:update scope can edit their assigned system
3. Enable the alphaRbac feature flag → viewer without update scopes sees "Read-only access" alert and disabled form
4. Run Cypress: npx cypress run --spec "cypress/e2e/systems/plus/viewer-assigned-system.cy.ts"

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
• UX feedback:
  • No UX review needed
• Followup issues:
  • No followup issues
• Database migrations:
  • No migrations
• Documentation:
  • No documentation updates required

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ready	Preview, Comment	Mar 25, 2026 4:58pm

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
fides-privacy-center	Ignored		Mar 25, 2026 4:58pm

[claude]

Code Review

The fix is clean and the intent is clear. Two issues worth noting:

Critical

None.

Suggestions

Per-system scope check is too coarse (SystemInformationForm.tsx:99): useHasPermission([SYSTEM_UPDATE, SYSTEM_MANAGER_UPDATE]) treats SYSTEM_MANAGER_UPDATE as a global boolean — if it exists anywhere in total_scopes, the form unlocks for every system the user visits, including ones they don't manage. The backend will correctly reject unauthorized saves, but the user will see an editable form and only get an error on submit. The right long-term fix is to compare the current system's fides key against the user's assigned systems list. This is likely out of scope for this regression fix, but worth tracking as a follow-up.

Nice to Have

Misleading // BUG: comments in the Cypress test (lines 61, 64): those comments label the assertions for the fixed behavior, which will cause confusion for anyone reading them later. A quick rename to // FIXED (ENG-3137): or just removing the prefix would help.

---

Overall the core fix is correct: gating isReadOnly behind flags.alphaRbac restores pre-RBAC behavior when the flag is off, and adding SYSTEM_MANAGER_UPDATE to the scope check unblocks assigned viewers. The Cypress coverage for both the positive and negative case is a good addition.

[greptile-apps]

Greptile Summary

This PR fixes two related RBAC bugs in SystemInformationForm: the read-only gate was being applied unconditionally (regardless of the alphaRbac feature flag), and only the global SYSTEM_UPDATE scope was checked, leaving viewers with per-system SYSTEM_MANAGER_UPDATE scope locked out of editing their assigned systems.

• SystemInformationForm.tsx: isReadOnly is now correctly computed as flags.alphaRbac && passedInSystem && !canUpdateSystems. useHasPermission already performs OR logic across the provided scopes, so passing both SYSTEM_UPDATE and SYSTEM_MANAGER_UPDATE is the right approach and correctly unblocks assigned-system editors.
• Cypress test: A new regression test in viewer-assigned-system.cy.ts covers both scenarios (viewer with/without system_manager:update) and uses the existing overrideFeatureFlag and assumeRole custom commands appropriately.
• Minor: The test uses raw string literals for scope values instead of ScopeRegistryEnum enum constants, which makes refactoring harder and removes compile-time safety.

Confidence Score: 5/5

• Safe to merge — the fix is minimal, targeted, and correctly restores the intended feature-flag contract without side effects.
• The logic change is a single-line guard plus an extra scope in the permission array. useHasPermission already has OR semantics, so the expanded check is semantically correct. The only open item is a stylistic improvement (using ScopeRegistryEnum constants in the test), which does not affect runtime correctness.
• No files require special attention.

Important Files Changed

Filename	Overview
clients/admin-ui/src/features/system/SystemInformationForm.tsx	Correctly gates isReadOnly behind flags.alphaRbac and expands the permission check to include SYSTEM_MANAGER_UPDATE alongside SYSTEM_UPDATE; useHasPermission already uses OR logic so the fix is sound.
clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts	Covers both the happy-path (viewer+system_manager:update can edit) and the restricted path (viewer without update scopes sees read-only); minor style issue — scope strings are hardcoded literals instead of ScopeRegistryEnum values.
changelog/7754-fix-viewer-assigned-system-readonly.yaml	Changelog entry is well-formed; describes the fix accurately.

_{Reviews (1): Last reviewed commit: "Add changelog entry for ENG-3137 fix" | Re-trigger Greptile}

[nrxsmith]

This makes sense to me.

We (And I really include myself there, I'll check if there are any Cardea gaps) should double check we don't have similar problems other user types.