From 86aee14c88a286e5eaa3d51fc55c090e5a59aa07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Halil=20=C4=B0brahim=20Kalkan?= Date: Mon, 6 Apr 2020 22:42:51 +0300 Subject: [PATCH] Resolved #3486: Allow to Enable/Disable a permission. --- docs/en/Authorization.md | 26 +++++++++++++ .../IPermissionDefinitionContext.cs | 20 ++++++++++ .../Permissions/PermissionChecker.cs | 9 ++++- .../Permissions/PermissionDefinition.cs | 23 +++++++++-- .../PermissionDefinitionContext.cs | 33 +++++++++++++++- .../Permissions/PermissionGroupDefinition.cs | 39 ++++++++++++++++++- ...izationTestPermissionDefinitionProvider.cs | 11 ++++-- .../PermissionAppService.cs | 5 +++ .../PermissionManagement/PermissionManager.cs | 11 ++++++ .../PermissionAppService_Tests.cs | 1 - .../PermissionChecker_User_Tests.cs | 11 ++++++ .../PermissionTestDataBuilder.cs | 9 +++++ .../TestPermissionDefinitionProvider.cs | 1 + 13 files changed, 188 insertions(+), 11 deletions(-) diff --git a/docs/en/Authorization.md b/docs/en/Authorization.md index 888d907b59..1522fa586a 100644 --- a/docs/en/Authorization.md +++ b/docs/en/Authorization.md @@ -142,6 +142,20 @@ myGroup.AddPermission( ); ``` +#### Enable/Disable Permissions + +A permission is enabled by default. It is possible to disable a permission. A disabled permission will be prohibited for everyone. You can still check for the permission, but it will always return prohibited. + +Example definition: + +````csharp +myGroup.AddPermission("Author_Management", isEnabled: false); +```` + +You normally don't need to define a disabled permission (unless you temporary want disable a feature of your application). However, you may want to disable a permission defined in a depended module. In this way you can disable the related application functionality. See the "*Changing Permission Definitions of a Depended Module*" section below for an example usage. + +> Note: Checking an undefined permission will throw an exception while a disabled permission check simply returns prohibited (false). + #### Child Permissions A permission may have child permissions. It is especially useful when you want to create a hierarchical permission tree where a permission may have additional sub permissions which are available only if the parent permission has been granted. @@ -208,6 +222,18 @@ See [policy based authorization](https://docs.microsoft.com/en-us/aspnet/core/se A class deriving from the `PermissionDefinitionProvider` (just like the example above) can also get existing permission definitions (defined by the depended [modules](Module-Development-Basics.md)) and change their definitions. +Example: + +````csharp +context + .GetPermissionOrNull(IdentityPermissions.Roles.Delete) + .IsEnabled = false; +```` + +When you write this code inside your permission definition provider, it finds the "role deletion" permission of the [Identity Module](Modules/Identity.md) and disabled the permission, so no one can delete a role on the application. + +> Tip: It is better to check the value returned by the `GetPermissionOrNull` method since it may return null if the given permission was not defined. + ## IAuthorizationService ASP.NET Core provides the `IAuthorizationService` that can be used to check for authorization. Once you inject, you can use it in your code to conditionally control the authorization. diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/IPermissionDefinitionContext.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/IPermissionDefinitionContext.cs index e5541caf3f..0b649287ea 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/IPermissionDefinitionContext.cs +++ b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/IPermissionDefinitionContext.cs @@ -7,13 +7,33 @@ namespace Volo.Abp.Authorization.Permissions public interface IPermissionDefinitionContext { //TODO: Add Get methods to find and modify a permission or group. + + /// + /// Gets a pre-defined permission group. + /// Throws if can not find the given group. + /// + /// Name of the group + /// + PermissionGroupDefinition GetGroup([NotNull] string name); + + /// + /// Tries to get a pre-defined permission group. + /// Returns null if can not find the given group. + /// + /// Name of the group + /// + [NotNull] PermissionGroupDefinition GetGroupOrNull(string name); + [CanBeNull] PermissionGroupDefinition AddGroup( [NotNull] string name, ILocalizableString displayName = null, MultiTenancySides multiTenancySide = MultiTenancySides.Both); void RemoveGroup(string name); + + [CanBeNull] + PermissionDefinition GetPermissionOrNull([NotNull] string name); } } \ No newline at end of file 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 51c8530ee6..931c9d1d23 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 @@ -32,12 +32,19 @@ namespace Volo.Abp.Authorization.Permissions return await IsGrantedAsync(PrincipalAccessor.Principal, name); } - public virtual async Task IsGrantedAsync(ClaimsPrincipal claimsPrincipal, string name) + public virtual async Task IsGrantedAsync( + ClaimsPrincipal claimsPrincipal, + string name) { Check.NotNull(name, nameof(name)); var permission = PermissionDefinitionManager.Get(name); + if (!permission.IsEnabled) + { + return false; + } + var multiTenancySide = claimsPrincipal?.GetMultiTenancySide() ?? CurrentTenant.GetMultiTenancySide(); diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionDefinition.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionDefinition.cs index 2e274965b2..602fdd9c50 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionDefinition.cs +++ b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionDefinition.cs @@ -46,6 +46,19 @@ namespace Volo.Abp.Authorization.Permissions /// public Dictionary Properties { get; } + /// + /// Indicates whether this permission is enabled or disabled. + /// A permission is normally enabled. + /// A disabled permission can not be granted to anyone, but it is still + /// will be available to check its value (while it will always be false). + /// + /// Disabling a permission would be helpful to hide a related application + /// functionality from users/clients. + /// + /// Default: true. + /// + public bool IsEnabled { get; set; } + /// /// Gets/sets a key-value on the . /// @@ -63,11 +76,13 @@ namespace Volo.Abp.Authorization.Permissions protected internal PermissionDefinition( [NotNull] string name, ILocalizableString displayName = null, - MultiTenancySides multiTenancySide = MultiTenancySides.Both) + MultiTenancySides multiTenancySide = MultiTenancySides.Both, + bool isEnabled = true) { Name = Check.NotNull(name, nameof(name)); DisplayName = displayName ?? new FixedLocalizableString(name); MultiTenancySide = multiTenancySide; + IsEnabled = isEnabled; Properties = new Dictionary(); Providers = new List(); @@ -77,12 +92,14 @@ namespace Volo.Abp.Authorization.Permissions public virtual PermissionDefinition AddChild( [NotNull] string name, ILocalizableString displayName = null, - MultiTenancySides multiTenancySide = MultiTenancySides.Both) + MultiTenancySides multiTenancySide = MultiTenancySides.Both, + bool isEnabled = true) { var child = new PermissionDefinition( name, displayName, - multiTenancySide) + multiTenancySide, + isEnabled) { Parent = this }; diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionDefinitionContext.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionDefinitionContext.cs index 777cd1f7b0..95f02b5aee 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionDefinitionContext.cs +++ b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionDefinitionContext.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using JetBrains.Annotations; using Volo.Abp.Localization; using Volo.Abp.MultiTenancy; @@ -28,7 +29,20 @@ namespace Volo.Abp.Authorization.Permissions return Groups[name] = new PermissionGroupDefinition(name, displayName, multiTenancySide); } - public virtual PermissionGroupDefinition GetGroupOrNull(string name) + [NotNull] + public virtual PermissionGroupDefinition GetGroup([NotNull] string name) + { + var group = GetGroupOrNull(name); + + if (group == null) + { + throw new AbpException($"Could not find a permission definition group with the given name: {name}"); + } + + return group; + } + + public virtual PermissionGroupDefinition GetGroupOrNull([NotNull] string name) { Check.NotNull(name, nameof(name)); @@ -51,5 +65,22 @@ namespace Volo.Abp.Authorization.Permissions Groups.Remove(name); } + + public virtual PermissionDefinition GetPermissionOrNull([NotNull] string name) + { + Check.NotNull(name, nameof(name)); + + foreach (var groupDefinition in Groups.Values) + { + var permissionDefinition = groupDefinition.GetPermissionOrNull(name); + + if (permissionDefinition != null) + { + return permissionDefinition; + } + } + + return null; + } } } \ No newline at end of file diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionGroupDefinition.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionGroupDefinition.cs index 5582739a11..5038e8e064 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionGroupDefinition.cs +++ b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionGroupDefinition.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using System.Collections.Immutable; +using JetBrains.Annotations; using Volo.Abp.Localization; using Volo.Abp.MultiTenancy; @@ -60,9 +61,15 @@ namespace Volo.Abp.Authorization.Permissions public virtual PermissionDefinition AddPermission( string name, ILocalizableString displayName = null, - MultiTenancySides multiTenancySide = MultiTenancySides.Both) + MultiTenancySides multiTenancySide = MultiTenancySides.Both, + bool isEnabled = true) { - var permission = new PermissionDefinition(name, displayName, multiTenancySide); + var permission = new PermissionDefinition( + name, + displayName, + multiTenancySide, + isEnabled + ); _permissions.Add(permission); @@ -95,5 +102,33 @@ namespace Volo.Abp.Authorization.Permissions { return $"[{nameof(PermissionGroupDefinition)} {Name}]"; } + + [CanBeNull] + public PermissionDefinition GetPermissionOrNull([NotNull] string name) + { + Check.NotNull(name, nameof(name)); + + return GetPermissionOrNullRecursively(Permissions, name); + } + + private PermissionDefinition GetPermissionOrNullRecursively( + IReadOnlyList permissions, string name) + { + foreach (var permission in permissions) + { + if (permission.Name == name) + { + return permission; + } + + var childPermission = GetPermissionOrNullRecursively(permission.Children, name); + if (childPermission != null) + { + return childPermission; + } + } + + return null; + } } } \ No newline at end of file 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 bb28b4ea99..8cff4c361f 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 @@ -1,4 +1,5 @@ -using Volo.Abp.Authorization.Permissions; +using Shouldly; +using Volo.Abp.Authorization.Permissions; namespace Volo.Abp.Authorization.TestServices { @@ -6,14 +7,18 @@ namespace Volo.Abp.Authorization.TestServices { public override void Define(IPermissionDefinitionContext context) { - PermissionGroupDefinition getGroup = context.GetGroupOrNull("TestGetGroup"); + var getGroup = context.GetGroupOrNull("TestGetGroup"); if (getGroup == null) { getGroup = context.AddGroup("TestGetGroup"); } - PermissionGroupDefinition group = context.AddGroup("TestGroup"); + + var group = context.AddGroup("TestGroup"); + group.AddPermission("MyAuthorizedService1"); + group.GetPermissionOrNull("MyAuthorizedService1").ShouldNotBeNull(); + context.RemoveGroup("TestGetGroup"); } } 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 0c644e5d73..c358edf4a8 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 @@ -52,6 +52,11 @@ namespace Volo.Abp.PermissionManagement foreach (var permission in group.GetPermissionsWithChildren()) { + if (!permission.IsEnabled) + { + continue; + } + if (permission.Providers.Any() && !permission.Providers.Contains(providerName)) { continue; 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 9712c09f4b..9a2a89e340 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 @@ -71,6 +71,12 @@ namespace Volo.Abp.PermissionManagement { var permission = PermissionDefinitionManager.Get(permissionName); + if (!permission.IsEnabled) + { + //TODO: BusinessException + throw new ApplicationException($"The permission named '{permission.Name}' is disabled!"); + } + if (permission.Providers.Any() && !permission.Providers.Contains(providerName)) { //TODO: BusinessException @@ -109,6 +115,11 @@ namespace Volo.Abp.PermissionManagement { var result = new PermissionWithGrantedProviders(permission.Name, false); + if (!permission.IsEnabled) + { + return result; + } + if (!permission.MultiTenancySide.HasFlag(CurrentTenant.GetMultiTenancySide())) { return result; 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 e6240a34b2..fe528cbc3c 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 @@ -31,7 +31,6 @@ namespace Volo.Abp.PermissionManagement.Application.Tests.Volo.Abp.PermissionMan permissionListResultDto.Groups.Count.ShouldBe(1); permissionListResultDto.Groups.ShouldContain(x => x.Name == "TestGroup"); - permissionListResultDto.Groups.First().Permissions.Count.ShouldBe(4); permissionListResultDto.Groups.First().Permissions.ShouldContain(x => x.Name == "MyPermission1"); permissionListResultDto.Groups.First().Permissions.ShouldContain(x => x.Name == "MyPermission2"); permissionListResultDto.Groups.First().Permissions.ShouldContain(x => x.Name == "MyPermission2.ChildPermission1"); diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionChecker_User_Tests.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionChecker_User_Tests.cs index b49ff94f90..0d455d11d8 100644 --- a/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionChecker_User_Tests.cs +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionChecker_User_Tests.cs @@ -35,6 +35,17 @@ namespace Volo.Abp.PermissionManagement )).ShouldBeFalse(); } + + [Fact] + public async Task Should_Return_False_For_Granted_Current_User_If_The_Permission_Is_Disabled() + { + //Disabled permissions always returns false! + (await _permissionChecker.IsGrantedAsync( + CreatePrincipal(PermissionTestDataBuilder.User1Id), + "MyDisabledPermission1" + )).ShouldBeFalse(); + } + [Fact] public async Task Should_Return_False_For_Current_User_If_Anonymous() { diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/PermissionTestDataBuilder.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/PermissionTestDataBuilder.cs index 662d0b17c8..e763c24d0b 100644 --- a/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/PermissionTestDataBuilder.cs +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/PermissionTestDataBuilder.cs @@ -31,6 +31,15 @@ namespace Volo.Abp.PermissionManagement ) ); + await _permissionGrantRepository.InsertAsync( + new PermissionGrant( + _guidGenerator.Create(), + "MyDisabledPermission1", + UserPermissionValueProvider.ProviderName, + User1Id.ToString() + ) + ); + await _permissionGrantRepository.InsertAsync( new PermissionGrant( _guidGenerator.Create(), diff --git a/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/TestPermissionDefinitionProvider.cs b/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/TestPermissionDefinitionProvider.cs index 5dfc6431c4..584c4dd2be 100644 --- a/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/TestPermissionDefinitionProvider.cs +++ b/modules/permission-management/test/Volo.Abp.PermissionManagement.TestBase/Volo/Abp/PermissionManagement/TestPermissionDefinitionProvider.cs @@ -10,6 +10,7 @@ namespace Volo.Abp.PermissionManagement var testGroup = context.AddGroup("TestGroup"); testGroup.AddPermission("MyPermission1"); + testGroup.AddPermission("MyDisabledPermission1", isEnabled: false); var myPermission2 = testGroup.AddPermission("MyPermission2"); myPermission2.AddChild("MyPermission2.ChildPermission1");