Re: [PATCH] remote-curl: fall back to Basic auth if Negotiate fails.

From: Jeff King <peff@peff.net>
Date: Dec 27, 2014, 17:56 UTC
Message-ID: <20141227175604.GA23732@peff.net>
In-Reply-To: <1419652893-477694-1-git-send-email-sandals@crustytoothpaste.net>

On Sat, Dec 27, 2014 at 04:01:33AM +0000, brian m. carlson wrote:

Show 19 quoted lines

> Apache servers using mod_auth_kerb can be configured to allow the user
> to authenticate either using Negotiate (using the Kerberos ticket) or
> Basic authentication (using the Kerberos password).  Often, one will
> want to use Negotiate authentication if it is available, but fall back
> to Basic authentication if the ticket is missing or expired.
> 
> Teach the HTTP client code to stop trying authentication mechanisms that
> don't use a password (currently Negotiate) after the first failure,
> since if they failed the first time, they will never succeed.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> I was able to reproduce the problem on my server.  This fixes the
> problem for me both when info/refs requires authentication and when it
> does not.  Dan, please try and see if this fixes the problem for you.
> 
> I'm not clear on whether NTLM is a passwordless authentication method.
> Since I don't use Windows or NTLM, I can't test it, but if it is, just
> adding it to HTTP_AUTH_PASSWORDLESS should be sufficient.

I don't think this should make things any worse for NTLM if it is. It would just not get the benefit of the feature you are adding, and somebody with a working setup can test and add it at that time, right?

I'm not familiar enough with Negotiate auth to do give a thorough review on the logic above. But FWIW, it makes sense to me, and the code looks correct.

The number of places you needed to add calls to disable_passwordless_auth is a sign that we might need to factor out our http auth handling better. Though that is not a problem you are introducing. :)

The credential struct is already a global for all requests. If you made the "no_passwordless" flag similarly global, it would be enough to set it in handle_curl_result and respect it in get_curl_handle.

-Peff