diff --git a/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs b/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs index 55cfa5b01..5e9a7c238 100644 --- a/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs @@ -311,6 +311,11 @@ protected virtual Task HandleSignOutAsync(SignOutContext context) return TaskCache.CompletedTask; } + /// + /// 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; @@ -323,7 +328,7 @@ protected virtual Task HandleForbiddenAsync(ChallengeContext context) /// changing the 401 result to 302 of a login page or external sign-in location.) /// /// - /// True if no other handlers should be called + /// The returned boolean is no longer used. protected virtual Task HandleUnauthorizedAsync(ChallengeContext context) { Response.StatusCode = 401; @@ -333,7 +338,6 @@ protected virtual Task HandleUnauthorizedAsync(ChallengeContext context) public async Task ChallengeAsync(ChallengeContext context) { ChallengeCalled = true; - var handled = false; if (ShouldHandleScheme(context.AuthenticationScheme, Options.AutomaticChallenge)) { switch (context.Behavior) @@ -347,18 +351,18 @@ public async Task ChallengeAsync(ChallengeContext context) } goto case ChallengeBehavior.Unauthorized; case ChallengeBehavior.Unauthorized: - handled = await HandleUnauthorizedAsync(context); + await HandleUnauthorizedAsync(context); Logger.AuthenticationSchemeChallenged(Options.AuthenticationScheme); break; case ChallengeBehavior.Forbidden: - handled = await HandleForbiddenAsync(context); + await HandleForbiddenAsync(context); Logger.AuthenticationSchemeForbidden(Options.AuthenticationScheme); break; } context.Accept(); } - if (!handled && PriorHandler != null) + if (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 aa9aef07c..2cf11669d 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/AuthenticationHandlerFacts.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/AuthenticationHandlerFacts.cs @@ -75,6 +75,49 @@ 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() + { + var handler = await TestHandler.Create("Alpha"); + var previous = new PreviousHandler(); + + handler.PriorHandler = previous; + await handler.ChallengeAsync(new ChallengeContext("Alpha")); + Assert.True(previous.ChallengeCalled); + } + + private class PreviousHandler : IAuthenticationHandler + { + public bool ChallengeCalled = false; + + public Task AuthenticateAsync(AuthenticateContext context) + { + throw new NotImplementedException(); + } + + public Task ChallengeAsync(ChallengeContext context) + { + ChallengeCalled = true; + return Task.FromResult(0); + } + + public void GetDescriptions(DescribeSchemesContext context) + { + throw new NotImplementedException(); + } + + public Task SignInAsync(SignInContext context) + { + throw new NotImplementedException(); + } + + public Task SignOutAsync(SignOutContext context) + { + throw new NotImplementedException(); + } + } + private class CountOptions : AuthenticationOptions { } private class CountHandler : AuthenticationHandler @@ -109,6 +152,8 @@ private class TestHandler : AuthenticationHandler { private TestHandler() { } + public AuthenticateResult Result = AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(), new AuthenticationProperties(), "whatever")); + public static async Task Create(string scheme) { var handler = new TestHandler(); @@ -124,7 +169,7 @@ await handler.InitializeAsync( protected override Task HandleAuthenticateAsync() { - return Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(), new AuthenticationProperties(), "whatever"))); + return Task.FromResult(Result); } } @@ -220,7 +265,6 @@ public int StatusCode set { - throw new NotImplementedException(); } }