diff --git a/src/Microsoft.AspNetCore.Authorization/AuthorizationOptions.cs b/src/Microsoft.AspNetCore.Authorization/AuthorizationOptions.cs index fa9e9ef1ee..6899913afb 100644 --- a/src/Microsoft.AspNetCore.Authorization/AuthorizationOptions.cs +++ b/src/Microsoft.AspNetCore.Authorization/AuthorizationOptions.cs @@ -13,6 +13,12 @@ namespace Microsoft.AspNetCore.Authorization { private IDictionary PolicyMap { get; } = new Dictionary(StringComparer.OrdinalIgnoreCase); + /// + /// Determines whether authentication handlers should be invoked after a failure. + /// Defaults to true. + /// + public bool InvokeHandlersAfterFailure { get; set; } = true; + /// /// Gets or sets the default authorization policy. /// diff --git a/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationService.cs b/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationService.cs index 89777eab01..8980d51ac6 100644 --- a/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationService.cs +++ b/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationService.cs @@ -8,6 +8,7 @@ using System.Security.Claims; using System.Security.Principal; using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Authorization { @@ -16,6 +17,7 @@ namespace Microsoft.AspNetCore.Authorization /// public class DefaultAuthorizationService : IAuthorizationService { + private readonly AuthorizationOptions _options; private readonly IAuthorizationHandlerContextFactory _contextFactory; private readonly IAuthorizationEvaluator _evaluator; private readonly IAuthorizationPolicyProvider _policyProvider; @@ -28,7 +30,7 @@ namespace Microsoft.AspNetCore.Authorization /// The used to provide policies. /// The handlers used to fulfill s. /// The logger used to log messages, warnings and errors. - public DefaultAuthorizationService(IAuthorizationPolicyProvider policyProvider, IEnumerable handlers, ILogger logger) : this(policyProvider, handlers, logger, new DefaultAuthorizationHandlerContextFactory(), new DefaultAuthorizationEvaluator()) { } + public DefaultAuthorizationService(IAuthorizationPolicyProvider policyProvider, IEnumerable handlers, ILogger logger) : this(policyProvider, handlers, logger, new DefaultAuthorizationHandlerContextFactory(), new DefaultAuthorizationEvaluator(), Options.Create(new AuthorizationOptions())) { } /// /// Creates a new instance of . @@ -38,8 +40,13 @@ namespace Microsoft.AspNetCore.Authorization /// The logger used to log messages, warnings and errors. /// The used to create the context to handle the authorization. /// The used to determine if authorzation was successful. - public DefaultAuthorizationService(IAuthorizationPolicyProvider policyProvider, IEnumerable handlers, ILogger logger, IAuthorizationHandlerContextFactory contextFactory, IAuthorizationEvaluator evaluator) + /// The used. + public DefaultAuthorizationService(IAuthorizationPolicyProvider policyProvider, IEnumerable handlers, ILogger logger, IAuthorizationHandlerContextFactory contextFactory, IAuthorizationEvaluator evaluator, IOptions options) { + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } if (policyProvider == null) { throw new ArgumentNullException(nameof(policyProvider)); @@ -61,6 +68,7 @@ namespace Microsoft.AspNetCore.Authorization throw new ArgumentNullException(nameof(evaluator)); } + _options = options.Value; _handlers = handlers.ToArray(); _policyProvider = policyProvider; _logger = logger; @@ -89,6 +97,10 @@ namespace Microsoft.AspNetCore.Authorization foreach (var handler in _handlers) { await handler.HandleAsync(authContext); + if (!_options.InvokeHandlersAfterFailure && authContext.HasFailed) + { + break; + } } if (_evaluator.HasSucceeded(authContext)) diff --git a/test/Microsoft.AspNetCore.Authorization.Test/DefaultAuthorizationServiceTests.cs b/test/Microsoft.AspNetCore.Authorization.Test/DefaultAuthorizationServiceTests.cs index 7f9e5642ae..039740ab2f 100644 --- a/test/Microsoft.AspNetCore.Authorization.Test/DefaultAuthorizationServiceTests.cs +++ b/test/Microsoft.AspNetCore.Authorization.Test/DefaultAuthorizationServiceTests.cs @@ -20,11 +20,7 @@ namespace Microsoft.AspNetCore.Authorization.Test var services = new ServiceCollection(); services.AddAuthorization(); services.AddLogging(); - services.AddOptions(); - if (setupServices != null) - { - setupServices(services); - } + setupServices?.Invoke(services); return services.BuildServiceProvider().GetRequiredService(); } @@ -109,6 +105,72 @@ namespace Microsoft.AspNetCore.Authorization.Test Assert.True(allowed); } + public async Task Authorize_ShouldInvokeAllHandlersByDefault() + { + // Arrange + var handler1 = new FailHandler(); + var handler2 = new FailHandler(); + var authorizationService = BuildAuthorizationService(services => + { + services.AddSingleton(handler1); + services.AddSingleton(handler2); + services.AddAuthorization(options => + { + options.AddPolicy("Custom", policy => policy.Requirements.Add(new CustomRequirement())); + }); + }); + + // Act + var allowed = await authorizationService.AuthorizeAsync(new ClaimsPrincipal(), "Custom"); + + // Assert + Assert.False(allowed); + Assert.True(handler1.Invoked); + Assert.True(handler2.Invoked); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Authorize_ShouldInvokeAllHandlersDependingOnSetting(bool invokeAllHandlers) + { + // Arrange + var handler1 = new FailHandler(); + var handler2 = new FailHandler(); + var authorizationService = BuildAuthorizationService(services => + { + services.AddSingleton(handler1); + services.AddSingleton(handler2); + services.AddAuthorization(options => + { + options.InvokeHandlersAfterFailure = invokeAllHandlers; + options.AddPolicy("Custom", policy => policy.Requirements.Add(new CustomRequirement())); + }); + }); + + // Act + var allowed = await authorizationService.AuthorizeAsync(new ClaimsPrincipal(), "Custom"); + + // Assert + Assert.False(allowed); + Assert.True(handler1.Invoked); + Assert.Equal(invokeAllHandlers, handler2.Invoked); + } + + private class FailHandler : IAuthorizationHandler + { + public bool Invoked { get; set; } + + public Task HandleAsync(AuthorizationHandlerContext context) + { + Invoked = true; + context.Fail(); + return Task.FromResult(0); + } + } + + + [Fact] public async Task Authorize_ShouldFailWhenAllRequirementsNotHandled() { @@ -584,8 +646,11 @@ namespace Microsoft.AspNetCore.Authorization.Test public class CustomRequirement : IAuthorizationRequirement { } public class CustomHandler : AuthorizationHandler { + public bool Invoked { get; set; } + protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, CustomRequirement requirement) { + Invoked = true; context.Succeed(requirement); return Task.FromResult(0); }