diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizationApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizationApplicationModelProvider.cs index 9455b6ea29..2640f2fa62 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizationApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizationApplicationModelProvider.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Authorization; @@ -36,7 +35,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal foreach (var controllerModel in context.Result.Controllers) { - controllerModel.Filters.Add(new AuthorizeFilter(_policyProvider, controllerModel.Attributes.OfType())); + var controllerModelAuthData = controllerModel.Attributes.OfType().ToArray(); + if (controllerModelAuthData.Length > 0) + { + controllerModel.Filters.Add(GetFilter(controllerModelAuthData)); + } foreach (var attribute in controllerModel.Attributes.OfType()) { controllerModel.Filters.Add(new AllowAnonymousFilter()); @@ -44,7 +47,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal foreach (var actionModel in controllerModel.Actions) { - actionModel.Filters.Add(new AuthorizeFilter(_policyProvider, actionModel.Attributes.OfType())); + var actionModelAuthData = actionModel.Attributes.OfType().ToArray(); + if (actionModelAuthData.Length > 0) + { + actionModel.Filters.Add(GetFilter(actionModelAuthData)); + } + foreach (var attribute in actionModel.Attributes.OfType()) { actionModel.Filters.Add(new AllowAnonymousFilter()); @@ -52,5 +60,20 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } } + + private AuthorizeFilter GetFilter(IEnumerable 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)) + { + var policy = AuthorizationPolicy.CombineAsync(_policyProvider, authData).GetAwaiter().GetResult(); + return new AuthorizeFilter(policy); + } + else + { + return new AuthorizeFilter(_policyProvider, authData); + } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs index 6e047555ca..51f3d69c64 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs @@ -52,7 +52,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization var authorizeFilter = new AuthorizeFilter(policyProvider.Object, new AuthorizeAttribute[] { new AuthorizeAttribute("whatever") }); var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); - // Act + // Act & Assert await authorizeFilter.OnAuthorizationAsync(authorizationContext); Assert.Equal(1, getPolicyCount); Assert.Null(authorizationContext.Result); @@ -64,6 +64,9 @@ namespace Microsoft.AspNetCore.Mvc.Authorization await authorizeFilter.OnAuthorizationAsync(authorizationContext); Assert.Equal(3, getPolicyCount); Assert.Null(authorizationContext.Result); + + // Make sure we don't cache the policy + Assert.Null(authorizeFilter.Policy); } [Fact] @@ -98,7 +101,6 @@ namespace Microsoft.AspNetCore.Mvc.Authorization public async Task Invoke_EmptyClaimsShouldRejectAnonymousUser() { // Arrange - var authorizationOptions = new AuthorizationOptions(); var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build()); var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization(), diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AuthorizationApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AuthorizationApplicationModelProviderTest.cs index ae5bbf5de8..46add52373 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AuthorizationApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AuthorizationApplicationModelProviderTest.cs @@ -1,11 +1,15 @@ // 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.Collections.Generic; using System.Linq; using System.Reflection; +using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Authorization.Infrastructure; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Authorization; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Mvc.Internal @@ -57,8 +61,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal var authorizeFilters = action.Filters.OfType(); Assert.Single(authorizeFilters); - Assert.NotNull(authorizeFilters.First().PolicyProvider); - Assert.Equal(2, authorizeFilters.First().AuthorizeData.Count()); // Base + Derived authorize + Assert.NotNull(authorizeFilters.First().Policy); + Assert.Equal(3, authorizeFilters.First().Policy.Requirements.Count()); // Basic + Basic2 + Derived authorize } [Fact] @@ -81,6 +85,103 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Single(action.Filters, f => f is AllowAnonymousFilter); } + [Fact] + public void OnProvidersExecuting_DefaultPolicyProvider_NoAuthorizationData_NoFilterCreated() + { + // Arrange + var requirements = new IAuthorizationRequirement[] + { + new AssertionRequirement((con) => { return true; }) + }; + var authorizationPolicy = new AuthorizationPolicy(requirements, new string[] { "dingos" }); + var authOptions = new TestOptionsManager(); + authOptions.Value.AddPolicy("Base", authorizationPolicy); + var policyProvider = new DefaultAuthorizationPolicyProvider(authOptions); + + var provider = new AuthorizationApplicationModelProvider(policyProvider); + var defaultProvider = new DefaultApplicationModelProvider(new TestOptionsManager()); + + // Act + var action = GetBaseControllerActionModel(provider, defaultProvider); + + // Assert + var authorizationFilter = Assert.IsType(Assert.Single(action.Filters)); + Assert.NotNull(authorizationFilter.Policy); + Assert.Null(authorizationFilter.AuthorizeData); + Assert.Null(authorizationFilter.PolicyProvider); + } + + [Fact] + public void OnProvidersExecuting_NonDefaultPolicyProvider_HasNoPolicy_HasPolicyProviderAndAuthorizeData() + { + // Arrange + var requirements = new IAuthorizationRequirement[] + { + new AssertionRequirement((con) => { return true; }) + }; + var authorizationPolicy = new AuthorizationPolicy(requirements, new string[] { "dingos" }); + var authorizationPolicyProviderMock = new Mock(); + authorizationPolicyProviderMock + .Setup(s => s.GetPolicyAsync(It.IsAny())) + .Returns(Task.FromResult(authorizationPolicy)) + .Verifiable(); + + var provider = new AuthorizationApplicationModelProvider(authorizationPolicyProviderMock.Object); + var defaultProvider = new DefaultApplicationModelProvider(new TestOptionsManager()); + + // Act + var action = GetBaseControllerActionModel(provider, defaultProvider); + + // Assert + var actionFilter = Assert.IsType(Assert.Single(action.Filters)); + Assert.Null(actionFilter.Policy); + Assert.NotNull(actionFilter.AuthorizeData); + Assert.NotNull(actionFilter.PolicyProvider); + } + + [Fact] + public void CreateControllerModelAndActionModel_NoAuthNoFilter() + { + // Arrange + var provider = new AuthorizationApplicationModelProvider( + new DefaultAuthorizationPolicyProvider( + new TestOptionsManager() + )); + var defaultProvider = new DefaultApplicationModelProvider(new TestOptionsManager()); + + var context = new ApplicationModelProviderContext(new[] { typeof(NoAuthController).GetTypeInfo() }); + defaultProvider.OnProvidersExecuting(context); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + Assert.Empty(controller.Filters); + var action = Assert.Single(controller.Actions); + Assert.Empty(action.Filters); + } + + private ActionModel GetBaseControllerActionModel( + IApplicationModelProvider authorizationApplicationModelProvider, + IApplicationModelProvider applicationModelProvider) + { + var context = new ApplicationModelProviderContext(new[] { typeof(BaseController).GetTypeInfo() }); + applicationModelProvider.OnProvidersExecuting(context); + var authorizeData = new List + { + new AuthorizeAttribute("POLICY") + }; + + authorizationApplicationModelProvider.OnProvidersExecuting(context); + + var controller = Assert.Single(context.Result.Controllers); + Assert.Empty(controller.Filters); + var action = Assert.Single(controller.Actions); + + return action; + } + private class BaseController { [Authorize(Policy = "Base")] @@ -102,6 +203,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal { } + public class NoAuthController + { + public void NoAuthAction() + { } + } + [AllowAnonymous] public class AnonymousController { diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/AuthorizeFilterIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/AuthorizeFilterIntegrationTest.cs new file mode 100644 index 0000000000..ac185cd120 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/AuthorizeFilterIntegrationTest.cs @@ -0,0 +1,105 @@ +// 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.Reflection; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Authorization.Infrastructure; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.ApplicationModels; +using Microsoft.AspNetCore.Mvc.Authorization; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.IntegrationTests +{ + public class AuthorizeFilterIntegrationTest + { + // This is a test for security, because we can't assume that any IAuthorizationPolicyProvider other than + // DefaultAuthorizationPolicyProvider will return the same result for the same input. So a cache could cause + // undesired access. + [Fact] + public async Task AuthorizeFilter_CalledTwiceWithNonDefaultProvider() + { + // Arrange + var applicationModelProviderContext = new ApplicationModelProviderContext( + new[] { typeof(AuthorizeController).GetTypeInfo() }); + + var policyProvider = new TestAuthorizationPolicyProvider(); + var defaultProvider = new DefaultApplicationModelProvider(new TestOptionsManager()); + + defaultProvider.OnProvidersExecuting(applicationModelProviderContext); + + var controller = Assert.Single(applicationModelProviderContext.Result.Controllers); + var action = Assert.Single(controller.Actions); + var authorizeData = action.Attributes.OfType(); + var authorizeFilter = new AuthorizeFilter(policyProvider, authorizeData); + + var actionContext = new ActionContext(GetHttpContext(), new RouteData(), new ControllerActionDescriptor()); + + var authorizationFilterContext = new AuthorizationFilterContext(actionContext, action.Filters); + + // Act + await authorizeFilter.OnAuthorizationAsync(authorizationFilterContext); + await authorizeFilter.OnAuthorizationAsync(authorizationFilterContext); + + // Assert + Assert.Equal(2, policyProvider.GetPolicyCount); + } + + private HttpContext GetHttpContext() + { + var httpContext = new DefaultHttpContext(); + + httpContext.RequestServices = GetServices(); + return httpContext; + } + + private static IServiceProvider GetServices() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddAuthorization(); + serviceCollection.AddMvc(); + serviceCollection + .AddTransient() + .AddTransient, Logger>(); + + return serviceCollection.BuildServiceProvider(); + } + + public class TestAuthorizationPolicyProvider : IAuthorizationPolicyProvider + { + public int GetPolicyCount = 0; + + public Task GetDefaultPolicyAsync() + { + throw new NotImplementedException(); + } + + public Task GetPolicyAsync(string policyName) + { + GetPolicyCount++; + + var requirements = new IAuthorizationRequirement[] + { + new AssertionRequirement((con) => { return true; }) + }; + return Task.FromResult(new AuthorizationPolicy(requirements, new string[] { })); + } + } + + public class AuthorizeController + { + [Authorize(Policy = "Base")] + public virtual void Authorize() + { } + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs index f97ef5d448..7babfffd68 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ApplicationParts; using Microsoft.AspNetCore.Mvc.Controllers; @@ -57,7 +58,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests } public static DefaultControllerArgumentBinder GetArgumentBinder( - IModelMetadataProvider metadataProvider, + IModelMetadataProvider metadataProvider, IModelBinderProvider binderProvider = null) { var services = GetServices(); @@ -109,11 +110,13 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests private static IServiceProvider GetServices(Action updateOptions = null) { var serviceCollection = new ServiceCollection(); + serviceCollection.AddAuthorization(); serviceCollection.AddSingleton(new ApplicationPartManager()); serviceCollection.AddMvc(); serviceCollection .AddSingleton() - .AddTransient(); + .AddTransient() + .AddTransient, Logger>(); if (updateOptions != null) {