Handle missing Authorization header on login path

[optiz0r]

Its a fairly typical usage pattern for clients to not pre-emptively send kerberos credentials in a HTTP request, but react to a 401 response containing WWW-Authenticate: Negotiate and retry with credentials.

Prior to this change, the plugin would panic when request.Headers is nil due to an unchecked call to len(nil). This change checks whether the Authorization header was set, and tests that before attempting to access it, thus avoiding the panic. The code will continue until the absence of an authorizationString results in a 401 authentication required response, as expected.

[hashicorp-cla]

All committers have signed the CLA.

[nvx]

Prior to this change, the plugin would panic when request.Headers is nil due to an unchecked call to len(nil)

Calling len() on a nil slice does not panic, it instead returns 0. See https://play.golang.org/p/IxMIvGefxL- for some example code demonstrating the behaviour.

[optiz0r]

Maybe that's changed in newer versions of go since this patch was written?

We have been running a fork containing this PR in production for over a year. It was written to solve a panic in the plugin that would happen without this patch.

The playground outputs value [] len 0, not value nil len 0, so that behaviour doesn't appear to be the same as I reported a year ago. the panic was happening due to len(nil) not len([]).

[nvx]

net/http.Header is defined as

type Header map[string][]string

Accessing a non-existent map key will return the zero value of that type, which for a slice is nil. But there's a difference between a typed and an untyped nil, hence the println showing [] not nil since it's looking up the type information. This is demonstrated here with an added foo == nil check showing that foo is indeed nil https://play.golang.org/p/8Io6BhQddYB

I'm not aware of any language change that would have affected this behaviour, especially not in the last couple of years. The type of net/http.Header has not changed, nor has the behaviour of len() on a nil slice.

I saw this PR and that it fixed a panic so naturally was quite interested as I've just started using Kerberos with Vault so a potential crash bug would be rather important, but I can't see how this would have occurred either on current or previous versions.

[fairclothjm]

Thanks for the contribution! However, this change is not necessary.