From 948d8f707d7d98506b6f87f4157721e8b2838c22 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Thu, 20 Mar 2025 14:41:58 -0500 Subject: [PATCH] [PM-18858] Security Task email bugs (#5536) * make "Review at-risk passwords" bold * add owner and admin email address to the bottom of the security notification email * fix plurality of text email --- .../SecurityTasksNotification.html.hbs | 11 ++++- .../SecurityTasksNotification.text.hbs | 9 ++++ .../Mail/SecurityTaskNotificationViewModel.cs | 2 + src/Core/Services/IMailService.cs | 2 +- .../Implementations/HandlebarsMailService.cs | 42 ++++++++++++++++++- .../NoopImplementations/NoopMailService.cs | 2 +- .../CreateManyTaskNotificationsCommand.cs | 10 ++++- 7 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs b/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs index 039806f44b..ca015e3e83 100644 --- a/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs +++ b/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs @@ -15,14 +15,21 @@ + style="display: table; width:100%; padding-bottom: 24px; text-align: center;" align="center"> +
+ style="display: inline-block; font-weight: bold; color: #ffffff; text-decoration: none; text-align: center; cursor: pointer; border-radius: 999px; background-color: #175DDC; border-color: #175DDC; border-style: solid; border-width: 10px 20px; margin: 0; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; line-height: 25px; -webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none;"> Review at-risk passwords
+ + +
+ {{formatAdminOwnerEmails AdminOwnerEmails}} +
{{/SecurityTasksHtmlLayout}} diff --git a/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.text.hbs b/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.text.hbs index ba8650ad10..f5493e4503 100644 --- a/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.text.hbs +++ b/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.text.hbs @@ -5,4 +5,13 @@ breach. Launch the Bitwarden extension to review your at-risk passwords. Review at-risk passwords ({{{ReviewPasswordsUrl}}}) + +{{#if (eq (length AdminOwnerEmails) 1)}} +This request was initiated by {{AdminOwnerEmails.[0]}}. +{{else}} +This request was initiated by +{{#each AdminOwnerEmails}} + {{#if @last}}and {{/if}}{{this}}{{#unless @last}}, {{/unless}} +{{/each}}. +{{/if}} {{/SecurityTasksHtmlLayout}} diff --git a/src/Core/Models/Mail/SecurityTaskNotificationViewModel.cs b/src/Core/Models/Mail/SecurityTaskNotificationViewModel.cs index 7f93ac2439..8871a53424 100644 --- a/src/Core/Models/Mail/SecurityTaskNotificationViewModel.cs +++ b/src/Core/Models/Mail/SecurityTaskNotificationViewModel.cs @@ -8,5 +8,7 @@ public class SecurityTaskNotificationViewModel : BaseMailModel public bool TaskCountPlural => TaskCount != 1; + public IEnumerable AdminOwnerEmails { get; set; } + public string ReviewPasswordsUrl => $"{WebVaultUrl}/browser-extension-prompt"; } diff --git a/src/Core/Services/IMailService.cs b/src/Core/Services/IMailService.cs index 04b302bad9..e61127c57a 100644 --- a/src/Core/Services/IMailService.cs +++ b/src/Core/Services/IMailService.cs @@ -99,5 +99,5 @@ public interface IMailService string organizationName); Task SendClaimedDomainUserEmailAsync(ManagedUserDomainClaimedEmails emailList); Task SendDeviceApprovalRequestedNotificationEmailAsync(IEnumerable adminEmails, Guid organizationId, string email, string userName); - Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable securityTaskNotifications); + Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable securityTaskNotifications, IEnumerable adminOwnerEmails); } diff --git a/src/Core/Services/Implementations/HandlebarsMailService.cs b/src/Core/Services/Implementations/HandlebarsMailService.cs index 588365f8c9..edb99809f7 100644 --- a/src/Core/Services/Implementations/HandlebarsMailService.cs +++ b/src/Core/Services/Implementations/HandlebarsMailService.cs @@ -740,6 +740,45 @@ public class HandlebarsMailService : IMailService var clickTrackingText = (clickTrackingOff ? "clicktracking=off" : string.Empty); writer.WriteSafeString($"{text}"); }); + + // Construct markup for admin and owner email addresses. + // Using conditionals within the handlebar syntax was including extra spaces around + // concatenated strings, which this helper avoids. + Handlebars.RegisterHelper("formatAdminOwnerEmails", (writer, context, parameters) => + { + if (parameters.Length == 0) + { + writer.WriteSafeString(string.Empty); + return; + } + + var emailList = ((IEnumerable)parameters[0]).ToList(); + if (emailList.Count == 0) + { + writer.WriteSafeString(string.Empty); + return; + } + + string constructAnchorElement(string email) + { + return $"{email}"; + } + + var outputMessage = "This request was initiated by "; + + if (emailList.Count == 1) + { + outputMessage += $"{constructAnchorElement(emailList[0])}."; + } + else + { + outputMessage += string.Join(", ", emailList.Take(emailList.Count - 1) + .Select(email => constructAnchorElement(email))); + outputMessage += $", and {constructAnchorElement(emailList.Last())}."; + } + + writer.WriteSafeString($"{outputMessage}"); + }); } public async Task SendEmergencyAccessInviteEmailAsync(EmergencyAccess emergencyAccess, string name, string token) @@ -1201,7 +1240,7 @@ public class HandlebarsMailService : IMailService await _mailDeliveryService.SendEmailAsync(message); } - public async Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable securityTaskNotifications) + public async Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable securityTaskNotifications, IEnumerable adminOwnerEmails) { MailQueueMessage CreateMessage(UserSecurityTasksCount notification) { @@ -1211,6 +1250,7 @@ public class HandlebarsMailService : IMailService { OrgName = CoreHelpers.SanitizeForEmail(sanitizedOrgName, false), TaskCount = notification.TaskCount, + AdminOwnerEmails = adminOwnerEmails, WebVaultUrl = _globalSettings.BaseServiceUri.VaultWithHash, }; message.Category = "SecurityTasksNotification"; diff --git a/src/Core/Services/NoopImplementations/NoopMailService.cs b/src/Core/Services/NoopImplementations/NoopMailService.cs index 776dd07f19..d829fbbacb 100644 --- a/src/Core/Services/NoopImplementations/NoopMailService.cs +++ b/src/Core/Services/NoopImplementations/NoopMailService.cs @@ -324,7 +324,7 @@ public class NoopMailService : IMailService return Task.FromResult(0); } - public Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable securityTaskNotifications) + public Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable securityTaskNotifications, IEnumerable adminOwnerEmails) { return Task.FromResult(0); } diff --git a/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs b/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs index f939816301..a335b059a4 100644 --- a/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs +++ b/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs @@ -17,19 +17,22 @@ public class CreateManyTaskNotificationsCommand : ICreateManyTaskNotificationsCo private readonly IMailService _mailService; private readonly ICreateNotificationCommand _createNotificationCommand; private readonly IPushNotificationService _pushNotificationService; + private readonly IOrganizationUserRepository _organizationUserRepository; public CreateManyTaskNotificationsCommand( IGetSecurityTasksNotificationDetailsQuery getSecurityTasksNotificationDetailsQuery, IOrganizationRepository organizationRepository, IMailService mailService, ICreateNotificationCommand createNotificationCommand, - IPushNotificationService pushNotificationService) + IPushNotificationService pushNotificationService, + IOrganizationUserRepository organizationUserRepository) { _getSecurityTasksNotificationDetailsQuery = getSecurityTasksNotificationDetailsQuery; _organizationRepository = organizationRepository; _mailService = mailService; _createNotificationCommand = createNotificationCommand; _pushNotificationService = pushNotificationService; + _organizationUserRepository = organizationUserRepository; } public async Task CreateAsync(Guid orgId, IEnumerable securityTasks) @@ -45,8 +48,11 @@ public class CreateManyTaskNotificationsCommand : ICreateManyTaskNotificationsCo }).ToList(); var organization = await _organizationRepository.GetByIdAsync(orgId); + var orgAdminEmails = await _organizationUserRepository.GetManyDetailsByRoleAsync(orgId, OrganizationUserType.Admin); + var orgOwnerEmails = await _organizationUserRepository.GetManyDetailsByRoleAsync(orgId, OrganizationUserType.Owner); + var orgAdminAndOwnerEmails = orgAdminEmails.Concat(orgOwnerEmails).Select(x => x.Email).Distinct().ToList(); - await _mailService.SendBulkSecurityTaskNotificationsAsync(organization, userTaskCount); + await _mailService.SendBulkSecurityTaskNotificationsAsync(organization, userTaskCount, orgAdminAndOwnerEmails); // Break securityTaskCiphers into separate lists by user Id var securityTaskCiphersByUser = securityTaskCiphers.GroupBy(x => x.UserId)