Add FailureReasons (#35425)

HaoK · pranavkm · commit aa04f8ee8ed8 · 2021-08-23T17:51:19.000-07:00
diff --git a/src/Security/Authorization/Core/src/AuthorizationFailure.cs b/src/Security/Authorization/Core/src/AuthorizationFailure.cs
@@ -15,7 +15,7 @@ public class AuthorizationFailure
         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 @@ private AuthorizationFailure() { }
         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> FailureReasons { 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,
+                FailureReasons = reasons
             };
 
         /// <summary>
diff --git a/src/Security/Authorization/Core/src/AuthorizationFailureReason.cs 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; }
+
+        /// <summary>
+        /// The <see cref="IAuthorizationHandler"/> responsible for this failure reason.
+        /// </summary>
+        public IAuthorizationHandler Handler { get; }
+    }
+}
diff --git 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 List<AuthorizationFailureReason>? _failedReasons;
         private bool _failCalled;
         private bool _succeedCalled;
 
@@ -59,6 +60,12 @@ public AuthorizationHandlerContext(
         /// </summary>
         public virtual IEnumerable<IAuthorizationRequirement> PendingRequirements { get { return _pendingRequirements; } }
 
+        /// <summary>
+        /// Gets the reasons why authorization has failed.
+        /// </summary>
+        public virtual IEnumerable<AuthorizationFailureReason> FailureReasons
+            => (IEnumerable<AuthorizationFailureReason>?)_failedReasons ?? Array.Empty<AuthorizationFailureReason>();
+
         /// <summary>
         /// Flag indicating whether the current authorization processing has failed.
         /// </summary>
@@ -84,6 +91,25 @@ public virtual void Fail()
             _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)
+            {
+                if (_failedReasons == null)
+                {
+                    _failedReasons = new List<AuthorizationFailureReason>();
+                }
+
+                _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
@@ -17,7 +17,7 @@ public AuthorizationResult Evaluate(AuthorizationHandlerContext context)
             => 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
@@ -1,3 +1,11 @@
 #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.FailureReasons.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.Message.get -> string!
+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
@@ -174,6 +174,54 @@ public Task HandleAsync(AuthorizationHandlerContext context)
             }
         }
 
+        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.FailureReasons.Count());
+            var first = allowed.Failure.FailureReasons.First();
+            Assert.Equal("1", first.Message);
+            Assert.Equal(handler1, first.Handler);
+            var second = allowed.Failure.FailureReasons.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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -31,6 +31,14 @@ public async Task HandleAsync(
             // if the authorization was forbidden, let's use custom logic to handle that.
             if (policyAuthorizationResult.Forbidden && policyAuthorizationResult.AuthorizationFailure != null)
             {
+                if (policyAuthorizationResult.AuthorizationFailure.FailureReasons.Any())
+                {
+                    await httpContext.Response.WriteAsync(policyAuthorizationResult.AuthorizationFailure.FailureReasons.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
@@ -7,5 +7,6 @@ public static class SamplePolicyNames
     {
         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
@@ -24,5 +24,12 @@ public string GetWithCustomPolicy()
         {
             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
@@ -37,13 +37,17 @@ public void ConfigureServices(IServiceCollection services)
             {
                 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()));
             });
 
             services.AddTransient<IAuthorizationHandler, SampleRequirementHandler>();
             services.AddTransient<IAuthorizationHandler, SampleWithCustomMessageRequirementHandler>();
+            services.AddTransient<IAuthorizationHandler, SampleWithFailureReasonRequirementHandler>();
             services.AddTransient<IAuthorizationMiddlewareResultHandler, SampleAuthorizationMiddlewareResultHandler>();
         }
 

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ public AuthorizationResult Evaluate(AuthorizationHandlerContext context)`
`17`	`17`	`=> context.HasSucceeded`
`18`	`18`	`? AuthorizationResult.Success()`
`19`	`19`	`: AuthorizationResult.Failed(context.HasFailed`
`20`		`- ? AuthorizationFailure.ExplicitFail()`
	`20`	`+ ? AuthorizationFailure.Failed(context.FailureReasons)`
`21`	`21`	`: AuthorizationFailure.Failed(context.PendingRequirements));`
`22`	`22`	`}`
`23`	`23`	`}`