diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationLists.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationLists.razor index e8b1110f..5b855d87 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationLists.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationLists.razor @@ -2,6 +2,7 @@ @using ZB.MOM.WW.ScadaBridge.Security @using ZB.MOM.WW.ScadaBridge.Commons.Entities.Notifications @using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories +@using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums @attribute [Authorize(Policy = AuthorizationPolicies.RequireDesign)] @inject INotificationRepository NotificationRepository @inject NavigationManager NavigationManager @@ -67,7 +68,13 @@ { @foreach (var r in recipients) { - @r.Name <@r.EmailAddress> + @* Type-aware contact: SMS recipients carry a PhoneNumber and a null + EmailAddress, so an email-shaped badge would render "Name <>". Fall + back to whichever contact field is populated. *@ + var contact = list.Type == NotificationType.Sms + ? r.PhoneNumber + : r.EmailAddress; + @r.Name <@contact> } } diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Entities/Notifications/NotificationRecipient.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Entities/Notifications/NotificationRecipient.cs index 055176e7..921a66e7 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Entities/Notifications/NotificationRecipient.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Entities/Notifications/NotificationRecipient.cs @@ -20,7 +20,17 @@ public class NotificationRecipient /// Email address of the recipient. public NotificationRecipient(string name, string emailAddress) { - Name = name ?? throw new ArgumentNullException(nameof(name)); + // Match the ForEmail factory's guard so the invariant ("a recipient always + // has a non-blank display name") holds regardless of construction path. EF + // materializes via the private parameterless ctor + property injection — an + // SMS-only recipient has a null EmailAddress — so this ctor is only reached + // by code that genuinely intends the email path. + if (string.IsNullOrWhiteSpace(name)) + { + throw new ArgumentException("Name must not be empty.", nameof(name)); + } + + Name = name; EmailAddress = emailAddress ?? throw new ArgumentNullException(nameof(emailAddress)); } diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Entities/Notifications/SmsConfiguration.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Entities/Notifications/SmsConfiguration.cs index ececac0f..063c6f47 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Entities/Notifications/SmsConfiguration.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Entities/Notifications/SmsConfiguration.cs @@ -17,11 +17,25 @@ public class SmsConfiguration public string? MessagingServiceSid { get; set; } /// Gets or sets the Twilio REST API base URL, or null to use the provider default. public string? ApiBaseUrl { get; set; } - /// Gets or sets the connection timeout in seconds. + /// Gets or sets the connection timeout in seconds. Honored per-send by the SMS delivery adapter. public int ConnectionTimeoutSeconds { get; set; } - /// Gets or sets the maximum number of delivery retries before parking. + /// + /// Gets or sets the maximum number of delivery retries before parking. + /// + /// RESERVED: the Notification Outbox dispatcher currently derives the retry policy + /// (max-retries + interval) from the central SMTP configuration for all + /// notification types — see the "retry reuses central SMTP max-retry-count and fixed + /// interval" design decision and NotificationOutboxActor.ResolveRetryPolicyAsync. + /// This per-SMS value is persisted/transported for forward-compatibility but is NOT + /// yet read at dispatch time. Honoring it per-type is a deferred enhancement (it would + /// supersede the shared-SMTP-policy decision and is not a silent behavioral change). + /// + /// public int MaxRetries { get; set; } - /// Gets or sets the delay between retry attempts. + /// + /// Gets or sets the delay between retry attempts. RESERVED — see : + /// the dispatcher currently uses the shared SMTP-derived retry interval for all types. + /// public TimeSpan RetryDelay { get; set; } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Migrations/20260619135543_AddSmsNotifications.cs b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Migrations/20260619135543_AddSmsNotifications.cs index 15f8d047..84049d42 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Migrations/20260619135543_AddSmsNotifications.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Migrations/20260619135543_AddSmsNotifications.cs @@ -51,6 +51,17 @@ IF OBJECT_ID(N'dbo.SmsConfigurations', N'U') IS NULL /// protected override void Down(MigrationBuilder migrationBuilder) { + // ConfigurationDatabase-NNN: refuse to roll back if SMS-only recipients exist. + // Such a row carries a PhoneNumber and a (legitimately) NULL EmailAddress. The + // backfill below sets every NULL EmailAddress to '' so the column can revert to + // NOT NULL — but for an SMS-only recipient that silently destroys its contact + // info (PhoneNumber is also dropped) and leaves an invalid empty-string email. + // This guard MUST run before DROP COLUMN [PhoneNumber], while the column is still + // queryable, so the operator gets a clear error instead of silent data loss. + migrationBuilder.Sql(@" +IF EXISTS (SELECT 1 FROM [NotificationRecipients] WHERE [EmailAddress] IS NULL AND [PhoneNumber] IS NOT NULL) + THROW 60000, 'Cannot roll back AddSmsNotifications: SMS-only recipients (PhoneNumber set, EmailAddress NULL) exist. Reassign or delete them before rolling back -- otherwise their PhoneNumber would be dropped and EmailAddress backfilled to an invalid empty string.', 1;"); + migrationBuilder.Sql(@" IF OBJECT_ID(N'dbo.SmsConfigurations', N'U') IS NOT NULL DROP TABLE [SmsConfigurations];"); @@ -61,6 +72,8 @@ IF EXISTS (SELECT 1 FROM sys.columns WHERE Name='PhoneNumber' AND Object_ID=Obje // Reversing to NOT NULL requires backfilling existing NULLs first, otherwise the // ALTER fails. Mirrors EF's generated default ('') for the previously-required column. + // The guard above guarantees any remaining NULL here belongs to a pre-existing email + // recipient that never had an address, not an SMS-only recipient. migrationBuilder.Sql("UPDATE [NotificationRecipients] SET [EmailAddress] = '' WHERE [EmailAddress] IS NULL;"); migrationBuilder.Sql("ALTER TABLE [NotificationRecipients] ALTER COLUMN [EmailAddress] nvarchar(500) NOT NULL;"); } diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index e2090302..36b91662 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -174,7 +174,14 @@ public class ManagementActor : ReceiveActor // should use the REST endpoint; this command is retained for // backward compatibility with the CentralUI Configuration Audit // Log page (Management-018). - or QueryAuditLogCommand => Roles.Administrator, + or QueryAuditLogCommand + // MgmtSvc-021: SMTP/SMS provider-config updates rotate stored secrets + // (SMTP Credentials, Twilio AuthToken). Both the /notifications/smtp and + // /notifications/sms Central UI pages enforce RequireAdmin, so the actor + // 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 => Roles.Administrator, // Designer operations CreateAreaCommand or DeleteAreaCommand @@ -186,8 +193,6 @@ public class ManagementActor : ReceiveActor or DeleteExternalSystemMethodCommand or CreateNotificationListCommand or UpdateNotificationListCommand or DeleteNotificationListCommand - or UpdateSmtpConfigCommand - or UpdateSmsConfigCommand or CreateDataConnectionCommand or UpdateDataConnectionCommand or DeleteDataConnectionCommand or MoveDataConnectionCommand or AddTemplateAttributeCommand or UpdateTemplateAttributeCommand or DeleteTemplateAttributeCommand @@ -1838,7 +1843,7 @@ public class ManagementActor : ReceiveActor var repo = sp.GetRequiredService(); var configs = await repo.GetAllSmsConfigurationsAsync(); // MgmtSvc-020: project away the AuthToken field — read access to this - // list is broader than the Designer-gated UpdateSmsConfig path that owns + // list is broader than the Admin-only UpdateSmsConfig path that owns // the secret. return configs.Select(SmsConfigPublicShape).ToList(); } @@ -1856,7 +1861,13 @@ public class ManagementActor : ReceiveActor // existing values intact (non-breaking for callers that do not send them, // and so the secret AuthToken survives a config edit that does not rotate it). if (cmd.ApiBaseUrl is not null) config.ApiBaseUrl = cmd.ApiBaseUrl; - if (cmd.AuthToken is not null) config.AuthToken = cmd.AuthToken; + // MgmtSvc-021: treat an empty/whitespace AuthToken as "omitted", not "clear". + // A Twilio Auth Token is always required, so an empty value is never a valid + // rotation — guarding against IsNullOrEmpty (not just null) stops an accidental + // `--auth-token ""` from silently wiping the stored secret and 401-ing every + // subsequent send. (Contrast SmtpConfiguration.Credentials, which MAY be cleared + // when switching to anonymous SMTP, so that path keeps its null-only guard.) + if (!string.IsNullOrWhiteSpace(cmd.AuthToken)) config.AuthToken = cmd.AuthToken; await repo.UpdateSmsConfigurationAsync(config); await repo.SaveChangesAsync(); // MgmtSvc-020: audit the credential-free shape — the AuthToken secret is diff --git a/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Delivery/SmsNotificationDeliveryAdapter.cs b/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Delivery/SmsNotificationDeliveryAdapter.cs index c064a163..b8c8c5eb 100644 --- a/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Delivery/SmsNotificationDeliveryAdapter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Delivery/SmsNotificationDeliveryAdapter.cs @@ -364,7 +364,18 @@ public sealed class SmsNotificationDeliveryAdapter : INotificationDeliveryAdapte return composed[..max]; } - return string.Concat(composed.AsSpan(0, max - ellipsis.Length), ellipsis); + // Cut at (max - ellipsis) UTF-16 code units, backing off by one if that boundary + // would split a surrogate pair (e.g. an emoji in the alarm subject/body). `cut` + // is the index of the first dropped char, so a split is signalled by a high + // surrogate immediately before it; dropping that lone surrogate keeps the body + // well-formed and still within the cap. + var cut = max - ellipsis.Length; + if (char.IsHighSurrogate(composed[cut - 1])) + { + cut--; + } + + return string.Concat(composed.AsSpan(0, cut), ellipsis); } /// The classified outcome of a single per-recipient Twilio attempt.