From 2dd5e318d74b3acb78fd293faecc19248cefe848 Mon Sep 17 00:00:00 2001 From: Anatolii Hrynchuk Date: Sun, 18 Oct 2026 00:17:25 +0300 Subject: [PATCH] fix(notification): enable web smtp test with saved password Allow Web SMTP egress on TCP 587 and reuse the saved channel password only when connection settings match. Ref: IT-1601 --- .../AdminSmtpEditorViewTests.cs | 34 ++++++ .../AdminSmtpTestControllerTests.cs | 112 ++++++++++++++++++ .../RecordingAdminSmtpMediator.cs | 43 +++++++ .../Admin/AdminChannelsController.cs | 24 +++- .../Controllers/Admin/TestSmtpRequest.cs | 6 + .../Views/AdminChannels/Edit.cshtml | 4 +- .../kubernetes/shared-notification/README.md | 11 ++ .../production-network.yaml | 29 +++++ docs/email-template-definition-api.md | 2 + 9 files changed, 260 insertions(+), 5 deletions(-) create mode 100644 HrynCo.NotificationService.Web.IntegrationTests/AdminSmtpEditorViewTests.cs create mode 100644 HrynCo.NotificationService.Web.IntegrationTests/AdminSmtpTestControllerTests.cs create mode 100644 HrynCo.NotificationService.Web.IntegrationTests/RecordingAdminSmtpMediator.cs create mode 100644 HrynCo.NotificationService.Web/Controllers/Admin/TestSmtpRequest.cs diff --git a/HrynCo.NotificationService.Web.IntegrationTests/AdminSmtpEditorViewTests.cs b/HrynCo.NotificationService.Web.IntegrationTests/AdminSmtpEditorViewTests.cs new file mode 100644 index 0000000..bb57edb --- /dev/null +++ b/HrynCo.NotificationService.Web.IntegrationTests/AdminSmtpEditorViewTests.cs @@ -0,0 +1,34 @@ +using HrynCo.NotificationService.DAL.Abstract.Providers; +using HrynCo.NotificationService.DAL.Abstract.Repositories; +using HrynCo.NotificationService.DAL.EF; +using Microsoft.Extensions.DependencyInjection; + +namespace HrynCo.NotificationService.Web.IntegrationTests; + +public sealed class AdminSmtpEditorViewTests(TemplateDatabaseFixture fixture) : IClassFixture +{ + [Fact] + public async Task ExistingChannelTestIncludesIdWithoutRenderingSavedPassword() + { + await using var context = await DefinitionApiTestContext.CreateAsync(fixture); + using var scope = context.App.Services.CreateScope(); + var channel = new EmailChannel + { + ServiceName = "TestService", + EmailChannelType = EmailChannelType.Smtp, + Settings = new SmtpChannelSettings + { + Host = "smtp.example.invalid", Username = "test-user", + Password = "saved-secret-must-not-render", FromEmail = "sender@example.invalid" + } + }; + await scope.ServiceProvider.GetRequiredService().AddAsync(channel); + await scope.ServiceProvider.GetRequiredService().SaveChangesAsync(); + + var html = await context.Client.GetStringAsync($"/admin/channels/{channel.Id}"); + + Assert.Contains($"value=\"{channel.Id}\"", html); + Assert.Contains("const channelId = form.querySelector('[name=\"Id\"]').value", html); + Assert.DoesNotContain("saved-secret-must-not-render", html); + } +} diff --git a/HrynCo.NotificationService.Web.IntegrationTests/AdminSmtpTestControllerTests.cs b/HrynCo.NotificationService.Web.IntegrationTests/AdminSmtpTestControllerTests.cs new file mode 100644 index 0000000..856d8e1 --- /dev/null +++ b/HrynCo.NotificationService.Web.IntegrationTests/AdminSmtpTestControllerTests.cs @@ -0,0 +1,112 @@ +using System.Reflection; +using System.Text.Json; +using HrynCo.NotificationService.DAL.Abstract.Providers; +using HrynCo.NotificationService.Web.Controllers.Admin; +using MediatR; +using Microsoft.AspNetCore.Mvc; + +namespace HrynCo.NotificationService.Web.IntegrationTests; + +public sealed class AdminSmtpTestControllerTests +{ + private static readonly Guid ChannelId = Guid.NewGuid(); + + [Fact] + public async Task BlankPasswordUsesSavedSecretAndCurrentUnsavedSender() + { + var (controller, recorder) = CreateController(); + var response = await controller.TestSmtp(Request(), CancellationToken.None); + + Assert.True(Succeeded(response)); + Assert.Equal("saved-test-secret", recorder.SentCommand?.Password); + Assert.Equal("changed@example.invalid", recorder.SentCommand?.FromEmail); + Assert.Equal("Changed sender", recorder.SentCommand?.FromName); + Assert.Equal(1, recorder.LookupCount); + Assert.DoesNotContain("saved-test-secret", JsonSerializer.Serialize(((OkObjectResult)response).Value)); + } + + [Fact] + public async Task ExplicitPasswordOverridesSavedWithoutLookup() + { + var (controller, recorder) = CreateController(); + var response = await controller.TestSmtp(Request() with { Password = "typed-test-secret" }, CancellationToken.None); + + Assert.True(Succeeded(response)); + Assert.Equal("typed-test-secret", recorder.SentCommand?.Password); + Assert.Equal(0, recorder.LookupCount); + } + + [Theory] + [InlineData("host")] + [InlineData("port")] + [InlineData("username")] + [InlineData("ssl")] + public async Task ChangedConnectionRequiresExplicitPassword(string field) + { + var (controller, recorder) = CreateController(); + var request = field switch + { + "host" => Request() with { Host = "other.example.invalid" }, + "port" => Request() with { Port = 2525 }, + "username" => Request() with { Username = "other-user" }, + _ => Request() with { UseSsl = false } + }; + + var response = await controller.TestSmtp(request, CancellationToken.None); + + Assert.False(Succeeded(response)); + Assert.Null(recorder.SentCommand); + } + + [Fact] + public async Task UnknownChannelDoesNotSend() + { + var (controller, recorder) = CreateController(); + recorder.Channel = null; + + var response = await controller.TestSmtp(Request(), CancellationToken.None); + + Assert.False(Succeeded(response)); + Assert.Null(recorder.SentCommand); + } + + [Fact] + public async Task NewChannelKeepsAdHocTestBehavior() + { + var (controller, recorder) = CreateController(); + + var response = await controller.TestSmtp(Request() with { ChannelId = null }, CancellationToken.None); + + Assert.True(Succeeded(response)); + Assert.Equal(string.Empty, recorder.SentCommand?.Password); + Assert.Equal(0, recorder.LookupCount); + } + + private static (AdminChannelsController Controller, RecordingAdminSmtpMediator Recorder) CreateController() + { + var mediator = DispatchProxy.Create(); + var recorder = (RecordingAdminSmtpMediator)mediator; + recorder.Channel = new EmailChannel + { + Id = ChannelId, + ServiceName = "TestService", + EmailChannelType = EmailChannelType.Smtp, + Settings = new SmtpChannelSettings + { + Host = "smtp.example.invalid", Port = 587, + Username = "test-user", Password = "saved-test-secret", UseSsl = true + } + }; + return (new AdminChannelsController(mediator), recorder); + } + + private static TestSmtpRequest Request() => new( + "smtp.example.invalid", 587, "test-user", "", true, + "changed@example.invalid", "Changed sender", "recipient@example.invalid", ChannelId); + + private static bool Succeeded(IActionResult response) + { + var body = JsonSerializer.Serialize(((OkObjectResult)response).Value); + return JsonDocument.Parse(body).RootElement.GetProperty("success").GetBoolean(); + } +} diff --git a/HrynCo.NotificationService.Web.IntegrationTests/RecordingAdminSmtpMediator.cs b/HrynCo.NotificationService.Web.IntegrationTests/RecordingAdminSmtpMediator.cs new file mode 100644 index 0000000..b52b25a --- /dev/null +++ b/HrynCo.NotificationService.Web.IntegrationTests/RecordingAdminSmtpMediator.cs @@ -0,0 +1,43 @@ +using System.Reflection; +using HrynCo.NotificationService.DAL.Abstract.Providers; +using HrynCo.NotificationService.Services.Core; +using HrynCo.NotificationService.Services.EmailChannels.Get; +using HrynCo.NotificationService.Services.EmailChannels.TestSmtp; +using MediatR; +using ServiceUnit = HrynCo.NotificationService.Services.Core.Unit; + +namespace HrynCo.NotificationService.Web.IntegrationTests; + +public class RecordingAdminSmtpMediator : DispatchProxy +{ + public EmailChannel? Channel { get; set; } + public TestSmtpCommand? SentCommand { get; private set; } + public int LookupCount { get; private set; } + + protected override object Invoke(MethodInfo? targetMethod, object?[]? args) + { + if (targetMethod?.Name != nameof(IMediator.Send) || args?.Length is null or 0) + throw new NotSupportedException(); + + return args[0] switch + { + GetEmailChannelQuery => RecordLookup(), + TestSmtpCommand command => RecordSend(command), + _ => throw new NotSupportedException() + }; + } + + private Task> RecordLookup() + { + LookupCount++; + return Task.FromResult(Channel is null + ? ServiceResult.Failure("Channel not found.") + : ServiceResult.Success(Channel)); + } + + private Task> RecordSend(TestSmtpCommand command) + { + SentCommand = command; + return Task.FromResult(ServiceResult.Success(ServiceUnit.Value)); + } +} diff --git a/HrynCo.NotificationService.Web/Controllers/Admin/AdminChannelsController.cs b/HrynCo.NotificationService.Web/Controllers/Admin/AdminChannelsController.cs index 6810f58..792e83c 100644 --- a/HrynCo.NotificationService.Web/Controllers/Admin/AdminChannelsController.cs +++ b/HrynCo.NotificationService.Web/Controllers/Admin/AdminChannelsController.cs @@ -138,8 +138,21 @@ public class AdminChannelsController(IMediator mediator) : Controller [HttpPost("test-smtp")] public async Task TestSmtp([FromBody] TestSmtpRequest request, CancellationToken ct) { + var password = request.Password; + if (string.IsNullOrEmpty(password) && request.ChannelId is Guid channelId) + { + var channelResult = await mediator.Send(new GetEmailChannelQuery(channelId), ct); + if (!channelResult.IsSuccess || channelResult.Result?.Settings is not SmtpChannelSettings saved) + return Ok(new { success = false, message = "SMTP channel not found." }); + + if (!MatchesSavedSmtpConnection(request, saved)) + return Ok(new { success = false, message = "Enter the password to test changed SMTP connection settings." }); + + password = saved.Password; + } + var result = await mediator.Send(new TestSmtpCommand( - request.Host, request.Port, request.Username, request.Password, + request.Host, request.Port, request.Username, password ?? string.Empty, request.UseSsl, request.FromEmail, request.FromName, request.ToEmail), ct); if (!result.IsSuccess) @@ -148,6 +161,12 @@ public class AdminChannelsController(IMediator mediator) : Controller return Ok(new { success = true, message = $"Test email sent to {request.ToEmail}." }); } + private static bool MatchesSavedSmtpConnection(TestSmtpRequest request, SmtpChannelSettings saved) => + string.Equals(request.Host, saved.Host, StringComparison.OrdinalIgnoreCase) + && request.Port == saved.Port + && string.Equals(request.Username, saved.Username, StringComparison.Ordinal) + && request.UseSsl == saved.UseSsl; + // POST /admin/channels/{id}/test [HttpPost("{id:guid}/test")] public async Task Test(Guid id, [FromBody] TestChannelRequest request, CancellationToken ct) @@ -180,6 +199,3 @@ public class AdminChannelsController(IMediator mediator) : Controller } public record TestChannelRequest(string ToEmail); -public record TestSmtpRequest( - string Host, int Port, string Username, string Password, - bool UseSsl, string FromEmail, string FromName, string ToEmail); diff --git a/HrynCo.NotificationService.Web/Controllers/Admin/TestSmtpRequest.cs b/HrynCo.NotificationService.Web/Controllers/Admin/TestSmtpRequest.cs new file mode 100644 index 0000000..afe0029 --- /dev/null +++ b/HrynCo.NotificationService.Web/Controllers/Admin/TestSmtpRequest.cs @@ -0,0 +1,6 @@ +namespace HrynCo.NotificationService.Web.Controllers.Admin; + +public record TestSmtpRequest( + string Host, int Port, string Username, string Password, + bool UseSsl, string FromEmail, string FromName, string ToEmail, + Guid? ChannelId = null); diff --git a/HrynCo.NotificationService.Web/Views/AdminChannels/Edit.cshtml b/HrynCo.NotificationService.Web/Views/AdminChannels/Edit.cshtml index c91baac..0cd330f 100644 --- a/HrynCo.NotificationService.Web/Views/AdminChannels/Edit.cshtml +++ b/HrynCo.NotificationService.Web/Views/AdminChannels/Edit.cshtml @@ -138,7 +138,7 @@
-

Tests the current form settings — no need to save first.

+

Tests the current form settings — no need to save first. For an existing channel, a blank password uses the saved password only when the SMTP connection settings are unchanged.

@@ -173,7 +173,9 @@ } const form = document.getElementById('channelForm'); + const channelId = form.querySelector('[name="Id"]').value; const payload = { + channelId: channelId === '00000000-0000-0000-0000-000000000000' ? null : channelId, host: form.querySelector('[name="Host"]').value, port: parseInt(form.querySelector('[name="Port"]').value, 10), username: form.querySelector('[name="Username"]').value, diff --git a/docker/kubernetes/shared-notification/README.md b/docker/kubernetes/shared-notification/README.md index e4bc400..914243a 100644 --- a/docker/kubernetes/shared-notification/README.md +++ b/docker/kubernetes/shared-notification/README.md @@ -80,6 +80,17 @@ published, accepted by the Gmail SMTP channel, acknowledged as delivery successe Invemory Outbox, and confirmed in the recipient's Gmail conversation. Today's channel usage increased by two. Both queues ended with zero ready and unacknowledged messages. +### Administrative SMTP test from Web — 2026-10-18 + +The channel editor's **Send Test Email** action sends directly from Web through SMTP; +it does not use the Worker or RabbitMQ. Its TCP 587 connection timed out while the +production network policy allowed external SMTP only from Worker. The additive +`web-to-external-smtp` policy in `production-network.yaml` grants Web the same public +IPv4 TCP 587 egress as Worker, with the same private and cluster address exclusions. +Apply this production overlay separately from the sandbox-only `notification.py` +render. No SMTP credentials, channel settings, templates, or workload restart are +required for this network change. + The activation helper also persists the production user, queue and expanded Worker ACL in the `rabbit-definitions` Secret. The private password and matching RabbitMQ hash are saved with mode `0600`; updated `notification.py` reproduces that Secret during diff --git a/docker/kubernetes/shared-notification/production-network.yaml b/docker/kubernetes/shared-notification/production-network.yaml index 9e2ef91..c739d95 100644 --- a/docker/kubernetes/shared-notification/production-network.yaml +++ b/docker/kubernetes/shared-notification/production-network.yaml @@ -83,3 +83,32 @@ spec: ports: - protocol: TCP port: 587 +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: web-to-external-smtp + namespace: shared-notification +spec: + podSelector: + matchLabels: + app.kubernetes.io/component: web + app.kubernetes.io/name: hrynco-shared-notification + app.kubernetes.io/managed-by: hrynco-shared-notification + policyTypes: [Egress] + egress: + - to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 10.0.0.0/8 + - 100.64.0.0/10 + - 127.0.0.0/8 + - 169.254.0.0/16 + - 172.16.0.0/12 + - 192.168.0.0/16 + - 224.0.0.0/4 + - 240.0.0.0/4 + ports: + - protocol: TCP + port: 587 diff --git a/docs/email-template-definition-api.md b/docs/email-template-definition-api.md index 903cf5b..ee404c3 100644 --- a/docs/email-template-definition-api.md +++ b/docs/email-template-definition-api.md @@ -73,6 +73,8 @@ The service/catalog lookup, validation, revision check, explicit removals, trans ## Email channels +In the admin channel editor, **Send Test Email** tests the form's current values without saving them. An explicitly entered password takes precedence. When the password field is blank for an existing channel, the Web server reads its saved password only if host, port, username, and SSL/TLS mode still match the saved SMTP connection. Changed connection settings require an explicit password. The saved password is not placed in the page or test response. A new channel keeps the ad-hoc test behavior, including unauthenticated SMTP when username and password are empty. + - `GET /api/v1/email-channels/all` enumerates **all** channels, including channels whose exact `serviceName` has no service row. - Existing `GET /api/v1/email-channels?serviceName={exactName}`, GET by ID, POST, PUT by ID and DELETE by ID remain available. Create returns 201/Location and its existing GUID response; update/delete return 200/Unit. - Create supports SMTP (`channelType: 1`). Request/response `settings` explicitly maps `host`, `port`, `username`, `password`, `useSsl`, `fromEmail`, and `fromName`; reads no longer lose derived SMTP fields through an abstract JSON type. Host/fromEmail and a valid port are required. The optional empty username/password support unauthenticated development SMTP. -- 2.52.0