diff --git a/docs/test_infra/test_infra_smtp.md b/docs/test_infra/test_infra_smtp.md index 76f72d4..a9dceab 100644 --- a/docs/test_infra/test_infra_smtp.md +++ b/docs/test_infra/test_infra_smtp.md @@ -29,18 +29,30 @@ For `appsettings.Development.json` (Notification Service): "Smtp": { "Server": "localhost", "Port": 1025, - "AuthMode": "None", + "AuthMode": "Basic", + "Credentials": "test:test", + "TlsMode": "None", "FromAddress": "scada-notifications@company.com", "ConnectionTimeout": 30 } } ``` -Since `MP_SMTP_AUTH_ACCEPT_ANY` is enabled, the Notification Service can use any auth mode: -- **No auth**: Connect directly, no credentials needed. -- **Basic Auth**: Any username/password will be accepted (useful for testing the auth code path without a real server). +> **`Server` host**: use `localhost` only when the Notification Service runs directly on +> the host. When it runs inside the docker cluster, set `Server` to the container name +> `scadalink-smtp` — the cluster compose stack and the infra compose stack share the +> `scadalink-net` network, so the container is reachable by name. + +The delivery service (`MailKitSmtpClientWrapper`) only accepts `Basic` or `OAuth2` — +there is no "no auth" mode — so the working config above uses `Basic`: +- **Basic Auth**: `MP_SMTP_AUTH_ACCEPT_ANY` makes Mailpit accept any `username:password`, + so use a throwaway value such as `test:test`. This exercises the real auth code path + without a real server. - **OAuth2**: Not supported by Mailpit. For OAuth2 testing, use a real Microsoft 365 tenant. +`TlsMode` **must** be `None`: Mailpit on port 1025 is plain SMTP and does not offer +STARTTLS. `StartTLS` or `SSL` would fail the connection. + ## Mailpit API Mailpit exposes a REST API at `http://localhost:8025/api` for programmatic access: diff --git a/src/ScadaLink.CLI/Commands/NotificationCommands.cs b/src/ScadaLink.CLI/Commands/NotificationCommands.cs index 82b1d8f..685599b 100644 --- a/src/ScadaLink.CLI/Commands/NotificationCommands.cs +++ b/src/ScadaLink.CLI/Commands/NotificationCommands.cs @@ -69,33 +69,72 @@ public static class NotificationCommands }); group.Add(listCmd); - var idOption = new Option("--id") { Description = "SMTP config ID", Required = true }; - var serverOption = new Option("--server") { Description = "SMTP server", Required = true }; - var portOption = new Option("--port") { Description = "SMTP port", Required = true }; - var authModeOption = new Option("--auth-mode") { Description = "Auth mode", Required = true }; - var fromOption = new Option("--from-address") { Description = "From email address", Required = true }; var updateCmd = new Command("update") { Description = "Update SMTP configuration" }; - updateCmd.Add(idOption); - updateCmd.Add(serverOption); - updateCmd.Add(portOption); - updateCmd.Add(authModeOption); - updateCmd.Add(fromOption); + updateCmd.Add(SmtpIdOption); + updateCmd.Add(SmtpServerOption); + updateCmd.Add(SmtpPortOption); + updateCmd.Add(SmtpAuthModeOption); + updateCmd.Add(SmtpFromOption); + updateCmd.Add(SmtpTlsModeOption); + updateCmd.Add(SmtpCredentialsOption); updateCmd.SetAction(async (ParseResult result) => { - var id = result.GetValue(idOption); - var server = result.GetValue(serverOption)!; - var port = result.GetValue(portOption); - var authMode = result.GetValue(authModeOption)!; - var from = result.GetValue(fromOption)!; return await CommandHelpers.ExecuteCommandAsync( result, urlOption, formatOption, usernameOption, passwordOption, - new UpdateSmtpConfigCommand(id, server, port, authMode, from)); + BuildUpdateSmtpConfigCommand(result)); }); group.Add(updateCmd); return group; } + // SMTP update options are static so the parsed values can be read back both + // from the SetAction and from BuildUpdateSmtpConfigCommand (used by tests). + private static readonly Option SmtpIdOption = + new("--id") { Description = "SMTP config ID", Required = true }; + private static readonly Option SmtpServerOption = + new("--server") { Description = "SMTP server", Required = true }; + private static readonly Option SmtpPortOption = + new("--port") { Description = "SMTP port", Required = true }; + private static readonly Option SmtpAuthModeOption = + new("--auth-mode") { Description = "Auth mode", Required = true }; + private static readonly Option SmtpFromOption = + new("--from-address") { Description = "From email address", Required = true }; + private static readonly Option SmtpTlsModeOption = CreateTlsModeOption(); + private static readonly Option SmtpCredentialsOption = + new("--credentials") + { + Description = "SMTP credentials — 'username:password' for Basic, or client secret " + + "for OAuth2 (optional; preserves existing if omitted)", + }; + + private static Option CreateTlsModeOption() + { + var option = new Option("--tls-mode") + { + Description = "TLS mode: None, StartTLS, or SSL (optional; preserves existing if omitted)", + }; + option.AcceptOnlyFromAmong("None", "StartTLS", "SSL"); + return option; + } + + /// + /// Builds the from a parsed smtp update + /// invocation. The optional --tls-mode / --credentials flags map to + /// null when omitted so the server-side handler preserves the existing values. + /// + internal static UpdateSmtpConfigCommand BuildUpdateSmtpConfigCommand(ParseResult result) + { + var id = result.GetValue(SmtpIdOption); + var server = result.GetValue(SmtpServerOption)!; + var port = result.GetValue(SmtpPortOption); + var authMode = result.GetValue(SmtpAuthModeOption)!; + var from = result.GetValue(SmtpFromOption)!; + var tlsMode = result.GetValue(SmtpTlsModeOption); + var credentials = result.GetValue(SmtpCredentialsOption); + return new UpdateSmtpConfigCommand(id, server, port, authMode, from, tlsMode, credentials); + } + private static Command BuildList(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var cmd = new Command("list") { Description = "List all notification lists" }; diff --git a/src/ScadaLink.CentralUI/Components/Pages/Notifications/SmtpConfiguration.razor b/src/ScadaLink.CentralUI/Components/Pages/Notifications/SmtpConfiguration.razor index 3f87a6a..d938b9b 100644 --- a/src/ScadaLink.CentralUI/Components/Pages/Notifications/SmtpConfiguration.razor +++ b/src/ScadaLink.CentralUI/Components/Pages/Notifications/SmtpConfiguration.razor @@ -50,6 +50,8 @@
@smtp.Host:@smtp.Port
Auth Type
@smtp.AuthType
+
TLS Mode
+
@(string.IsNullOrWhiteSpace(smtp.TlsMode) ? "(not set)" : smtp.TlsMode)
From Address
@smtp.FromAddress
Credentials
@@ -73,13 +75,21 @@ -
+
+
+ + +
R public record UpdateNotificationListCommand(int NotificationListId, string Name, IReadOnlyList RecipientEmails); public record DeleteNotificationListCommand(int NotificationListId); public record ListSmtpConfigsCommand; -public record UpdateSmtpConfigCommand(int SmtpConfigId, string Server, int Port, string AuthMode, string FromAddress); +public record UpdateSmtpConfigCommand(int SmtpConfigId, string Server, int Port, string AuthMode, string FromAddress, string? TlsMode = null, string? Credentials = null); diff --git a/src/ScadaLink.ManagementService/ManagementActor.cs b/src/ScadaLink.ManagementService/ManagementActor.cs index 3d3f784..10e79d7 100644 --- a/src/ScadaLink.ManagementService/ManagementActor.cs +++ b/src/ScadaLink.ManagementService/ManagementActor.cs @@ -1124,6 +1124,10 @@ public class ManagementActor : ReceiveActor config.Port = cmd.Port; config.AuthType = cmd.AuthMode; config.FromAddress = cmd.FromAddress; + // Preserve-if-null: an update that omits TlsMode/Credentials leaves the + // existing values intact (non-breaking for callers that do not send them). + if (cmd.TlsMode is not null) config.TlsMode = cmd.TlsMode; + if (cmd.Credentials is not null) config.Credentials = cmd.Credentials; await repo.UpdateSmtpConfigurationAsync(config); await repo.SaveChangesAsync(); await AuditAsync(sp, user, "Update", "SmtpConfiguration", config.Id.ToString(), config.Host, config); diff --git a/tests/ScadaLink.CLI.Tests/Commands/SmtpUpdateCommandTests.cs b/tests/ScadaLink.CLI.Tests/Commands/SmtpUpdateCommandTests.cs new file mode 100644 index 0000000..d644a53 --- /dev/null +++ b/tests/ScadaLink.CLI.Tests/Commands/SmtpUpdateCommandTests.cs @@ -0,0 +1,104 @@ +using System.CommandLine; +using ScadaLink.CLI.Commands; +using ScadaLink.Commons.Messages.Management; + +namespace ScadaLink.CLI.Tests.Commands; + +/// +/// Tests for the scadalink notification smtp update subcommand. The command +/// gained two optional flags — --tls-mode and --credentials — that plumb +/// through to . These tests pin that the flags +/// parse, are genuinely optional (non-breaking), and that --tls-mode rejects +/// values outside the canonical {None, StartTLS, SSL} set. +/// +public class SmtpUpdateCommandTests +{ + private static readonly Option Url = new("--url") { Recursive = true }; + private static readonly Option Username = new("--username") { Recursive = true }; + private static readonly Option Password = new("--password") { Recursive = true }; + private static readonly Option Format = CliOptions.CreateFormatOption(); + + private static Command SmtpUpdateCommand() + { + var notification = NotificationCommands.Build(Url, Format, Username, Password); + var smtp = notification.Subcommands.Single(c => c.Name == "smtp"); + return smtp.Subcommands.Single(c => c.Name == "update"); + } + + private static ParseResult ParseUpdate(params string[] args) + => SmtpUpdateCommand().Parse(args); + + [Fact] + public void Update_WithTlsModeAndCredentials_ProducesCommandCarryingThem() + { + var parse = ParseUpdate( + "--id", "1", "--server", "smtp.example.com", "--port", "587", + "--auth-mode", "Basic", "--from-address", "noreply@example.com", + "--tls-mode", "None", "--credentials", "user:pass"); + + Assert.Empty(parse.Errors); + var cmd = NotificationCommands.BuildUpdateSmtpConfigCommand(parse); + + Assert.Equal(1, cmd.SmtpConfigId); + Assert.Equal("smtp.example.com", cmd.Server); + Assert.Equal(587, cmd.Port); + Assert.Equal("Basic", cmd.AuthMode); + Assert.Equal("noreply@example.com", cmd.FromAddress); + Assert.Equal("None", cmd.TlsMode); + Assert.Equal("user:pass", cmd.Credentials); + } + + [Fact] + public void Update_WithoutTlsModeAndCredentials_ProducesCommandWithNulls() + { + var parse = ParseUpdate( + "--id", "2", "--server", "smtp.example.com", "--port", "25", + "--auth-mode", "OAuth2", "--from-address", "noreply@example.com"); + + Assert.Empty(parse.Errors); + var cmd = NotificationCommands.BuildUpdateSmtpConfigCommand(parse); + + Assert.Equal(2, cmd.SmtpConfigId); + Assert.Null(cmd.TlsMode); + Assert.Null(cmd.Credentials); + } + + [Theory] + [InlineData("None")] + [InlineData("StartTLS")] + [InlineData("SSL")] + public void Update_TlsModeOption_AcceptsCanonicalValues(string value) + { + var parse = ParseUpdate( + "--id", "1", "--server", "smtp.example.com", "--port", "587", + "--auth-mode", "Basic", "--from-address", "noreply@example.com", + "--tls-mode", value); + + Assert.Empty(parse.Errors); + } + + [Theory] + [InlineData("Bogus")] + [InlineData("tls")] + [InlineData("none")] // AcceptOnlyFromAmong is case-sensitive: constrain to canonical spelling + public void Update_TlsModeOption_RejectsValuesOutsideCanonicalSet(string value) + { + var parse = ParseUpdate( + "--id", "1", "--server", "smtp.example.com", "--port", "587", + "--auth-mode", "Basic", "--from-address", "noreply@example.com", + "--tls-mode", value); + + Assert.NotEmpty(parse.Errors); + } + + [Fact] + public void Update_TlsModeAndCredentials_AreNotRequired() + { + var update = SmtpUpdateCommand(); + var tls = update.Options.Single(o => o.Name == "--tls-mode"); + var creds = update.Options.Single(o => o.Name == "--credentials"); + + Assert.False(tls.Required, "--tls-mode must be optional (preserve-if-omitted)."); + Assert.False(creds.Required, "--credentials must be optional (preserve-if-omitted)."); + } +} diff --git a/tests/ScadaLink.CentralUI.Tests/Pages/SmtpConfigurationPageTests.cs b/tests/ScadaLink.CentralUI.Tests/Pages/SmtpConfigurationPageTests.cs new file mode 100644 index 0000000..74a3797 --- /dev/null +++ b/tests/ScadaLink.CentralUI.Tests/Pages/SmtpConfigurationPageTests.cs @@ -0,0 +1,112 @@ +using System.Security.Claims; +using Bunit; +using Microsoft.AspNetCore.Components.Authorization; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; +using ScadaLink.Commons.Entities.Notifications; +using ScadaLink.Commons.Interfaces.Repositories; +using SmtpConfigurationPage = ScadaLink.CentralUI.Components.Pages.Notifications.SmtpConfiguration; + +namespace ScadaLink.CentralUI.Tests.Pages; + +/// +/// bUnit rendering tests for the SMTP Configuration page — specifically the TlsMode +/// field added so the UI exposes all five user-relevant SmtpConfiguration fields. +/// +public class SmtpConfigurationPageTests : BunitContext +{ + private void WireAuth() + { + var claims = new[] + { + new Claim("Username", "tester"), + new Claim(ClaimTypes.Role, "Admin"), + }; + var user = new ClaimsPrincipal(new ClaimsIdentity(claims, "TestAuth")); + Services.AddSingleton(new TestAuthStateProvider(user)); + Services.AddAuthorizationCore(); + } + + private static SmtpConfiguration Sample() => + new("smtp.example.com", "Basic", "noreply@example.com") + { + Id = 1, + Port = 587, + TlsMode = "StartTLS", + Credentials = "user:pass", + }; + + [Fact] + public void EditForm_RendersTlsModeSelectWithAllThreeModes() + { + var repo = Substitute.For(); + repo.GetAllSmtpConfigurationsAsync() + .Returns(Task.FromResult>( + new List { Sample() })); + Services.AddSingleton(repo); + WireAuth(); + + var cut = Render(); + cut.WaitForState(() => cut.Markup.Contains("smtp.example.com")); + + cut.FindAll("button").First(b => b.TextContent.Contains("Edit")).Click(); + + cut.WaitForAssertion(() => + { + var selects = cut.FindAll("select"); + var tlsSelect = selects.Single(s => s.QuerySelectorAll("option") + .Any(o => o.TextContent == "StartTLS")); + var modes = tlsSelect.QuerySelectorAll("option").Select(o => o.TextContent).ToList(); + Assert.Equal(new[] { "None", "StartTLS", "SSL" }, modes); + }); + } + + [Fact] + public void ReadOnlyView_ShowsTlsMode() + { + var repo = Substitute.For(); + repo.GetAllSmtpConfigurationsAsync() + .Returns(Task.FromResult>( + new List { Sample() })); + Services.AddSingleton(repo); + WireAuth(); + + var cut = Render(); + + cut.WaitForAssertion(() => + { + Assert.Contains("TLS Mode", cut.Markup); + Assert.Contains("StartTLS", cut.Markup); + }); + } + + [Fact] + public void SavingEdit_PersistsChosenTlsMode() + { + var config = Sample(); + var repo = Substitute.For(); + repo.GetAllSmtpConfigurationsAsync() + .Returns(Task.FromResult>( + new List { config })); + Services.AddSingleton(repo); + WireAuth(); + + var cut = Render(); + cut.WaitForState(() => cut.Markup.Contains("smtp.example.com")); + + cut.FindAll("button").First(b => b.TextContent.Contains("Edit")).Click(); + + var tlsSelect = cut.FindAll("select") + .Single(s => s.QuerySelectorAll("option").Any(o => o.TextContent == "StartTLS")); + tlsSelect.Change("SSL"); + + cut.FindAll("button").First(b => b.TextContent.Contains("Save")).Click(); + + cut.WaitForAssertion(() => + { + repo.Received().UpdateSmtpConfigurationAsync( + Arg.Is(c => c.TlsMode == "SSL")); + repo.Received().SaveChangesAsync(); + }); + } +} diff --git a/tests/ScadaLink.ManagementService.Tests/ManagementActorTests.cs b/tests/ScadaLink.ManagementService.Tests/ManagementActorTests.cs index d2b4787..8bad690 100644 --- a/tests/ScadaLink.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ScadaLink.ManagementService.Tests/ManagementActorTests.cs @@ -1002,6 +1002,72 @@ public class ManagementActorTests : TestKit, IDisposable Assert.Contains(envelope.CorrelationId, response.Error); } + // ======================================================================== + // UpdateSmtpConfig — TlsMode + Credentials plumbing (preserve-if-null) + // ======================================================================== + + [Fact] + public void UpdateSmtpConfig_WithTlsModeAndCredentials_PersistsThem() + { + var notifRepo = Substitute.For(); + var existing = new Commons.Entities.Notifications.SmtpConfiguration( + "old.example.com", "OAuth2", "old@example.com") + { + Id = 1, + Port = 25, + TlsMode = "StartTLS", + Credentials = "old-secret", + }; + notifRepo.GetSmtpConfigurationByIdAsync(1, Arg.Any()).Returns(existing); + _services.AddScoped(_ => notifRepo); + + var actor = CreateActor(); + var envelope = Envelope( + new UpdateSmtpConfigCommand(1, "new.example.com", 465, "Basic", "new@example.com", "SSL", "user:pass"), + "Design"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(envelope.CorrelationId, response.CorrelationId); + Assert.Equal("SSL", existing.TlsMode); + Assert.Equal("user:pass", existing.Credentials); + Assert.Equal("new.example.com", existing.Host); + Assert.Equal("Basic", existing.AuthType); + } + + [Fact] + public void UpdateSmtpConfig_WithNullTlsModeAndCredentials_PreservesExistingValues() + { + var notifRepo = Substitute.For(); + var existing = new Commons.Entities.Notifications.SmtpConfiguration( + "old.example.com", "OAuth2", "old@example.com") + { + Id = 1, + Port = 25, + TlsMode = "StartTLS", + Credentials = "old-secret", + }; + notifRepo.GetSmtpConfigurationByIdAsync(1, Arg.Any()).Returns(existing); + _services.AddScoped(_ => notifRepo); + + var actor = CreateActor(); + var envelope = Envelope( + new UpdateSmtpConfigCommand(1, "new.example.com", 465, "Basic", "new@example.com"), + "Design"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(envelope.CorrelationId, response.CorrelationId); + // Omitted fields are preserved, not nulled. + Assert.Equal("StartTLS", existing.TlsMode); + Assert.Equal("old-secret", existing.Credentials); + // Provided fields are still updated. + Assert.Equal("new.example.com", existing.Host); + Assert.Equal("Basic", existing.AuthType); + } + [Fact] public void CuratedHandlerFailure_SurfacesTheCuratedMessage() {