Merge pull request #2659 from mperk/#214-authorization-improvements

#214 authorization improvements

framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/MethodInvocationAuthorizationService.cs

@@ -3,26 +3,21 @@ using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authorization;
-using Volo.Abp.Clients;
 using Volo.Abp.DependencyInjection;
-using Volo.Abp.Users;
 
 namespace Volo.Abp.Authorization
 {
     public class MethodInvocationAuthorizationService : IMethodInvocationAuthorizationService, ITransientDependency
     {
-        private readonly IAuthorizationService _authorizationService;
-        private readonly ICurrentUser _currentUser;
-        private readonly ICurrentClient _currentClient;
+        private readonly IAbpAuthorizationPolicyProvider _abpAuthorizationPolicyProvider;
+        private readonly IAbpAuthorizationService _abpAuthorizationService;
 
         public MethodInvocationAuthorizationService(
-            IAuthorizationService authorizationService, 
-            ICurrentUser currentUser,
-            ICurrentClient currentClient)
+            IAbpAuthorizationPolicyProvider abpAuthorizationPolicyProvider,
+            IAbpAuthorizationService abpAuthorizationService)
         {
-            _authorizationService = authorizationService;
-            _currentUser = currentUser;
-            _currentClient = currentClient;
+            _abpAuthorizationPolicyProvider = abpAuthorizationPolicyProvider;
+            _abpAuthorizationService = abpAuthorizationService;
         }
 
         public async Task CheckAsync(MethodInvocationAuthorizationContext context)
@@ -32,10 +27,14 @@ namespace Volo.Abp.Authorization
                 return;
             }
 
-            foreach (var authorizationAttribute in GetAuthorizationDataAttributes(context.Method))
+            var authorizationPolicy = await AuthorizationPolicy.CombineAsync(_abpAuthorizationPolicyProvider,
+                GetAuthorizationDataAttributes(context.Method));
+            if (authorizationPolicy == null)
             {
-                await CheckAsync(authorizationAttribute);
+                return;
             }
+            
+            await _abpAuthorizationService.CheckAsync(authorizationPolicy);
         }
 
         protected virtual bool AllowAnonymous(MethodInvocationAuthorizationContext context)
@@ -49,7 +48,7 @@ namespace Volo.Abp.Authorization
                 .GetCustomAttributes(true)
                 .OfType<IAuthorizeData>();
 
-            if (methodInfo.IsPublic)
+            if (methodInfo.IsPublic && methodInfo.DeclaringType != null)
             {
                 attributes = attributes
                     .Union(
@@ -61,23 +60,5 @@ namespace Volo.Abp.Authorization
 
             return attributes;
         }
-
-        protected async Task CheckAsync(IAuthorizeData authorizationAttribute)
-        {
-            if (authorizationAttribute.Policy == null)
-            {
-                //TODO: Can we find a better, unified, way of checking if current request has been authenticated
-                if (!_currentUser.IsAuthenticated && !_currentClient.IsAuthenticated)
-                {
-                    throw new AbpAuthorizationException("Authorization failed! User has not logged in.");
-                }
-            }
-            else
-            {
-                await _authorizationService.CheckAsync(authorizationAttribute.Policy);
-            }
-
-            //TODO: What about roles and other props?
-        }
     }
 }

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

@@ -1,4 +1,10 @@
-using Volo.Abp.Testing;
+using Microsoft.Extensions.DependencyInjection;
+using NSubstitute;
+using System.Collections.Generic;
+using System.Security.Claims;
+using System.Threading;
+using Volo.Abp.Security.Claims;
+using Volo.Abp.Testing;
 
 namespace Volo.Abp.Authorization
 {
@@ -8,5 +14,20 @@ namespace Volo.Abp.Authorization
         {
             options.UseAutofac();
         }
+
+        protected override void AfterAddApplication(IServiceCollection services)
+        {
+            var claims = new List<Claim>() {
+                                new Claim(AbpClaimTypes.UserName, "Douglas"),
+                                new Claim(AbpClaimTypes.UserId, "1fcf46b2-28c3-48d0-8bac-fa53268a2775"),
+                                new Claim(AbpClaimTypes.Role, "MyRole")
+                            };
+
+            var identity = new ClaimsIdentity(claims);
+            var claimsPrincipal = new ClaimsPrincipal(identity);
+            var principalAccessor = Substitute.For<ICurrentPrincipalAccessor>();
+            principalAccessor.Principal.Returns(ci => claimsPrincipal);
+            Thread.CurrentPrincipal = claimsPrincipal;
+        }
     }
-}
+}

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

@@ -9,11 +9,13 @@ namespace Volo.Abp.Authorization
     public class Authorization_Tests : AuthorizationTestBase
     {
         private readonly IMyAuthorizedService1 _myAuthorizedService1;
+        private readonly IMyAuthorizedServiceWithRole _myAuthorizedServiceWithRole;
         private readonly IPermissionDefinitionManager _permissionDefinitionManager;
 
         public Authorization_Tests()
         {
             _myAuthorizedService1 = GetRequiredService<IMyAuthorizedService1>();
+            _myAuthorizedServiceWithRole = GetRequiredService<IMyAuthorizedServiceWithRole>();
             _permissionDefinitionManager = GetRequiredService<IPermissionDefinitionManager>();
         }
 
@@ -52,5 +54,31 @@ namespace Volo.Abp.Authorization
         {
             _permissionDefinitionManager.GetGroups().Count.ShouldBe(1);
         }
+
+        [Fact]
+        public async Task Should_Not_Allow_To_Call_Method_If_Has_No_Role_ProtectedByRole_Async()
+        {
+            await Assert.ThrowsAsync<AbpAuthorizationException>(async () =>
+            {
+                await _myAuthorizedServiceWithRole.ProtectedByAnotherRole().ConfigureAwait(false);
+            }).ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async Task Should_Allow_To_Call_Method_If_Has_No_Role_ProtectedByRole_Async()
+        {
+            int result = await _myAuthorizedServiceWithRole.ProtectedByRole().ConfigureAwait(false);
+            result.ShouldBe(42);
+        }
+
+
+        [Fact]
+        public async Task Should_Not_Allow_To_Call_Method_If_Has_No_Role_ProtectedByScheme_Async()
+        {
+            await Assert.ThrowsAsync<AbpAuthorizationException>(async () =>
+            {
+                await _myAuthorizedServiceWithRole.ProtectedByScheme().ConfigureAwait(false);
+            }).ConfigureAwait(false);
+        }
     }
 }

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

@@ -11,5 +11,6 @@ namespace Volo.Abp.Authorization.TestServices
         Task<int> ProtectedByClass();
 
         Task<int> ProtectedByClassAsync();
+
     }
 }

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

@@ -0,0 +1,13 @@
+using System.Threading.Tasks;
+
+namespace Volo.Abp.Authorization.TestServices
+{
+    public interface IMyAuthorizedServiceWithRole
+    {
+        Task<int> ProtectedByRole();
+
+        Task<int> ProtectedByScheme();
+
+        Task<int> ProtectedByAnotherRole();
+    }
+}

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

@@ -31,4 +31,4 @@ namespace Volo.Abp.Authorization.TestServices
             return 42;
         }
     }
-}
+}

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

@@ -0,0 +1,27 @@
+using Microsoft.AspNetCore.Authorization;
+using System.Threading.Tasks;
+using Volo.Abp.DependencyInjection;
+
+namespace Volo.Abp.Authorization.TestServices
+{
+    [Authorize(Roles = "MyRole")]
+    public class MyAuthorizedServiceWithRole : IMyAuthorizedServiceWithRole, ITransientDependency
+    {
+        public virtual Task<int> ProtectedByRole()
+        {
+            return Task.FromResult(42);
+        }
+
+        [Authorize(Roles = "MyAnotherRole")]
+        public virtual Task<int> ProtectedByAnotherRole()
+        {
+            return Task.FromResult(42);
+        }
+
+        [Authorize(AuthenticationSchemes = "Bearer")]
+        public virtual Task<int> ProtectedByScheme()
+        {
+            return Task.FromResult(42);
+        }
+    }
+}