From 9b9222d223cdf307b053d0c0249bb267765f5f93 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:46:08 -0400 Subject: [PATCH] fix(security): enforce LDAP-mapping site scope on secured-write submit/approve/reject (plan R2-07 T4) --- .../ManagementActor.cs | 14 +++ .../SecuredWriteHandlerTests.cs | 85 +++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 71e692e4..3cc76e66 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -1247,6 +1247,11 @@ public class ManagementActor : ReceiveActor var site = await siteRepo.GetSiteByIdentifierAsync(cmd.SiteId) ?? throw new ManagementCommandException($"Site '{cmd.SiteId}' not found."); + // Site scope (arch-review R2 N3): an LDAP-mapping-scoped Operator may only + // submit device writes to their permitted sites — same rule C2 applies to + // DeployArtifacts, on a higher-consequence path. Checked before the row exists. + EnforceSiteScope(user, site.Id); + var connections = await siteRepo.GetDataConnectionsBySiteIdAsync(site.Id); var conn = connections.FirstOrDefault(c => string.Equals(c.Name, cmd.ConnectionName, StringComparison.Ordinal)) @@ -1293,6 +1298,11 @@ public class ManagementActor : ReceiveActor throw new ManagementCommandException( $"Secured write {cmd.Id} is '{row.Status}', not Pending; it cannot be approved."); + // Site scope (arch-review R2 N3): the approving Verifier must be permitted on + // the row's target site — checked BEFORE the TTL/self-approval/CAS chain so an + // out-of-scope verifier can neither consume the Pending transition nor relay. + await EnforceSiteScopeForIdentifier(sp, user, row.SiteId); + // Server-side TTL: a Pending write older than the configured age is transitioned // to Expired and can never be approved — the stale setpoint is never relayed // (arch-review S2). Checked BEFORE the self-approval/CAS so an overdue row never @@ -1450,6 +1460,10 @@ public class ManagementActor : ReceiveActor throw new ManagementCommandException( $"Secured write {cmd.Id} is '{entity.Status}', not Pending; it cannot be rejected."); + // Site scope (arch-review R2 N3): the rejecting Verifier must be permitted on + // the row's target site — checked BEFORE the TTL/self-approval chain. + await EnforceSiteScopeForIdentifier(sp, user, entity.SiteId); + // Server-side TTL: an overdue Pending write expires here too, so a reject on a // stale row deterministically reports the expiry rather than recording a // human decision (arch-review S2). diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs index 4d2199f4..7c8a6450 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs @@ -151,6 +151,9 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable new(new AuthenticatedUser(username, username, roles, Array.Empty()), command, Guid.NewGuid().ToString("N")); + private static ManagementEnvelope ScopedEnvelope(object command, string username, string[] permittedSiteIds, params string[] roles) => + new(new AuthenticatedUser(username, username, roles, permittedSiteIds), command, Guid.NewGuid().ToString("N")); + void IDisposable.Dispose() => Shutdown(); /// Wires a site whose single connection uses the given protocol. @@ -899,4 +902,86 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable var execute = SingleOfKind(captured, AuditKind.SecuredWriteExecute); Assert.Equal(ZB.MOM.WW.Audit.AuditOutcome.Failure, execute.Outcome); } + + // ------------------------------------------------------------------------ + // Site scope on submit / approve / reject (arch-review R2 N3) + // ------------------------------------------------------------------------ + + [Fact] + public void Submit_SiteScopedOperator_OutOfScopeSite_Unauthorized_NoRowInserted() + { + SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway"); + var actor = CreateActor(); + actor.Tell(ScopedEnvelope( + new SubmitSecuredWriteCommand("SITE1", "Gw1", "Tag.A", "true", "Boolean", "go"), + "alice", ["3"], "Operator")); + ExpectMsg(TimeSpan.FromSeconds(5)); + _securedWriteRepo.DidNotReceiveWithAnyArgs().AddAsync(default!, default); + } + + [Fact] + public void Submit_SiteScopedOperator_InScopeSite_Succeeds() + { + SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway"); + var actor = CreateActor(); + actor.Tell(ScopedEnvelope( + new SubmitSecuredWriteCommand("SITE1", "Gw1", "Tag.A", "true", "Boolean", "go"), + "alice", ["1"], "Operator")); + ExpectMsg(TimeSpan.FromSeconds(5)); + } + + [Fact] + public void Approve_SiteScopedVerifier_OutOfScopeSite_Unauthorized_NeverRelays() + { + SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway"); + _securedWriteRepo.GetAsync(42, Arg.Any()).Returns(new PendingSecuredWrite + { + Id = 42, SiteId = "SITE1", ConnectionName = "Gw1", TagPath = "Tag.A", + ValueJson = "true", ValueType = "Boolean", Status = "Pending", + OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow, + }); + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new ApproveSecuredWriteCommand(42, "ok"), "bob", ["3"], "Verifier")); + ExpectMsg(TimeSpan.FromSeconds(5)); + _securedWriteRepo.DidNotReceiveWithAnyArgs().TryMarkApprovedAsync(default, default!, default, default, default); + Assert.Equal(0, _comms.CallCount); // the device write is NEVER relayed + } + + [Fact] + public void Reject_SiteScopedVerifier_OutOfScopeSite_Unauthorized() + { + SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway"); + _securedWriteRepo.GetAsync(42, Arg.Any()).Returns(new PendingSecuredWrite + { + Id = 42, SiteId = "SITE1", ConnectionName = "Gw1", TagPath = "Tag.A", + ValueJson = "true", ValueType = "Boolean", Status = "Pending", + OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow, + }); + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new RejectSecuredWriteCommand(42, "no"), "bob", ["3"], "Verifier")); + ExpectMsg(TimeSpan.FromSeconds(5)); + _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + } + + [Fact] + public void Approve_ScopedAdministrator_BypassesSiteScope() + { + // A scoped Verifier who is ALSO an Administrator bypasses site scope + // (EnforceSiteScope admin bypass, ManagementActor.cs:499) — the approve + // proceeds past the scope gate rather than returning ManagementUnauthorized. + SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway"); + _securedWriteRepo.GetAsync(42, Arg.Any()).Returns(new PendingSecuredWrite + { + Id = 42, SiteId = "SITE1", ConnectionName = "Gw1", TagPath = "Tag.A", + ValueJson = "true", ValueType = "Boolean", Status = "Pending", + OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow, + }); + _securedWriteRepo.TryMarkApprovedAsync( + 42, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new ApproveSecuredWriteCommand(42, "ok"), "carol", ["3"], "Verifier", "Administrator")); + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.IsNotType(response); + } }