#1044 Revert "Auth: Always call prior handlers during Challenge"

This reverts commit e12838e.
aspnet · Jan 19, 2017 · e609cda · e609cda
1 parent 673df3e
commit e609cda
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 10 deletions.
diff --git a/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs b/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs
@@ -327,7 +327,6 @@ protected virtual Task HandleSignOutAsync(SignOutContext context)
         /// Override this method to deal with a challenge that is forbidden.
         /// </summary>
         /// <param name="context"></param>
-        /// <returns>The returned boolean is ignored.</returns>
         protected virtual Task<bool> HandleForbiddenAsync(ChallengeContext context)
         {
             Response.StatusCode = 403;
@@ -340,7 +339,7 @@ protected virtual Task<bool> HandleForbiddenAsync(ChallengeContext context)
         /// changing the 401 result to 302 of a login page or external sign-in location.)
         /// </summary>
         /// <param name="context"></param>
-        /// <returns>The returned boolean is no longer used.</returns>
+        /// <returns>True if no other handlers should be called</returns>
         protected virtual Task<bool> HandleUnauthorizedAsync(ChallengeContext context)
         {
             Response.StatusCode = 401;
@@ -350,6 +349,7 @@ protected virtual Task<bool> HandleUnauthorizedAsync(ChallengeContext context)
         public async Task ChallengeAsync(ChallengeContext context)
         {
             ChallengeCalled = true;
+            var handled = false;
             if (ShouldHandleScheme(context.AuthenticationScheme, Options.AutomaticChallenge))
             {
                 switch (context.Behavior)
@@ -363,18 +363,18 @@ public async Task ChallengeAsync(ChallengeContext context)
                         }
                         goto case ChallengeBehavior.Unauthorized;
                     case ChallengeBehavior.Unauthorized:
-                        await HandleUnauthorizedAsync(context);
+                        handled = await HandleUnauthorizedAsync(context);
                         Logger.AuthenticationSchemeChallenged(Options.AuthenticationScheme);
                         break;
                     case ChallengeBehavior.Forbidden:
-                        await HandleForbiddenAsync(context);
+                        handled = await HandleForbiddenAsync(context);
                         Logger.AuthenticationSchemeForbidden(Options.AuthenticationScheme);
                         break;
                 }
                 context.Accept();
             }
 
-            if (PriorHandler != null)
+            if (!handled && PriorHandler != null)
             {
                 await PriorHandler.ChallengeAsync(context);
             }

diff --git a/test/Microsoft.AspNetCore.Authentication.Test/AuthenticationHandlerFacts.cs b/test/Microsoft.AspNetCore.Authentication.Test/AuthenticationHandlerFacts.cs
@@ -75,16 +75,17 @@ public async Task AuthHandlerAuthenticateCachesTicket(string scheme)
             Assert.Equal(1, handler.AuthCount);
         }
 
-        // Prior to https://github.com/aspnet/Security/issues/930 we wouldn't call prior if handled
-        [Fact]
-        public async Task AuthHandlerChallengeAlwaysCallsPriorHandler()
+        [Theory]
+        [InlineData("Alpha", false)]
+        [InlineData("Bravo", true)]
+        public async Task AuthHandlerChallengeCallsPriorHandlerIfNotHandled(string challenge, bool passedThrough)
         {
             var handler = await TestHandler.Create("Alpha");
             var previous = new PreviousHandler();
 
             handler.PriorHandler = previous;
-            await handler.ChallengeAsync(new ChallengeContext("Alpha"));
-            Assert.True(previous.ChallengeCalled);
+            await handler.ChallengeAsync(new ChallengeContext(challenge));
+            Assert.Equal(passedThrough, previous.ChallengeCalled);
         }
 
         private class PreviousHandler : IAuthenticationHandler