diff --git a/.github/workflows/backend.yml b/.github/workflows/backend.yml index dc5e395f7c..40f81b1f12 100644 --- a/.github/workflows/backend.yml +++ b/.github/workflows/backend.yml @@ -96,7 +96,7 @@ jobs: - name: Run unit tests with coverage run: | - for proj in Architecture Auditing Caching Generic Identity Multitenancy Billing Catalog Chat Files Framework Webhooks; do + for proj in Architecture Auditing Caching Generic Identity Multitenancy Billing Catalog Chat Files Framework Notifications Webhooks; do echo "::group::${proj}.Tests" dotnet test "src/Tests/${proj}.Tests" -c Release --no-build \ --collect:"XPlat Code Coverage" --settings coverage.runsettings \ diff --git a/deploy/docker/README.md b/deploy/docker/README.md index bcb1304593..0219164b7c 100644 --- a/deploy/docker/README.md +++ b/deploy/docker/README.md @@ -8,9 +8,9 @@ This brings up the full stack on a single host: | `admin` | `fsh/admin:local` | `FSH_ADMIN_PORT` (default 8081) | Operator console (nginx + React) | | `dashboard` | `fsh/dashboard:local` | `FSH_DASHBOARD_PORT` (default 8082) | Tenant dashboard (nginx + React) | | `migrator` | `fsh/dbmigrator:local` | — | One-shot: applies EF migrations + seeds the root tenant + creates the default admin user | -| `postgres` | `postgres:17-alpine` | (internal) | Identity, tenant catalog, module schemas | -| `redis` | `redis:7-alpine` | (internal) | HybridCache L2, Data Protection keys, idempotency store | -| `minio` | `minio/minio:latest` | (internal) | S3-compatible blob store for the Files module | +| `postgres` | `postgres:18-alpine` | (internal) | Identity, tenant catalog, module schemas | +| `redis` | `valkey/valkey:9.1.0-alpine` | (internal) | HybridCache L2, Data Protection keys, idempotency store | +| `minio` | `quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z` | (internal) | S3-compatible blob store for the Files module | The compose file does **not** include a reverse proxy or TLS terminator. You bring your own edge — Cloudflare Tunnel, AWS ALB, Tailscale Funnel, your existing nginx, anything that can route a TLS subdomain to a host:port on this machine. diff --git a/deploy/docker/docker-compose.yml b/deploy/docker/docker-compose.yml index d43c744f5b..dea9a817ff 100644 --- a/deploy/docker/docker-compose.yml +++ b/deploy/docker/docker-compose.yml @@ -54,7 +54,8 @@ services: # - "6379:6379" minio: - image: minio/minio:latest + # quay.io: minio/minio is gone from Docker Hub. Tag pinned; quay stopped moving :latest. + image: quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z container_name: fsh-minio restart: unless-stopped command: ["server", "/data", "--console-address", ":9001"] @@ -79,7 +80,8 @@ services: # policy is set — objects are served via the API / presigned URLs, not a # public bucket. minio-init: - image: minio/mc:latest + # quay.io: minio/mc is gone from Docker Hub too. Tag pinned; quay stopped moving :latest. + image: quay.io/minio/mc:RELEASE.2025-08-13T08-35-41Z container_name: fsh-minio-init restart: "no" depends_on: diff --git a/src/BuildingBlocks/Mailing/MailRequest.cs b/src/BuildingBlocks/Mailing/MailRequest.cs index 597d2e9728..fb20f3a6c5 100644 --- a/src/BuildingBlocks/Mailing/MailRequest.cs +++ b/src/BuildingBlocks/Mailing/MailRequest.cs @@ -2,14 +2,26 @@ namespace FSH.Framework.Mailing; -public class MailRequest(Collection to, string subject, string? body = null, string? from = null, string? displayName = null, string? replyTo = null, string? replyToName = null, Collection? bcc = null, Collection? cc = null, IDictionary? attachmentData = null, IDictionary? headers = null) +public class MailRequest(Collection to, string subject, string? body = null, string? from = null, string? displayName = null, string? replyTo = null, string? replyToName = null, Collection? bcc = null, Collection? cc = null, IDictionary? attachmentData = null, IDictionary? headers = null, string? textBody = null) { public Collection To { get; } = to; public string Subject { get; } = subject; + /// + /// The HTML body. Every provider sends this as text/html, so a caller that passes bare text + /// gets a message whose URLs are not anchors — most clients do not auto-link inside HTML — and whose + /// interpolated values are parsed as markup. Build real HTML here and put the fallback in + /// . + /// public string? Body { get; } = body; + /// + /// Optional text/plain alternative, sent alongside as multipart/alternative. + /// Clients that cannot render HTML (and spam filters, which score HTML-only mail worse) fall back to it. + /// + public string? TextBody { get; } = textBody; + public string? From { get; } = from; public string? DisplayName { get; } = displayName; diff --git a/src/BuildingBlocks/Mailing/Mailing.csproj b/src/BuildingBlocks/Mailing/Mailing.csproj index 7b558fb91b..73e4f31fad 100644 --- a/src/BuildingBlocks/Mailing/Mailing.csproj +++ b/src/BuildingBlocks/Mailing/Mailing.csproj @@ -18,4 +18,8 @@ + + + + diff --git a/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs b/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs index 96334b4124..3392abe57d 100644 --- a/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs +++ b/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs @@ -34,11 +34,18 @@ public async Task SendAsync(MailRequest request, CancellationToken ct) } var from = CreateFromAddress(request); + // plainTextContent and htmlContent are distinct parts: passing Body to both shipped the HTML + // template as the text alternative, so a text-only client rendered raw markup. + // + // Falling back to Body when TextBody is absent is deliberate. CreateSingleEmail drops the + // text/plain part entirely for a null or empty string, so a caller outside this repo that + // still passes only Body would silently go from "two parts" to "HTML only" — a regression + // for them, in a template other people build on, with no compiler error to warn them. var msg = MailHelper.CreateSingleEmail( from, new EmailAddress(request.To[0]), request.Subject, - request.Body, + request.TextBody ?? request.Body, request.Body); ConfigureRecipients(msg, request); diff --git a/src/BuildingBlocks/Mailing/Services/SmtpMailService.cs b/src/BuildingBlocks/Mailing/Services/SmtpMailService.cs index 6be16bf5e2..de4592fddb 100644 --- a/src/BuildingBlocks/Mailing/Services/SmtpMailService.cs +++ b/src/BuildingBlocks/Mailing/Services/SmtpMailService.cs @@ -108,9 +108,13 @@ private static void ConfigureContent(MimeMessage email, MailRequest request) email.Subject = request.Subject; } - private static async Task AddAttachmentsAsync(MimeMessage email, MailRequest request, CancellationToken ct) + // internal so the suite can assert the MIME shape without an SMTP server: the transport is the + // one thing a test cannot reach here, and it is not where the bodies get mapped. + internal static async Task AddAttachmentsAsync(MimeMessage email, MailRequest request, CancellationToken ct) { - var builder = new BodyBuilder { HtmlBody = request.Body }; + // Both parts when the caller supplies them: MailKit emits multipart/alternative and the client + // picks. HtmlBody alone leaves text-only clients with nothing. + var builder = new BodyBuilder { HtmlBody = request.Body, TextBody = request.TextBody }; if (request.AttachmentData is not null) { diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 0d38b28190..854deb9530 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -9,7 +9,8 @@ - + + @@ -122,9 +123,10 @@ - - - + + + + diff --git a/src/FSH.Starter.slnx b/src/FSH.Starter.slnx index 998d538836..c8861e8a70 100644 --- a/src/FSH.Starter.slnx +++ b/src/FSH.Starter.slnx @@ -79,6 +79,7 @@ + diff --git a/src/Host/FSH.Starter.AppHost/AppHost.cs b/src/Host/FSH.Starter.AppHost/AppHost.cs index e7a70abd05..4fc689599d 100644 --- a/src/Host/FSH.Starter.AppHost/AppHost.cs +++ b/src/Host/FSH.Starter.AppHost/AppHost.cs @@ -52,7 +52,10 @@ var minioUser = builder.AddParameter("minio-user", "minioadmin"); var minioPassword = builder.AddParameter("minio-password", "minioadmin", secret: true); +// quay.io: minio/minio is gone from Docker Hub. Tag pinned; quay stopped moving :latest. var minio = builder.AddContainer("minio", "minio/minio") + .WithImageRegistry("quay.io") + .WithImageTag("RELEASE.2025-09-07T16-13-09Z") .WithArgs("server", "/data", "--console-address", ":9001") .WithHttpEndpoint(port: 9000, targetPort: 9000, name: "api") .WithHttpEndpoint(port: 9001, targetPort: 9001, name: "console") @@ -73,6 +76,8 @@ """).ReplaceLineEndings("\n"); var minioInit = builder.AddContainer("minio-init", "minio/mc") + .WithImageRegistry("quay.io") + .WithImageTag("RELEASE.2025-08-13T08-35-41Z") .WithEntrypoint("/bin/sh") .WithArgs("-c", minioInitScript) .WithEnvironment("MC_USER", minioUser) diff --git a/src/Modules/Identity/Modules.Identity/Events/UserRegisteredEmailHandler.cs b/src/Modules/Identity/Modules.Identity/Events/UserRegisteredEmailHandler.cs index 5971ea21a6..b123831eca 100644 --- a/src/Modules/Identity/Modules.Identity/Events/UserRegisteredEmailHandler.cs +++ b/src/Modules/Identity/Modules.Identity/Events/UserRegisteredEmailHandler.cs @@ -2,6 +2,7 @@ using FSH.Framework.Mailing; using FSH.Framework.Mailing.Services; using FSH.Modules.Identity.Contracts.Events; +using FSH.Modules.Identity.Services; using Microsoft.Extensions.Logging; namespace FSH.Modules.Identity.Events; @@ -34,10 +35,14 @@ public async Task HandleAsync(UserRegisteredIntegrationEvent @event, Cancellatio try { + // The body is sent as text/html, so the name — user-supplied — has to be encoded or a + // first name containing '<' is parsed as markup instead of shown. + var greeting = $"Hi {@event.FirstName}, thanks for registering."; var mail = new MailRequest( to: new System.Collections.ObjectModel.Collection { @event.Email }, subject: "Welcome!", - body: $"Hi {@event.FirstName}, thanks for registering."); + body: EmailBodies.NoticeHtml("Welcome!", greeting), + textBody: greeting); await _mailService.SendAsync(mail, ct).ConfigureAwait(false); } diff --git a/src/Modules/Identity/Modules.Identity/Services/EmailBodies.cs b/src/Modules/Identity/Modules.Identity/Services/EmailBodies.cs new file mode 100644 index 0000000000..b5dfd4da34 --- /dev/null +++ b/src/Modules/Identity/Modules.Identity/Services/EmailBodies.cs @@ -0,0 +1,92 @@ +using System.Net; + +namespace FSH.Modules.Identity.Services; + +/// +/// Builds the HTML bodies for identity e-mails. Every value that reaches the markup is HTML-encoded: +/// bodies are sent as text/html, so an unescaped name or URL is parsed as markup rather than shown. +/// +internal static class EmailBodies +{ + /// + /// A message whose point is a single action link, rendered as a real anchor so mail clients make it + /// clickable. Callers pair this with a text/plain alternative carrying the same URL. + /// + internal static string LinkActionHtml(string heading, string intro, string actionUrl, string actionLabel) + { + string safeHeading = WebUtility.HtmlEncode(heading); + string safeIntro = WebUtility.HtmlEncode(intro); + string safeUrl = WebUtility.HtmlEncode(actionUrl); + string safeLabel = WebUtility.HtmlEncode(actionLabel); + + return $""" + + + + + + {safeHeading} + + + + + + +
+ + + + +
+

{safeHeading}

+

{safeIntro}

+

+ {safeLabel} +

+

+ If the button does not work, copy this address into your browser:
+ {safeUrl} +

+
+
+ + + """; + } + + /// + /// A short informational message with no action link. + /// + internal static string NoticeHtml(string heading, string message) + { + string safeHeading = WebUtility.HtmlEncode(heading); + string safeMessage = WebUtility.HtmlEncode(message); + + return $""" + + + + + + {safeHeading} + + + + + + +
+ + + + +
+

{safeHeading}

+

{safeMessage}

+
+
+ + + """; + } +} diff --git a/src/Modules/Identity/Modules.Identity/Services/UserPasswordService.cs b/src/Modules/Identity/Modules.Identity/Services/UserPasswordService.cs index f29a3eb8fd..46d1251dba 100644 --- a/src/Modules/Identity/Modules.Identity/Services/UserPasswordService.cs +++ b/src/Modules/Identity/Modules.Identity/Services/UserPasswordService.cs @@ -51,10 +51,17 @@ public async Task ForgotPasswordAsync(string email, string origin, CancellationT ["email"] = email, ["tenant"] = multiTenantContextAccessor?.MultiTenantContext?.TenantInfo?.Id, }); + // The body is sent as text/html, so the link has to be an anchor: a bare URL in an HTML part is + // not auto-linked by most clients, which is how the reset link reached users as dead text. var mailRequest = new MailRequest( new Collection { user.Email }, "Reset Password", - $"Please reset your password using the following link: {resetPasswordUri}"); + EmailBodies.LinkActionHtml( + heading: "Reset your password", + intro: "Use the link below to choose a new password.", + actionUrl: resetPasswordUri, + actionLabel: "Reset password"), + textBody: $"Please reset your password using the following link: {resetPasswordUri}"); jobService.Enqueue(() => mailService.SendAsync(mailRequest, CancellationToken.None)); } diff --git a/src/Modules/Identity/Modules.Identity/Services/UserRegistrationService.cs b/src/Modules/Identity/Modules.Identity/Services/UserRegistrationService.cs index 79409e4379..c344802553 100644 --- a/src/Modules/Identity/Modules.Identity/Services/UserRegistrationService.cs +++ b/src/Modules/Identity/Modules.Identity/Services/UserRegistrationService.cs @@ -309,7 +309,8 @@ private async Task SendConfirmationEmailAsync(FshUser user, string origin, Cance var mailRequest = new MailRequest( new Collection { user.Email }, "Confirm Your Email Address", - emailBody); + emailBody, + textBody: $"Please confirm your email address using the following link: {emailVerificationUri}"); jobService.Enqueue("email", () => mailService.SendAsync(mailRequest, cancellationToken)); } diff --git a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailBodies.cs b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailBodies.cs index c2ff2d7d3c..1ae5c7b281 100644 --- a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailBodies.cs +++ b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailBodies.cs @@ -3,50 +3,67 @@ namespace FSH.Modules.Notifications.IntegrationEventHandlers; /// -/// Builds the subject + HTML body for tenant billing emails. Plain interpolated HTML (the framework -/// has no template engine); kept here so the handlers stay thin and the copy is easy to review. +/// Builds the subject + HTML body + text/plain alternative for tenant billing emails. Plain interpolated +/// HTML (the framework has no template engine); kept here so the handlers stay thin and the copy is easy +/// to review. Every message carries both parts: HTML-only mail leaves text-only clients with nothing and +/// scores worse with spam filters. /// internal static class BillingEmailBodies { private static string Date(DateTime utc) => utc.ToString("MMMM d, yyyy", CultureInfo.InvariantCulture); - public static (string Subject, string Body) NearingExpiry(string tenantName, string? planKey, DateTime validUpto, int daysRemaining) + public static (string Subject, string Body, string TextBody) NearingExpiry(string tenantName, string? planKey, DateTime validUpto, int daysRemaining) { var subject = daysRemaining <= 1 ? "Your subscription expires tomorrow" : $"Your subscription expires in {daysRemaining} days"; + var plan = planKey ?? "current"; var body = Wrap(subject, $"

Hi {Escape(tenantName)},

" + - $"

Your {Escape(planKey ?? "current")} subscription is valid until " + + $"

Your {Escape(plan)} subscription is valid until " + $"{Date(validUpto)} ({daysRemaining} day(s) remaining).

" + "

Please contact your account operator to renew and avoid any interruption to your service.

"); - return (subject, body); + var text = Text(subject, + $"Hi {tenantName},", + $"Your {plan} subscription is valid until {Date(validUpto)} ({daysRemaining} day(s) remaining).", + "Please contact your account operator to renew and avoid any interruption to your service."); + return (subject, body, text); } - public static (string Subject, string Body) EnteredGrace(string tenantName, string? planKey, DateTime validUpto, DateTime graceEnds) + public static (string Subject, string Body, string TextBody) EnteredGrace(string tenantName, string? planKey, DateTime validUpto, DateTime graceEnds) { const string subject = "Your subscription has lapsed — grace period active"; + var plan = planKey ?? "current"; var body = Wrap(subject, $"

Hi {Escape(tenantName)},

" + - $"

Your {Escape(planKey ?? "current")} subscription expired on " + + $"

Your {Escape(plan)} subscription expired on " + $"{Date(validUpto)}. Your service continues during a grace period that ends on " + $"{Date(graceEnds)}.

" + "

Please renew before the grace period ends to keep your access uninterrupted.

"); - return (subject, body); + var text = Text(subject, + $"Hi {tenantName},", + $"Your {plan} subscription expired on {Date(validUpto)}. Your service continues during a grace period that ends on {Date(graceEnds)}.", + "Please renew before the grace period ends to keep your access uninterrupted."); + return (subject, body, text); } - public static (string Subject, string Body) Expired(string tenantName, string? planKey, DateTime validUpto) + public static (string Subject, string Body, string TextBody) Expired(string tenantName, string? planKey, DateTime validUpto) { const string subject = "Your subscription has expired"; + var plan = planKey ?? "current"; var body = Wrap(subject, $"

Hi {Escape(tenantName)},

" + - $"

Your {Escape(planKey ?? "current")} subscription expired on " + + $"

Your {Escape(plan)} subscription expired on " + $"{Date(validUpto)} and the grace period has ended, so access is now suspended.

" + "

Contact your account operator to renew and restore access.

"); - return (subject, body); + var text = Text(subject, + $"Hi {tenantName},", + $"Your {plan} subscription expired on {Date(validUpto)} and the grace period has ended, so access is now suspended.", + "Contact your account operator to renew and restore access."); + return (subject, body, text); } - public static (string Subject, string Body) InvoiceIssued(string invoiceNumber, decimal amount, string currency, DateTime? dueAtUtc) + public static (string Subject, string Body, string TextBody) InvoiceIssued(string invoiceNumber, decimal amount, string currency, DateTime? dueAtUtc) { var subject = $"Invoice {invoiceNumber} issued"; var amountText = $"{amount.ToString("0.00", CultureInfo.InvariantCulture)} {currency}"; @@ -55,7 +72,11 @@ public static (string Subject, string Body) InvoiceIssued(string invoiceNumber, $"

A new invoice {Escape(invoiceNumber)} for {amountText} has been issued.

" + due + "

You can view and download this invoice from your dashboard.

"); - return (subject, body); + var text = Text(subject, + $"A new invoice {invoiceNumber} for {amountText} has been issued.", + dueAtUtc is null ? string.Empty : $"Due by {Date(dueAtUtc.Value)}.", + "You can view and download this invoice from your dashboard."); + return (subject, body, text); } private static string Wrap(string heading, string innerHtml) => @@ -65,6 +86,19 @@ private static string Wrap(string heading, string innerHtml) => "

This is an automated message.

" + ""; + /// + /// The text/plain twin of : same copy, no markup, empty paragraphs dropped so an + /// optional line (an absent due date) does not leave a blank gap. + /// + private static string Text(string heading, params string[] paragraphs) + { + var lines = new List { heading, string.Empty }; + lines.AddRange(paragraphs.Where(p => !string.IsNullOrWhiteSpace(p))); + lines.Add(string.Empty); + lines.Add("This is an automated message."); + return string.Join(Environment.NewLine + Environment.NewLine, lines.Where(l => l.Length > 0)); + } + private static string Escape(string value) => value.Replace("&", "&", StringComparison.Ordinal) .Replace("<", "<", StringComparison.Ordinal) diff --git a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailSender.cs b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailSender.cs index 9159df9b8b..da0bc81be3 100644 --- a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailSender.cs +++ b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailSender.cs @@ -10,7 +10,7 @@ namespace FSH.Modules.Notifications.IntegrationEventHandlers; internal static class BillingEmailSender { public static async Task SendAsync( - IMailService mail, ILogger logger, string? email, string subject, string body, string context, CancellationToken ct) + IMailService mail, ILogger logger, string? email, string subject, string body, string textBody, string context, CancellationToken ct) { if (string.IsNullOrWhiteSpace(email)) { @@ -22,7 +22,8 @@ public static async Task SendAsync( await mail.SendAsync(new MailRequest( to: new Collection { email }, subject: subject, - body: body), ct).ConfigureAwait(false); + body: body, + textBody: textBody), ct).ConfigureAwait(false); } catch (Exception ex) { diff --git a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/InvoiceIssuedEmailHandler.cs b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/InvoiceIssuedEmailHandler.cs index 91d5a8c9e0..012449dc04 100644 --- a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/InvoiceIssuedEmailHandler.cs +++ b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/InvoiceIssuedEmailHandler.cs @@ -29,9 +29,9 @@ public async Task HandleAsync(InvoiceIssuedIntegrationEvent @event, Cancellation return; } - var (subject, body) = BillingEmailBodies.InvoiceIssued( + var (subject, body, textBody) = BillingEmailBodies.InvoiceIssued( @event.InvoiceNumber, @event.Amount, @event.Currency, @event.DueAtUtc); - await BillingEmailSender.SendAsync(mailService, logger, tenant.AdminEmail, subject, body, "invoice-issued", ct) + await BillingEmailSender.SendAsync(mailService, logger, tenant.AdminEmail, subject, body, textBody, "invoice-issued", ct) .ConfigureAwait(false); } } diff --git a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantEnteredGraceEmailHandler.cs b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantEnteredGraceEmailHandler.cs index 3505838e53..5bccdf5727 100644 --- a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantEnteredGraceEmailHandler.cs +++ b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantEnteredGraceEmailHandler.cs @@ -14,9 +14,9 @@ public sealed class TenantEnteredGraceEmailHandler( public async Task HandleAsync(TenantEnteredGraceIntegrationEvent @event, CancellationToken ct = default) { ArgumentNullException.ThrowIfNull(@event); - var (subject, body) = BillingEmailBodies.EnteredGrace( + var (subject, body, textBody) = BillingEmailBodies.EnteredGrace( @event.TenantName, @event.PlanKey, @event.ValidUpto, @event.GraceEndsUtc); - await BillingEmailSender.SendAsync(mailService, logger, @event.AdminEmail, subject, body, "entered-grace", ct) + await BillingEmailSender.SendAsync(mailService, logger, @event.AdminEmail, subject, body, textBody, "entered-grace", ct) .ConfigureAwait(false); } } diff --git a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantExpiredEmailHandler.cs b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantExpiredEmailHandler.cs index 6279e4d5d6..23d62714d5 100644 --- a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantExpiredEmailHandler.cs +++ b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantExpiredEmailHandler.cs @@ -14,8 +14,8 @@ public sealed class TenantExpiredEmailHandler( public async Task HandleAsync(TenantExpiredIntegrationEvent @event, CancellationToken ct = default) { ArgumentNullException.ThrowIfNull(@event); - var (subject, body) = BillingEmailBodies.Expired(@event.TenantName, @event.PlanKey, @event.ValidUpto); - await BillingEmailSender.SendAsync(mailService, logger, @event.AdminEmail, subject, body, "expired", ct) + var (subject, body, textBody) = BillingEmailBodies.Expired(@event.TenantName, @event.PlanKey, @event.ValidUpto); + await BillingEmailSender.SendAsync(mailService, logger, @event.AdminEmail, subject, body, textBody, "expired", ct) .ConfigureAwait(false); } } diff --git a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantNearingExpiryEmailHandler.cs b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantNearingExpiryEmailHandler.cs index 3a7a77a055..768c0b875c 100644 --- a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantNearingExpiryEmailHandler.cs +++ b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/TenantNearingExpiryEmailHandler.cs @@ -14,9 +14,9 @@ public sealed class TenantNearingExpiryEmailHandler( public async Task HandleAsync(TenantNearingExpiryIntegrationEvent @event, CancellationToken ct = default) { ArgumentNullException.ThrowIfNull(@event); - var (subject, body) = BillingEmailBodies.NearingExpiry( + var (subject, body, textBody) = BillingEmailBodies.NearingExpiry( @event.TenantName, @event.PlanKey, @event.ValidUpto, @event.DaysRemaining); - await BillingEmailSender.SendAsync(mailService, logger, @event.AdminEmail, subject, body, "nearing-expiry", ct) + await BillingEmailSender.SendAsync(mailService, logger, @event.AdminEmail, subject, body, textBody, "nearing-expiry", ct) .ConfigureAwait(false); } } diff --git a/src/Modules/Notifications/Modules.Notifications/Modules.Notifications.csproj b/src/Modules/Notifications/Modules.Notifications/Modules.Notifications.csproj index e97c3074bb..500b34ef04 100644 --- a/src/Modules/Notifications/Modules.Notifications/Modules.Notifications.csproj +++ b/src/Modules/Notifications/Modules.Notifications/Modules.Notifications.csproj @@ -21,4 +21,8 @@ + + + + diff --git a/src/Tests/Framework.Tests/Mailing/MailRequestTests.cs b/src/Tests/Framework.Tests/Mailing/MailRequestTests.cs index 6d5b9a6bad..b21020ed4b 100644 --- a/src/Tests/Framework.Tests/Mailing/MailRequestTests.cs +++ b/src/Tests/Framework.Tests/Mailing/MailRequestTests.cs @@ -20,12 +20,13 @@ public void Ctor_Should_AssignProvidedValues_When_FullArgsGiven() // Act var request = new MailRequest( to, "subject", "body", "from@x.com", "Sender", - "reply@x.com", "Reply", bcc, cc, attachments, headers); + "reply@x.com", "Reply", bcc, cc, attachments, headers, "plain body"); // Assert request.To.ShouldBe(to); request.Subject.ShouldBe("subject"); request.Body.ShouldBe("body"); + request.TextBody.ShouldBe("plain body"); request.From.ShouldBe("from@x.com"); request.DisplayName.ShouldBe("Sender"); request.ReplyTo.ShouldBe("reply@x.com"); @@ -48,6 +49,7 @@ public void Ctor_Should_DefaultCollections_When_OptionalArgsOmitted() // Assert — nullable collections default to empty (never null). request.Body.ShouldBeNull(); + request.TextBody.ShouldBeNull(); request.From.ShouldBeNull(); request.Cc.ShouldNotBeNull(); request.Cc.ShouldBeEmpty(); diff --git a/src/Tests/Framework.Tests/Mailing/SendGridMailServiceTests.cs b/src/Tests/Framework.Tests/Mailing/SendGridMailServiceTests.cs index 87150fec8b..1b2ff410b7 100644 --- a/src/Tests/Framework.Tests/Mailing/SendGridMailServiceTests.cs +++ b/src/Tests/Framework.Tests/Mailing/SendGridMailServiceTests.cs @@ -37,6 +37,51 @@ private static ISendGridClient ClientReturning(HttpStatusCode status) private static MailRequest ValidRequest() => new(to: ["dest@x.com"], subject: "hi", body: "body"); + [Fact] + public async Task SendAsync_Should_MapTheBodies_ToTheirOwnMimeParts() + { + // Arrange — Body and TextBody are distinct parts. Sending Body as both shipped raw markup to + // text-only clients. + var client = ClientReturning(HttpStatusCode.Accepted); + var service = BuildService(client); + var request = new MailRequest( + to: ["dest@x.com"], + subject: "hi", + body: "

rich

", + textBody: "plain"); + + // Act + await service.SendAsync(request, CancellationToken.None); + + // Assert + // CreateSingleEmail folds both bodies into Contents (the HtmlContent/PlainTextContent properties + // stay null once the message is built), so assert on the MIME parts themselves. + var sent = (SendGridMessage)client.ReceivedCalls().Single().GetArguments()[0]!; + sent.Contents.Single(c => c.Type == "text/html").Value.ShouldBe("

rich

"); + sent.Contents.Single(c => c.Type == "text/plain").Value.ShouldBe("plain"); + } + + // A caller outside this repo — this is a template — may still build a MailRequest with only + // `body`. CreateSingleEmail drops the text/plain part for a null plainTextContent, so without + // the fallback that caller silently goes from a two-part message to HTML-only, which is worse + // for text clients and scores worse with spam filters. + [Fact] + public async Task SendAsync_Should_KeepAPlainTextPart_When_OnlyTheBodyIsSupplied() + { + // Arrange + var client = ClientReturning(HttpStatusCode.Accepted); + var service = BuildService(client); + var request = new MailRequest(to: ["dest@x.com"], subject: "hi", body: "just text"); + + // Act + await service.SendAsync(request, CancellationToken.None); + + // Assert + var sent = (SendGridMessage)client.ReceivedCalls().Single().GetArguments()[0]!; + sent.Contents.Single(c => c.Type == "text/plain").Value.ShouldBe("just text"); + sent.Contents.Single(c => c.Type == "text/html").Value.ShouldBe("just text"); + } + [Theory] [InlineData(HttpStatusCode.TooManyRequests)] // 429 — rate limited [InlineData(HttpStatusCode.InternalServerError)] // 500 — SendGrid-side diff --git a/src/Tests/Framework.Tests/Mailing/SmtpMailServiceTests.cs b/src/Tests/Framework.Tests/Mailing/SmtpMailServiceTests.cs new file mode 100644 index 0000000000..746805e376 --- /dev/null +++ b/src/Tests/Framework.Tests/Mailing/SmtpMailServiceTests.cs @@ -0,0 +1,72 @@ +using FSH.Framework.Mailing; +using FSH.Framework.Mailing.Services; +using MimeKit; + +namespace Framework.Tests.Mailing; + +// SMTP is the default provider (UseSendGrid defaults to false), and the body mapping this PR changed +// had a gate only on the optional one. The transport needs a server and is not what changed; the MIME +// shape is, so that is what these drive, through the real builder. +public sealed class SmtpMailServiceTests +{ + private static async Task BuildAsync(MailRequest request) + { + var email = new MimeMessage(); + await SmtpMailService.AddAttachmentsAsync(email, request, CancellationToken.None); + return email; + } + + [Fact] + public async Task Body_Should_CarryBothParts_When_TheCallerSuppliesText() + { + // Arrange + var request = new MailRequest( + to: ["dest@x.com"], + subject: "hi", + body: "

rich

", + textBody: "plain"); + + // Act + using var email = await BuildAsync(request); + + // Assert — a text-only client reads the plain part; sending the markup as both is what this + // fixes, and it is exactly what a client that cannot render HTML would then display. + email.HtmlBody.ShouldBe("

rich

"); + email.TextBody.ShouldBe("plain"); + email.Body.ShouldNotBeNull().ContentType.MimeType.ShouldBe("multipart/alternative"); + } + + [Fact] + public async Task Body_Should_BeHtmlOnly_When_TheCallerSuppliesNoText() + { + // Every caller outside the templated notifications still passes Body alone. Inventing a plain + // part from the markup is what the old code effectively did, so assert it does not. + var request = new MailRequest(to: ["dest@x.com"], subject: "hi", body: "

rich

"); + + using var email = await BuildAsync(request); + + email.HtmlBody.ShouldBe("

rich

"); + email.TextBody.ShouldBeNull(); + email.Body.ShouldNotBeNull().ContentType.MimeType.ShouldBe("text/html"); + } + + [Fact] + public async Task Body_Should_KeepBothParts_When_AnAttachmentIsPresent() + { + // The attachment wraps the alternative in a multipart/mixed. A builder that appended the + // attachment to the wrong part would lose the plain text and still "have" an attachment. + var request = new MailRequest( + to: ["dest@x.com"], + subject: "hi", + body: "

rich

", + textBody: "plain", + attachmentData: new Dictionary { ["invoice.pdf"] = [1, 2, 3] }); + + using var email = await BuildAsync(request); + + email.Body.ShouldNotBeNull().ContentType.MimeType.ShouldBe("multipart/mixed"); + email.HtmlBody.ShouldBe("

rich

"); + email.TextBody.ShouldBe("plain"); + email.Attachments.Single().ContentDisposition.ShouldNotBeNull().FileName.ShouldBe("invoice.pdf"); + } +} diff --git a/src/Tests/Identity.Tests/Events/UserRegisteredEmailHandlerTests.cs b/src/Tests/Identity.Tests/Events/UserRegisteredEmailHandlerTests.cs new file mode 100644 index 0000000000..68cdef09d2 --- /dev/null +++ b/src/Tests/Identity.Tests/Events/UserRegisteredEmailHandlerTests.cs @@ -0,0 +1,83 @@ +using FSH.Framework.Mailing; +using FSH.Framework.Mailing.Services; +using FSH.Modules.Identity.Contracts.Events; +using FSH.Modules.Identity.Events; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; + +namespace Identity.Tests.Events; + +/// +/// The welcome mail is sent as text/html, so a user-supplied first name reaches an HTML parser. It has to +/// be encoded, and the message needs a text/plain twin like every other mail the kit sends. +/// +public sealed class UserRegisteredEmailHandlerTests +{ + private readonly IMailService _mailService = Substitute.For(); + + private UserRegisteredEmailHandler CreateSut() => + new(_mailService, NullLogger.Instance); + + private static UserRegisteredIntegrationEvent EventWithFirstName(string firstName) => + new( + Id: Guid.NewGuid(), + OccurredOnUtc: DateTime.UtcNow, + TenantId: "root", + CorrelationId: Guid.NewGuid().ToString(), + Source: "self-registration", + UserId: Guid.NewGuid().ToString(), + Email: "new.user@codefi.com.br", + FirstName: firstName, + LastName: "Maciel"); + + private MailRequest CaptureSentMail() + { + var call = _mailService.ReceivedCalls().Single(); + return (MailRequest)call.GetArguments()[0]!; + } + + [Fact] + public async Task HandleAsync_Should_EncodeTheFirstName_When_ItContainsMarkup() + { + // Arrange — a first name is user-supplied; unencoded, this closes the surrounding element and + // injects a tag into the rendered mail. + var sut = CreateSut(); + + // Act + await sut.HandleAsync(EventWithFirstName(""), CancellationToken.None); + + // Assert + var body = CaptureSentMail().Body!; + body.ShouldNotContain(" & Co"; + + // Act + var (_, body, text) = BillingEmailBodies.NearingExpiry(Hostile, "pro", ValidUpto, daysRemaining: 3); + + // Assert — escaped where it is parsed as markup… + body.ShouldNotContain("