Prevent Privilege Escalation: Add Assignment Restrictions for Roles and Permissions

framework/src/Volo.Abp.Security/Volo/Abp/Roles/AbpRoleConsts.cs

@@ -0,0 +1,10 @@
+namespace Volo.Abp.Roles;
+
+public static class AbpRoleConsts
+{
+    /// <summary>
+    /// The static name of the admin role.
+    /// Default value: "admin"
+    /// </summary>
+    public const string AdminRoleName = "admin";
+}

modules/identity/src/Volo.Abp.Identity.Application/Volo/Abp/Identity/IdentityUserAppService.cs

@@ -9,6 +9,8 @@
 using Volo.Abp.Authorization.Permissions;
 using Volo.Abp.Data;
 using Volo.Abp.ObjectExtending;
+using Volo.Abp.Roles;
+using Volo.Abp.Users;
 
 namespace Volo.Abp.Identity;
 
@@ -69,7 +71,18 @@ public virtual async Task<ListResultDto<IdentityRoleDto>> GetRolesAsync(Guid id)
     [Authorize(IdentityPermissions.Users.Default)]
     public virtual async Task<ListResultDto<IdentityRoleDto>> GetAssignableRolesAsync()
     {
-        var list = (await RoleRepository.GetListAsync()).OrderBy(x => x.Name).ToList();
+        List<IdentityRole> list;
+
+        if (await HasAdminRoleAsync())
+        {
+            list = (await RoleRepository.GetListAsync()).OrderBy(x => x.Name).ToList();
+        }
+        else
+        {
+            var currentUserRoles = await UserManager.GetRolesAsync(await UserManager.GetByIdAsync(CurrentUser.GetId()));
+            list = (await RoleRepository.GetListAsync(currentUserRoles)).OrderBy(x => x.Name).ToList();
+        }
+
         return new ListResultDto<IdentityRoleDto>(ObjectMapper.Map<List<IdentityRole>, List<IdentityRoleDto>>(list));
     }
 
@@ -145,7 +158,9 @@ public virtual async Task UpdateRolesAsync(Guid id, IdentityUserUpdateRolesDto i
     {
         await IdentityOptions.SetAsync();
         var user = await UserManager.GetByIdAsync(id);
-        (await UserManager.SetRolesAsync(user, input.RoleNames)).CheckErrors();
+
+        var effectiveRoles = await FilterRolesByCurrentUserAsync(user, input.RoleNames);
+        (await UserManager.SetRolesAsync(user, effectiveRoles)).CheckErrors();
         await UserRepository.UpdateAsync(user);
     }
 
@@ -189,7 +204,38 @@ protected virtual async Task UpdateUserByInput(IdentityUser user, IdentityUserCr
         (await UserManager.UpdateAsync(user)).CheckErrors();
         if (input.RoleNames != null && await PermissionChecker.IsGrantedAsync(IdentityPermissions.Users.ManageRoles))
         {
-            (await UserManager.SetRolesAsync(user, input.RoleNames)).CheckErrors();
+            var effectiveRoles = await FilterRolesByCurrentUserAsync(user, input.RoleNames);
+            (await UserManager.SetRolesAsync(user, effectiveRoles)).CheckErrors();
+        }
+    }
+
+    protected virtual async Task<string[]> FilterRolesByCurrentUserAsync(IdentityUser user, string[] inputRoleNames)
+    {
+        if (await HasAdminRoleAsync())
+        {
+            return (inputRoleNames ?? Array.Empty<string>())
+                .Distinct(StringComparer.OrdinalIgnoreCase)
+                .ToArray();
         }
+
+        var targetCurrentRoleSet = (await UserManager.GetRolesAsync(user)).ToHashSet(StringComparer.OrdinalIgnoreCase);
+
+        var operatorUser = await UserManager.GetByIdAsync(CurrentUser.GetId());
+        var operatorOwnRoleSet = (await UserManager.GetRolesAsync(operatorUser)).ToHashSet(StringComparer.OrdinalIgnoreCase);
+
+        var inputRoleNameSet = new HashSet<string>(inputRoleNames ?? Array.Empty<string>(), StringComparer.OrdinalIgnoreCase);
+        var keepUnmanageableRoles = targetCurrentRoleSet.Except(operatorOwnRoleSet, StringComparer.OrdinalIgnoreCase);
+
+        var desiredManageableRoles = inputRoleNameSet.Intersect(operatorOwnRoleSet, StringComparer.OrdinalIgnoreCase);
+
+        return keepUnmanageableRoles
+            .Concat(desiredManageableRoles)
+            .Distinct(StringComparer.OrdinalIgnoreCase)
+            .ToArray();
+    }
+
+    protected virtual Task<bool> HasAdminRoleAsync()
+    {
+        return Task.FromResult(CurrentUser.IsInRole(AbpRoleConsts.AdminRoleName));
     }
 }

modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor

@@ -142,13 +142,13 @@
                                 <TabPanel Name="Roles">
                                     @if (NewUserRoles != null)
                                     {
-                                        @foreach (var role in NewUserRoles)
-                                        {
-                                            <Field>
-                                                <input type="hidden" @bind-value="@role.Name" />
-                                                <Check TValue="bool" @bind-Checked="@role.IsAssigned">@role.Name</Check>
-                                            </Field>
-                                        }
+                                            @foreach (var role in NewUserRoles)
+                                            {
+                                                <Field>
+                                                    <input type="hidden" @bind-value="@role.Name" />
+                                                    <Check TValue="bool" @bind-Checked="@role.IsAssigned" Disabled="@(!role.IsAssignable)">@role.Name</Check>
+                                                </Field>
+                                            }
                                     }
                                 </TabPanel>
                             </Content>
@@ -277,7 +277,7 @@
                                             {
                                                 <Field>
                                                     <input type="hidden" @bind-value="@role.Name" />
-                                                    <Check TValue="bool" @bind-Checked="@role.IsAssigned">@role.Name</Check>
+                                                    <Check TValue="bool" @bind-Checked="@role.IsAssigned" Disabled="@(!role.IsAssignable)">@role.Name</Check>
                                                 </Field>
                                             }
                                         }

modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor.cs

@@ -103,7 +103,8 @@ protected override async Task OpenCreateModalAsync()
         NewUserRoles = Roles.Select(x => new AssignedRoleViewModel
         {
             Name = x.Name,
-            IsAssigned = x.IsDefault
+            IsAssigned = x.IsDefault,
+            IsAssignable = true
         }).ToArray();
 
         ChangePasswordTextRole(TextRole.Password);
@@ -130,12 +131,23 @@ protected override async Task OpenEditModalAsync(IdentityUserDto entity)
 
             if (await PermissionChecker.IsGrantedAsync(IdentityPermissions.Users.ManageRoles))
             {
-                var userRoleIds = (await AppService.GetRolesAsync(entity.Id)).Items.Select(r => r.Id).ToList();
+                var assignableRoles = Roles ?? (await AppService.GetAssignableRolesAsync()).Items;
+                var currentRoles = (await AppService.GetRolesAsync(entity.Id)).Items;
 
-                EditUserRoles = Roles.Select(x => new AssignedRoleViewModel
+                var combinedRoles = assignableRoles
+                    .Concat(currentRoles)
+                    .GroupBy(role => role.Id)
+                    .Select(group => group.First())
+                    .ToList();
+
+                var currentRoleIds = currentRoles.Select(r => r.Id).ToHashSet();
+                var assignableRoleIds = assignableRoles.Select(r => r.Id).ToHashSet();
+
+                EditUserRoles = combinedRoles.Select(x => new AssignedRoleViewModel
                 {
                     Name = x.Name,
-                    IsAssigned = userRoleIds.Contains(x.Id)
+                    IsAssigned = currentRoleIds.Contains(x.Id),
+                    IsAssignable = assignableRoleIds.Contains(x.Id)
                 }).ToArray();
 
                 ChangePasswordTextRole(TextRole.Password);
@@ -262,4 +274,6 @@ public class AssignedRoleViewModel
     public string Name { get; set; }
 
     public bool IsAssigned { get; set; }
+
+    public bool IsAssignable { get; set; }
 }

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

@@ -5,6 +5,7 @@
 using Volo.Abp.DependencyInjection;
 using Volo.Abp.Guids;
 using Volo.Abp.MultiTenancy;
+using Volo.Abp.Roles;
 using Volo.Abp.Uow;
 
 namespace Volo.Abp.Identity;
@@ -83,7 +84,7 @@ public virtual async Task<IdentityDataSeedResult> SeedAsync(
             result.CreatedAdminUser = true;
 
             //"admin" role
-            const string adminRoleName = "admin";
+            const string adminRoleName = AbpRoleConsts.AdminRoleName;
             var adminRole =
                 await RoleRepository.FindByNormalizedNameAsync(LookupNormalizer.NormalizeName(adminRoleName));
             if (adminRole == null)

modules/identity/src/Volo.Abp.Identity.Web/Pages/Identity/Users/EditModal.cshtml

@@ -83,7 +83,14 @@
                             @for (var i = 0; i < Model.Roles.Length; i++)
                             {
                                 var role = Model.Roles[i];
-                                <abp-input abp-id-name="@Model.Roles[i].IsAssigned" asp-for="@role.IsAssigned" label="@role.Name" />
+                                @if (role.IsAssignable)
+                                {
+                                    <abp-input abp-id-name="@Model.Roles[i].IsAssigned" asp-for="@role.IsAssigned" label="@role.Name" />
+                                }
+                                else
+                                {
+                                    <abp-input abp-id-name="@Model.Roles[i].IsAssigned" asp-for="@role.IsAssigned" label="@role.Name" disabled="true" readonly="true" />
+                                }
                                 <input abp-id-name="@Model.Roles[i].Name" asp-for="@role.Name" />
                             }
                         </div>

modules/identity/src/Volo.Abp.Identity.Web/Pages/Identity/Users/EditModal.cshtml.cs

@@ -41,18 +41,32 @@ public virtual async Task<IActionResult> OnGetAsync(Guid id)
         UserInfo = ObjectMapper.Map<IdentityUserDto, UserInfoViewModel>(user);
         if (await PermissionChecker.IsGrantedAsync(IdentityPermissions.Users.ManageRoles))
         {
-            Roles = ObjectMapper.Map<IReadOnlyList<IdentityRoleDto>, AssignedRoleViewModel[]>((await IdentityUserAppService.GetAssignableRolesAsync()).Items);
-        }
-        IsEditCurrentUser = CurrentUser.Id == id;
-
-        var userRoleIds = (await IdentityUserAppService.GetRolesAsync(UserInfo.Id)).Items.Select(r => r.Id).ToList();
-        foreach (var role in Roles)
-        {
-            if (userRoleIds.Contains(role.Id))
+            var assignableRoles = (await IdentityUserAppService.GetAssignableRolesAsync()).Items;
+            var currentRoles = (await IdentityUserAppService.GetRolesAsync(id)).Items;
+
+            // Combine assignable and current roles to show all roles user has
+            var combinedRoles = assignableRoles
+                .Concat(currentRoles)
+                .GroupBy(role => role.Id)
+                .Select(group => group.First())
+                .ToList();
+
+            Roles = ObjectMapper.Map<IReadOnlyList<IdentityRoleDto>, AssignedRoleViewModel[]>(combinedRoles);
+
+            var currentRoleIds = currentRoles.Select(r => r.Id).ToHashSet();
+            var assignableRoleIds = assignableRoles.Select(r => r.Id).ToHashSet();
+            foreach (var role in Roles)
             {
-                role.IsAssigned = true;
+                role.IsAssigned = currentRoleIds.Contains(role.Id);
+                role.IsAssignable = assignableRoleIds.Contains(role.Id);
             }
         }
+        else
+        {
+            Roles = Array.Empty<AssignedRoleViewModel>();
+        }
+
+        IsEditCurrentUser = CurrentUser.Id == id;
 
         Detail = ObjectMapper.Map<IdentityUserDto, DetailViewModel>(user);
 
@@ -129,6 +143,8 @@ public class AssignedRoleViewModel
         public string Name { get; set; }
 
         public bool IsAssigned { get; set; }
+
+        public bool IsAssignable { get; set; }
     }
 
     public class DetailViewModel

modules/identity/test/Volo.Abp.Identity.Application.Tests/Volo/Abp/Identity/FakeCurrentPrincipalAccessor.cs

@@ -0,0 +1,39 @@
+using System;
+using System.Collections.Generic;
+using System.Security.Claims;
+using Volo.Abp.DependencyInjection;
+using Volo.Abp.Security.Claims;
+
+namespace Volo.Abp.Identity;
+
+[Dependency(ReplaceServices = true)]
+public class FakeCurrentPrincipalAccessor : ThreadCurrentPrincipalAccessor
+{
+    private readonly IdentityTestData _testData;
+    private readonly Lazy<ClaimsPrincipal> _principal;
+
+    public FakeCurrentPrincipalAccessor(IdentityTestData testData)
+    {
+        _testData = testData;
+        _principal = new Lazy<ClaimsPrincipal>(() => new ClaimsPrincipal(
+            new ClaimsIdentity(
+                new List<Claim>
+                {
+                    new Claim(AbpClaimTypes.UserId, _testData.UserAdminId.ToString()),
+                    new Claim(AbpClaimTypes.UserName, "administrator"),
+                    new Claim(AbpClaimTypes.Email, "administrator@abp.io")
+                }
+            )
+        ));
+    }
+
+    protected override ClaimsPrincipal GetClaimsPrincipal()
+    {
+        return GetPrincipal();
+    }
+
+    private ClaimsPrincipal GetPrincipal()
+    {
+        return _principal.Value;
+    }
+}