diff --git a/src/BuildingBlocks/Mailing/HtmlEmail.cs b/src/BuildingBlocks/Mailing/HtmlEmail.cs new file mode 100644 index 0000000000..a4fc4ae565 --- /dev/null +++ b/src/BuildingBlocks/Mailing/HtmlEmail.cs @@ -0,0 +1,107 @@ +using System.Net; + +namespace FSH.Framework.Mailing; + +/// +/// The single HTML shell and the single encoder for outbound mail. Bodies are sent as +/// text/html, so any value reaching the markup has to be encoded or it is parsed as markup +/// rather than shown. Keeping both here means a module cannot ship its own weaker escaping. +/// +/// +/// Pair every HTML body with a text/plain alternative on : +/// HTML-only mail leaves text-only clients with nothing and scores worse with spam filters. +/// +public static class HtmlEmail +{ + /// + /// HTML-encodes a value for insertion into markup. Covers quotes and apostrophes as well as + /// &, < and >, so the same call is safe in an attribute and in + /// element content. + /// + public static string Encode(string value) + { + ArgumentNullException.ThrowIfNull(value); + + return WebUtility.HtmlEncode(value); + } + + /// + /// Wraps already-built markup in the shared document: doctype, charset, viewport, and the + /// centred card every e-mail from the kit renders in. + /// + /// Plain text. Encoded here, and also used as the document title. + /// + /// TRUSTED markup, inserted verbatim and NOT encoded. Build it from literals plus + /// d values; never pass user input straight through. + /// + public static string Shell(string heading, string innerHtml) + { + ArgumentNullException.ThrowIfNull(heading); + ArgumentNullException.ThrowIfNull(innerHtml); + + string safeHeading = Encode(heading); + + return $""" + + + + + + {safeHeading} + + + + + + +
+ + + + +
+

{safeHeading}

+ {innerHtml} +
+
+ + + """; + } + + /// + /// A message whose point is a single action link, rendered as a real anchor so mail clients make + /// it clickable. The address is repeated as text underneath for clients that strip buttons. + /// + public static string LinkAction(string heading, string intro, string actionUrl, string actionLabel) + { + ArgumentNullException.ThrowIfNull(intro); + ArgumentNullException.ThrowIfNull(actionUrl); + ArgumentNullException.ThrowIfNull(actionLabel); + + string safeIntro = Encode(intro); + string safeUrl = Encode(actionUrl); + string safeLabel = Encode(actionLabel); + + return Shell(heading, $""" +

{safeIntro}

+

+ {safeLabel} +

+

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

+ """); + } + + /// + /// A short informational message with no action link. + /// + public static string Notice(string heading, string message) + { + ArgumentNullException.ThrowIfNull(message); + + return Shell(heading, $"""

{Encode(message)}

"""); + } +} 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/Services/SendGridMailService.cs b/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs index 96334b4124..4506204d4e 100644 --- a/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs +++ b/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs @@ -34,11 +34,13 @@ 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. var msg = MailHelper.CreateSingleEmail( from, new EmailAddress(request.To[0]), request.Subject, - request.Body, + request.TextBody, request.Body); ConfigureRecipients(msg, request); diff --git a/src/BuildingBlocks/Mailing/Services/SmtpMailService.cs b/src/BuildingBlocks/Mailing/Services/SmtpMailService.cs index 6be16bf5e2..071127d9a6 100644 --- a/src/BuildingBlocks/Mailing/Services/SmtpMailService.cs +++ b/src/BuildingBlocks/Mailing/Services/SmtpMailService.cs @@ -110,7 +110,9 @@ private static void ConfigureContent(MimeMessage email, MailRequest request) private 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..7674befa8f 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -143,5 +143,12 @@ AccessViolation). Transitive pinning is enabled, so this entry alone bumps it. Remove once the SignalR backplane package depends on a patched version itself. --> + + \ No newline at end of file diff --git a/src/Modules/Identity/Modules.Identity/Events/UserRegisteredEmailHandler.cs b/src/Modules/Identity/Modules.Identity/Events/UserRegisteredEmailHandler.cs index 5971ea21a6..a1ab7c44a0 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: HtmlEmail.Notice("Welcome!", greeting), + textBody: greeting); await _mailService.SendAsync(mail, ct).ConfigureAwait(false); } diff --git a/src/Modules/Identity/Modules.Identity/Services/UserPasswordService.cs b/src/Modules/Identity/Modules.Identity/Services/UserPasswordService.cs index f29a3eb8fd..528da410a4 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}"); + HtmlEmail.LinkAction( + 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..c8395ca042 100644 --- a/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailBodies.cs +++ b/src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailBodies.cs @@ -1,72 +1,110 @@ using System.Globalization; +using FSH.Framework.Mailing; 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. The document shell and the encoder come from +/// so every module escapes the same way. /// 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 " + + $"

Hi {HtmlEmail.Encode(tenantName)},

" + + $"

Your {HtmlEmail.Encode(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 " + + $"

Hi {HtmlEmail.Encode(tenantName)},

" + + $"

Your {HtmlEmail.Encode(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 " + + $"

Hi {HtmlEmail.Encode(tenantName)},

" + + $"

Your {HtmlEmail.Encode(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}"; var due = dueAtUtc is null ? string.Empty : $"

Due by {Date(dueAtUtc.Value)}.

"; var body = Wrap(subject, - $"

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

" + + // amountText embeds the currency, which is data rather than a literal, so it is encoded + // for the HTML part while the text/plain twin below keeps it verbatim. + $"

A new invoice {HtmlEmail.Encode(invoiceNumber)} for {HtmlEmail.Encode(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); } + /// + /// The billing copy inside the shared document shell. Only the automated-message footer is + /// specific to these e-mails; the doctype, charset and card come from . + /// private static string Wrap(string heading, string innerHtml) => - "
" + - $"

{Escape(heading)}

" + - innerHtml + - "

This is an automated message.

" + - "
"; + HtmlEmail.Shell( + heading, + "
" + + innerHtml + + "

This is an automated message.

" + + "
"); - private static string Escape(string value) => - value.Replace("&", "&", StringComparison.Ordinal) - .Replace("<", "<", StringComparison.Ordinal) - .Replace(">", ">", StringComparison.Ordinal); + /// + /// 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)); + } } 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/Tests/Framework.Tests/Mailing/HtmlEmailTests.cs b/src/Tests/Framework.Tests/Mailing/HtmlEmailTests.cs new file mode 100644 index 0000000000..6a24d1bc09 --- /dev/null +++ b/src/Tests/Framework.Tests/Mailing/HtmlEmailTests.cs @@ -0,0 +1,200 @@ +using FSH.Framework.Mailing; + +namespace Framework.Tests.Mailing; + +public sealed class HtmlEmailTests +{ + #region Encoding + + [Fact] + public void Encode_Should_NeutraliseMarkup_When_ValueContainsTags() + { + // Act + var encoded = HtmlEmail.Encode(""); + + // Assert + encoded.ShouldNotContain(", thanks for registering."); + + // Assert + html.ShouldStartWith(""); + html.ShouldNotContain(""), CancellationToken.None); + + // Assert + var body = CaptureSentMail().Body!; + body.ShouldNotContain("