From 9c16127bd4d9c6cb690c3a83a96179e5b606004e Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Mon, 31 Mar 2025 14:00:43 -0500 Subject: [PATCH] [PM-14406] Fix security task email sends (#5571) * convert `AdminOwnerEmails` to List rather than IEnumerable * check for JSON array in `formatAdminOwnerEmails` * remove trailing comma for admin/owners * Use display block on tables to enforce padding * update padding around review at-risk passwords --- .../SecurityTasksNotification.html.hbs | 9 ++++---- .../Mail/SecurityTaskNotificationViewModel.cs | 2 +- .../Implementations/HandlebarsMailService.cs | 21 ++++++++++++++++--- .../CreateManyTaskNotificationsCommand.cs | 13 +++++++++--- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs b/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs index ca015e3e83..79c3893785 100644 --- a/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs +++ b/src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs @@ -14,18 +14,17 @@ - +
- -
+ Review at-risk passwords
+
+
{{formatAdminOwnerEmails AdminOwnerEmails}} diff --git a/src/Core/Models/Mail/SecurityTaskNotificationViewModel.cs b/src/Core/Models/Mail/SecurityTaskNotificationViewModel.cs index 8871a53424..9b4ede6e01 100644 --- a/src/Core/Models/Mail/SecurityTaskNotificationViewModel.cs +++ b/src/Core/Models/Mail/SecurityTaskNotificationViewModel.cs @@ -8,7 +8,7 @@ public class SecurityTaskNotificationViewModel : BaseMailModel public bool TaskCountPlural => TaskCount != 1; - public IEnumerable AdminOwnerEmails { get; set; } + public List AdminOwnerEmails { get; set; } public string ReviewPasswordsUrl => $"{WebVaultUrl}/browser-extension-prompt"; } diff --git a/src/Core/Services/Implementations/HandlebarsMailService.cs b/src/Core/Services/Implementations/HandlebarsMailService.cs index edb99809f7..430636f44d 100644 --- a/src/Core/Services/Implementations/HandlebarsMailService.cs +++ b/src/Core/Services/Implementations/HandlebarsMailService.cs @@ -1,5 +1,6 @@ using System.Net; using System.Reflection; +using System.Text.Json; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Models.Mail; @@ -752,7 +753,21 @@ public class HandlebarsMailService : IMailService return; } - var emailList = ((IEnumerable)parameters[0]).ToList(); + var emailList = new List(); + if (parameters[0] is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Array) + { + emailList = jsonElement.EnumerateArray().Select(e => e.GetString()).ToList(); + } + else if (parameters[0] is IEnumerable emails) + { + emailList = emails.ToList(); + } + else + { + writer.WriteSafeString(string.Empty); + return; + } + if (emailList.Count == 0) { writer.WriteSafeString(string.Empty); @@ -774,7 +789,7 @@ public class HandlebarsMailService : IMailService { outputMessage += string.Join(", ", emailList.Take(emailList.Count - 1) .Select(email => constructAnchorElement(email))); - outputMessage += $", and {constructAnchorElement(emailList.Last())}."; + outputMessage += $" and {constructAnchorElement(emailList.Last())}."; } writer.WriteSafeString($"{outputMessage}"); @@ -1250,7 +1265,7 @@ public class HandlebarsMailService : IMailService { OrgName = CoreHelpers.SanitizeForEmail(sanitizedOrgName, false), TaskCount = notification.TaskCount, - AdminOwnerEmails = adminOwnerEmails, + AdminOwnerEmails = adminOwnerEmails.ToList(), WebVaultUrl = _globalSettings.BaseServiceUri.VaultWithHash, }; message.Category = "SecurityTasksNotification"; diff --git a/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs b/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs index a335b059a4..e68a2ed726 100644 --- a/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs +++ b/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs @@ -48,9 +48,16 @@ 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(); + var orgAdminEmails = (await _organizationUserRepository.GetManyDetailsByRoleAsync(orgId, OrganizationUserType.Admin)) + .Select(u => u.Email) + .ToList(); + + var orgOwnerEmails = (await _organizationUserRepository.GetManyDetailsByRoleAsync(orgId, OrganizationUserType.Owner)) + .Select(u => u.Email) + .ToList(); + + // Ensure proper deserialization of emails + var orgAdminAndOwnerEmails = orgAdminEmails.Concat(orgOwnerEmails).Distinct().ToList(); await _mailService.SendBulkSecurityTaskNotificationsAsync(organization, userTaskCount, orgAdminAndOwnerEmails);