Enforce management permission checks for resource permissions

maliming · maliming · commit ea32e2c96da5 · 2025-12-06T19:25:33.000+08:00
diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/Resources/ResourcePermissionChecker.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/Resources/ResourcePermissionChecker.cs
@@ -17,19 +17,22 @@ public class ResourcePermissionChecker : IResourcePermissionChecker, ITransientD
     protected ICurrentTenant CurrentTenant { get; }
     protected IResourcePermissionValueProviderManager PermissionValueProviderManager { get; }
     protected ISimpleStateCheckerManager<PermissionDefinition> StateCheckerManager { get; }
+    protected IPermissionChecker PermissionChecker { get; }
 
     public ResourcePermissionChecker(
         ICurrentPrincipalAccessor principalAccessor,
         IPermissionDefinitionManager permissionDefinitionManager,
         ICurrentTenant currentTenant,
         IResourcePermissionValueProviderManager permissionValueProviderManager,
-        ISimpleStateCheckerManager<PermissionDefinition> stateCheckerManager)
+        ISimpleStateCheckerManager<PermissionDefinition> stateCheckerManager,
+        IPermissionChecker permissionChecker)
     {
         PrincipalAccessor = principalAccessor;
         PermissionDefinitionManager = permissionDefinitionManager;
         CurrentTenant = currentTenant;
         PermissionValueProviderManager = permissionValueProviderManager;
         StateCheckerManager = stateCheckerManager;
+        PermissionChecker = permissionChecker;
     }
 
     public virtual async Task<bool> IsGrantedAsync(string name, string resourceName, string resourceKey)
@@ -69,6 +72,11 @@ public virtual async Task<bool> IsGrantedAsync(
             return false;
         }
 
+        if (!await PermissionChecker.IsGrantedAsync(claimsPrincipal, permission.ManagementPermissionName!))
+        {
+            return false;
+        }
+
         var isGranted = false;
         var context = new ResourcePermissionValueCheckContext(permission, claimsPrincipal, resourceName, resourceKey);
         foreach (var provider in PermissionValueProviderManager.ValueProviders)
@@ -96,7 +104,7 @@ public virtual async Task<bool> IsGrantedAsync(
 
     public async Task<MultiplePermissionGrantResult> IsGrantedAsync(string[] names, string resourceName, string resourceKey)
     {
-        return await IsGrantedAsync(PrincipalAccessor.Principal, names, resourceName,resourceKey);
+        return await IsGrantedAsync(PrincipalAccessor.Principal, names, resourceName, resourceKey);
     }
 
     public async Task<MultiplePermissionGrantResult> IsGrantedAsync(ClaimsPrincipal? claimsPrincipal, string[] names, string resourceName, string resourceKey)
@@ -116,7 +124,8 @@ public async Task<MultiplePermissionGrantResult> IsGrantedAsync(ClaimsPrincipal?
         foreach (var name in names)
         {
             var permission = await PermissionDefinitionManager.GetResourcePermissionOrNullAsync(resourceName, name);
-            if (permission == null)
+            if (permission == null ||
+                !await PermissionChecker.IsGrantedAsync(claimsPrincipal, permission.ManagementPermissionName!))
             {
                 result.Result.Add(name, PermissionGrantResult.Prohibited);
                 continue;
diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/Resources/ResourcePermissionPopulator.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/Resources/ResourcePermissionPopulator.cs
@@ -9,19 +9,20 @@ namespace Volo.Abp.Authorization.Permissions.Resources;
 public class ResourcePermissionPopulator : ITransientDependency
 {
     protected IPermissionDefinitionManager PermissionDefinitionManager { get; }
-
     protected IResourcePermissionChecker ResourcePermissionChecker { get; }
-
     protected IResourcePermissionStore ResourcePermissionStore { get; }
+    protected IPermissionChecker PermissionChecker { get; }
 
     public ResourcePermissionPopulator(
         IPermissionDefinitionManager permissionDefinitionManager,
         IResourcePermissionChecker resourcePermissionChecker,
-        IResourcePermissionStore resourcePermissionStore)
+        IResourcePermissionStore resourcePermissionStore,
+        IPermissionChecker permissionChecker)
     {
         PermissionDefinitionManager = permissionDefinitionManager;
         ResourcePermissionChecker = resourcePermissionChecker;
         ResourcePermissionStore = resourcePermissionStore;
+        PermissionChecker = permissionChecker;
     }
 
     public virtual async Task PopulateAsync<TResource>(TResource resource, string resourceName)
@@ -36,9 +37,8 @@ public virtual async Task PopulateAsync<TResource>(List<TResource> resources, st
         Check.NotNull(resources, nameof(resources));
         Check.NotNullOrWhiteSpace(resourceName, nameof(resourceName));
 
-        var resopurcePermissionNames = (await PermissionDefinitionManager.GetResourcePermissionsAsync())
+        var resopurcePermissions = (await PermissionDefinitionManager.GetResourcePermissionsAsync())
             .Where(x => x.ResourceName == resourceName)
-            .Select(x => x.Name)
             .ToArray();
 
         foreach (var resource in resources)
@@ -48,17 +48,22 @@ public virtual async Task PopulateAsync<TResource>(List<TResource> resources, st
             {
                 throw new AbpException("Resource key can not be null or empty.");
             }
-            
-            var results = await ResourcePermissionChecker.IsGrantedAsync(resopurcePermissionNames, resourceName, resourceKey);
-            foreach (var resopurcePermission in resopurcePermissionNames)
+
+            var results = await ResourcePermissionChecker.IsGrantedAsync(resopurcePermissions.Select(x => x.Name).ToArray(), resourceName, resourceKey);
+            foreach (var resopurcePermission in resopurcePermissions)
             {
+                if (!await PermissionChecker.IsGrantedAsync(resopurcePermission.ManagementPermissionName!))
+                {
+                    continue;
+                }
+
                 if (resource.ResourcePermissions == null)
                 {
                      ObjectHelper.TrySetProperty(resource, x => x.ResourcePermissions, () => new Dictionary<string, bool>());
                 }
-                
-                var hasPermission = results.Result.TryGetValue(resopurcePermission, out var granted) && granted == PermissionGrantResult.Granted;
-                resource.ResourcePermissions![resopurcePermission] = hasPermission;
+
+                var hasPermission = results.Result.TryGetValue(resopurcePermission.Name, out var granted) && granted == PermissionGrantResult.Granted;
+                resource.ResourcePermissions![resopurcePermission.Name] = hasPermission;
             }
         }
     }
diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/ResourcePermissionChecker_Tests.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/ResourcePermissionChecker_Tests.cs
@@ -21,6 +21,7 @@ public async Task IsGrantedAsync()
     {
         (await _resourcePermissionChecker.IsGrantedAsync("MyResourcePermission5", TestEntityResource.ResourceName, TestEntityResource.ResourceKey5)).ShouldBe(true);
         (await _resourcePermissionChecker.IsGrantedAsync("UndefinedResourcePermission", TestEntityResource.ResourceName, TestEntityResource.ResourceKey5)).ShouldBe(false);
+        (await _resourcePermissionChecker.IsGrantedAsync("MyResourcePermission8", TestEntityResource.ResourceName, TestEntityResource.ResourceKey5)).ShouldBe(false);
     }
 
     [Fact]
@@ -33,7 +34,8 @@ public async Task IsGranted_Multiple_Result_Async()
             "UndefinedPermission",
             "MyResourcePermission3",
             "MyResourcePermission4",
-            "MyResourcePermission5"
+            "MyResourcePermission5",
+            "MyResourcePermission8"
         }, TestEntityResource.ResourceName, TestEntityResource.ResourceKey5);
 
         result.Result["MyResourcePermission1"].ShouldBe(PermissionGrantResult.Undefined);
@@ -42,11 +44,13 @@ public async Task IsGranted_Multiple_Result_Async()
         result.Result["MyResourcePermission3"].ShouldBe(PermissionGrantResult.Granted);
         result.Result["MyResourcePermission4"].ShouldBe(PermissionGrantResult.Prohibited);
         result.Result["MyResourcePermission5"].ShouldBe(PermissionGrantResult.Granted);
+        result.Result["MyResourcePermission8"].ShouldBe(PermissionGrantResult.Prohibited);
 
         result = await _resourcePermissionChecker.IsGrantedAsync(new []
         {
             "MyResourcePermission6",
         }, TestEntityResource.ResourceName, TestEntityResource.ResourceKey6);
+
         result.Result["MyResourcePermission6"].ShouldBe(PermissionGrantResult.Granted);
 
         result = await _resourcePermissionChecker.IsGrantedAsync(new []
diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/ResourcePermissionPopulator_Test.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/ResourcePermissionPopulator_Test.cs
@@ -0,0 +1,58 @@
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using Shouldly;
+using Volo.Abp.Authorization.Permissions.Resources;
+using Volo.Abp.Authorization.TestServices.Resources;
+using Xunit;
+
+namespace Volo.Abp.Authorization;
+
+public class ResourcePermissionPopulator_Tests : AuthorizationTestBase
+{
+    private readonly ResourcePermissionPopulator _resourcePermissionPopulator;
+
+    public ResourcePermissionPopulator_Tests()
+    {
+        _resourcePermissionPopulator = GetRequiredService<ResourcePermissionPopulator>();
+    }
+
+    [Fact]
+    public async Task PopulateAsync()
+    {
+        var testResourceObject = new TestEntityResource(TestEntityResource.ResourceKey5);
+        testResourceObject.ResourcePermissions.IsNullOrEmpty().ShouldBeTrue();
+
+        await _resourcePermissionPopulator.PopulateAsync<TestEntityResource>(
+            testResourceObject,
+            TestEntityResource.ResourceName
+        );
+
+        testResourceObject.ResourcePermissions.ShouldNotBeNull();
+        testResourceObject.ResourcePermissions.Count.ShouldBe(7); // Does not include MyResourcePermission8 because current user has no TestEntityManagementPermission2
+        testResourceObject.ResourcePermissions["MyResourcePermission1"].ShouldBe(false);
+        testResourceObject.ResourcePermissions["MyResourcePermission2"].ShouldBe(false);
+        testResourceObject.ResourcePermissions["MyResourcePermission3"].ShouldBe(true);
+        testResourceObject.ResourcePermissions["MyResourcePermission4"].ShouldBe(false);
+        testResourceObject.ResourcePermissions["MyResourcePermission5"].ShouldBe(true);
+        testResourceObject.ResourcePermissions["MyResourcePermission6"].ShouldBe(false);
+        testResourceObject.ResourcePermissions["MyResourcePermission7"].ShouldBe(false);
+
+        testResourceObject = new TestEntityResource(TestEntityResource.ResourceKey6);
+        testResourceObject.ResourcePermissions.IsNullOrEmpty().ShouldBeTrue();
+
+        await _resourcePermissionPopulator.PopulateAsync<TestEntityResource>(
+            testResourceObject,
+            TestEntityResource.ResourceName
+        );
+
+        testResourceObject.ResourcePermissions.ShouldNotBeNull();
+        testResourceObject.ResourcePermissions.Count.ShouldBe(7); // Does not include MyResourcePermission8 because current user has no TestEntityManagementPermission2
+        testResourceObject.ResourcePermissions["MyResourcePermission1"].ShouldBe(false);
+        testResourceObject.ResourcePermissions["MyResourcePermission2"].ShouldBe(false);
+        testResourceObject.ResourcePermissions["MyResourcePermission3"].ShouldBe(false);
+        testResourceObject.ResourcePermissions["MyResourcePermission4"].ShouldBe(false);
+        testResourceObject.ResourcePermissions["MyResourcePermission5"].ShouldBe(false);
+        testResourceObject.ResourcePermissions["MyResourcePermission6"].ShouldBe(true);
+        testResourceObject.ResourcePermissions["MyResourcePermission7"].ShouldBe(false);
+    }
+}
diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/Resources/AuthorizationTestResourcePermissionDefinitionProvider.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/Resources/AuthorizationTestResourcePermissionDefinitionProvider.cs
@@ -14,6 +14,7 @@ public override void Define(IPermissionDefinitionContext context)
             getGroup = context.AddGroup("TestGroup");
         }
         getGroup.AddPermission("TestEntityManagementPermission");
+        getGroup.AddPermission("TestEntityManagementPermission2");
 
         var permission1 = context.AddResourcePermission("MyResourcePermission1", resourceName: TestEntityResource.ResourceName, "TestEntityManagementPermission");
         Assert.Throws<AbpException>(() =>
@@ -29,6 +30,7 @@ public override void Define(IPermissionDefinitionContext context)
         context.AddResourcePermission("MyResourcePermission5", resourceName: typeof(TestEntityResource).FullName!, "TestEntityManagementPermission");
         context.AddResourcePermission("MyResourcePermission6", resourceName: typeof(TestEntityResource).FullName!, "TestEntityManagementPermission").WithProviders(nameof(TestResourcePermissionValueProvider1));
         context.AddResourcePermission("MyResourcePermission7", resourceName: typeof(TestEntityResource).FullName!, "TestEntityManagementPermission").WithProviders(nameof(TestResourcePermissionValueProvider2));
+        context.AddResourcePermission("MyResourcePermission8", resourceName: typeof(TestEntityResource).FullName!, "TestEntityManagementPermission2");
 
         Assert.Throws<AbpException>(() =>
         {
diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/Resources/FakeResourcePermissionStore.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/Resources/FakeResourcePermissionStore.cs
@@ -19,7 +19,7 @@ public Task<MultiplePermissionGrantResult> IsGrantedAsync(string[] names, string
         var result = new MultiplePermissionGrantResult();
         foreach (var name in names)
         {
-            result.Result.Add(name, (name == "MyResourcePermission3" || name == "MyResourcePermission5" &&
+            result.Result.Add(name, ((name == "MyResourcePermission3" || name == "MyResourcePermission5") &&
                 resourceName == TestEntityResource.ResourceName &&
                 (resourceKey == TestEntityResource.ResourceKey3 || resourceKey == TestEntityResource.ResourceKey5)
                     ? PermissionGrantResult.Granted
diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/Resources/TestEntityResource.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/Resources/TestEntityResource.cs
@@ -1,8 +1,10 @@
 using System;
+using System.Collections.Generic;
+using Volo.Abp.Authorization.Permissions.Resources;
 
 namespace Volo.Abp.Authorization.TestServices.Resources;
 
-public class TestEntityResource
+public class TestEntityResource : IHasResourcePermissions
 {
     public static readonly string ResourceName = typeof(TestEntityResource).FullName;
 
@@ -13,6 +15,20 @@ public class TestEntityResource
     public static readonly string ResourceKey5 = Guid.NewGuid().ToString();
     public static readonly string ResourceKey6 = Guid.NewGuid().ToString();
     public static readonly string ResourceKey7 = Guid.NewGuid().ToString();
+
+    private string Id { get; }
+
+    public TestEntityResource(string id)
+    {
+        Id = id;
+    }
+
+    public string GetObjectKey()
+    {
+        return Id;
+    }
+
+    public Dictionary<string, bool> ResourcePermissions { get; set; }
 }
 
 public class TestEntityResource2
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
@@ -267,7 +267,10 @@ public virtual async Task<GetResourcePermissionListResultDto> GetResourceAsync(s
                 }
             }
 
-            result.Permissions.Add(resourcePermissionGrantInfoDto);
+            if(resourcePermissionGrantInfoDto.Permissions.Any())
+            {
+                result.Permissions.Add(resourcePermissionGrantInfoDto);
+            }
         }
 
         return result;

Original file line number	Diff line number	Diff line change
`@@ -9,19 +9,20 @@ namespace Volo.Abp.Authorization.Permissions.Resources;`
`9`	`9`	`public class ResourcePermissionPopulator : ITransientDependency`
`10`	`10`	`{`
`11`	`11`	`protected IPermissionDefinitionManager PermissionDefinitionManager { get; }`
`12`		`-`
`13`	`12`	`protected IResourcePermissionChecker ResourcePermissionChecker { get; }`
`14`		`-`
`15`	`13`	`protected IResourcePermissionStore ResourcePermissionStore { get; }`
	`14`	`+ protected IPermissionChecker PermissionChecker { get; }`
`16`	`15`
`17`	`16`	`public ResourcePermissionPopulator(`
`18`	`17`	`IPermissionDefinitionManager permissionDefinitionManager,`
`19`	`18`	`IResourcePermissionChecker resourcePermissionChecker,`
`20`		`- IResourcePermissionStore resourcePermissionStore)`
	`19`	`+ IResourcePermissionStore resourcePermissionStore,`
	`20`	`+ IPermissionChecker permissionChecker)`
`21`	`21`	`{`
`22`	`22`	`PermissionDefinitionManager = permissionDefinitionManager;`
`23`	`23`	`ResourcePermissionChecker = resourcePermissionChecker;`
`24`	`24`	`ResourcePermissionStore = resourcePermissionStore;`
	`25`	`+ PermissionChecker = permissionChecker;`
`25`	`26`	`}`
`26`	`27`
`27`	`28`	`public virtual async Task PopulateAsync<TResource>(TResource resource, string resourceName)`
`@@ -36,9 +37,8 @@ public virtual async Task PopulateAsync<TResource>(List<TResource> resources, st`
`36`	`37`	`Check.NotNull(resources, nameof(resources));`
`37`	`38`	`Check.NotNullOrWhiteSpace(resourceName, nameof(resourceName));`
`38`	`39`
`39`		`- var resopurcePermissionNames = (await PermissionDefinitionManager.GetResourcePermissionsAsync())`
	`40`	`+ var resopurcePermissions = (await PermissionDefinitionManager.GetResourcePermissionsAsync())`
`40`	`41`	`.Where(x => x.ResourceName == resourceName)`
`41`		`- .Select(x => x.Name)`
`42`	`42`	`.ToArray();`
`43`	`43`
`44`	`44`	`foreach (var resource in resources)`
`@@ -48,17 +48,22 @@ public virtual async Task PopulateAsync<TResource>(List<TResource> resources, st`
`48`	`48`	`{`
`49`	`49`	`throw new AbpException("Resource key can not be null or empty.");`
`50`	`50`	`}`
`51`		`-`
`52`		`- var results = await ResourcePermissionChecker.IsGrantedAsync(resopurcePermissionNames, resourceName, resourceKey);`
`53`		`- foreach (var resopurcePermission in resopurcePermissionNames)`
	`51`	`+`
	`52`	`+ var results = await ResourcePermissionChecker.IsGrantedAsync(resopurcePermissions.Select(x => x.Name).ToArray(), resourceName, resourceKey);`
	`53`	`+ foreach (var resopurcePermission in resopurcePermissions)`
`54`	`54`	`{`
	`55`	`+ if (!await PermissionChecker.IsGrantedAsync(resopurcePermission.ManagementPermissionName!))`
	`56`	`+ {`
	`57`	`+ continue;`
	`58`	`+ }`
	`59`	`+`
`55`	`60`	`if (resource.ResourcePermissions == null)`
`56`	`61`	`{`
`57`	`62`	`ObjectHelper.TrySetProperty(resource, x => x.ResourcePermissions, () => new Dictionary<string, bool>());`
`58`	`63`	`}`
`59`		`-`
`60`		`- var hasPermission = results.Result.TryGetValue(resopurcePermission, out var granted) && granted == PermissionGrantResult.Granted;`
`61`		`- resource.ResourcePermissions![resopurcePermission] = hasPermission;`
	`64`	`+`
	`65`	`+ var hasPermission = results.Result.TryGetValue(resopurcePermission.Name, out var granted) && granted == PermissionGrantResult.Granted;`
	`66`	`+ resource.ResourcePermissions![resopurcePermission.Name] = hasPermission;`
`62`	`67`	`}`
`63`	`68`	`}`
`64`	`69`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,7 @@ public override void Define(IPermissionDefinitionContext context)`
`14`	`14`	`getGroup = context.AddGroup("TestGroup");`
`15`	`15`	`}`
`16`	`16`	`getGroup.AddPermission("TestEntityManagementPermission");`
	`17`	`+ getGroup.AddPermission("TestEntityManagementPermission2");`
`17`	`18`
`18`	`19`	`var permission1 = context.AddResourcePermission("MyResourcePermission1", resourceName: TestEntityResource.ResourceName, "TestEntityManagementPermission");`
`19`	`20`	`Assert.Throws<AbpException>(() =>`
`@@ -29,6 +30,7 @@ public override void Define(IPermissionDefinitionContext context)`
`29`	`30`	`context.AddResourcePermission("MyResourcePermission5", resourceName: typeof(TestEntityResource).FullName!, "TestEntityManagementPermission");`
`30`	`31`	`context.AddResourcePermission("MyResourcePermission6", resourceName: typeof(TestEntityResource).FullName!, "TestEntityManagementPermission").WithProviders(nameof(TestResourcePermissionValueProvider1));`
`31`	`32`	`context.AddResourcePermission("MyResourcePermission7", resourceName: typeof(TestEntityResource).FullName!, "TestEntityManagementPermission").WithProviders(nameof(TestResourcePermissionValueProvider2));`
	`33`	`+ context.AddResourcePermission("MyResourcePermission8", resourceName: typeof(TestEntityResource).FullName!, "TestEntityManagementPermission2");`
`32`	`34`
`33`	`35`	`Assert.Throws<AbpException>(() =>`
`34`	`36`	`{`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ public Task<MultiplePermissionGrantResult> IsGrantedAsync(string[] names, string`
`19`	`19`	`var result = new MultiplePermissionGrantResult();`
`20`	`20`	`foreach (var name in names)`
`21`	`21`	`{`
`22`		`- result.Result.Add(name, (name == "MyResourcePermission3" \|\| name == "MyResourcePermission5" &&`
	`22`	`+ result.Result.Add(name, ((name == "MyResourcePermission3" \|\| name == "MyResourcePermission5") &&`
`23`	`23`	`resourceName == TestEntityResource.ResourceName &&`
`24`	`24`	`(resourceKey == TestEntityResource.ResourceKey3 \|\| resourceKey == TestEntityResource.ResourceKey5)`
`25`	`25`	`? PermissionGrantResult.Granted`
Original file line number	Diff line number	Diff line change
`@@ -267,7 +267,10 @@ public virtual async Task<GetResourcePermissionListResultDto> GetResourceAsync(s`
`267`	`267`	`}`
`268`	`268`	`}`
`269`	`269`
`270`		`- result.Permissions.Add(resourcePermissionGrantInfoDto);`
	`270`	`+ if(resourcePermissionGrantInfoDto.Permissions.Any())`
	`271`	`+ {`
	`272`	`+ result.Permissions.Add(resourcePermissionGrantInfoDto);`
	`273`	`+ }`
`271`	`274`	`}`
`272`	`275`
`273`	`276`	`return result;`