Non-base64 decode-able authentication throws

[avalara-stephen-hickey]

Description of the problem you are seeing

When the Authorization header has a value that is non-base64 decode-able, the middleware throws an exception that it does not handle.

What do you think should be happening?

The same thing that happens when invalid credentials are supplied - an AuthenticationFail result is returned.

What is actually happening?

An exception is thrown with the message "Failed to decode credentials : {non-decode-able credentials}".

My ConfigureServices() code

services.AddAuthentication(BasicAuthenticationDefaults.AuthenticationScheme)
             .AddBasic(options =>
             {
                 options.Realm = "example";
             });

Steps to reproduce

Send a request with the header "Authorization: Basic abc"

[blowdart]

In all the standard auth middlewares this is the pattern, which you can then handle via the AuthenticationFailed failed event. Given this can be an indication of concern I don't feel comfortable supressing it in such a way.

[avalara-stephen-hickey]

I don't think the failure should be suppressed, but I don't think the middleware should be throwing an uncaught exception.

catch (Exception ex)
{
    var authenticationFailedContext = new BasicAuthenticationFailedContext(Context, Scheme, Options)
    {
        Exception = ex
    };

    await Events.AuthenticationFailed(authenticationFailedContext);

    if (authenticationFailedContext.Result != null)
    {
        return authenticationFailedContext.Result;
    }

    throw;
}

In this block of code from the handler, the authenticationFailedContext.Result is always null because there's no way to set Result on the context in the AuthenticationFailed event. How do you recommend I set the result inside my event as I don't see a way to set that.

[blowdart]

Hmm @Tratcher any thoughts on how this should behave?

[Tratcher]

Agreed, it's good to catch exceptions for bad data input like bad base64, log it, and convert it to an auth failure rather than an app exception.

That catch-all is for unexpected failures.

[blowdart]

Darn it. OK, I'll see what time I have this weekend.

[blowdart]

OK all done and published in the 2.2.0 package.

[avalara-stephen-hickey]

Confirmed working as expected. Thank you for your work!