dentityUserAppService.UpdateAsync wipes a user's Organization Unit membership when only updating roles (OrganizationUnitIds left null) #10879

2 Answer(s)

• 

  0

  AI-Bot created 20 days ago

  Support Team ABP Agent

  • This is a known bug in the IdentityUserAppService update flow (as you diagnosed): UpdateUserRolesBasedOnOrganizationUnits(...) mutates input.OrganizationUnitIds (??=), which unintentionally turns “not provided” (null) into “provided but empty” ([]). Since the caller also has ManageOU, the later block runs and calls SetOrganizationUnitsAsync(user, []), clearing all OU memberships.

  • Workaround (no framework modification): override UpdateAsync and preserve the original intent (null)

    1. Create a class that derives from IdentityUserAppService.
    2. Capture input.OrganizationUnitIds into a local variable before calling base.
    3. After base call, if the caller did not intend to update OUs (original was null), restore OU memberships (re-apply existing memberships), or run a custom “roles-only” update path.

    Example approach (restore memberships when the input didn’t specify OUs):

  using Volo.Abp.Identity;
    using Volo.Abp.Application.Dtos;
  
    public class MyIdentityUserAppService : IdentityUserAppService
    {
        public MyIdentityUserAppService(
            IdentityUserManager userManager,
            IIdentityUserRepository userRepository,
            IIdentityRoleRepository roleRepository,
            IIdentityClaimTypeRepository claimTypeRepository,
            IOrganizationUnitRepository organizationUnitRepository)
            : base(userManager, userRepository, roleRepository, claimTypeRepository, organizationUnitRepository)
        {
        }
  
        public override async Task<IdentityUserDto> UpdateAsync(Guid id, IdentityUserUpdateDto input)
        {
            // Preserve original caller intent
            var originalOuIds = input.OrganizationUnitIds;
  
            // If caller didn't send OUs, fetch current memberships so we can restore them
            Guid[]? currentOuIds = null;
            if (originalOuIds == null)
            {
                var user = await UserManager.GetByIdAsync(id);
                var ous = await UserManager.GetOrganizationUnitsAsync(user);
                currentOuIds = ous.Select(x => x.Id).ToArray();
            }
  
            var result = await base.UpdateAsync(id, input);
  
            // Restore memberships only when caller didn't touch OUs
            if (originalOuIds == null && currentOuIds != null)
            {
                var user = await UserManager.GetByIdAsync(id);
                await UserManager.SetOrganizationUnitsAsync(user, currentOuIds);
            }
  
            return result;
        }
    }
  

  Register your derived service as a replacement (ABP standard DI replacement pattern).

  • Proper fix (framework side): exactly what you suggested—UpdateUserRolesBasedOnOrganizationUnits should use a local variable instead of mutating input.OrganizationUnitIds. Until that’s released in the framework, the override workaround above prevents OU loss.

  • If you also control the UI/API contract, a simpler mitigation is: always send OrganizationUnitIds explicitly when the current user has ManageOU (e.g., send the existing OU ids back), but your “roles tab” scenario is valid, so the service-side workaround is usually preferable.

  (Verified docs related to Organization Units, for context only:)

  • https://abp.io/docs/10.6/modules/identity
  • https://abp.io/docs/latest/modules/identity-pro

  ---

  This response was generated by AI. If it didn’t solve your problem, feel free to post a new answer — our support team will jump in to help you soon.

  Markdown supported.

  Copy, paste, or drag & drop images and files (max 100 MB per file, 100 MB total per post)

  Save Cancel
• 

  0

  maliming created 20 days ago

  Support Team Fullstack Developer

  Hi,

  That's a bug on our side. UpdateUserRolesBasedOnOrganizationUnits shouldn't write back to input, and we're changing it to a local variable exactly the way you suggested.

  Until that ships, you can override the app service and put the caller's original value back after the base call:

  using System.Threading.Tasks;
  using Microsoft.AspNetCore.Identity;
  using Microsoft.Extensions.Options;
  using Volo.Abp.Authorization.Permissions;
  using Volo.Abp.Caching;
  using Volo.Abp.DependencyInjection;
  using Volo.Abp.EventBus.Distributed;
  using Volo.Abp.Identity;
  using Volo.Abp.Identity.Emailing;
  using Volo.Abp.Identity.UserInvitations;
  using Volo.Abp.Identity.UserSharing;
  using Volo.Abp.Threading;
  
  namespace MyCompanyName.MyProjectName.Identity;
  
  [Dependency(ReplaceServices = true)]
  [ExposeServices(typeof(IIdentityUserAppService))]
  public class MyIdentityUserAppService : IdentityUserAppService
  {
      public MyIdentityUserAppService(
          IdentityUserManager userManager,
          IIdentityUserRepository userRepository,
          IIdentityRoleRepository roleRepository,
          IOrganizationUnitRepository organizationUnitRepository,
          IIdentityClaimTypeRepository identityClaimTypeRepository,
          IdentityProTwoFactorManager identityProTwoFactorManager,
          IOptions<IdentityOptions> identityOptions,
          IDistributedEventBus distributedEventBus,
          IOptions<AbpIdentityOptions> abpIdentityOptions,
          IPermissionChecker permissionChecker,
          IDistributedCache<IdentityUserDownloadTokenCacheItem, string> downloadTokenCache,
          IDistributedCache<ImportInvalidUsersCacheItem, string> importInvalidUsersCache,
          IdentitySessionManager identitySessionManager,
          IdentityUserTwoFactorChecker identityUserTwoFactorChecker,
          ICancellationTokenProvider cancellationTokenProvider,
          UserSharingManager userSharingManager,
          UserInvitationManager userInvitationManager,
          IIdentityUserInvitationRepository userInvitationRepository,
          IIdentityEmailSender identityEmailSender)
          : base(
              userManager,
              userRepository,
              roleRepository,
              organizationUnitRepository,
              identityClaimTypeRepository,
              identityProTwoFactorManager,
              identityOptions,
              distributedEventBus,
              abpIdentityOptions,
              permissionChecker,
              downloadTokenCache,
              importInvalidUsersCache,
              identitySessionManager,
              identityUserTwoFactorChecker,
              cancellationTokenProvider,
              userSharingManager,
              userInvitationManager,
              userInvitationRepository,
              identityEmailSender)
      {
      }
  
      protected override async Task UpdateUserRolesBasedOnOrganizationUnits(IdentityUser user, IdentityUserCreateOrUpdateDtoBase input)
      {
          var organizationUnitIds = input.OrganizationUnitIds;
          await base.UpdateUserRolesBasedOnOrganizationUnits(user, input);
          input.OrganizationUnitIds = organizationUnitIds;
      }
  }
  

  Drop it into your *.Application project. That constructor is the 10.4 one and it does change between versions, so re-check it if you upgrade before the fix is out.

  If you'd rather not replace the service, sending the current ids from the caller works too:

  input.OrganizationUnitIds = (await _identityUserAppService.GetOrganizationUnitsAsync(id))
      .Select(x => x.Id)
      .ToArray();
  
  await _identityUserAppService.UpdateAsync(id, input);
  

  The fix will be in v10.7.0. Your ticket has been refunded.

  Thanks

  Markdown supported.

  Copy, paste, or drag & drop images and files (max 100 MB per file, 100 MB total per post)

  Save Cancel