credential: new attribute password_expiry_utc

Some passwords have an expiry date known at generation. This may be
years away for a personal access token or hours for an OAuth access
token.

When multiple credential helpers are configured, `credential fill` tries
each helper in turn until it has a username and password, returning
early. If Git authentication succeeds, `credential approve`
stores the successful credential in all helpers. If authentication
fails, `credential reject` erases matching credentials in all helpers.
Helpers implement corresponding operations: get, store, erase.

The credential protocol has no expiry attribute, so helpers cannot
store expiry information. Even if a helper returned an improvised
expiry attribute, git credential discards unrecognised attributes
between operations and between helpers.

This is a particular issue when a storage helper and a
credential-generating helper are configured together:

	[credential]
		helper = storage  # eg. cache or osxkeychain
		helper = generate  # eg. oauth

`credential approve` stores the generated credential in both helpers
without expiry information. Later `credential fill` may return an
expired credential from storage. There is no workaround, no matter how
clever the second helper. The user sees authentication fail (a retry
will succeed).

Introduce a password expiry attribute. In `credential fill`, ignore
expired passwords and continue to query subsequent helpers.

In the example above, `credential fill` ignores the expired password
and a fresh credential is generated. If authentication succeeds,
`credential approve` replaces the expired password in storage.
If authentication fails, the expired credential is erased by
`credential reject`. It is unnecessary but harmless for storage
helpers to self prune expired credentials.

Add support for the new attribute to credential-cache.
Eventually, I hope to see support in other popular storage helpers.

Example usage in a credential-generating helper
https://github.com/hickford/git-credential-oauth/pull/16

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
Reviewed-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
M Hickford 2023-02-18 06:32:57 +00:00 committed by Junio C Hamano
parent 23c56f7bd5
commit d208bfdfef
6 changed files with 125 additions and 2 deletions

View file

@ -144,6 +144,12 @@ Git understands the following attributes:
The credential's password, if we are asking it to be stored.
`password_expiry_utc`::
Generated passwords such as an OAuth access token may have an expiry date.
When reading credentials from helpers, `git credential fill` ignores expired
passwords. Represented as Unix time UTC, seconds since 1970.
`url`::
When this special attribute is read by `git credential`, the

View file

@ -167,7 +167,7 @@ helper::
If there are multiple instances of the `credential.helper` configuration
variable, each helper will be tried in turn, and may provide a username,
password, or nothing. Once Git has acquired both a username and a
password, no more helpers will be tried.
non-expired password, no more helpers will be tried.
+
If `credential.helper` is configured to the empty string, this resets
the helper list to empty (so you may override a helper set by a

View file

@ -127,6 +127,9 @@ static void serve_one_client(FILE *in, FILE *out)
if (e) {
fprintf(out, "username=%s\n", e->item.username);
fprintf(out, "password=%s\n", e->item.password);
if (e->item.password_expiry_utc != TIME_MAX)
fprintf(out, "password_expiry_utc=%"PRItime"\n",
e->item.password_expiry_utc);
}
}
else if (!strcmp(action.buf, "exit")) {

View file

@ -7,6 +7,7 @@
#include "prompt.h"
#include "sigchain.h"
#include "urlmatch.h"
#include "git-compat-util.h"
void credential_init(struct credential *c)
{
@ -234,6 +235,11 @@ int credential_read(struct credential *c, FILE *fp)
} else if (!strcmp(key, "path")) {
free(c->path);
c->path = xstrdup(value);
} else if (!strcmp(key, "password_expiry_utc")) {
errno = 0;
c->password_expiry_utc = parse_timestamp(value, NULL, 10);
if (c->password_expiry_utc == 0 || errno == ERANGE)
c->password_expiry_utc = TIME_MAX;
} else if (!strcmp(key, "url")) {
credential_from_url(c, value);
} else if (!strcmp(key, "quit")) {
@ -269,6 +275,11 @@ 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);
if (c->password_expiry_utc != TIME_MAX) {
char *s = xstrfmt("%"PRItime, c->password_expiry_utc);
credential_write_item(fp, "password_expiry_utc", s, 0);
free(s);
}
}
static int run_credential_helper(struct credential *c,
@ -342,6 +353,12 @@ void credential_fill(struct credential *c)
for (i = 0; i < c->helpers.nr; i++) {
credential_do(c, c->helpers.items[i].string, "get");
if (c->password_expiry_utc < time(NULL)) {
/* Discard expired password */
FREE_AND_NULL(c->password);
/* Reset expiry to maintain consistency */
c->password_expiry_utc = TIME_MAX;
}
if (c->username && c->password)
return;
if (c->quit)
@ -360,7 +377,7 @@ void credential_approve(struct credential *c)
if (c->approved)
return;
if (!c->username || !c->password)
if (!c->username || !c->password || c->password_expiry_utc < time(NULL))
return;
credential_apply_config(c);
@ -381,6 +398,7 @@ void credential_reject(struct credential *c)
FREE_AND_NULL(c->username);
FREE_AND_NULL(c->password);
c->password_expiry_utc = TIME_MAX;
c->approved = 0;
}

View file

@ -126,10 +126,12 @@ struct credential {
char *protocol;
char *host;
char *path;
timestamp_t password_expiry_utc;
};
#define CREDENTIAL_INIT { \
.helpers = STRING_LIST_INIT_DUP, \
.password_expiry_utc = TIME_MAX, \
}
/* Initialize a credential structure, setting all fields to empty. */

View file

@ -35,6 +35,16 @@ test_expect_success 'setup helper scripts' '
test -z "$pass" || echo password=$pass
EOF
write_script git-credential-verbatim-with-expiry <<-\EOF &&
user=$1; shift
pass=$1; shift
pexpiry=$1; shift
. ./dump
test -z "$user" || echo username=$user
test -z "$pass" || echo password=$pass
test -z "$pexpiry" || echo password_expiry_utc=$pexpiry
EOF
PATH="$PWD:$PATH"
'
@ -109,6 +119,43 @@ test_expect_success 'credential_fill continues through partial response' '
EOF
'
test_expect_success 'credential_fill populates password_expiry_utc' '
check fill "verbatim-with-expiry one two 9999999999" <<-\EOF
protocol=http
host=example.com
--
protocol=http
host=example.com
username=one
password=two
password_expiry_utc=9999999999
--
verbatim-with-expiry: get
verbatim-with-expiry: protocol=http
verbatim-with-expiry: host=example.com
EOF
'
test_expect_success 'credential_fill ignores expired password' '
check fill "verbatim-with-expiry one two 5" "verbatim three four" <<-\EOF
protocol=http
host=example.com
--
protocol=http
host=example.com
username=three
password=four
--
verbatim-with-expiry: get
verbatim-with-expiry: protocol=http
verbatim-with-expiry: host=example.com
verbatim: get
verbatim: protocol=http
verbatim: host=example.com
verbatim: username=one
EOF
'
test_expect_success 'credential_fill passes along metadata' '
check fill "verbatim one two" <<-\EOF
protocol=ftp
@ -149,6 +196,24 @@ test_expect_success 'credential_approve calls all helpers' '
EOF
'
test_expect_success 'credential_approve stores password expiry' '
check approve useless <<-\EOF
protocol=http
host=example.com
username=foo
password=bar
password_expiry_utc=9999999999
--
--
useless: store
useless: protocol=http
useless: host=example.com
useless: username=foo
useless: password=bar
useless: password_expiry_utc=9999999999
EOF
'
test_expect_success 'do not bother storing password-less credential' '
check approve useless <<-\EOF
protocol=http
@ -159,6 +224,17 @@ test_expect_success 'do not bother storing password-less credential' '
EOF
'
test_expect_success 'credential_approve does not store expired password' '
check approve useless <<-\EOF
protocol=http
host=example.com
username=foo
password=bar
password_expiry_utc=5
--
--
EOF
'
test_expect_success 'credential_reject calls all helpers' '
check reject useless "verbatim one two" <<-\EOF
@ -181,6 +257,24 @@ test_expect_success 'credential_reject calls all helpers' '
EOF
'
test_expect_success 'credential_reject erases credential regardless of expiry' '
check reject useless <<-\EOF
protocol=http
host=example.com
username=foo
password=bar
password_expiry_utc=5
--
--
useless: erase
useless: protocol=http
useless: host=example.com
useless: username=foo
useless: password=bar
useless: password_expiry_utc=5
EOF
'
test_expect_success 'usernames can be preserved' '
check fill "verbatim \"\" three" <<-\EOF
protocol=http