From 4e633b1e64b8233123c3603e5565ddaf40e922e0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 06:33:29 -0400 Subject: [PATCH] feat(management): Transport on UpdateSmtpConfigCommand with EWS shape validation --- .../Management/NotificationCommands.cs | 4 +- .../ManagementActor.cs | 59 +++++++ .../ManagementActorTests.cs | 148 ++++++++++++++++++ 3 files changed, 210 insertions(+), 1 deletion(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/NotificationCommands.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/NotificationCommands.cs index 38001eec..2701fe8d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/NotificationCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/NotificationCommands.cs @@ -8,7 +8,9 @@ public record CreateNotificationListCommand(string Name, IReadOnlyList R public record UpdateNotificationListCommand(int NotificationListId, string Name, IReadOnlyList RecipientEmails, NotificationType Type = NotificationType.Email, IReadOnlyList? RecipientPhones = null); public record DeleteNotificationListCommand(int NotificationListId); public record ListSmtpConfigsCommand; -public record UpdateSmtpConfigCommand(int SmtpConfigId, string Server, int Port, string AuthMode, string FromAddress, string? TlsMode = null, string? Credentials = null, string? OAuth2Authority = null, string? OAuth2Scope = null); +// Transport is a trailing additive parameter (Smtp or Ews; null = preserve the +// stored value, which is itself null for every pre-EWS row and means Smtp). +public record UpdateSmtpConfigCommand(int SmtpConfigId, string Server, int Port, string AuthMode, string FromAddress, string? TlsMode = null, string? Credentials = null, string? OAuth2Authority = null, string? OAuth2Scope = null, string? Transport = null); public record ListSmsConfigsCommand; // FromNumber is optional: a Twilio Messaging-Service-only config supplies MessagingServiceSid // instead. At-least-one-of (FromNumber, MessagingServiceSid) is validated at the CLI/UI boundary. diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 3033e20c..6f31113f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -2120,6 +2120,7 @@ public class ManagementActor : ReceiveActor c.AuthType, c.FromAddress, c.TlsMode, + c.Transport, c.ConnectionTimeoutSeconds, c.MaxConcurrentConnections, c.MaxRetries, @@ -2156,6 +2157,10 @@ public class ManagementActor : ReceiveActor // omitted authority/scope leaves the stored value (or its M365 default) intact. if (cmd.OAuth2Authority is not null) config.OAuth2Authority = cmd.OAuth2Authority; if (cmd.OAuth2Scope is not null) config.OAuth2Scope = cmd.OAuth2Scope; + // Preserve-if-null for the transport too: an omitted value keeps the stored + // one (null on every pre-EWS row, which means SMTP). + if (cmd.Transport is not null) config.Transport = cmd.Transport; + ValidateSmtpConfigTransportShape(config); await repo.UpdateSmtpConfigurationAsync(config); await repo.SaveChangesAsync(); // Audit the credential-free shape — the *fact of* the change @@ -2166,6 +2171,60 @@ public class ManagementActor : ReceiveActor return publicShape; } + /// + /// Write-path gate for the email transport: validates the EFFECTIVE + /// post-assignment state of an SmtpConfiguration so an operator cannot persist a + /// row the delivery adapter would immediately park every notification on. The + /// adapter re-validates at delivery time and remains the authoritative check — + /// this is a fail-fast at the point of the mistake, not a second source of truth. + /// + /// The configuration with all command assignments already applied. + /// The transport is unknown, or the EWS shape rules are violated. + private static void ValidateSmtpConfigTransportShape(Commons.Entities.Notifications.SmtpConfiguration config) + { + // Mirrors NotificationService's EmailTransportParser, inlined because + // ManagementService does not (and should not) reference NotificationService + // just for a two-value parse. Keep the accepted values in step with it. + var isEws = false; + var transport = config.Transport; + if (!string.IsNullOrWhiteSpace(transport)) + { + switch (transport.Trim().ToLowerInvariant()) + { + case "smtp": + break; + case "ews": + isEws = true; + break; + default: + throw new ManagementCommandException( + $"Unknown email transport '{transport}'. Expected one of: Smtp, Ews."); + } + } + + if (!isEws) + { + return; + } + + // EWS reuses Host as the SOAP endpoint URL, not a mail-server hostname. + if (!Uri.TryCreate(config.Host, UriKind.Absolute, out var endpoint) + || !string.Equals(endpoint.Scheme, Uri.UriSchemeHttps, StringComparison.Ordinal)) + { + throw new ManagementCommandException( + "EWS transport requires Host to be an absolute https:// EWS endpoint URL " + + "(e.g. https://mail.example.com/ews/exchange.asmx)."); + } + + // The EWS sender authenticates with HTTP Basic only — no OAuth2 flow exists + // on that path, so an OAuth2 row would park on every send. + if (!string.Equals(config.AuthType, "basic", StringComparison.OrdinalIgnoreCase)) + { + throw new ManagementCommandException( + "EWS transport supports only Basic authentication; set AuthMode to 'basic'."); + } + } + /// /// Project an SmsConfiguration to a credential-free shape so the /// stored AuthToken (Twilio Auth Token secret) never leaves this boundary via diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index 7a43e95d..e125b036 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -1667,6 +1667,154 @@ public class ManagementActorTests : TestKit, IDisposable Assert.Equal("old.scope/.default", existing.OAuth2Scope); } + // ======================================================================== + // UpdateSmtpConfig — Transport (EWS) write-path gate + // ======================================================================== + + [Fact] + public void UpdateSmtpConfig_WithEwsTransport_PersistsTransportAndSurfacesItInPublicShape() + { + var notifRepo = Substitute.For(); + var existing = new Commons.Entities.Notifications.SmtpConfiguration( + "old.example.com", "Basic", "old@example.com") + { + Id = 1, + Port = 25, + }; + notifRepo.GetSmtpConfigurationByIdAsync(1, Arg.Any()).Returns(existing); + _services.AddScoped(_ => notifRepo); + + var actor = CreateActor(); + var envelope = Envelope( + new UpdateSmtpConfigCommand( + 1, "https://ews.example.test/ews/exchange.asmx", 443, "basic", "new@example.com", + Transport: "Ews"), + "Administrator"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(envelope.CorrelationId, response.CorrelationId); + Assert.Equal("Ews", existing.Transport); + Assert.Equal("https://ews.example.test/ews/exchange.asmx", existing.Host); + // The transport is a non-secret field, so it belongs on the public shape + // the response and audit afterState carry. + Assert.Contains("\"transport\":\"Ews\"", response.JsonData, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void UpdateSmtpConfig_WithUnknownTransport_ReturnsManagementError() + { + var notifRepo = Substitute.For(); + var existing = new Commons.Entities.Notifications.SmtpConfiguration( + "old.example.com", "Basic", "old@example.com") + { + Id = 1, + Port = 25, + }; + notifRepo.GetSmtpConfigurationByIdAsync(1, Arg.Any()).Returns(existing); + _services.AddScoped(_ => notifRepo); + + var actor = CreateActor(); + var envelope = Envelope( + new UpdateSmtpConfigCommand( + 1, "https://graph.example.test/", 443, "basic", "new@example.com", + Transport: "Graph"), + "Administrator"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal("COMMAND_FAILED", response.ErrorCode); + Assert.Contains("Unknown email transport", response.Error); + Assert.Contains("Graph", response.Error); + } + + [Fact] + public void UpdateSmtpConfig_WithEwsTransportAndNonHttpsHost_ReturnsManagementError() + { + var notifRepo = Substitute.For(); + var existing = new Commons.Entities.Notifications.SmtpConfiguration( + "old.example.com", "Basic", "old@example.com") + { + Id = 1, + Port = 25, + }; + notifRepo.GetSmtpConfigurationByIdAsync(1, Arg.Any()).Returns(existing); + _services.AddScoped(_ => notifRepo); + + var actor = CreateActor(); + var envelope = Envelope( + new UpdateSmtpConfigCommand( + 1, "http://ews.example.test/ews/exchange.asmx", 80, "basic", "new@example.com", + Transport: "Ews"), + "Administrator"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal("COMMAND_FAILED", response.ErrorCode); + Assert.Contains("https://", response.Error); + } + + [Fact] + public void UpdateSmtpConfig_WithEwsTransportAndOAuth2Auth_ReturnsManagementError() + { + var notifRepo = Substitute.For(); + var existing = new Commons.Entities.Notifications.SmtpConfiguration( + "old.example.com", "Basic", "old@example.com") + { + Id = 1, + Port = 25, + }; + notifRepo.GetSmtpConfigurationByIdAsync(1, Arg.Any()).Returns(existing); + _services.AddScoped(_ => notifRepo); + + var actor = CreateActor(); + var envelope = Envelope( + new UpdateSmtpConfigCommand( + 1, "https://ews.example.test/ews/exchange.asmx", 443, "oauth2", "new@example.com", + Transport: "Ews"), + "Administrator"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal("COMMAND_FAILED", response.ErrorCode); + Assert.Contains("Basic authentication", response.Error); + } + + [Fact] + public void UpdateSmtpConfig_WithNullTransport_PreservesExistingValue() + { + var notifRepo = Substitute.For(); + var existing = new Commons.Entities.Notifications.SmtpConfiguration( + "https://ews.example.test/ews/exchange.asmx", "basic", "old@example.com") + { + Id = 1, + Port = 443, + Transport = "Ews", + }; + notifRepo.GetSmtpConfigurationByIdAsync(1, Arg.Any()).Returns(existing); + _services.AddScoped(_ => notifRepo); + + var actor = CreateActor(); + // Transport omitted (a pre-EWS caller, or Newtonsoft deserializing a + // payload without the field): the stored transport survives, and the + // EWS shape rules are still enforced against that preserved value. + var envelope = Envelope( + new UpdateSmtpConfigCommand( + 1, "https://ews2.example.test/ews/exchange.asmx", 443, "basic", "new@example.com"), + "Administrator"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(envelope.CorrelationId, response.CorrelationId); + Assert.Equal("Ews", existing.Transport); + Assert.Equal("https://ews2.example.test/ews/exchange.asmx", existing.Host); + } + // ======================================================================== // SMS Notifications (S5) — list Type discriminator + SMS-config management // ========================================================================