UpdateRolesAsync() changes in IdentityUserAppService caused bug #10630

1 Answer(s)

• 

  0

  maliming created 4 months ago

  Support Team Fullstack Developer

  Hi,

  This is the intended behavior, not a bug. The change comes from PR #24775 - Prevent Privilege Escalation: Add Assignment Restrictions for Roles and Permissions, which closes a real privilege escalation hole: a non-admin operator with IdentityPermissions.Users.Update could otherwise assign any role (including admin) to other users, or to themselves.

  The new rule in IdentityUserAppService.UpdateRolesAsync is:

  • If the current operator is in the admin role (HasAdminRoleAsync() returns true), the input is used as-is.
  • Otherwise, the operator can only grant roles they themselves already have. Roles the target user already has but the operator does not have are preserved (so the operator cannot strip a higher-privileged role either).

  The "is operator an admin?" check intentionally uses the well-known AbpRoleConsts.AdminRoleName ("admin") because that's the role the framework itself seeds and treats as superuser everywhere (data seeding, permission management, etc.). It's a const, and changing it to a settable property is something we'd rather not do - it would break binary compatibility (a const is inlined at compile time into every consumer), and IdentityDataSeeder does const string adminRoleName = AbpRoleConsts.AdminRoleName;, which only compiles because the right-hand side is a compile-time constant. So we don't recommend renaming the framework's admin role - it's much safer to keep an admin role in your DB even if your business uses different role names elsewhere.

  That said, if you really need a different role name to act as the "superuser" for this check, you don't need to override UpdateRolesAsync. We left a smaller, more precise extension point: HasAdminRoleAsync() is protected virtual. Just override that one method in your *.Application project:

  using System.Threading.Tasks;
  using Microsoft.AspNetCore.Identity;
  using Microsoft.Extensions.Caching.Distributed;
  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 YourProject.Identity;
  
  [Dependency(ReplaceServices = true)]
  [ExposeServices(typeof(IIdentityUserAppService), typeof(IdentityUserAppService))]
  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 Task<bool> HasAdminRoleAsync()
      {
          return Task.FromResult(CurrentUser.IsInRole("YourCustomAdminRoleName"));
      }
  }
  

  I verified locally on Volo.Abp.Identity.Pro.Application.Tests against ABP 10.3 - the override is picked up correctly, an operator with the custom admin role claim can assign any role (including ones they don't have themselves), and all the existing role-filtering / unmanageable-role-preservation tests keep passing.

  Thanks.

  Markdown supported.

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

  Save Cancel