diff --git a/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs b/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs index 64fa4c1138..4e44a09fb4 100644 --- a/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs +++ b/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs @@ -16,8 +16,9 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure /// 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 OnResponseStartingDelegate = OnResponseStarting; private readonly RequestDelegate _next; @@ -137,7 +138,7 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure // 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(); diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs index ec8d9e68e4..63cfb5b503 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs @@ -22,6 +22,9 @@ namespace Microsoft.AspNetCore.Mvc.Authorization /// public class AuthorizeFilter : IAsyncAuthorizationFilter, IFilterFactory { + // Property key set by authorization middleware when it is run + private const string AuthorizationMiddlewareInvokedKey = "__AuthorizationMiddlewareInvoked"; + private AuthorizationPolicy _effectivePolicy; /// @@ -170,11 +173,18 @@ namespace Microsoft.AspNetCore.Mvc.Authorization 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) { diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs index 9350abb80b..88e25a37a4 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs +++ b/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 @@ namespace Microsoft.AspNetCore.Mvc.Authorization Assert.IsType(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 @@ namespace Microsoft.AspNetCore.Mvc.Authorization httpContext.Object.User = validUser; } httpContext.SetupGet(c => c.RequestServices).Returns(serviceProvider); + var contextItems = new Dictionary(); + httpContext.SetupGet(c => c.Items).Returns(contextItems); // AuthorizationFilterContext var actionContext = new ActionContext( diff --git a/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs b/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs index 1da5b123fe..e33bdd9bce 100644 --- a/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs +++ b/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 @@ namespace Microsoft.AspNetCore.Authorization 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() ?? Array.Empty(); var policy = await AuthorizationPolicy.CombineAsync(_policyProvider, authorizeData); @@ -101,4 +109,4 @@ namespace Microsoft.AspNetCore.Authorization await _next(context); } } -} \ No newline at end of file +}