Browse Source

RequiredPolicy reborn and less demanding as FallbackPolicy (#9759)

Hao Kung 6 years ago
parent
commit
0425808b70

+ 1 - 1
src/Mvc/Mvc.Core/test/Authorization/AuthorizeFilterTest.cs

@@ -251,7 +251,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
                 return Task.FromResult(policyName == "true" ? _true : _false);
             }
 
-            public Task<AuthorizationPolicy> GetRequiredPolicyAsync()
+            public Task<AuthorizationPolicy> GetFallbackPolicyAsync()
                 => Task.FromResult<AuthorizationPolicy>(null);
         }
 

+ 1 - 1
src/Mvc/test/Mvc.IntegrationTests/AuthorizeFilterIntegrationTest.cs

@@ -209,7 +209,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
                 return Task.FromResult(new AuthorizationPolicy(requirements, new string[] { }));
             }
 
-            public Task<AuthorizationPolicy> GetRequiredPolicyAsync()
+            public Task<AuthorizationPolicy> GetFallbackPolicyAsync()
             {
                 return Task.FromResult<AuthorizationPolicy>(null);
             }

+ 4 - 0
src/Security/Authorization/Core/ref/Microsoft.AspNetCore.Authorization.netcoreapp3.0.cs

@@ -45,12 +45,14 @@ namespace Microsoft.AspNetCore.Authorization
     public partial class AuthorizationMiddleware
     {
         public AuthorizationMiddleware(Microsoft.AspNetCore.Http.RequestDelegate next, Microsoft.AspNetCore.Authorization.IAuthorizationPolicyProvider policyProvider) { }
+        [System.Diagnostics.DebuggerStepThroughAttribute]
         public System.Threading.Tasks.Task Invoke(Microsoft.AspNetCore.Http.HttpContext context) { throw null; }
     }
     public partial class AuthorizationOptions
     {
         public AuthorizationOptions() { }
         public Microsoft.AspNetCore.Authorization.AuthorizationPolicy DefaultPolicy { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
+        public Microsoft.AspNetCore.Authorization.AuthorizationPolicy FallbackPolicy { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public bool InvokeHandlersAfterFailure { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public void AddPolicy(string name, Microsoft.AspNetCore.Authorization.AuthorizationPolicy policy) { }
         public void AddPolicy(string name, System.Action<Microsoft.AspNetCore.Authorization.AuthorizationPolicyBuilder> configurePolicy) { }
@@ -130,6 +132,7 @@ namespace Microsoft.AspNetCore.Authorization
     {
         public DefaultAuthorizationPolicyProvider(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Authorization.AuthorizationOptions> options) { }
         public System.Threading.Tasks.Task<Microsoft.AspNetCore.Authorization.AuthorizationPolicy> GetDefaultPolicyAsync() { throw null; }
+        public System.Threading.Tasks.Task<Microsoft.AspNetCore.Authorization.AuthorizationPolicy> GetFallbackPolicyAsync() { throw null; }
         public virtual System.Threading.Tasks.Task<Microsoft.AspNetCore.Authorization.AuthorizationPolicy> GetPolicyAsync(string policyName) { throw null; }
     }
     public partial class DefaultAuthorizationService : Microsoft.AspNetCore.Authorization.IAuthorizationService
@@ -159,6 +162,7 @@ namespace Microsoft.AspNetCore.Authorization
     public partial interface IAuthorizationPolicyProvider
     {
         System.Threading.Tasks.Task<Microsoft.AspNetCore.Authorization.AuthorizationPolicy> GetDefaultPolicyAsync();
+        System.Threading.Tasks.Task<Microsoft.AspNetCore.Authorization.AuthorizationPolicy> GetFallbackPolicyAsync();
         System.Threading.Tasks.Task<Microsoft.AspNetCore.Authorization.AuthorizationPolicy> GetPolicyAsync(string policyName);
     }
     public partial interface IAuthorizationRequirement

+ 4 - 25
src/Security/Authorization/Core/src/Policy/AuthorizationMiddleware.cs → src/Security/Authorization/Core/src/AuthorizationMiddleware.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authentication;
@@ -23,21 +22,11 @@ namespace Microsoft.AspNetCore.Authorization
 
         public AuthorizationMiddleware(RequestDelegate next, IAuthorizationPolicyProvider policyProvider)
         {
-            if (next == null)
-            {
-                throw new ArgumentNullException(nameof(next));
-            }
-
-            if (policyProvider == null)
-            {
-                throw new ArgumentNullException(nameof(policyProvider));
-            }
-
-            _next = next;
-            _policyProvider = policyProvider;
+            _next = next ?? throw new ArgumentNullException(nameof(next));
+            _policyProvider = policyProvider ?? throw new ArgumentNullException(nameof(policyProvider));
         }
 
-        public Task Invoke(HttpContext context)
+        public async Task Invoke(HttpContext context)
         {
             if (context == null)
             {
@@ -49,18 +38,8 @@ namespace Microsoft.AspNetCore.Authorization
             // Flag to indicate to other systems, e.g. MVC, that authorization middleware was run for this request
             context.Items[AuthorizationMiddlewareInvokedKey] = AuthorizationMiddlewareInvokedValue;
 
-            var authorizeData = endpoint?.Metadata.GetOrderedMetadata<IAuthorizeData>();
-            if (authorizeData == null || authorizeData.Count() == 0)
-            {
-                return _next(context);
-            }
-
-            return EvaluatePolicy(context, endpoint, authorizeData);
-        }
-
-        private async Task EvaluatePolicy(HttpContext context, Endpoint endpoint, IEnumerable<IAuthorizeData> authorizeData)
-        {
             // IMPORTANT: Changes to authorization logic should be mirrored in MVC's AuthorizeFilter
+            var authorizeData = endpoint?.Metadata.GetOrderedMetadata<IAuthorizeData>() ?? Array.Empty<IAuthorizeData>();
             var policy = await AuthorizationPolicy.CombineAsync(_policyProvider, authorizeData);
             if (policy == null)
             {

+ 11 - 1
src/Security/Authorization/Core/src/AuthorizationOptions.cs

@@ -27,6 +27,16 @@ namespace Microsoft.AspNetCore.Authorization
         /// </remarks>
         public AuthorizationPolicy DefaultPolicy { get; set; } = new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build();
 
+        /// <summary>
+        /// Gets or sets the fallback authorization policy used by <see cref="AuthorizationPolicy.CombineAsync(IAuthorizationPolicyProvider, IEnumerable{IAuthorizeData})"/>
+        /// when no IAuthorizeData have been provided. As a result, the <see cref="AuthorizationMiddleware"/> uses the fallback policy
+        /// if there are no <see cref="IAuthorizeData"/> instances for a resource. If a resource has any <see cref="IAuthorizeData"/>
+        /// then they are evaluated instead of the fallback policy. By default the fallback policy is null, and usually will have no 
+        /// effect unless you have the <see cref="AuthorizationMiddleware"/> middleware in your pipeline. It is not used in any way by the 
+        /// default <see cref="IAuthorizationService"/>.
+        /// </summary>
+        public AuthorizationPolicy FallbackPolicy { get; set; }
+
         /// <summary>
         /// Add an authorization policy with the provided name.
         /// </summary>
@@ -84,4 +94,4 @@ namespace Microsoft.AspNetCore.Authorization
             return PolicyMap.ContainsKey(name) ? PolicyMap[name] : null;
         }
     }
-}
+}

+ 11 - 1
src/Security/Authorization/Core/src/AuthorizationPolicy.cs

@@ -176,7 +176,17 @@ namespace Microsoft.AspNetCore.Authorization
                 }
             }
 
+            // If we have no policy by now, use the fallback policy if we have one
+            if (policyBuilder == null)
+            {
+                var fallbackPolicy = await policyProvider.GetFallbackPolicyAsync();
+                if (fallbackPolicy != null)
+                {
+                    return fallbackPolicy;
+                }
+            }
+
             return policyBuilder?.Build();
         }
     }
-}
+}

+ 10 - 0
src/Security/Authorization/Core/src/DefaultAuthorizationPolicyProvider.cs

@@ -15,6 +15,7 @@ namespace Microsoft.AspNetCore.Authorization
     {
         private readonly AuthorizationOptions _options;
         private Task<AuthorizationPolicy> _cachedDefaultPolicy;
+        private Task<AuthorizationPolicy> _cachedFallbackPolicy;
 
         /// <summary>
         /// Creates a new instance of <see cref="DefaultAuthorizationPolicyProvider"/>.
@@ -39,6 +40,15 @@ namespace Microsoft.AspNetCore.Authorization
             return GetCachedPolicy(ref _cachedDefaultPolicy, _options.DefaultPolicy);
         }
 
+        /// <summary>
+        /// Gets the fallback authorization policy.
+        /// </summary>
+        /// <returns>The fallback authorization policy.</returns>
+        public Task<AuthorizationPolicy> GetFallbackPolicyAsync()
+        {
+            return GetCachedPolicy(ref _cachedFallbackPolicy, _options.FallbackPolicy);
+        }
+
         private Task<AuthorizationPolicy> GetCachedPolicy(ref Task<AuthorizationPolicy> cachedPolicy, AuthorizationPolicy currentPolicy)
         {
             var local = cachedPolicy;

+ 6 - 0
src/Security/Authorization/Core/src/IAuthorizationPolicyProvider.cs

@@ -22,5 +22,11 @@ namespace Microsoft.AspNetCore.Authorization
         /// </summary>
         /// <returns>The default authorization policy.</returns>
         Task<AuthorizationPolicy> GetDefaultPolicyAsync();
+
+        /// <summary>
+        /// Gets the fallback authorization policy.
+        /// </summary>
+        /// <returns>The fallback authorization policy.</returns>
+        Task<AuthorizationPolicy> GetFallbackPolicyAsync();
     }
 }

+ 66 - 0
src/Security/Authorization/test/AuthorizationMiddlewareTests.cs

@@ -40,6 +40,25 @@ namespace Microsoft.AspNetCore.Authorization.Test
             Assert.True(next.Called);
         }
 
+        [Fact]
+        public async Task NoEndpointWithFallback_AnonymousUser_Challenges()
+        {
+            // Arrange
+            var policy = new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build();
+            var policyProvider = new Mock<IAuthorizationPolicyProvider>();
+            policyProvider.Setup(p => p.GetFallbackPolicyAsync()).ReturnsAsync(policy);
+            var next = new TestRequestDelegate();
+
+            var middleware = CreateMiddleware(next.Invoke, policyProvider.Object);
+            var context = GetHttpContext(anonymous: true);
+
+            // Act
+            await middleware.Invoke(context);
+
+            // Assert
+            Assert.False(next.Called);
+        }
+
         [Fact]
         public async Task HasEndpointWithoutAuth_AnonymousUser_Allows()
         {
@@ -59,6 +78,47 @@ namespace Microsoft.AspNetCore.Authorization.Test
             Assert.True(next.Called);
         }
 
+        [Fact]
+        public async Task HasEndpointWithFallbackWithoutAuth_AnonymousUser_Challenges()
+        {
+            // Arrange
+            var policy = new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build();
+            var policyProvider = new Mock<IAuthorizationPolicyProvider>();
+            policyProvider.Setup(p => p.GetDefaultPolicyAsync()).ReturnsAsync(policy);
+            policyProvider.Setup(p => p.GetFallbackPolicyAsync()).ReturnsAsync(policy);
+            var next = new TestRequestDelegate();
+
+            var middleware = CreateMiddleware(next.Invoke, policyProvider.Object);
+            var context = GetHttpContext(anonymous: true, endpoint: CreateEndpoint());
+
+            // Act
+            await middleware.Invoke(context);
+
+            // Assert
+            Assert.False(next.Called);
+        }
+
+        [Fact]
+        public async Task HasEndpointWithOnlyFallbackAuth_AnonymousUser_Allows()
+        {
+            // Arrange
+            var policy = new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build();
+            var policyProvider = new Mock<IAuthorizationPolicyProvider>();
+            policyProvider.Setup(p => p.GetDefaultPolicyAsync()).ReturnsAsync(new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build());
+            policyProvider.Setup(p => p.GetFallbackPolicyAsync()).ReturnsAsync(policy);
+            var next = new TestRequestDelegate();
+            var authenticationService = new TestAuthenticationService();
+
+            var middleware = CreateMiddleware(next.Invoke, policyProvider.Object);
+            var context = GetHttpContext(anonymous: true, endpoint: CreateEndpoint(new AuthorizeAttribute()), authenticationService: authenticationService);
+
+            // Act
+            await middleware.Invoke(context);
+
+            // Assert
+            Assert.True(next.Called);
+        }
+
         [Fact]
         public async Task HasEndpointWithAuth_AnonymousUser_Challenges()
         {
@@ -108,8 +168,11 @@ namespace Microsoft.AspNetCore.Authorization.Test
             var policy = new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build();
             var policyProvider = new Mock<IAuthorizationPolicyProvider>();
             var getPolicyCount = 0;
+            var getFallbackPolicyCount = 0;
             policyProvider.Setup(p => p.GetPolicyAsync(It.IsAny<string>())).ReturnsAsync(policy)
                 .Callback(() => getPolicyCount++);
+            policyProvider.Setup(p => p.GetFallbackPolicyAsync()).ReturnsAsync(policy)
+                .Callback(() => getFallbackPolicyCount++);
             var next = new TestRequestDelegate();
             var middleware = CreateMiddleware(next.Invoke, policyProvider.Object);
             var context = GetHttpContext(anonymous: true, endpoint: CreateEndpoint(new AuthorizeAttribute("whatever")));
@@ -117,14 +180,17 @@ namespace Microsoft.AspNetCore.Authorization.Test
             // Act & Assert
             await middleware.Invoke(context);
             Assert.Equal(1, getPolicyCount);
+            Assert.Equal(0, getFallbackPolicyCount);
             Assert.Equal(1, next.CalledCount);
 
             await middleware.Invoke(context);
             Assert.Equal(2, getPolicyCount);
+            Assert.Equal(0, getFallbackPolicyCount);
             Assert.Equal(2, next.CalledCount);
 
             await middleware.Invoke(context);
             Assert.Equal(3, getPolicyCount);
+            Assert.Equal(0, getFallbackPolicyCount);
             Assert.Equal(3, next.CalledCount);
         }
 

+ 2 - 2
src/Security/Authorization/test/DefaultAuthorizationServiceTests.cs

@@ -1025,7 +1025,7 @@ namespace Microsoft.AspNetCore.Authorization.Test
                 return Task.FromResult(new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build());
             }
 
-            public Task<AuthorizationPolicy> GetRequiredPolicyAsync()
+            public Task<AuthorizationPolicy> GetFallbackPolicyAsync()
             {
                 return Task.FromResult<AuthorizationPolicy>(null);
             }
@@ -1064,7 +1064,7 @@ namespace Microsoft.AspNetCore.Authorization.Test
                 return Task.FromResult(new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build());
             }
 
-            public Task<AuthorizationPolicy> GetRequiredPolicyAsync()
+            public Task<AuthorizationPolicy> GetFallbackPolicyAsync()
             {
                 return Task.FromResult<AuthorizationPolicy>(null);
             }

+ 2 - 0
src/Security/samples/CustomPolicyProvider/Authorization/MinimumAgePolicyProvider.cs

@@ -26,6 +26,8 @@ namespace CustomPolicyProvider
         }
 
         public Task<AuthorizationPolicy> GetDefaultPolicyAsync() => FallbackPolicyProvider.GetDefaultPolicyAsync();
+        
+        public Task<AuthorizationPolicy> GetFallbackPolicyAsync() => FallbackPolicyProvider.GetFallbackPolicyAsync();
 
         // Policies are looked up by string name, so expect 'parameters' (like age)
         // to be embedded in the policy names. This is abstracted away from developers