Update AuthorizeFilter to no-op when AuthorizationMiddleware has run (#6346)

Author: JamesNK

src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs

@@ -16,8 +16,9 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
     /// </summary>
     public class CorsMiddleware
     {
-        // Property key is used by MVC filters to check if CORS middleware has run
+        // Property key is used by other systems, e.g. MVC, to check if CORS middleware has run
         private const string CorsMiddlewareInvokedKey = "__CorsMiddlewareInvoked";
+        private static readonly object CorsMiddlewareInvokedValue = new object();
 
         private readonly Func<object, Task> OnResponseStartingDelegate = OnResponseStarting;
         private readonly RequestDelegate _next;
@@ -137,7 +138,7 @@ private async Task InvokeCore(HttpContext context, ICorsPolicyProvider corsPolic
             // 3. If there is no policy on middleware then use name on middleware
 
             // Flag to indicate to other systems, e.g. MVC, that CORS middleware was run for this request
-            context.Items[CorsMiddlewareInvokedKey] = true;
+            context.Items[CorsMiddlewareInvokedKey] = CorsMiddlewareInvokedValue;
 
             var endpoint = context.GetEndpoint();
 

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

@@ -22,6 +22,9 @@ namespace Microsoft.AspNetCore.Mvc.Authorization
     /// </summary>
     public class AuthorizeFilter : IAsyncAuthorizationFilter, IFilterFactory
     {
+        // Property key set by authorization middleware when it is run
+        private const string AuthorizationMiddlewareInvokedKey = "__AuthorizationMiddlewareInvoked";
+
         private AuthorizationPolicy _effectivePolicy;
 
         /// <summary>
@@ -170,11 +173,18 @@ public virtual async Task OnAuthorizationAsync(AuthorizationFilterContext contex
                 throw new ArgumentNullException(nameof(context));
             }
 
+            if (context.HttpContext.Items.ContainsKey(AuthorizationMiddlewareInvokedKey))
+            {
+                // Authorization has already run in middleware. Don't re-run for performance
+                return;
+            }
+
             if (!context.IsEffectivePolicy(this))
             {
                 return;
             }
 
+            // IMPORTANT: Changes to authorization logic should be mirrored in security's AuthorizationMiddleware
             var effectivePolicy = await GetEffectivePolicyAsync(context);
             if (effectivePolicy == null)
             {

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

@@ -2,6 +2,7 @@
 // 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.Security.Claims;
 using System.Threading.Tasks;
@@ -48,6 +49,26 @@ public async Task DefaultConstructor_DeniesAnonymousUsers()
             Assert.IsType<ChallengeResult>(authorizationContext.Result);
         }
 
+        [Fact]
+        public async Task OnAuthorizationAsync_AuthorizationMiddlewareHasRun_NoOp()
+        {
+            // Arrange
+            var authorizationContext = GetAuthorizationContext(anonymous: true);
+            authorizationContext.HttpContext.Items["__AuthorizationMiddlewareInvoked"] = new object();
+
+            var authorizeFilterFactory = new AuthorizeFilter();
+            var filterFactory = authorizeFilterFactory as IFilterFactory;
+            var authorizeFilter = (AuthorizeFilter)filterFactory.CreateInstance(
+                authorizationContext.HttpContext.RequestServices);
+            authorizationContext.Filters.Add(authorizeFilter);
+
+            // Act
+            await authorizeFilter.OnAuthorizationAsync(authorizationContext);
+
+            // Assert
+            Assert.Null(authorizationContext.Result);
+        }
+
         [Fact]
         public async Task AuthorizeFilter_CreatedWithAuthorizeData_ThrowsWhenOnAuthorizationAsyncIsCalled()
         {
@@ -557,6 +578,8 @@ private AuthorizationFilterContext GetAuthorizationContext(
                 httpContext.Object.User = validUser;
             }
             httpContext.SetupGet(c => c.RequestServices).Returns(serviceProvider);
+            var contextItems = new Dictionary<object, object>();
+            httpContext.SetupGet(c => c.Items).Returns(contextItems);
 
             // AuthorizationFilterContext
             var actionContext = new ActionContext(

src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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;
@@ -15,6 +15,10 @@ namespace Microsoft.AspNetCore.Authorization
 {
     public class AuthorizationMiddleware
     {
+        // Property key is used by other systems, e.g. MVC, to check if authorization middleware has run
+        private const string AuthorizationMiddlewareInvokedKey = "__AuthorizationMiddlewareInvoked";
+        private static readonly object AuthorizationMiddlewareInvokedValue = new object();
+
         private readonly RequestDelegate _next;
         private readonly IAuthorizationPolicyProvider _policyProvider;
 
@@ -41,6 +45,10 @@ public async Task Invoke(HttpContext context)
                 throw new ArgumentNullException(nameof(context));
             }
 
+            // Flag to indicate to other systems, e.g. MVC, that authorization middleware was run for this request
+            context.Items[AuthorizationMiddlewareInvokedKey] = AuthorizationMiddlewareInvokedValue;
+
+            // IMPORTANT: Changes to authorization logic should be mirrored in MVC's AuthorizeFilter
             var endpoint = context.GetEndpoint();
             var authorizeData = endpoint?.Metadata.GetOrderedMetadata<IAuthorizeData>() ?? Array.Empty<IAuthorizeData>();
             var policy = await AuthorizationPolicy.CombineAsync(_policyProvider, authorizeData);
@@ -101,4 +109,4 @@ public async Task Invoke(HttpContext context)
             await _next(context);
         }
     }
-}
+}