diff --git a/framework/src/Volo.Abp.Security/Volo/Abp/Roles/AbpRoleConsts.cs b/framework/src/Volo.Abp.Security/Volo/Abp/Roles/AbpRoleConsts.cs new file mode 100644 index 0000000000..675699788d --- /dev/null +++ b/framework/src/Volo.Abp.Security/Volo/Abp/Roles/AbpRoleConsts.cs @@ -0,0 +1,10 @@ +namespace Volo.Abp.Roles; + +public static class AbpRoleConsts +{ + /// + /// The static name of the admin role. + /// Default value: "admin" + /// + public const string AdminRoleName = "admin"; +} diff --git a/modules/identity/src/Volo.Abp.Identity.Application/Volo/Abp/Identity/IdentityUserAppService.cs b/modules/identity/src/Volo.Abp.Identity.Application/Volo/Abp/Identity/IdentityUserAppService.cs index 8d7e2f4062..0220d2218d 100644 --- a/modules/identity/src/Volo.Abp.Identity.Application/Volo/Abp/Identity/IdentityUserAppService.cs +++ b/modules/identity/src/Volo.Abp.Identity.Application/Volo/Abp/Identity/IdentityUserAppService.cs @@ -9,6 +9,7 @@ using Volo.Abp.Application.Dtos; 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; @@ -70,8 +71,18 @@ public class IdentityUserAppService : IdentityAppServiceBase, IIdentityUserAppSe [Authorize(IdentityPermissions.Users.Default)] public virtual async Task> GetAssignableRolesAsync() { - var currentUserRoles = await UserManager.GetRolesAsync(await UserManager.GetByIdAsync(CurrentUser.GetId())); - var list = (await RoleRepository.GetListAsync(currentUserRoles)).OrderBy(x => x.Name).ToList(); + List 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(ObjectMapper.Map, List>(list)); } @@ -200,6 +211,13 @@ public class IdentityUserAppService : IdentityAppServiceBase, IIdentityUserAppSe protected virtual async Task FilterRolesByCurrentUserAsync(IdentityUser user, string[] inputRoleNames) { + if (await HasAdminRoleAsync()) + { + return (inputRoleNames ?? Array.Empty()) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); + } + var targetCurrentRoleSet = (await UserManager.GetRolesAsync(user)).ToHashSet(StringComparer.OrdinalIgnoreCase); var operatorUser = await UserManager.GetByIdAsync(CurrentUser.GetId()); @@ -215,4 +233,9 @@ public class IdentityUserAppService : IdentityAppServiceBase, IIdentityUserAppSe .Distinct(StringComparer.OrdinalIgnoreCase) .ToArray(); } + + protected virtual Task HasAdminRoleAsync() + { + return Task.FromResult(CurrentUser.IsInRole(AbpRoleConsts.AdminRoleName)); + } } diff --git a/modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor b/modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor index c2c54814ff..803ed76819 100644 --- a/modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor +++ b/modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor @@ -142,13 +142,13 @@ @if (NewUserRoles != null) { - @foreach (var role in NewUserRoles) - { - - - @role.Name - - } + @foreach (var role in NewUserRoles) + { + + + @role.Name + + } } @@ -277,7 +277,7 @@ { - @role.Name + @role.Name } } diff --git a/modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor.cs b/modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor.cs index a52474b8b1..fc35245801 100644 --- a/modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor.cs +++ b/modules/identity/src/Volo.Abp.Identity.Blazor/Pages/Identity/UserManagement.razor.cs @@ -103,7 +103,8 @@ public partial class UserManagement 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 @@ public partial class UserManagement 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; } } diff --git a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityDataSeeder.cs b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityDataSeeder.cs index 1c153effd1..6cfa5a329d 100644 --- a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityDataSeeder.cs +++ b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityDataSeeder.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Options; 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 class IdentityDataSeeder : ITransientDependency, IIdentityDataSeeder 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) diff --git a/modules/identity/test/Volo.Abp.Identity.Application.Tests/Volo/Abp/Identity/IdentityUserAppService_Tests.cs b/modules/identity/test/Volo.Abp.Identity.Application.Tests/Volo/Abp/Identity/IdentityUserAppService_Tests.cs index dd80c8c9d0..0c1a8d6ffc 100644 --- a/modules/identity/test/Volo.Abp.Identity.Application.Tests/Volo/Abp/Identity/IdentityUserAppService_Tests.cs +++ b/modules/identity/test/Volo.Abp.Identity.Application.Tests/Volo/Abp/Identity/IdentityUserAppService_Tests.cs @@ -311,6 +311,29 @@ public class IdentityUserAppService_Tests : AbpIdentityApplicationTestBase roleNames.ShouldNotContain("supporter"); } + [Fact] + public async Task UpdateRolesAsync_Admin_Can_Assign_Any_Role() + { + // admin user can assign roles they do not have (e.g. "sale") + using (_currentPrincipalAccessor.Change(new[] + { + new Claim(AbpClaimTypes.UserId, _testData.UserAdminId.ToString()), + new Claim(AbpClaimTypes.Role, "admin") + })) + { + await _userAppService.UpdateRolesAsync( + _testData.UserDavidId, + new IdentityUserUpdateRolesDto + { + RoleNames = new[] { "sale" } + } + ); + } + + var roleNames = await _userRepository.GetRoleNamesAsync(_testData.UserDavidId); + roleNames.ShouldContain("sale"); + } + [Fact] public async Task UpdateRolesAsync_Self_Cannot_Add_New_Roles() { diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs index 7cd83916f2..5baf233c8c 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs @@ -10,6 +10,8 @@ using Volo.Abp.Localization; using Volo.Abp.MultiTenancy; using Volo.Abp.SimpleStateChecking; using Volo.Abp.PermissionManagement.Localization; +using Volo.Abp.Roles; +using Volo.Abp.Users; namespace Volo.Abp.PermissionManagement; @@ -136,6 +138,11 @@ public class PermissionAppService : ApplicationService, IPermissionAppService protected virtual async Task FilterOutputPermissionsByCurrentUserAsync(GetPermissionListResultDto result) { + if (await HasAdminRoleAsync()) + { + return; + } + // Collect all permission names var allPermissionNames = result.Groups .SelectMany(g => g.Permissions) @@ -417,6 +424,11 @@ public class PermissionAppService : ApplicationService, IPermissionAppService protected virtual async Task FilterInputPermissionsByCurrentUserAsync(UpdatePermissionsDto input) { + if (await HasAdminRoleAsync()) + { + return; + } + if (input.Permissions.IsNullOrEmpty()) { input.Permissions = Array.Empty(); @@ -432,4 +444,9 @@ public class PermissionAppService : ApplicationService, IPermissionAppService // Filters the input DTO in-place to only include manageable permissions. input.Permissions = input.Permissions.Where(x => grantedPermissions.Contains(x.Name)).ToArray(); } + + protected virtual Task HasAdminRoleAsync() + { + return Task.FromResult(CurrentUser.IsInRole(AbpRoleConsts.AdminRoleName)); + } } diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Blazor/Components/PermissionManagementModal.razor.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Blazor/Components/PermissionManagementModal.razor.cs index b3c917177b..09c5867102 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Blazor/Components/PermissionManagementModal.razor.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Blazor/Components/PermissionManagementModal.razor.cs @@ -28,8 +28,6 @@ public partial class PermissionManagementModal protected List _allGroups; protected List _groups; - protected List _disabledPermissions = new List(); - protected string _selectedTabName; protected bool _selectAllDisabled; @@ -102,19 +100,6 @@ public partial class PermissionManagementModal { _selectAllDisabled = _groups.All(IsPermissionGroupDisabled); - if (checkDisabledPermissions) - { - _disabledPermissions.Clear(); - } - - foreach (var permission in _groups.SelectMany(x => x.Permissions)) - { - if (checkDisabledPermissions && permission.IsGranted && permission.GrantedProviders.All(x => x.ProviderName != _providerName)) - { - _disabledPermissions.Add(permission); - } - } - foreach (var group in _groups) { SetPermissionDepths(group.Permissions, null, 0); @@ -285,12 +270,28 @@ public partial class PermissionManagementModal protected virtual bool IsDisabledPermission(PermissionGrantInfoDto permissionGrantInfo) { - return _disabledPermissions.Any(x => x == permissionGrantInfo); + if (!permissionGrantInfo.IsEditable) + { + return true; + } + + return permissionGrantInfo.IsGranted && + permissionGrantInfo.GrantedProviders.All(p => p.ProviderName != _providerName); } protected virtual string GetShownName(PermissionGrantInfoDto permissionGrantInfo) { - if (!IsDisabledPermission(permissionGrantInfo)) + if (permissionGrantInfo.GrantedProviders.All(p => p.ProviderName == _providerName)) + { + return permissionGrantInfo.DisplayName; + } + + var grantedByOtherProviders = permissionGrantInfo.GrantedProviders + .Where(p => p.ProviderName != _providerName) + .Select(p => p.ProviderName) + .ToList(); + + if (!grantedByOtherProviders.Any()) { return permissionGrantInfo.DisplayName; } @@ -298,10 +299,7 @@ public partial class PermissionManagementModal return string.Format( "{0} ({1})", permissionGrantInfo.DisplayName, - permissionGrantInfo.GrantedProviders - .Where(p => p.ProviderName != _providerName) - .Select(p => p.ProviderName) - .JoinAsString(", ") + grantedByOtherProviders.JoinAsString(", ") ); } @@ -313,10 +311,7 @@ public partial class PermissionManagementModal protected virtual bool IsPermissionGroupDisabled(PermissionGroupDto group) { - var permissions = group.Permissions; - var grantedProviders = permissions.SelectMany(x => x.GrantedProviders); - - return permissions.All(x => x.IsGranted) && grantedProviders.Any(p => p.ProviderName != _providerName); + return group.Permissions.All(IsDisabledPermission); } protected virtual async Task ResetSearchTextAsync() diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionDataSeedContributor.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionDataSeedContributor.cs index 71570aafce..89390d1515 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionDataSeedContributor.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionDataSeedContributor.cs @@ -4,6 +4,7 @@ using Volo.Abp.Authorization.Permissions; using Volo.Abp.Data; using Volo.Abp.DependencyInjection; using Volo.Abp.MultiTenancy; +using Volo.Abp.Roles; namespace Volo.Abp.PermissionManagement; @@ -34,7 +35,7 @@ public class PermissionDataSeedContributor : IDataSeedContributor, ITransientDep await PermissionDataSeeder.SeedAsync( RolePermissionValueProvider.ProviderName, - "admin", + AbpRoleConsts.AdminRoleName, permissionNames, context?.TenantId ); diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.Application.Tests/Volo/Abp/PermissionManagement/AbpPermissionManagementApplicationTestBase.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.Application.Tests/Volo/Abp/PermissionManagement/AbpPermissionManagementApplicationTestBase.cs index 986d751a9e..6756b96da8 100644 --- a/modules/permission-management/test/Volo.Abp.PermissionManagement.Application.Tests/Volo/Abp/PermissionManagement/AbpPermissionManagementApplicationTestBase.cs +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.Application.Tests/Volo/Abp/PermissionManagement/AbpPermissionManagementApplicationTestBase.cs @@ -1,11 +1,7 @@ using System; -using System.Collections.Generic; -using System.Text; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; -using NSubstitute; using Volo.Abp.Authorization.Permissions; -using Volo.Abp.Users; namespace Volo.Abp.PermissionManagement; @@ -19,12 +15,6 @@ public class AbpPermissionManagementApplicationTestBase : PermissionManagementTe } protected override void AfterAddApplication(IServiceCollection services) { - var currentUser = Substitute.For(); - currentUser.Roles.Returns(new[] { "admin" }); - currentUser.IsAuthenticated.Returns(true); - - services.AddSingleton(currentUser); - var fakePermissionChecker = new FakePermissionChecker(); services.AddSingleton(fakePermissionChecker); services.Replace(ServiceDescriptor.Singleton(fakePermissionChecker)); diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.Application.Tests/Volo/Abp/PermissionManagement/PermissionAppService_Tests.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.Application.Tests/Volo/Abp/PermissionManagement/PermissionAppService_Tests.cs index a0c08850d3..1fddf07181 100644 --- a/modules/permission-management/test/Volo.Abp.PermissionManagement.Application.Tests/Volo/Abp/PermissionManagement/PermissionAppService_Tests.cs +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.Application.Tests/Volo/Abp/PermissionManagement/PermissionAppService_Tests.cs @@ -163,6 +163,26 @@ public class PermissionAppService_Tests : AbpPermissionManagementApplicationTest testGroup.Permissions.First(p => p.Name == "MyPermission6.ChildPermission2").IsEditable.ShouldBeFalse(); } + [Fact] + public async Task Get_Should_Allow_Admin_To_Edit_All_Permissions() + { + // Current user does NOT have these permissions, but has admin role + _fakePermissionChecker.SetGrantedPermissions("MyPermission1"); + + using (_currentPrincipalAccessor.Change(new Claim(AbpClaimTypes.Role, "admin"))) + { + var result = await _permissionAppService.GetAsync( + UserPermissionValueProvider.ProviderName, + PermissionTestDataBuilder.User1Id.ToString()); + + var testGroup = result.Groups.FirstOrDefault(g => g.Name == "TestGroup"); + testGroup.ShouldNotBeNull(); + + testGroup.Permissions.First(p => p.Name == "MyPermission3").IsEditable.ShouldBeTrue(); + testGroup.Permissions.First(p => p.Name == "MyPermission6").IsEditable.ShouldBeTrue(); + } + } + [Fact] public async Task Update_Should_Not_Grant_Permission_That_Current_User_Does_Not_Have() { @@ -214,4 +234,25 @@ public class PermissionAppService_Tests : AbpPermissionManagementApplicationTest // MyPermission2 should still be granted (current user doesn't have it, revoke filtered out) (await _permissionGrantRepository.FindAsync("MyPermission2", "Test", "Test")).ShouldNotBeNull(); } + + [Fact] + public async Task Update_Should_Allow_Admin_To_Grant_Permissions_Without_Having_Them() + { + (await _permissionGrantRepository.FindAsync("MyPermission2", "Test", "Test")).ShouldBeNull(); + + _fakePermissionChecker.SetGrantedPermissions(); + + using (_currentPrincipalAccessor.Change(new Claim(AbpClaimTypes.Role, "admin"))) + { + await _permissionAppService.UpdateAsync("Test", "Test", new UpdatePermissionsDto() + { + Permissions = new UpdatePermissionDto[] + { + new UpdatePermissionDto() { IsGranted = true, Name = "MyPermission2" } + } + }); + } + + (await _permissionGrantRepository.FindAsync("MyPermission2", "Test", "Test")).ShouldNotBeNull(); + } }