ignore unsupported http authentication contexts

[palmin]

auth_context_match returns -1 when the authentication context is not supported,
but this made parse_authenticate_response fail in situations where some authentication
contexts are supported and others are not.

parse_authenticate_response will instead ignore authentication contexts where
auth_context_match returns -1

This specifically handles recent changes to visualstudio.com that responds with:

HTTP/1.1 401 Unauthorized
WWW-Authenticate: Bearer authorization_uri=https://login.microsoftonline.com/a4c0aef7-6fdd-4c0e-b678-0dd23c73d08a
WWW-Authenticate: Basic realm=\"https://tfsprodweu2.app.visualstudio.com/\"
WWW-Authenticate: TFS-Federated

when asking for authentication and the http transport didn't know what to do with WWW-Authenticate: Bearer returning error=-1 without any further explanation.

I presume the intersection of libgit2 and visualstudio.com users are mostly using the winhttp transport but this change improves behaviour when using the plain http transport.

[ethomson]

🤔

One problem here is that it seems that we have overloaded the return value of auth_context_match, and it's returning -1 when there was no scheme match. I think that might be the root cause of the problems here.

I think that we should change auth_context_match to return 0 (and leave context unset) when there's no match, instead of returning -1. I think that this bit can then remain unchanged - on auth_context_match returning -1 on error, it will propagate this error, and returning 0 without setting context, it will continue.

[palmin]

Yes, this might be a better idea.

The only other usage is by apply_credentials and I'm not sure if this needs to do something more to handle the situation where return value is 0 but the output context is NULL?

libgit2/src/transports/http.c

Lines 169 to 192 in a8d447f

	static int apply_credentials(git_buf *buf, http_subtransport *t)
	{
	git_cred *cred = t->cred;
	git_http_auth_context *context;
	/* Apply the credentials given to us in the URL */
	if (!cred && t->connection_data.user && t->connection_data.pass) {
	if (!t->url_cred &&
	git_cred_userpass_plaintext_new(&t->url_cred,
	t->connection_data.user, t->connection_data.pass) < 0)
	return -1;
	cred = t->url_cred;
	}
	if (!cred)
	return 0;
	/* Get or create a context for the best scheme for this cred type */
	if (auth_context_match(&context, t, credtype_match, &cred->credtype) < 0)
	return -1;
	return context->next_token(buf, context, cred);
	}

[ethomson]

Ooh, good call. A quick glance suggests that yes, we should probably return 0 there but I'd appreciate another set of eyes. :)

[palmin]

Pushed a new commit such that auth_context_matchreturns 0 for unknown schemes and such that apply_credentials is prepared for that.

[palmin]

Will clean this up by squashing the commits into one and making sure whitespace matches the rest of the file.

[ethomson]

Thanks for this!