Add CombineAuthorizeFilters option

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

@@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Mvc.Core;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.Internal;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Mvc.Authorization
 {
@@ -22,6 +23,9 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
     /// </summary>
     public class AuthorizeFilter : IAsyncAuthorizationFilter, IFilterFactory
     {
+        private MvcOptions _mvcOptions;
+        private AuthorizationPolicy _effectivePolicy;
+
         /// <summary>
         /// Initializes a new <see cref="AuthorizeFilter"/> instance.
         /// </summary>
@@ -104,15 +108,46 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
 
         bool IFilterFactory.IsReusable => true;
 
-        /// <inheritdoc />
+        private async Task<AuthorizationPolicy> GetEffectivePolicyAsync(AuthorizationFilterContext context)
-        public virtual async Task OnAuthorizationAsync(AuthorizationFilterContext context)
         {
-            if (context == null)
+            if (_effectivePolicy != null)
             {
-                throw new ArgumentNullException(nameof(context));
+                return _effectivePolicy;
             }
 
             var effectivePolicy = Policy;
+
+            if (_mvcOptions == null) 
+            {
+                _mvcOptions = context.HttpContext.RequestServices.GetRequiredService<IOptions<MvcOptions>>().Value;
+            }
+
+            if (_mvcOptions.CombineAuthorizeFilters)
+            {
+                if (!context.IsEffectivePolicy<AuthorizeFilter>(this))
+                {
+                    return null;
+                }
+
+                // Combine all authorize filters into single effective policy that's only run on the closest filter
+                AuthorizationPolicyBuilder builder = null;
+                for (var i = 0; i < context.Filters.Count; i++)
+                {
+                    if (ReferenceEquals(this, context.Filters[i]))
+                    {
+                        continue;
+                    }
+                    
+                    if (context.Filters[i] is AuthorizeFilter authorizeFilter)
+                    {
+                        builder = builder ?? new AuthorizationPolicyBuilder(effectivePolicy);
+                        builder.Combine(authorizeFilter.Policy);
+                    }
+                }
+
+                effectivePolicy = builder?.Build() ?? effectivePolicy;
+            }
+
             if (effectivePolicy == null)
             {
                 if (PolicyProvider == null)
@@ -126,6 +161,24 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
                 effectivePolicy = await AuthorizationPolicy.CombineAsync(PolicyProvider, AuthorizeData);
             }
 
+            // We can cache the effective policy when there is no custom policy provider 
+            if (PolicyProvider == null)
+            {
+                _effectivePolicy = effectivePolicy;
+            }
+
+            return effectivePolicy;
+        }
+
+        /// <inheritdoc />
+        public virtual async Task OnAuthorizationAsync(AuthorizationFilterContext context)
+        {
+            if (context == null)
+            {
+                throw new ArgumentNullException(nameof(context));
+            }
+
+            var effectivePolicy = await GetEffectivePolicyAsync(context);
             if (effectivePolicy == null)
             {
                 return;

src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs

@@ -5,6 +5,7 @@ using System;
 using System.Collections;
 using System.Collections.Generic;
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
+using Microsoft.AspNetCore.Mvc.Authorization;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.Formatters;
 using Microsoft.AspNetCore.Mvc.Infrastructure;
@@ -242,6 +243,13 @@ namespace Microsoft.AspNetCore.Mvc
         /// </summary>
         public bool RequireHttpsPermanent { get; set; }
 
+        /// <summary>
+        /// Gets or sets a value that determines if policies on instances of <see cref="AuthorizeFilter" />
+        /// will be combined into a single effective policy. This was always to be the intended behavior, 
+        /// but was not the case.
+        /// </summary>
+        public bool CombineAuthorizeFilters { get; set;}
+
         IEnumerator<ICompatibilitySwitch> IEnumerable<ICompatibilitySwitch>.GetEnumerator()
         {
             return ((IEnumerable<ICompatibilitySwitch>)_switches).GetEnumerator();

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

@@ -221,6 +221,68 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
             Assert.Null(authorizationContext.Result);
         }
 
+        [Fact]
+        public async Task AuthorizationFilterCombinesMultipleFilters()
+        {
+            // Arrange
+            var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAssertion(a => true).Build());
+            var authorizationContext = GetAuthorizationContext(anonymous: false, registerServices: s => s.Configure<MvcOptions>(o => o.CombineAuthorizeFilters = true));
+            // Effective policy should fail, if both are combined
+            authorizationContext.Filters.Add(authorizeFilter);
+            var secondFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAssertion(a => false).Build());
+            authorizationContext.Filters.Add(secondFilter);
+
+            // Act
+            await secondFilter.OnAuthorizationAsync(authorizationContext);
+
+            // Assert
+            Assert.IsType<ForbidResult>(authorizationContext.Result);
+        }
+
+        [Fact]
+        public async Task AuthorizationFilterIgnoresFirstFilterWhenCombining()
+        {
+            // Arrange
+            var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAssertion(a => false).Build());
+            var authorizationContext = GetAuthorizationContext(anonymous: false, registerServices: s => s.Configure<MvcOptions>(o => o.CombineAuthorizeFilters = true));
+            // Effective policy should fail, if both are combined
+            authorizationContext.Filters.Add(authorizeFilter);
+            var secondFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAssertion(a => false).Build());
+            authorizationContext.Filters.Add(secondFilter);
+
+            // Act
+            await authorizeFilter.OnAuthorizationAsync(authorizationContext);
+
+            // Assert
+            Assert.Null(authorizationContext.Result);
+        }
+
+        [Fact]
+        public async Task AuthorizationFilterCombinesDerivedFilters()
+        {
+            // Arrange
+            var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAssertion(a => true).Build());
+            var authorizationContext = GetAuthorizationContext(anonymous: false, registerServices: s => s.Configure<MvcOptions>(o => o.CombineAuthorizeFilters = true));
+            // Effective policy should fail, if both are combined
+            authorizationContext.Filters.Add(authorizeFilter);
+            authorizationContext.Filters.Add(new DerivedAuthorizeFilter());
+            authorizationContext.Filters.Add(new DerivedAuthorizeFilter());
+            var lastFilter = new DerivedAuthorizeFilter();
+            authorizationContext.Filters.Add(lastFilter);
+
+            // Act
+            await lastFilter.OnAuthorizationAsync(authorizationContext);
+
+            // Assert
+            Assert.IsType<ForbidResult>(authorizationContext.Result);
+        }
+    
+        public class DerivedAuthorizeFilter : AuthorizeFilter 
+        {
+            public DerivedAuthorizeFilter() : base(new AuthorizationPolicyBuilder().RequireAssertion(a => false).Build())
+            { }
+        }
+
         [Fact]
         public async Task AuthZResourceShouldBeAuthorizationFilterContext()
         {

test/Microsoft.AspNetCore.Mvc.FunctionalTests/CombineAuthorizeTests.cs

@@ -0,0 +1,43 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Linq;
+using System.Net;
+using System.Net.Http;
+using System.Threading.Tasks;
+using SecurityWebSite;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Mvc.FunctionalTests
+{
+    public class CombineAuthorizeTests : IClassFixture<MvcTestFixture<StartupWithGlobalAuthorizeAndCombineAuthorizeFilters>>
+    {
+        public CombineAuthorizeTests(MvcTestFixture<StartupWithGlobalAuthorizeAndCombineAuthorizeFilters> fixture)
+        {
+            Client = fixture.Client;
+        }
+
+        public HttpClient Client { get; }
+
+        [Fact]
+        public async Task CanAuthorizeWithOnlyCookie2()
+        {
+            // Arrange & Act
+            var response = await Client.PostAsync("http://localhost/Administration/SignInCookie2", null);
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.True(response.Headers.Contains("Set-Cookie"));
+
+            var cookie2 = response.Headers.GetValues("Set-Cookie").SingleOrDefault();
+
+            var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/Administration/EitherCookie");
+            request.Headers.Add("Cookie", cookie2);
+
+            response = await Client.SendAsync(request);
+            var body = await response.Content.ReadAsStringAsync();
+            Assert.Equal("Administration.EitherCookie:AuthorizeCount=1", body);
+         }
+    }
+}

test/Microsoft.AspNetCore.Mvc.FunctionalTests/GlobalAuthorizationFilterTest.cs

@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.Linq;
 using System.Net;
 using System.Net.Http;
 using System.Threading.Tasks;
@@ -42,5 +43,30 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
             var body = await response.Content.ReadAsStringAsync();
             Assert.Equal("Administration.AllowAnonymousAction", body);
         }
+
+        [Fact]
+        public async Task AuthorizationPoliciesDoNotCombineByDefault()
+        {
+            // Arrange & Act
+            var response = await Client.PostAsync("http://localhost/Administration/SignInCookie2", null);
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.True(response.Headers.Contains("Set-Cookie"));
+
+            var cookie2 = response.Headers.GetValues("Set-Cookie").SingleOrDefault();
+
+            var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/Administration/EitherCookie");
+            request.Headers.Add("Cookie", cookie2);
+
+            // Will fail because default cookie is not sent so [Authorize] fails
+            response = await Client.SendAsync(request);
+            Assert.Equal(HttpStatusCode.Redirect, response.StatusCode);
+            Assert.NotNull(response.Headers.Location);
+            Assert.Equal(
+                "http://localhost/Home/Login?ReturnUrl=%2FAdministration%2FEitherCookie",
+                response.Headers.Location.ToString());
+         }
+        
     }
 }

test/Microsoft.AspNetCore.Mvc.IntegrationTests/AuthorizeFilterIntegrationTest.cs

@@ -17,6 +17,7 @@ using Microsoft.AspNetCore.Routing;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using Microsoft.Extensions.ObjectPool;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Mvc.IntegrationTests
@@ -70,8 +71,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
             serviceCollection.AddMvc();
             serviceCollection
                 .AddTransient<ILoggerFactory, LoggerFactory>()
-                .AddTransient<ILogger<DefaultAuthorizationService>, Logger<DefaultAuthorizationService>>();
+                .AddTransient<ILogger<DefaultAuthorizationService>, Logger<DefaultAuthorizationService>>()
-
+                .AddSingleton<ObjectPoolProvider, DefaultObjectPoolProvider>();
             return serviceCollection.BuildServiceProvider();
         }
 

test/WebSites/SecurityWebSite/Controllers/AdministrationController.cs

@@ -1,8 +1,13 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.Security.Claims;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Authorization.Policy;
 using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.DependencyInjection;
 
 namespace SecurityWebSite.Controllers
 {
@@ -15,10 +20,27 @@ namespace SecurityWebSite.Controllers
             return Content("Administration.Index");
         }
 
+        // Either cookie should allow access to this action (if CombineAuthorizeFilters is true)
+        // If CombineAuthorizeFilters is false, the main cookie is required.
+        [Authorize(AuthenticationSchemes = "Cookie2")]
+        public IActionResult EitherCookie()
+        {
+            var countEvaluator = (CountingPolicyEvaluator)HttpContext.RequestServices.GetRequiredService<IPolicyEvaluator>();
+            return Content("Administration.EitherCookie:AuthorizeCount="+countEvaluator.AuthorizeCount);
+        }
+
         [AllowAnonymous]
         public IActionResult AllowAnonymousAction()
         {
             return Content("Administration.AllowAnonymousAction");
         }
+
+        [AllowAnonymous]
+        public async Task<IActionResult> SignInCookie2()
+        {
+            await HttpContext.SignInAsync("Cookie2", new ClaimsPrincipal(new ClaimsIdentity("Cookie2")));
+            return Content("SignInCookie2");
+        }
+
     }
 }

test/WebSites/SecurityWebSite/CountingPolicyEvaluator.cs

@@ -0,0 +1,24 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Authentication;
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Authorization.Policy;
+using Microsoft.AspNetCore.Http;
+
+namespace SecurityWebSite
+{
+    public class CountingPolicyEvaluator : PolicyEvaluator
+    {
+        public int AuthorizeCount { get; private set; }
+
+        public CountingPolicyEvaluator(IAuthorizationService authorization) : base(authorization) { }
+
+        public override Task<PolicyAuthorizationResult> AuthorizeAsync(AuthorizationPolicy policy, AuthenticateResult authenticationResult, HttpContext context, object resource) {
+            AuthorizeCount++;
+            return base.AuthorizeAsync(policy, authenticationResult, context, resource);
+        }
+    }
+}

test/WebSites/SecurityWebSite/SecurityWebSite.csproj

@@ -7,6 +7,7 @@
   <ItemGroup>
     <ProjectReference Include="..\..\..\src\Microsoft.AspNetCore.Mvc\Microsoft.AspNetCore.Mvc.csproj" />
 
+    <PackageReference Include="Microsoft.AspNetCore.Authorization.Policy" Version="$(MicrosoftAspNetCoreAuthorizationPolicyPackageVersion)" />
     <PackageReference Include="Microsoft.AspNetCore.Authentication.Cookies" Version="$(MicrosoftAspNetCoreAuthenticationCookiesPackageVersion)" />
     <PackageReference Include="Microsoft.AspNetCore.Server.Kestrel" Version="$(MicrosoftAspNetCoreServerKestrelPackageVersion)" />
     <PackageReference Include="Microsoft.AspNetCore.Hosting" Version="$(MicrosoftAspNetCoreHostingPackageVersion)" />

test/WebSites/SecurityWebSite/Startup.cs

@@ -1,8 +1,9 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Authentication.Cookies;
+using Microsoft.AspNetCore.Authorization.Policy;
+using Microsoft.AspNetCore.Builder;
 using Microsoft.Extensions.DependencyInjection;
 
 namespace SecurityWebSite
@@ -19,7 +20,9 @@ namespace SecurityWebSite
             {
                 options.LoginPath = "/Home/Login";
                 options.LogoutPath = "/Home/Logout";
-            });
+            }).AddCookie("Cookie2");
+
+            services.AddScoped<IPolicyEvaluator, CountingPolicyEvaluator>();
         }
 
         // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.

test/WebSites/SecurityWebSite/StartupWithGlobalAuthorizeAndCombineAuthorizeFilters.cs

@@ -0,0 +1,47 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Microsoft.AspNetCore.Authentication.Cookies;
+using Microsoft.AspNetCore.Authorization.Policy;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Mvc.Authorization;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace SecurityWebSite
+{
+    public class StartupWithGlobalAuthorizeAndCombineAuthorizeFilters
+    {
+        // This method gets called by the runtime. Use this method to add services to the container.
+        public void ConfigureServices(IServiceCollection services)
+        {
+            // Add framework services.
+            services.AddMvc(o =>
+            {
+                o.CombineAuthorizeFilters = true;
+                o.Filters.Add(new AuthorizeFilter());
+            });
+
+            services.AddAntiforgery();
+            services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme).AddCookie(options => 
+            {
+                options.LoginPath = "/Home/Login";
+                options.LogoutPath = "/Home/Logout";
+            }).AddCookie("Cookie2");
+
+            services.AddScoped<IPolicyEvaluator, CountingPolicyEvaluator>();
+        }
+
+        // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
+        public void Configure(IApplicationBuilder app)
+        {
+            app.UseAuthentication();
+
+            app.UseMvc(routes =>
+            {
+                routes.MapRoute(
+                    name: "default",
+                    template: "{controller=Home}/{action=Index}/{id?}");
+            });
+        }
+    }
+}

test/WebSites/SecurityWebSite/StartupWithGlobalDenyAnonymousFilter.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using Microsoft.AspNetCore.Authentication.Cookies;
+using Microsoft.AspNetCore.Authorization.Policy;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Mvc.Authorization;
 using Microsoft.Extensions.DependencyInjection;
@@ -18,12 +19,14 @@ namespace SecurityWebSite
                 {
                     options.LoginPath = "/Home/Login";
                     options.LogoutPath = "/Home/Logout";
-                });
+                }).AddCookie("Cookie2");
 
             services.AddMvc(o =>
             {
                 o.Filters.Add(new AuthorizeFilter());
             });
+
+            services.AddScoped<IPolicyEvaluator, CountingPolicyEvaluator>();
         }
 
         public void Configure(IApplicationBuilder app)