Returning true from HandleUnauthorizedAsync doesn't prevent the other automatic handlers from being invoked #930
Description
When refactoring ASOS code, I discovered a weird bug in AuthenticationHandler that prevents HandleUnauthorizedAsync from working correctly. The following conditions must be met for this annoying bug to occur:
- The ASP.NET Core pipeline must contain at least 2 authentication middleware (e.g cookies + a custom middleware).
- The first authentication middleware must be configured with
AutomaticChallenge = true. The challenge mode - automatic or manual - used for the second middleware is not important. - The second authentication middleware must override
HandleUnauthorizedAsyncand must returntrue(which is supposed to prevent the prior handlers from being invoked). AuthenticationManager.ChallengeAsyncmust be called with the authentication scheme associed with the second middleware.
When all these conditions are met, returning true from HandleUnauthorizedAsync has no effect: the first automatic middleware is still ultimately called and overrides the challenge applied by the second one (which is the only one that should handle it).
This bug is likely caused by AuthenticationHandler.HandleAutomaticChallengeIfNeeded.
Unlike AuthenticationHandler.ChallengeAsync, this method - executed by all the authentication handlers in the pipeline when headers are sent - doesn't check whether the prior handler handled the challenge and blindly calls HandleUnauthorizedAsync.
private async Task HandleAutomaticChallengeIfNeeded()
{
if (!ChallengeCalled && Options.AutomaticChallenge && Response.StatusCode == 401)
{
await HandleUnauthorizedAsync(new ChallengeContext(Options.AuthenticationScheme));
}
}Ironically, the JWT middleware is not impacted by this bug because it returns false to allow the other middleware to handle the challenge: the ChallengeAsync of all the prior handlers is called, which automatically sets ChallengeCalled. Since the authentication scheme doesn't correspond to the prior middleware, ChallengeAsync ignores the challenge and things work like they are supposed to.