Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Do I have to check the response status in ApplyResponseChallengeAsync?

I've written a fairly basic AuthenticationHandler<T> derived class for performing custom authentication for my REST services.

I had assumed (yes, I know, bad idea) that ApplyResponseChallengeAsync would only be called if I actually need to apply my challenge - e.g. it's described as:

Override this method to dela(sic) with 401 challenge concerns, if an authentication scheme in question deals an authentication interaction as part of it's request flow. (like adding a response header, or changing the 401 result to 302 of a login page or external sign-in location.)

Which sounded to me that it would only be called if a 401 was being issued. However, in some limited testing, we're seeing some exceptions as follows:

System.Web.HttpException (0x80004005): Server cannot append header after HTTP headers have been sent.
at System.Web.HttpHeaderCollection.SetHeader(String name, String value, Boolean replace)
at Microsoft.Owin.Host.SystemWeb.CallHeaders.AspNetResponseHeaders.Set(String key, String[] values)
at Microsoft.Owin.Infrastructure.OwinHelpers.AppendHeader(IDictionary`2 headers, String key, String values)
at OurAuthHandler.ApplyResponseChallengeAsync()
at Microsoft.Owin.Security.Infrastructure.AuthenticationHandler.<ApplyResponseCoreAsync>d__8.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Owin.Security.Infrastructure.AuthenticationHandler.<TeardownAsync>d__5.MoveNext()
--- And so on

So, wanting to investigate this, I changed the code inside our method slightly so that I could use the debugger to inspect the circumstances for this exception occurring:

protected override Task ApplyResponseChallengeAsync()
{
    try
    {
        foreach (var uri in Options.IdentityConfiguration.AudienceRestriction.AllowedAudienceUris)
        {
            Response.Headers.Append("WWW-Authenticate", "Bearer realm=\"" + uri + "\"");
        }
        return base.ApplyResponseChallengeAsync();
    }
    catch
    {
        throw; //Set a breakpoint here
    }
}

And, lo and behold, when my breakpoint is hit, I'm seeing that the Responses status code is 200/OK.

The question(s)

So, the question is, am I meant to have to check the status code myself, is there some flag I have to pass/set somewhere so that this method is only called for 401s, or am I missing something else?

like image 777
Damien_The_Unbeliever Avatar asked Mar 26 '14 11:03

Damien_The_Unbeliever


1 Answers

Yes, you have to check the status code yourself. The documentation is misleading.

Note how each of the existing AuthenticationHandlers in the Katana project check the status code, too:

public class OpenIdConnectAuthenticationHandler : AuthenticationHandler<OpenIdConnectAuthenticationOptions>
{
    ...
    protected override async Task ApplyResponseChallengeAsync()
    {
        if (Response.StatusCode == 401)
        {
            ....
        }
    }
    ...
}

internal class TwitterAuthenticationHandler : AuthenticationHandler<TwitterAuthenticationOptions>
{
    ...
    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2202:Do not dispose objects multiple times", Justification = "MemoryStream.Dispose is idempotent")]
    protected override async Task ApplyResponseChallengeAsync()
    {
        if (Response.StatusCode != 401)
        {
            return;
        }
    }
    ...
}

public class WsFederationAuthenticationHandler : AuthenticationHandler<WsFederationAuthenticationOptions>
{
    ...
    protected override async Task ApplyResponseChallengeAsync()
    {
        if (Response.StatusCode == 401)
        {
            ...
        }
    }
    ...
}

I also checked the source code of the Katana project: there's no way to change this behaviour via a flag or something.

like image 157
sloth Avatar answered Sep 17 '22 23:09

sloth