Re: [PATCH v2] remote-curl: fall back to basic auth if Negotiate fails

On 2/16/21 11:44 PM, Junio C Hamano wrote:

"Christopher via GitGitGadget" [off-list ref] writes:

quoted

From: Christopher Schenk <redacted>

When the username and password are supplied in a url like this
https://myuser:secret@git.exampe/myrepo.git and the server supports the
negotiate authenticaten method git does not fall back to basic auth and

s/method git/method, git/;

quoted

libcurl hardly tries to authenticate with the negotiate method.

Thanks.

quoted

Stop using the Negotiate authentication method after the first failure
because if it fails on the first try it will never succeed.

Is this patch needed because we are using cURL library incorrectly,
or is it the limitation of the cURL library?

I'm no cURL expert, but in my opinon this is a limitation of the cURL Libary.

quoted

V1 of this patch somehow did not make it to the mailing list so i will
try to send this patch again

The last paragraph does not belong to the commit log message; if
nobody on the list saw the "v1", as far as the project is concerned,
it never happened.

I have adapted the commit message accordingly.

quoted

Signed-off-by: Christopher Schenk <redacted>
---

quoted

diff --git a/http.c b/http.c
index 8b23a546afdf..36f113d46c23 100644
--- a/http.c
+++ b/http.c

@@ -1642,6 +1642,14 @@ static int handle_curl_result(struct slot_results *results)
  		return HTTP_MISSING_TARGET;
  	else if (results->http_code == 401) {
  		if (http_auth.username && http_auth.password) {
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+			if (results->auth_avail & CURLAUTH_GSSNEGOTIATE) {
+				http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+				http_auth_methods &= results->auth_avail;
+				http_auth_methods_restricted = 1;
+				return HTTP_REAUTH;
+			}
+#endif
  			credential_reject(&http_auth);
  			return HTTP_NOAUTH;
  		} else {

Hmph, is this an extension to what 4dbe6646 (remote-curl: fall back
to Basic auth if Negotiate fails, 2015-01-08) tried to do?  What
happens on the "else" side after the context of this patch, which
seems to have come from:

  - 4dbe6646 (remote-curl: fall back to Basic auth if Negotiate
    fails, 2015-01-08)
  
  - 840398fe (http: restrict auth methods to what the server
    advertises, 2017-02-22), and

  - 40a18fc7 (http: add an "auto" mode for http.emptyauth,
    2017-02-25),

looks essentially the same as what this patch is adding, and I am
wondering if there is a room for simplification.  It almost looks
to me that the only difference between "credential fully given" and
other case is if we "reject" the credential after this patch.

Asking contributors who made these past contributions for input.

Thanks.

I have simplified the code and sent the patch again.

Thanks.