Carry the tenant id in the GDPR events

- Delete the link users after the user is saved, a concurrency failure kept them deleted
- Create organization units of the current tenant only and insert them after every group is valid

Author: maliming

framework/src/Volo.Abp.Gdpr.Abstractions/Volo.Abp.Gdpr.Abstractions.csproj

@@ -12,6 +12,7 @@
 
     <ItemGroup>
         <ProjectReference Include="..\Volo.Abp.Core\Volo.Abp.Core.csproj" />
+        <ProjectReference Include="..\Volo.Abp.EventBus.Abstractions\Volo.Abp.EventBus.Abstractions.csproj" />
     </ItemGroup>
 
 </Project>

framework/src/Volo.Abp.Gdpr.Abstractions/Volo/Abp/Gdpr/AbpGdprAbstractionsModule.cs

@@ -1,7 +1,11 @@
 using Volo.Abp.Modularity;
+using Volo.Abp.EventBus.Abstractions;
 
 namespace Volo.Abp.Gdpr;
 
+[DependsOn(
+    typeof(AbpEventBusAbstractionsModule)
+)]
 public class AbpGdprAbstractionsModule : AbpModule
 {
 }

framework/src/Volo.Abp.Gdpr.Abstractions/Volo/Abp/Gdpr/GdprUserDataDeletionRequestedEto.cs

@@ -1,9 +1,18 @@
 using System;
+using Volo.Abp.EventBus;
 
 namespace Volo.Abp.Gdpr;
 
 [Serializable]
-public class GdprUserDataDeletionRequestedEto
+public class GdprUserDataDeletionRequestedEto : IEventDataMayHaveTenantId
 {
+    public Guid? TenantId { get; set; }
+
     public Guid UserId { get; set; }
-}
+
+    public bool IsMultiTenant(out Guid? tenantId)
+    {
+        tenantId = TenantId;
+        return TenantId.HasValue;
+    }
+}

framework/src/Volo.Abp.Gdpr.Abstractions/Volo/Abp/Gdpr/GdprUserDataPreparedEto.cs

@@ -1,13 +1,22 @@
 using System;
+using Volo.Abp.EventBus;
 
 namespace Volo.Abp.Gdpr;
 
 [Serializable]
-public class GdprUserDataPreparedEto
+public class GdprUserDataPreparedEto : IEventDataMayHaveTenantId
 {
+    public Guid? TenantId { get; set; }
+
     public Guid RequestId { get; set; }
 
     public string Provider { get; set; } = default!;
 
     public GdprDataInfo Data { get; set; } = default!;
-}
+
+    public bool IsMultiTenant(out Guid? tenantId)
+    {
+        tenantId = TenantId;
+        return TenantId.HasValue;
+    }
+}

framework/src/Volo.Abp.Gdpr.Abstractions/Volo/Abp/Gdpr/GdprUserDataRequestedEto.cs

@@ -1,11 +1,20 @@
 using System;
+using Volo.Abp.EventBus;
 
 namespace Volo.Abp.Gdpr;
 
 [Serializable]
-public class GdprUserDataRequestedEto
+public class GdprUserDataRequestedEto : IEventDataMayHaveTenantId
 {
+    public Guid? TenantId { get; set; }
+
     public Guid UserId { get; set; }
 
     public Guid RequestId { get; set; }
-}
+
+    public bool IsMultiTenant(out Guid? tenantId)
+    {
+        tenantId = TenantId;
+        return TenantId.HasValue;
+    }
+}

modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs

@@ -115,15 +115,16 @@ public async override Task<IdentityResult> DeleteAsync(IdentityUser user)
         user.PasswordHistories.Clear();
         user.Passkeys.Clear();
 
+        //Soft deleting reloads the original values, the store saves the changes without validating the user.
+        //Nothing else is deleted before this succeeds, it is where the user is checked for concurrency.
+        (await Store.UpdateAsync(user, CancellationToken)).CheckErrors();
+
         //They are in the host database and deleting them here covers the current unit of work.
         using (CurrentTenant.Change(null))
         {
             await IdentityLinkUserRepository.DeleteAsync(new IdentityLinkUserInfo(user.Id, user.TenantId), CancellationToken);
         }
 
-        //Soft deleting reloads the original values, the store saves the changes without validating the user.
-        (await Store.UpdateAsync(user, CancellationToken)).CheckErrors();
-
         return await base.DeleteAsync(user);
     }
 

modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/OrganizationUnitManager.cs

@@ -50,38 +50,47 @@ public virtual async Task CreateAsync(OrganizationUnit organizationUnit)
 
     /// <summary>
     /// 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 <see cref="ValidateOrganizationUnitAsync(OrganizationUnit, List{OrganizationUnit})"/>.
+    /// per organization unit. They all have to belong to the current tenant and their parents must
+    /// already exist. <see cref="GetNextChildCodeAsync"/> is used for the first organization unit of a
+    /// parent, the codes of the rest follow it. Custom validation should be added by overriding
+    /// <see cref="ValidateOrganizationUnitAsync(OrganizationUnit, List{OrganizationUnit})"/>.
     /// </summary>
     [UnitOfWork]
     public virtual async Task CreateManyAsync(List<OrganizationUnit> organizationUnits)
     {
         Check.NotNull(organizationUnits, nameof(organizationUnits));
 
-        foreach (var group in organizationUnits.GroupBy(x => new { x.TenantId, x.ParentId }))
+        if (organizationUnits.Any(x => x.TenantId != CurrentTenant.Id))
         {
-            //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);
+            throw new AbpException("Organization units of another tenant can not be created, change the current tenant instead!");
+        }
 
-                var siblings = await FindChildrenAsync(group.Key.ParentId);
-                var lastCode = siblings.OrderBy(x => x.Code).LastOrDefault()?.Code;
+        var groups = organizationUnits.GroupBy(x => x.ParentId).ToList();
+
+        foreach (var group in groups)
+        {
+            await ValidateParentTenantAsync(group.Key, CurrentTenant.Id);
 
-                foreach (var organizationUnit in group)
-                {
-                    await ValidateOrganizationUnitAsync(organizationUnit, siblings);
+            var siblings = await FindChildrenAsync(group.Key);
+            string lastCode = null;
 
-                    organizationUnit.Code = lastCode = lastCode == null
-                        ? await GetNextChildCodeAsync(group.Key.ParentId)
-                        : OrganizationUnit.CalculateNextCode(lastCode);
+            foreach (var organizationUnit in group)
+            {
+                organizationUnit.Code = lastCode = lastCode == null
+                    ? await GetNextChildCodeAsync(group.Key)
+                    : OrganizationUnit.CalculateNextCode(lastCode);
 
-                    siblings.Add(organizationUnit);
-                }
+                await ValidateOrganizationUnitAsync(organizationUnit, siblings);
 
-                await OrganizationUnitRepository.InsertManyAsync(group.ToList());
+                siblings.Add(organizationUnit);
             }
         }
+
+        //Nothing is inserted before every group is validated.
+        foreach (var group in groups)
+        {
+            await OrganizationUnitRepository.InsertManyAsync(group.ToList());
+        }
     }
 
     public virtual async Task UpdateAsync(OrganizationUnit organizationUnit)

modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_CreateMany_Tests.cs

@@ -0,0 +1,5 @@
+namespace Volo.Abp.Identity;
+
+public class OrganizationUnitManager_CreateMany_Tests : OrganizationUnitManager_CreateMany_Tests<AbpIdentityDomainTestModule>
+{
+}

modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs

@@ -5,6 +5,12 @@
 using System.Linq;
 using System.Threading.Tasks;
 using Volo.Abp.Data;
+using Volo.Abp.DependencyInjection;
+using Volo.Abp.Threading;
+using Volo.Abp.Security.Claims;
+using Volo.Abp.Identity.Localization;
+using Volo.Abp.Caching;
+using Microsoft.Extensions.Localization;
 using Volo.Abp.Guids;
 using Volo.Abp.MultiTenancy;
 using Volo.Abp.Uow;
@@ -232,4 +238,84 @@ await _organizationUnitManager.CreateManyAsync([
             await uow.CompleteAsync();
         }
     }
+
+    [Fact]
+    public async Task CreateManyAsync_Should_Not_Create_Organization_Units_Of_Another_Tenant()
+    {
+        using (var uow = _unitOfWorkManager.Begin())
+        {
+            await Should.ThrowAsync<AbpException>(async () =>
+                await _organizationUnitManager.CreateManyAsync([
+                    new OrganizationUnit(_guidGenerator.Create(), "another-tenant", null, Guid.NewGuid())
+                ]));
+
+            await uow.CompleteAsync();
+        }
+    }
+
+    [Fact]
+    public async Task CreateManyAsync_Should_Use_The_Overridden_Extension_Points()
+    {
+        var manager = new TestOrganizationUnitManager(
+            _organizationUnitRepository,
+            GetRequiredService<IStringLocalizer<IdentityResource>>(),
+            _identityRoleRepository,
+            GetRequiredService<IDistributedCache<AbpDynamicClaimCacheItem>>(),
+            GetRequiredService<ICancellationTokenProvider>())
+        {
+            LazyServiceProvider = GetRequiredService<IAbpLazyServiceProvider>()
+        };
+
+        using (var uow = _unitOfWorkManager.Begin())
+        {
+            await Should.ThrowAsync<BusinessException>(async () =>
+                await manager.CreateManyAsync([new OrganizationUnit(_guidGenerator.Create(), "rejected-by-the-override")]));
+
+            await manager.CreateManyAsync([
+                new OrganizationUnit(_guidGenerator.Create(), $"extension-point-1-{Guid.NewGuid():N}"),
+                new OrganizationUnit(_guidGenerator.Create(), $"extension-point-2-{Guid.NewGuid():N}")
+            ]);
+
+            await uow.CompleteAsync();
+        }
+
+        //Every organization unit is validated, the code generator is only used for the first one of a parent.
+        //Both calls above created a root organization unit, so the code generator was used twice.
+        manager.ValidateCallCount.ShouldBe(3);
+        manager.GetNextChildCodeCallCount.ShouldBe(2);
+    }
+
+    public class TestOrganizationUnitManager : OrganizationUnitManager
+    {
+        public int ValidateCallCount { get; private set; }
+        public int GetNextChildCodeCallCount { get; private set; }
+
+        public TestOrganizationUnitManager(
+            IOrganizationUnitRepository organizationUnitRepository,
+            IStringLocalizer<IdentityResource> localizer,
+            IIdentityRoleRepository identityRoleRepository,
+            IDistributedCache<AbpDynamicClaimCacheItem> dynamicClaimCache,
+            ICancellationTokenProvider cancellationTokenProvider)
+            : base(organizationUnitRepository, localizer, identityRoleRepository, dynamicClaimCache, cancellationTokenProvider)
+        {
+        }
+
+        public override async Task<string> GetNextChildCodeAsync(Guid? parentId)
+        {
+            GetNextChildCodeCallCount++;
+            return await base.GetNextChildCodeAsync(parentId);
+        }
+
+        protected override async Task ValidateOrganizationUnitAsync(OrganizationUnit organizationUnit, List<OrganizationUnit> siblings)
+        {
+            ValidateCallCount++;
+
+            if (organizationUnit.DisplayName == "rejected-by-the-override")
+            {
+                throw new BusinessException(IdentityErrorCodes.DuplicateOrganizationUnitDisplayName);
+            }
+
+            await base.ValidateOrganizationUnitAsync(organizationUnit, siblings);
+        }
+    }
 }

modules/identity/test/Volo.Abp.Identity.MongoDB.Tests/Volo/Abp/Identity/MongoDB/OrganizationUnitManager_CreateMany_Tests.cs

@@ -0,0 +1,8 @@
+using Xunit;
+
+namespace Volo.Abp.Identity.MongoDB;
+
+[Collection(MongoTestCollection.Name)]
+public class OrganizationUnitManager_CreateMany_Tests : OrganizationUnitManager_CreateMany_Tests<AbpIdentityMongoDbTestModule>
+{
+}