fix(security): scope-filter secured-write listing + align spec with enforced site scoping (plan R2-07 T6)

docs/requirements/Component-Security.md

@@ -138,7 +138,7 @@ Set in a local or docker-dev environment via the environment variable `ScadaBrid
   - Approve or reject a pending secured write from the Secured Writes page — the *approving* half of the two-person write workflow.
 - **Purpose**: The approving counterpart to **Operator**. Separation of duties is enforced **server-side**: the ManagementActor rejects any approval where the approving user equals the submitting Operator (no self-approval), so the two roles must be held by distinct principals for a write to execute. (See Component-ManagementService.md and Component-CentralUI.md.)
 
-> **Two-person secured-write workflow.** `Operator` and `Verifier` are deliberately separate global roles so a single principal cannot both initiate and approve a write through the MxAccess Gateway. Both are coarse global roles like the others; any site scoping is layered on at the LDAP-mapping level.
+> **Two-person secured-write workflow.** `Operator` and `Verifier` are deliberately separate global roles so a single principal cannot both initiate and approve a write through the MxAccess Gateway. Both are coarse global roles like the others. **Site scoping from the LDAP mapping is enforced server-side on every secured-write command** (arch-review R2 N3): submit checks the target site, approve/reject check the row's site (before the TTL/self-approval/CAS chain), and the list constrains a scoped caller to their permitted sites — mirroring the deployment commands' `EnforceSiteScope` rule. Administrators and system-wide principals (empty permitted-site set) are unrestricted.
 >
 > **Deployment-configuration hazard — never grant one principal both roles.** The whole control collapses if the same identity (or LDAP group) maps to *both* `Operator` and `Verifier`: server-side no-self-approval blocks approving the *exact* write you submitted, but a dual-role principal can still trivially pair-approve with a second submission, so a single compromised or careless account regains unilateral write. Keep `SCADA-Operators` and `SCADA-Verifiers` group membership disjoint. The dev `DisableLogin` caveat is the extreme of this: with `DisableLogin` on, the auto-login principal holds **all** roles, so the two-person flow cannot be exercised end-to-end by a single identity — which is exactly why `DisableLogin=true` is refused outside Development (see the `DisableLoginGuard` note above). No-self-approval is covered by handler tests; real two-person use requires two real, distinct-role identities.
 >

src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs

@@ -467,7 +467,7 @@ public class ManagementActor : ReceiveActor
             SubmitSecuredWriteCommand cmd => await HandleSubmitSecuredWrite(sp, cmd, user),
             ApproveSecuredWriteCommand cmd => await HandleApproveSecuredWrite(sp, cmd, user),
             RejectSecuredWriteCommand cmd => await HandleRejectSecuredWrite(sp, cmd, user),
-            ListSecuredWritesCommand cmd => await HandleListSecuredWrites(sp, cmd),
+            ListSecuredWritesCommand cmd => await HandleListSecuredWrites(sp, cmd, user),
 
             // Transport bundle operations
             ExportBundleCommand cmd => await HandleExportBundle(sp, cmd, user.Username),
@@ -1493,16 +1493,36 @@ public class ManagementActor : ReceiveActor
     }
 
     private static async Task<object?> HandleListSecuredWrites(
-        IServiceProvider sp, ListSecuredWritesCommand cmd)
+        IServiceProvider sp, ListSecuredWritesCommand cmd, AuthenticatedUser user)
     {
+        // Site scoping (arch-review R2 N3): an explicit SiteId filter is scope-checked
+        // like every other identifier-keyed command; an UNFILTERED list from a
+        // site-scoped non-admin caller is constrained to their permitted sites at the
+        // query level (rows AND totalCount both scoped — see ISecuredWriteRepository).
+        IReadOnlyCollection<string>? permittedIdentifiers = null;
+        if (cmd.SiteId is not null)
+        {
+            await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteId);
+        }
+        else if (user.PermittedSiteIds.Length > 0
+            && !user.Roles.Contains(Roles.Administrator, StringComparer.OrdinalIgnoreCase))
+        {
+            var siteRepo = sp.GetRequiredService<ISiteRepository>();
+            var permitted = new HashSet<string>(user.PermittedSiteIds);
+            permittedIdentifiers = (await siteRepo.GetAllSitesAsync())
+                .Where(s => permitted.Contains(s.Id.ToString()))
+                .Select(s => s.SiteIdentifier)
+                .ToList();
+        }
+
         var repo = sp.GetRequiredService<ISecuredWriteRepository>();
 
         // Offset paging (arch-review P3): clamp untrusted paging inputs so a crafted
         // command can neither request a negative offset nor an unbounded page.
         var take = Math.Clamp(cmd.Take, 1, 500);
         var skip = Math.Max(0, cmd.Skip);
-        var rows = await repo.QueryAsync(cmd.Status, cmd.SiteId, skip, take);
+        var rows = await repo.QueryAsync(cmd.Status, cmd.SiteId, skip, take, permittedIdentifiers);
-        var totalCount = await repo.CountAsync(cmd.Status, cmd.SiteId);
+        var totalCount = await repo.CountAsync(cmd.Status, cmd.SiteId, permittedIdentifiers);
 
         // Opportunistic expiry sweep (arch-review S2): every time the list is read, walk
         // the page and expire any overdue Pending row. TryExpireIfStaleAsync self-filters

tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs

@@ -145,7 +145,7 @@ public class ManagementActorTests : TestKit, IDisposable
         var securedWriteRepo = Substitute.For<ISecuredWriteRepository>();
         securedWriteRepo.QueryAsync(
                 Arg.Any<string?>(), Arg.Any<string?>(), Arg.Any<int>(), Arg.Any<int>(),
-                Arg.Any<CancellationToken>())
+                Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
             .Returns(Task.FromResult<IReadOnlyList<PendingSecuredWrite>>(
                 Array.Empty<PendingSecuredWrite>()));
         _services.AddScoped(_ => securedWriteRepo);

tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs

@@ -349,7 +349,7 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
     [Fact]
     public void List_FiltersByStatusAndSite()
     {
-        _securedWriteRepo.QueryAsync("Pending", "SITE1", 0, 200, Arg.Any<CancellationToken>())
+        _securedWriteRepo.QueryAsync("Pending", "SITE1", 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
             .Returns(new List<PendingSecuredWrite>
             {
                 new()
@@ -372,13 +372,13 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
         Assert.Equal(envelope.CorrelationId, response.CorrelationId);
         Assert.Contains("\"id\":3", response.JsonData);
         Assert.Contains("alice", response.JsonData);
-        _securedWriteRepo.Received(1).QueryAsync("Pending", "SITE1", 0, 200, Arg.Any<CancellationToken>());
+        _securedWriteRepo.Received(1).QueryAsync("Pending", "SITE1", 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>());
     }
 
     [Fact]
     public void List_NoFilters_PassesNullsToRepository()
     {
-        _securedWriteRepo.QueryAsync(null, null, 0, 200, Arg.Any<CancellationToken>())
+        _securedWriteRepo.QueryAsync(null, null, 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
             .Returns(new List<PendingSecuredWrite>());
 
         var actor = CreateActor();
@@ -388,7 +388,49 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
         actor.Tell(envelope);
 
         ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
-        _securedWriteRepo.Received(1).QueryAsync(null, null, 0, 200, Arg.Any<CancellationToken>());
+        _securedWriteRepo.Received(1).QueryAsync(null, null, 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>());
+    }
+
+    [Fact]
+    public void List_SiteScopedReader_ExplicitOutOfScopeSiteFilter_Unauthorized()
+    {
+        SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway");
+        var actor = CreateActor();
+        actor.Tell(ScopedEnvelope(new ListSecuredWritesCommand(null, "SITE1"), "bob", ["3"], "Verifier"));
+        ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+    }
+
+    [Fact]
+    public void List_SiteScopedReader_Unfiltered_QueriesOnlyPermittedIdentifiers()
+    {
+        _siteRepo.GetAllSitesAsync(Arg.Any<CancellationToken>()).Returns(new List<Site>
+        {
+            new("Site1", "SITE1") { Id = 1 },
+            new("Site3", "SITE3") { Id = 3 },
+        });
+        _securedWriteRepo.QueryAsync(null, null, 0, 200,
+                Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
+            .Returns(new List<PendingSecuredWrite>());
+        var actor = CreateActor();
+        actor.Tell(ScopedEnvelope(new ListSecuredWritesCommand(null, null), "bob", ["3"], "Verifier"));
+        ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
+        _securedWriteRepo.Received(1).QueryAsync(null, null, 0, 200,
+            Arg.Is<IReadOnlyCollection<string>?>(s => s != null && s.Single() == "SITE3"),
+            Arg.Any<CancellationToken>());
+    }
+
+    [Fact]
+    public void List_SystemWideReader_PassesNullPermittedFilter()
+    {
+        _securedWriteRepo.QueryAsync(null, null, 0, 200,
+                Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
+            .Returns(new List<PendingSecuredWrite>());
+        var actor = CreateActor();
+        actor.Tell(Envelope(new ListSecuredWritesCommand(null, null), "carol", "Operator"));
+        ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
+        _securedWriteRepo.Received(1).QueryAsync(null, null, 0, 200,
+            Arg.Is<IReadOnlyCollection<string>?>(s => s == null),
+            Arg.Any<CancellationToken>());
     }
 
     // ------------------------------------------------------------------------
@@ -623,16 +665,16 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
     [Fact]
     public void List_WithSkipTake_ForwardsPagingAndReturnsTotalCount()
     {
-        _securedWriteRepo.QueryAsync(null, null, 10, 25, Arg.Any<CancellationToken>())
+        _securedWriteRepo.QueryAsync(null, null, 10, 25, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
             .Returns(new List<PendingSecuredWrite>());
-        _securedWriteRepo.CountAsync(null, null, Arg.Any<CancellationToken>()).Returns(87);
+        _securedWriteRepo.CountAsync(null, null, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>()).Returns(87);
 
         var actor = CreateActor();
         actor.Tell(Envelope(new ListSecuredWritesCommand(null, null, Skip: 10, Take: 25), "carol", "Operator"));
 
         var resp = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
-        _securedWriteRepo.Received(1).QueryAsync(null, null, 10, 25, Arg.Any<CancellationToken>());
+        _securedWriteRepo.Received(1).QueryAsync(null, null, 10, 25, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>());
-        _securedWriteRepo.Received(1).CountAsync(null, null, Arg.Any<CancellationToken>());
+        _securedWriteRepo.Received(1).CountAsync(null, null, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>());
         Assert.Contains("\"totalCount\":87", resp.JsonData);
     }
 
@@ -655,7 +697,7 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
             ValueJson = "false", ValueType = "Boolean", Status = "Pending",
             OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow.AddHours(-25) // TTL default 24h
         };
-        _securedWriteRepo.QueryAsync(null, null, 0, 200, Arg.Any<CancellationToken>())
+        _securedWriteRepo.QueryAsync(null, null, 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
             .Returns(new List<PendingSecuredWrite> { fresh, stale });
         _securedWriteRepo.TryMarkExpiredAsync(2, Arg.Any<DateTime>(), Arg.Any<CancellationToken>())
             .Returns(true);