From 17deab142da56b656bb2cd84dce294ca15135397 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 12 May 2015 13:57:23 -0700 Subject: [PATCH] AuthZ: Sugar to make resource parameter optional --- .../AuthorizationPolicy.cs | 6 +- .../AuthorizationServiceExtensions.cs | 72 +++++++++++++++++++ .../DefaultAuthorizationService.cs | 5 +- .../IAuthorizationService.cs | 10 +-- .../Properties/Resources.Designer.cs | 16 +++++ .../Resources.resx | 3 + .../DefaultAuthorizationServiceTests.cs | 61 ++++++---------- 7 files changed, 128 insertions(+), 45 deletions(-) diff --git a/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs b/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs index ef16f968c8..3799264508 100644 --- a/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs +++ b/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs @@ -10,8 +10,12 @@ namespace Microsoft.AspNet.Authorization { public class AuthorizationPolicy { - public AuthorizationPolicy(IEnumerable requirements, IEnumerable activeAuthenticationSchemes) + public AuthorizationPolicy([NotNull] IEnumerable requirements, [NotNull] IEnumerable activeAuthenticationSchemes) { + if (requirements.Count() == 0) + { + throw new InvalidOperationException(Resources.Exception_AuthorizationPolicyEmpty); + } Requirements = new List(requirements).AsReadOnly(); ActiveAuthenticationSchemes = new List(activeAuthenticationSchemes).AsReadOnly(); } diff --git a/src/Microsoft.AspNet.Authorization/AuthorizationServiceExtensions.cs b/src/Microsoft.AspNet.Authorization/AuthorizationServiceExtensions.cs index 07619f37c7..14951fa075 100644 --- a/src/Microsoft.AspNet.Authorization/AuthorizationServiceExtensions.cs +++ b/src/Microsoft.AspNet.Authorization/AuthorizationServiceExtensions.cs @@ -10,6 +10,18 @@ namespace Microsoft.AspNet.Authorization { public static class AuthorizationServiceExtensions { + /// + /// Checks if a user meets a specific requirement for the specified resource + /// + /// + /// + /// + /// + public static Task AuthorizeAsync([NotNull] this IAuthorizationService service, ClaimsPrincipal user, object resource, [NotNull] IAuthorizationRequirement requirement) + { + return service.AuthorizeAsync(user, resource, new IAuthorizationRequirement[] { requirement }); + } + /// /// Checks if a user meets a specific authorization policy /// @@ -23,6 +35,42 @@ namespace Microsoft.AspNet.Authorization return service.AuthorizeAsync(user, resource, policy.Requirements.ToArray()); } + /// + /// Checks if a user meets a specific authorization policy + /// + /// The authorization service. + /// The user to check the policy against. + /// The policy to check against a specific context. + /// true when the user fulfills the policy, false otherwise. + public static Task AuthorizeAsync([NotNull] this IAuthorizationService service, ClaimsPrincipal user, [NotNull] AuthorizationPolicy policy) + { + return service.AuthorizeAsync(user, resource: null, policy: policy); + } + + /// + /// Checks if a user meets a specific authorization policy + /// + /// The authorization service. + /// The user to check the policy against. + /// The name of the policy to check against a specific context. + /// true when the user fulfills the policy, false otherwise. + public static Task AuthorizeAsync([NotNull] this IAuthorizationService service, ClaimsPrincipal user, [NotNull] string policyName) + { + return service.AuthorizeAsync(user, resource: null, policyName: policyName); + } + + /// + /// Checks if a user meets a specific requirement for the specified resource + /// + /// + /// + /// + /// + public static bool Authorize([NotNull] this IAuthorizationService service, ClaimsPrincipal user, object resource, [NotNull] IAuthorizationRequirement requirement) + { + return service.Authorize(user, resource, new IAuthorizationRequirement[] { requirement }); + } + /// /// Checks if a user meets a specific authorization policy /// @@ -36,5 +84,29 @@ namespace Microsoft.AspNet.Authorization return service.Authorize(user, resource, policy.Requirements.ToArray()); } + /// + /// Checks if a user meets a specific authorization policy + /// + /// The authorization service. + /// The user to check the policy against. + /// The policy to check against a specific context. + /// true when the user fulfills the policy, false otherwise. + public static bool Authorize([NotNull] this IAuthorizationService service, ClaimsPrincipal user, [NotNull] AuthorizationPolicy policy) + { + return service.Authorize(user, resource: null, requirements: policy.Requirements.ToArray()); + } + + /// + /// Checks if a user meets a specific authorization policy + /// + /// The authorization service. + /// The user to check the policy against. + /// The name of the policy to check against a specific context. + /// true when the user fulfills the policy, false otherwise. + public static bool Authorize([NotNull] this IAuthorizationService service, ClaimsPrincipal user, [NotNull] string policyName) + { + return service.Authorize(user, resource: null, policyName: policyName); + } + } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Authorization/DefaultAuthorizationService.cs b/src/Microsoft.AspNet.Authorization/DefaultAuthorizationService.cs index abc8662769..e66721a5c1 100644 --- a/src/Microsoft.AspNet.Authorization/DefaultAuthorizationService.cs +++ b/src/Microsoft.AspNet.Authorization/DefaultAuthorizationService.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using System.Security.Claims; using System.Threading.Tasks; +using Microsoft.Framework.Internal; using Microsoft.Framework.OptionsModel; namespace Microsoft.AspNet.Authorization @@ -28,7 +29,7 @@ namespace Microsoft.AspNet.Authorization : this.Authorize(user, resource, policy); } - public bool Authorize(ClaimsPrincipal user, object resource, params IAuthorizationRequirement[] requirements) + public bool Authorize(ClaimsPrincipal user, object resource, [NotNull] IEnumerable requirements) { var authContext = new AuthorizationContext(requirements, user, resource); foreach (var handler in _handlers) @@ -38,7 +39,7 @@ namespace Microsoft.AspNet.Authorization return authContext.HasSucceeded; } - public async Task AuthorizeAsync(ClaimsPrincipal user, object resource, params IAuthorizationRequirement[] requirements) + public async Task AuthorizeAsync(ClaimsPrincipal user, object resource, [NotNull] IEnumerable requirements) { var authContext = new AuthorizationContext(requirements, user, resource); foreach (var handler in _handlers) diff --git a/src/Microsoft.AspNet.Authorization/IAuthorizationService.cs b/src/Microsoft.AspNet.Authorization/IAuthorizationService.cs index dedfcc37c4..876fd76016 100644 --- a/src/Microsoft.AspNet.Authorization/IAuthorizationService.cs +++ b/src/Microsoft.AspNet.Authorization/IAuthorizationService.cs @@ -1,8 +1,10 @@ // 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.Security.Claims; using System.Threading.Tasks; +using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Authorization { @@ -18,7 +20,7 @@ namespace Microsoft.AspNet.Authorization /// /// /// - Task AuthorizeAsync(ClaimsPrincipal user, object resource, params IAuthorizationRequirement[] requirements); + Task AuthorizeAsync(ClaimsPrincipal user, object resource, [NotNull] IEnumerable requirements); /// /// Checks if a user meets a specific set of requirements for the specified resource @@ -27,7 +29,7 @@ namespace Microsoft.AspNet.Authorization /// /// /// - bool Authorize(ClaimsPrincipal user, object resource, params IAuthorizationRequirement[] requirements); + bool Authorize(ClaimsPrincipal user, object resource, [NotNull] IEnumerable requirements); /// /// Checks if a user meets a specific authorization policy @@ -36,7 +38,7 @@ namespace Microsoft.AspNet.Authorization /// The resource the policy should be checked with. /// The name of the policy to check against a specific context. /// true when the user fulfills the policy, false otherwise. - Task AuthorizeAsync(ClaimsPrincipal user, object resource, string policyName); + Task AuthorizeAsync(ClaimsPrincipal user, object resource, [NotNull] string policyName); /// /// Checks if a user meets a specific authorization policy @@ -45,6 +47,6 @@ namespace Microsoft.AspNet.Authorization /// The resource the policy should be checked with. /// The name of the policy to check against a specific context. /// true when the user fulfills the policy, false otherwise. - bool Authorize(ClaimsPrincipal user, object resource, string policyName); + bool Authorize(ClaimsPrincipal user, object resource, [NotNull] string policyName); } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Authorization/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Authorization/Properties/Resources.Designer.cs index b627118275..29d82385f2 100644 --- a/src/Microsoft.AspNet.Authorization/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Authorization/Properties/Resources.Designer.cs @@ -26,6 +26,22 @@ namespace Microsoft.AspNet.Authorization return string.Format(CultureInfo.CurrentCulture, GetString("Exception_AuthorizationPolicyNotFound"), p0); } + /// + /// AuthorizationPolicy must have at least one requirement. + /// + internal static string Exception_AuthorizationPolicyEmpty + { + get { return GetString("Exception_AuthorizationPolicyEmpty"); } + } + + /// + /// AuthorizationPolicy must have at least one requirement. + /// + internal static string FormatException_AuthorizationPolicyEmpty(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("Exception_AuthorizationPolicyEmpty"), p0); + } + /// /// At least one role must be specified. /// diff --git a/src/Microsoft.AspNet.Authorization/Resources.resx b/src/Microsoft.AspNet.Authorization/Resources.resx index ee3f8cd88a..a36e55d6b0 100644 --- a/src/Microsoft.AspNet.Authorization/Resources.resx +++ b/src/Microsoft.AspNet.Authorization/Resources.resx @@ -117,6 +117,9 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + AuthorizationPolicy must have at least one requirement. + The AuthorizationPolicy named: '{0}' was not found. diff --git a/test/Microsoft.AspNet.Authorization.Test/DefaultAuthorizationServiceTests.cs b/test/Microsoft.AspNet.Authorization.Test/DefaultAuthorizationServiceTests.cs index 2683eddce6..d5491164d8 100644 --- a/test/Microsoft.AspNet.Authorization.Test/DefaultAuthorizationServiceTests.cs +++ b/test/Microsoft.AspNet.Authorization.Test/DefaultAuthorizationServiceTests.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Security.Claims; using System.Threading.Tasks; @@ -47,7 +46,7 @@ namespace Microsoft.AspNet.Authorization.Test var user = new ClaimsPrincipal(new ClaimsIdentity(new Claim[] { new Claim("Permission", "CanViewPage") }, "Basic")); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.True(allowed); @@ -67,7 +66,7 @@ namespace Microsoft.AspNet.Authorization.Test var user = new ClaimsPrincipal(new ClaimsIdentity(new Claim[] { new Claim("Permission", "CanViewPage") }, "Basic")); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.True(allowed); @@ -94,7 +93,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.True(allowed); @@ -120,7 +119,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.False(allowed); @@ -146,7 +145,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.False(allowed); @@ -172,7 +171,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.False(allowed); @@ -196,7 +195,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.False(allowed); @@ -235,7 +234,7 @@ namespace Microsoft.AspNet.Authorization.Test var user = new ClaimsPrincipal(new ClaimsIdentity()); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.False(allowed); @@ -261,7 +260,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.True(allowed); @@ -281,7 +280,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.False(allowed); @@ -304,7 +303,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, policy.Build()); + var allowed = await authorizationService.AuthorizeAsync(user, policy.Build()); // Assert Assert.True(allowed); @@ -325,7 +324,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, policy.Build()); + var allowed = await authorizationService.AuthorizeAsync(user, policy.Build()); // Assert Assert.True(allowed); @@ -342,7 +341,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, policy.Build()); + var allowed = await authorizationService.AuthorizeAsync(user, policy.Build()); // Assert Assert.True(allowed); @@ -375,7 +374,7 @@ namespace Microsoft.AspNet.Authorization.Test new ClaimsIdentity(new Claim[] { new Claim(ClaimTypes.Role, "Users") }, "AuthType")); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, policy.Build()); + var allowed = await authorizationService.AuthorizeAsync(user, policy.Build()); // Assert Assert.True(allowed); @@ -396,7 +395,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, policy.Build()); + var allowed = await authorizationService.AuthorizeAsync(user, policy.Build()); // Assert Assert.False(allowed); @@ -421,36 +420,22 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); + var allowed = await authorizationService.AuthorizeAsync(user, "Basic"); // Assert Assert.False(allowed); } [Fact] - public async Task PolicyFailsWithNoRequirements() + public void PolicyThrowsWithNoRequirements() { - // Arrange - var authorizationService = BuildAuthorizationService(services => + Assert.Throws(() => BuildAuthorizationService(services => { services.ConfigureAuthorization(options => { options.AddPolicy("Basic", policy => { }); }); - }); - var user = new ClaimsPrincipal( - new ClaimsIdentity( - new Claim[] { - new Claim(ClaimTypes.Name, "Name"), - }, - "AuthType") - ); - - // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Basic"); - - // Assert - Assert.False(allowed); + })); } [Fact] @@ -473,7 +458,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Any"); + var allowed = await authorizationService.AuthorizeAsync(user, "Any"); // Assert Assert.False(allowed); @@ -499,7 +484,7 @@ namespace Microsoft.AspNet.Authorization.Test ); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Hao"); + var allowed = await authorizationService.AuthorizeAsync(user, "Hao"); // Assert Assert.True(allowed); @@ -521,7 +506,7 @@ namespace Microsoft.AspNet.Authorization.Test var user = new ClaimsPrincipal(identity); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Hao"); + var allowed = await authorizationService.AuthorizeAsync(user, "Hao"); // Assert Assert.True(allowed); @@ -543,7 +528,7 @@ namespace Microsoft.AspNet.Authorization.Test var user = new ClaimsPrincipal(identity); // Act - var allowed = await authorizationService.AuthorizeAsync(user, null, "Hao"); + var allowed = await authorizationService.AuthorizeAsync(user, "Hao"); // Assert Assert.True(allowed);