From 8df15b34b84fbd25b9b34104889e1055c663138a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 06:43:43 -0400 Subject: [PATCH] fix(transport): carry Transport + OAuth2 authority/scope on SmtpConfigDto (OAuth2 fields were silently dropped) --- .../Import/ArtifactDiff.cs | 6 + .../Import/BundleImporter.cs | 5 + .../Serialization/EntityDtos.cs | 18 ++- .../Serialization/EntitySerializer.cs | 12 +- .../RoundTripEquivalenceTests.cs | 6 + .../Serialization/EntitySerializerTests.cs | 114 ++++++++++++++++++ 6 files changed, 159 insertions(+), 2 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/ArtifactDiff.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/ArtifactDiff.cs index e48a95d3..cee98bf3 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/ArtifactDiff.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/ArtifactDiff.cs @@ -266,6 +266,12 @@ public sealed class ArtifactDiff AddIfDifferent(changes, "MaxConcurrentConnections", existing.MaxConcurrentConnections, incoming.MaxConcurrentConnections); AddIfDifferent(changes, "MaxRetries", existing.MaxRetries, incoming.MaxRetries); AddIfDifferent(changes, "RetryDelay", existing.RetryDelay.ToString(), incoming.RetryDelay.ToString()); + // Non-sensitive delivery metadata: diffed by value (unlike the credential, + // which is presence-only below). Both sides null on pre-EWS bundles, so a + // legacy configuration still classifies Identical. + AddIfDifferent(changes, "OAuth2Authority", existing.OAuth2Authority, incoming.OAuth2Authority); + AddIfDifferent(changes, "OAuth2Scope", existing.OAuth2Scope, incoming.OAuth2Scope); + AddIfDifferent(changes, "Transport", existing.Transport, incoming.Transport); var existingHasSecret = !string.IsNullOrEmpty(existing.Credentials); var incomingHasSecret = incoming.Secrets is not null && incoming.Secrets.Values.ContainsKey("Credentials"); diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index db25a8fd..e39c57f6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -3437,6 +3437,11 @@ public sealed class BundleImporter : IBundleImporter target.MaxConcurrentConnections = dto.MaxConcurrentConnections; target.MaxRetries = dto.MaxRetries; target.RetryDelay = dto.RetryDelay; + // Non-sensitive delivery metadata; null on a bundle written before these + // were carried, which is the entity's "provider defaults / Smtp" state. + target.OAuth2Authority = dto.OAuth2Authority; + target.OAuth2Scope = dto.OAuth2Scope; + target.Transport = dto.Transport; target.Credentials = dto.Secrets?.Values.TryGetValue("Credentials", out var cred) == true ? cred : null; } diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntityDtos.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntityDtos.cs index 82c25b68..2e5ea0bf 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntityDtos.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntityDtos.cs @@ -285,6 +285,12 @@ public sealed record NotificationRecipientDto( // WhenWritingNull policy keeps it out of email-only bundles' JSON. string? PhoneNumber = null); +/// +/// An email delivery configuration (the SmtpConfiguration entity). Every +/// non-sensitive field is carried directly and only the credential (SMTP password +/// or OAuth2 client secret) rides inside , so a future +/// "share without secrets" export can drop it as a unit. +/// public sealed record SmtpConfigDto( string Host, int Port, @@ -295,7 +301,17 @@ public sealed record SmtpConfigDto( int MaxConcurrentConnections, int MaxRetries, TimeSpan RetryDelay, - SecretsBlock? Secrets); + SecretsBlock? Secrets, + // OAuth2 token-endpoint authority and scope: endpoint URLs, not secrets, so + // they belong on the public DTO rather than in the SecretsBlock. Trailing + + // nullable so a bundle written before these were carried deserializes them as + // null (= provider defaults); WhenWritingNull keeps them out of the JSON of + // configurations that don't set them. + string? OAuth2Authority = null, + string? OAuth2Scope = null, + // Delivery transport ("Smtp" or "Ews"); null means Smtp. Trailing + nullable + // for the same additive-evolution reason — no bundleFormatVersion bump. + string? Transport = null); /// /// An SMS provider configuration (the SmsConfiguration entity). Mirrors diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntitySerializer.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntitySerializer.cs index 4dd35d79..a39335ed 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntitySerializer.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntitySerializer.cs @@ -186,7 +186,12 @@ public sealed class EntitySerializer MaxConcurrentConnections: smtp.MaxConcurrentConnections, MaxRetries: smtp.MaxRetries, RetryDelay: smtp.RetryDelay, - Secrets: secrets); + Secrets: secrets, + // Non-sensitive delivery metadata: the OAuth2 endpoint pair and + // the Smtp/Ews transport selector travel as plain DTO fields. + OAuth2Authority: smtp.OAuth2Authority, + OAuth2Scope: smtp.OAuth2Scope, + Transport: smtp.Transport); }).ToList(), // Inbound API keys are not transported between environments: // the bundle carries API methods only. ApiMethod.ApprovedApiKeyIds is also @@ -486,6 +491,11 @@ public sealed class EntitySerializer MaxConcurrentConnections = dto.MaxConcurrentConnections, MaxRetries = dto.MaxRetries, RetryDelay = dto.RetryDelay, + // Null on a bundle written before these fields were carried, which + // is exactly the entity's "use the provider defaults / Smtp" state. + OAuth2Authority = dto.OAuth2Authority, + OAuth2Scope = dto.OAuth2Scope, + Transport = dto.Transport, }) .ToList(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/RoundTripEquivalenceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/RoundTripEquivalenceTests.cs index 4b31bdf7..97a30eb4 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/RoundTripEquivalenceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/RoundTripEquivalenceTests.cs @@ -633,6 +633,12 @@ public sealed class RoundTripEquivalenceTests : IDisposable MaxConcurrentConnections = 4, MaxRetries = 9, RetryDelay = TimeSpan.FromSeconds(33), + // Non-sensitive delivery metadata. Left null, these three were + // silently dropped by the bundle without the guard noticing + // (null == null), so they are seeded explicitly here. + Transport = "Ews", + OAuth2Authority = "https://login.example.test/token", + OAuth2Scope = "https://mail.example.test/.default", }); await ctx.SaveChangesAsync(); }); diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Serialization/EntitySerializerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Serialization/EntitySerializerTests.cs index 1846aab7..17907ec3 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Serialization/EntitySerializerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Serialization/EntitySerializerTests.cs @@ -938,4 +938,118 @@ public sealed class EntitySerializerTests var rt = new EntitySerializer().FromBundleContent(dto); Assert.Null(Assert.Single(rt.SmsConfigurations).AuthToken); } + + // --- EWS: Transport + the pre-existing OAuth2 authority/scope drop --------- + + [Fact] + public void Roundtrip_smtp_config_preserves_transport_and_oauth2_fields() + { + // Transport is the EWS selector; OAuth2Authority/OAuth2Scope predate it and + // were silently dropped by the bundle (they were never on the DTO). All + // three are non-sensitive endpoint metadata, so they ride as plain DTO + // fields — only Credentials belongs in the SecretsBlock. + var smtp = new SmtpConfiguration("https://mail.example.test/EWS/Exchange.asmx", "Basic", "noreply@example.com") + { + Id = 1, + Port = 587, + Credentials = "user:p@ssw0rd", + TlsMode = "StartTLS", + ConnectionTimeoutSeconds = 25, + MaxConcurrentConnections = 3, + MaxRetries = 4, + RetryDelay = TimeSpan.FromSeconds(90), + Transport = "Ews", + OAuth2Authority = "https://login.example.test/token", + OAuth2Scope = "https://mail.example.test/.default", + }; + var aggregate = MakeEmptyAggregate() with { SmtpConfigurations = new[] { smtp } }; + + var sut = new EntitySerializer(); + var dto = sut.ToBundleContent(aggregate); + + var dtoSmtp = Assert.Single(dto.SmtpConfigs); + Assert.Equal("Ews", dtoSmtp.Transport); + Assert.Equal("https://login.example.test/token", dtoSmtp.OAuth2Authority); + Assert.Equal("https://mail.example.test/.default", dtoSmtp.OAuth2Scope); + + var rt = sut.FromBundleContent(dto); + + var rtSmtp = Assert.Single(rt.SmtpConfigurations); + Assert.Equal("Ews", rtSmtp.Transport); + Assert.Equal("https://login.example.test/token", rtSmtp.OAuth2Authority); + Assert.Equal("https://mail.example.test/.default", rtSmtp.OAuth2Scope); + // The pre-existing fields still survive alongside the new ones. + Assert.Equal("https://mail.example.test/EWS/Exchange.asmx", rtSmtp.Host); + Assert.Equal(587, rtSmtp.Port); + Assert.Equal("StartTLS", rtSmtp.TlsMode); + Assert.Equal("user:p@ssw0rd", rtSmtp.Credentials); + Assert.Equal(25, rtSmtp.ConnectionTimeoutSeconds); + Assert.Equal(3, rtSmtp.MaxConcurrentConnections); + Assert.Equal(4, rtSmtp.MaxRetries); + Assert.Equal(TimeSpan.FromSeconds(90), rtSmtp.RetryDelay); + } + + [Fact] + public void FromDto_smtp_config_without_transport_or_oauth2_fields_yields_nulls() + { + // Backward-compat at the DTO level (mirrors the PhoneNumber precedent): an + // SMTP DTO built without the three trailing params — as an old bundle's JSON + // deserializes, since those properties are simply absent — imports with all + // three null, i.e. legacy SMTP delivery and provider-default OAuth2 endpoints. + var dto = new BundleContentDto( + TemplateFolders: Array.Empty(), + Templates: Array.Empty(), + SharedScripts: Array.Empty(), + ExternalSystems: Array.Empty(), + DatabaseConnections: Array.Empty(), + NotificationLists: Array.Empty(), + SmtpConfigs: new[] + { + new SmtpConfigDto( + Host: "smtp.example.com", + Port: 587, + AuthType: "Basic", + FromAddress: "noreply@example.com", + TlsMode: "StartTLS", + ConnectionTimeoutSeconds: 30, + MaxConcurrentConnections: 2, + MaxRetries: 3, + RetryDelay: TimeSpan.FromSeconds(60), + Secrets: null), + }, + ApiMethods: Array.Empty()); + + var aggregate = new EntitySerializer().FromBundleContent(dto); + + var smtp = Assert.Single(aggregate.SmtpConfigurations); + Assert.Null(smtp.Transport); + Assert.Null(smtp.OAuth2Authority); + Assert.Null(smtp.OAuth2Scope); + Assert.Equal("smtp.example.com", smtp.Host); + } + + [Fact] + public void SmtpConfigDto_omits_null_transport_and_oauth2_fields_from_json() + { + // WhenWritingNull keeps the three new properties out of a plain-SMTP + // bundle's JSON entirely, so the wire shape is unchanged for existing + // exports (additive evolution — no bundleFormatVersion bump). + var dto = new SmtpConfigDto( + Host: "smtp.example.com", + Port: 587, + AuthType: "Basic", + FromAddress: "noreply@example.com", + TlsMode: "StartTLS", + ConnectionTimeoutSeconds: 30, + MaxConcurrentConnections: 2, + MaxRetries: 3, + RetryDelay: TimeSpan.FromSeconds(60), + Secrets: null); + + var json = JsonSerializer.Serialize(dto, BundleJsonOptions.Default); + + Assert.DoesNotContain("Transport", json, StringComparison.Ordinal); + Assert.DoesNotContain("OAuth2Authority", json, StringComparison.Ordinal); + Assert.DoesNotContain("OAuth2Scope", json, StringComparison.Ordinal); + } }