From 0a1238f887bf349674cee95ed387e92b2b0226d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Mon, 4 Nov 2024 16:37:21 +0000 Subject: [PATCH] [PM-11406] Account Management: Prevent a verified user from deleting their account (#4878) * Add check for managed user before purging account * Rename IOrganizationRepository.GetByClaimedUserDomainAsync to GetByVerifiedUserEmailDomainAsync and refactor to return a list. Remove ManagedByOrganizationId from ProfileResponseMode. Add ManagesActiveUser to ProfileOrganizationResponseModel * Rename the property ManagesActiveUser to UserIsManagedByOrganization * Remove whole class #nullable enable and add it to specific places * [PM-11405] Account Deprovisioning: Prevent a verified user from changing their email address * Remove unnecessary .ToList() * Refactor IUserService methods GetOrganizationsManagingUserAsync and IsManagedByAnyOrganizationAsync to not return nullable objects. Update ProfileOrganizationResponseModel.UserIsManagedByOrganization to not be nullable * Prevent deletion of accounts managed by an organization when Account Deprovisioning is enabled * Add CannotDeleteManagedAccountViewModel and email templates - Added CannotDeleteManagedAccountViewModel class to handle emails related to preventing deletion of accounts managed by an organization. - Added HTML and text email templates for sending notifications about the inability to delete an account owned by an organization. - Updated IMailService interface with a new method to send the cannot delete managed account email. - Implemented the SendCannotDeleteManagedAccountEmailAsync method in HandlebarsMailService. - Added a check in UserService to send the cannot delete managed account email if the user is managed by any organization. - Added a no-op implementation for SendCannotDeleteManagedAccountEmailAsync in NoopMailService. * Update error message when unable to purge vault for managed account * Update error message when unable to change email for managed account * Update error message when unable to delete account when managed by organization * Update error message in test for deleting organization-owned accounts --- .../Auth/Controllers/AccountsController.cs | 7 +++++ .../CannotDeleteManagedAccountViewModel.cs | 7 +++++ .../CannotDeleteManagedAccount.html.hbs | 15 ++++++++++ .../CannotDeleteManagedAccount.text.hbs | 6 ++++ src/Core/Services/IMailService.cs | 1 + .../Implementations/HandlebarsMailService.cs | 13 +++++++++ .../Services/Implementations/UserService.cs | 6 ++++ .../NoopImplementations/NoopMailService.cs | 5 ++++ .../Controllers/AccountsControllerTests.cs | 28 +++++++++++++++++++ 9 files changed, 88 insertions(+) create mode 100644 src/Core/Auth/Models/Mail/CannotDeleteManagedAccountViewModel.cs create mode 100644 src/Core/MailTemplates/Handlebars/AdminConsole/CannotDeleteManagedAccount.html.hbs create mode 100644 src/Core/MailTemplates/Handlebars/AdminConsole/CannotDeleteManagedAccount.text.hbs diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index d9dfbafc7..193077dc1 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -580,6 +580,13 @@ public class AccountsController : Controller } else { + // If Account Deprovisioning is enabled, we need to check if the user is managed by any organization. + if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + && await _userService.IsManagedByAnyOrganizationAsync(user.Id)) + { + throw new BadRequestException("Cannot delete accounts owned by an organization. Contact your organization administrator for additional details."); + } + var result = await _userService.DeleteAsync(user); if (result.Succeeded) { diff --git a/src/Core/Auth/Models/Mail/CannotDeleteManagedAccountViewModel.cs b/src/Core/Auth/Models/Mail/CannotDeleteManagedAccountViewModel.cs new file mode 100644 index 000000000..02549a959 --- /dev/null +++ b/src/Core/Auth/Models/Mail/CannotDeleteManagedAccountViewModel.cs @@ -0,0 +1,7 @@ +using Bit.Core.Models.Mail; + +namespace Bit.Core.Auth.Models.Mail; + +public class CannotDeleteManagedAccountViewModel : BaseMailModel +{ +} diff --git a/src/Core/MailTemplates/Handlebars/AdminConsole/CannotDeleteManagedAccount.html.hbs b/src/Core/MailTemplates/Handlebars/AdminConsole/CannotDeleteManagedAccount.html.hbs new file mode 100644 index 000000000..e867bf4f1 --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/AdminConsole/CannotDeleteManagedAccount.html.hbs @@ -0,0 +1,15 @@ +{{#>FullHtmlLayout}} + + + + + + + +
+ You have requested to delete your account. This action cannot be completed because your account is owned by an organization. +
+ Please contact your organization administrator for additional details. +
+
+{{/FullHtmlLayout}} diff --git a/src/Core/MailTemplates/Handlebars/AdminConsole/CannotDeleteManagedAccount.text.hbs b/src/Core/MailTemplates/Handlebars/AdminConsole/CannotDeleteManagedAccount.text.hbs new file mode 100644 index 000000000..3b71a1b1f --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/AdminConsole/CannotDeleteManagedAccount.text.hbs @@ -0,0 +1,6 @@ +{{#>BasicTextLayout}} +You have requested to delete your account. This action cannot be completed because your account is owned by an organization. + +Please contact your organization administrator for additional details. + +{{/BasicTextLayout}} diff --git a/src/Core/Services/IMailService.cs b/src/Core/Services/IMailService.cs index 9ae275719..fe801af81 100644 --- a/src/Core/Services/IMailService.cs +++ b/src/Core/Services/IMailService.cs @@ -18,6 +18,7 @@ public interface IMailService ProductTierType productTier, IEnumerable products); Task SendVerifyDeleteEmailAsync(string email, Guid userId, string token); + Task SendCannotDeleteManagedAccountEmailAsync(string email); Task SendChangeEmailAlreadyExistsEmailAsync(string fromEmail, string toEmail); Task SendChangeEmailEmailAsync(string newEmailAddress, string token); Task SendTwoFactorEmailAsync(string email, string token); diff --git a/src/Core/Services/Implementations/HandlebarsMailService.cs b/src/Core/Services/Implementations/HandlebarsMailService.cs index b22be0622..007231299 100644 --- a/src/Core/Services/Implementations/HandlebarsMailService.cs +++ b/src/Core/Services/Implementations/HandlebarsMailService.cs @@ -112,6 +112,19 @@ public class HandlebarsMailService : IMailService await _mailDeliveryService.SendEmailAsync(message); } + public async Task SendCannotDeleteManagedAccountEmailAsync(string email) + { + var message = CreateDefaultMessage("Delete Your Account", email); + var model = new CannotDeleteManagedAccountViewModel + { + WebVaultUrl = _globalSettings.BaseServiceUri.VaultWithHash, + SiteName = _globalSettings.SiteName, + }; + await AddMessageContentAsync(message, "AdminConsole.CannotDeleteManagedAccount", model); + message.Category = "CannotDeleteManagedAccount"; + await _mailDeliveryService.SendEmailAsync(message); + } + public async Task SendChangeEmailAlreadyExistsEmailAsync(string fromEmail, string toEmail) { var message = CreateDefaultMessage("Your Email Change", toEmail); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index f2e1d183d..2199d0a7a 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -297,6 +297,12 @@ public class UserService : UserManager, IUserService, IDisposable return; } + if (await IsManagedByAnyOrganizationAsync(user.Id)) + { + await _mailService.SendCannotDeleteManagedAccountEmailAsync(user.Email); + return; + } + var token = await base.GenerateUserTokenAsync(user, TokenOptions.DefaultProvider, "DeleteAccount"); await _mailService.SendVerifyDeleteEmailAsync(user.Email, user.Id, token); } diff --git a/src/Core/Services/NoopImplementations/NoopMailService.cs b/src/Core/Services/NoopImplementations/NoopMailService.cs index 469673057..edada01da 100644 --- a/src/Core/Services/NoopImplementations/NoopMailService.cs +++ b/src/Core/Services/NoopImplementations/NoopMailService.cs @@ -94,6 +94,11 @@ public class NoopMailService : IMailService return Task.FromResult(0); } + public Task SendCannotDeleteManagedAccountEmailAsync(string email) + { + return Task.FromResult(0); + } + public Task SendPasswordlessSignInAsync(string returnUrl, string token, string email) { return Task.FromResult(0); diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index 4127c92ee..13c80f856 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -534,6 +534,34 @@ public class AccountsControllerTests : IDisposable await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(model)); } + [Fact] + public async Task Delete_WhenAccountDeprovisioningIsEnabled_WithUserManagedByAnOrganization_ThrowsBadRequestException() + { + var user = GenerateExampleUser(); + ConfigureUserServiceToReturnValidPrincipalFor(user); + ConfigureUserServiceToAcceptPasswordFor(user); + _featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning).Returns(true); + _userService.IsManagedByAnyOrganizationAsync(user.Id).Returns(true); + + var result = await Assert.ThrowsAsync(() => _sut.Delete(new SecretVerificationRequestModel())); + + Assert.Equal("Cannot delete accounts owned by an organization. Contact your organization administrator for additional details.", result.Message); + } + + [Fact] + public async Task Delete_WhenAccountDeprovisioningIsEnabled_WithUserNotManagedByAnOrganization_ShouldSucceed() + { + var user = GenerateExampleUser(); + ConfigureUserServiceToReturnValidPrincipalFor(user); + ConfigureUserServiceToAcceptPasswordFor(user); + _featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning).Returns(true); + _userService.IsManagedByAnyOrganizationAsync(user.Id).Returns(false); + _userService.DeleteAsync(user).Returns(IdentityResult.Success); + + await _sut.Delete(new SecretVerificationRequestModel()); + + await _userService.Received(1).DeleteAsync(user); + } // Below are helper functions that currently belong to this // test class, but ultimately may need to be split out into