| Message ID | pull.849.v2.git.1613494656636.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] remote-curl: fall back to basic auth if Negotiate fails | expand |
On 2/16/21 11:44 PM, Junio C Hamano wrote: > "Christopher via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Christopher Schenk <christopher@cschenk.net> >> >> 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/; > >> libcurl hardly tries to authenticate with the negotiate method. > > Thanks. > >> 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. > >> 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. > >> Signed-off-by: Christopher Schenk <christopher@cschenk.net> >> --- > >> 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.
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 {