dohertj2/ScadaBridge
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(security): enforce LDAP-mapping site scope on secured-write submit/approve/reject (plan R2-07 T4)

Browse Source
This commit is contained in:
Joseph Doherty
2026-07-13 10:46:08 -04:00
parent 24c2097ec3
commit 9b9222d223
2 changed files with 99 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs
+14
View File
@@ -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).
tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs
+85
View File
@@ -151,6 +151,9 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
new(new AuthenticatedUser(username, username, roles, Array.Empty<string>()),
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();
/// <summary>Wires a site whose single connection uses the given protocol.</summary>
@@ -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<ManagementUnauthorized>(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<ManagementSuccess>(TimeSpan.FromSeconds(5));
}
[Fact]
public void Approve_SiteScopedVerifier_OutOfScopeSite_Unauthorized_NeverRelays()
{
SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway");
_securedWriteRepo.GetAsync(42, Arg.Any<CancellationToken>()).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<ManagementUnauthorized>(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<CancellationToken>()).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<ManagementUnauthorized>(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<CancellationToken>()).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<string>(), Arg.Any<string?>(), Arg.Any<DateTime>(), Arg.Any<CancellationToken>())
.Returns(true);
var actor = CreateActor();
actor.Tell(ScopedEnvelope(new ApproveSecuredWriteCommand(42, "ok"), "carol", ["3"], "Verifier", "Administrator"));
var response = ExpectMsg<object>(TimeSpan.FromSeconds(5));
Assert.IsNotType<ManagementUnauthorized>(response);
}
}