From 5415e6566faec1a12cc53dc8aab4bfaac35522d7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 10 Jul 2026 05:29:57 -0400 Subject: [PATCH] feat(management): Admin-gated actor dispatch for cert-trust commands (arch-review C4) Made CommunicationService.TrustServerCertAsync/RemoveServerCertAsync/ListServerCertsAsync virtual to support the test seam (mirrors existing virtual WriteTagAsync). Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj --- .../Component-ManagementService.md | 2 +- .../CommunicationService.cs | 6 +- .../ManagementActor.cs | 43 ++++++- .../ManagementActorTests.cs | 108 ++++++++++++++++++ 4 files changed, 154 insertions(+), 5 deletions(-) diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 92637b25..9483c525 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -210,7 +210,7 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B - **BrowseNodeCommand**: Browse an OPC UA / MxGateway connection's address space one level at a time; supports a `BrowseNext` continuation token (paging) and returns per-node type-info for OPC UA Variables. Actor-dispatched — the handler requires the command's `SiteIdentifier`, enforces site-scope, then relays via `CommunicationService.BrowseNodeAsync` (arch-review C4, CLI/UI parity). (`Design` role.) - **SearchAddressSpaceCommand**: Bounded recursive address-space search (depth + result caps, case-insensitive substring) against an OPC UA connection. Actor-dispatched, same `SiteIdentifier`-required + site-scope pattern, relayed via `CommunicationService.SearchAddressSpaceAsync` (arch-review C4). (`Design` role.) - **VerifyEndpointCommand**: Ask a site to probe an OPC UA endpoint (temporary client, short timeout) and report success / typed failure / a captured-but-untrusted server cert. Read-only — never trusts. Actor-dispatched, `SiteIdentifier`-required + site-scope, relayed via `CommunicationService.VerifyEndpointAsync` (arch-review C4). (`Design` role — runs inside the Admin-gated connection editor.) -- **TrustServerCertCommand** / **RemoveServerCertCommand** / **ListServerCertsCommand**: Manage the site's OPC UA trusted-peer PKI store; Trust/Remove broadcast to **both** site nodes (see Component-SiteRuntime.md). (`Admin` role.) +- **TrustServerCertCommand** / **RemoveServerCertCommand** / **ListServerCertsCommand**: Manage the site's OPC UA trusted-peer PKI store; Trust/Remove broadcast to **both** site nodes (see Component-SiteRuntime.md). Actor-dispatched and `Admin`-gated at the actor (arch-review C4) — each handler requires the command's `SiteIdentifier`, enforces site-scope, then relays via `CommunicationService.TrustServerCertAsync` / `RemoveServerCertAsync` / `ListServerCertsAsync`; the site-side reconcile across both nodes is owned by the site `CertStoreActor`. (`Admin` role.) ## Authorization diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/CommunicationService.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/CommunicationService.cs index 115754f3..8a5a40b5 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/CommunicationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/CommunicationService.cs @@ -432,7 +432,7 @@ public class CommunicationService /// The trust-server-cert command (connection name + DER + thumbprint). /// Cancellation token. /// The cert-trust result (per-node aggregate success + first error). - public Task TrustServerCertAsync( + public virtual Task TrustServerCertAsync( string siteId, TrustServerCertCommand command, CancellationToken cancellationToken = default) @@ -453,7 +453,7 @@ public class CommunicationService /// The list-server-certs command. /// Cancellation token. /// The cert-trust result carrying the listed certificates on success. - public Task ListServerCertsAsync( + public virtual Task ListServerCertsAsync( string siteId, ListServerCertsCommand command, CancellationToken cancellationToken = default) @@ -474,7 +474,7 @@ public class CommunicationService /// The remove-server-cert command (thumbprint). /// Cancellation token. /// The cert-trust result (per-node aggregate success + first error). - public Task RemoveServerCertAsync( + public virtual Task RemoveServerCertAsync( string siteId, RemoveServerCertCommand command, CancellationToken cancellationToken = default) diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 79556338..dd1229cf 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -212,7 +212,12 @@ public class ManagementActor : ReceiveActor // gate must match — otherwise a Designer blocked in the UI could still // rotate a production credential via the CLI/Management API. or UpdateSmtpConfigCommand - or UpdateSmsConfigCommand => AdminOnly, + or UpdateSmsConfigCommand + // Server-certificate trust (arch-review C4). These mutate the site's + // trusted-peer PKI store — Admin, matching the Admin-gated UI cert + // page. Site-side reconcile semantics are owned by the site + // CertStoreActor (plan 03); this handler only relays. + or TrustServerCertCommand or RemoveServerCertCommand or ListServerCertsCommand => AdminOnly, // Area management — any-of [Designer, Deployer] (arch-review C6). // Exposed both in the Designer tooling and inside the Central UI @@ -443,6 +448,9 @@ public class ManagementActor : ReceiveActor BrowseNodeCommand cmd => await HandleBrowseNode(sp, cmd, user), SearchAddressSpaceCommand cmd => await HandleSearchAddressSpace(sp, cmd, user), VerifyEndpointCommand cmd => await HandleVerifyEndpoint(sp, cmd, user), + TrustServerCertCommand cmd => await HandleTrustServerCert(sp, cmd, user), + RemoveServerCertCommand cmd => await HandleRemoveServerCert(sp, cmd, user), + ListServerCertsCommand cmd => await HandleListServerCerts(sp, cmd, user), QueryParkedMessagesCommand cmd => await HandleQueryParkedMessages(sp, cmd, user), RetryParkedMessageCommand cmd => await HandleRetryParkedMessage(sp, cmd, user), DiscardParkedMessageCommand cmd => await HandleDiscardParkedMessage(sp, cmd, user), @@ -2955,6 +2963,39 @@ public class ManagementActor : ReceiveActor return await commService.VerifyEndpointAsync(site, cmd); } + // Server-certificate trust (arch-review C4). Admin-gated at the role check + // above. These mutate the site's trusted-peer PKI store — the handler only + // relays; the site-side broadcast/reconcile across both nodes is owned by + // the site CertStoreActor (plan 03). SiteIdentifier is REQUIRED so the actor + // knows which site to route to. + + private static async Task HandleTrustServerCert(IServiceProvider sp, TrustServerCertCommand cmd, AuthenticatedUser user) + { + var site = cmd.SiteIdentifier + ?? throw new ManagementCommandException("SiteIdentifier is required when trusting a server certificate via the management API."); + await EnforceSiteScopeForIdentifier(sp, user, site); + var commService = sp.GetRequiredService(); + return await commService.TrustServerCertAsync(site, cmd); + } + + private static async Task HandleRemoveServerCert(IServiceProvider sp, RemoveServerCertCommand cmd, AuthenticatedUser user) + { + var site = cmd.SiteIdentifier + ?? throw new ManagementCommandException("SiteIdentifier is required when removing a server certificate via the management API."); + await EnforceSiteScopeForIdentifier(sp, user, site); + var commService = sp.GetRequiredService(); + return await commService.RemoveServerCertAsync(site, cmd); + } + + private static async Task HandleListServerCerts(IServiceProvider sp, ListServerCertsCommand cmd, AuthenticatedUser user) + { + var site = cmd.SiteIdentifier + ?? throw new ManagementCommandException("SiteIdentifier is required when listing server certificates via the management API."); + await EnforceSiteScopeForIdentifier(sp, user, site); + var commService = sp.GetRequiredService(); + return await commService.ListServerCertsAsync(site, cmd); + } + private static async Task HandleQueryParkedMessages(IServiceProvider sp, QueryParkedMessagesCommand cmd, AuthenticatedUser user) { await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteIdentifier); diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index 888c80bc..7c29dd2e 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -3250,6 +3250,84 @@ public class ManagementActorTests : TestKit, IDisposable ExpectMsg(TimeSpan.FromSeconds(5)); } + // ======================================================================== + // Admin-gated actor dispatch for the site cert-trust commands + // (arch-review C4): TrustServerCert / RemoveServerCert / ListServerCerts. + // These mutate the site's trusted-peer PKI store, so they are gated + // Administrator — matching the Admin-gated cert-management UI. SiteIdentifier + // is REQUIRED; the handler only relays (site-side reconcile is CertStoreActor). + // ======================================================================== + + [Fact] + public void TrustServerCert_DesignerOnly_Unauthorized() + { + var actor = CreateActor(); + actor.Tell(Envelope(new TrustServerCertCommand("conn", "ZGVy", "AB12", "SITE1"), "Designer")); + ExpectMsg(TimeSpan.FromSeconds(5)); + } + + [Fact] + public void TrustServerCert_WithoutSiteIdentifier_ReturnsError() + { + var actor = CreateActor(); + actor.Tell(Envelope(new TrustServerCertCommand("conn", "ZGVy", "AB12"), "Administrator")); + var resp = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("SiteIdentifier", resp.Error); + } + + [Fact] + public void TrustServerCert_Admin_RelaysToSite() + { + var stub = new StubCommunicationService(); + _services.AddSingleton(stub); + var actor = CreateActor(); + actor.Tell(Envelope(new TrustServerCertCommand("conn", "ZGVy", "AB12", "SITE1"), "Administrator")); + ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(1, stub.TrustCount); + Assert.Equal("SITE1", stub.LastTrustSite); + } + + [Fact] + public void RemoveServerCert_Admin_RelaysToSite() + { + var stub = new StubCommunicationService(); + _services.AddSingleton(stub); + var actor = CreateActor(); + actor.Tell(Envelope(new RemoveServerCertCommand("AB12", "SITE1"), "Administrator")); + ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(1, stub.RemoveCount); + Assert.Equal("SITE1", stub.LastRemoveSite); + } + + [Fact] + public void RemoveServerCert_WithoutSiteIdentifier_ReturnsError() + { + var actor = CreateActor(); + actor.Tell(Envelope(new RemoveServerCertCommand("AB12"), "Administrator")); + var resp = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("SiteIdentifier", resp.Error); + } + + [Fact] + public void ListServerCerts_Admin_RelaysToSite() + { + var stub = new StubCommunicationService(); + _services.AddSingleton(stub); + var actor = CreateActor(); + actor.Tell(Envelope(new ListServerCertsCommand("SITE1"), "Administrator")); + ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(1, stub.ListCertsCount); + Assert.Equal("SITE1", stub.LastListCertsSite); + } + + [Fact] + public void ListServerCerts_DesignerOnly_Unauthorized() + { + var actor = CreateActor(); + actor.Tell(Envelope(new ListServerCertsCommand("SITE1"), "Designer")); + ExpectMsg(TimeSpan.FromSeconds(5)); + } + /// /// Records remote-query relays for the actor-dispatch tests. The browse / /// search / verify / cert-trust methods on @@ -3268,6 +3346,36 @@ public class ManagementActorTests : TestKit, IDisposable public string? LastSearchSite { get; private set; } public int VerifyCount { get; private set; } public string? LastVerifySite { get; private set; } + public int TrustCount { get; private set; } + public string? LastTrustSite { get; private set; } + public int RemoveCount { get; private set; } + public string? LastRemoveSite { get; private set; } + public int ListCertsCount { get; private set; } + public string? LastListCertsSite { get; private set; } + + public override Task TrustServerCertAsync( + string siteId, TrustServerCertCommand command, CancellationToken ct = default) + { + TrustCount++; + LastTrustSite = siteId; + return Task.FromResult(new CertTrustResult(true, null, null)); + } + + public override Task RemoveServerCertAsync( + string siteId, RemoveServerCertCommand command, CancellationToken ct = default) + { + RemoveCount++; + LastRemoveSite = siteId; + return Task.FromResult(new CertTrustResult(true, null, null)); + } + + public override Task ListServerCertsAsync( + string siteId, ListServerCertsCommand command, CancellationToken ct = default) + { + ListCertsCount++; + LastListCertsSite = siteId; + return Task.FromResult(new CertTrustResult(true, null, null)); + } public override Task BrowseNodeAsync( string siteId, BrowseNodeCommand command, CancellationToken ct = default)