From d75fb66347bec04771fda2cef97a9a62df89b528 Mon Sep 17 00:00:00 2001 From: maliming Date: Thu, 27 Nov 2025 11:13:17 +0800 Subject: [PATCH 1/4] Refactor permission grant result handling in PermissionChecker --- .../Permissions/PermissionChecker.cs | 26 +++++++++---- .../AbpAuthorizationTestModule.cs | 1 + .../Authorization/PermissionChecker_Tests.cs | 27 +++++++++++-- ...izationTestPermissionDefinitionProvider.cs | 6 ++- .../TestServices/FakePermissionStore.cs | 4 +- .../TestProhibitedPermissionValueProvider.cs | 38 +++++++++++++++++++ 6 files changed, 86 insertions(+), 16 deletions(-) create mode 100644 framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider.cs diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs index 7e7457b753..61eee8a348 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs +++ b/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; @@ -146,16 +146,26 @@ 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; + break; + } } - if (result.AllGranted || result.AllProhibited) + if (result.AllProhibited) { break; } diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/AbpAuthorizationTestModule.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/AbpAuthorizationTestModule.cs index a4980a58ec..20b1b91ff1 100644 --- a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/AbpAuthorizationTestModule.cs +++ b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/AbpAuthorizationTestModule.cs @@ -31,6 +31,7 @@ public class AbpAuthorizationTestModule : AbpModule { options.ValueProviders.Add(); options.ValueProviders.Add(); + options.ValueProviders.Add(); }); } } diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/PermissionChecker_Tests.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/PermissionChecker_Tests.cs index f39c48e435..c641f21e92 100644 --- a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/PermissionChecker_Tests.cs +++ b/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(); @@ -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); - } -} \ No newline at end of file + + [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); + } +} diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/AuthorizationTestPermissionDefinitionProvider.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/AuthorizationTestPermissionDefinitionProvider.cs index b2a9179415..7e8ed5e949 100644 --- a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/AuthorizationTestPermissionDefinitionProvider.cs +++ b/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(); diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/FakePermissionStore.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/FakePermissionStore.cs index 4582417d3b..8fe364e132 100644 --- a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/FakePermissionStore.cs +++ b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/FakePermissionStore.cs @@ -8,7 +8,7 @@ public class FakePermissionStore : IPermissionStore, ITransientDependency { public Task IsGrantedAsync(string name, string providerName, string providerKey) { - return Task.FromResult(name == "MyPermission3" || name == "MyPermission5"); + return Task.FromResult(name == "MyPermission3" || name == "MyPermission5" || name == "MyPermission8"); } public Task IsGrantedAsync(string[] names, string providerName, string providerKey) @@ -16,7 +16,7 @@ public class FakePermissionStore : IPermissionStore, ITransientDependency var result = new MultiplePermissionGrantResult(); foreach (var name in names) { - result.Result.Add(name, name == "MyPermission3" || name == "MyPermission5" + result.Result.Add(name, name == "MyPermission3" || name == "MyPermission5" || name == "MyPermission8" ? PermissionGrantResult.Granted : PermissionGrantResult.Prohibited); } diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider.cs new file mode 100644 index 0000000000..06c87a958a --- /dev/null +++ b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider.cs @@ -0,0 +1,38 @@ +using System.Linq; +using System.Threading.Tasks; +using Volo.Abp.Authorization.Permissions; + +namespace Volo.Abp.Authorization.TestServices; + +public class TestProhibitedPermissionValueProvider : PermissionValueProvider +{ + public TestProhibitedPermissionValueProvider(IPermissionStore permissionStore) : base(permissionStore) + { + } + + public override string Name => "TestProhibitedPermissionValueProvider"; + + public override Task 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 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); + } +} From 4ee5529380c15ab9fe9c9fb24830c7591c67d430 Mon Sep 17 00:00:00 2001 From: maliming Date: Thu, 27 Nov 2025 11:24:09 +0800 Subject: [PATCH 2/4] Add prohibited permission value providers and update permission checks --- .../AbpAuthorizationTestModule.cs | 3 +- .../TestServices/FakePermissionStore.cs | 4 +- .../TestProhibitedPermissionValueProvider1.cs | 38 +++++++++++++++++++ ...TestProhibitedPermissionValueProvider2.cs} | 6 +-- 4 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider1.cs rename framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/{TestProhibitedPermissionValueProvider.cs => TestProhibitedPermissionValueProvider2.cs} (84%) diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/AbpAuthorizationTestModule.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/AbpAuthorizationTestModule.cs index 20b1b91ff1..fca3d209b0 100644 --- a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/AbpAuthorizationTestModule.cs +++ b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/AbpAuthorizationTestModule.cs @@ -31,7 +31,8 @@ public class AbpAuthorizationTestModule : AbpModule { options.ValueProviders.Add(); options.ValueProviders.Add(); - options.ValueProviders.Add(); + options.ValueProviders.Add(); + options.ValueProviders.Add(); }); } } diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/FakePermissionStore.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/FakePermissionStore.cs index 8fe364e132..4582417d3b 100644 --- a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/FakePermissionStore.cs +++ b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/FakePermissionStore.cs @@ -8,7 +8,7 @@ public class FakePermissionStore : IPermissionStore, ITransientDependency { public Task IsGrantedAsync(string name, string providerName, string providerKey) { - return Task.FromResult(name == "MyPermission3" || name == "MyPermission5" || name == "MyPermission8"); + return Task.FromResult(name == "MyPermission3" || name == "MyPermission5"); } public Task IsGrantedAsync(string[] names, string providerName, string providerKey) @@ -16,7 +16,7 @@ public class FakePermissionStore : IPermissionStore, ITransientDependency var result = new MultiplePermissionGrantResult(); foreach (var name in names) { - result.Result.Add(name, name == "MyPermission3" || name == "MyPermission5" || name == "MyPermission8" + result.Result.Add(name, name == "MyPermission3" || name == "MyPermission5" ? PermissionGrantResult.Granted : PermissionGrantResult.Prohibited); } diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider1.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider1.cs new file mode 100644 index 0000000000..2f667cb791 --- /dev/null +++ b/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 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 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); + } +} diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider2.cs similarity index 84% rename from framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider.cs rename to framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider2.cs index 06c87a958a..0311c19a65 100644 --- a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider.cs +++ b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/TestProhibitedPermissionValueProvider2.cs @@ -4,13 +4,13 @@ using Volo.Abp.Authorization.Permissions; namespace Volo.Abp.Authorization.TestServices; -public class TestProhibitedPermissionValueProvider : PermissionValueProvider +public class TestProhibitedPermissionValueProvider2 : PermissionValueProvider { - public TestProhibitedPermissionValueProvider(IPermissionStore permissionStore) : base(permissionStore) + public TestProhibitedPermissionValueProvider2(IPermissionStore permissionStore) : base(permissionStore) { } - public override string Name => "TestProhibitedPermissionValueProvider"; + public override string Name => "TestProhibitedPermissionValueProvider2"; public override Task CheckAsync(PermissionValueCheckContext context) { From b90713bdf8382adeed6e3eebbc0358d109f05fe1 Mon Sep 17 00:00:00 2001 From: maliming Date: Thu, 27 Nov 2025 11:26:31 +0800 Subject: [PATCH 3/4] Remove prohibited permissions from definitions --- .../Volo/Abp/Authorization/Permissions/PermissionChecker.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs index 61eee8a348..75d6af1fb9 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs +++ b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs @@ -161,6 +161,7 @@ public class PermissionChecker : IPermissionChecker, ITransientDependency } case PermissionGrantResult.Prohibited: result.Result[grantResult.Key] = PermissionGrantResult.Prohibited; + permissionDefinitions.RemoveAll(x => x.Name == grantResult.Key); break; } } From 5cac9f8a7b3073b6338649f5f25338bd648c66b5 Mon Sep 17 00:00:00 2001 From: maliming Date: Thu, 27 Nov 2025 12:21:40 +0800 Subject: [PATCH 4/4] Make IsGrantedAsync methods virtual in PermissionChecker --- .../Volo/Abp/Authorization/Permissions/PermissionChecker.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs index 75d6af1fb9..4a5e48c220 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs +++ b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs @@ -92,12 +92,12 @@ public class PermissionChecker : IPermissionChecker, ITransientDependency return isGranted; } - public async Task IsGrantedAsync(string[] names) + public virtual async Task IsGrantedAsync(string[] names) { return await IsGrantedAsync(PrincipalAccessor.Principal, names); } - public async Task IsGrantedAsync(ClaimsPrincipal? claimsPrincipal, string[] names) + public virtual async Task IsGrantedAsync(ClaimsPrincipal? claimsPrincipal, string[] names) { Check.NotNull(names, nameof(names));