Merge pull request #848 from abpframework/feature-system

Refactored permission checker

framework/src/Volo.Abp.AspNetCore.Mvc.Client/Volo/Abp/AspNetCore/Mvc/Client/RemotePermissionChecker.cs

@@ -14,19 +14,17 @@ namespace Volo.Abp.AspNetCore.Mvc.Client
             ConfigurationClient = configurationClient;
         }
 
-        public async Task<PermissionGrantInfo> CheckAsync(string name)
+        public async Task<bool> IsGrantedAsync(string name)
         {
             var configuration = await ConfigurationClient.GetAsync();
 
-            return new PermissionGrantInfo(
+            return configuration.Auth.GrantedPolicies.ContainsKey(name);
-                name,
-                configuration.Auth.GrantedPolicies.ContainsKey(name)
-            );
         }
 
-        public Task<PermissionGrantInfo> CheckAsync(ClaimsPrincipal claimsPrincipal, string name)
+        public Task<bool> IsGrantedAsync(ClaimsPrincipal claimsPrincipal, string name)
         {
-            return CheckAsync(name);
+            /* This provider always works for the current principal. */
+            return IsGrantedAsync(name);
         }
     }
 }

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

@@ -1,5 +1,6 @@
 using System.Security.Claims;
 using System.Threading.Tasks;
+using Volo.Abp.Threading;
 
 namespace Volo.Abp.Authorization.Permissions
 {
@@ -11,14 +12,14 @@ namespace Volo.Abp.Authorization.Permissions
     /// </summary>
     public class AlwaysAllowPermissionChecker : IPermissionChecker
     {
-        public Task<PermissionGrantInfo> CheckAsync(string name)
+        public Task<bool> IsGrantedAsync(string name)
         {
-            return Task.FromResult(new PermissionGrantInfo(name, true, "AlwaysAllow"));
+            return TaskCache.TrueResult;
         }
 
-        public Task<PermissionGrantInfo> CheckAsync(ClaimsPrincipal claimsPrincipal, string name)
+        public Task<bool> IsGrantedAsync(ClaimsPrincipal claimsPrincipal, string name)
         {
-            return Task.FromResult(new PermissionGrantInfo(name, true, "AlwaysAllow"));
+            return TaskCache.TrueResult;
         }
     }
 }

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

@@ -15,21 +15,18 @@ namespace Volo.Abp.Authorization.Permissions
 
         }
 
-        public override async Task<PermissionValueProviderGrantInfo> CheckAsync(PermissionValueCheckContext context)
+        public override async Task<PermissionGrantResult> CheckAsync(PermissionValueCheckContext context)
         {
             var clientId = context.Principal?.FindFirst(AbpClaimTypes.ClientId)?.Value;
 
             if (clientId == null)
             {
-                return PermissionValueProviderGrantInfo.NonGranted;
+                return PermissionGrantResult.Undefined;
             }
 
-            if (await PermissionStore.IsGrantedAsync(context.Permission.Name, Name, clientId))
+            return await PermissionStore.IsGrantedAsync(context.Permission.Name, Name, clientId)
-            {
+                ? PermissionGrantResult.Granted
-                return new PermissionValueProviderGrantInfo(true, clientId);
+                : PermissionGrantResult.Undefined;
-            }
-
-            return PermissionValueProviderGrantInfo.NonGranted;
         }
     }
 }

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

@@ -6,8 +6,8 @@ namespace Volo.Abp.Authorization.Permissions
 {
     public interface IPermissionChecker
     {
-        Task<PermissionGrantInfo> CheckAsync([NotNull]string name);
+        Task<bool> IsGrantedAsync([NotNull]string name);
 
-        Task<PermissionGrantInfo> CheckAsync([CanBeNull] ClaimsPrincipal claimsPrincipal, [NotNull]string name);
+        Task<bool> IsGrantedAsync([CanBeNull] ClaimsPrincipal claimsPrincipal, [NotNull]string name);
     }
 }

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

@@ -7,6 +7,7 @@ namespace Volo.Abp.Authorization.Permissions
     {
         string Name { get; }
 
-        Task<PermissionValueProviderGrantInfo> CheckAsync(PermissionValueCheckContext context);
+        //TODO: Rename to GetResult? (CheckAsync throws exception by naming convention)
+        Task<PermissionGrantResult> CheckAsync(PermissionValueCheckContext context);
     }
 }

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

@@ -2,6 +2,7 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 using Volo.Abp.DependencyInjection;
+using Volo.Abp.Threading;
 
 namespace Volo.Abp.Authorization.Permissions
 {
@@ -16,7 +17,7 @@ namespace Volo.Abp.Authorization.Permissions
 
         public Task<bool> IsGrantedAsync(string name, string providerName, string providerKey)
         {
-            return Task.FromResult(false);
+            return TaskCache.FalseResult;
         }
     }
 }

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

@@ -41,12 +41,12 @@ namespace Volo.Abp.Authorization.Permissions
             );
         }
 
-        public virtual Task<PermissionGrantInfo> CheckAsync(string name)
+        public virtual Task<bool> IsGrantedAsync(string name)
         {
-            return CheckAsync(PrincipalAccessor.Principal, name);
+            return IsGrantedAsync(PrincipalAccessor.Principal, name);
         }
 
-        public virtual async Task<PermissionGrantInfo> CheckAsync(ClaimsPrincipal claimsPrincipal, string name)
+        public virtual async Task<bool> IsGrantedAsync(ClaimsPrincipal claimsPrincipal, string name)
         {
             Check.NotNull(name, nameof(name));
 
@@ -55,6 +55,8 @@ namespace Volo.Abp.Authorization.Permissions
                 claimsPrincipal
             );
 
+            var isGranted = false;
+
             foreach (var provider in ValueProviders)
             {
                 if (context.Permission.Providers.Any() &&
@@ -64,13 +66,18 @@ namespace Volo.Abp.Authorization.Permissions
                 }
 
                 var result = await provider.CheckAsync(context);
-                if (result.IsGranted)
+
+                if (result == PermissionGrantResult.Granted)
+                {
+                    isGranted = true;
+                }
+                else if (result == PermissionGrantResult.Prohibited)
                 {
-                    return new PermissionGrantInfo(context.Permission.Name, true, provider.Name, result.ProviderKey);
+                    return false;
                 }
             }
 
-            return new PermissionGrantInfo(context.Permission.Name, false);
+            return isGranted;
         }
     }
 }

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

@@ -1,20 +0,0 @@
-using System.Security.Claims;
-using System.Threading.Tasks;
-
-namespace Volo.Abp.Authorization.Permissions
-{
-    public static class PermissionCheckerExtensions
-    {
-        public static async Task<bool> IsGrantedAsync(this IPermissionChecker permissionChecker, string name)
-        {
-            return (await permissionChecker.CheckAsync(name)).IsGranted;
-        }
-
-        public static async Task<bool> IsGrantedAsync(this IPermissionChecker permissionChecker, ClaimsPrincipal principal, string name)
-        {
-            return (await permissionChecker.CheckAsync(principal, name)).IsGranted;
-        }
-
-        //TODO: Add sync extensions
-    }
-}

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

@@ -22,7 +22,7 @@ namespace Volo.Abp.Authorization.Permissions
         /// A list of allowed providers to get/set value of this permission.
         /// An empty list indicates that all providers are allowed.
         /// </summary>
-        public List<string> Providers { get; }
+        public List<string> Providers { get; } //TODO: Rename to AllowedProviders?
 
         public ILocalizableString DisplayName
         {
@@ -53,7 +53,9 @@ namespace Volo.Abp.Authorization.Permissions
             set => Properties[name] = value;
         }
 
-        protected internal PermissionDefinition([NotNull] string name, ILocalizableString displayName = null)
+        protected internal PermissionDefinition(
+            [NotNull] string name, 
+            ILocalizableString displayName = null)
         {
             Name = Check.NotNull(name, nameof(name));
             DisplayName = displayName ?? new FixedLocalizableString(name);
@@ -63,7 +65,9 @@ namespace Volo.Abp.Authorization.Permissions
             _children = new List<PermissionDefinition>();
         }
 
-        public virtual PermissionDefinition AddChild([NotNull] string name, ILocalizableString displayName = null)
+        public virtual PermissionDefinition AddChild(
+            [NotNull] string name, 
+            ILocalizableString displayName = null)
         {
             var child = new PermissionDefinition(name, displayName)
             {

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

@@ -0,0 +1,9 @@
+namespace Volo.Abp.Authorization.Permissions
+{
+    public enum PermissionGrantResult
+    {
+        Undefined,
+        Granted,
+        Prohibited
+    }
+}

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

@@ -37,7 +37,9 @@ namespace Volo.Abp.Authorization.Permissions
             set => Properties[name] = value;
         }
 
-        protected internal PermissionGroupDefinition(string name, ILocalizableString displayName = null)
+        protected internal PermissionGroupDefinition(
+            string name, 
+            ILocalizableString displayName = null)
         {
             Name = name;
             DisplayName = displayName ?? new FixedLocalizableString(Name);
@@ -46,7 +48,9 @@ namespace Volo.Abp.Authorization.Permissions
             _permissions = new List<PermissionDefinition>();
         }
 
-        public virtual PermissionDefinition AddPermission(string name, ILocalizableString displayName = null)
+        public virtual PermissionDefinition AddPermission(
+            string name, 
+            ILocalizableString displayName = null)
         {
             var permission = new PermissionDefinition(name, displayName);
 

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

@@ -11,7 +11,9 @@ namespace Volo.Abp.Authorization.Permissions
         [CanBeNull]
         public ClaimsPrincipal Principal { get; }
 
-        public PermissionValueCheckContext([NotNull] PermissionDefinition permission, [CanBeNull] ClaimsPrincipal principal)
+        public PermissionValueCheckContext(
+            [NotNull] PermissionDefinition permission, 
+            [CanBeNull] ClaimsPrincipal principal)
         {
             Check.NotNull(permission, nameof(permission));
 

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

@@ -13,6 +13,6 @@ namespace Volo.Abp.Authorization.Permissions
             PermissionStore = permissionStore;
         }
 
-        public abstract Task<PermissionValueProviderGrantInfo> CheckAsync(PermissionValueCheckContext context);
+        public abstract Task<PermissionGrantResult> CheckAsync(PermissionValueCheckContext context);
     }
 }

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

@@ -16,23 +16,23 @@ namespace Volo.Abp.Authorization.Permissions
 
         }
 
-        public override async Task<PermissionValueProviderGrantInfo> CheckAsync(PermissionValueCheckContext context)
+        public override async Task<PermissionGrantResult> CheckAsync(PermissionValueCheckContext context)
         {
             var roles = context.Principal?.FindAll(AbpClaimTypes.Role).Select(c => c.Value).ToArray();
             if (roles == null || !roles.Any())
             {
-                return PermissionValueProviderGrantInfo.NonGranted;
+                return PermissionGrantResult.Undefined;
             }
 
             foreach (var role in roles)
             {
                 if (await PermissionStore.IsGrantedAsync(context.Permission.Name, Name, role))
                 {
-                    return new PermissionValueProviderGrantInfo(true, role);
+                    return PermissionGrantResult.Granted;
                 }
             }
 
-            return PermissionValueProviderGrantInfo.NonGranted;
+            return PermissionGrantResult.Undefined;
         }
     }
 }

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

@@ -15,21 +15,18 @@ namespace Volo.Abp.Authorization.Permissions
 
         }
 
-        public override async Task<PermissionValueProviderGrantInfo> CheckAsync(PermissionValueCheckContext context)
+        public override async Task<PermissionGrantResult> CheckAsync(PermissionValueCheckContext context)
         {
             var userId = context.Principal?.FindFirst(AbpClaimTypes.UserId)?.Value;
 
             if (userId == null)
             {
-                return PermissionValueProviderGrantInfo.NonGranted;
+                return PermissionGrantResult.Undefined;
             }
 
-            if (await PermissionStore.IsGrantedAsync(context.Permission.Name, Name, userId))
+            return await PermissionStore.IsGrantedAsync(context.Permission.Name, Name, userId)
-            {
+                ? PermissionGrantResult.Granted
-                return new PermissionValueProviderGrantInfo(true, userId);
+                : PermissionGrantResult.Undefined;
-            }
-
-            return PermissionValueProviderGrantInfo.NonGranted;
         }
     }
 }

framework/src/Volo.Abp.Core/Volo/Abp/Threading/TaskCache.cs

@@ -0,0 +1,16 @@
+using System.Threading.Tasks;
+
+namespace Volo.Abp.Threading
+{
+    public static class TaskCache
+    {
+        public static Task<bool> TrueResult { get; }
+        public static Task<bool> FalseResult { get; }
+
+        static TaskCache()
+        {
+            TrueResult = Task.FromResult(true);
+            FalseResult = Task.FromResult(false);
+        }
+    }
+}

framework/src/Volo.Abp.MultiTenancy.Abstractions/Volo.Abp.MultiTenancy.Abstractions.csproj

@@ -14,7 +14,6 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <ProjectReference Include="..\Volo.Abp.Core\Volo.Abp.Core.csproj" />
     <ProjectReference Include="..\Volo.Abp.Data\Volo.Abp.Data.csproj" />
     <ProjectReference Include="..\Volo.Abp.Security\Volo.Abp.Security.csproj" />
   </ItemGroup>

framework/src/Volo.Abp.MultiTenancy.Abstractions/Volo/Abp/MultiTenancy/AbpMultiTenancyAbstractionsModule.cs

@@ -1,10 +1,12 @@
 using Volo.Abp.Data;
 using Volo.Abp.Modularity;
+using Volo.Abp.Security;
 
 namespace Volo.Abp.MultiTenancy
 {
     [DependsOn(
-        typeof(AbpDataModule)
+        typeof(AbpDataModule),
+        typeof(AbpSecurityModule)
         )]
     public class AbpMultiTenancyAbstractionsModule : AbpModule //TODO: Rename to AbpMultiTenancyModule?
     {

framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionValueProviderGrantInfo.cs → modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionValueProviderGrantInfo.cs

@@ -1,8 +1,8 @@
 using JetBrains.Annotations;
 
-namespace Volo.Abp.Authorization.Permissions
+namespace Volo.Abp.PermissionManagement
 {
-    public class PermissionValueProviderGrantInfo
+    public class PermissionValueProviderGrantInfo //TODO: Rename to PermissionGrantInfo
     {
         public static PermissionValueProviderGrantInfo NonGranted { get; } = new PermissionValueProviderGrantInfo(false);