Re: [PATCH 06/14] introduce credentials API

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

 



Jeff King <peff@xxxxxxxx> writes:

> +`struct credential`::
> +
> +	This struct represents a single username/password combination.
> +	The `username` and `password` fields should be heap-allocated
> +	strings (or NULL if they are not yet known). The `unique` field,
> +	if non-NULL, should be a heap-allocated string indicating a
> +	unique context for this credential (e.g., a protocol and server
> +	name for a remote credential). The `description` field, if
> +	non-NULL, should point to a string containing a human-readable
> +	description of this credential.

Am I confused to say "unique" is for code/machine and "description" is for
human to identify what remote resource is being accessed using the
credential? Can two credentials have the same description but different
unique, and if so what happens? A plausible answer may be "the user cannot
tell what username/password pair to give, but it is expected that such a
nondescriptive context is only for certificate and it also is expected
that only one certificate is used at a time", perhaps?

I think "unique" vs "description" are secondary aspects of these things,
and the primary aspect that they share is that they are about "context",
as [11/14] describes.  Perhaps "context-id" vs "context-description" may
be better names to reduce confusion?  I dunno.

> +`credential_fill`::
> +
> +	Like `credential_fill_gently`, but `die()` if credentials cannot
> +	be gathered.
> +
> +`credential_reject`::
> +
> +	Inform the credential subsystem that the provided credentials
> +	have been rejected. This will clear the username and password
> +	fields in `struct credential`, as well as notify any helpers of
> +	the rejection (which may, for example, purge the invalid
> +	credentials from storage).

What hints do helpers get when this is called? Do they get username,
unique and description to allow them to selectively purge the bad ones
while keeping good ones, or the username is already cleared by the time
the helpers are notified and they have no clue?

> +`credential_getpass`::
> +
> +	Fetch credentials from the user either using an "askpass" helper
> +	(see the discussion of core.askpass and GIT_ASKPASS in
> +	linkgit:git-config[1] and linkgit:git[1], respectively) or by
> +	prompting the user via the terminal.

It sounds like that users of the API should call fill_credential() and let
the machinery fall back to this function as needed. Does this need to be
part of the public API functions?

> +static int read_credential_response(struct credential *c, FILE *fp)
> +{
> +	struct strbuf response = STRBUF_INIT;
> +
> +	while (strbuf_getline(&response, fp, '\n') != EOF) {
> +		char *key = response.buf;
> +		char *value = strchr(key, '=');
> +
> +		if (!value) {
> +			warning("bad output from credential helper: %s", key);
> +			strbuf_release(&response);
> +			return -1;
> +		}
> +		*value++ = '\0';
> +
> +		if (!strcmp(key, "username")) {
> +			free(c->username);
> +			c->username = xstrdup(value);

The document did not say anything about escaping/quoting of values, but it
may not be a bad idea to make it more explicit over there.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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]