From 434d158c7620fd75492ea5d03b6885246745b26c Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Wed, 6 May 2015 14:24:20 -0700 Subject: [PATCH] Support custom name and role claims --- .../AuthorizationPolicy.cs | 4 +- .../AuthorizationPolicyBuilder.cs | 16 ++--- .../AuthorizationServiceExtensions.cs | 12 ---- .../ClaimsAuthorizationRequirement.cs | 36 ++++++++++- .../DenyAnonymousAuthorizationRequirement.cs | 17 ++++- .../NameAuthorizationRequirement.cs | 35 ++++++++++ .../Properties/Resources.Designer.cs | 64 +++++-------------- .../Resources.resx | 12 +--- .../RolesAuthorizationRequirement.cs | 47 ++++++++++++++ .../ServiceCollectionExtensions.cs | 2 - .../AuthorizationPolicyFacts.cs | 10 ++- .../DefaultAuthorizationServiceTests.cs | 45 +++++++++++++ 12 files changed, 209 insertions(+), 91 deletions(-) create mode 100644 src/Microsoft.AspNet.Authorization/NameAuthorizationRequirement.cs create mode 100644 src/Microsoft.AspNet.Authorization/RolesAuthorizationRequirement.cs diff --git a/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs b/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs index 3cc52abe5d..ef16f968c8 100644 --- a/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs +++ b/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs @@ -38,7 +38,7 @@ namespace Microsoft.AspNet.Authorization public static AuthorizationPolicy Combine([NotNull] AuthorizationOptions options, [NotNull] IEnumerable attributes) { var policyBuilder = new AuthorizationPolicyBuilder(); - bool any = false; + var any = false; foreach (var authorizeAttribute in attributes.OfType()) { any = true; @@ -59,7 +59,7 @@ namespace Microsoft.AspNet.Authorization policyBuilder.RequireRole(rolesSplit); requireAnyAuthenticated = false; } - string[] authTypesSplit = authorizeAttribute.ActiveAuthenticationSchemes?.Split(','); + var authTypesSplit = authorizeAttribute.ActiveAuthenticationSchemes?.Split(','); if (authTypesSplit != null && authTypesSplit.Any()) { foreach (var authType in authTypesSplit) diff --git a/src/Microsoft.AspNet.Authorization/AuthorizationPolicyBuilder.cs b/src/Microsoft.AspNet.Authorization/AuthorizationPolicyBuilder.cs index 128545032c..f41fe73692 100644 --- a/src/Microsoft.AspNet.Authorization/AuthorizationPolicyBuilder.cs +++ b/src/Microsoft.AspNet.Authorization/AuthorizationPolicyBuilder.cs @@ -55,21 +55,13 @@ namespace Microsoft.AspNet.Authorization public AuthorizationPolicyBuilder RequireClaim([NotNull] string claimType, IEnumerable requiredValues) { - Requirements.Add(new ClaimsAuthorizationRequirement - { - ClaimType = claimType, - AllowedValues = requiredValues - }); + Requirements.Add(new ClaimsAuthorizationRequirement(claimType, requiredValues)); return this; } public AuthorizationPolicyBuilder RequireClaim([NotNull] string claimType) { - Requirements.Add(new ClaimsAuthorizationRequirement - { - ClaimType = claimType, - AllowedValues = null - }); + Requirements.Add(new ClaimsAuthorizationRequirement(claimType, allowedValues: null)); return this; } @@ -80,13 +72,13 @@ namespace Microsoft.AspNet.Authorization public AuthorizationPolicyBuilder RequireRole([NotNull] IEnumerable roles) { - RequireClaim(ClaimTypes.Role, roles); + Requirements.Add(new RolesAuthorizationRequirement(roles)); return this; } public AuthorizationPolicyBuilder RequireUserName([NotNull] string userName) { - RequireClaim(ClaimTypes.Name, userName); + Requirements.Add(new NameAuthorizationRequirement(userName)); return this; } diff --git a/src/Microsoft.AspNet.Authorization/AuthorizationServiceExtensions.cs b/src/Microsoft.AspNet.Authorization/AuthorizationServiceExtensions.cs index 298a2f0818..07619f37c7 100644 --- a/src/Microsoft.AspNet.Authorization/AuthorizationServiceExtensions.cs +++ b/src/Microsoft.AspNet.Authorization/AuthorizationServiceExtensions.cs @@ -20,12 +20,6 @@ namespace Microsoft.AspNet.Authorization /// true when the user fulfills the policy, false otherwise. public static Task AuthorizeAsync([NotNull] this IAuthorizationService service, ClaimsPrincipal user, object resource, [NotNull] AuthorizationPolicy policy) { - // TODO RENABLE - //if (policy.ActiveAuthenticationSchemes != null && policy.ActiveAuthenticationSchemes.Any() && user != null) - //{ - // // Filter the user to only contain the active authentication types - // user = new ClaimsPrincipal(user.Identities.Where(i => policy.ActiveAuthenticationSchemes.Contains(i.AuthenticationScheme))); - //} return service.AuthorizeAsync(user, resource, policy.Requirements.ToArray()); } @@ -39,12 +33,6 @@ namespace Microsoft.AspNet.Authorization /// true when the user fulfills the policy, false otherwise. public static bool Authorize([NotNull] this IAuthorizationService service, ClaimsPrincipal user, object resource, [NotNull] AuthorizationPolicy policy) { - // TODO: REeanble - //if (policy.ActiveAuthenticationSchemes != null && policy.ActiveAuthenticationSchemes.Any() && user != null) - //{ - // // Filter the user to only contain the active authentication types - // user = new ClaimsPrincipal(user.Identities.Where(i => policy.ActiveAuthenticationSchemes.Contains(i.AuthenticationScheme))); - //} return service.Authorize(user, resource, policy.Requirements.ToArray()); } diff --git a/src/Microsoft.AspNet.Authorization/ClaimsAuthorizationRequirement.cs b/src/Microsoft.AspNet.Authorization/ClaimsAuthorizationRequirement.cs index b8994b8b7a..c5af9449b1 100644 --- a/src/Microsoft.AspNet.Authorization/ClaimsAuthorizationRequirement.cs +++ b/src/Microsoft.AspNet.Authorization/ClaimsAuthorizationRequirement.cs @@ -1,15 +1,45 @@ // 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.Collections.Generic; +using System.Linq; +using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Authorization { // Must contain a claim with the specified name, and at least one of the required values // If AllowedValues is null or empty, that means any claim is valid - public class ClaimsAuthorizationRequirement : IAuthorizationRequirement + public class ClaimsAuthorizationRequirement : AuthorizationHandler, IAuthorizationRequirement { - public string ClaimType { get; set; } - public IEnumerable AllowedValues { get; set; } + public ClaimsAuthorizationRequirement([NotNull] string claimType, IEnumerable allowedValues) + { + ClaimType = claimType; + AllowedValues = allowedValues; + } + + public string ClaimType { get; } + public IEnumerable AllowedValues { get; } + + public override void Handle(AuthorizationContext context, ClaimsAuthorizationRequirement requirement) + { + if (context.User != null) + { + var found = false; + if (requirement.AllowedValues == null || !requirement.AllowedValues.Any()) + { + found = context.User.Claims.Any(c => string.Equals(c.Type, requirement.ClaimType, StringComparison.OrdinalIgnoreCase)); + } + else + { + found = context.User.Claims.Any(c => string.Equals(c.Type, requirement.ClaimType, StringComparison.OrdinalIgnoreCase) + && requirement.AllowedValues.Contains(c.Value, StringComparer.Ordinal)); + } + if (found) + { + context.Succeed(requirement); + } + } + } } } diff --git a/src/Microsoft.AspNet.Authorization/DenyAnonymousAuthorizationRequirement.cs b/src/Microsoft.AspNet.Authorization/DenyAnonymousAuthorizationRequirement.cs index fce23a3dc8..92f48c1fe4 100644 --- a/src/Microsoft.AspNet.Authorization/DenyAnonymousAuthorizationRequirement.cs +++ b/src/Microsoft.AspNet.Authorization/DenyAnonymousAuthorizationRequirement.cs @@ -1,9 +1,22 @@ // 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 Microsoft.AspNet.Authorization; +using System.Linq; namespace Microsoft.AspNet.Authorization { - public class DenyAnonymousAuthorizationRequirement : IAuthorizationRequirement { } + public class DenyAnonymousAuthorizationRequirement : AuthorizationHandler, IAuthorizationRequirement + { + public override void Handle(AuthorizationContext context, DenyAnonymousAuthorizationRequirement requirement) + { + var user = context.User; + var userIsAnonymous = + user?.Identity == null || + !user.Identities.Any(i => i.IsAuthenticated); + if (!userIsAnonymous) + { + context.Succeed(requirement); + } + } + } } diff --git a/src/Microsoft.AspNet.Authorization/NameAuthorizationRequirement.cs b/src/Microsoft.AspNet.Authorization/NameAuthorizationRequirement.cs new file mode 100644 index 0000000000..7d64f1b71c --- /dev/null +++ b/src/Microsoft.AspNet.Authorization/NameAuthorizationRequirement.cs @@ -0,0 +1,35 @@ +// 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.Collections.Generic; +using System.Linq; +using Microsoft.Framework.Internal; + +namespace Microsoft.AspNet.Authorization +{ + /// + /// Requirement that ensures a specific Name + /// + public class NameAuthorizationRequirement : AuthorizationHandler, IAuthorizationRequirement + { + public NameAuthorizationRequirement([NotNull] string requiredName) + { + RequiredName = requiredName; + } + + public string RequiredName { get; } + + public override void Handle(AuthorizationContext context, NameAuthorizationRequirement requirement) + { + if (context.User != null) + { + // REVIEW: Do we need to do normalization? casing/loc? + if (context.User.Identities.Any(i => string.Equals(i.Name, requirement.RequiredName))) + { + context.Succeed(requirement); + } + } + } + } +} diff --git a/src/Microsoft.AspNet.Authorization/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Authorization/Properties/Resources.Designer.cs index b8ab205960..b627118275 100644 --- a/src/Microsoft.AspNet.Authorization/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Authorization/Properties/Resources.Designer.cs @@ -10,54 +10,6 @@ namespace Microsoft.AspNet.Authorization private static readonly ResourceManager _resourceManager = new ResourceManager("Microsoft.AspNet.Authorization.Resources", typeof(Resources).GetTypeInfo().Assembly); - /// - /// The default data protection provider may only be used when the IApplicationBuilder.Properties contains an appropriate 'host.AppName' key. - /// - internal static string Exception_DefaultDpapiRequiresAppNameKey - { - get { return GetString("Exception_DefaultDpapiRequiresAppNameKey"); } - } - - /// - /// The default data protection provider may only be used when the IApplicationBuilder.Properties contains an appropriate 'host.AppName' key. - /// - internal static string FormatException_DefaultDpapiRequiresAppNameKey() - { - return GetString("Exception_DefaultDpapiRequiresAppNameKey"); - } - - /// - /// The state passed to UnhookAuthentication may only be the return value from HookAuthentication. - /// - internal static string Exception_UnhookAuthenticationStateType - { - get { return GetString("Exception_UnhookAuthenticationStateType"); } - } - - /// - /// The state passed to UnhookAuthentication may only be the return value from HookAuthentication. - /// - internal static string FormatException_UnhookAuthenticationStateType() - { - return GetString("Exception_UnhookAuthenticationStateType"); - } - - /// - /// The AuthenticationTokenProvider's required synchronous events have not been registered. - /// - internal static string Exception_AuthenticationTokenDoesNotProvideSyncMethods - { - get { return GetString("Exception_AuthenticationTokenDoesNotProvideSyncMethods"); } - } - - /// - /// The AuthenticationTokenProvider's required synchronous events have not been registered. - /// - internal static string FormatException_AuthenticationTokenDoesNotProvideSyncMethods() - { - return GetString("Exception_AuthenticationTokenDoesNotProvideSyncMethods"); - } - /// /// The AuthorizationPolicy named: '{0}' was not found. /// @@ -74,6 +26,22 @@ namespace Microsoft.AspNet.Authorization return string.Format(CultureInfo.CurrentCulture, GetString("Exception_AuthorizationPolicyNotFound"), p0); } + /// + /// At least one role must be specified. + /// + internal static string Exception_RoleRequirementEmpty + { + get { return GetString("Exception_RoleRequirementEmpty"); } + } + + /// + /// At least one role must be specified. + /// + internal static string FormatException_RoleRequirementEmpty() + { + return GetString("Exception_RoleRequirementEmpty"); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Authorization/Resources.resx b/src/Microsoft.AspNet.Authorization/Resources.resx index 3e72c4a2ce..ee3f8cd88a 100644 --- a/src/Microsoft.AspNet.Authorization/Resources.resx +++ b/src/Microsoft.AspNet.Authorization/Resources.resx @@ -117,16 +117,10 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - The default data protection provider may only be used when the IApplicationBuilder.Properties contains an appropriate 'host.AppName' key. - - - The state passed to UnhookAuthentication may only be the return value from HookAuthentication. - - - The AuthenticationTokenProvider's required synchronous events have not been registered. - The AuthorizationPolicy named: '{0}' was not found. + + At least one role must be specified. + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Authorization/RolesAuthorizationRequirement.cs b/src/Microsoft.AspNet.Authorization/RolesAuthorizationRequirement.cs new file mode 100644 index 0000000000..f3336237ea --- /dev/null +++ b/src/Microsoft.AspNet.Authorization/RolesAuthorizationRequirement.cs @@ -0,0 +1,47 @@ +// 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.Collections.Generic; +using System.Linq; +using Microsoft.Framework.Internal; + +namespace Microsoft.AspNet.Authorization +{ + // Must belong to with one of specified roles + // If AllowedRoles is null or empty, that means any role is valid + public class RolesAuthorizationRequirement : AuthorizationHandler, IAuthorizationRequirement + { + public RolesAuthorizationRequirement([NotNull] IEnumerable allowedRoles) + { + if (allowedRoles.Count() == 0) + { + throw new InvalidOperationException(Resources.Exception_RoleRequirementEmpty); + } + AllowedRoles = allowedRoles; + } + + public IEnumerable AllowedRoles { get; } + + public override void Handle(AuthorizationContext context, RolesAuthorizationRequirement requirement) + { + if (context.User != null) + { + bool found = false; + if (requirement.AllowedRoles == null || !requirement.AllowedRoles.Any()) + { + // Review: What do we want to do here? No roles requested is auto success? + } + else + { + found = requirement.AllowedRoles.Any(r => context.User.IsInRole(r)); + } + if (found) + { + context.Succeed(requirement); + } + } + } + + } +} diff --git a/src/Microsoft.AspNet.Authorization/ServiceCollectionExtensions.cs b/src/Microsoft.AspNet.Authorization/ServiceCollectionExtensions.cs index 4deced289f..d77512a64e 100644 --- a/src/Microsoft.AspNet.Authorization/ServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNet.Authorization/ServiceCollectionExtensions.cs @@ -23,8 +23,6 @@ namespace Microsoft.Framework.DependencyInjection { services.AddOptions(); services.TryAdd(ServiceDescriptor.Transient()); - services.AddTransient(); - services.AddTransient(); services.AddTransient(); return services; } diff --git a/test/Microsoft.AspNet.Authorization.Test/AuthorizationPolicyFacts.cs b/test/Microsoft.AspNet.Authorization.Test/AuthorizationPolicyFacts.cs index 535e8d5278..208c53b1c3 100644 --- a/test/Microsoft.AspNet.Authorization.Test/AuthorizationPolicyFacts.cs +++ b/test/Microsoft.AspNet.Authorization.Test/AuthorizationPolicyFacts.cs @@ -1,6 +1,7 @@ // 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 Microsoft.AspNet.Authorization; using Xunit; @@ -9,6 +10,12 @@ namespace Microsoft.AspNet.Authroization.Test { public class AuthorizationPolicyFacts { + [Fact] + public void RequireRoleThrowsIfEmpty() + { + Assert.Throws(() => new AuthorizationPolicyBuilder().RequireRole()); + } + [Fact] public void CanCombineAuthorizeAttributes() { @@ -32,7 +39,8 @@ namespace Microsoft.AspNet.Authroization.Test Assert.True(combined.ActiveAuthenticationSchemes.Contains("roles")); Assert.Equal(4, combined.Requirements.Count()); Assert.True(combined.Requirements.Any(r => r is DenyAnonymousAuthorizationRequirement)); - Assert.Equal(3, combined.Requirements.OfType().Count()); + Assert.Equal(2, combined.Requirements.OfType().Count()); + Assert.Equal(1, combined.Requirements.OfType().Count()); } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Authorization.Test/DefaultAuthorizationServiceTests.cs b/test/Microsoft.AspNet.Authorization.Test/DefaultAuthorizationServiceTests.cs index 4c26f10b3b..2683eddce6 100644 --- a/test/Microsoft.AspNet.Authorization.Test/DefaultAuthorizationServiceTests.cs +++ b/test/Microsoft.AspNet.Authorization.Test/DefaultAuthorizationServiceTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Security.Claims; using System.Threading.Tasks; @@ -504,6 +505,50 @@ namespace Microsoft.AspNet.Authorization.Test Assert.True(allowed); } + [Fact] + public async Task CanRequireUserNameWithDiffClaimType() + { + // Arrange + var authorizationService = BuildAuthorizationService(services => + { + services.ConfigureAuthorization(options => + { + options.AddPolicy("Hao", policy => policy.RequireUserName("Hao")); + }); + }); + var identity = new ClaimsIdentity("AuthType", "Name", "Role"); + identity.AddClaim(new Claim("Name", "Hao")); + var user = new ClaimsPrincipal(identity); + + // Act + var allowed = await authorizationService.AuthorizeAsync(user, null, "Hao"); + + // Assert + Assert.True(allowed); + } + + [Fact] + public async Task CanRequireRoleWithDiffClaimType() + { + // Arrange + var authorizationService = BuildAuthorizationService(services => + { + services.ConfigureAuthorization(options => + { + options.AddPolicy("Hao", policy => policy.RequireRole("Hao")); + }); + }); + var identity = new ClaimsIdentity("AuthType", "Name", "Role"); + identity.AddClaim(new Claim("Role", "Hao")); + var user = new ClaimsPrincipal(identity); + + // Act + var allowed = await authorizationService.AuthorizeAsync(user, null, "Hao"); + + // Assert + Assert.True(allowed); + } + [Fact] public async Task CanApproveAnyAuthenticatedUser() {