git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 8/8] http: prompt for credentials on failed POST

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 27, 2012, 17:48 UTC
Message-ID
<7v3938rztf.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20120827132714.GH17375@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 8 quoted lines
> Most of the time, this is not a big deal; for both fetching
> and pushing, we make a GET request before doing any POSTs,
> so typically we figure out the credentials during the first
> request, then reuse them during the POST. However, some
> servers may allow a client to get the list of refs from
> receive-pack without authentication, and then require
> authentication when the client actually tries to POST the
> pack.

A silly question. Does the initial GET request when we push look any different from the initial GET request when we fetch? Can we make them look different in an updated client, so that the server side can say "this GET is about pushing into us, and we require authentication"?

Show 30 quoted lines
> Unfortunately, it is not as easy as simply calling post_rpc
> again when it fails, due to the input issue mentioned above.
> However, we can still make this specific case work by
> retrying in two specific instances:
>
>   1. If the request is large (bigger than LARGE_PACKET_MAX),
>      we will first send a probe request with a single flush
>      packet. Since this request is static, we can freely
>      retry it.
>
>   2. If the request is small and we are not using gzip, then
>      we have the whole thing in-core, and we can freely
>      retry.
>
> That means we will not retry in some instances, including:
>
>   1. If we are using gzip. However, we only do so when
>      calling git-upload-pack, so it does not apply to
>      pushes.
>
>   2. If we have a large request, the probe succeeds, but
>      then the real POST wants authentication. This is an
>      extremely unlikely configuration and not worth worrying
>      about.
>
> While it might be nice to cover those instances, doing so
> would be significantly more complex for very little
> real-world gain. In the long run, we will be much better off
> when curl learns to internally handle authentication as a
> callback, and we can cleanly handle all cases that way.

I suspect that in real life, almost nobody runs smart HTTP server that allows anonymous push.

How much usability penalty would it be if we always fill credential before pushing? Alternatively, how much latency penalty would it incur if we always send a probe request regardless of the request size when we try to push without having an authentication material?

Show 79 quoted lines
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Sorry for the wordy explanation. I really tried to refactor this into a
> nice single code path for making both GET and POST requests, but I think
> there are just too many corner cases. Suggestions welcome if somebody
> has a better idea of how to refactor it (preferably in the form of a
> patch).
>
>  remote-curl.c        | 23 +++++++++++++++--------
>  t/t5541-http-push.sh |  2 +-
>  2 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/remote-curl.c b/remote-curl.c
> index 04a9d62..3ec474f 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -362,16 +362,17 @@ static size_t rpc_in(char *ptr, size_t eltsize,
>  
>  static int run_slot(struct active_request_slot *slot)
>  {
> -	int err = 0;
> +	int err;
>  	struct slot_results results;
>  
>  	slot->results = &results;
>  	slot->curl_result = curl_easy_perform(slot->curl);
>  	finish_active_slot(slot);
>  
> -	if (results.curl_result != CURLE_OK) {
> -		err |= error("RPC failed; result=%d, HTTP code = %ld",
> -			results.curl_result, results.http_code);
> +	err = handle_curl_result(slot);
> +	if (err != HTTP_OK && err != HTTP_REAUTH) {
> +		error("RPC failed; result=%d, HTTP code = %ld",
> +		      results.curl_result, results.http_code);
>  	}
>  
>  	return err;
> @@ -436,9 +437,11 @@ static int post_rpc(struct rpc_state *rpc)
>  	}
>  
>  	if (large_request) {
> -		err = probe_rpc(rpc);
> -		if (err)
> -			return err;
> +		do {
> +			err = probe_rpc(rpc);
> +		} while (err == HTTP_REAUTH);
> +		if (err != HTTP_OK)
> +			return -1;
>  	}
>  
>  	slot = get_active_slot();
> @@ -525,7 +528,11 @@ static int post_rpc(struct rpc_state *rpc)
>  	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
>  	curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
>  
> -	err = run_slot(slot);
> +	do {
> +		err = run_slot(slot);
> +	} while (err == HTTP_REAUTH && !large_request && !use_gzip);
> +	if (err != HTTP_OK)
> +		err = -1;
>  
>  	curl_slist_free_all(headers);
>  	free(gzip_body);
> diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
> index 9b1cd60..ef6d6b6 100755
> --- a/t/t5541-http-push.sh
> +++ b/t/t5541-http-push.sh
> @@ -280,7 +280,7 @@ test_expect_success 'push over smart http with auth' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure 'push to auth-only-for-push repo' '
> +test_expect_success 'push to auth-only-for-push repo' '
>  	cd "$ROOT_PATH/test_repo_clone" &&
>  	echo push-half-auth >expect &&
>  	test_commit push-half-auth &&
Previous: Jeff KingNext: Jeff King
Message 17 of 22 in “git no longer prompting for password”
  1. Iain PatonAug 24, 2012
  2. Jeff KingAug 24, 2012
  3. Jeff KingAug 25, 2012
  4. Iain PatonAug 26, 2012
  5. Jeff KingAug 26, 2012
  6. Iain PatonAug 26, 2012
  7. 0/8 fix password prompting for "half-auth" serversJeff King, Aug 27, 2012
  8. 1/8 t5550: put auth-required repo in auth/dumbJeff King, Aug 27, 2012
  9. 2/8 t5550: factor out http auth setupJeff King, Aug 27, 2012
  10. 3/8 t/lib-httpd: only route auth/dumb to dumb reposJeff King, Aug 27, 2012
  11. 4/8 t/lib-httpd: recognize */smart/* repos as smart-httpJeff King, Aug 27, 2012
  12. 5/8 t: test basic smart-http authenticationJeff King, Aug 27, 2012
  13. 6/8 t: test http access to "half-auth" repositoriesJeff King, Aug 27, 2012
  14. 7/8 http: factor out http error code handlingJeff King, Aug 27, 2012
  15. Junio C HamanoAug 28, 2012
  16. 8/8 http: prompt for credentials on failed POSTJeff King, Aug 27, 2012
  17. Junio C HamanoAug 27, 2012
  18. Jeff KingAug 27, 2012
  19. Junio C HamanoAug 27, 2012
  20. Junio C HamanoAug 27, 2012
  21. Iain PatonAug 27, 2012
  22. BJ HargraveAug 27, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.