From c7a46e4caf0d1f25dd5f8bcd385af50b34b0f5dd Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 31 May 2016 13:47:58 -0700 Subject: [PATCH] AuthPolicy should use IPolicyProvider --- .../Authorization/AuthorizeFilter.cs | 51 ++++++++++++++++--- .../AuthorizationApplicationModelProvider.cs | 29 +++-------- .../Authorization/AuthorizeFilterTest.cs | 25 +++++++++ ...thorizationApplicationModelProviderTest.cs | 10 ++-- 4 files changed, 82 insertions(+), 33 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs index 520eb2fc23..7afdd0d228 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.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; @@ -29,14 +30,44 @@ namespace Microsoft.AspNetCore.Mvc.Authorization { throw new ArgumentNullException(nameof(policy)); } - Policy = policy; } /// - /// Gets the authorization policy to be used. + /// Initialize a new instance. /// - public AuthorizationPolicy Policy { get; } + /// The to use to resolve policy names. + /// The to combine into an . + public AuthorizeFilter(IAuthorizationPolicyProvider policyProvider, IEnumerable authorizeData) + { + if (policyProvider == null) + { + throw new ArgumentNullException(nameof(policyProvider)); + } + if (authorizeData == null) + { + throw new ArgumentNullException(nameof(authorizeData)); + } + + PolicyProvider = policyProvider; + AuthorizeData = authorizeData; + } + + /// + /// The to use to resolve policy names. + /// + public IAuthorizationPolicyProvider PolicyProvider { get; } + + /// + /// The to combine into an . + /// + public IEnumerable AuthorizeData { get; } + + /// + /// Gets the authorization policy to be used. If null, the policy will be constructed via + /// AuthorizePolicy.CombineAsync(PolicyProvider, AuthorizeData) + /// + public AuthorizationPolicy Policy { get; private set; } /// public virtual async Task OnAuthorizationAsync(AuthorizationFilterContext context) @@ -46,11 +77,17 @@ namespace Microsoft.AspNetCore.Mvc.Authorization throw new ArgumentNullException(nameof(context)); } + var effectivePolicy = Policy ?? await AuthorizationPolicy.CombineAsync(PolicyProvider, AuthorizeData); + if (effectivePolicy == null) + { + return; + } + // Build a ClaimsPrincipal with the Policy's required authentication types - if (Policy.AuthenticationSchemes != null && Policy.AuthenticationSchemes.Any()) + if (effectivePolicy.AuthenticationSchemes != null && effectivePolicy.AuthenticationSchemes.Any()) { ClaimsPrincipal newPrincipal = null; - foreach (var scheme in Policy.AuthenticationSchemes) + foreach (var scheme in effectivePolicy.AuthenticationSchemes) { var result = await context.HttpContext.Authentication.AuthenticateAsync(scheme); if (result != null) @@ -76,9 +113,9 @@ namespace Microsoft.AspNetCore.Mvc.Authorization var authService = httpContext.RequestServices.GetRequiredService(); // Note: Default Anonymous User is new ClaimsPrincipal(new ClaimsIdentity()) - if (!await authService.AuthorizeAsync(httpContext.User, context, Policy)) + if (!await authService.AuthorizeAsync(httpContext.User, context, effectivePolicy)) { - context.Result = new ChallengeResult(Policy.AuthenticationSchemes.ToArray()); + context.Result = new ChallengeResult(effectivePolicy.AuthenticationSchemes.ToArray()); } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizationApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizationApplicationModelProvider.cs index d8edf1e488..9455b6ea29 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizationApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizationApplicationModelProvider.cs @@ -2,21 +2,22 @@ // 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.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Authorization; -using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Internal { public class AuthorizationApplicationModelProvider : IApplicationModelProvider { - private readonly AuthorizationOptions _authorizationOptions; + private readonly IAuthorizationPolicyProvider _policyProvider; - public AuthorizationApplicationModelProvider(IOptions authorizationOptionsAccessor) + public AuthorizationApplicationModelProvider(IAuthorizationPolicyProvider policyProvider) { - _authorizationOptions = authorizationOptionsAccessor.Value; + _policyProvider = policyProvider; } public int Order { get { return -1000 + 10; } } @@ -33,18 +34,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(context)); } - AuthorizationPolicy policy; - foreach (var controllerModel in context.Result.Controllers) { - policy = AuthorizationPolicy.Combine( - _authorizationOptions, - controllerModel.Attributes.OfType()); - if (policy != null) - { - controllerModel.Filters.Add(new AuthorizeFilter(policy)); - } - + controllerModel.Filters.Add(new AuthorizeFilter(_policyProvider, controllerModel.Attributes.OfType())); foreach (var attribute in controllerModel.Attributes.OfType()) { controllerModel.Filters.Add(new AllowAnonymousFilter()); @@ -52,14 +44,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal foreach (var actionModel in controllerModel.Actions) { - policy = AuthorizationPolicy.Combine( - _authorizationOptions, - actionModel.Attributes.OfType()); - if (policy != null) - { - actionModel.Filters.Add(new AuthorizeFilter(policy)); - } - + actionModel.Filters.Add(new AuthorizeFilter(_policyProvider, actionModel.Attributes.OfType())); foreach (var attribute in actionModel.Attributes.OfType()) { actionModel.Filters.Add(new AllowAnonymousFilter()); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs index 276d37cb63..6e047555ca 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs @@ -41,6 +41,31 @@ namespace Microsoft.AspNetCore.Mvc.Authorization Assert.Null(authorizationContext.Result); } + [Fact] + public async Task AuthorizeFilterWillCallPolicyProviderOnAuthorization() + { + // Arrange + var policyProvider = new Mock(); + var getPolicyCount = 0; + policyProvider.Setup(p => p.GetPolicyAsync(It.IsAny())).ReturnsAsync(new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build()) + .Callback(() => getPolicyCount++); + var authorizeFilter = new AuthorizeFilter(policyProvider.Object, new AuthorizeAttribute[] { new AuthorizeAttribute("whatever") }); + var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + + // Act + await authorizeFilter.OnAuthorizationAsync(authorizationContext); + Assert.Equal(1, getPolicyCount); + Assert.Null(authorizationContext.Result); + + await authorizeFilter.OnAuthorizationAsync(authorizationContext); + Assert.Equal(2, getPolicyCount); + Assert.Null(authorizationContext.Result); + + await authorizeFilter.OnAuthorizationAsync(authorizationContext); + Assert.Equal(3, getPolicyCount); + Assert.Null(authorizationContext.Result); + } + [Fact] public async Task AuthorizeFilterCanAuthorizeNullUser() { diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AuthorizationApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AuthorizationApplicationModelProviderTest.cs index 905f085aa7..91501e17ae 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AuthorizationApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AuthorizationApplicationModelProviderTest.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal public void CreateControllerModel_AuthorizeAttributeAddsAuthorizeFilter() { // Arrange - var provider = new AuthorizationApplicationModelProvider(new TestOptionsManager()); + var provider = new AuthorizationApplicationModelProvider(new DefaultAuthorizationPolicyProvider(new TestOptionsManager())); var defaultProvider = new DefaultApplicationModelProvider(new TestOptionsManager()); var context = new ApplicationModelProviderContext(new[] { typeof(AccountController).GetTypeInfo() }); @@ -38,7 +38,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal options.Value.AddPolicy("Base", policy => policy.RequireClaim("Basic").RequireClaim("Basic2")); options.Value.AddPolicy("Derived", policy => policy.RequireClaim("Derived")); - var provider = new AuthorizationApplicationModelProvider(options); + var provider = new AuthorizationApplicationModelProvider(new DefaultAuthorizationPolicyProvider(options)); var defaultProvider = new DefaultApplicationModelProvider(new TestOptionsManager()); var context = new ApplicationModelProviderContext(new[] { typeof(DerivedController).GetTypeInfo() }); @@ -56,14 +56,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Empty(attributeRoutes); var authorizeFilters = action.Filters.OfType(); Assert.Single(authorizeFilters); - Assert.Equal(3, authorizeFilters.First().Policy.Requirements.Count); + + Assert.NotNull(authorizeFilters.First().PolicyProvider); + Assert.Equal(3, authorizeFilters.First().AuthorizeData.Count()); } [Fact] public void CreateControllerModelAndActionModel_AllowAnonymousAttributeAddsAllowAnonymousFilter() { // Arrange - var provider = new AuthorizationApplicationModelProvider(new TestOptionsManager()); + var provider = new AuthorizationApplicationModelProvider(new DefaultAuthorizationPolicyProvider(new TestOptionsManager())); var defaultProvider = new DefaultApplicationModelProvider(new TestOptionsManager()); var context = new ApplicationModelProviderContext(new[] { typeof(AnonymousController).GetTypeInfo() });