From c257c9528f16db78a33eed810b9a66f796146b3c Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 31 May 2016 13:45:30 -0700 Subject: [PATCH] AuthZ: Combine needs to use policy provider Fixes https://github.com/aspnet/Security/issues/841 --- .../AuthorizationPolicy.cs | 18 +++++----- .../DefaultAuthorizationPolicyProvider.cs | 7 +++- .../IAuthorizationPolicyProvider.cs | 6 ++++ .../AuthorizationPolicyFacts.cs | 36 ++++++++++++------- .../DefaultAuthorizationServiceTests.cs | 18 ++++++++-- 5 files changed, 61 insertions(+), 24 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authorization/AuthorizationPolicy.cs b/src/Microsoft.AspNetCore.Authorization/AuthorizationPolicy.cs index 97aa5b381b..171fe014d6 100644 --- a/src/Microsoft.AspNetCore.Authorization/AuthorizationPolicy.cs +++ b/src/Microsoft.AspNetCore.Authorization/AuthorizationPolicy.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; namespace Microsoft.AspNetCore.Authorization { @@ -57,27 +58,27 @@ namespace Microsoft.AspNetCore.Authorization return builder.Build(); } - public static AuthorizationPolicy Combine(AuthorizationOptions options, IEnumerable attributes) + public static async Task CombineAsync(IAuthorizationPolicyProvider policyProvider, IEnumerable authorizeData) { - if (options == null) + if (policyProvider == null) { - throw new ArgumentNullException(nameof(options)); + throw new ArgumentNullException(nameof(policyProvider)); } - if (attributes == null) + if (authorizeData == null) { - throw new ArgumentNullException(nameof(attributes)); + throw new ArgumentNullException(nameof(authorizeData)); } var policyBuilder = new AuthorizationPolicyBuilder(); var any = false; - foreach (var authorizeAttribute in attributes.OfType()) + foreach (var authorizeAttribute in authorizeData.OfType()) { any = true; var useDefaultPolicy = true; if (!string.IsNullOrWhiteSpace(authorizeAttribute.Policy)) { - var policy = options.GetPolicy(authorizeAttribute.Policy); + var policy = await policyProvider.GetPolicyAsync(authorizeAttribute.Policy); if (policy == null) { throw new InvalidOperationException(Resources.FormatException_AuthorizationPolicyNotFound(authorizeAttribute.Policy)); @@ -89,7 +90,6 @@ namespace Microsoft.AspNetCore.Authorization if (rolesSplit != null && rolesSplit.Any()) { var trimmedRolesSplit = rolesSplit.Where(r => !string.IsNullOrWhiteSpace(r)).Select(r => r.Trim()); - policyBuilder.RequireRole(trimmedRolesSplit); useDefaultPolicy = false; } @@ -106,7 +106,7 @@ namespace Microsoft.AspNetCore.Authorization } if (useDefaultPolicy) { - policyBuilder.Combine(options.DefaultPolicy); + policyBuilder.Combine(await policyProvider.GetDefaultPolicyAsync()); } } return any ? policyBuilder.Build() : null; diff --git a/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationPolicyProvider.cs b/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationPolicyProvider.cs index 97b806e87d..e053e41f95 100644 --- a/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationPolicyProvider.cs +++ b/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationPolicyProvider.cs @@ -22,7 +22,12 @@ namespace Microsoft.AspNetCore.Authorization } _options = options.Value; - } + } + + public Task GetDefaultPolicyAsync() + { + return Task.FromResult(_options.DefaultPolicy); + } /// /// Gets a from the given diff --git a/src/Microsoft.AspNetCore.Authorization/IAuthorizationPolicyProvider.cs b/src/Microsoft.AspNetCore.Authorization/IAuthorizationPolicyProvider.cs index 1a0dbace60..ac141b327d 100644 --- a/src/Microsoft.AspNetCore.Authorization/IAuthorizationPolicyProvider.cs +++ b/src/Microsoft.AspNetCore.Authorization/IAuthorizationPolicyProvider.cs @@ -16,5 +16,11 @@ namespace Microsoft.AspNetCore.Authorization /// /// Task GetPolicyAsync(string policyName); + + /// + /// Returns the default . + /// + /// + Task GetDefaultPolicyAsync(); } } diff --git a/test/Microsoft.AspNetCore.Authorization.Test/AuthorizationPolicyFacts.cs b/test/Microsoft.AspNetCore.Authorization.Test/AuthorizationPolicyFacts.cs index f74e461523..6825dd868b 100644 --- a/test/Microsoft.AspNetCore.Authorization.Test/AuthorizationPolicyFacts.cs +++ b/test/Microsoft.AspNetCore.Authorization.Test/AuthorizationPolicyFacts.cs @@ -3,8 +3,10 @@ using System; using System.Linq; +using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Authorization.Infrastructure; +using Microsoft.Extensions.Options; using Xunit; namespace Microsoft.AspNetCore.Authroization.Test @@ -18,7 +20,7 @@ namespace Microsoft.AspNetCore.Authroization.Test } [Fact] - public void CanCombineAuthorizeAttributes() + public async Task CanCombineAuthorizeAttributes() { // Arrange var attributes = new AuthorizeAttribute[] { @@ -31,8 +33,10 @@ namespace Microsoft.AspNetCore.Authroization.Test options.AddPolicy("1", policy => policy.RequireClaim("1")); options.AddPolicy("2", policy => policy.RequireClaim("2")); + var provider = new DefaultAuthorizationPolicyProvider(Options.Create(options)); + // Act - var combined = AuthorizationPolicy.Combine(options, attributes); + var combined = await AuthorizationPolicy.CombineAsync(provider, attributes); // Assert Assert.Equal(2, combined.AuthenticationSchemes.Count()); @@ -45,7 +49,7 @@ namespace Microsoft.AspNetCore.Authroization.Test } [Fact] - public void CanReplaceDefaultPolicy() + public async Task CanReplaceDefaultPolicy() { // Arrange var attributes = new AuthorizeAttribute[] { @@ -56,8 +60,10 @@ namespace Microsoft.AspNetCore.Authroization.Test options.DefaultPolicy = new AuthorizationPolicyBuilder("default").RequireClaim("default").Build(); options.AddPolicy("2", policy => policy.RequireClaim("2")); + var provider = new DefaultAuthorizationPolicyProvider(Options.Create(options)); + // Act - var combined = AuthorizationPolicy.Combine(options, attributes); + var combined = await AuthorizationPolicy.CombineAsync(provider, attributes); // Assert Assert.Equal(2, combined.AuthenticationSchemes.Count()); @@ -69,16 +75,17 @@ namespace Microsoft.AspNetCore.Authroization.Test } [Fact] - public void CombineMustTrimRoles() + public async Task CombineMustTrimRoles() { // Arrange var attributes = new AuthorizeAttribute[] { new AuthorizeAttribute() { Roles = "r1 , r2" } }; var options = new AuthorizationOptions(); + var provider = new DefaultAuthorizationPolicyProvider(Options.Create(options)); // Act - var combined = AuthorizationPolicy.Combine(options, attributes); + var combined = await AuthorizationPolicy.CombineAsync(provider, attributes); // Assert Assert.True(combined.Requirements.Any(r => r is RolesAuthorizationRequirement)); @@ -89,7 +96,7 @@ namespace Microsoft.AspNetCore.Authroization.Test } [Fact] - public void CombineMustTrimAuthenticationScheme() + public async Task CombineMustTrimAuthenticationScheme() { // Arrange var attributes = new AuthorizeAttribute[] { @@ -97,8 +104,10 @@ namespace Microsoft.AspNetCore.Authroization.Test }; var options = new AuthorizationOptions(); + var provider = new DefaultAuthorizationPolicyProvider(Options.Create(options)); + // Act - var combined = AuthorizationPolicy.Combine(options, attributes); + var combined = await AuthorizationPolicy.CombineAsync(provider, attributes); // Assert Assert.Equal(2, combined.AuthenticationSchemes.Count()); @@ -107,7 +116,7 @@ namespace Microsoft.AspNetCore.Authroization.Test } [Fact] - public void CombineMustIgnoreEmptyAuthenticationScheme() + public async Task CombineMustIgnoreEmptyAuthenticationScheme() { // Arrange var attributes = new AuthorizeAttribute[] { @@ -115,8 +124,10 @@ namespace Microsoft.AspNetCore.Authroization.Test }; var options = new AuthorizationOptions(); + var provider = new DefaultAuthorizationPolicyProvider(Options.Create(options)); + // Act - var combined = AuthorizationPolicy.Combine(options, attributes); + var combined = await AuthorizationPolicy.CombineAsync(provider, attributes); // Assert Assert.Equal(2, combined.AuthenticationSchemes.Count()); @@ -125,16 +136,17 @@ namespace Microsoft.AspNetCore.Authroization.Test } [Fact] - public void CombineMustIgnoreEmptyRoles() + public async Task CombineMustIgnoreEmptyRoles() { // Arrange var attributes = new AuthorizeAttribute[] { new AuthorizeAttribute() { Roles = "r1 , ,, , r2" } }; var options = new AuthorizationOptions(); + var provider = new DefaultAuthorizationPolicyProvider(Options.Create(options)); // Act - var combined = AuthorizationPolicy.Combine(options, attributes); + var combined = await AuthorizationPolicy.CombineAsync(provider, attributes); // Assert Assert.True(combined.Requirements.Any(r => r is RolesAuthorizationRequirement)); diff --git a/test/Microsoft.AspNetCore.Authorization.Test/DefaultAuthorizationServiceTests.cs b/test/Microsoft.AspNetCore.Authorization.Test/DefaultAuthorizationServiceTests.cs index 42948c936f..b6383ffb48 100644 --- a/test/Microsoft.AspNetCore.Authorization.Test/DefaultAuthorizationServiceTests.cs +++ b/test/Microsoft.AspNetCore.Authorization.Test/DefaultAuthorizationServiceTests.cs @@ -8,6 +8,7 @@ using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization.Infrastructure; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Xunit; namespace Microsoft.AspNetCore.Authorization.Test @@ -28,9 +29,12 @@ namespace Microsoft.AspNetCore.Authorization.Test } [Fact] - public void AuthorizeCombineThrowsOnUnknownPolicy() + public async Task AuthorizeCombineThrowsOnUnknownPolicy() { - Assert.Throws(() => AuthorizationPolicy.Combine(new AuthorizationOptions(), new AuthorizeAttribute[] { + var provider = new DefaultAuthorizationPolicyProvider(Options.Create(new AuthorizationOptions())); + + // Act + await Assert.ThrowsAsync(() => AuthorizationPolicy.CombineAsync(provider, new AuthorizeAttribute[] { new AuthorizeAttribute { Policy = "Wut" } })); } @@ -944,6 +948,11 @@ namespace Microsoft.AspNetCore.Authorization.Test public class StaticPolicyProvider : IAuthorizationPolicyProvider { + public Task GetDefaultPolicyAsync() + { + return Task.FromResult(new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build()); + } + public Task GetPolicyAsync(string policyName) { return Task.FromResult(new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build()); @@ -973,6 +982,11 @@ namespace Microsoft.AspNetCore.Authorization.Test public class DynamicPolicyProvider : IAuthorizationPolicyProvider { + public Task GetDefaultPolicyAsync() + { + return Task.FromResult(new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build()); + } + public Task GetPolicyAsync(string policyName) { return Task.FromResult(new AuthorizationPolicyBuilder().RequireClaim(policyName).Build());