nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-04-04 12:40:22 -05:00
Code

[PM-18858] Security Task email bugs (#5536)

Browse Source 
* 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
This commit is contained in:
Nick Krantz 2025-03-20 14:41:58 -05:00 committed by GitHub
parent 2d02ad3f61
commit 948d8f707d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 71 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs
View File

@ -15,14 +15,21 @@
</tr>
</table>
<table width="100%" border="0" cellpadding="0" cellspacing="0"
style="display: table; width:100%; padding-bottom: 35px; text-align: center;" align="center">
style="display: table; width:100%; padding-bottom: 24px; text-align: center;" align="center">
<tr>
<td display="display: table-cell">
<a href="{{ReviewPasswordsUrl}}" clicktracking=off target="_blank"
style="display: inline-block; 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;">
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
</a>
</td>
</tr>
<table width="100%" border="0" cellpadding="0" cellspacing="0"
style="display: table; width:100%; padding-bottom: 24px; text-align: center;" align="center">
<tr>
<td display="font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-style: normal; font-weight: 400; font-size: 12px; line-height: 16px;">
{{formatAdminOwnerEmails AdminOwnerEmails}}
</td>
</tr>
</table>
{{/SecurityTasksHtmlLayout}}

9
src/Core/MailTemplates/Handlebars/SecurityTasksNotification.text.hbs
View File

@ -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}}

2
src/Core/Models/Mail/SecurityTaskNotificationViewModel.cs
View File

@ -8,5 +8,7 @@ public class SecurityTaskNotificationViewModel : BaseMailModel
public bool TaskCountPlural => TaskCount != 1;
public IEnumerable<string> AdminOwnerEmails { get; set; }
public string ReviewPasswordsUrl => $"{WebVaultUrl}/browser-extension-prompt";
}

2
src/Core/Services/IMailService.cs
View File

@ -99,5 +99,5 @@ public interface IMailService
string organizationName);
Task SendClaimedDomainUserEmailAsync(ManagedUserDomainClaimedEmails emailList);
Task SendDeviceApprovalRequestedNotificationEmailAsync(IEnumerable<string> adminEmails, Guid organizationId, string email, string userName);
Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable<UserSecurityTasksCount> securityTaskNotifications);
Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable<UserSecurityTasksCount> securityTaskNotifications, IEnumerable<string> adminOwnerEmails);
}

42
src/Core/Services/Implementations/HandlebarsMailService.cs
View File

@ -740,6 +740,45 @@ public class HandlebarsMailService : IMailService
var clickTrackingText = (clickTrackingOff ? "clicktracking=off" : string.Empty);
writer.WriteSafeString($"<a href=\"{href}\" target=\"_blank\" {clickTrackingText}>{text}</a>");
});
// 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<string>)parameters[0]).ToList();
if (emailList.Count == 0)
{
writer.WriteSafeString(string.Empty);
return;
}
string constructAnchorElement(string email)
{
return $"<a style=\"color: #175DDC\" href=\"mailto:{email}\">{email}</a>";
}
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<UserSecurityTasksCount> securityTaskNotifications)
public async Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable<UserSecurityTasksCount> securityTaskNotifications, IEnumerable<string> 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";

2
src/Core/Services/NoopImplementations/NoopMailService.cs
View File

@ -324,7 +324,7 @@ public class NoopMailService : IMailService
return Task.FromResult(0);
}
public Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable<UserSecurityTasksCount> securityTaskNotifications)
public Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable<UserSecurityTasksCount> securityTaskNotifications, IEnumerable<string> adminOwnerEmails)
{
return Task.FromResult(0);
}

10
src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs
View File

@ -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<SecurityTask> 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)