Make AuthorizeFilter constructable

* Make AuthorizeFilter constructable

Fixes #5253

src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs

@@ -3,11 +3,14 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Security.Claims;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Mvc.Core;
 using Microsoft.AspNetCore.Mvc.Filters;
+using Microsoft.AspNetCore.Mvc.Internal;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Internal;
 
@@ -18,7 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
     /// <see cref="AuthorizationPolicy"/>. MVC recognizes the <see cref="AuthorizeAttribute"/> and adds an instance of
     /// this filter to the associated action or controller.
     /// </summary>
-    public class AuthorizeFilter : IAsyncAuthorizationFilter
+    public class AuthorizeFilter : IAsyncAuthorizationFilter, IFilterFactory
     {
         /// <summary>
         /// Initialize a new <see cref="AuthorizeFilter"/> instance.
@@ -30,6 +33,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
             {
                 throw new ArgumentNullException(nameof(policy));
             }
+
             Policy = policy;
         }
 
@@ -39,20 +43,39 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
         /// <param name="policyProvider">The <see cref="IAuthorizationPolicyProvider"/> to use to resolve policy names.</param>
         /// <param name="authorizeData">The <see cref="IAuthorizeData"/> to combine into an <see cref="IAuthorizeData"/>.</param>
         public AuthorizeFilter(IAuthorizationPolicyProvider policyProvider, IEnumerable<IAuthorizeData> authorizeData)
+            : this(authorizeData)
         {
             if (policyProvider == null)
             {
                 throw new ArgumentNullException(nameof(policyProvider));
             }
+
+            PolicyProvider = policyProvider;
+        }
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="AuthorizeFilter"/>.
+        /// </summary>
+        /// <param name="authorizeData">The <see cref="IAuthorizeData"/> to combine into an <see cref="IAuthorizeData"/>.</param>
+        public AuthorizeFilter(IEnumerable<IAuthorizeData> authorizeData)
+        {
             if (authorizeData == null)
             {
                 throw new ArgumentNullException(nameof(authorizeData));
             }
 
-            PolicyProvider = policyProvider;
             AuthorizeData = authorizeData;
         }
 
+        /// <summary>
+        /// Initializes a new instance of <see cref="AuthorizeFilter"/>.
+        /// </summary>
+        /// <param name="policy">The name of the policy to require for authorization.</param>
+        public AuthorizeFilter(string policy)
+            : this(new[] { new AuthorizeAttribute(policy) })
+        {
+        }
+
         /// <summary>
         /// The <see cref="IAuthorizationPolicyProvider"/> to use to resolve policy names.
         /// </summary>
@@ -64,11 +87,16 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
         public IEnumerable<IAuthorizeData> AuthorizeData { get; }
 
         /// <summary>
-        /// Gets the authorization policy to be used.  If null, the policy will be constructed via
+        /// Gets the authorization policy to be used.
-        /// AuthorizePolicy.CombineAsync(PolicyProvider, AuthorizeData)
         /// </summary>
+        /// <remarks>
+        /// If<c>null</c>, the policy will be constructed using
+        /// <see cref="AuthorizationPolicy.CombineAsync(IAuthorizationPolicyProvider, IEnumerable{IAuthorizeData})"/>.
+        /// </remarks>
         public AuthorizationPolicy Policy { get; private set; }
 
+        bool IFilterFactory.IsReusable => true;
+
         /// <inheritdoc />
         public virtual async Task OnAuthorizationAsync(AuthorizationFilterContext context)
         {
@@ -77,18 +105,32 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
                 throw new ArgumentNullException(nameof(context));
             }
 
-            var effectivePolicy = Policy ?? await AuthorizationPolicy.CombineAsync(PolicyProvider, AuthorizeData);
+            var effectivePolicy = Policy;
+            if (effectivePolicy == null)
+            {
+                if (PolicyProvider == null)
+                {
+                    throw new InvalidOperationException(
+                        Resources.FormatAuthorizeFilter_AuthorizationPolicyCannotBeCreated(
+                            nameof(AuthorizationPolicy),
+                            nameof(IAuthorizationPolicyProvider)));
+                }
+
+                effectivePolicy = await AuthorizationPolicy.CombineAsync(PolicyProvider, AuthorizeData);
+            }
+
             if (effectivePolicy == null)
             {
                 return;
             }
 
             // Build a ClaimsPrincipal with the Policy's required authentication types
-            if (effectivePolicy.AuthenticationSchemes != null && effectivePolicy.AuthenticationSchemes.Any())
+            if (effectivePolicy.AuthenticationSchemes != null && effectivePolicy.AuthenticationSchemes.Count > 0)
             {
                 ClaimsPrincipal newPrincipal = null;
-                foreach (var scheme in effectivePolicy.AuthenticationSchemes)
+                for (var i = 0; i < effectivePolicy.AuthenticationSchemes.Count; i++)
                 {
+                    var scheme = effectivePolicy.AuthenticationSchemes[i];
                     var result = await context.HttpContext.Authentication.AuthenticateAsync(scheme);
                     if (result != null)
                     {
@@ -118,5 +160,18 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
                 context.Result = new ChallengeResult(effectivePolicy.AuthenticationSchemes.ToArray());
             }
         }
+
+        IFilterMetadata IFilterFactory.CreateInstance(IServiceProvider serviceProvider)
+        {
+            if (Policy != null || PolicyProvider != null)
+            {
+                // The filter is fully constructed. Use the current instance to authorize.
+                return this;
+            }
+
+            Debug.Assert(AuthorizeData != null);
+            var policyProvider = serviceProvider.GetRequiredService<IAuthorizationPolicyProvider>();
+            return AuthorizationApplicationModelProvider.GetFilter(policyProvider, AuthorizeData);
+        }
     }
 }

src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizationApplicationModelProvider.cs

@@ -38,7 +38,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 var controllerModelAuthData = controllerModel.Attributes.OfType<IAuthorizeData>().ToArray();
                 if (controllerModelAuthData.Length > 0)
                 {
-                    controllerModel.Filters.Add(GetFilter(controllerModelAuthData));
+                    controllerModel.Filters.Add(GetFilter(_policyProvider, controllerModelAuthData));
                 }
                 foreach (var attribute in controllerModel.Attributes.OfType<IAllowAnonymous>())
                 {
@@ -50,7 +50,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                     var actionModelAuthData = actionModel.Attributes.OfType<IAuthorizeData>().ToArray();
                     if (actionModelAuthData.Length > 0)
                     {
-                        actionModel.Filters.Add(GetFilter(actionModelAuthData));
+                        actionModel.Filters.Add(GetFilter(_policyProvider, actionModelAuthData));
                     }
 
                     foreach (var attribute in actionModel.Attributes.OfType<IAllowAnonymous>())
@@ -61,18 +61,18 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             }
         }
 
-        private AuthorizeFilter GetFilter(IEnumerable<IAuthorizeData> authData)
+        public static AuthorizeFilter GetFilter(IAuthorizationPolicyProvider policyProvider, IEnumerable<IAuthorizeData> authData)
         {
             // The default policy provider will make the same policy for given input, so make it only once.
             // This will always execute syncronously.
-            if (_policyProvider.GetType() == typeof(DefaultAuthorizationPolicyProvider))
+            if (policyProvider.GetType() == typeof(DefaultAuthorizationPolicyProvider))
             {
-                var policy = AuthorizationPolicy.CombineAsync(_policyProvider, authData).GetAwaiter().GetResult();
+                var policy = AuthorizationPolicy.CombineAsync(policyProvider, authData).GetAwaiter().GetResult();
                 return new AuthorizeFilter(policy);
             }
             else
             {
-                return new AuthorizeFilter(_policyProvider, authData);
+                return new AuthorizeFilter(policyProvider, authData);
             }
         }
     }

src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs

@@ -1211,7 +1211,7 @@ namespace Microsoft.AspNetCore.Mvc.Core
         }
 
         /// <summary>
-        /// Having multiple overloads of method '{0}' is not supported.
+        /// Multiple overloads of method '{0}' are not supported.
         /// </summary>
         internal static string MiddewareFilter_ConfigureMethodOverload
         {
@@ -1219,7 +1219,7 @@ namespace Microsoft.AspNetCore.Mvc.Core
         }
 
         /// <summary>
-        /// Having multiple overloads of method '{0}' is not supported.
+        /// Multiple overloads of method '{0}' are not supported.
         /// </summary>
         internal static string FormatMiddewareFilter_ConfigureMethodOverload(object p0)
         {
@@ -1259,7 +1259,7 @@ namespace Microsoft.AspNetCore.Mvc.Core
         }
 
         /// <summary>
-        /// '{0}' property cannot be null.
+        /// The '{0}' property cannot be null.
         /// </summary>
         internal static string MiddlewareFilterBuilder_NullApplicationBuilder
         {
@@ -1267,7 +1267,7 @@ namespace Microsoft.AspNetCore.Mvc.Core
         }
 
         /// <summary>
-        /// '{0}' property cannot be null.
+        /// The '{0}' property cannot be null.
         /// </summary>
         internal static string FormatMiddlewareFilterBuilder_NullApplicationBuilder(object p0)
         {
@@ -1306,6 +1306,22 @@ namespace Microsoft.AspNetCore.Mvc.Core
             return string.Format(CultureInfo.CurrentCulture, GetString("MiddlewareFilter_ServiceResolutionFail"), p0, p1, p2, p3);
         }
 
+        /// <summary>
+        /// An {0} cannot be created without a valid instance of {1}.
+        /// </summary>
+        internal static string AuthorizeFilter_AuthorizationPolicyCannotBeCreated
+        {
+            get { return GetString("AuthorizeFilter_AuthorizationPolicyCannotBeCreated"); }
+        }
+
+        /// <summary>
+        /// An {0} cannot be created without a valid instance of {1}.
+        /// </summary>
+        internal static string FormatAuthorizeFilter_AuthorizationPolicyCannotBeCreated(object p0, object p1)
+        {
+            return string.Format(CultureInfo.CurrentCulture, GetString("AuthorizeFilter_AuthorizationPolicyCannotBeCreated"), p0, p1);
+        }
+
         private static string GetString(string name, params string[] formatterNames)
         {
             var value = _resourceManager.GetString(name);

src/Microsoft.AspNetCore.Mvc.Core/Resources.resx

@@ -370,4 +370,7 @@
   <data name="MiddlewareFilter_ServiceResolutionFail" xml:space="preserve">
     <value>Could not resolve a service of type '{0}' for the parameter '{1}' of method '{2}' on type '{3}'.</value>
   </data>
+  <data name="AuthorizeFilter_AuthorizationPolicyCannotBeCreated" xml:space="preserve">
+    <value>An {0} cannot be created without a valid instance of {1}.</value>
+  </data>
 </root>

test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs

@@ -26,6 +26,36 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
             Assert.True(authorizationContext.HttpContext.User.Identities.Any(i => i.IsAuthenticated));
         }
 
+        [Fact]
+        public async Task AuthorizeFilter_CreatedWithAuthorizeData_ThrowsWhenOnAuthorizationAsyncIsCalled()
+        {
+            // Arrange
+            var authorizeFilter = new AuthorizeFilter(new[] { new AuthorizeAttribute() });
+            var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization());
+            var expected = "An AuthorizationPolicy cannot be created without a valid instance of " +
+                "IAuthorizationPolicyProvider.";
+
+            // Act & Assert
+            var ex = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => authorizeFilter.OnAuthorizationAsync(authorizationContext));
+            Assert.Equal(expected, ex.Message);
+        }
+
+        [Fact]
+        public async Task AuthorizeFilter_CreatedWithPolicy_ThrowsWhenOnAuthorizationAsyncIsCalled()
+        {
+            // Arrange
+            var authorizeFilter = new AuthorizeFilter(new[] { new AuthorizeAttribute() });
+            var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization());
+            var expected = "An AuthorizationPolicy cannot be created without a valid instance of " +
+                "IAuthorizationPolicyProvider.";
+
+            // Act & Assert
+            var ex = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => authorizeFilter.OnAuthorizationAsync(authorizationContext));
+            Assert.Equal(expected, ex.Message);
+        }
+
         [Fact]
         public async Task AuthorizeFilterCanAuthorizeNonAuthenticatedUser()
         {
@@ -333,7 +363,98 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
             Assert.NotNull(authorizationContext.Result);
         }
 
-        private Filters.AuthorizationFilterContext GetAuthorizationContext(
+        [Fact]
+        public void CreateInstance_ReturnsSelfIfPolicyIsSet()
+        {
+            // Arrange
+            var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder()
+                .RequireAssertion(_ => true)
+                .Build());
+            var factory = (IFilterFactory)authorizeFilter;
+
+            // Act
+            var result = factory.CreateInstance(new ServiceCollection().BuildServiceProvider());
+
+            // Assert
+            Assert.Same(authorizeFilter, result);
+        }
+
+        [Fact]
+        public void CreateInstance_ReturnsSelfIfPolicyProviderIsSet()
+        {
+            // Arrange
+            var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder()
+                .RequireAssertion(_ => true)
+                .Build());
+            var factory = (IFilterFactory)authorizeFilter;
+
+            // Act
+            var result = factory.CreateInstance(new ServiceCollection().BuildServiceProvider());
+
+            // Assert
+            Assert.Same(authorizeFilter, result);
+        }
+
+        public static TheoryData AuthorizeFiltersCreatedWithoutPolicyOrPolicyProvider
+        {
+            get
+            {
+                return new TheoryData<AuthorizeFilter>
+                {
+                    new AuthorizeFilter(new[] { new AuthorizeAttribute()}),
+                    new AuthorizeFilter("some-policy"),
+                };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(AuthorizeFiltersCreatedWithoutPolicyOrPolicyProvider))]
+        public void CreateInstance_ReturnsNewFilterIfPolicyAndPolicyProviderAreNotSet(AuthorizeFilter authorizeFilter)
+        {
+            // Arrange
+            var factory = (IFilterFactory)authorizeFilter;
+            var serviceProvider = new ServiceCollection()
+                .AddOptions()
+                .AddAuthorization(options =>
+                {
+                    var policy = new AuthorizationPolicyBuilder()
+                        .RequireAssertion(_ => true)
+                        .Build();
+                    options.AddPolicy("some-policy", policy);
+                })
+                .BuildServiceProvider();
+
+            // Act
+            var result = factory.CreateInstance(serviceProvider);
+
+            // Assert
+            Assert.NotSame(authorizeFilter, result);
+            var actual = Assert.IsType<AuthorizeFilter>(result);
+            Assert.NotNull(actual.Policy);
+        }
+
+        [Theory]
+        [MemberData(nameof(AuthorizeFiltersCreatedWithoutPolicyOrPolicyProvider))]
+        public void CreateInstance_ReturnsNewFilterIfPolicyAndPolicyProviderAreNotSetAndCustomProviderIsUsed(
+            AuthorizeFilter authorizeFilter)
+        {
+            // Arrange
+            var factory = (IFilterFactory)authorizeFilter;
+            var policyProvider = Mock.Of<IAuthorizationPolicyProvider>();
+            var serviceProvider = new ServiceCollection()
+                .AddSingleton(policyProvider)
+                .BuildServiceProvider();
+
+            // Act
+            var result = factory.CreateInstance(serviceProvider);
+
+            // Assert
+            Assert.NotSame(authorizeFilter, result);
+            var actual = Assert.IsType<AuthorizeFilter>(result);
+            Assert.Same(policyProvider, actual.PolicyProvider);
+        }
+
+        private AuthorizationFilterContext GetAuthorizationContext(
             Action<ServiceCollection> registerServices,
             bool anonymous = false)
         {