From 895a04127a16949cbf825ee3f39ad9e2251eea68 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Wed, 25 Mar 2026 08:45:20 -0700 Subject: [PATCH 1/5] ENG-3137: Fix viewer users unable to edit assigned systems The RBAC commit 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 viewer users with assigned systems to see a read-only form, even though they should be able to edit via SYSTEM_MANAGER_UPDATE. - Gate the read-only check behind the alphaRbac feature flag so it has no effect when RBAC is disabled - Check both SYSTEM_UPDATE and SYSTEM_MANAGER_UPDATE scopes so viewers with assigned systems can still edit them - Add Cypress regression test for the scenario Co-Authored-By: Claude Opus 4.6 (1M context) --- .../systems/plus/viewer-assigned-system.cy.ts | 86 +++++++++++++++++++ .../features/system/SystemInformationForm.tsx | 15 +++- 2 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts diff --git a/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts b/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts new file mode 100644 index 00000000000..6d7f1f7155b --- /dev/null +++ b/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts @@ -0,0 +1,86 @@ +/** + * Regression test for ENG-3137: Viewer user can't edit a system assigned to them + * + * A viewer user with an assigned system should be able to edit that system. + * The RBAC commit (8e9788e) introduced a read-only check that only looks at + * the global SYSTEM_UPDATE scope, ignoring SYSTEM_MANAGER_UPDATE which + * allows per-system editing for assigned systems. + */ +import { + stubDatasetCrud, + stubPlus, + stubSystemCrud, + stubSystemIntegrations, + stubSystemVendors, + stubTaxonomyEntities, +} from "cypress/support/stubs"; + +import { SYSTEM_ROUTE } from "~/features/common/nav/routes"; +import { RoleRegistryEnum } from "~/types/api"; + +describe("ENG-3137: Viewer with assigned system should be able to edit", () => { + beforeEach(() => { + cy.login(); + stubSystemCrud(); + stubTaxonomyEntities(); + stubPlus(true); + cy.intercept("GET", "/api/v1/system", { + fixture: "systems/systems.json", + }).as("getSystems"); + cy.intercept({ method: "POST", url: "/api/v1/system*" }).as( + "postDictSystem", + ); + cy.intercept("/api/v1/config?api_set=false", {}); + stubDatasetCrud(); + stubSystemIntegrations(); + stubSystemVendors(); + }); + + it("viewer with system_manager:update can edit assigned system", () => { + // Simulate a viewer who also has system_manager:update for their assigned system + cy.fixture("login.json").then((body) => { + const { id: userId } = body.user_data; + cy.intercept(`/api/v1/user/${userId}/permission`, { + body: { + id: userId, + user_id: userId, + roles: [RoleRegistryEnum.VIEWER], + total_scopes: [ + // Standard viewer scopes + "system:read", + "system_manager:read", + "system_manager:update", + // Other viewer scopes + "user:read", + "organization:read", + ], + }, + }).as("getUserPermission"); + }); + + cy.visit(`${SYSTEM_ROUTE}/configure/demo_analytics_system`); + cy.wait("@getUserPermission"); + + // The form should NOT be read-only for a viewer with system_manager:update + cy.getByTestId("input-name").should("exist"); + + // BUG: The read-only alert should NOT appear + cy.contains("Read-only access").should("not.exist"); + + // BUG: The form fields should be editable, not disabled + cy.get("fieldset[disabled]").should("not.exist"); + }); + + it("viewer WITHOUT system_manager:update sees read-only form", () => { + // Standard viewer without system_manager:update + cy.assumeRole(RoleRegistryEnum.VIEWER); + + cy.visit(`${SYSTEM_ROUTE}/configure/demo_analytics_system`); + + cy.getByTestId("input-name").should("exist"); + + // This viewer SHOULD see read-only since they have no update permissions + cy.contains("Read-only access").should("exist"); + cy.get("fieldset[disabled]").should("exist"); + }); +}); diff --git a/clients/admin-ui/src/features/system/SystemInformationForm.tsx b/clients/admin-ui/src/features/system/SystemInformationForm.tsx index 26a901adebb..6283ecfd6b3 100644 --- a/clients/admin-ui/src/features/system/SystemInformationForm.tsx +++ b/clients/admin-ui/src/features/system/SystemInformationForm.tsx @@ -6,6 +6,7 @@ import { useMemo } from "react"; import * as Yup from "yup"; import { useAppDispatch, useAppSelector } from "~/app/hooks"; +import { useFlags } from "~/features/common/features"; import { CustomFieldsList, useCustomFields, @@ -86,10 +87,16 @@ const SystemInformationForm = ({ }: Props) => { const { data: systems = [] } = useGetAllSystemsQuery(); const { plus: systemGroupsEnabled } = useFeatures(); - - // Check if user has permission to update systems - const canUpdateSystems = useHasPermission([ScopeRegistryEnum.SYSTEM_UPDATE]); - const isReadOnly = passedInSystem && !canUpdateSystems; + const { flags } = useFlags(); + + // Check if user has permission to update systems (only when RBAC is enabled) + // Checks both global SYSTEM_UPDATE and per-system SYSTEM_MANAGER_UPDATE + // so that viewers with assigned systems can still edit them + const canUpdateSystems = useHasPermission([ + ScopeRegistryEnum.SYSTEM_UPDATE, + ScopeRegistryEnum.SYSTEM_MANAGER_UPDATE, + ]); + const isReadOnly = flags.alphaRbac && passedInSystem && !canUpdateSystems; const dispatch = useAppDispatch(); From 999f206102ad182db193c8e06e45e3ff76b17564 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Wed, 25 Mar 2026 09:06:06 -0700 Subject: [PATCH 2/5] Enable alphaRbac flag in Cypress regression test Without this, the test passes vacuously because isReadOnly is gated behind flags.alphaRbac which defaults to false in the test environment. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../cypress/e2e/systems/plus/viewer-assigned-system.cy.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts b/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts index 6d7f1f7155b..0398e6630d9 100644 --- a/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts +++ b/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts @@ -21,6 +21,7 @@ import { RoleRegistryEnum } from "~/types/api"; describe("ENG-3137: Viewer with assigned system should be able to edit", () => { beforeEach(() => { cy.login(); + cy.overrideFeatureFlag("alphaRbac", true); stubSystemCrud(); stubTaxonomyEntities(); stubPlus(true); From 556cf47933f71884b0af2bb39818e6ea489c4b5e Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Wed, 25 Mar 2026 09:07:18 -0700 Subject: [PATCH 3/5] Add changelog entry for ENG-3137 fix Co-Authored-By: Claude Opus 4.6 (1M context) --- changelog/7754-fix-viewer-assigned-system-readonly.yaml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/7754-fix-viewer-assigned-system-readonly.yaml diff --git a/changelog/7754-fix-viewer-assigned-system-readonly.yaml b/changelog/7754-fix-viewer-assigned-system-readonly.yaml new file mode 100644 index 00000000000..a9cc6c0d414 --- /dev/null +++ b/changelog/7754-fix-viewer-assigned-system-readonly.yaml @@ -0,0 +1,4 @@ +type: Fixed +description: Fixed viewer users being unable to edit systems assigned to them due to an ungated read-only permission check +pr: 7754 +labels: [] From ac18bc826a7a436096f439e581025bc88d235020 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Wed, 25 Mar 2026 09:18:15 -0700 Subject: [PATCH 4/5] Fix import sort order in SystemInformationForm Co-Authored-By: Claude Opus 4.6 (1M context) --- clients/admin-ui/src/features/system/SystemInformationForm.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/admin-ui/src/features/system/SystemInformationForm.tsx b/clients/admin-ui/src/features/system/SystemInformationForm.tsx index 6283ecfd6b3..422010d98e6 100644 --- a/clients/admin-ui/src/features/system/SystemInformationForm.tsx +++ b/clients/admin-ui/src/features/system/SystemInformationForm.tsx @@ -6,12 +6,12 @@ import { useMemo } from "react"; import * as Yup from "yup"; import { useAppDispatch, useAppSelector } from "~/app/hooks"; -import { useFlags } from "~/features/common/features"; import { CustomFieldsList, useCustomFields, } from "~/features/common/custom-fields"; import { LegacyResourceTypes } from "~/features/common/custom-fields/types"; +import { useFlags } from "~/features/common/features"; import { useFeatures } from "~/features/common/features/features.slice"; import { ControlledSelect } from "~/features/common/form/ControlledSelect"; import { CustomSwitch, CustomTextInput } from "~/features/common/form/inputs"; From be19f513a7391859b1cb1a4b2c67f7fc68207e1f Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Wed, 25 Mar 2026 09:19:10 -0700 Subject: [PATCH 5/5] Address review comments on regression test - Use ScopeRegistryEnum instead of hardcoded scope strings - Replace misleading "BUG:" comment labels with "Regression guard" Co-Authored-By: Claude Opus 4.6 (1M context) --- .../systems/plus/viewer-assigned-system.cy.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts b/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts index 0398e6630d9..21222a347d4 100644 --- a/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts +++ b/clients/admin-ui/cypress/e2e/systems/plus/viewer-assigned-system.cy.ts @@ -16,7 +16,7 @@ import { } from "cypress/support/stubs"; import { SYSTEM_ROUTE } from "~/features/common/nav/routes"; -import { RoleRegistryEnum } from "~/types/api"; +import { RoleRegistryEnum, ScopeRegistryEnum } from "~/types/api"; describe("ENG-3137: Viewer with assigned system should be able to edit", () => { beforeEach(() => { @@ -48,12 +48,12 @@ describe("ENG-3137: Viewer with assigned system should be able to edit", () => { roles: [RoleRegistryEnum.VIEWER], total_scopes: [ // Standard viewer scopes - "system:read", - "system_manager:read", - "system_manager:update", + ScopeRegistryEnum.SYSTEM_READ, + ScopeRegistryEnum.SYSTEM_MANAGER_READ, + ScopeRegistryEnum.SYSTEM_MANAGER_UPDATE, // Other viewer scopes - "user:read", - "organization:read", + ScopeRegistryEnum.USER_READ, + ScopeRegistryEnum.ORGANIZATION_READ, ], }, }).as("getUserPermission"); @@ -65,10 +65,10 @@ describe("ENG-3137: Viewer with assigned system should be able to edit", () => { // The form should NOT be read-only for a viewer with system_manager:update cy.getByTestId("input-name").should("exist"); - // BUG: The read-only alert should NOT appear + // Regression guard (ENG-3137): read-only alert should NOT appear cy.contains("Read-only access").should("not.exist"); - // BUG: The form fields should be editable, not disabled + // Regression guard (ENG-3137): form fields should be editable, not disabled cy.get("fieldset[disabled]").should("not.exist"); });