From 51cff0775373e41c7ac8b9b592aa10fd9ad96850 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 10 Jul 2026 05:23:57 -0400 Subject: [PATCH] fix(management): reconcile area role gate to Designer|Deployer across actor and all three docs (arch-review C6) Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj --- docs/requirements/Component-CentralUI.md | 3 ++- .../Component-ManagementService.md | 3 ++- docs/requirements/Component-Security.md | 5 +++- .../ManagementActor.cs | 18 ++++++++++--- .../ManagementActorTests.cs | 27 +++++++++++++++++++ 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/docs/requirements/Component-CentralUI.md b/docs/requirements/Component-CentralUI.md index f358c286..eaeaefe5 100644 --- a/docs/requirements/Component-CentralUI.md +++ b/docs/requirements/Component-CentralUI.md @@ -103,7 +103,8 @@ Central cluster only. Sites have no user interface. - `Export Bundle` (`/design/transport/export`) — multi-step wizard for exporting a `.scadabundle` artifact containing templates, shared scripts, external systems, and central-only configuration. Visible to users with `RequireDesign`. - `Import Bundle` (`/design/transport/import`) — multi-step wizard for uploading and applying a `.scadabundle` into the current environment, with per-artifact diff review and conflict resolution. Visible to users with `RequireAdmin`. -### Area Management (Admin Role) +### Area Management (Designer or Deployer Role) +- Areas are authored inside the Deployment Topology workflow (`Topology.razor`, `RequireDeployment`); managing them is gated **any-of [Designer, Deployer]**, not Administrator (arch-review C6). - Define hierarchical area structures per site. - Parent-child area relationships. - Assign areas when managing instances. diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 59c3b572..1f010103 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -216,9 +216,10 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B Every incoming message carries the authenticated user's identity and roles. The ManagementActor enforces the same role-based authorization rules as the Central UI: -- **Admin** role required for: site management, area management, API key management, role mapping management, scope rule management, system configuration. +- **Admin** role required for: site management, API key management, role mapping management, scope rule management, system configuration. - **Design** role required for: template authoring (including template member management: attributes, alarms, native alarm sources, scripts, compositions), shared scripts, external system definitions, database connection definitions, notification lists, inbound API method definitions. - **Deployment** role required for: instance management (including instance alarm overrides and native alarm source overrides), deployments, debug view, debug snapshot, parked message queries, site event log queries. Site scoping is enforced for site-scoped Deployment users. +- **Any of Design / Deployment** required for: area management (`CreateAreaCommand` / `UpdateAreaCommand` / `DeleteAreaCommand`). Areas are authored both in the Designer tooling and inside the Central UI Deployment Topology workflow (`Topology.razor` is `RequireDeployment`), so either role qualifies — enforced as an any-of gate (arch-review C6). Administrator is not implicitly included. - **Operator** role required for: submitting a secured write (`SubmitSecuredWriteCommand`). - **Verifier** role required for: approving / rejecting a secured write (`ApproveSecuredWriteCommand` / `RejectSecuredWriteCommand`). The no-self-approval rule (`Operator ≠ Verifier`) is enforced in the handler, independent of the role check. - **Any of Operator / Verifier / Administrator** required for: listing / querying secured writes (`ListSecuredWritesCommand`). Read-only, but the history exposes process-sensitive tag values, so it is gated any-of rather than open to every authenticated user (arch-review UA1). Enforced as an any-of gate (`GetRequiredRoles` returns the permitted set; the caller must hold at least one). diff --git a/docs/requirements/Component-Security.md b/docs/requirements/Component-Security.md index 3e5678f9..014a21a1 100644 --- a/docs/requirements/Component-Security.md +++ b/docs/requirements/Component-Security.md @@ -85,7 +85,6 @@ Set in a local or docker-dev environment via the environment variable `ScadaBrid - **Permissions**: - Manage site definitions. - Manage site-level data connections (define and assign to sites). - - Manage area definitions per site. - Manage LDAP group-to-role mappings. - Manage API keys (create, enable/disable, delete). - System-level configuration. @@ -100,12 +99,14 @@ Set in a local or docker-dev environment via the environment variable `ScadaBrid - Manage database connection definitions. - Manage notification lists and SMTP configuration. - Manage inbound API method definitions. + - Manage area definitions per site (any-of with Deployer — see note below). - Run on-demand validation (template flattening, script compilation). ### Deployer - **Scope**: System-wide or site-scoped. - **Permissions**: - Create and manage instances (overrides, connection bindings, area assignment). + - Manage area definitions per site (any-of with Designer — see note below). - Disable, enable, and delete instances. - Deploy configurations to instances. - Deploy system-wide artifacts (shared scripts, external system definitions, DB connections, data connections) to all sites. @@ -116,6 +117,8 @@ Set in a local or docker-dev environment via the environment variable `ScadaBrid - View site event logs. - **Site scoping**: A user with site-scoped Deployer role can only perform these actions for instances at their permitted sites. +> **Area management (arch-review C6)**: Managing area definitions per site is gated **any-of [Designer, Deployer]**, not Administrator. Areas are authored both in the Designer tooling and inside the Central UI Deployment Topology workflow (`Topology.razor` is `RequireDeployment`), so either role qualifies; Administrator is not implicitly included in the actor gate. + ### Viewer - **Scope**: System-wide (always). - **Permissions**: diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index ae49d023..732598c4 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -167,6 +167,15 @@ public class ManagementActor : ReceiveActor private static readonly string[] OperatorOnly = [Roles.Operator]; private static readonly string[] VerifierOnly = [Roles.Verifier]; + // arch-review C6: area management is reconciled to any-of [Designer, Deployer]. + // Areas are authored on both real surfaces — the Designer template/topology + // tooling AND the Central UI Deployment Topology workflow (Topology.razor is + // RequireDeployment) — so a pure Deployer must be able to create/update/delete + // them. The three requirements docs' historical "Administrator" assignment was + // stale; Administrator is not implicitly included here, matching the actor's + // any-of treatment of every other Designer/Deployer command. + private static readonly string[] AreaManagers = [Roles.Designer, Roles.Deployer]; + // arch-review UA1: the Secured Writes history table exposes // process-sensitive tag values, so reading it is restricted to the // two-person Secured Write roles plus Administrator (any-of) — NOT any @@ -205,9 +214,13 @@ public class ManagementActor : ReceiveActor or UpdateSmtpConfigCommand or UpdateSmsConfigCommand => AdminOnly, + // Area management — any-of [Designer, Deployer] (arch-review C6). + // Exposed both in the Designer tooling and inside the Central UI + // Deployment Topology workflow (RequireDeployment), so both roles qualify. + CreateAreaCommand or UpdateAreaCommand or DeleteAreaCommand => AreaManagers, + // Designer operations - CreateAreaCommand or DeleteAreaCommand - or CreateTemplateCommand or UpdateTemplateCommand or DeleteTemplateCommand + CreateTemplateCommand or UpdateTemplateCommand or DeleteTemplateCommand or ValidateTemplateCommand or CreateExternalSystemCommand or UpdateExternalSystemCommand or DeleteExternalSystemCommand @@ -227,7 +240,6 @@ public class ManagementActor : ReceiveActor or CreateSharedSchemaCommand or UpdateSharedSchemaCommand or DeleteSharedSchemaCommand or CreateDatabaseConnectionDefCommand or UpdateDatabaseConnectionDefCommand or DeleteDatabaseConnectionDefCommand or CreateApiMethodCommand or UpdateApiMethodCommand or DeleteApiMethodCommand - or UpdateAreaCommand or CreateTemplateFolderCommand or RenameTemplateFolderCommand or MoveTemplateFolderCommand or ReorderTemplateFolderCommand or DeleteTemplateFolderCommand or MoveTemplateToFolderCommand diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index a19c52ae..46be4b0a 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -3125,4 +3125,31 @@ public class ManagementActorTests : TestKit, IDisposable Arg.Is(d => d.TimeoutSeconds == 42), Arg.Any()); } + + // ======================================================================== + // Area management role gate (arch-review C6): reconciled to any-of + // [Designer, Deployer]. The Central UI creates areas inside the Deployment + // Topology workflow (RequireDeployment), so a pure-Deployer must be allowed. + // ======================================================================== + + [Theory] + [InlineData("Designer")] + [InlineData("Deployer")] + public void CreateArea_DesignerOrDeployer_Allowed(string role) + { + var actor = CreateActor(); + actor.Tell(Envelope(new CreateAreaCommand(1, "Area1", null), role)); + // The gate must let both roles through. Dispatch may still fail (no + // AreaService wired), but it must NOT be an authorization rejection. + var resp = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.IsNotType(resp); + } + + [Fact] + public void CreateArea_ViewerOnly_Unauthorized() + { + var actor = CreateActor(); + actor.Tell(Envelope(new CreateAreaCommand(1, "Area1", null), "Viewer")); + ExpectMsg(TimeSpan.FromSeconds(5)); + } }