From 4196f1c816ffd5fa895fc2ecc24254c5c340ddf9 Mon Sep 17 00:00:00 2001 From: maliming Date: Mon, 10 Aug 2026 13:35:12 +0800 Subject: [PATCH] Remove all related data when deleting a user - Clear password histories and passkeys, load the collections if they are missing - Delete sessions, user delegations and link users with UserDeletedEventHandler - Add OrganizationUnitManager.CreateManyAsync --- .../Volo/Abp/Identity/IdentityUserManager.cs | 23 +- .../Abp/Identity/OrganizationUnitManager.cs | 50 +++- .../Abp/Identity/UserDeletedEventHandler.cs | 52 +++++ .../IdentityUserManager_Delete_Tests.cs | 5 + .../Identity/OrganizationUnitManager_Tests.cs | 53 +++++ .../IdentityUserManager_Delete_Tests.cs | 8 + .../IdentityUserManager_Delete_Tests.cs | 217 ++++++++++++++++++ 7 files changed, 402 insertions(+), 6 deletions(-) create mode 100644 modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/UserDeletedEventHandler.cs create mode 100644 modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/IdentityUserManager_Delete_Tests.cs create mode 100644 modules/identity/test/Volo.Abp.Identity.MongoDB.Tests/Volo/Abp/Identity/MongoDB/IdentityUserManager_Delete_Tests.cs create mode 100644 modules/identity/test/Volo.Abp.Identity.TestBase/Volo/Abp/Identity/IdentityUserManager_Delete_Tests.cs diff --git a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs index ba11999b09..e136f71f73 100644 --- a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs +++ b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs @@ -98,13 +98,32 @@ public class IdentityUserManager : UserManager, IDomainService public async override Task DeleteAsync(IdentityUser user) { + //The user may have been loaded without details. + await UserRepository.EnsureCollectionLoadedAsync(user, x => x.Claims, CancellationToken); + await UserRepository.EnsureCollectionLoadedAsync(user, x => x.Roles, CancellationToken); + await UserRepository.EnsureCollectionLoadedAsync(user, x => x.Tokens, CancellationToken); + await UserRepository.EnsureCollectionLoadedAsync(user, x => x.Logins, CancellationToken); + await UserRepository.EnsureCollectionLoadedAsync(user, x => x.OrganizationUnits, CancellationToken); + await UserRepository.EnsureCollectionLoadedAsync(user, x => x.PasswordHistories, CancellationToken); + await UserRepository.EnsureCollectionLoadedAsync(user, x => x.Passkeys, CancellationToken); + user.Claims.Clear(); user.Roles.Clear(); user.Tokens.Clear(); user.Logins.Clear(); user.OrganizationUnits.Clear(); - await IdentityLinkUserRepository.DeleteAsync(new IdentityLinkUserInfo(user.Id, user.TenantId), CancellationToken); - await UpdateAsync(user); + user.PasswordHistories.Clear(); + user.Passkeys.Clear(); + + //UserDeletedEventHandler deletes them after the changes are saved, this keeps + //them gone for the rest of the current unit of work. They are in the host database. + using (CurrentTenant.Change(null)) + { + await IdentityLinkUserRepository.DeleteAsync(new IdentityLinkUserInfo(user.Id, user.TenantId), CancellationToken); + } + + //Soft deleting an entity reloads its original values. + (await UpdateAsync(user)).CheckErrors(); return await base.DeleteAsync(user); } diff --git a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/OrganizationUnitManager.cs b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/OrganizationUnitManager.cs index cb8fa17a77..efc78184a9 100644 --- a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/OrganizationUnitManager.cs +++ b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/OrganizationUnitManager.cs @@ -48,6 +48,42 @@ public class OrganizationUnitManager : DomainService await OrganizationUnitRepository.InsertAsync(organizationUnit); } + /// + /// Creates the given organization units by querying the siblings of a parent once instead of once + /// per organization unit. The parents must already exist. Custom validation should be added by + /// overriding . + /// + [UnitOfWork] + public virtual async Task CreateManyAsync(List organizationUnits) + { + Check.NotNull(organizationUnits, nameof(organizationUnits)); + + foreach (var group in organizationUnits.GroupBy(x => new { x.TenantId, x.ParentId })) + { + //Siblings, codes and the database of a group belong to its own tenant. + using (CurrentTenant.Change(group.Key.TenantId)) + { + await ValidateParentTenantAsync(group.Key.ParentId, group.Key.TenantId); + + var siblings = await FindChildrenAsync(group.Key.ParentId); + var lastCode = siblings.OrderBy(x => x.Code).LastOrDefault()?.Code; + + foreach (var organizationUnit in group) + { + await ValidateOrganizationUnitAsync(organizationUnit, siblings); + + organizationUnit.Code = lastCode = lastCode == null + ? await GetNextChildCodeAsync(group.Key.ParentId) + : OrganizationUnit.CalculateNextCode(lastCode); + + siblings.Add(organizationUnit); + } + + await OrganizationUnitRepository.InsertManyAsync(group.ToList()); + } + } + } + public virtual async Task UpdateAsync(OrganizationUnit organizationUnit) { await ValidateOrganizationUnitAsync(organizationUnit); @@ -141,15 +177,21 @@ public class OrganizationUnitManager : DomainService protected virtual async Task ValidateOrganizationUnitAsync(OrganizationUnit organizationUnit) { - var siblings = (await FindChildrenAsync(organizationUnit.ParentId)) - .Where(ou => ou.Id != organizationUnit.Id) - .ToList(); + await ValidateOrganizationUnitAsync(organizationUnit, await FindChildrenAsync(organizationUnit.ParentId)); + } - if (siblings.Any(ou => ou.DisplayName == organizationUnit.DisplayName)) + /// + /// Validates the organization unit against the given siblings, so they are not queried again. + /// + protected virtual Task ValidateOrganizationUnitAsync(OrganizationUnit organizationUnit, List siblings) + { + if (siblings.Any(ou => ou.Id != organizationUnit.Id && ou.DisplayName == organizationUnit.DisplayName)) { throw new BusinessException(IdentityErrorCodes.DuplicateOrganizationUnitDisplayName) .WithData("0", organizationUnit.DisplayName); } + + return Task.CompletedTask; } protected virtual async Task ValidateParentTenantAsync(Guid? parentId, Guid? tenantId) diff --git a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/UserDeletedEventHandler.cs b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/UserDeletedEventHandler.cs new file mode 100644 index 0000000000..42d1e30dc0 --- /dev/null +++ b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/UserDeletedEventHandler.cs @@ -0,0 +1,52 @@ +using System.Linq; +using System.Threading.Tasks; +using Volo.Abp.DependencyInjection; +using Volo.Abp.Domain.Entities.Events; +using Volo.Abp.EventBus; +using Volo.Abp.MultiTenancy; +using Volo.Abp.Uow; + +namespace Volo.Abp.Identity; + +//Sessions, user delegations and link users have no navigation from IdentityUser, +//so clearing the user's collections doesn't cover them. +public class UserDeletedEventHandler : + ILocalEventHandler>, + ITransientDependency +{ + protected IIdentitySessionRepository IdentitySessionRepository { get; } + protected IIdentityUserDelegationRepository IdentityUserDelegationRepository { get; } + protected IIdentityLinkUserRepository IdentityLinkUserRepository { get; } + protected ICurrentTenant CurrentTenant { get; } + + public UserDeletedEventHandler( + IIdentitySessionRepository identitySessionRepository, + IIdentityUserDelegationRepository identityUserDelegationRepository, + IIdentityLinkUserRepository identityLinkUserRepository, + ICurrentTenant currentTenant) + { + IdentitySessionRepository = identitySessionRepository; + IdentityUserDelegationRepository = identityUserDelegationRepository; + IdentityLinkUserRepository = identityLinkUserRepository; + CurrentTenant = currentTenant; + } + + [UnitOfWork] + public virtual async Task HandleEventAsync(EntityDeletedEventData eventData) + { + var user = eventData.Entity; + + await IdentitySessionRepository.DeleteAllAsync(user.Id); + + var delegations = await IdentityUserDelegationRepository.GetListAsync(sourceUserId: user.Id, targetUserId: null); + delegations.AddRange(await IdentityUserDelegationRepository.GetListAsync(sourceUserId: null, targetUserId: user.Id)); + //A delegation of the user to itself is returned by both queries. + await IdentityUserDelegationRepository.DeleteManyAsync(delegations.DistinctBy(x => x.Id).ToList()); + + //Link users are stored in the host database. + using (CurrentTenant.Change(null)) + { + await IdentityLinkUserRepository.DeleteAsync(new IdentityLinkUserInfo(user.Id, user.TenantId)); + } + } +} diff --git a/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/IdentityUserManager_Delete_Tests.cs b/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/IdentityUserManager_Delete_Tests.cs new file mode 100644 index 0000000000..d01350db95 --- /dev/null +++ b/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/IdentityUserManager_Delete_Tests.cs @@ -0,0 +1,5 @@ +namespace Volo.Abp.Identity; + +public class IdentityUserManager_Delete_Tests : IdentityUserManager_Delete_Tests +{ +} diff --git a/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs b/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs index 43c97a945a..0c0083c88d 100644 --- a/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs +++ b/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs @@ -1,6 +1,7 @@ using Microsoft.AspNetCore.Identity; using Shouldly; using System; +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Volo.Abp.Data; @@ -179,4 +180,56 @@ public class OrganizationUnitManager_Tests : AbpIdentityDomainTestBase } } } + + [Fact] + public async Task CreateManyAsync() + { + List organizationUnits; + + using (var uow = _unitOfWorkManager.Begin()) + { + var lastRootCode = (await _organizationUnitRepository.GetChildrenAsync(null)) + .OrderBy(x => x.Code).Last().Code; + + organizationUnits = Enumerable.Range(0, 5) + .Select(_ => new OrganizationUnit(_guidGenerator.Create(), $"batch-{Guid.NewGuid():N}")) + .ToList(); + + await _organizationUnitManager.CreateManyAsync(organizationUnits); + await uow.CompleteAsync(); + + foreach (var organizationUnit in organizationUnits) + { + lastRootCode = OrganizationUnit.CalculateNextCode(lastRootCode); + organizationUnit.Code.ShouldBe(lastRootCode); + } + } + + using (var uow = _unitOfWorkManager.Begin()) + { + foreach (var organizationUnit in organizationUnits) + { + (await _organizationUnitRepository.GetAsync(organizationUnit.Id)).Code.ShouldBe(organizationUnit.Code); + } + + await uow.CompleteAsync(); + } + } + + [Fact] + public async Task CreateManyAsync_Should_Not_Allow_Duplicate_Display_Name_In_The_Batch() + { + using (var uow = _unitOfWorkManager.Begin()) + { + var displayName = $"batch-duplicate-{Guid.NewGuid():N}"; + + await Should.ThrowAsync(async () => + await _organizationUnitManager.CreateManyAsync([ + new OrganizationUnit(_guidGenerator.Create(), displayName), + new OrganizationUnit(_guidGenerator.Create(), displayName) + ])); + + await uow.CompleteAsync(); + } + } } diff --git a/modules/identity/test/Volo.Abp.Identity.MongoDB.Tests/Volo/Abp/Identity/MongoDB/IdentityUserManager_Delete_Tests.cs b/modules/identity/test/Volo.Abp.Identity.MongoDB.Tests/Volo/Abp/Identity/MongoDB/IdentityUserManager_Delete_Tests.cs new file mode 100644 index 0000000000..415727d35c --- /dev/null +++ b/modules/identity/test/Volo.Abp.Identity.MongoDB.Tests/Volo/Abp/Identity/MongoDB/IdentityUserManager_Delete_Tests.cs @@ -0,0 +1,8 @@ +using Xunit; + +namespace Volo.Abp.Identity.MongoDB; + +[Collection(MongoTestCollection.Name)] +public class IdentityUserManager_Delete_Tests : IdentityUserManager_Delete_Tests +{ +} diff --git a/modules/identity/test/Volo.Abp.Identity.TestBase/Volo/Abp/Identity/IdentityUserManager_Delete_Tests.cs b/modules/identity/test/Volo.Abp.Identity.TestBase/Volo/Abp/Identity/IdentityUserManager_Delete_Tests.cs new file mode 100644 index 0000000000..73f7043db7 --- /dev/null +++ b/modules/identity/test/Volo.Abp.Identity.TestBase/Volo/Abp/Identity/IdentityUserManager_Delete_Tests.cs @@ -0,0 +1,217 @@ +using System; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; +using Shouldly; +using Volo.Abp.Data; +using Volo.Abp.Modularity; +using Volo.Abp.Uow; +using Xunit; + +namespace Volo.Abp.Identity; + +public abstract class IdentityUserManager_Delete_Tests : AbpIdentityTestBase + where TStartupModule : IAbpModule +{ + protected IdentityUserManager IdentityUserManager { get; } + protected IIdentityUserRepository IdentityUserRepository { get; } + protected IOrganizationUnitRepository OrganizationUnitRepository { get; } + protected IdentityLinkUserManager IdentityLinkUserManager { get; } + protected IIdentitySessionRepository IdentitySessionRepository { get; } + protected IIdentityUserDelegationRepository IdentityUserDelegationRepository { get; } + protected ILookupNormalizer LookupNormalizer { get; } + protected IUnitOfWorkManager UnitOfWorkManager { get; } + protected IDataFilter DataFilter { get; } + + protected IdentityUserManager_Delete_Tests() + { + IdentityUserManager = GetRequiredService(); + IdentityUserRepository = GetRequiredService(); + OrganizationUnitRepository = GetRequiredService(); + IdentityLinkUserManager = GetRequiredService(); + IdentitySessionRepository = GetRequiredService(); + IdentityUserDelegationRepository = GetRequiredService(); + LookupNormalizer = GetRequiredService(); + UnitOfWorkManager = GetRequiredService(); + DataFilter = GetRequiredService(); + } + + [Fact] + public virtual async Task DeleteAsync_Should_Remove_All_Related_Data() + { + var userId = Guid.NewGuid(); + var linkedUserId = Guid.NewGuid(); + + using (var uow = UnitOfWorkManager.Begin()) + { + (await IdentityUserManager.CreateAsync( + new IdentityUser(userId, $"delete-{userId:N}", $"delete-{userId:N}@abp.io"))).CheckErrors(); + (await IdentityUserManager.CreateAsync( + new IdentityUser(linkedUserId, $"linked-{linkedUserId:N}", $"linked-{linkedUserId:N}@abp.io"))).CheckErrors(); + + await uow.CompleteAsync(); + } + + using (var uow = UnitOfWorkManager.Begin()) + { + var user = await IdentityUserManager.GetByIdAsync(userId); + + await IdentityUserManager.AddClaimAsync(user, new Claim("test", "test")); + await IdentityUserManager.AddLoginAsync(user, new UserLoginInfo("test", "test", "test")); + await IdentityUserManager.AddToRoleAsync(user, "moderator"); + user.SetToken("test", "test", "test"); + user.AddPasswordHistory("test"); + user.AddPasskey([1, 2, 3], new IdentityPasskeyData()); + await IdentityUserManager.AddToOrganizationUnitAsync( + user, + await OrganizationUnitRepository.GetAsync(LookupNormalizer.NormalizeName("OU11"))); + await IdentityLinkUserManager.LinkAsync( + new IdentityLinkUserInfo(userId), + new IdentityLinkUserInfo(linkedUserId)); + + await IdentitySessionRepository.InsertAsync(new IdentitySession( + Guid.NewGuid(), $"session-{userId:N}", "Web", "Chrome", userId, null, "MyApp", "127.0.0.1", DateTime.UtcNow)); + await IdentityUserDelegationRepository.InsertAsync(new IdentityUserDelegation( + Guid.NewGuid(), userId, linkedUserId, DateTime.UtcNow, DateTime.UtcNow.AddDays(1))); + await IdentityUserDelegationRepository.InsertAsync(new IdentityUserDelegation( + Guid.NewGuid(), linkedUserId, userId, DateTime.UtcNow, DateTime.UtcNow.AddDays(1))); + + await uow.CompleteAsync(); + } + + using (var uow = UnitOfWorkManager.Begin()) + { + var user = await IdentityUserManager.GetByIdAsync(userId); + + user.Claims.Count.ShouldBeGreaterThan(0); + user.Logins.Count.ShouldBeGreaterThan(0); + user.Roles.Count.ShouldBeGreaterThan(0); + user.Tokens.Count.ShouldBeGreaterThan(0); + user.OrganizationUnits.Count.ShouldBeGreaterThan(0); + user.PasswordHistories.Count.ShouldBeGreaterThan(0); + user.Passkeys.Count.ShouldBeGreaterThan(0); + + (await IdentityUserManager.DeleteAsync(user)).CheckErrors(); + + await uow.CompleteAsync(); + } + + //The user is soft deleted, disable the filter to see what is left behind. + using (var uow = UnitOfWorkManager.Begin()) + using (DataFilter.Disable()) + { + var deletedUser = await IdentityUserRepository.FindAsync(userId); + deletedUser.ShouldNotBeNull(); + + deletedUser.Claims.Count.ShouldBe(0); + deletedUser.Logins.Count.ShouldBe(0); + deletedUser.Roles.Count.ShouldBe(0); + deletedUser.Tokens.Count.ShouldBe(0); + deletedUser.OrganizationUnits.Count.ShouldBe(0); + deletedUser.PasswordHistories.Count.ShouldBe(0); + deletedUser.Passkeys.Count.ShouldBe(0); + + (await IdentityLinkUserManager.IsLinkedAsync( + new IdentityLinkUserInfo(userId), + new IdentityLinkUserInfo(linkedUserId))).ShouldBeFalse(); + + (await IdentitySessionRepository.GetCountAsync(userId: userId)).ShouldBe(0); + (await IdentityUserDelegationRepository.GetListAsync(sourceUserId: userId, targetUserId: null)).ShouldBeEmpty(); + (await IdentityUserDelegationRepository.GetListAsync(sourceUserId: null, targetUserId: userId)).ShouldBeEmpty(); + + await uow.CompleteAsync(); + } + } + + [Fact] + public virtual async Task DeleteAsync_Should_Remove_Related_Data_Of_A_User_Loaded_Without_Details() + { + var userId = Guid.NewGuid(); + var userName = $"no-details-{userId:N}"; + + using (var uow = UnitOfWorkManager.Begin()) + { + (await IdentityUserManager.CreateAsync( + new IdentityUser(userId, userName, $"{userName}@abp.io"))).CheckErrors(); + + await uow.CompleteAsync(); + } + + using (var uow = UnitOfWorkManager.Begin()) + { + var user = await IdentityUserManager.GetByIdAsync(userId); + await IdentityUserManager.AddToRoleAsync(user, "moderator"); + await uow.CompleteAsync(); + } + + using (var uow = UnitOfWorkManager.Begin()) + { + var user = await IdentityUserRepository.FindByNormalizedUserNameAsync( + LookupNormalizer.NormalizeName(userName), + includeDetails: false); + + (await IdentityUserManager.DeleteAsync(user)).CheckErrors(); + + await uow.CompleteAsync(); + } + + using (var uow = UnitOfWorkManager.Begin()) + using (DataFilter.Disable()) + { + var deletedUser = await IdentityUserRepository.FindAsync(userId); + deletedUser.ShouldNotBeNull(); + deletedUser.Roles.Count.ShouldBe(0); + + await uow.CompleteAsync(); + } + } + + [Fact] + public virtual async Task Deleting_A_User_Through_The_Repository_Should_Remove_Sessions_Delegations_And_Links() + { + var userId = Guid.NewGuid(); + var linkedUserId = Guid.NewGuid(); + + using (var uow = UnitOfWorkManager.Begin()) + { + (await IdentityUserManager.CreateAsync( + new IdentityUser(userId, $"repo-{userId:N}", $"repo-{userId:N}@abp.io"))).CheckErrors(); + (await IdentityUserManager.CreateAsync( + new IdentityUser(linkedUserId, $"repo-linked-{linkedUserId:N}", $"repo-linked-{linkedUserId:N}@abp.io"))).CheckErrors(); + + await uow.CompleteAsync(); + } + + using (var uow = UnitOfWorkManager.Begin()) + { + await IdentityLinkUserManager.LinkAsync( + new IdentityLinkUserInfo(userId), + new IdentityLinkUserInfo(linkedUserId)); + + await IdentitySessionRepository.InsertAsync(new IdentitySession( + Guid.NewGuid(), $"repo-session-{userId:N}", "Web", "Chrome", userId, null, "MyApp", "127.0.0.1", DateTime.UtcNow)); + await IdentityUserDelegationRepository.InsertAsync(new IdentityUserDelegation( + Guid.NewGuid(), userId, linkedUserId, DateTime.UtcNow, DateTime.UtcNow.AddDays(1))); + + await uow.CompleteAsync(); + } + + //Custom code may delete the user without using IdentityUserManager. + using (var uow = UnitOfWorkManager.Begin()) + { + await IdentityUserRepository.DeleteAsync(await IdentityUserRepository.GetAsync(userId)); + await uow.CompleteAsync(); + } + + using (var uow = UnitOfWorkManager.Begin()) + { + (await IdentitySessionRepository.GetCountAsync(userId: userId)).ShouldBe(0); + (await IdentityUserDelegationRepository.GetListAsync(sourceUserId: userId, targetUserId: null)).ShouldBeEmpty(); + (await IdentityLinkUserManager.IsLinkedAsync( + new IdentityLinkUserInfo(userId), + new IdentityLinkUserInfo(linkedUserId))).ShouldBeFalse(); + + await uow.CompleteAsync(); + } + } +}