From 3b4c8afea05f14c6ecb5d04305aaf9327dfe781b Mon Sep 17 00:00:00 2001 From: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Date: Thu, 6 Jul 2023 10:03:49 -0500 Subject: [PATCH] [AC-1191] TDE admin approval email (#3044) * feat: add new command for updating request and emailing user, refs AC-1191 * feat: inject service with organization service collection extensions, refs AC-1191 * feat: add function to send admin approval email to mail services (interface/noop/handlebars), refs AC-1191 * feat: add html/text mail templates and add view model for email data, refs AC-1191 * feat: update org auth request controller to use new command during auth request update, refs AC-1191 * fix: dotnet format, refs AC-1191 * refactor: update user not found error, FirstOrDefault for enum type display name, refs AC-1191 * refactor: update user not found to log error instead of throws, refs AC-1191 * fix: remove whitespace lint errors, refs AC-1191 * refactor: update hardcoded UTC timezone string, refs AC-1191 * refactor: add unit test for new command, refs AC-1191 * refactor: improve enum name fallback and identifier string creation, refs AC-1191 * refactor: add addtional unit tests, refs AC-1191 * refactor: update success test to use more generated params, refs AC-1191 * fix: dotnet format...again, refs AC-1191 * refactor: make UTC display a constant for handlebars mail service, refs AC-1191 * refactor: update displayTypeIdentifer to displayTypeAndIdentifier for clarity, refs AC-1191 --- .../OrganizationAuthRequestsController.cs | 10 +- .../IUpdateOrganizationAuthRequestCommand.cs | 6 ++ .../UpdateOrganizationAuthRequestCommand.cs | 55 ++++++++++ .../Auth/TrustedDeviceAdminApproval.html.hbs | 22 ++++ .../Auth/TrustedDeviceAdminApproval.text.hbs | 9 ++ .../TrustedDeviceAdminApprovalViewModel.cs | 3 + ...OrganizationServiceCollectionExtensions.cs | 11 +- src/Core/Services/IMailService.cs | 2 + .../Implementations/HandlebarsMailService.cs | 27 ++++- .../NoopImplementations/NoopMailService.cs | 6 ++ ...dateOrganizationAuthRequestCommandTests.cs | 100 ++++++++++++++++++ 11 files changed, 243 insertions(+), 8 deletions(-) create mode 100644 src/Core/AdminConsole/OrganizationAuth/Interfaces/IUpdateOrganizationAuthRequestCommand.cs create mode 100644 src/Core/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommand.cs create mode 100644 src/Core/MailTemplates/Handlebars/Auth/TrustedDeviceAdminApproval.html.hbs create mode 100644 src/Core/MailTemplates/Handlebars/Auth/TrustedDeviceAdminApproval.text.hbs create mode 100644 src/Core/Models/Mail/TrustedDeviceAdminApprovalViewModel.cs create mode 100644 test/Core.Test/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommandTests.cs diff --git a/src/Api/Controllers/OrganizationAuthRequestsController.cs b/src/Api/Controllers/OrganizationAuthRequestsController.cs index eff206a05..5435b44d9 100644 --- a/src/Api/Controllers/OrganizationAuthRequestsController.cs +++ b/src/Api/Controllers/OrganizationAuthRequestsController.cs @@ -2,6 +2,7 @@ using Bit.Api.Auth.Models.Response; using Bit.Api.Models.Response; using Bit.Core; +using Bit.Core.AdminConsole.OrganizationAuth.Interfaces; using Bit.Core.Auth.Models.Api.Request.AuthRequest; using Bit.Core.Auth.Services; using Bit.Core.Context; @@ -21,13 +22,16 @@ public class OrganizationAuthRequestsController : Controller private readonly IAuthRequestRepository _authRequestRepository; private readonly ICurrentContext _currentContext; private readonly IAuthRequestService _authRequestService; + private readonly IUpdateOrganizationAuthRequestCommand _updateOrganizationAuthRequestCommand; public OrganizationAuthRequestsController(IAuthRequestRepository authRequestRepository, - ICurrentContext currentContext, IAuthRequestService authRequestService) + ICurrentContext currentContext, IAuthRequestService authRequestService, + IUpdateOrganizationAuthRequestCommand updateOrganizationAuthRequestCommand) { _authRequestRepository = authRequestRepository; _currentContext = currentContext; _authRequestService = authRequestService; + _updateOrganizationAuthRequestCommand = updateOrganizationAuthRequestCommand; } [HttpGet("")] @@ -55,8 +59,7 @@ public class OrganizationAuthRequestsController : Controller throw new NotFoundException(); } - await _authRequestService.UpdateAuthRequestAsync(authRequest.Id, authRequest.UserId, - new AuthRequestUpdateRequestModel { RequestApproved = model.RequestApproved, Key = model.EncryptedUserKey }); + await _updateOrganizationAuthRequestCommand.UpdateAsync(authRequest.Id, authRequest.UserId, model.RequestApproved, model.EncryptedUserKey); } [HttpPost("deny")] @@ -81,3 +84,4 @@ public class OrganizationAuthRequestsController : Controller } } } + diff --git a/src/Core/AdminConsole/OrganizationAuth/Interfaces/IUpdateOrganizationAuthRequestCommand.cs b/src/Core/AdminConsole/OrganizationAuth/Interfaces/IUpdateOrganizationAuthRequestCommand.cs new file mode 100644 index 000000000..4b119d1e8 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationAuth/Interfaces/IUpdateOrganizationAuthRequestCommand.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.AdminConsole.OrganizationAuth.Interfaces; + +public interface IUpdateOrganizationAuthRequestCommand +{ + Task UpdateAsync(Guid requestId, Guid userId, bool requestApproved, string encryptedUserKey); +} diff --git a/src/Core/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommand.cs b/src/Core/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommand.cs new file mode 100644 index 000000000..79d2d70e8 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommand.cs @@ -0,0 +1,55 @@ +using System.ComponentModel.DataAnnotations; +using System.Reflection; +using Bit.Core.AdminConsole.OrganizationAuth.Interfaces; +using Bit.Core.Auth.Models.Api.Request.AuthRequest; +using Bit.Core.Auth.Services; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Microsoft.Extensions.Logging; + +namespace Bit.Core.AdminConsole.OrganizationAuth; + +public class UpdateOrganizationAuthRequestCommand : IUpdateOrganizationAuthRequestCommand +{ + private readonly IAuthRequestService _authRequestService; + private readonly IMailService _mailService; + private readonly IUserRepository _userRepository; + private readonly ILogger _logger; + + public UpdateOrganizationAuthRequestCommand( + IAuthRequestService authRequestService, + IMailService mailService, + IUserRepository userRepository, + ILogger logger) + { + _authRequestService = authRequestService; + _mailService = mailService; + _userRepository = userRepository; + _logger = logger; + } + + public async Task UpdateAsync(Guid requestId, Guid userId, bool requestApproved, string encryptedUserKey) + { + var updatedAuthRequest = await _authRequestService.UpdateAuthRequestAsync(requestId, userId, + new AuthRequestUpdateRequestModel { RequestApproved = requestApproved, Key = encryptedUserKey }); + + if (updatedAuthRequest.Approved is true) + { + var user = await _userRepository.GetByIdAsync(userId); + if (user == null) + { + _logger.LogError("User ({id}) not found. Trusted device admin approval email not sent.", userId); + return; + } + var approvalDateTime = updatedAuthRequest.ResponseDate ?? DateTime.UtcNow; + var deviceTypeDisplayName = updatedAuthRequest.RequestDeviceType.GetType() + .GetMember(updatedAuthRequest.RequestDeviceType.ToString()) + .FirstOrDefault()? + .GetCustomAttribute()?.Name ?? "Unknown"; + var deviceTypeAndIdentifier = $"{deviceTypeDisplayName} - {updatedAuthRequest.RequestDeviceIdentifier}"; + await _mailService.SendTrustedDeviceAdminApprovalEmailAsync(user.Email, approvalDateTime, + updatedAuthRequest.RequestIpAddress, deviceTypeAndIdentifier); + } + } +} + diff --git a/src/Core/MailTemplates/Handlebars/Auth/TrustedDeviceAdminApproval.html.hbs b/src/Core/MailTemplates/Handlebars/Auth/TrustedDeviceAdminApproval.html.hbs new file mode 100644 index 000000000..6ffe9ba04 --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/Auth/TrustedDeviceAdminApproval.html.hbs @@ -0,0 +1,22 @@ +{{#>FullHtmlLayout}} + + + + + + + + + + +
+ You must log in on the device below within 12 hours or approval will expire. +
+ Device: {{DeviceType}}
+ IP Address: {{IpAddress}}
+ Date: {{TheDate}} at {{TheTime}} {{TimeZone}}
+
+ If you do not recognize this device, contact your organization administrator. +
+{{/FullHtmlLayout}} + diff --git a/src/Core/MailTemplates/Handlebars/Auth/TrustedDeviceAdminApproval.text.hbs b/src/Core/MailTemplates/Handlebars/Auth/TrustedDeviceAdminApproval.text.hbs new file mode 100644 index 000000000..8f08a3f46 --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/Auth/TrustedDeviceAdminApproval.text.hbs @@ -0,0 +1,9 @@ +{{#>BasicTextLayout}} +You must log in on the device below within 12 hours or approval will expire. + +Device Type: {{DeviceType}} +IP Address: {{IpAddress}} +Date: {{TheDate}} at {{TheTime}} {{TimeZone}} + +If you do not recognize this device, contact your organization administrator. +{{/BasicTextLayout}} diff --git a/src/Core/Models/Mail/TrustedDeviceAdminApprovalViewModel.cs b/src/Core/Models/Mail/TrustedDeviceAdminApprovalViewModel.cs new file mode 100644 index 000000000..f267e0e16 --- /dev/null +++ b/src/Core/Models/Mail/TrustedDeviceAdminApprovalViewModel.cs @@ -0,0 +1,3 @@ +namespace Bit.Core.Models.Mail; + +public class TrustedDeviceAdminApprovalViewModel : NewDeviceLoggedInModel { } diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index 9e3653779..f302aef90 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -1,4 +1,6 @@ -using Bit.Core.Models.Business.Tokenables; +using Bit.Core.AdminConsole.OrganizationAuth; +using Bit.Core.AdminConsole.OrganizationAuth.Interfaces; +using Bit.Core.Models.Business.Tokenables; using Bit.Core.OrganizationFeatures.Groups; using Bit.Core.OrganizationFeatures.Groups.Interfaces; using Bit.Core.OrganizationFeatures.OrganizationApiKeys; @@ -41,6 +43,7 @@ public static class OrganizationServiceCollectionExtensions services.AddOrganizationGroupCommands(); services.AddOrganizationLicenseCommandsQueries(); services.AddOrganizationDomainCommandsQueries(); + services.AddOrganizationAuthCommands(); } private static void AddOrganizationConnectionCommands(this IServiceCollection services) @@ -110,6 +113,11 @@ public static class OrganizationServiceCollectionExtensions services.AddScoped(); } + private static void AddOrganizationAuthCommands(this IServiceCollection services) + { + services.AddScoped(); + } + private static void AddTokenizers(this IServiceCollection services) { services.AddSingleton>(serviceProvider => @@ -121,3 +129,4 @@ public static class OrganizationServiceCollectionExtensions ); } } + diff --git a/src/Core/Services/IMailService.cs b/src/Core/Services/IMailService.cs index 933127367..9d2bfabd5 100644 --- a/src/Core/Services/IMailService.cs +++ b/src/Core/Services/IMailService.cs @@ -55,4 +55,6 @@ public interface IMailService Task SendFailedLoginAttemptsEmailAsync(string email, DateTime utcNow, string ip); Task SendFailedTwoFactorAttemptsEmailAsync(string email, DateTime utcNow, string ip); Task SendUnverifiedOrganizationDomainEmailAsync(IEnumerable adminEmails, string organizationId, string domainName); + Task SendTrustedDeviceAdminApprovalEmailAsync(string email, DateTime utcNow, string ip, string deviceTypeAndIdentifier); } + diff --git a/src/Core/Services/Implementations/HandlebarsMailService.cs b/src/Core/Services/Implementations/HandlebarsMailService.cs index 66fe70a01..03b309abd 100644 --- a/src/Core/Services/Implementations/HandlebarsMailService.cs +++ b/src/Core/Services/Implementations/HandlebarsMailService.cs @@ -17,6 +17,7 @@ namespace Bit.Core.Services; public class HandlebarsMailService : IMailService { private const string Namespace = "Bit.Core.MailTemplates.Handlebars"; + private const string _utcTimeZoneDisplay = "UTC"; private readonly GlobalSettings _globalSettings; private readonly IMailDeliveryService _mailDeliveryService; @@ -353,7 +354,7 @@ public class HandlebarsMailService : IMailService DeviceType = deviceType, TheDate = timestamp.ToLongDateString(), TheTime = timestamp.ToShortTimeString(), - TimeZone = "UTC", + TimeZone = _utcTimeZoneDisplay, IpAddress = ip }; await AddMessageContentAsync(message, "NewDeviceLoggedIn", model); @@ -370,7 +371,7 @@ public class HandlebarsMailService : IMailService SiteName = _globalSettings.SiteName, TheDate = timestamp.ToLongDateString(), TheTime = timestamp.ToShortTimeString(), - TimeZone = "UTC", + TimeZone = _utcTimeZoneDisplay, IpAddress = ip }; await AddMessageContentAsync(message, "Auth.RecoverTwoFactor", model); @@ -856,7 +857,7 @@ public class HandlebarsMailService : IMailService { TheDate = utcNow.ToLongDateString(), TheTime = utcNow.ToShortTimeString(), - TimeZone = "UTC", + TimeZone = _utcTimeZoneDisplay, IpAddress = ip, AffectedEmail = email @@ -873,7 +874,7 @@ public class HandlebarsMailService : IMailService { TheDate = utcNow.ToLongDateString(), TheTime = utcNow.ToShortTimeString(), - TimeZone = "UTC", + TimeZone = _utcTimeZoneDisplay, IpAddress = ip, AffectedEmail = email @@ -896,8 +897,26 @@ public class HandlebarsMailService : IMailService await _mailDeliveryService.SendEmailAsync(message); } + public async Task SendTrustedDeviceAdminApprovalEmailAsync(string email, DateTime utcNow, string ip, + string deviceTypeAndIdentifier) + { + var message = CreateDefaultMessage("Login request approved", email); + var model = new TrustedDeviceAdminApprovalViewModel + { + TheDate = utcNow.ToLongDateString(), + TheTime = utcNow.ToShortTimeString(), + TimeZone = _utcTimeZoneDisplay, + IpAddress = ip, + DeviceType = deviceTypeAndIdentifier, + }; + await AddMessageContentAsync(message, "Auth.TrustedDeviceAdminApproval", model); + message.Category = "TrustedDeviceAdminApproval"; + await _mailDeliveryService.SendEmailAsync(message); + } + private static string GetUserIdentifier(string email, string userName) { return string.IsNullOrEmpty(userName) ? email : CoreHelpers.SanitizeForEmail(userName, false); } } + diff --git a/src/Core/Services/NoopImplementations/NoopMailService.cs b/src/Core/Services/NoopImplementations/NoopMailService.cs index 808489efb..c26fee973 100644 --- a/src/Core/Services/NoopImplementations/NoopMailService.cs +++ b/src/Core/Services/NoopImplementations/NoopMailService.cs @@ -238,4 +238,10 @@ public class NoopMailService : IMailService { return Task.FromResult(0); } + + public Task SendTrustedDeviceAdminApprovalEmailAsync(string email, DateTime utcNow, string ip, string deviceTypeAndIdentifier) + { + return Task.FromResult(0); + } } + diff --git a/test/Core.Test/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommandTests.cs new file mode 100644 index 000000000..ac37158c5 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommandTests.cs @@ -0,0 +1,100 @@ +using Bit.Core.AdminConsole.OrganizationAuth; +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Models.Api.Request.AuthRequest; +using Bit.Core.Auth.Services; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationAuth; + +[SutProviderCustomize] +public class UpdateOrganizationAuthRequestCommandTests +{ + [Theory] + [BitAutoData] + public async Task UpdateOrgAuthRequest_Approved_SendEmail_Success( + DateTime responseDate, string email, DeviceType deviceType, string deviceIdentifier, + string requestIpAddress, Guid requestId, Guid userId, bool requestApproved, + string encryptedUserKey, SutProvider sutProvider) + { + var expectedDeviceTypeAndIdentifier = $"{deviceType} - {deviceIdentifier}"; + + sutProvider.GetDependency() + .UpdateAuthRequestAsync(requestId, userId, + Arg.Is(x => + x.RequestApproved == requestApproved && x.Key == encryptedUserKey)) + .Returns(new AuthRequest() + { + UserId = userId, + Approved = true, + ResponseDate = responseDate, + RequestDeviceType = deviceType, + RequestDeviceIdentifier = deviceIdentifier, + RequestIpAddress = requestIpAddress, + }); + + sutProvider.GetDependency() + .GetByIdAsync(userId) + .Returns(new User() + { + Email = email + }); + + await sutProvider.Sut.UpdateAsync(requestId, userId, requestApproved, encryptedUserKey); + + await sutProvider.GetDependency().Received(1).GetByIdAsync(userId); + await sutProvider.GetDependency().Received(1) + .SendTrustedDeviceAdminApprovalEmailAsync(email, responseDate, requestIpAddress, expectedDeviceTypeAndIdentifier); + } + + [Theory] + [BitAutoData] + public async Task UpdateOrgAuthRequest_Denied_NonExecutes( + SutProvider sutProvider, Guid requestId, Guid userId, + bool requestApproved, string encryptedUserKey) + { + sutProvider.GetDependency() + .UpdateAuthRequestAsync(requestId, userId, + Arg.Is(x => + x.RequestApproved == requestApproved && x.Key == encryptedUserKey)) + .Returns(new AuthRequest() { Approved = false }); + + await sutProvider.Sut.UpdateAsync(requestId, userId, requestApproved, encryptedUserKey); + + await sutProvider.GetDependency().DidNotReceive().GetByIdAsync(userId); + await sutProvider.GetDependency().DidNotReceive() + .SendTrustedDeviceAdminApprovalEmailAsync(Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task UpdateOrgAuthRequest_Approved_UserNotFound( + SutProvider sutProvider, Guid requestId, Guid userId, + bool requestApproved, string encryptedUserKey) + { + sutProvider.GetDependency() + .UpdateAuthRequestAsync(requestId, userId, + Arg.Is(x => + x.RequestApproved == requestApproved && x.Key == encryptedUserKey)) + .Returns(new AuthRequest() { Approved = true, }); + + sutProvider.GetDependency() + .GetByIdAsync(userId) + .ReturnsNull(); + + await sutProvider.Sut.UpdateAsync(requestId, userId, requestApproved, encryptedUserKey); + + await sutProvider.GetDependency().Received(1).GetByIdAsync(userId); + await sutProvider.GetDependency().DidNotReceive() + .SendTrustedDeviceAdminApprovalEmailAsync(Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any()); + } +}