diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 1dbcef5c..a9a04ff0 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -131,7 +131,7 @@ The whole folder surface is reachable from the CLI as `template folder list|crea ### Sites - **ListSites** / **GetSite**: Query site definitions. -- **CreateSite** / **UpdateSite** / **DeleteSite**: Manage site definitions. +- **CreateSite** / **UpdateSite** / **DeleteSite**: Manage site definitions. Create/Update validate `GrpcNodeAAddress` / `GrpcNodeBAddress` at the write boundary with `GrpcEndpointRules.TryValidate` (both optional; a non-empty value must be an absolute `http://` or `https://` URL with a host, e.g. `http://host:8083`) and reject a malformed value with a readable `COMMAND_FAILED` error naming the field — a scheme-less address used to be persisted and then throw inside central's channel build on the next site-address refresh (review 2026-10-21 03/R-1). - **ListAreas** / **CreateArea** / **UpdateArea** / **DeleteArea**: Manage area hierarchies per site. - **MoveArea** (`AreaId`, `NewParentAreaId?`): Re-parent an area within its own site; a null `NewParentAreaId` moves it to the site root. Delegates every rule to `AreaService.MoveAreaAsync` — rejects self-parent, descendant-parent (cycle), cross-site parent, and sibling name collision at the target level; the service writes the `"Move"` audit row. Same any-of `[Designer, Deployer]` gate as the other area mutations. CLI: `site area move`. diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 9e9002f7..0cde809d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -29,6 +29,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types.Deployment; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi; using ZB.MOM.WW.ScadaBridge.Commons.Types.Transport; +using ZB.MOM.WW.ScadaBridge.Commons.Validation; using ZB.MOM.WW.ScadaBridge.DeploymentManager; using ZB.MOM.WW.ScadaBridge.HealthMonitoring; using ZB.MOM.WW.ScadaBridge.Communication; @@ -1885,8 +1886,23 @@ public class ManagementActor : ReceiveActor return site; } + /// + /// Rejects a malformed site gRPC address at the write boundary (review 2026-10-21 + /// 03/R-1). Both addresses are optional, but a non-empty value must be an absolute + /// http(s) URL — anything else makes the central channel build throw inside + /// SitePairChannelProvider.UpdateSite on the next address refresh. + /// + private static void ValidateSiteGrpcAddresses(string? grpcNodeAAddress, string? grpcNodeBAddress) + { + if (!GrpcEndpointRules.TryValidate(grpcNodeAAddress, allowEmpty: true, out var errorA)) + throw new ManagementCommandException($"GrpcNodeAAddress: {errorA}"); + if (!GrpcEndpointRules.TryValidate(grpcNodeBAddress, allowEmpty: true, out var errorB)) + throw new ManagementCommandException($"GrpcNodeBAddress: {errorB}"); + } + private static async Task HandleCreateSite(IServiceProvider sp, CreateSiteCommand cmd, string user) { + ValidateSiteGrpcAddresses(cmd.GrpcNodeAAddress, cmd.GrpcNodeBAddress); var repo = sp.GetRequiredService(); var site = new Site(cmd.Name, cmd.SiteIdentifier) { @@ -1906,6 +1922,7 @@ public class ManagementActor : ReceiveActor private static async Task HandleUpdateSite(IServiceProvider sp, UpdateSiteCommand cmd, string user) { + ValidateSiteGrpcAddresses(cmd.GrpcNodeAAddress, cmd.GrpcNodeBAddress); var repo = sp.GetRequiredService(); var site = await repo.GetSiteByIdAsync(cmd.SiteId) ?? throw new ManagementCommandException($"Site with ID {cmd.SiteId} not found."); diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SiteGrpcEndpointValidationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SiteGrpcEndpointValidationTests.cs new file mode 100644 index 00000000..3b9968ac --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SiteGrpcEndpointValidationTests.cs @@ -0,0 +1,80 @@ +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; + +namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests; + +/// +/// Review 2026-10-21 03/R-1: a malformed site gRPC address used to be persisted +/// unvalidated and then throw inside SitePairChannelProvider.UpdateSite. +/// Create/Update must reject it at the write boundary with a readable error. +/// +public class SiteGrpcEndpointValidationTests : TestKit +{ + private readonly ISiteRepository _siteRepo = Substitute.For(); + private readonly ServiceCollection _services; + + /// Initializes the harness. + public SiteGrpcEndpointValidationTests() + { + _services = new ServiceCollection().AddTestConfigurationTransaction(); + _services.AddScoped(_ => _siteRepo); + _services.AddScoped(_ => Substitute.For()); + _siteRepo.GetSiteByIdAsync(7, Arg.Any()) + .Returns(new Site("S", "S1") { Id = 7 }); + } + + private IActorRef CreateActor() => + Sys.ActorOf(Props.Create(() => new ManagementActor( + _services.BuildServiceProvider(), NullLogger.Instance))); + + private static ManagementEnvelope Admin(object command) => + new(new AuthenticatedUser("admin", "Admin", new[] { "Administrator" }, Array.Empty()), + command, Guid.NewGuid().ToString("N")); + + /// Create with a scheme-less node-A address is rejected and nothing is saved. + [Theory] + [InlineData("scadabridge-site-a-node-a")] + [InlineData("site-a-node-a:8083")] + [InlineData("ftp://host:8083")] + public void CreateSite_MalformedGrpcAddress_ReturnsReadableError(string bad) + { + var actor = CreateActor(); + actor.Tell(Admin(new CreateSiteCommand("S", "S1", null, GrpcNodeAAddress: bad))); + + var error = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("GrpcNodeAAddress", error.Error); + Assert.Contains("http://", error.Error); + _siteRepo.DidNotReceive().AddSiteAsync(Arg.Any(), Arg.Any()); + } + + /// Update with a malformed node-B address is rejected and nothing is saved. + [Fact] + public void UpdateSite_MalformedGrpcNodeB_ReturnsReadableError() + { + var actor = CreateActor(); + actor.Tell(Admin(new UpdateSiteCommand(7, "S", null, + GrpcNodeAAddress: "http://a:8083", GrpcNodeBAddress: "b:8083"))); + + var error = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("GrpcNodeBAddress", error.Error); + _siteRepo.DidNotReceive().UpdateSiteAsync(Arg.Any(), Arg.Any()); + } + + /// Both gRPC addresses are optional; valid http(s) URLs (and blanks) are accepted. + [Fact] + public void CreateSite_ValidOrEmptyGrpcAddresses_Succeeds() + { + var actor = CreateActor(); + actor.Tell(Admin(new CreateSiteCommand("S", "S1", null, + GrpcNodeAAddress: "http://site-a-node-a:8083", GrpcNodeBAddress: null))); + + ExpectMsg(TimeSpan.FromSeconds(5)); + } +}