diff mbox series

[v8,3/3] credential: add WWW-Authenticate header to cred requests

Message ID 149aedf55010718e22669575a148988eed0d8dcb.1675711789.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Enhance credential helper protocol to include auth headers | expand

Commit Message

Matthew John Cheetham Feb. 6, 2023, 7:29 p.m. UTC
From: Matthew John Cheetham <mjcheetham@outlook.com>

Add the value of the WWW-Authenticate response header to credential
requests. Credential helpers that understand and support HTTP
authentication and authorization can use this standard header (RFC 2616
Section 14.47 [1]) to generate valid credentials.

WWW-Authenticate headers can contain information pertaining to the
authority, authentication mechanism, or extra parameters/scopes that are
required.

The current I/O format for credential helpers only allows for unique
names for properties/attributes, so in order to transmit multiple header
values (with a specific order) we introduce a new convention whereby a
C-style array syntax is used in the property name to denote multiple
ordered values for the same property.

In this case we send multiple `wwwauth[]` properties where the order
that the repeated attributes appear in the conversation reflects the
order that the WWW-Authenticate headers appeared in the HTTP response.

Add a set of tests to exercise the HTTP authentication header parsing
and the interop with credential helpers. Credential helpers will receive
WWW-Authenticate information in credential requests.

[1] https://datatracker.ietf.org/doc/html/rfc2616#section-14.47

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
---
 Documentation/git-credential.txt |  19 ++-
 credential.c                     |  11 ++
 t/t5563-simple-http-auth.sh      | 236 +++++++++++++++++++++++++++++++
 3 files changed, 265 insertions(+), 1 deletion(-)

Comments

Ævar Arnfjörð Bjarmason Feb. 6, 2023, 8:45 p.m. UTC | #1
On Mon, Feb 06 2023, Matthew John Cheetham via GitGitGadget wrote:

> From: Matthew John Cheetham <mjcheetham@outlook.com>

> @@ -263,6 +263,16 @@ static void credential_write_item(FILE *fp, const char *key, const char *value,
>  	fprintf(fp, "%s=%s\n", key, value);
>  }
>  
> +static void credential_write_strvec(FILE *fp, const char *key,
> +				    const struct strvec *vec)
> +{
> +	char *full_key = xstrfmt("%s[]", key);

FWIW you could avoid this allocation if you just renamed the current
"credential_write_item()" to "credential_write_fmt()", and had it take a
format instead of its current hardcoded "%s=%s\n".

Then you could have two wrappers, credential_write_item() and
credential_write_items() (instead of "strvec"), the first passing
"%s=%s\n", the other "%s[]=%s\n".

Just a thought.

> +	for (size_t i = 0; i < vec->nr; i++) {
> +		credential_write_item(fp, full_key, vec->v[i], 0);

The {} here can be dropped in any case.
Matthew John Cheetham Feb. 15, 2023, 7:19 p.m. UTC | #2
On 2023-02-06 12:45, Ævar Arnfjörð Bjarmason wrote:

> 
> On Mon, Feb 06 2023, Matthew John Cheetham via GitGitGadget wrote:
> 
>> From: Matthew John Cheetham <mjcheetham@outlook.com>
> 
>> @@ -263,6 +263,16 @@ static void credential_write_item(FILE *fp, const char *key, const char *value,
>>  	fprintf(fp, "%s=%s\n", key, value);
>>  }
>>  
>> +static void credential_write_strvec(FILE *fp, const char *key,
>> +				    const struct strvec *vec)
>> +{
>> +	char *full_key = xstrfmt("%s[]", key);
> 
> FWIW you could avoid this allocation if you just renamed the current
> "credential_write_item()" to "credential_write_fmt()", and had it take a
> format instead of its current hardcoded "%s=%s\n".
> 
> Then you could have two wrappers, credential_write_item() and
> credential_write_items() (instead of "strvec"), the first passing
> "%s=%s\n", the other "%s[]=%s\n".
> 
> Just a thought.

Given this is the only `items/strvec` that I'm writing out, just inlining
the for-loop and calling `credential_write_item` directly with the key as
"wwwauth[]" would avoid the allocation without needing to make the write_item
function more complicated.

-static void credential_write_strvec(FILE *fp, const char *key,
-                                   const struct strvec *vec)
-{
-       char *full_key = xstrfmt("%s[]", key);
-       for (size_t i = 0; i < vec->nr; i++) {
-               credential_write_item(fp, full_key, vec->v[i], 0);
-       }
-       free(full_key);
-}
-
 void credential_write(const struct credential *c, FILE *fp)
 {
        credential_write_item(fp, "protocol", c->protocol, 1);
@@ -280,7 +270,10 @@ void credential_write(const struct credential *c, FILE *fp)
        credential_write_item(fp, "path", c->path, 0);
        credential_write_item(fp, "username", c->username, 0);
        credential_write_item(fp, "password", c->password, 0);
-       credential_write_strvec(fp, "wwwauth", &c->wwwauth_headers);
+       for (size_t i = 0; i < c->wwwauth_headers.nr; i++)
+               credential_write_item(fp, "wwwauth[]", c->wwwauth_headers.v[i],
+                                     0);
 }


>> +	for (size_t i = 0; i < vec->nr; i++) {
>> +		credential_write_item(fp, full_key, vec->v[i], 0);
> 
> The {} here can be dropped in any case.
>
diff mbox series

Patch

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index ac2818b9f66..50759153ef1 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -113,7 +113,13 @@  separated by an `=` (equals) sign, followed by a newline.
 The key may contain any bytes except `=`, newline, or NUL. The value may
 contain any bytes except newline or NUL.
 
-In both cases, all bytes are treated as-is (i.e., there is no quoting,
+Attributes with keys that end with C-style array brackets `[]` can have
+multiple values. Each instance of a multi-valued attribute forms an
+ordered list of values - the order of the repeated attributes defines
+the order of the values. An empty multi-valued attribute (`key[]=\n`)
+acts to clear any previous entries and reset the list.
+
+In all cases, all bytes are treated as-is (i.e., there is no quoting,
 and one cannot transmit a value with newline or NUL in it). The list of
 attributes is terminated by a blank line or end-of-file.
 
@@ -160,6 +166,17 @@  empty string.
 Components which are missing from the URL (e.g., there is no
 username in the example above) will be left unset.
 
+`wwwauth[]`::
+
+	When an HTTP response is received by Git that includes one or more
+	'WWW-Authenticate' authentication headers, these will be passed by Git
+	to credential helpers.
++
+Each 'WWW-Authenticate' header value is passed as a multi-valued
+attribute 'wwwauth[]', where the order of the attributes is the same as
+they appear in the HTTP response. This attribute is 'one-way' from Git
+to pass additional information to credential helpers.
+
 Unrecognised attributes are silently discarded.
 
 GIT
diff --git a/credential.c b/credential.c
index 897b4679333..9f39ebc3c7e 100644
--- a/credential.c
+++ b/credential.c
@@ -263,6 +263,16 @@  static void credential_write_item(FILE *fp, const char *key, const char *value,
 	fprintf(fp, "%s=%s\n", key, value);
 }
 
+static void credential_write_strvec(FILE *fp, const char *key,
+				    const struct strvec *vec)
+{
+	char *full_key = xstrfmt("%s[]", key);
+	for (size_t i = 0; i < vec->nr; i++) {
+		credential_write_item(fp, full_key, vec->v[i], 0);
+	}
+	free(full_key);
+}
+
 void credential_write(const struct credential *c, FILE *fp)
 {
 	credential_write_item(fp, "protocol", c->protocol, 1);
@@ -270,6 +280,7 @@  void credential_write(const struct credential *c, FILE *fp)
 	credential_write_item(fp, "path", c->path, 0);
 	credential_write_item(fp, "username", c->username, 0);
 	credential_write_item(fp, "password", c->password, 0);
+	credential_write_strvec(fp, "wwwauth", &c->wwwauth_headers);
 }
 
 static int run_credential_helper(struct credential *c,
diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
index 004eac5d1ed..a7b1e5bd1af 100755
--- a/t/t5563-simple-http-auth.sh
+++ b/t/t5563-simple-http-auth.sh
@@ -73,6 +73,242 @@  test_expect_success 'access using basic auth' '
 	expect_credential_query get <<-EOF &&
 	protocol=http
 	host=$HTTPD_DEST
+	wwwauth[]=Basic realm="example.com"
+	EOF
+
+	expect_credential_query store <<-EOF
+	protocol=http
+	host=$HTTPD_DEST
+	username=alice
+	password=secret-passwd
+	EOF
+'
+
+test_expect_success 'access using basic auth invalid credentials' '
+	test_when_finished "per_test_cleanup" &&
+
+	set_credential_reply get <<-EOF &&
+	username=baduser
+	password=wrong-passwd
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+	Basic YWxpY2U6c2VjcmV0LXBhc3N3ZA==
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
+	WWW-Authenticate: Basic realm="example.com"
+	EOF
+
+	test_config_global credential.helper test-helper &&
+	test_must_fail git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
+
+	expect_credential_query get <<-EOF &&
+	protocol=http
+	host=$HTTPD_DEST
+	wwwauth[]=Basic realm="example.com"
+	EOF
+
+	expect_credential_query erase <<-EOF
+	protocol=http
+	host=$HTTPD_DEST
+	username=baduser
+	password=wrong-passwd
+	wwwauth[]=Basic realm="example.com"
+	EOF
+'
+
+test_expect_success 'access using basic auth with extra challenges' '
+	test_when_finished "per_test_cleanup" &&
+
+	set_credential_reply get <<-EOF &&
+	username=alice
+	password=secret-passwd
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+	Basic YWxpY2U6c2VjcmV0LXBhc3N3ZA==
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
+	WWW-Authenticate: FooBar param1="value1" param2="value2"
+	WWW-Authenticate: Bearer authorize_uri="id.example.com" p=1 q=0
+	WWW-Authenticate: Basic realm="example.com"
+	EOF
+
+	test_config_global credential.helper test-helper &&
+	git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
+
+	expect_credential_query get <<-EOF &&
+	protocol=http
+	host=$HTTPD_DEST
+	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
+	protocol=http
+	host=$HTTPD_DEST
+	username=alice
+	password=secret-passwd
+	EOF
+'
+
+test_expect_success 'access using basic auth mixed-case wwwauth header name' '
+	test_when_finished "per_test_cleanup" &&
+
+	set_credential_reply get <<-EOF &&
+	username=alice
+	password=secret-passwd
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+	Basic YWxpY2U6c2VjcmV0LXBhc3N3ZA==
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
+	www-authenticate: foobar param1="value1" param2="value2"
+	WWW-AUTHENTICATE: BEARER authorize_uri="id.example.com" p=1 q=0
+	WwW-aUtHeNtIcAtE: baSiC realm="example.com"
+	EOF
+
+	test_config_global credential.helper test-helper &&
+	git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
+
+	expect_credential_query get <<-EOF &&
+	protocol=http
+	host=$HTTPD_DEST
+	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
+	protocol=http
+	host=$HTTPD_DEST
+	username=alice
+	password=secret-passwd
+	EOF
+'
+
+test_expect_success 'access using basic auth with wwwauth header continuations' '
+	test_when_finished "per_test_cleanup" &&
+
+	set_credential_reply get <<-EOF &&
+	username=alice
+	password=secret-passwd
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+	Basic YWxpY2U6c2VjcmV0LXBhc3N3ZA==
+	EOF
+
+	# Note that leading and trailing whitespace is important to correctly
+	# simulate a continuation/folded header.
+	cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
+	WWW-Authenticate: FooBar param1="value1"
+	 param2="value2"
+	WWW-Authenticate: Bearer authorize_uri="id.example.com"
+	 p=1
+	 q=0
+	WWW-Authenticate: Basic realm="example.com"
+	EOF
+
+	test_config_global credential.helper test-helper &&
+	git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
+
+	expect_credential_query get <<-EOF &&
+	protocol=http
+	host=$HTTPD_DEST
+	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
+	protocol=http
+	host=$HTTPD_DEST
+	username=alice
+	password=secret-passwd
+	EOF
+'
+
+test_expect_success 'access using basic auth with wwwauth header empty continuations' '
+	test_when_finished "per_test_cleanup" &&
+
+	set_credential_reply get <<-EOF &&
+	username=alice
+	password=secret-passwd
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+	Basic YWxpY2U6c2VjcmV0LXBhc3N3ZA==
+	EOF
+
+	CHALLENGE="$HTTPD_ROOT_PATH/custom-auth.challenge" &&
+
+	# Note that leading and trailing whitespace is important to correctly
+	# simulate a continuation/folded header.
+	printf "">$CHALLENGE &&
+	printf "WWW-Authenticate: FooBar param1=\"value1\"\r\n" >$CHALLENGE &&
+	printf " \r\n" >>$CHALLENGE &&
+	printf " param2=\"value2\"\r\n" >>$CHALLENGE &&
+	printf "WWW-Authenticate: Bearer authorize_uri=\"id.example.com\"\r\n" >>$CHALLENGE &&
+	printf " p=1\r\n" >>$CHALLENGE &&
+	printf " \r\n" >>$CHALLENGE &&
+	printf " q=0\r\n" >>$CHALLENGE &&
+	printf "WWW-Authenticate: Basic realm=\"example.com\"\r\n" >>$CHALLENGE &&
+
+	test_config_global credential.helper test-helper &&
+	git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
+
+	expect_credential_query get <<-EOF &&
+	protocol=http
+	host=$HTTPD_DEST
+	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
+	protocol=http
+	host=$HTTPD_DEST
+	username=alice
+	password=secret-passwd
+	EOF
+'
+
+test_expect_success 'access using basic auth with wwwauth header mixed line-endings' '
+	test_when_finished "per_test_cleanup" &&
+
+	set_credential_reply get <<-EOF &&
+	username=alice
+	password=secret-passwd
+	EOF
+
+	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+	Basic YWxpY2U6c2VjcmV0LXBhc3N3ZA==
+	EOF
+
+	CHALLENGE="$HTTPD_ROOT_PATH/custom-auth.challenge" &&
+
+	# Note that leading and trailing whitespace is important to correctly
+	# simulate a continuation/folded header.
+	printf "">$CHALLENGE &&
+	printf "WWW-Authenticate: FooBar param1=\"value1\"\r\n" >$CHALLENGE &&
+	printf " \r\n" >>$CHALLENGE &&
+	printf "\tparam2=\"value2\"\r\n" >>$CHALLENGE &&
+	printf "WWW-Authenticate: Basic realm=\"example.com\"" >>$CHALLENGE &&
+
+	test_config_global credential.helper test-helper &&
+	git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
+
+	expect_credential_query get <<-EOF &&
+	protocol=http
+	host=$HTTPD_DEST
+	wwwauth[]=FooBar param1="value1" param2="value2"
+	wwwauth[]=Basic realm="example.com"
 	EOF
 
 	expect_credential_query store <<-EOF