diff --git a/modules/identity/src/Volo.Abp.PermissionManagement.Domain.Identity/Volo/Abp/PermissionManagement/Identity/RolePermissionManagementProvider.cs b/modules/identity/src/Volo.Abp.PermissionManagement.Domain.Identity/Volo/Abp/PermissionManagement/Identity/RolePermissionManagementProvider.cs index 5cd2dda193..affe9c90c3 100644 --- a/modules/identity/src/Volo.Abp.PermissionManagement.Domain.Identity/Volo/Abp/PermissionManagement/Identity/RolePermissionManagementProvider.cs +++ b/modules/identity/src/Volo.Abp.PermissionManagement.Domain.Identity/Volo/Abp/PermissionManagement/Identity/RolePermissionManagementProvider.cs @@ -65,9 +65,18 @@ public class RolePermissionManagementProvider : PermissionManagementProvider return multiplePermissionValueProviderGrantInfo; } + var permissionGrantsByName = new Dictionary(); + foreach (var permissionGrant in permissionGrants) + { + if (!permissionGrantsByName.ContainsKey(permissionGrant.Name)) + { + permissionGrantsByName[permissionGrant.Name] = permissionGrant; + } + } + foreach (var permissionName in names) { - var permissionGrant = permissionGrants.FirstOrDefault(x => x.Name == permissionName); + var permissionGrant = permissionGrantsByName.GetOrDefault(permissionName); if (permissionGrant != null) { multiplePermissionValueProviderGrantInfo.Result[permissionName] = new PermissionValueProviderGrantInfo(true, permissionGrant.ProviderKey); diff --git a/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/PermissionManager_Tests.cs b/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/PermissionManager_Tests.cs index e0cfe2169c..4f8ecbac41 100644 --- a/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/PermissionManager_Tests.cs +++ b/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/PermissionManager_Tests.cs @@ -87,6 +87,24 @@ public class PermissionManager_Tests : AbpIdentityDomainTestBase ShouldNotHavePermission(grantInfos, TestPermissionNames.MyPermission2_ChildPermission1); } + [Fact] + public async Task Should_Report_A_Single_Role_Provider_When_Several_Roles_Grant_The_Permission() + { + var user = GetUser("john.nash"); + + var grantInfos = await _permissionManager.GetAllForUserAsync(user.Id); + + var grantInfo = grantInfos.Single(x => x.Name == TestPermissionNames.MyPermission1); + grantInfo.IsGranted.ShouldBeTrue(); + + var roleProviders = grantInfo.Providers + .Where(x => x.Name == RolePermissionValueProvider.ProviderName) + .ToList(); + + roleProviders.Count.ShouldBe(1); + roleProviders.Single().Key.ShouldBeOneOf("moderator", "supporter"); + } + private static void RoleShouldHavePermission(List grantInfos, string roleName, string permissionName) { grantInfos.ShouldContain( 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 5baf233c8c..679017d1e1 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 @@ -78,20 +78,29 @@ public class PermissionAppService : ApplicationService, IPermissionAppService .Where(x => !x.Providers.Any() || x.Providers.Contains(providerName)) .Where(x => x.MultiTenancySide.HasFlag(multiTenancySide)); - var neededCheckPermissions = new List(); - foreach (var permission in permissions) + var candidatePermissions = permissions.Distinct().ToArray(); + var childrenByParent = candidatePermissions + .Where(x => x.Parent != null) + .GroupBy(x => x.Parent!) + .ToDictionary(x => x.Key, x => x.ToArray()); + + /* The state checkers of a permission only run when its parent is enabled, + so each tree level is checked in its own batch. */ + var enabledPermissions = new HashSet(); + var currentLevel = candidatePermissions.Where(x => x.Parent == null).ToArray(); + while (currentLevel.Any()) { - if (permission.Parent != null && !neededCheckPermissions.Contains(permission.Parent)) - { - continue; - } + var levelResult = await SimpleStateCheckerManager.IsEnabledAsync(currentLevel); + var enabledLevelPermissions = currentLevel.Where(x => levelResult[x]).ToArray(); + enabledPermissions.UnionWith(enabledLevelPermissions); - if (await SimpleStateCheckerManager.IsEnabledAsync(permission)) - { - neededCheckPermissions.Add(permission); - } + currentLevel = enabledLevelPermissions + .SelectMany(x => childrenByParent.GetOrDefault(x) ?? Array.Empty()) + .ToArray(); } + var neededCheckPermissions = candidatePermissions.Where(enabledPermissions.Contains).ToList(); + if (!neededCheckPermissions.Any()) { continue; @@ -106,11 +115,20 @@ public class PermissionAppService : ApplicationService, IPermissionAppService providerName, providerKey); + var grantInfoByName = new Dictionary(); + foreach (var grantInfo in multipleGrantInfo.Result) + { + if (!grantInfoByName.ContainsKey(grantInfo.Name)) + { + grantInfoByName[grantInfo.Name] = grantInfo; + } + } + foreach (var permissionGroup in permissionGroups) { foreach (var permission in permissionGroup.Permissions) { - var grantInfo = multipleGrantInfo.Result.FirstOrDefault(x => x.Name == permission.Name); + var grantInfo = grantInfoByName.GetOrDefault(permission.Name); if (grantInfo == null) { continue; diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Blazor.MudBlazor/Components/PermissionManagementModal.razor.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Blazor.MudBlazor/Components/PermissionManagementModal.razor.cs index 9ed212236c..946472ef68 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Blazor.MudBlazor/Components/PermissionManagementModal.razor.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Blazor.MudBlazor/Components/PermissionManagementModal.razor.cs @@ -28,6 +28,7 @@ public partial class PermissionManagementModal protected string? _entityDisplayName; protected List? _allGroups; + protected Dictionary _loadedPermissionValues = new Dictionary(); protected List? _groups; protected int _activeTabIndex = 0; @@ -60,6 +61,10 @@ public partial class PermissionManagementModal _allGroups = result.Groups.OrderBy(x => x.DisplayName).ToList(); _groups = _allGroups.ToList(); + _loadedPermissionValues = _allGroups + .SelectMany(x => x.Permissions) + .ToDictionary(x => x.Name, x => x.IsGranted); + NormalizePermissionGroup(); GrantAll = _allGroups.SelectMany(x => x.Permissions).All(p => p.IsGranted); @@ -134,15 +139,14 @@ public partial class PermissionManagementModal return; } - var updateDto = new UpdatePermissionsDto - { - Permissions = _allGroups - .SelectMany(g => g.Permissions) - .Select(p => new UpdatePermissionDto { IsGranted = p.IsGranted, Name = p.Name }) - .ToArray() - }; + var permissions = _allGroups.SelectMany(g => g.Permissions).ToList(); + + var changedPermissions = permissions + .Where(p => !_loadedPermissionValues.TryGetValue(p.Name, out var loadedValue) || loadedValue != p.IsGranted) + .Select(p => new UpdatePermissionDto { IsGranted = p.IsGranted, Name = p.Name }) + .ToArray(); - if (!updateDto.Permissions.Any(x => x.IsGranted)) + if (!permissions.Any(p => p.IsGranted)) { var confirmed = await DialogService.ShowMessageBoxAsync( L["Warning"], @@ -156,15 +160,21 @@ public partial class PermissionManagementModal } } - await PermissionAppService.UpdateAsync(_providerName!, _providerKey!, updateDto); - - Guid? userId = null; - if (_providerName == UserPermissionValueProvider.ProviderName && Guid.TryParse(_providerKey, out var parsedUserId)) + if (changedPermissions.Any()) { - userId = parsedUserId; - } + await PermissionAppService.UpdateAsync(_providerName!, _providerKey!, new UpdatePermissionsDto + { + Permissions = changedPermissions + }); - await CurrentApplicationConfigurationCacheResetService.ResetAsync(userId); + Guid? userId = null; + if (_providerName == UserPermissionValueProvider.ProviderName && Guid.TryParse(_providerKey, out var parsedUserId)) + { + userId = parsedUserId; + } + + await CurrentApplicationConfigurationCacheResetService.ResetAsync(userId); + } _isVisible = false; await InvokeAsync(StateHasChanged); 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 3e1a5aa18b..58eafcbeb1 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 @@ -26,6 +26,8 @@ public partial class PermissionManagementModal protected string _entityDisplayName; protected List _allGroups; + + protected Dictionary _loadedPermissionValues = new Dictionary(); protected List _groups; protected string _selectedTabName; @@ -58,6 +60,10 @@ public partial class PermissionManagementModal _allGroups = result.Groups.OrderBy(x => x.DisplayName).ToList(); _groups = _allGroups.ToList(); + _loadedPermissionValues = _allGroups + .SelectMany(x => x.Permissions) + .ToDictionary(x => x.Name, x => x.IsGranted); + NormalizePermissionGroup(); GrantAll = _allGroups.SelectMany(x => x.Permissions).All(p => p.IsGranted); @@ -121,15 +127,14 @@ public partial class PermissionManagementModal try { - var updateDto = new UpdatePermissionsDto - { - Permissions = _allGroups - .SelectMany(g => g.Permissions) - .Select(p => new UpdatePermissionDto { IsGranted = p.IsGranted, Name = p.Name }) - .ToArray() - }; + var permissions = _allGroups.SelectMany(g => g.Permissions).ToList(); - if (!updateDto.Permissions.Any(x => x.IsGranted)) + var changedPermissions = permissions + .Where(p => !_loadedPermissionValues.TryGetValue(p.Name, out var loadedValue) || loadedValue != p.IsGranted) + .Select(p => new UpdatePermissionDto { IsGranted = p.IsGranted, Name = p.Name }) + .ToArray(); + + if (!permissions.Any(p => p.IsGranted)) { if (!await Message.Confirm(L["SaveWithoutAnyPermissionsWarningMessage"].Value)) { @@ -137,15 +142,21 @@ public partial class PermissionManagementModal } } - await PermissionAppService.UpdateAsync(_providerName, _providerKey, updateDto); - - Guid? userId = null; - if (_providerName == UserPermissionValueProvider.ProviderName && Guid.TryParse(_providerKey, out var parsedUserId)) + if (changedPermissions.Any()) { - userId = parsedUserId; - } + await PermissionAppService.UpdateAsync(_providerName, _providerKey, new UpdatePermissionsDto + { + Permissions = changedPermissions + }); - await CurrentApplicationConfigurationCacheResetService.ResetAsync(userId); + Guid? userId = null; + if (_providerName == UserPermissionValueProvider.ProviderName && Guid.TryParse(_providerKey, out var parsedUserId)) + { + userId = parsedUserId; + } + + await CurrentApplicationConfigurationCacheResetService.ResetAsync(userId); + } await InvokeAsync(_modal.Hide); await Notify.Success(L["SavedSuccessfully"]); diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManagementProvider.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManagementProvider.cs index e853377ddb..b96fed734e 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManagementProvider.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManagementProvider.cs @@ -1,4 +1,5 @@ -using System.Linq; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Volo.Abp.Domain.Repositories; using Volo.Abp.Guids; @@ -44,10 +45,11 @@ public abstract class PermissionManagementProvider : IPermissionManagementProvid } var permissionGrants = await PermissionGrantRepository.GetListAsync(names, providerName, providerKey); + var grantedPermissionNames = new HashSet(permissionGrants.Select(x => x.Name)); foreach (var permissionName in names) { - var isGrant = permissionGrants.Any(x => x.Name == permissionName); + var isGrant = grantedPermissionNames.Contains(permissionName); multiplePermissionValueProviderGrantInfo.Result[permissionName] = new PermissionValueProviderGrantInfo(isGrant, providerKey); } diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManager.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManager.cs index 0bfdf1eba3..d7c91bcad5 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManager.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManager.cs @@ -218,14 +218,22 @@ public class PermissionManager : IPermissionManager, ISingletonDependency var permissionNames = permissions.Select(x => x.Name).ToArray(); var multiplePermissionWithGrantedProviders = new MultiplePermissionWithGrantedProviders(permissionNames); + var stateCheckPermissions = permissions + .Where(x => x.IsEnabled) + .Where(x => x.MultiTenancySide.HasFlag(CurrentTenant.GetMultiTenancySide())) + .Where(x => !x.Providers.Any() || x.Providers.Contains(providerName)) + .Distinct() + .ToArray(); + + var stateCheckResult = stateCheckPermissions.Any() + ? await SimpleStateCheckerManager.IsEnabledAsync(stateCheckPermissions) + : new SimpleStateCheckerResult(); + var neededCheckPermissions = new List(); - foreach (var permission in permissions - .Where(x => x.IsEnabled) - .Where(x => x.MultiTenancySide.HasFlag(CurrentTenant.GetMultiTenancySide())) - .Where(x => !x.Providers.Any() || x.Providers.Contains(providerName))) + foreach (var permission in stateCheckPermissions) { - if (await SimpleStateCheckerManager.IsEnabledAsync(permission)) + if (stateCheckResult[permission]) { neededCheckPermissions.Add(permission); } @@ -236,6 +244,15 @@ public class PermissionManager : IPermissionManager, ISingletonDependency return multiplePermissionWithGrantedProviders; } + var permissionsWithGrantedProvidersByName = new Dictionary(); + foreach (var permissionWithGrantedProviders in multiplePermissionWithGrantedProviders.Result) + { + if (!permissionsWithGrantedProvidersByName.ContainsKey(permissionWithGrantedProviders.Name)) + { + permissionsWithGrantedProvidersByName[permissionWithGrantedProviders.Name] = permissionWithGrantedProviders; + } + } + foreach (var provider in ManagementProviders) { permissionNames = neededCheckPermissions.Select(x => x.Name).ToArray(); @@ -245,8 +262,7 @@ public class PermissionManager : IPermissionManager, ISingletonDependency { if (providerResultDict.Value.IsGranted) { - var permissionWithGrantedProvider = multiplePermissionWithGrantedProviders.Result - .First(x => x.Name == providerResultDict.Key); + var permissionWithGrantedProvider = permissionsWithGrantedProvidersByName[providerResultDict.Key]; permissionWithGrantedProvider.IsGranted = true; permissionWithGrantedProvider.Providers.Add(new PermissionValueProviderInfo(provider.Name, providerResultDict.Value.ProviderKey)); diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionStore.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionStore.cs index 49e956ad1b..84f6ba3b11 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionStore.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionStore.cs @@ -154,16 +154,29 @@ public class PermissionStore : IPermissionStore, ITransientDependency var newCacheItems = await SetCacheItemsAsync(providerName, providerKey, notCacheKeys); - var result = new List>(); - foreach (var key in cacheKeys) + var newCacheItemsByKey = new Dictionary(); + foreach (var newCacheItem in newCacheItems) { - var item = newCacheItems.FirstOrDefault(x => x.Key == key); - if (item.Value == null) + if (!newCacheItemsByKey.ContainsKey(newCacheItem.Key)) { - item = cacheItems.FirstOrDefault(x => x.Key == key); + newCacheItemsByKey[newCacheItem.Key] = newCacheItem.Value; } + } - result.Add(new KeyValuePair(key, item.Value)); + var cacheItemsByKey = new Dictionary(); + foreach (var cacheItem in cacheItems) + { + if (!cacheItemsByKey.ContainsKey(cacheItem.Key)) + { + cacheItemsByKey[cacheItem.Key] = cacheItem.Value; + } + } + + var result = new List>(); + foreach (var key in cacheKeys) + { + var item = newCacheItemsByKey.GetOrDefault(key) ?? cacheItemsByKey.GetOrDefault(key); + result.Add(new KeyValuePair(key, item)); } return result; diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Web/Pages/AbpPermissionManagement/PermissionManagementModal.cshtml.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Web/Pages/AbpPermissionManagement/PermissionManagementModal.cshtml.cs index 8b62db692d..ee1a3846d6 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Web/Pages/AbpPermissionManagement/PermissionManagementModal.cshtml.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Web/Pages/AbpPermissionManagement/PermissionManagementModal.cshtml.cs @@ -32,6 +32,16 @@ public class PermissionManagementModal : AbpPageModel [BindProperty] public List Groups { get; set; } + /* A replaced view that still posts the whole Groups tree does not set this, so it keeps working. */ + [BindProperty] + public bool OnlyChangedPermissions { get; set; } + + [BindProperty] + public string GrantedPermissionNames { get; set; } + + [BindProperty] + public string RevokedPermissionNames { get; set; } + public string EntityDisplayName { get; set; } public bool SelectAllInThisTab { get; set; } @@ -89,14 +99,21 @@ public class PermissionManagementModal : AbpPageModel { ValidateModel(); - var updatePermissionDtos = Groups - .SelectMany(g => g.Permissions) - .Select(p => new UpdatePermissionDto - { - Name = p.Name, - IsGranted = p.IsGranted - }) - .ToArray(); + var updatePermissionDtos = OnlyChangedPermissions + ? GetChangedPermissions() + : Groups + .SelectMany(g => g.Permissions) + .Select(p => new UpdatePermissionDto + { + Name = p.Name, + IsGranted = p.IsGranted + }) + .ToArray(); + + if (updatePermissionDtos.IsNullOrEmpty()) + { + return NoContent(); + } await PermissionAppService.UpdateAsync( ProviderName, @@ -118,6 +135,32 @@ public class PermissionManagementModal : AbpPageModel return NoContent(); } + protected virtual UpdatePermissionDto[] GetChangedPermissions() + { + var permissions = new Dictionary(); + + foreach (var name in SplitPermissionNames(GrantedPermissionNames)) + { + permissions[name] = true; + } + + foreach (var name in SplitPermissionNames(RevokedPermissionNames)) + { + permissions[name] = false; + } + + return permissions + .Select(permission => new UpdatePermissionDto { Name = permission.Key, IsGranted = permission.Value }) + .ToArray(); + } + + protected virtual string[] SplitPermissionNames(string names) + { + return names.IsNullOrWhiteSpace() + ? Array.Empty() + : names.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); + } + public class PermissionGroupViewModel { public string Name { get; set; } diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Web/Pages/AbpPermissionManagement/permission-management-modal.js b/modules/permission-management/src/Volo.Abp.PermissionManagement.Web/Pages/AbpPermissionManagement/permission-management-modal.js index df44e7f31c..ac3c3b9fdf 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Web/Pages/AbpPermissionManagement/permission-management-modal.js +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Web/Pages/AbpPermissionManagement/permission-management-modal.js @@ -299,6 +299,48 @@ var abp = abp || {}; setSelectAllInAllTabs(); var $form = $("#PermissionManagementForm"); + + // defaultChecked holds the state the server rendered. + function submitChangedPermissions() { + // A replaced view without these attributes posts the whole tree instead. + var $permissions = $form.find('[data-permission-name]'); + if (!$permissions.length) { + $form.submit(); + return; + } + + var granted = []; + var revoked = []; + + $permissions.each(function () { + var $permission = $(this); + var checkbox = $permission.find('input[type="checkbox"]')[0]; + if (!checkbox || checkbox.checked === checkbox.defaultChecked) { + return; + } + + (checkbox.checked ? granted : revoked) + .push($permission.attr('data-permission-name')); + }); + + var $treeInputs = $form.find('fieldset').find('input').not(':disabled'); + var $postedInputs = $() + .add($('')) + .add($('').val(granted.join('\n'))) + .add($('').val(revoked.join('\n'))); + + $treeInputs.prop('disabled', true); + $form.append($postedInputs); + + try { + $form.submit(); + } finally { + // The form is serialized synchronously, so the inputs can be restored right away. + $treeInputs.prop('disabled', false); + $postedInputs.remove(); + } + } + var $submitButton = $form.find("button[type='submit']"); if ($submitButton) { $submitButton.click(function (e) { @@ -308,12 +350,12 @@ var abp = abp || {}; abp.message.confirm(l("SaveWithoutAnyPermissionsWarningMessage")) .then(function (confirmed) { if (confirmed) { - $form.submit(); + submitChangedPermissions(); } }); } else { - $form.submit(); + submitChangedPermissions(); } }); } 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 1fddf07181..aac6e05647 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 @@ -17,6 +17,7 @@ public class PermissionAppService_Tests : AbpPermissionManagementApplicationTest private readonly IPermissionGrantRepository _permissionGrantRepository; private readonly ICurrentPrincipalAccessor _currentPrincipalAccessor; private readonly FakePermissionChecker _fakePermissionChecker; + private readonly TestGlobalPermissionStateCheckerCounter _stateCheckerCounter; public PermissionAppService_Tests() { @@ -24,6 +25,29 @@ public class PermissionAppService_Tests : AbpPermissionManagementApplicationTest _permissionGrantRepository = GetRequiredService(); _currentPrincipalAccessor = GetRequiredService(); _fakePermissionChecker = GetRequiredService(); + _stateCheckerCounter = GetRequiredService(); + } + + [Fact] + public async Task Get_Should_Not_Check_The_State_Of_A_Permission_Whose_Parent_Is_Not_Enabled() + { + _stateCheckerCounter.Reset(); + + await _permissionAppService.GetAsync(UserPermissionValueProvider.ProviderName, + PermissionTestDataBuilder.User1Id.ToString()); + + _stateCheckerCounter.CheckedPermissionNames.ShouldContain("MyPermission5"); + _stateCheckerCounter.CheckedPermissionNames.ShouldNotContain("MyPermission5.ChildPermission1"); + + using (_currentPrincipalAccessor.Change(new Claim(AbpClaimTypes.Role, "super-admin"))) + { + _stateCheckerCounter.Reset(); + + await _permissionAppService.GetAsync(UserPermissionValueProvider.ProviderName, + PermissionTestDataBuilder.User1Id.ToString()); + + _stateCheckerCounter.CheckedPermissionNames.ShouldContain("MyPermission5.ChildPermission1"); + } } [Fact] diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/AbpPermissionManagementTestModule.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/AbpPermissionManagementTestModule.cs index bf32af126d..122aed491d 100644 --- a/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/AbpPermissionManagementTestModule.cs +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/AbpPermissionManagementTestModule.cs @@ -22,6 +22,7 @@ public class AbpPermissionManagementTestModule : AbpModule { context.Services.AddEntityFrameworkInMemoryDatabase(); + var databaseName = Guid.NewGuid().ToString(); Configure(options => diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionManager_Tests.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionManager_Tests.cs index b851c0daa9..cd9aabbc6d 100644 --- a/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionManager_Tests.cs +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionManager_Tests.cs @@ -1,10 +1,12 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Security.Claims; using System.Text; using System.Threading.Tasks; using Shouldly; using Volo.Abp.Authorization.Permissions; +using Volo.Abp.Security.Claims; using Xunit; namespace Volo.Abp.PermissionManagement; @@ -13,11 +15,15 @@ public class PermissionManager_Tests : PermissionTestBase { private readonly IPermissionManager _permissionManager; private readonly IPermissionGrantRepository _permissionGrantRepository; + private readonly ICurrentPrincipalAccessor _currentPrincipalAccessor; + private readonly TestGlobalPermissionStateCheckerCounter _stateCheckerCounter; public PermissionManager_Tests() { _permissionManager = GetRequiredService(); _permissionGrantRepository = GetRequiredService(); + _currentPrincipalAccessor = GetRequiredService(); + _stateCheckerCounter = GetRequiredService(); } [Fact] @@ -71,6 +77,63 @@ public class PermissionManager_Tests : PermissionTestBase grantedProviders.Result.Last().Providers.ShouldContain(x => x.Key == "Test"); } + [Fact] + public async Task Multiple_Get_Should_Apply_State_Checkers_Per_Permission() + { + await _permissionGrantRepository.InsertAsync(new PermissionGrant( + Guid.NewGuid(), + "MyPermission1", + "Test", + "Test") + ); + await _permissionGrantRepository.InsertAsync(new PermissionGrant( + Guid.NewGuid(), + "MyPermission5", + "Test", + "Test") + ); + + var names = new[] { "MyPermission1", "MyPermission5" }; + + _stateCheckerCounter.Reset(); + var grantedProviders = await _permissionManager.GetAsync(names, "Test", "Test"); + _stateCheckerCounter.BatchCheckCount.ShouldBe(1); + _stateCheckerCounter.SingleCheckCount.ShouldBe(0); + + grantedProviders.Result.Single(x => x.Name == "MyPermission1").IsGranted.ShouldBeTrue(); + grantedProviders.Result.Single(x => x.Name == "MyPermission5").IsGranted.ShouldBeFalse(); + + using (_currentPrincipalAccessor.Change(new Claim(AbpClaimTypes.Role, "super-admin"))) + { + grantedProviders = await _permissionManager.GetAsync(names, "Test", "Test"); + + grantedProviders.Result.Single(x => x.Name == "MyPermission1").IsGranted.ShouldBeTrue(); + grantedProviders.Result.Single(x => x.Name == "MyPermission5").IsGranted.ShouldBeTrue(); + } + } + + [Fact] + public async Task Multiple_Get_Should_Return_Not_Granted_When_Every_Permission_Is_Filtered_Out() + { + await _permissionGrantRepository.InsertAsync(new PermissionGrant( + Guid.NewGuid(), + "MyDisabledPermission1", + "Test", + "Test") + ); + + _stateCheckerCounter.Reset(); + var grantedProviders = await _permissionManager.GetAsync( + new[] { "MyDisabledPermission1", "MyPermission1NotExist" }, + "Test", + "Test"); + + grantedProviders.Result.Count.ShouldBe(2); + grantedProviders.Result.ShouldAllBe(x => !x.IsGranted); + _stateCheckerCounter.BatchCheckCount.ShouldBe(0); + _stateCheckerCounter.SingleCheckCount.ShouldBe(0); + } + [Fact] public async Task Get_Should_Return_Not_Granted_When_Permission_Undefined() { diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionStore_Tests.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionStore_Tests.cs index 5ca82b1bc9..2dbced3a1e 100644 --- a/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionStore_Tests.cs +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionStore_Tests.cs @@ -39,4 +39,23 @@ public class PermissionStore_Tests : PermissionTestBase result.Result.FirstOrDefault(x => x.Key == "MyPermission1").Value.ShouldBe(PermissionGrantResult.Granted); result.Result.FirstOrDefault(x => x.Key == "MyPermission1NotExist").Value.ShouldBe(PermissionGrantResult.Undefined); } + + [Fact] + public async Task IsGranted_Multiple_Should_Combine_Cached_And_Uncached_Permissions() + { + (await _permissionStore.IsGrantedAsync("MyPermission1", + UserPermissionValueProvider.ProviderName, + PermissionTestDataBuilder.User1Id.ToString())).ShouldBeTrue(); + + var result = await _permissionStore.IsGrantedAsync( + new[] { "MyPermission3", "MyPermission1", "MyPermission1NotExist" }, + UserPermissionValueProvider.ProviderName, + PermissionTestDataBuilder.User1Id.ToString()); + + result.Result.Count.ShouldBe(3); + + result.Result.FirstOrDefault(x => x.Key == "MyPermission3").Value.ShouldBe(PermissionGrantResult.Granted); + result.Result.FirstOrDefault(x => x.Key == "MyPermission1").Value.ShouldBe(PermissionGrantResult.Granted); + result.Result.FirstOrDefault(x => x.Key == "MyPermission1NotExist").Value.ShouldBe(PermissionGrantResult.Undefined); + } } diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/AbpPermissionManagementTestBaseModule.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/AbpPermissionManagementTestBaseModule.cs index 0372fa41d6..6a7d208971 100644 --- a/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/AbpPermissionManagementTestBaseModule.cs +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/AbpPermissionManagementTestBaseModule.cs @@ -4,6 +4,7 @@ using Volo.Abp.Authorization.Permissions; using Volo.Abp.Autofac; using Volo.Abp.DistributedLocking; using Volo.Abp.Modularity; +using Volo.Abp.SimpleStateChecking; using Volo.Abp.Threading; namespace Volo.Abp.PermissionManagement; @@ -19,6 +20,11 @@ public class AbpPermissionManagementTestBaseModule : AbpModule { context.Services.Replace(ServiceDescriptor.Singleton()); + context.Services.Configure>(options => + { + options.GlobalStateCheckers.Add(); + }); + context.Services.Configure(options => { options.ManagementProviders.Add(); diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/TestGlobalPermissionStateChecker.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/TestGlobalPermissionStateChecker.cs new file mode 100644 index 0000000000..9f0a8ecff3 --- /dev/null +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/TestGlobalPermissionStateChecker.cs @@ -0,0 +1,53 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Volo.Abp.Authorization.Permissions; +using Volo.Abp.DependencyInjection; +using Volo.Abp.SimpleStateChecking; + +namespace Volo.Abp.PermissionManagement; + +public class TestGlobalPermissionStateCheckerCounter : ISingletonDependency +{ + public int BatchCheckCount { get; set; } + + public int SingleCheckCount { get; set; } + + public HashSet CheckedPermissionNames { get; } = new HashSet(); + + public void Reset() + { + BatchCheckCount = 0; + SingleCheckCount = 0; + CheckedPermissionNames.Clear(); + } +} + +public class TestGlobalPermissionStateChecker : ISimpleBatchStateChecker, ITransientDependency +{ + public Task IsEnabledAsync(SimpleStateCheckerContext context) + { + var counter = GetCounter(context.ServiceProvider); + counter.SingleCheckCount++; + counter.CheckedPermissionNames.Add(context.State.Name); + return Task.FromResult(true); + } + + public Task> IsEnabledAsync(SimpleBatchStateCheckerContext context) + { + var counter = GetCounter(context.ServiceProvider); + counter.BatchCheckCount++; + foreach (var state in context.States) + { + counter.CheckedPermissionNames.Add(state.Name); + } + + return Task.FromResult(new SimpleStateCheckerResult(context.States)); + } + + private static TestGlobalPermissionStateCheckerCounter GetCounter(IServiceProvider serviceProvider) + { + return serviceProvider.GetRequiredService(); + } +}