From 2413ce10abb09155a57b3642db9f96c8906628ca Mon Sep 17 00:00:00 2001 From: Jason Ng Date: Mon, 9 Feb 2026 10:46:30 -0500 Subject: [PATCH] [PM-31745] Allow user to perm delete unassigned items (#6956) * update DeleteAdmin to grab items that are unassigned and ciphersControllerTests --- .../Vault/Controllers/CiphersController.cs | 4 +- .../Controllers/CiphersControllerTests.cs | 92 ++++++++++--------- 2 files changed, 53 insertions(+), 43 deletions(-) diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 9e107b491d..c9ca7525fa 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -976,14 +976,14 @@ public class CiphersController : Controller public async Task DeleteAdmin(Guid id) { var userId = _userService.GetProperUserId(User).Value; - var cipher = await GetByIdAsync(id, userId); + var cipher = await GetByIdAsyncAdmin(id); if (cipher == null || !cipher.OrganizationId.HasValue || !await CanDeleteOrRestoreCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id })) { throw new NotFoundException(); } - await _cipherService.DeleteAsync(cipher, userId, true); + await _cipherService.DeleteAsync(new CipherDetails(cipher), userId, true); } [HttpPost("{id}/delete-admin")] diff --git a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs index 238053464c..6fba9730a7 100644 --- a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs @@ -158,9 +158,9 @@ public class CiphersControllerTests [BitAutoData(OrganizationUserType.Custom, false, false)] public async Task CanEditCiphersAsAdminAsync_FlexibleCollections_Success( OrganizationUserType userType, bool allowAdminsAccessToAllItems, bool shouldSucceed, - CurrentContextOrganization organization, Guid userId, CipherDetails cipherDetails, SutProvider sutProvider) + CurrentContextOrganization organization, Guid userId, CipherOrganizationDetails cipherOrgDetails, SutProvider sutProvider) { - cipherDetails.OrganizationId = organization.Id; + cipherOrgDetails.OrganizationId = organization.Id; organization.Type = userType; if (userType == OrganizationUserType.Custom) { @@ -171,9 +171,9 @@ public class CiphersControllerTests sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(new User { Id = userId }); - sutProvider.GetDependency().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails); + sutProvider.GetDependency().GetOrganizationDetailsByIdAsync(cipherOrgDetails.Id).Returns(cipherOrgDetails); - sutProvider.GetDependency().GetManyByOrganizationIdAsync(organization.Id).Returns(new List { cipherDetails }); + sutProvider.GetDependency().GetManyByOrganizationIdAsync(organization.Id).Returns(new List { cipherOrgDetails }); sutProvider.GetDependency().GetOrganizationAbilityAsync(organization.Id).Returns(new OrganizationAbility { @@ -183,13 +183,13 @@ public class CiphersControllerTests if (shouldSucceed) { - await sutProvider.Sut.DeleteAdmin(cipherDetails.Id); + await sutProvider.Sut.DeleteAdmin(cipherOrgDetails.Id); await sutProvider.GetDependency().ReceivedWithAnyArgs() .DeleteAsync(default, default); } else { - await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteAdmin(cipherDetails.Id)); + await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteAdmin(cipherOrgDetails.Id)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .DeleteAsync(default, default); } @@ -199,25 +199,23 @@ public class CiphersControllerTests [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.Admin)] public async Task DeleteAdmin_WithOwnerOrAdmin_WithManagePermission_DeletesCipher( - OrganizationUserType organizationUserType, CipherDetails cipherDetails, Guid userId, + OrganizationUserType organizationUserType, CipherOrganizationDetails cipherOrgDetails, Guid userId, CurrentContextOrganization organization, SutProvider sutProvider) { - cipherDetails.UserId = null; - cipherDetails.OrganizationId = organization.Id; - cipherDetails.Edit = true; - cipherDetails.Manage = true; + cipherOrgDetails.UserId = null; + cipherOrgDetails.OrganizationId = organization.Id; organization.Type = organizationUserType; sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(new User { Id = userId }); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails); + sutProvider.GetDependency().GetOrganizationDetailsByIdAsync(cipherOrgDetails.Id).Returns(cipherOrgDetails); sutProvider.GetDependency() .GetManyByUserIdAsync(userId) .Returns(new List { - cipherDetails + new CipherDetails(cipherOrgDetails) { Edit = true, Manage = true } }); sutProvider.GetDependency() .GetOrganizationAbilityAsync(organization.Id) @@ -227,34 +225,35 @@ public class CiphersControllerTests LimitItemDeletion = true }); - await sutProvider.Sut.DeleteAdmin(cipherDetails.Id); + await sutProvider.Sut.DeleteAdmin(cipherOrgDetails.Id); - await sutProvider.GetDependency().Received(1).DeleteAsync(cipherDetails, userId, true); + await sutProvider.GetDependency().Received(1).DeleteAsync( + Arg.Is(c => c.Id == cipherOrgDetails.Id && c.OrganizationId == cipherOrgDetails.OrganizationId), + userId, + true); } [Theory] [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.Admin)] public async Task DeleteAdmin_WithOwnerOrAdmin_WithoutManagePermission_ThrowsNotFoundException( - OrganizationUserType organizationUserType, CipherDetails cipherDetails, Guid userId, + OrganizationUserType organizationUserType, CipherOrganizationDetails cipherOrgDetails, Guid userId, CurrentContextOrganization organization, SutProvider sutProvider) { - cipherDetails.UserId = null; - cipherDetails.OrganizationId = organization.Id; - cipherDetails.Edit = true; - cipherDetails.Manage = false; + cipherOrgDetails.UserId = null; + cipherOrgDetails.OrganizationId = organization.Id; organization.Type = organizationUserType; sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(new User { Id = userId }); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails); + sutProvider.GetDependency().GetOrganizationDetailsByIdAsync(cipherOrgDetails.Id).Returns(cipherOrgDetails); sutProvider.GetDependency() .GetManyByUserIdAsync(userId) .Returns(new List { - cipherDetails + new CipherDetails(cipherOrgDetails) { Edit = true, Manage = false } }); sutProvider.GetDependency() .GetOrganizationAbilityAsync(organization.Id) @@ -264,7 +263,7 @@ public class CiphersControllerTests LimitItemDeletion = true }); - await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteAdmin(cipherDetails.Id)); + await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteAdmin(cipherOrgDetails.Id)); await sutProvider.GetDependency().DidNotReceive().DeleteAsync(Arg.Any(), Arg.Any(), Arg.Any()); } @@ -273,21 +272,21 @@ public class CiphersControllerTests [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.Admin)] public async Task DeleteAdmin_WithOwnerOrAdmin_WithAccessToUnassignedCipher_DeletesCipher( - OrganizationUserType organizationUserType, CipherDetails cipherDetails, Guid userId, + OrganizationUserType organizationUserType, CipherOrganizationDetails cipherOrgDetails, Guid userId, CurrentContextOrganization organization, SutProvider sutProvider) { - cipherDetails.OrganizationId = organization.Id; + cipherOrgDetails.OrganizationId = organization.Id; organization.Type = organizationUserType; sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(new User { Id = userId }); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails); + sutProvider.GetDependency().GetOrganizationDetailsByIdAsync(cipherOrgDetails.Id).Returns(cipherOrgDetails); sutProvider.GetDependency() .GetManyUnassignedOrganizationDetailsByOrganizationIdAsync(organization.Id) .Returns(new List { - new() { Id = cipherDetails.Id, OrganizationId = cipherDetails.OrganizationId } + new() { Id = cipherOrgDetails.Id, OrganizationId = cipherOrgDetails.OrganizationId } }); sutProvider.GetDependency() .GetOrganizationAbilityAsync(organization.Id) @@ -297,54 +296,65 @@ public class CiphersControllerTests LimitItemDeletion = true }); - await sutProvider.Sut.DeleteAdmin(cipherDetails.Id); + await sutProvider.Sut.DeleteAdmin(cipherOrgDetails.Id); - await sutProvider.GetDependency().Received(1).DeleteAsync(cipherDetails, userId, true); + await sutProvider.GetDependency().Received(1).DeleteAsync(Arg.Is(c => c.Id == cipherOrgDetails.Id && c.OrganizationId == cipherOrgDetails.OrganizationId), + userId, + true); } [Theory] [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.Admin)] public async Task DeleteAdmin_WithAdminOrOwner_WithAccessToAllCollectionItems_DeletesCipher( - OrganizationUserType organizationUserType, CipherDetails cipherDetails, Guid userId, + OrganizationUserType organizationUserType, CipherOrganizationDetails cipherOrgDetails, Guid userId, CurrentContextOrganization organization, SutProvider sutProvider) { - cipherDetails.OrganizationId = organization.Id; + + organization.Type = organizationUserType; + + cipherOrgDetails.OrganizationId = organization.Id; organization.Type = organizationUserType; sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails); - sutProvider.GetDependency().GetManyByOrganizationIdAsync(organization.Id).Returns(new List { cipherDetails }); + sutProvider.GetDependency().GetOrganizationDetailsByIdAsync(cipherOrgDetails.Id).Returns(cipherOrgDetails); + sutProvider.GetDependency().GetManyByOrganizationIdAsync(organization.Id).Returns(new List { cipherOrgDetails }); sutProvider.GetDependency().GetOrganizationAbilityAsync(organization.Id).Returns(new OrganizationAbility { Id = organization.Id, AllowAdminAccessToAllCollectionItems = true }); - await sutProvider.Sut.DeleteAdmin(cipherDetails.Id); + await sutProvider.Sut.DeleteAdmin(cipherOrgDetails.Id); - await sutProvider.GetDependency().Received(1).DeleteAsync(cipherDetails, userId, true); + await sutProvider.GetDependency().Received(1).DeleteAsync( + Arg.Is(c => c.Id == cipherOrgDetails.Id && c.OrganizationId == cipherOrgDetails.OrganizationId), + userId, + true); } [Theory] [BitAutoData] public async Task DeleteAdmin_WithCustomUser_WithEditAnyCollectionTrue_DeletesCipher( - CipherDetails cipherDetails, Guid userId, + CipherOrganizationDetails cipherOrgDetails, Guid userId, CurrentContextOrganization organization, SutProvider sutProvider) { - cipherDetails.OrganizationId = organization.Id; + cipherOrgDetails.OrganizationId = organization.Id; organization.Type = OrganizationUserType.Custom; organization.Permissions.EditAnyCollection = true; sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails); - sutProvider.GetDependency().GetManyByOrganizationIdAsync(organization.Id).Returns(new List { cipherDetails }); + sutProvider.GetDependency().GetOrganizationDetailsByIdAsync(cipherOrgDetails.Id).Returns(cipherOrgDetails); + sutProvider.GetDependency().GetManyByOrganizationIdAsync(organization.Id).Returns(new List { cipherOrgDetails }); - await sutProvider.Sut.DeleteAdmin(cipherDetails.Id); + await sutProvider.Sut.DeleteAdmin(cipherOrgDetails.Id); - await sutProvider.GetDependency().Received(1).DeleteAsync(cipherDetails, userId, true); + await sutProvider.GetDependency().Received(1).DeleteAsync( + Arg.Is(c => c.Id == cipherOrgDetails.Id && c.OrganizationId == cipherOrgDetails.OrganizationId), + userId, + true); } [Theory]