From d8aca78baeecaecbd747388f23de8fc71e94c820 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:54:38 -0700 Subject: [PATCH 1/2] add claimed domain check to token redemption path --- .../Auth/Controllers/AccountsController.cs | 7 -- .../Services/Implementations/UserService.cs | 6 ++ .../Controllers/AccountsControllerTests.cs | 70 +++++++++++++- test/Core.Test/Services/UserServiceTests.cs | 96 +++++++++++++++++++ 4 files changed, 170 insertions(+), 9 deletions(-) diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 010dcd644ebe..44f035ca10af 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -7,7 +7,6 @@ using Bit.Api.Models.Response; using Bit.Core; using Bit.Core.AdminConsole.Enums.Provider; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Repositories; @@ -583,12 +582,6 @@ public async Task Delete([FromBody] SecretVerificationRequestModel model) } else { - // Check if the user is claimed by any organization. - if (await _userService.IsClaimedByAnyOrganizationAsync(user.Id)) - { - throw new BadRequestException(new CannotDeleteClaimedAccountError().Message); - } - var result = await _userService.DeleteAsync(user); if (result.Succeeded) { diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 288288ccef9a..830227f80b13 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -5,6 +5,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccount; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; @@ -222,6 +223,11 @@ public async Task SaveUserAsync(User user, bool push = false) public override async Task DeleteAsync(User user) { + if (await IsClaimedByAnyOrganizationAsync(user.Id)) + { + throw new BadRequestException(new CannotDeleteClaimedAccountError().Message); + } + // Check if user is the only owner of any organizations. var onlyOwnerCount = await _organizationUserRepository.GetCountByOnlyOwnerAsync(user.Id); if (onlyOwnerCount > 0) diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index 095c605731bf..1f30dff60b65 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -714,7 +714,8 @@ public async Task Delete_WithUserManagedByAnOrganization_ThrowsBadRequestExcepti var user = GenerateExampleUser(); ConfigureUserServiceToReturnValidPrincipalFor(user); ConfigureUserServiceToAcceptPasswordFor(user); - _userService.IsClaimedByAnyOrganizationAsync(user.Id).Returns(true); + _userService.DeleteAsync(user) + .ThrowsAsync(new BadRequestException(new CannotDeleteClaimedAccountError().Message)); var result = await Assert.ThrowsAsync(() => _sut.Delete(new SecretVerificationRequestModel())); @@ -727,7 +728,6 @@ public async Task Delete_WithUserNotManagedByAnOrganization_ShouldSucceed() var user = GenerateExampleUser(); ConfigureUserServiceToReturnValidPrincipalFor(user); ConfigureUserServiceToAcceptPasswordFor(user); - _userService.IsClaimedByAnyOrganizationAsync(user.Id).Returns(false); _userService.DeleteAsync(user).Returns(IdentityResult.Success); await _sut.Delete(new SecretVerificationRequestModel()); @@ -735,6 +735,72 @@ public async Task Delete_WithUserNotManagedByAnOrganization_ShouldSucceed() await _userService.Received(1).DeleteAsync(user); } + [Fact] + public async Task PostDeleteRecoverToken_WhenUserDoesNotExist_ShouldThrowUnauthorizedAccessException() + { + ConfigureUserServiceToReturnNullUserId(); + + await Assert.ThrowsAsync( + () => _sut.PostDeleteRecoverToken(new VerifyDeleteRecoverRequestModel + { + UserId = Guid.NewGuid().ToString(), + Token = "token" + }) + ); + } + + [Fact] + public async Task PostDeleteRecoverToken_WithValidToken_ShouldDeleteAccount() + { + var user = GenerateExampleUser(); + ConfigureUserServiceToReturnValidIdFor(user); + _userService.DeleteAsync(user, "token").Returns(Task.FromResult(IdentityResult.Success)); + + await _sut.PostDeleteRecoverToken(new VerifyDeleteRecoverRequestModel + { + UserId = Guid.NewGuid().ToString(), + Token = "token" + }); + + await _userService.Received(1).DeleteAsync(user, "token"); + } + + [Fact] + public async Task PostDeleteRecoverToken_WithInvalidToken_ShouldThrowBadRequestException() + { + var user = GenerateExampleUser(); + ConfigureUserServiceToReturnValidIdFor(user); + _userService.DeleteAsync(user, "token") + .Returns(Task.FromResult(IdentityResult.Failed(new IdentityError { Description = "Invalid token." }))); + + await Assert.ThrowsAsync( + () => _sut.PostDeleteRecoverToken(new VerifyDeleteRecoverRequestModel + { + UserId = Guid.NewGuid().ToString(), + Token = "token" + }) + ); + } + + [Fact] + public async Task PostDeleteRecoverToken_WithClaimedAccount_ThrowsBadRequestException() + { + var user = GenerateExampleUser(); + ConfigureUserServiceToReturnValidIdFor(user); + _userService.DeleteAsync(user, "token") + .ThrowsAsync(new BadRequestException(new CannotDeleteClaimedAccountError().Message)); + + var exception = await Assert.ThrowsAsync( + () => _sut.PostDeleteRecoverToken(new VerifyDeleteRecoverRequestModel + { + UserId = Guid.NewGuid().ToString(), + Token = "token" + }) + ); + + Assert.Equal(new CannotDeleteClaimedAccountError().Message, exception.Message); + } + [Theory] [BitAutoData] public async Task SetVerifyDevices_WhenUserDoesNotExist_ShouldThrowUnauthorizedAccessException( diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 8d63eb0c3971..a121978279d3 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -3,6 +3,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; @@ -542,6 +543,28 @@ public async Task AdminResetPasswordAsync_EmptyOrWhitespaceResetPasswordKey_Thro Assert.Equal("Organization User not valid", exception.Message); } + [Theory, BitAutoData] + public async Task DeleteAsync_WithClaimedAccount_ThrowsBadRequestException( + User user, + Organization organization, + SutProvider sutProvider) + { + organization.Enabled = true; + organization.UseOrganizationDomains = true; + + sutProvider.GetDependency() + .GetByVerifiedUserEmailDomainAsync(user.Id) + .Returns([organization]); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteAsync(user)); + + Assert.Equal(new CannotDeleteClaimedAccountError().Message, exception.Message); + await sutProvider.GetDependency() + .DidNotReceive().GetCountByOnlyOwnerAsync(user.Id); + await sutProvider.GetDependency().DidNotReceive().DeleteAsync(user); + } + [Theory, BitAutoData] public async Task DeleteAsync_WithGatewaySubscription_CallsSubscriberService( User user, @@ -549,6 +572,10 @@ public async Task DeleteAsync_WithGatewaySubscription_CallsSubscriberService( { user.GatewaySubscriptionId = "sub_test"; + sutProvider.GetDependency() + .GetByVerifiedUserEmailDomainAsync(user.Id) + .Returns([]); + sutProvider.GetDependency() .GetCountByOnlyOwnerAsync(user.Id) .Returns(0); @@ -580,6 +607,10 @@ public async Task DeleteAsync_WithFileSends_DeletesFilesBeforeDbRecords( // 3. File blob still exists but with no parent Send user.GatewaySubscriptionId = null; + sutProvider.GetDependency() + .GetByVerifiedUserEmailDomainAsync(user.Id) + .Returns([]); + sutProvider.GetDependency() .GetCountByOnlyOwnerAsync(user.Id) .Returns(0); @@ -606,6 +637,66 @@ await sutProvider.GetDependency() Assert.Equal(new[] { "file", "db" }, callOrder); } + [Theory, BitAutoData] + public async Task DeleteAsync_WithTokenAndInvalidToken_ReturnsFailedResult(User user) + { + var sutProvider = new SutProvider() + .CreateWithUserServiceCustomizations(user); + + var result = await sutProvider.Sut.DeleteAsync(user, "not_the_right_token"); + + Assert.False(result.Succeeded); + await sutProvider.GetDependency() + .DidNotReceive().GetByVerifiedUserEmailDomainAsync(Arg.Any()); + await sutProvider.GetDependency().DidNotReceive().DeleteAsync(user); + } + + [Theory, BitAutoData] + public async Task DeleteAsync_WithTokenAndClaimedAccount_ThrowsBadRequestException( + User user, Organization organization) + { + organization.Enabled = true; + organization.UseOrganizationDomains = true; + + var sutProvider = new SutProvider() + .CreateWithUserServiceCustomizations(user); + + sutProvider.GetDependency() + .GetByVerifiedUserEmailDomainAsync(user.Id) + .Returns([organization]); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteAsync(user, "otp_token")); + + Assert.Equal(new CannotDeleteClaimedAccountError().Message, exception.Message); + await sutProvider.GetDependency().DidNotReceive().DeleteAsync(user); + } + + [Theory, BitAutoData] + public async Task DeleteAsync_WithTokenAndUnclaimedAccount_DeletesUser(User user) + { + user.GatewaySubscriptionId = null; + + var sutProvider = new SutProvider() + .CreateWithUserServiceCustomizations(user); + + sutProvider.GetDependency() + .GetByVerifiedUserEmailDomainAsync(user.Id) + .Returns([]); + + sutProvider.GetDependency() + .GetCountByOnlyOwnerAsync(user.Id) + .Returns(0); + sutProvider.GetDependency() + .GetCountByOnlyOwnerAsync(user.Id) + .Returns(0); + + var result = await sutProvider.Sut.DeleteAsync(user, "otp_token"); + + Assert.True(result.Succeeded); + await sutProvider.GetDependency().Received(1).DeleteAsync(user); + } + // PM-37165: locks in the legacy path's non-write of LastApiKeyRotationDate. Once the // PM37165_RotateUserApiKeyCommand flag is cleaned up and this method is deleted, this // test goes with it. @@ -656,6 +747,11 @@ private static SutProvider SetFakeTokenProvider(this SutProvider() { ["Email"] = new TokenProviderDescriptor(typeof(IUserTwoFactorTokenProvider)) + { + ProviderInstance = fakeUserTwoFactorProvider, + }, + // Used by TokenOptions.DefaultProvider (e.g. the delete-account recovery token). + [TokenOptions.DefaultProvider] = new TokenProviderDescriptor(typeof(IUserTwoFactorTokenProvider)) { ProviderInstance = fakeUserTwoFactorProvider, } From 767976e7a19dc2d2854d3a7ca25a714d73e56845 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Wed, 26 Aug 2026 12:21:47 -0700 Subject: [PATCH 2/2] update comment for clarity --- test/Core.Test/Services/UserServiceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index a121978279d3..4c56aab90882 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -750,7 +750,7 @@ private static SutProvider SetFakeTokenProvider(this SutProvider)) { ProviderInstance = fakeUserTwoFactorProvider,