From 43372b71684cc9a209d038b834cf8aa933d4bd26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Tue, 29 Jul 2025 16:17:16 +0100 Subject: [PATCH] [PM-20010] Fix purge logic to skip claimed user check for organization vault (#6107) * Implement unit tests for PostPurge method in CiphersController to handle various scenarios * Refactor PostPurge method in CiphersController to use Guid for organizationId parameter and update related unit tests * Refactor PostPurge method in CiphersController to skip checking if user is claimed if its purging the org vault --- .../Vault/Controllers/CiphersController.cs | 20 ++- .../Controllers/CiphersControllerTests.cs | 121 +++++++++++++++++- 2 files changed, 129 insertions(+), 12 deletions(-) diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 98ec78e9a0..761a5a3726 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -1113,7 +1113,7 @@ public class CiphersController : Controller } [HttpPost("purge")] - public async Task PostPurge([FromBody] SecretVerificationRequestModel model, string organizationId = null) + public async Task PostPurge([FromBody] SecretVerificationRequestModel model, Guid? organizationId = null) { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) @@ -1128,24 +1128,22 @@ public class CiphersController : Controller throw new BadRequestException(ModelState); } - // Check if the user is claimed by any organization. - if (await _userService.IsClaimedByAnyOrganizationAsync(user.Id)) - { - throw new BadRequestException("Cannot purge accounts owned by an organization. Contact your organization administrator for additional details."); - } - - if (string.IsNullOrWhiteSpace(organizationId)) + if (organizationId == null) { + // Check if the user is claimed by any organization. + if (await _userService.IsClaimedByAnyOrganizationAsync(user.Id)) + { + throw new BadRequestException("Cannot purge accounts owned by an organization. Contact your organization administrator for additional details."); + } await _cipherRepository.DeleteByUserIdAsync(user.Id); } else { - var orgId = new Guid(organizationId); - if (!await _currentContext.EditAnyCollection(orgId)) + if (!await _currentContext.EditAnyCollection(organizationId!.Value)) { throw new NotFoundException(); } - await _cipherService.PurgeAsync(orgId); + await _cipherService.PurgeAsync(organizationId!.Value); } } diff --git a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs index 2819fb8880..416b92f841 100644 --- a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs @@ -1,5 +1,6 @@ using System.Security.Claims; using System.Text.Json; +using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Vault.Controllers; using Bit.Api.Vault.Models; using Bit.Api.Vault.Models.Request; @@ -1789,5 +1790,123 @@ public class CiphersControllerTests ); } -} + [Theory, BitAutoData] + public async Task PostPurge_WhenUserNotFound_ThrowsUnauthorizedAccessException( + SecretVerificationRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetUserByPrincipalAsync(Arg.Any()) + .Returns((User)null); + await Assert.ThrowsAsync(() => sutProvider.Sut.PostPurge(model)); + } + + [Theory, BitAutoData] + public async Task PostPurge_WhenUserVerificationFails_ThrowsBadRequestException( + User user, + SecretVerificationRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetUserByPrincipalAsync(Arg.Any()) + .Returns(user); + sutProvider.GetDependency() + .VerifySecretAsync(user, model.Secret) + .Returns(false); + + await Assert.ThrowsAsync(() => sutProvider.Sut.PostPurge(model)); + } + + [Theory, BitAutoData] + public async Task PostPurge_UserPurge_WithClaimedUser_ThrowsBadRequestException( + User user, + SecretVerificationRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetUserByPrincipalAsync(Arg.Any()) + .Returns(user); + sutProvider.GetDependency() + .VerifySecretAsync(user, model.Secret) + .Returns(true); + sutProvider.GetDependency() + .IsClaimedByAnyOrganizationAsync(user.Id) + .Returns(true); + + await Assert.ThrowsAsync(() => sutProvider.Sut.PostPurge(model)); + } + + [Theory, BitAutoData] + public async Task PostPurge_UserPurge_WithUnclaimedUser_Successful( + User user, + SecretVerificationRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetUserByPrincipalAsync(Arg.Any()) + .Returns(user); + sutProvider.GetDependency() + .VerifySecretAsync(user, model.Secret) + .Returns(true); + sutProvider.GetDependency() + .IsClaimedByAnyOrganizationAsync(user.Id) + .Returns(false); + + await sutProvider.Sut.PostPurge(model); + + await sutProvider.GetDependency() + .Received(1) + .DeleteByUserIdAsync(user.Id); + } + + [Theory, BitAutoData] + public async Task PostPurge_OrganizationPurge_WithEditAnyCollectionPermission_Successful( + User user, + SecretVerificationRequestModel model, + Guid organizationId, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetUserByPrincipalAsync(Arg.Any()) + .Returns(user); + sutProvider.GetDependency() + .VerifySecretAsync(user, model.Secret) + .Returns(true); + sutProvider.GetDependency() + .IsClaimedByAnyOrganizationAsync(user.Id) + .Returns(true); + sutProvider.GetDependency() + .EditAnyCollection(organizationId) + .Returns(true); + + await sutProvider.Sut.PostPurge(model, organizationId); + + await sutProvider.GetDependency() + .Received(1) + .PurgeAsync(organizationId); + } + + [Theory, BitAutoData] + public async Task PostPurge_OrganizationPurge_WithInsufficientPermissions_ThrowsNotFoundException( + User user, + Guid organizationId, + SecretVerificationRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetUserByPrincipalAsync(Arg.Any()) + .Returns(user); + sutProvider.GetDependency() + .VerifySecretAsync(user, model.Secret) + .Returns(true); + sutProvider.GetDependency() + .IsClaimedByAnyOrganizationAsync(user.Id) + .Returns(false); + sutProvider.GetDependency() + .EditAnyCollection(organizationId) + .Returns(false); + + await Assert.ThrowsAsync(() => sutProvider.Sut.PostPurge(model, organizationId)); + } +}