Merge pull request #24283 from abpframework/PermissionChecker

Refactor multiple permissions grant result handling in `PermissionChecker`

framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs

@@ -48,7 +48,7 @@ public class PermissionChecker : IPermissionChecker, ITransientDependency
         {
             return false;
         }
-    
+
         if (!permission.IsEnabled)
         {
             return false;
@@ -92,12 +92,12 @@ public class PermissionChecker : IPermissionChecker, ITransientDependency
         return isGranted;
     }
 
-    public async Task<MultiplePermissionGrantResult> IsGrantedAsync(string[] names)
+    public virtual async Task<MultiplePermissionGrantResult> IsGrantedAsync(string[] names)
     {
         return await IsGrantedAsync(PrincipalAccessor.Principal, names);
     }
 
-    public async Task<MultiplePermissionGrantResult> IsGrantedAsync(ClaimsPrincipal? claimsPrincipal, string[] names)
+    public virtual async Task<MultiplePermissionGrantResult> IsGrantedAsync(ClaimsPrincipal? claimsPrincipal, string[] names)
     {
         Check.NotNull(names, nameof(names));
 
@@ -146,16 +146,27 @@ public class PermissionChecker : IPermissionChecker, ITransientDependency
                 claimsPrincipal);
 
             var multipleResult = await provider.CheckAsync(context);
-            foreach (var grantResult in multipleResult.Result.Where(grantResult =>
-                result.Result.ContainsKey(grantResult.Key) &&
-                result.Result[grantResult.Key] == PermissionGrantResult.Undefined &&
-                grantResult.Value != PermissionGrantResult.Undefined))
+
+            foreach (var grantResult in multipleResult.Result.Where(x => result.Result.ContainsKey(x.Key)))
             {
-                result.Result[grantResult.Key] = grantResult.Value;
-                permissionDefinitions.RemoveAll(x => x.Name == grantResult.Key);
+                switch (grantResult.Value)
+                {
+                    case PermissionGrantResult.Granted:
+                    {
+                        if (result.Result[grantResult.Key] != PermissionGrantResult.Prohibited)
+                        {
+                            result.Result[grantResult.Key] = PermissionGrantResult.Granted;
+                        }
+                        break;
+                    }
+                    case PermissionGrantResult.Prohibited:
+                        result.Result[grantResult.Key] = PermissionGrantResult.Prohibited;
+                        permissionDefinitions.RemoveAll(x => x.Name == grantResult.Key);
+                        break;
+                }
             }
 
-            if (result.AllGranted || result.AllProhibited)
+            if (result.AllProhibited)
             {
                 break;
             }

framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/AbpAuthorizationTestModule.cs

@@ -31,6 +31,8 @@ public class AbpAuthorizationTestModule : AbpModule
         {
             options.ValueProviders.Add<TestPermissionValueProvider1>();
             options.ValueProviders.Add<TestPermissionValueProvider2>();
+            options.ValueProviders.Add<TestProhibitedPermissionValueProvider1>();
+            options.ValueProviders.Add<TestProhibitedPermissionValueProvider2>();
         });
     }
 }

framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/PermissionChecker_Tests.cs

@@ -8,7 +8,7 @@ namespace Volo.Abp.Authorization;
 public class PermissionChecker_Tests: AuthorizationTestBase
 {
     private readonly IPermissionChecker _permissionChecker;
-    
+
     public PermissionChecker_Tests()
     {
         _permissionChecker = GetRequiredService<IPermissionChecker>();
@@ -21,6 +21,13 @@ public class PermissionChecker_Tests: AuthorizationTestBase
         (await _permissionChecker.IsGrantedAsync("UndefinedPermission")).ShouldBe(false);
     }
 
+    [Fact]
+    public async Task IsGranted_ProhibitedAsync()
+    {
+        (await _permissionChecker.IsGrantedAsync("MyPermission8")).ShouldBe(false);
+        (await _permissionChecker.IsGrantedAsync("MyPermission9")).ShouldBe(false);
+    }
+
     [Fact]
     public async Task IsGranted_Multiple_Result_Async()
     {
@@ -35,7 +42,7 @@ public class PermissionChecker_Tests: AuthorizationTestBase
             "MyPermission6",
             "MyPermission7"
         });
-        
+
         result.Result["MyPermission1"].ShouldBe(PermissionGrantResult.Undefined);
         result.Result["MyPermission2"].ShouldBe(PermissionGrantResult.Prohibited);
         result.Result["UndefinedPermission"].ShouldBe(PermissionGrantResult.Prohibited);
@@ -44,6 +51,18 @@ public class PermissionChecker_Tests: AuthorizationTestBase
         result.Result["MyPermission5"].ShouldBe(PermissionGrantResult.Granted);
         result.Result["MyPermission6"].ShouldBe(PermissionGrantResult.Granted);
         result.Result["MyPermission7"].ShouldBe(PermissionGrantResult.Granted);
-        
     }
-}
+
+    [Fact]
+    public async Task IsGranted_Multiple_Result_ProhibitedAsync()
+    {
+        var result = await _permissionChecker.IsGrantedAsync(new []
+        {
+            "MyPermission8",
+            "MyPermission9"
+        });
+
+        result.Result["MyPermission8"].ShouldBe(PermissionGrantResult.Prohibited);
+        result.Result["MyPermission9"].ShouldBe(PermissionGrantResult.Prohibited);
+    }
+}

framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/AuthorizationTestPermissionDefinitionProvider.cs

@@ -16,11 +16,11 @@ public class AuthorizationTestPermissionDefinitionProvider : PermissionDefinitio
         var group = context.AddGroup("TestGroup");
 
         group[PermissionDefinitionContext.KnownPropertyNames.CurrentProviderName].ShouldBe(typeof(AuthorizationTestPermissionDefinitionProvider).FullName);
-        
+
         var permission1 = group.AddPermission("MyAuthorizedService1");
 
         permission1[PermissionDefinitionContext.KnownPropertyNames.CurrentProviderName].ShouldBe(typeof(AuthorizationTestPermissionDefinitionProvider).FullName);
-       
+
         group.AddPermission("MyPermission1").StateCheckers.Add(new TestRequireEditionPermissionSimpleStateChecker());
         group.AddPermission("MyPermission2");
         group.AddPermission("MyPermission3");
@@ -28,6 +28,8 @@ public class AuthorizationTestPermissionDefinitionProvider : PermissionDefinitio
         group.AddPermission("MyPermission5");
         group.AddPermission("MyPermission6").WithProviders(nameof(TestPermissionValueProvider1));
         group.AddPermission("MyPermission7").WithProviders(nameof(TestPermissionValueProvider2));
+        group.AddPermission("MyPermission8");
+        group.AddPermission("MyPermission9");
 
         group.GetPermissionOrNull("MyAuthorizedService1").ShouldNotBeNull();
 

framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider1.cs

@@ -0,0 +1,38 @@
+using System.Linq;
+using System.Threading.Tasks;
+using Volo.Abp.Authorization.Permissions;
+
+namespace Volo.Abp.Authorization.TestServices;
+
+public class TestProhibitedPermissionValueProvider1 : PermissionValueProvider
+{
+    public TestProhibitedPermissionValueProvider1(IPermissionStore permissionStore) : base(permissionStore)
+    {
+    }
+
+    public override string Name => "TestProhibitedPermissionValueProvider1";
+
+    public override Task<PermissionGrantResult> CheckAsync(PermissionValueCheckContext context)
+    {
+        var result = PermissionGrantResult.Undefined;
+        if (context.Permission.Name == "MyPermission8" || context.Permission.Name == "MyPermission9")
+        {
+            result = PermissionGrantResult.Granted;
+        }
+
+        return Task.FromResult(result);
+    }
+
+    public override Task<MultiplePermissionGrantResult> CheckAsync(PermissionValuesCheckContext context)
+    {
+        var result = new MultiplePermissionGrantResult();
+        foreach (var name in context.Permissions.Select(x => x.Name))
+        {
+            result.Result.Add(name, name == "MyPermission8" || name == "MyPermission9"
+                ? PermissionGrantResult.Granted
+                : PermissionGrantResult.Undefined);
+        }
+
+        return Task.FromResult(result);
+    }
+}

framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider2.cs

@@ -0,0 +1,38 @@
+using System.Linq;
+using System.Threading.Tasks;
+using Volo.Abp.Authorization.Permissions;
+
+namespace Volo.Abp.Authorization.TestServices;
+
+public class TestProhibitedPermissionValueProvider2 : PermissionValueProvider
+{
+    public TestProhibitedPermissionValueProvider2(IPermissionStore permissionStore) : base(permissionStore)
+    {
+    }
+
+    public override string Name => "TestProhibitedPermissionValueProvider2";
+
+    public override Task<PermissionGrantResult> CheckAsync(PermissionValueCheckContext context)
+    {
+        var result = PermissionGrantResult.Undefined;
+        if (context.Permission.Name == "MyPermission8" || context.Permission.Name == "MyPermission9")
+        {
+            result = PermissionGrantResult.Prohibited;
+        }
+
+        return Task.FromResult(result);
+    }
+
+    public override Task<MultiplePermissionGrantResult> CheckAsync(PermissionValuesCheckContext context)
+    {
+        var result = new MultiplePermissionGrantResult();
+        foreach (var name in context.Permissions.Select(x => x.Name))
+        {
+            result.Result.Add(name, name == "MyPermission8" || name == "MyPermission9"
+                ? PermissionGrantResult.Prohibited
+                : PermissionGrantResult.Undefined);
+        }
+
+        return Task.FromResult(result);
+    }
+}