Skip to content

Commit bb88a92

Browse files
aaronp24gitster
authored andcommitted
http: preserve wwwauth_headers across redirects
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. For example, I have a server that supports HTTP but always redirects to HTTPS before handling requests. This redirect breaks OAuth authentication: $ git ls-remote http://server/git => Send header: GET /git/info/refs?service=git-upload-pack HTTP/1.1 <= Recv header: HTTP/1.1 302 Found <= Recv header: Location: https://server.nvidia.com/git/info/refs?service=git-upload-pack == Info: Issue another request to this URL: 'https://server.nvidia.com/git/info/refs?service=git-upload-pack' => Send header: GET /git/info/refs?service=git-upload-pack HTTP/1.1 <= Recv header: HTTP/1.1 401 Unauthorized <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>" trace: run_command: 'git credential-cache --timeout 7200 get' trace: start_command: /bin/sh -c 'git credential-cache --timeout 7200 get' 'git credential-cache --timeout 7200 get' trace: built-in: git credential-cache --timeout 7200 get trace: run_command: 'git credential-msal get' trace: start_command: /bin/sh -c 'git credential-msal get' 'git credential-msal get' trace: exec: git-credential-msal get trace: run_command: git-credential-msal get trace: start_command: /usr/bin/git-credential-msal get Username for 'https://server.nvidia.com': ^C When git invokes the credential helper, it doesn't include the wwwauth[] array, so git-credential-msal doesn't think that OAuth is supported [1]. Fix the problem by preserving the wwwauth_headers strvec across the call to credential_from_url(). [1] https://github.com/Binary-Eater/git-credential-msal/blob/trunk/src/git_credential_msal/main.py#L69 Signed-off-by: Aaron Plattner <aplattner@nvidia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 94f0577 commit bb88a92

3 files changed

Lines changed: 60 additions & 0 deletions

File tree

http.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2354,7 +2354,21 @@ static int http_request_recoverable(const char *url,
23542354
if (options->effective_url && options->base_url) {
23552355
if (update_url_from_redirect(options->base_url,
23562356
url, options->effective_url)) {
2357+
struct strvec wwwauth_headers = STRVEC_INIT;
2358+
2359+
/*
2360+
* Preserve wwwauth_headers across the call to
2361+
* credential_from_url(): if the effective URL doesn't
2362+
* specify its own credentials, a credential helper
2363+
* might need the wwwauth[] array from the server's
2364+
* redirect response in order to authenticate.
2365+
*/
2366+
strvec_pushv(&wwwauth_headers,
2367+
http_auth.wwwauth_headers.v);
23572368
credential_from_url(&http_auth, options->base_url->buf);
2369+
strvec_pushv(&http_auth.wwwauth_headers,
2370+
wwwauth_headers.v);
2371+
strvec_clear(&wwwauth_headers);
23582372
url = options->effective_url->buf;
23592373
}
23602374
}

t/lib-httpd/apache.conf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ RewriteRule ^/dumb-redir/(.*)$ /dumb/$1 [R=301]
202202
RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301]
203203
RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]
204204
RewriteRule ^/smart-redir-auth/(.*)$ /auth/smart/$1 [R=301]
205+
RewriteRule ^/custom_auth_redir/(.*)$ /custom_auth/$1 [R=302]
205206
RewriteRule ^/smart-redir-limited/(.*)/info/refs$ /smart/$1/info/refs [R=301]
206207
RewriteRule ^/ftp-redir/(.*)$ ftp://localhost:1000/$1 [R=302]
207208

t/t5563-simple-http-auth.sh

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,51 @@ test_expect_success 'access using bearer auth' '
557557
EOF
558558
'
559559

560+
test_expect_success 'bearer auth after redirect preserves wwwauth headers' '
561+
test_when_finished "per_test_cleanup" &&
562+
563+
set_credential_reply get <<-EOF &&
564+
capability[]=authtype
565+
authtype=Bearer
566+
credential=YS1naXQtdG9rZW4=
567+
EOF
568+
569+
cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
570+
id=1 creds=Bearer YS1naXQtdG9rZW4=
571+
EOF
572+
573+
cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
574+
id=1 status=200
575+
id=default response=WWW-Authenticate: FooBar param1="value1" param2="value2"
576+
id=default response=WWW-Authenticate: Bearer authorize_uri="id.example.com" p=1 q=0
577+
id=default response=WWW-Authenticate: Basic realm="example.com"
578+
EOF
579+
580+
test_config_global credential.helper test-helper &&
581+
test_config_global credential.useHttpPath true &&
582+
git ls-remote "$HTTPD_URL/custom_auth_redir/repo.git" &&
583+
584+
expect_credential_query get <<-EOF &&
585+
capability[]=authtype
586+
capability[]=state
587+
protocol=http
588+
host=$HTTPD_DEST
589+
path=custom_auth/repo.git
590+
wwwauth[]=FooBar param1="value1" param2="value2"
591+
wwwauth[]=Bearer authorize_uri="id.example.com" p=1 q=0
592+
wwwauth[]=Basic realm="example.com"
593+
EOF
594+
595+
expect_credential_query store <<-EOF
596+
capability[]=authtype
597+
authtype=Bearer
598+
credential=YS1naXQtdG9rZW4=
599+
protocol=http
600+
host=$HTTPD_DEST
601+
path=custom_auth/repo.git
602+
EOF
603+
'
604+
560605
test_expect_success 'access using bearer auth with invalid credentials' '
561606
test_when_finished "per_test_cleanup" &&
562607

0 commit comments

Comments
 (0)