From da4730a3922dd885fdda013f5efa72010a7d0170 Mon Sep 17 00:00:00 2001 From: Chris R Date: Mon, 12 Dec 2016 15:39:18 -0800 Subject: [PATCH] #1044 Revert "Auth: Always call prior handlers during Challenge" This reverts commit e12838e38f5f8be8371c0a7b02d9ce47e0663ce0. --- .../AuthenticationHandler.cs | 10 +++++----- .../AuthenticationHandlerFacts.cs | 11 ++++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs b/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs index ae32424ebd..8e7e427659 100644 --- a/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs @@ -327,7 +327,6 @@ namespace Microsoft.AspNetCore.Authentication /// Override this method to deal with a challenge that is forbidden. /// /// - /// The returned boolean is ignored. protected virtual Task HandleForbiddenAsync(ChallengeContext context) { Response.StatusCode = 403; @@ -340,7 +339,7 @@ namespace Microsoft.AspNetCore.Authentication /// changing the 401 result to 302 of a login page or external sign-in location.) /// /// - /// The returned boolean is no longer used. + /// True if no other handlers should be called protected virtual Task HandleUnauthorizedAsync(ChallengeContext context) { Response.StatusCode = 401; @@ -350,6 +349,7 @@ namespace Microsoft.AspNetCore.Authentication public async Task ChallengeAsync(ChallengeContext context) { ChallengeCalled = true; + var handled = false; if (ShouldHandleScheme(context.AuthenticationScheme, Options.AutomaticChallenge)) { switch (context.Behavior) @@ -363,18 +363,18 @@ namespace Microsoft.AspNetCore.Authentication } 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 index 2cf11669d3..fade43716e 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/AuthenticationHandlerFacts.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/AuthenticationHandlerFacts.cs @@ -75,16 +75,17 @@ namespace Microsoft.AspNetCore.Authentication 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