From 08bdcfd2873906d8948dd7bf47093d52b6767d61 Mon Sep 17 00:00:00 2001 From: Hao Kung <HaoK@users.noreply.github.com> Date: Tue, 17 Aug 2021 20:57:51 -0700 Subject: [PATCH] Add FailureReasons (#35425) --- .../Core/src/AuthorizationFailure.cs | 20 +++++++- .../Core/src/AuthorizationFailureReason.cs | 32 +++++++++++++ .../Core/src/AuthorizationHandlerContext.cs | 21 ++++++++ .../Core/src/DefaultAuthorizationEvaluator.cs | 2 +- .../Core/src/PublicAPI.Unshipped.txt | 10 ++++ .../test/DefaultAuthorizationServiceTests.cs | 48 +++++++++++++++++++ ...yAnonymousAuthorizationRequirementTests.cs | 3 -- .../test/NameAuthorizationRequirementTests.cs | 3 -- .../OperationAuthorizationRequirementTests.cs | 3 -- .../RolesAuthorizationRequirementTests.cs | 3 -- ...mpleWithFailureReasonRequirementHandler.cs | 18 +++++++ .../SampleFailReasonRequirement.cs | 11 +++++ ...pleAuthorizationMiddlewareResultHandler.cs | 8 ++++ .../Authorization/SamplePolicyNames.cs | 1 + .../Controllers/SampleController.cs | 7 +++ .../Startup.cs | 4 ++ 16 files changed, 179 insertions(+), 15 deletions(-) create mode 100644 src/Security/Authorization/Core/src/AuthorizationFailureReason.cs create mode 100644 src/Security/samples/CustomAuthorizationFailureResponse/Authorization/Handlers/SampleWithFailureReasonRequirementHandler.cs create mode 100644 src/Security/samples/CustomAuthorizationFailureResponse/Authorization/Requirements/SampleFailReasonRequirement.cs diff --git a/src/Security/Authorization/Core/src/AuthorizationFailure.cs b/src/Security/Authorization/Core/src/AuthorizationFailure.cs index 722329b865f..64a5c8d1093 100644 --- a/src/Security/Authorization/Core/src/AuthorizationFailure.cs +++ b/src/Security/Authorization/Core/src/AuthorizationFailure.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Authorization private AuthorizationFailure() { } /// <summary> - /// Failure was due to <see cref="AuthorizationHandlerContext.Fail"/> being called. + /// Failure was due to <see cref="AuthorizationHandlerContext.Fail()"/> being called. /// </summary> public bool FailCalled { get; private set; } @@ -25,13 +25,29 @@ namespace Microsoft.AspNetCore.Authorization public IEnumerable<IAuthorizationRequirement> FailedRequirements { get; private set; } = Array.Empty<IAuthorizationRequirement>(); /// <summary> - /// Return a failure due to <see cref="AuthorizationHandlerContext.Fail"/> being called. + /// Allows <see cref="IAuthorizationHandler"/> to flow more detailed reasons for why authorization failed. + /// </summary> + public IEnumerable<AuthorizationFailureReason> Reasons { get; private set; } = Array.Empty<AuthorizationFailureReason>(); + + /// <summary> + /// Return a failure due to <see cref="AuthorizationHandlerContext.Fail()"/> being called. /// </summary> /// <returns>The failure.</returns> public static AuthorizationFailure ExplicitFail() + => new AuthorizationFailure + { + FailCalled = true + }; + + /// <summary> + /// Return a failure due to <see cref="AuthorizationHandlerContext.Fail(AuthorizationFailureReason)"/> being called. + /// </summary> + /// <returns>The failure.</returns> + public static AuthorizationFailure Failed(IEnumerable<AuthorizationFailureReason> reasons) => new AuthorizationFailure { FailCalled = true, + Reasons = reasons }; /// <summary> diff --git a/src/Security/Authorization/Core/src/AuthorizationFailureReason.cs b/src/Security/Authorization/Core/src/AuthorizationFailureReason.cs new file mode 100644 index 00000000000..b8c30f67d1d --- /dev/null +++ b/src/Security/Authorization/Core/src/AuthorizationFailureReason.cs @@ -0,0 +1,32 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Authorization +{ + /// <summary> + /// Encapsulates a reason why authorization failed. + /// </summary> + public class AuthorizationFailureReason + { + /// <summary> + /// Creates a new failure reason. + /// </summary> + /// <param name="handler">The handler responsible for this failure reason.</param> + /// <param name="message">The message describing the failure.</param> + public AuthorizationFailureReason(IAuthorizationHandler handler, string message) + { + Handler = handler; + Message = message; + } + + /// <summary> + /// A message describing the failure reason. + /// </summary> + public string Message { get; set; } + + /// <summary> + /// The <see cref="IAuthorizationHandler"/> responsible for this failure reason. + /// </summary> + public IAuthorizationHandler Handler { get; set; } + } +} diff --git a/src/Security/Authorization/Core/src/AuthorizationHandlerContext.cs b/src/Security/Authorization/Core/src/AuthorizationHandlerContext.cs index 2ad35047c4b..c89cc5cd216 100644 --- a/src/Security/Authorization/Core/src/AuthorizationHandlerContext.cs +++ b/src/Security/Authorization/Core/src/AuthorizationHandlerContext.cs @@ -14,6 +14,7 @@ namespace Microsoft.AspNetCore.Authorization public class AuthorizationHandlerContext { private readonly HashSet<IAuthorizationRequirement> _pendingRequirements; + private readonly List<AuthorizationFailureReason> _failedReasons; private bool _failCalled; private bool _succeedCalled; @@ -37,6 +38,7 @@ namespace Microsoft.AspNetCore.Authorization User = user; Resource = resource; _pendingRequirements = new HashSet<IAuthorizationRequirement>(requirements); + _failedReasons = new List<AuthorizationFailureReason>(); } /// <summary> @@ -59,6 +61,11 @@ namespace Microsoft.AspNetCore.Authorization /// </summary> public virtual IEnumerable<IAuthorizationRequirement> PendingRequirements { get { return _pendingRequirements; } } + /// <summary> + /// Gets the reasons why authorization has failed. + /// </summary> + public virtual IEnumerable<AuthorizationFailureReason> FailureReasons { get { return _failedReasons; } } + /// <summary> /// Flag indicating whether the current authorization processing has failed. /// </summary> @@ -84,6 +91,20 @@ namespace Microsoft.AspNetCore.Authorization _failCalled = true; } + /// <summary> + /// Called to indicate <see cref="AuthorizationHandlerContext.HasSucceeded"/> will + /// never return true, even if all requirements are met. + /// </summary> + /// <param name="reason">Optional <see cref="AuthorizationFailureReason"/> for why authorization failed.</param> + public virtual void Fail(AuthorizationFailureReason reason) + { + Fail(); + if (reason != null) + { + _failedReasons.Add(reason); + } + } + /// <summary> /// Called to mark the specified <paramref name="requirement"/> as being /// successfully evaluated. diff --git a/src/Security/Authorization/Core/src/DefaultAuthorizationEvaluator.cs b/src/Security/Authorization/Core/src/DefaultAuthorizationEvaluator.cs index 0055568f6f4..eb612716d28 100644 --- a/src/Security/Authorization/Core/src/DefaultAuthorizationEvaluator.cs +++ b/src/Security/Authorization/Core/src/DefaultAuthorizationEvaluator.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Authorization => context.HasSucceeded ? AuthorizationResult.Success() : AuthorizationResult.Failed(context.HasFailed - ? AuthorizationFailure.ExplicitFail() + ? AuthorizationFailure.Failed(context.FailureReasons) : AuthorizationFailure.Failed(context.PendingRequirements)); } } diff --git a/src/Security/Authorization/Core/src/PublicAPI.Unshipped.txt b/src/Security/Authorization/Core/src/PublicAPI.Unshipped.txt index 613684928cc..bed63d21e9d 100644 --- a/src/Security/Authorization/Core/src/PublicAPI.Unshipped.txt +++ b/src/Security/Authorization/Core/src/PublicAPI.Unshipped.txt @@ -1,3 +1,13 @@ #nullable enable *REMOVED*static Microsoft.AspNetCore.Authorization.AuthorizationServiceExtensions.AuthorizeAsync(this Microsoft.AspNetCore.Authorization.IAuthorizationService! service, System.Security.Claims.ClaimsPrincipal! user, object! resource, Microsoft.AspNetCore.Authorization.IAuthorizationRequirement! requirement) -> System.Threading.Tasks.Task<Microsoft.AspNetCore.Authorization.AuthorizationResult!>! +Microsoft.AspNetCore.Authorization.AuthorizationFailure.Reasons.get -> System.Collections.Generic.IEnumerable<Microsoft.AspNetCore.Authorization.AuthorizationFailureReason!>! +Microsoft.AspNetCore.Authorization.AuthorizationFailureReason +Microsoft.AspNetCore.Authorization.AuthorizationFailureReason.AuthorizationFailureReason(Microsoft.AspNetCore.Authorization.IAuthorizationHandler! handler, string! message) -> void +Microsoft.AspNetCore.Authorization.AuthorizationFailureReason.Handler.get -> Microsoft.AspNetCore.Authorization.IAuthorizationHandler! +Microsoft.AspNetCore.Authorization.AuthorizationFailureReason.Handler.set -> void +Microsoft.AspNetCore.Authorization.AuthorizationFailureReason.Message.get -> string! +Microsoft.AspNetCore.Authorization.AuthorizationFailureReason.Message.set -> void +static Microsoft.AspNetCore.Authorization.AuthorizationFailure.Failed(System.Collections.Generic.IEnumerable<Microsoft.AspNetCore.Authorization.AuthorizationFailureReason!>! reasons) -> Microsoft.AspNetCore.Authorization.AuthorizationFailure! static Microsoft.AspNetCore.Authorization.AuthorizationServiceExtensions.AuthorizeAsync(this Microsoft.AspNetCore.Authorization.IAuthorizationService! service, System.Security.Claims.ClaimsPrincipal! user, object? resource, Microsoft.AspNetCore.Authorization.IAuthorizationRequirement! requirement) -> System.Threading.Tasks.Task<Microsoft.AspNetCore.Authorization.AuthorizationResult!>! +virtual Microsoft.AspNetCore.Authorization.AuthorizationHandlerContext.Fail(Microsoft.AspNetCore.Authorization.AuthorizationFailureReason! reason) -> void +virtual Microsoft.AspNetCore.Authorization.AuthorizationHandlerContext.FailureReasons.get -> System.Collections.Generic.IEnumerable<Microsoft.AspNetCore.Authorization.AuthorizationFailureReason!>! diff --git a/src/Security/Authorization/test/DefaultAuthorizationServiceTests.cs b/src/Security/Authorization/test/DefaultAuthorizationServiceTests.cs index 41e42e0c952..ebd2dbcbd78 100644 --- a/src/Security/Authorization/test/DefaultAuthorizationServiceTests.cs +++ b/src/Security/Authorization/test/DefaultAuthorizationServiceTests.cs @@ -174,6 +174,54 @@ namespace Microsoft.AspNetCore.Authorization.Test } } + private class ReasonableFailHandler : IAuthorizationHandler + { + private string _reason; + + public ReasonableFailHandler(string reason) => _reason = reason; + + public bool Invoked { get; set; } + + public Task HandleAsync(AuthorizationHandlerContext context) + { + Invoked = true; + context.Fail(new AuthorizationFailureReason(this, _reason)); + return Task.FromResult(0); + } + } + + [Fact] + public async Task CanFailWithReasons() + { + var handler1 = new ReasonableFailHandler("1"); + var handler2 = new FailHandler(); + var handler3 = new ReasonableFailHandler("3"); + var authorizationService = BuildAuthorizationService(services => + { + services.AddSingleton<IAuthorizationHandler>(handler1); + services.AddSingleton<IAuthorizationHandler>(handler2); + services.AddSingleton<IAuthorizationHandler>(handler3); + 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.Succeeded); + Assert.NotNull(allowed.Failure); + Assert.Equal(2, allowed.Failure.Reasons.Count()); + var first = allowed.Failure.Reasons.First(); + Assert.Equal("1", first.Message); + Assert.Equal(handler1, first.Handler); + var second = allowed.Failure.Reasons.Last(); + Assert.Equal("3", second.Message); + Assert.Equal(handler3, second.Handler); + } + [Fact] public async Task Authorize_ShouldFailWhenAllRequirementsNotHandled() { diff --git a/src/Security/Authorization/test/DenyAnonymousAuthorizationRequirementTests.cs b/src/Security/Authorization/test/DenyAnonymousAuthorizationRequirementTests.cs index ed0dac0de82..11824a78a67 100644 --- a/src/Security/Authorization/test/DenyAnonymousAuthorizationRequirementTests.cs +++ b/src/Security/Authorization/test/DenyAnonymousAuthorizationRequirementTests.cs @@ -1,9 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Text; using Microsoft.AspNetCore.Authorization.Infrastructure; using Xunit; diff --git a/src/Security/Authorization/test/NameAuthorizationRequirementTests.cs b/src/Security/Authorization/test/NameAuthorizationRequirementTests.cs index cc653d27634..f8ef42b7d16 100644 --- a/src/Security/Authorization/test/NameAuthorizationRequirementTests.cs +++ b/src/Security/Authorization/test/NameAuthorizationRequirementTests.cs @@ -1,9 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Text; using Microsoft.AspNetCore.Authorization.Infrastructure; using Xunit; diff --git a/src/Security/Authorization/test/OperationAuthorizationRequirementTests.cs b/src/Security/Authorization/test/OperationAuthorizationRequirementTests.cs index 75b3c7392da..0c70f5860e9 100644 --- a/src/Security/Authorization/test/OperationAuthorizationRequirementTests.cs +++ b/src/Security/Authorization/test/OperationAuthorizationRequirementTests.cs @@ -1,9 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Text; using Microsoft.AspNetCore.Authorization.Infrastructure; using Xunit; diff --git a/src/Security/Authorization/test/RolesAuthorizationRequirementTests.cs b/src/Security/Authorization/test/RolesAuthorizationRequirementTests.cs index b2cab60b3f3..f69212e0470 100644 --- a/src/Security/Authorization/test/RolesAuthorizationRequirementTests.cs +++ b/src/Security/Authorization/test/RolesAuthorizationRequirementTests.cs @@ -1,9 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Text; using Microsoft.AspNetCore.Authorization.Infrastructure; using Xunit; diff --git a/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/Handlers/SampleWithFailureReasonRequirementHandler.cs b/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/Handlers/SampleWithFailureReasonRequirementHandler.cs new file mode 100644 index 00000000000..29ef971649d --- /dev/null +++ b/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/Handlers/SampleWithFailureReasonRequirementHandler.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Threading.Tasks; +using CustomAuthorizationFailureResponse.Authorization.Requirements; +using Microsoft.AspNetCore.Authorization; + +namespace CustomAuthorizationFailureResponse.Authorization.Handlers +{ + public class SampleWithFailureReasonRequirementHandler : AuthorizationHandler<SampleFailReasonRequirement> + { + protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, SampleFailReasonRequirement requirement) + { + context.Fail(new AuthorizationFailureReason(this, "This is a way to provide more failure reasons.")); + return Task.CompletedTask; + } + } +} diff --git a/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/Requirements/SampleFailReasonRequirement.cs b/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/Requirements/SampleFailReasonRequirement.cs new file mode 100644 index 00000000000..6926dcb7050 --- /dev/null +++ b/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/Requirements/SampleFailReasonRequirement.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Authorization; + +namespace CustomAuthorizationFailureResponse.Authorization.Requirements +{ + public class SampleFailReasonRequirement : IAuthorizationRequirement + { + } +} diff --git a/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/SampleAuthorizationMiddlewareResultHandler.cs b/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/SampleAuthorizationMiddlewareResultHandler.cs index 2a128498c20..be7e8d311f1 100644 --- a/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/SampleAuthorizationMiddlewareResultHandler.cs +++ b/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/SampleAuthorizationMiddlewareResultHandler.cs @@ -31,6 +31,14 @@ namespace CustomAuthorizationFailureResponse.Authorization // if the authorization was forbidden, let's use custom logic to handle that. if (policyAuthorizationResult.Forbidden && policyAuthorizationResult.AuthorizationFailure != null) { + if (policyAuthorizationResult.AuthorizationFailure.Reasons.Any()) + { + await httpContext.Response.WriteAsync(policyAuthorizationResult.AuthorizationFailure.Reasons.First().Message); + + // return right away as the default implementation would overwrite the status code + return; + } + // as an example, let's return 404 if specific requirement has failed if (policyAuthorizationResult.AuthorizationFailure.FailedRequirements.Any(requirement => requirement is SampleRequirement)) { diff --git a/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/SamplePolicyNames.cs b/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/SamplePolicyNames.cs index bd09ea18b96..392313b31ca 100644 --- a/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/SamplePolicyNames.cs +++ b/src/Security/samples/CustomAuthorizationFailureResponse/Authorization/SamplePolicyNames.cs @@ -7,5 +7,6 @@ namespace CustomAuthorizationFailureResponse.Authorization { public const string CustomPolicy = "Custom"; public const string CustomPolicyWithCustomForbiddenMessage = "CustomPolicyWithCustomForbiddenMessage"; + public const string FailureReasonPolicy = "FailureReason"; } } diff --git a/src/Security/samples/CustomAuthorizationFailureResponse/Controllers/SampleController.cs b/src/Security/samples/CustomAuthorizationFailureResponse/Controllers/SampleController.cs index ed6834b501b..7f43c30e91a 100644 --- a/src/Security/samples/CustomAuthorizationFailureResponse/Controllers/SampleController.cs +++ b/src/Security/samples/CustomAuthorizationFailureResponse/Controllers/SampleController.cs @@ -24,5 +24,12 @@ namespace CustomAuthorizationFailureResponse.Controllers { return "Hello world from GetWithCustomPolicy"; } + + [HttpGet("failureReason")] + [Authorize(Policy = SamplePolicyNames.FailureReasonPolicy)] + public string FailureReason() + { + return "Hello world from FailureReason"; + } } } diff --git a/src/Security/samples/CustomAuthorizationFailureResponse/Startup.cs b/src/Security/samples/CustomAuthorizationFailureResponse/Startup.cs index f970cde5d43..aac2e2eab25 100644 --- a/src/Security/samples/CustomAuthorizationFailureResponse/Startup.cs +++ b/src/Security/samples/CustomAuthorizationFailureResponse/Startup.cs @@ -37,6 +37,9 @@ namespace CustomAuthorizationFailureResponse { options.AddPolicy(SamplePolicyNames.CustomPolicy, policy => policy.AddRequirements(new SampleRequirement())); + + options.AddPolicy(SamplePolicyNames.FailureReasonPolicy, policy => + policy.AddRequirements(new SampleFailReasonRequirement())); options.AddPolicy(SamplePolicyNames.CustomPolicyWithCustomForbiddenMessage, policy => policy.AddRequirements(new SampleWithCustomMessageRequirement())); @@ -44,6 +47,7 @@ namespace CustomAuthorizationFailureResponse services.AddTransient<IAuthorizationHandler, SampleRequirementHandler>(); services.AddTransient<IAuthorizationHandler, SampleWithCustomMessageRequirementHandler>(); + services.AddTransient<IAuthorizationHandler, SampleWithFailureReasonRequirementHandler>(); services.AddTransient<IAuthorizationMiddlewareResultHandler, SampleAuthorizationMiddlewareResultHandler>(); } -- GitLab