[PATCH v2] http: preserve wwwauth_headers across redirects

[PATCH v2] http: preserve wwwauth_headers across redirects

[Aaron Plattner]

When cURL follows a redirect, it calls the CURLOPT_HEADERFUNCTION for
each header received including ones from a redirect. http_request() sets
fwrite_wwwauth() as the header function, which will record the wwwauth[]
entries for the last step in the redirection chain.

However, when http_request_recoverable() sees that cURL followed a
redirect, it attempts to update the credentials for the request from the
new URL using credential_from_url(). The first thing that does is call
credential_clear(), which clears everything including wwwauth_headers.

If the new URL should use a credential helper rather than credentials
embedded in the URL, this loses the list of authentication methods that
the server provided in the redirect.

The WWW-Authenticate challenge is not derived from the URL; it is
populated from the server's response, and after a redirect it describes
how to authenticate to the redirect target and it needs to survive the
URL update so that credential helpers can know which authentication
methods are allowed.

Add a new credential_update_url() that wraps credential_from_url() and
preserves wwwauth_headers specifically. Use SWAP() to avoid having to
copy the whole strbuf.

Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
---
I decided to come back to this after I noticed that at least one other
person had run into the same bug:

https://lore.kernel.org/all/CADoNwcscDrx+YcfbcW4YKONDZZQgnPiwEOxL4QYV_C7_=FOFcg@mail.gmail.com/

Rather than reworking everything about how credentials are stored, I
took your advice in [1] and just moved the code to preserve the wwwauth_headers
into credential.c. That way any future credential fields that need to be
preserved can be added there without having to hunt down other places
like http.c that are reaching into it.

[1] https://lore.kernel.org/all/xmqqpl28scll.fsf@gitster.g/
---
Changes in v2:
- Move strvec preservation into a helper function in credential.c
- Use SWAP instead of strvec_pushv() to avoid having to copy the
  contents of the strvec.
- Link to v1: https://patch.msgid.link/20260602161150.1527493-1-aplattner@nvidia.com
---
 credential.c                | 16 ++++++++++++++++
 credential.h                |  8 ++++++++
 http.c                      |  9 ++++++++-
 t/lib-httpd/apache.conf     |  1 +
 t/t5563-simple-http-auth.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/credential.c b/credential.c
index 2594c0c422..035399d7ee 100644
--- a/credential.c
+++ b/credential.c
@@ -708,3 +708,19 @@ void credential_from_url(struct credential *c, const char *url)
 	if (credential_from_url_gently(c, url, 0) < 0)
 		die(_("credential url cannot be parsed: %s"), url);
 }
+
+void credential_update_url(struct credential *c, const char *url)
+{
+	struct strvec wwwauth_headers = STRVEC_INIT;
+
+	/*
+	 * credential_from_url() clears the whole credential. Preserve the
+	 * WWW-Authenticate list, which is derived from the server's original
+	 * response rather than from the URL and is required to authenticate to
+	 * the new URL.
+	 */
+	SWAP(wwwauth_headers, c->wwwauth_headers);
+	credential_from_url(c, url);
+	SWAP(c->wwwauth_headers, wwwauth_headers);
+	strvec_clear(&wwwauth_headers);
+}
diff --git a/credential.h b/credential.h
index c78b72d110..b90f666e33 100644
--- a/credential.h
+++ b/credential.h
@@ -305,6 +305,14 @@ void credential_write(const struct credential *, FILE *,
 void credential_from_url(struct credential *, const char *url);
 int credential_from_url_gently(struct credential *, const char *url, int quiet);
 
+/*
+ * Update the URL-derived fields (protocol, host, path) of an existing
+ * credential to match a new URL. Unlike credential_from_url(), this function
+ * preserves state that was derived from a server's HTTP redirect response,
+ * such as the WWW-Authenticate headers.
+ */
+void credential_update_url(struct credential *c, const char *url);
+
 int credential_match(const struct credential *want,
 		     const struct credential *have, int match_password);
 
diff --git a/http.c b/http.c
index a0d399b274..e8abb9f95a 100644
--- a/http.c
+++ b/http.c
@@ -2427,7 +2427,14 @@ static int http_request_recoverable(const char *url,
 	if (options->effective_url && options->base_url) {
 		if (update_url_from_redirect(options->base_url,
 					     url, options->effective_url)) {
-			credential_from_url(&http_auth, options->base_url->buf);
+			/*
+			 * Use credential_update_url() rather than
+			 * credential_from_url() so that the WWW-Authenticate
+			 * challenge the server sent with the redirect target's
+			 * response is preserved and handed to the credential
+			 * helper.
+			 */
+			credential_update_url(&http_auth, options->base_url->buf);
 			url = options->effective_url->buf;
 		}
 	}
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 4149fc1078..0627ef1433 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -203,6 +203,7 @@ RewriteRule ^/dumb-redir/(.*)$ /dumb/$1 [R=301]
 RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301]
 RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]
 RewriteRule ^/smart-redir-auth/(.*)$ /auth/smart/$1 [R=301]
+RewriteRule ^/custom_auth_redir/(.*)$ /custom_auth/$1 [R=302]
 RewriteRule ^/smart-redir-limited/(.*)/info/refs$ /smart/$1/info/refs [R=301]
 RewriteRule ^/ftp-redir/(.*)$ ftp://localhost:1000/$1 [R=302]
 
diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
index a7d475dd68..349ae4ab39 100755
--- a/t/t5563-simple-http-auth.sh
+++ b/t/t5563-simple-http-auth.sh
@@ -557,6 +557,51 @@ test_expect_success 'access using bearer auth' '
 	EOF
 '
 
+test_expect_success 'bearer auth after redirect preserves wwwauth headers' '
+	test_when_finished "per_test_cleanup" &&
+
+	set_credential_reply get <<-EOF &&
+	capability[]=authtype
+	authtype=Bearer
+	credential=YS1naXQtdG9rZW4=
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+	id=1 creds=Bearer YS1naXQtdG9rZW4=
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
+	id=1 status=200
+	id=default response=WWW-Authenticate: FooBar param1="value1" param2="value2"
+	id=default response=WWW-Authenticate: Bearer authorize_uri="id.example.com" p=1 q=0
+	id=default response=WWW-Authenticate: Basic realm="example.com"
+	EOF
+
+	test_config_global credential.helper test-helper &&
+	test_config_global credential.useHttpPath true &&
+	git ls-remote "$HTTPD_URL/custom_auth_redir/repo.git" &&
+
+	expect_credential_query get <<-EOF &&
+	capability[]=authtype
+	capability[]=state
+	protocol=http
+	host=$HTTPD_DEST
+	path=custom_auth/repo.git
+	wwwauth[]=FooBar param1="value1" param2="value2"
+	wwwauth[]=Bearer authorize_uri="id.example.com" p=1 q=0
+	wwwauth[]=Basic realm="example.com"
+	EOF
+
+	expect_credential_query store <<-EOF
+	capability[]=authtype
+	authtype=Bearer
+	credential=YS1naXQtdG9rZW4=
+	protocol=http
+	host=$HTTPD_DEST
+	path=custom_auth/repo.git
+	EOF
+'
+
 test_expect_success 'access using bearer auth with invalid credentials' '
 	test_when_finished "per_test_cleanup" &&
 

---
base-commit: dea0ea3582e6980ddbc1173cc8e3e9f9db91cde0
change-id: 20260819-http-preserve-wwwauth-redirect-a3fe4dab6b35

Re: [PATCH v2] http: preserve wwwauth_headers across redirects

[Junio C Hamano]

For those of you who are watching from the sidelines, this v2 lacks
the threading history.

The v1 is at https://lore.kernel.org/git/20260602161150.1527493-1-aplattner@nvidia.com/

Thanks.

Aaron Plattner <aplattner@nvidia.com> writes:

> When cURL follows a redirect, it calls the CURLOPT_HEADERFUNCTION for
> each header received including ones from a redirect. http_request() sets
> fwrite_wwwauth() as the header function, which will record the wwwauth[]
> entries for the last step in the redirection chain.
>
> However, when http_request_recoverable() sees that cURL followed a
> redirect, it attempts to update the credentials for the request from the
> new URL using credential_from_url(). The first thing that does is call
> credential_clear(), which clears everything including wwwauth_headers.
>
> If the new URL should use a credential helper rather than credentials
> embedded in the URL, this loses the list of authentication methods that
> the server provided in the redirect.
>
> The WWW-Authenticate challenge is not derived from the URL; it is
> populated from the server's response, and after a redirect it describes
> how to authenticate to the redirect target and it needs to survive the
> URL update so that credential helpers can know which authentication
> methods are allowed.
>
> Add a new credential_update_url() that wraps credential_from_url() and
> preserves wwwauth_headers specifically. Use SWAP() to avoid having to
> copy the whole strbuf.
>
> Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
> ---
> I decided to come back to this after I noticed that at least one other
> person had run into the same bug:
>
> https://lore.kernel.org/all/CADoNwcscDrx+YcfbcW4YKONDZZQgnPiwEOxL4QYV_C7_=FOFcg@mail.gmail.com/
>
> Rather than reworking everything about how credentials are stored, I
> took your advice in [1] and just moved the code to preserve the wwwauth_headers
> into credential.c. That way any future credential fields that need to be
> preserved can be added there without having to hunt down other places
> like http.c that are reaching into it.
>
> [1] https://lore.kernel.org/all/xmqqpl28scll.fsf@gitster.g/
> ---
> Changes in v2:
> - Move strvec preservation into a helper function in credential.c
> - Use SWAP instead of strvec_pushv() to avoid having to copy the
>   contents of the strvec.
> - Link to v1: https://patch.msgid.link/20260602161150.1527493-1-aplattner@nvidia.com
> ---
>  credential.c                | 16 ++++++++++++++++
>  credential.h                |  8 ++++++++
>  http.c                      |  9 ++++++++-
>  t/lib-httpd/apache.conf     |  1 +
>  t/t5563-simple-http-auth.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 78 insertions(+), 1 deletion(-)
>
> diff --git a/credential.c b/credential.c
> index 2594c0c422..035399d7ee 100644
> --- a/credential.c
> +++ b/credential.c
> @@ -708,3 +708,19 @@ void credential_from_url(struct credential *c, const char *url)
>  	if (credential_from_url_gently(c, url, 0) < 0)
>  		die(_("credential url cannot be parsed: %s"), url);
>  }
> +
> +void credential_update_url(struct credential *c, const char *url)
> +{
> +	struct strvec wwwauth_headers = STRVEC_INIT;
> +
> +	/*
> +	 * credential_from_url() clears the whole credential. Preserve the
> +	 * WWW-Authenticate list, which is derived from the server's original
> +	 * response rather than from the URL and is required to authenticate to
> +	 * the new URL.
> +	 */
> +	SWAP(wwwauth_headers, c->wwwauth_headers);
> +	credential_from_url(c, url);
> +	SWAP(c->wwwauth_headers, wwwauth_headers);
> +	strvec_clear(&wwwauth_headers);
> +}
> diff --git a/credential.h b/credential.h
> index c78b72d110..b90f666e33 100644
> --- a/credential.h
> +++ b/credential.h
> @@ -305,6 +305,14 @@ void credential_write(const struct credential *, FILE *,
>  void credential_from_url(struct credential *, const char *url);
>  int credential_from_url_gently(struct credential *, const char *url, int quiet);
>  
> +/*
> + * Update the URL-derived fields (protocol, host, path) of an existing
> + * credential to match a new URL. Unlike credential_from_url(), this function
> + * preserves state that was derived from a server's HTTP redirect response,
> + * such as the WWW-Authenticate headers.
> + */
> +void credential_update_url(struct credential *c, const char *url);
> +
>  int credential_match(const struct credential *want,
>  		     const struct credential *have, int match_password);
>  
> diff --git a/http.c b/http.c
> index a0d399b274..e8abb9f95a 100644
> --- a/http.c
> +++ b/http.c
> @@ -2427,7 +2427,14 @@ static int http_request_recoverable(const char *url,
>  	if (options->effective_url && options->base_url) {
>  		if (update_url_from_redirect(options->base_url,
>  					     url, options->effective_url)) {
> -			credential_from_url(&http_auth, options->base_url->buf);
> +			/*
> +			 * Use credential_update_url() rather than
> +			 * credential_from_url() so that the WWW-Authenticate
> +			 * challenge the server sent with the redirect target's
> +			 * response is preserved and handed to the credential
> +			 * helper.
> +			 */
> +			credential_update_url(&http_auth, options->base_url->buf);
>  			url = options->effective_url->buf;
>  		}
>  	}
> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index 4149fc1078..0627ef1433 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -203,6 +203,7 @@ RewriteRule ^/dumb-redir/(.*)$ /dumb/$1 [R=301]
>  RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301]
>  RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]
>  RewriteRule ^/smart-redir-auth/(.*)$ /auth/smart/$1 [R=301]
> +RewriteRule ^/custom_auth_redir/(.*)$ /custom_auth/$1 [R=302]
>  RewriteRule ^/smart-redir-limited/(.*)/info/refs$ /smart/$1/info/refs [R=301]
>  RewriteRule ^/ftp-redir/(.*)$ ftp://localhost:1000/$1 [R=302]
>  
> diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
> index a7d475dd68..349ae4ab39 100755
> --- a/t/t5563-simple-http-auth.sh
> +++ b/t/t5563-simple-http-auth.sh
> @@ -557,6 +557,51 @@ test_expect_success 'access using bearer auth' '
>  	EOF
>  '
>  
> +test_expect_success 'bearer auth after redirect preserves wwwauth headers' '
> +	test_when_finished "per_test_cleanup" &&
> +
> +	set_credential_reply get <<-EOF &&
> +	capability[]=authtype
> +	authtype=Bearer
> +	credential=YS1naXQtdG9rZW4=
> +	EOF
> +
> +	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
> +	id=1 creds=Bearer YS1naXQtdG9rZW4=
> +	EOF
> +
> +	cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
> +	id=1 status=200
> +	id=default response=WWW-Authenticate: FooBar param1="value1" param2="value2"
> +	id=default response=WWW-Authenticate: Bearer authorize_uri="id.example.com" p=1 q=0
> +	id=default response=WWW-Authenticate: Basic realm="example.com"
> +	EOF
> +
> +	test_config_global credential.helper test-helper &&
> +	test_config_global credential.useHttpPath true &&
> +	git ls-remote "$HTTPD_URL/custom_auth_redir/repo.git" &&
> +
> +	expect_credential_query get <<-EOF &&
> +	capability[]=authtype
> +	capability[]=state
> +	protocol=http
> +	host=$HTTPD_DEST
> +	path=custom_auth/repo.git
> +	wwwauth[]=FooBar param1="value1" param2="value2"
> +	wwwauth[]=Bearer authorize_uri="id.example.com" p=1 q=0
> +	wwwauth[]=Basic realm="example.com"
> +	EOF
> +
> +	expect_credential_query store <<-EOF
> +	capability[]=authtype
> +	authtype=Bearer
> +	credential=YS1naXQtdG9rZW4=
> +	protocol=http
> +	host=$HTTPD_DEST
> +	path=custom_auth/repo.git
> +	EOF
> +'
> +
>  test_expect_success 'access using bearer auth with invalid credentials' '
>  	test_when_finished "per_test_cleanup" &&
>  
>
> ---
> base-commit: dea0ea3582e6980ddbc1173cc8e3e9f9db91cde0
> change-id: 20260819-http-preserve-wwwauth-redirect-a3fe4dab6b35