Skip to content

Commit

Permalink
credential: new attribute password_expiry_utc
Browse files Browse the repository at this point in the history
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
hickford/git-credential-oauth#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>
  • Loading branch information
hickford authored and gitster committed Feb 22, 2023
1 parent 23c56f7 commit d208bfd
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 2 deletions.
6 changes: 6 additions & 0 deletions Documentation/git-credential.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Documentation/gitcredentials.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions builtin/credential-cache--daemon.c
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Expand Down
20 changes: 19 additions & 1 deletion credential.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "prompt.h"
#include "sigchain.h"
#include "urlmatch.h"
#include "git-compat-util.h"

void credential_init(struct credential *c)
{
Expand Down Expand Up @@ -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")) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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;
}

Expand Down
2 changes: 2 additions & 0 deletions credential.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
94 changes: 94 additions & 0 deletions t/t0300-credentials.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
'

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit d208bfd

Please sign in to comment.