Re: [PATCH v4 2/8] credential: add WWW-Authenticate header to cred requests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Matthew John Cheetham via GitGitGadget wrote:
> From: Matthew John Cheetham <mjcheetham@xxxxxxxxxxx>
> 
> 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.
> 
> [1] https://datatracker.ietf.org/doc/html/rfc2616#section-14.47

...

> +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.
> +

The commit message & documentation changes (here and the 'www-auth[]'
definition below) are concise, easy-to-understand explanations of what
you're doing here with the 'www-authenticate' header values.

>  
> @@ -160,6 +166,16 @@ 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.

nit: if you're trying to get a paragraph break between "...to credential
helpers." and "Each 'WWW-Authenticate' header value", you need to add an
explicit break:

-------- 8< --------

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index bf0de0e940..50759153ef 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -171,10 +171,11 @@ username in the example above) will be left unset.
 	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.
++
+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.
 
-------- >8 --------

You can test to see how the docs look by running 'make doc' from the
repository root and looking at the generated 'git-credential.html' (note
that, if you've installed Git dependencies with Homebrew, you might need to
specify 'XML_CATALOG_FILES=$(brew --prefix)/etc/xml/catalog' to get it to
work).

> +
>  Unrecognised attributes are silently discarded.
>  
>  GIT
> diff --git a/credential.c b/credential.c
> index 897b4679333..8a3ad6c0ae2 100644
> --- a/credential.c
> +++ b/credential.c
> @@ -263,6 +263,17 @@ 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)
> +{
> +	int i = 0;
> +	const char *full_key = xstrfmt("%s[]", key);
> +	for (; i < vec->nr; i++) {
> +		credential_write_item(fp, full_key, vec->v[i], 0);
> +	}
> +	free((void*)full_key);
> +}
> +
>  void credential_write(const struct credential *c, FILE *fp)
>  {
>  	credential_write_item(fp, "protocol", c->protocol, 1);
> @@ -270,6 +281,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);

This implementation looks good to me.

>  }
>  
>  static int run_credential_helper(struct credential *c,




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux