From 4c2d727e38b593e4c1ebb8cf002618c2552912a2 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 25 May 2017 18:23:41 -0700 Subject: [PATCH] React to Auth + switch to Policy Evaluator --- .../Authorization/AuthorizeFilter.cs | 34 +--- .../MvcCoreMvcCoreBuilderExtensions.cs | 1 + .../Microsoft.AspNetCore.Mvc.Core.csproj | 2 +- .../Authorization/AuthorizeFilterTest.cs | 192 +++--------------- .../ChallengeResultTest.cs | 4 +- .../ForbidResultTest.cs | 12 +- 6 files changed, 52 insertions(+), 193 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs index 25ca6d0b94..fe492d701f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs @@ -9,6 +9,7 @@ using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Authorization.Policy; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Internal; @@ -125,26 +126,9 @@ namespace Microsoft.AspNetCore.Mvc.Authorization return; } - // Build a ClaimsPrincipal with the Policy's required authentication types - if (effectivePolicy.AuthenticationSchemes != null && effectivePolicy.AuthenticationSchemes.Count > 0) - { - ClaimsPrincipal newPrincipal = null; - for (var i = 0; i < effectivePolicy.AuthenticationSchemes.Count; i++) - { - var scheme = effectivePolicy.AuthenticationSchemes[i]; - var result = await context.HttpContext.AuthenticateAsync(scheme); - if (result.Succeeded) - { - newPrincipal = SecurityHelper.MergeUserPrincipal(newPrincipal, result.Principal); - } - } - // If all schemes failed authentication, provide a default identity anyways - if (newPrincipal == null) - { - newPrincipal = new ClaimsPrincipal(new ClaimsIdentity()); - } - context.HttpContext.User = newPrincipal; - } + var policyEvaluator = context.HttpContext.RequestServices.GetRequiredService(); + + var authenticateResult = await policyEvaluator.AuthenticateAsync(effectivePolicy, context.HttpContext); // Allow Anonymous skips all authorization if (context.Filters.Any(item => item is IAllowAnonymousFilter)) @@ -152,14 +136,16 @@ namespace Microsoft.AspNetCore.Mvc.Authorization return; } - var httpContext = context.HttpContext; - var authService = httpContext.RequestServices.GetRequiredService(); + var authorizeResult = await policyEvaluator.AuthorizeAsync(effectivePolicy, authenticateResult, context.HttpContext); - // Note: Default Anonymous User is new ClaimsPrincipal(new ClaimsIdentity()) - if (!await authService.AuthorizeAsync(httpContext.User, context, effectivePolicy)) + if (authorizeResult.Challenged) { context.Result = new ChallengeResult(effectivePolicy.AuthenticationSchemes.ToArray()); } + else if (authorizeResult.Forbidden) + { + context.Result = new ForbidResult(effectivePolicy.AuthenticationSchemes.ToArray()); + } } IFilterMetadata IFilterFactory.CreateInstance(IServiceProvider serviceProvider) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs index 641206da77..73eedfa8a7 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs @@ -92,6 +92,7 @@ namespace Microsoft.Extensions.DependencyInjection { services.AddAuthenticationCore(); services.AddAuthorization(); + services.AddAuthorizationPolicyEvaluator(); services.TryAddEnumerable( ServiceDescriptor.Transient()); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj b/src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj index 6fc978d20b..d8f5e7919e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj +++ b/src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj @@ -22,7 +22,7 @@ Microsoft.AspNetCore.Mvc.RouteAttribute - + diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs index 9c57379b6d..d20adafd8f 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Authorization/AuthorizeFilterTest.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization [Fact] public void InvalidUser() { - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + var authorizationContext = GetAuthorizationContext(); Assert.True(authorizationContext.HttpContext.User.Identities.Any(i => i.IsAuthenticated)); } @@ -31,7 +31,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization { // Arrange var authorizeFilter = new AuthorizeFilter(new[] { new AuthorizeAttribute() }); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + var authorizationContext = GetAuthorizationContext(); var expected = "An AuthorizationPolicy cannot be created without a valid instance of " + "IAuthorizationPolicyProvider."; @@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization { // Arrange var authorizeFilter = new AuthorizeFilter(new[] { new AuthorizeAttribute() }); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + var authorizationContext = GetAuthorizationContext(); var expected = "An AuthorizationPolicy cannot be created without a valid instance of " + "IAuthorizationPolicyProvider."; @@ -61,7 +61,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization { // Arrange var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization(), anonymous: true); + var authorizationContext = GetAuthorizationContext(anonymous: true); authorizationContext.HttpContext.User = new ClaimsPrincipal(); // Act @@ -80,7 +80,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization 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()); + var authorizationContext = GetAuthorizationContext(); // Act & Assert await authorizeFilter.OnAuthorizationAsync(authorizationContext); @@ -104,7 +104,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization { // Arrange var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization(), anonymous: true); + var authorizationContext = GetAuthorizationContext(anonymous: true); // Act await authorizeFilter.OnAuthorizationAsync(authorizationContext); @@ -118,7 +118,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization { // Arrange var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireClaim("Permission", "CanViewPage").Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + var authorizationContext = GetAuthorizationContext(); // Act await authorizeFilter.OnAuthorizationAsync(authorizationContext); @@ -128,19 +128,17 @@ namespace Microsoft.AspNetCore.Mvc.Authorization } [Fact] - public async Task Invoke_EmptyClaimsShouldRejectAnonymousUser() + public async Task Invoke_EmptyClaimsShouldChallengeAnonymousUser() { // Arrange var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build()); - var authorizationContext = GetAuthorizationContext(services => - services.AddAuthorization(), - anonymous: true); + var authorizationContext = GetAuthorizationContext(anonymous: true); // Act await authorizeFilter.OnAuthorizationAsync(authorizationContext); // Assert - Assert.NotNull(authorizationContext.Result); + Assert.IsType(authorizationContext.Result); } [Fact] @@ -148,8 +146,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization { // Arrange var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization(), - anonymous: true); + var authorizationContext = GetAuthorizationContext(anonymous: true); authorizationContext.Filters.Add(new AllowAnonymousFilter()); @@ -165,7 +162,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization { // Arrange var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + var authorizationContext = GetAuthorizationContext(); // Act await authorizeFilter.OnAuthorizationAsync(authorizationContext); @@ -181,7 +178,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder("Fails") .RequireAuthenticatedUser() .Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + var authorizationContext = GetAuthorizationContext(); // Act await authorizeFilter.OnAuthorizationAsync(authorizationContext); @@ -195,7 +192,7 @@ namespace Microsoft.AspNetCore.Mvc.Authorization { // Arrange var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireClaim("Permission", "CanViewComment", "CanViewPage").Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + var authorizationContext = GetAuthorizationContext(); // Act await authorizeFilter.OnAuthorizationAsync(authorizationContext); @@ -205,162 +202,33 @@ namespace Microsoft.AspNetCore.Mvc.Authorization } [Fact] - public async Task Invoke_RequireAdminRoleShouldFailWithNoHandlers() - { - // Arrange - var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireRole("Administrator").Build()); - var authorizationContext = GetAuthorizationContext(services => - { - services.AddOptions(); - services.AddAuthorization(); - - services.Remove(services.Where(sd => sd.ServiceType == typeof(IAuthorizationHandler)).Single()); - }); - - // Act - await authorizeFilter.OnAuthorizationAsync(authorizationContext); - - // Assert - Assert.NotNull(authorizationContext.Result); - } - - [Fact] - public async Task Invoke_RequireAdminAndUserRoleWithNoPolicyShouldSucceed() - { - // Arrange - var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireRole("Administrator").Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); - - // Act - await authorizeFilter.OnAuthorizationAsync(authorizationContext); - - // Assert - Assert.Null(authorizationContext.Result); - } - - [Fact] - public async Task Invoke_RequireUnknownRoleShouldFail() + public async Task Invoke_RequireUnknownRoleShouldForbid() { // Arrange var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireRole("Wut").Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + var authorizationContext = GetAuthorizationContext(); // Act await authorizeFilter.OnAuthorizationAsync(authorizationContext); // Assert - Assert.NotNull(authorizationContext.Result); + Assert.IsType(authorizationContext.Result); } [Fact] - public async Task Invoke_RequireAdminRoleButFailPolicyShouldFail() - { - // Arrange - var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder() - .RequireRole("Administrator") - .RequireClaim("Permission", "CanViewComment") - .Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); - - // Act - await authorizeFilter.OnAuthorizationAsync(authorizationContext); - - // Assert - Assert.NotNull(authorizationContext.Result); - } - - [Fact] - public async Task Invoke_InvalidClaimShouldFail() + public async Task Invoke_InvalidClaimShouldForbid() { // Arrange var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder() .RequireClaim("Permission", "CanViewComment") .Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); + var authorizationContext = GetAuthorizationContext(); // Act await authorizeFilter.OnAuthorizationAsync(authorizationContext); // Assert - Assert.NotNull(authorizationContext.Result); - } - - [Fact] - public async Task Invoke_FailedContextShouldNotCheckPermission() - { - // Arrange - bool authorizationServiceIsCalled = false; - var authorizationService = new Mock(); - authorizationService - .Setup(x => x.AuthorizeAsync(null, null, "CanViewComment")) - .Returns(() => - { - authorizationServiceIsCalled = true; - return Task.FromResult(true); - }); - - var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder() - .RequireClaim("Permission", "CanViewComment") - .Build()); - var authorizationContext = GetAuthorizationContext(services => - services.AddSingleton(authorizationService.Object)); - - authorizationContext.Result = new UnauthorizedResult(); - - // Act - await authorizeFilter.OnAuthorizationAsync(authorizationContext); - - // Assert - Assert.False(authorizationServiceIsCalled); - } - - [Fact] - public async Task Invoke_FailWhenLookingForClaimInOtherIdentity() - { - // Arrange - var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder() - .RequireClaim("Permission", "CanViewComment") - .Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); - - // Act - await authorizeFilter.OnAuthorizationAsync(authorizationContext); - - // Assert - Assert.NotNull(authorizationContext.Result); - } - - [Fact] - public async Task Invoke_CanLookingForClaimsInMultipleIdentities() - { - // Arrange - var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder("Basic", "Bearer") - .RequireClaim("Permission", "CanViewComment") - .RequireClaim("Permission", "CupBearer") - .Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); - - // Act - await authorizeFilter.OnAuthorizationAsync(authorizationContext); - - // Assert - Assert.NotNull(authorizationContext.Result); - } - - [Fact] - public async Task Invoke_CanFilterToOnlyBearerScheme() - { - // Arrange - var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder("Bearer") - .RequireClaim("Permission", "CanViewPage") - .Build()); - var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization()); - - // Act - await authorizeFilter.OnAuthorizationAsync(authorizationContext); - - // Assert - Assert.NotNull(authorizationContext.Result); + Assert.IsType(authorizationContext.Result); } [Fact] @@ -455,8 +323,8 @@ namespace Microsoft.AspNetCore.Mvc.Authorization } private AuthorizationFilterContext GetAuthorizationContext( - Action registerServices, - bool anonymous = false) + bool anonymous = false, + Action registerServices = null) { var basicPrincipal = new ClaimsPrincipal( new ClaimsIdentity( @@ -482,12 +350,16 @@ namespace Microsoft.AspNetCore.Mvc.Authorization // ServiceProvider var serviceCollection = new ServiceCollection(); + var auth = new Mock(); + + serviceCollection.AddOptions(); + serviceCollection.AddLogging(); + serviceCollection.AddSingleton(auth.Object); + serviceCollection.AddAuthorization(); + serviceCollection.AddAuthorizationPolicyEvaluator(); if (registerServices != null) { - serviceCollection.AddOptions(); - serviceCollection.AddLogging(); - serviceCollection.AddSingleton(auth.Object); registerServices(serviceCollection); } @@ -495,15 +367,15 @@ namespace Microsoft.AspNetCore.Mvc.Authorization // HttpContext var httpContext = new Mock(); + auth.Setup(c => c.AuthenticateAsync(httpContext.Object, "Bearer")).ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(bearerPrincipal, "Bearer"))); + auth.Setup(c => c.AuthenticateAsync(httpContext.Object, "Basic")).ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(basicPrincipal, "Basic"))); + auth.Setup(c => c.AuthenticateAsync(httpContext.Object, "Fails")).ReturnsAsync(AuthenticateResult.Fail("Fails")); httpContext.SetupProperty(c => c.User); if (!anonymous) { httpContext.Object.User = validUser; } httpContext.SetupGet(c => c.RequestServices).Returns(serviceProvider); - auth.Setup(c => c.AuthenticateAsync(httpContext.Object, "Bearer")).ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(bearerPrincipal, "Bearer"))); - auth.Setup(c => c.AuthenticateAsync(httpContext.Object, "Basic")).ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(basicPrincipal, "Basic"))); - auth.Setup(c => c.AuthenticateAsync(httpContext.Object, "Fails")).ReturnsAsync(AuthenticateResult.Fail("Fails")); // AuthorizationFilterContext var actionContext = new ActionContext( diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ChallengeResultTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ChallengeResultTest.cs index 289d4339a9..b1ec100e5f 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ChallengeResultTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ChallengeResultTest.cs @@ -39,7 +39,7 @@ namespace Microsoft.AspNetCore.Mvc await result.ExecuteResultAsync(actionContext); // Assert - auth.Verify(c => c.ChallengeAsync(httpContext.Object, "", null, ChallengeBehavior.Automatic), Times.Exactly(1)); + auth.Verify(c => c.ChallengeAsync(httpContext.Object, "", null), Times.Exactly(1)); } [Fact] @@ -64,7 +64,7 @@ namespace Microsoft.AspNetCore.Mvc await result.ExecuteResultAsync(actionContext); // Assert - auth.Verify(c => c.ChallengeAsync(httpContext.Object, null, null, ChallengeBehavior.Automatic), Times.Exactly(1)); + auth.Verify(c => c.ChallengeAsync(httpContext.Object, null, null), Times.Exactly(1)); } private static IServiceCollection CreateServices() diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ForbidResultTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ForbidResultTest.cs index 2991dab6f0..c9c895f154 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ForbidResultTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ForbidResultTest.cs @@ -19,13 +19,13 @@ namespace Microsoft.AspNetCore.Mvc public class ForbidResultTest { [Fact] - public async Task ExecuteResultAsync_InvokesForbidAsyncOnAuthenticationManager() + public async Task ExecuteResultAsync_InvokesForbidAsyncOnAuthenticationService() { // Arrange var httpContext = new Mock(); var auth = new Mock(); auth - .Setup(c => c.ChallengeAsync(httpContext.Object, "", null, ChallengeBehavior.Forbidden)) + .Setup(c => c.ForbidAsync(httpContext.Object, "", null)) .Returns(TaskCache.CompletedTask) .Verifiable(); httpContext.Setup(c => c.RequestServices).Returns(CreateServices(auth.Object)); @@ -52,11 +52,11 @@ namespace Microsoft.AspNetCore.Mvc var authProperties = new AuthenticationProperties(); var auth = new Mock(); auth - .Setup(c => c.ChallengeAsync(httpContext.Object, "Scheme1", authProperties, ChallengeBehavior.Forbidden)) + .Setup(c => c.ForbidAsync(httpContext.Object, "Scheme1", authProperties)) .Returns(TaskCache.CompletedTask) .Verifiable(); auth - .Setup(c => c.ChallengeAsync(httpContext.Object, "Scheme2", authProperties, ChallengeBehavior.Forbidden)) + .Setup(c => c.ForbidAsync(httpContext.Object, "Scheme2", authProperties)) .Returns(TaskCache.CompletedTask) .Verifiable(); httpContext.Setup(c => c.RequestServices).Returns(CreateServices(auth.Object)); @@ -90,7 +90,7 @@ namespace Microsoft.AspNetCore.Mvc var httpContext = new Mock(); var auth = new Mock(); auth - .Setup(c => c.ChallengeAsync(httpContext.Object, null, expected, ChallengeBehavior.Forbidden)) + .Setup(c => c.ForbidAsync(httpContext.Object, null, expected)) .Returns(TaskCache.CompletedTask) .Verifiable(); httpContext.Setup(c => c.RequestServices).Returns(CreateServices(auth.Object)); @@ -118,7 +118,7 @@ namespace Microsoft.AspNetCore.Mvc var httpContext = new Mock(); var auth = new Mock(); auth - .Setup(c => c.ChallengeAsync(httpContext.Object, null, expected, ChallengeBehavior.Forbidden)) + .Setup(c => c.ForbidAsync(httpContext.Object, null, expected)) .Returns(TaskCache.CompletedTask) .Verifiable(); httpContext.Setup(c => c.RequestServices).Returns(CreateServices(auth.Object));