From: M Hickford <mirth.hickford@xxxxxxxxx> 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. Currently the credential protocol has no expiry attribute. When multiple helpers are configured, `credential fill` tries each helper in turn until it has a username and password, returning early. When a storage helper and a credential-generating helper are configured together, the credential is necessarily stored without expiry, so `credential fill` may later return an expired credential from storage. ``` [credential] helper = storage # eg. cache or osxkeychain helper = generate # eg. oauth ``` An improvement is to introduce a password expiry attribute to the credential protocol. If the expiry date has passed, `credential fill` ignores the password attribute, so subsequent helpers can generate a fresh credential. This is backwards compatible -- no change in behaviour with helpers that discard the expiry attribute. Note that the expiry logic is entirely within the credential layer. Compatible helpers store and retrieve the new attribute like any other. This keeps the helper contract simple. This patch adds support for the new attribute to cache. Example usage in a credential-generating helper https://github.com/hickford/git-credential-oauth/pull/16 Future ideas: make it possible for a storage helper to provide OAuth refresh token to subsequent helpers. https://github.com/gitgitgadget/git/pull/1394 Signed-off-by: M Hickford <mirth.hickford@xxxxxxxxx> --- 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. Currently the credential protocol has no expiry attribute. When multiple helpers are configured, credential fill tries each helper in turn until it has a username and password, returning early. When a storage helper and a credential-generating helper are configured together, the credential is necessarily stored without expiry, so credential fill may later return an expired credential from storage. [credential] helper = storage # eg. cache or osxkeychain helper = generate # eg. oauth An improvement is to introduce a password expiry attribute to the credential protocol. If the password has expired, credential fill no longer returns early, so subsequent helpers can generate a fresh credential. This is backwards compatible -- no change in behaviour with helpers that discard the expiry attribute. Note that the expiry logic is entirely within the credential layer. Compatible helpers store and retrieve the new attribute like any other. This keeps the helper contract simple. This patch adds support for the new attribute to cache. Example usage in a credential-generating helper https://github.com/hickford/git-credential-oauth/pull/16 Future ideas: make it possible for a storage helper to provide OAuth refresh token to subsequent helpers. https://github.com/gitgitgadget/git/pull/1394 Questions for reviewers: * Does the behaviour implemented match the documentation? (I'm not famiiliar with C) * Any edge cases? * How to test in t0300-credentials.sh ? Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1443%2Fhickford%2Fpassword-expiry-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1443/hickford/password-expiry-v2 Pull-Request: https://github.com/git/git/pull/1443 Range-diff vs v1: 1: 184b0aa6514 ! 1: b9ee729ee4d credential: new attribute password_expiry_utc @@ Metadata ## Commit message ## credential: new attribute password_expiry_utc - If password has expired, credential fill no longer returns early, - so later helpers can generate a fresh credential. This is backwards - compatible -- no change in behaviour with helpers that discard the - expiry attribute. The expiry logic is entirely in the git credential - layer; compatible helpers simply store and return the expiry - attribute verbatim. + 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. - Store new attribute in cache. + Currently the credential protocol has no expiry attribute. When multiple + helpers are configured, `credential fill` tries each helper in turn + until it has a username and password, returning early. + + When a storage helper and a credential-generating helper are configured + together, the credential is necessarily stored without expiry, so + `credential fill` may later return an expired credential from storage. + + ``` + [credential] + helper = storage # eg. cache or osxkeychain + helper = generate # eg. oauth + ``` + + An improvement is to introduce a password expiry attribute to the + credential protocol. If the expiry date has passed, `credential fill` + ignores the password attribute, so subsequent helpers can generate a + fresh credential. This is backwards compatible -- no change in + behaviour with helpers that discard the expiry attribute. + + Note that the expiry logic is entirely within the credential layer. + Compatible helpers store and retrieve the new attribute like any other. + This keeps the helper contract simple. + + This patch adds support for the new attribute to cache. + + Example usage in a credential-generating helper + https://github.com/hickford/git-credential-oauth/pull/16 + + Future ideas: make it possible for a storage helper to provide OAuth + refresh token to subsequent helpers. + https://github.com/gitgitgadget/git/pull/1394 Signed-off-by: M Hickford <mirth.hickford@xxxxxxxxx> @@ Documentation/git-credential.txt: Git understands the following attributes: +`password_expiry_utc`:: + -+ If password is a personal access token or OAuth access token, it may have an expiry date. When getting credentials from a helper, `git credential fill` ignores the password attribute if the expiry date has passed. Storage helpers should store this attribute if possible. Helpers should not implement expiry logic themselves. Represented as Unix time UTC, seconds since 1970. ++ If password is a personal access token or OAuth access token, it may have an ++ expiry date. When getting credentials from a helper, `git credential fill` ++ ignores the password attribute if the expiry date has passed. Storage ++ helpers should store this attribute if possible. Helpers should not ++ implement expiry logic themselves. Represented as Unix time UTC, seconds ++ since 1970. + `url`:: @@ builtin/credential-cache--daemon.c: static void serve_one_client(FILE *in, FILE if (e) { fprintf(out, "username=%s\n", e->item.username); fprintf(out, "password=%s\n", e->item.password); -+ if (e->item.password_expiry_utc != 0) { -+ fprintf(out, "password_expiry_utc=%ld\n", e->item.password_expiry_utc); -+ } ++ 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")) { @@ credential.c #include "prompt.h" #include "sigchain.h" #include "urlmatch.h" -+#include <time.h> ++#include "git-compat-util.h" void credential_init(struct credential *c) { -@@ credential.c: void credential_clear(struct credential *c) - free(c->path); - free(c->username); - free(c->password); -+ c->password_expiry_utc = 0; - string_list_clear(&c->helpers, 0); +@@ credential.c: static void credential_getpass(struct credential *c) + if (!c->username) + c->username = credential_ask_one("Username", c, + PROMPT_ASKPASS|PROMPT_ECHO); +- if (!c->password) ++ if (!c->password || c->password_expiry_utc < time(NULL)) { ++ c->password_expiry_utc = TIME_MAX; + c->password = credential_ask_one("Password", c, + PROMPT_ASKPASS); ++ } + } + + int credential_read(struct credential *c, FILE *fp) + { + struct strbuf line = STRBUF_INIT; - credential_init(c); ++ int password_updated = 0; ++ timestamp_t this_password_expiry = TIME_MAX; ++ + while (strbuf_getline(&line, fp) != EOF) { + char *key = line.buf; + char *value = strchr(key, '='); +@@ credential.c: int credential_read(struct credential *c, FILE *fp) + } else if (!strcmp(key, "password")) { + free(c->password); + c->password = xstrdup(value); ++ password_updated = 1; + } else if (!strcmp(key, "protocol")) { + free(c->protocol); + c->protocol = xstrdup(value); @@ credential.c: 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")) { -+ // TODO: ignore if can't parse integer -+ c->password_expiry_utc = atoi(value); ++ this_password_expiry = parse_timestamp(value, NULL, 10); ++ if (this_password_expiry == 0 || errno) { ++ this_password_expiry = TIME_MAX; ++ } } else if (!strcmp(key, "url")) { credential_from_url(c, value); } else if (!strcmp(key, "quit")) { - c->quit = !!git_config_bool("quit", value); - } -+ -+ // if expiry date has passed, ignore password and expiry fields -+ if (c->password_expiry_utc != 0 && time(NULL) > c->password_expiry_utc) { -+ trace_printf(_("Password has expired.\n")); -+ FREE_AND_NULL(c->username); -+ FREE_AND_NULL(c->password); -+ c->password_expiry_utc = 0; -+ } +@@ credential.c: int credential_read(struct credential *c, FILE *fp) + */ + } + ++ if (password_updated) ++ c->password_expiry_utc = this_password_expiry; + - /* - * Ignore other lines; we don't know what they mean, but - * this future-proofs us when later versions of git do + strbuf_release(&line); + return 0; + } @@ credential.c: 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 != 0) { -+ int length = snprintf( NULL, 0, "%ld", c->password_expiry_utc); -+ char* str = malloc( length + 1 ); -+ snprintf( str, length + 1, "%ld", c->password_expiry_utc ); -+ credential_write_item(fp, "password_expiry_utc", str, 0); -+ free(str); ++ 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, +@@ credential.c: 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->username && c->password) ++ if (c->username && c->password && time(NULL) < c->password_expiry_utc) + return; + if (c->quit) + die("credential helper '%s' told us to quit", ## credential.h ## @@ credential.h: struct credential { char *protocol; char *host; char *path; -+ time_t password_expiry_utc; ++ 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. */ Documentation/git-credential.txt | 9 +++++++++ builtin/credential-cache--daemon.c | 3 +++ credential.c | 24 ++++++++++++++++++++++-- credential.h | 2 ++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt index ac2818b9f66..667c4d80e26 100644 --- a/Documentation/git-credential.txt +++ b/Documentation/git-credential.txt @@ -144,6 +144,15 @@ Git understands the following attributes: The credential's password, if we are asking it to be stored. +`password_expiry_utc`:: + + If password is a personal access token or OAuth access token, it may have an + expiry date. When getting credentials from a helper, `git credential fill` + ignores the password attribute if the expiry date has passed. Storage + helpers should store this attribute if possible. Helpers should not + implement expiry logic themselves. Represented as Unix time UTC, seconds + since 1970. + `url`:: When this special attribute is read by `git credential`, the diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c index f3c89831d4a..338058be7f9 100644 --- a/builtin/credential-cache--daemon.c +++ b/builtin/credential-cache--daemon.c @@ -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")) { diff --git a/credential.c b/credential.c index f6389a50684..354fa1652a9 100644 --- a/credential.c +++ b/credential.c @@ -7,6 +7,7 @@ #include "prompt.h" #include "sigchain.h" #include "urlmatch.h" +#include "git-compat-util.h" void credential_init(struct credential *c) { @@ -195,15 +196,20 @@ static void credential_getpass(struct credential *c) if (!c->username) c->username = credential_ask_one("Username", c, PROMPT_ASKPASS|PROMPT_ECHO); - if (!c->password) + if (!c->password || c->password_expiry_utc < time(NULL)) { + c->password_expiry_utc = TIME_MAX; c->password = credential_ask_one("Password", c, PROMPT_ASKPASS); + } } int credential_read(struct credential *c, FILE *fp) { struct strbuf line = STRBUF_INIT; + int password_updated = 0; + timestamp_t this_password_expiry = TIME_MAX; + while (strbuf_getline(&line, fp) != EOF) { char *key = line.buf; char *value = strchr(key, '='); @@ -225,6 +231,7 @@ int credential_read(struct credential *c, FILE *fp) } else if (!strcmp(key, "password")) { free(c->password); c->password = xstrdup(value); + password_updated = 1; } else if (!strcmp(key, "protocol")) { free(c->protocol); c->protocol = xstrdup(value); @@ -234,6 +241,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")) { + this_password_expiry = parse_timestamp(value, NULL, 10); + if (this_password_expiry == 0 || errno) { + this_password_expiry = TIME_MAX; + } } else if (!strcmp(key, "url")) { credential_from_url(c, value); } else if (!strcmp(key, "quit")) { @@ -246,6 +258,9 @@ int credential_read(struct credential *c, FILE *fp) */ } + if (password_updated) + c->password_expiry_utc = this_password_expiry; + strbuf_release(&line); return 0; } @@ -269,6 +284,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,7 +362,7 @@ 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->username && c->password) + if (c->username && c->password && time(NULL) < c->password_expiry_utc) return; if (c->quit) die("credential helper '%s' told us to quit", diff --git a/credential.h b/credential.h index f430e77fea4..935b28a70f1 100644 --- a/credential.h +++ b/credential.h @@ -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. */ base-commit: 5cc9858f1b470844dea5c5d3e936af183fdf2c68 -- gitgitgadget
