patchcredential: new attribute password_expiry_utc

From: M Hickford <mirth.hickford@gmail.com>

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.

Store new attribute in cache.

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
---
    credential: new attribute password_expiry_utc
    
    Some passwords, such as a personal access token or OAuth access token,
    may have an expiry date (as long as years for PATs or as short as hours
    for an OAuth access token). Add a new credential attribute
    password_expiry_utc.
    
    If password has expired, credential fill no longer returns early, so
    later helpers have opportunity to 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. Credential-generating helpers need only output the expiry
    attribute. Storage helpers should store the expiry if they can.
    
    Store expiry attribute in cache.
    
    This is particularly useful when a storage helper and a
    credential-generating helper are configured together, eg.
    
    [credential]
        helper = storage  # eg. cache or osxkeychain
        helper = generate  # eg. oauth
    
    
    Without this patch, credential fill may return an expired credential
    from storage, causing authentication to fail. With this patch: a fresh
    credential is generated if and only if the credential is expired.
    
    Example usage in a credential-generating helper
    https://github.com/hickford/git-credential-oauth/pull/16/files
    
    Signed-off-by: M Hickford mirth.hickford@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1443%2Fhickford%2Fpassword-expiry-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1443/hickford/password-expiry-v1
Pull-Request: https://github.com/git/git/pull/1443

 Documentation/git-credential.txt   |  4 ++++
 builtin/credential-cache--daemon.c |  3 +++
 credential.c                       | 21 +++++++++++++++++++++
 credential.h                       |  1 +
 4 files changed, 29 insertions(+)

"M Hickford via GitGitGadget" <gitgitgadget@gmail.com> writes:

It is unclear what you are describing in the above. The current behaviour without the patch? The behaviour of the code if this patch gets applied? Write it in such a way that it is clear why the patch is a good idea, not just "this would not hurt because it is backwards compatible".

The usual way to do so is to sell your change in this order:

 - Give background information to help readers understand what you
   are going to write in the following explanation.

 - Describe the current behaviour without any change to the code;

 - Present a situation where the current code results in an
   undesirable outcome. What exactly happens, what visible effect it
   has to the user, how the code could do better to help the user?

 - Propose an updated behaviour that would behave better in the
   above sample situation presented.

Curiously, what you wrote below the "---" line, that will not be part of the log message, looks to be organized better than the above. The first paragraph (except for the "Add ...") prepares the readers, It is still unclear if the second paragraph "when expired" describes what happens with the current code (i.e. highlighting why a change is needed) or what you want to happen with the patch, but the paragraph should first explain the problem in the current behaviour to motivate readers to learn why the updated code would lead to a better world. And follow that with the behaviour of the updated code and its effect (e.g. "without first trying a credential that is stale and see it fail before asking to reauthenticate, such a known-to-be-stale credential gets discarded automatically").

A overly long line. Please follow Documentation/CodingGuidelines and Documentation/SubmittingPatches

Style (multiple issues, check CodingGuidelines):

		if (e->item.password_expiry_utc)
			fprintf(out, "... overly long format template ...",
				e->item.password_expiry_utc);

 * Using integral value or pointer value as a truth value does not
   require an explicit comparison with 0;

 * A single-statement block does not need {} around it;

 * Overly long line should be folded, with properly indented.

Don't include system headers directly; often git-compat-util.h already has it, and if not, we need to find the right place to have it in git-compat-util.h file, as there are platforms that are finicky in inclusion order of the header files and definition of feature macros.

Not a huge deal, but if the rule is "an credential with expiry timestamp that is too old behaves as if it no longer exists or is valid", then a large integer, not zero, may serve as a better sentinel value for "this entry never expires". Instead of having to do

	if (expiry && expiry < time()) {
		... expired ...
	}

you can just do

	if (expiry < time()) {
		... expired ...
	}

and that would be simpler to understand for human readers, too.

Do not use // comment.  /* Our single-liner comment reads like this */

Don't use atoi(); make sure value is not followed by a non-number, e.g.

	const char *value = "43q";
	printf("%d<%s>\n", atoi(value), value);

would give you 43<43q>, but you want to reject and silently ignore such an expiry timestamp.

Ditto, but if you used a large value as sentinel for "never expires" and wrote it like this

		if (c->password_expiry_utc < time(NULL)) {

then it is clear enough that you do not even need such a comment. The expression itself makes it clear what is going on (i.e. the current time comes later than the expiry_utc value on the number line hence it appears on the right to it, clearly showing that it has passed the threshold).

On Sun, 29 Jan 2023 at 20:17, Junio C Hamano <gitster@pobox.com> wrote:

Thanks for the guidance. Writing a better commit message clarified my own thoughts.

M Hickford <mirth.hickford@gmail.com> writes:

It is great to hear that somebody agreeing with me on this point.

We try to write in a way that would make it clear to readers of "git log", but I often notice that trying to explain the changes to the code clearly does force me to realize better ways to write the changes.

On Sat, Jan 28, 2023 at 9:08 AM M Hickford via GitGitGadget <gitgitgadget@gmail.com> wrote:

Just a few comments in addition to those already provided by Junio...

Using `_(...)` marks a string for localization, but doing so is undesirable for debugging messages which are meant for the developer, not the end user (and it creates extra work for translators). No existing[1] trace_printf() calls in the codebase use `_(...)`.

[1]: Unfortunately, a couple examples exist in Documentation/MyFirstObjectWalk.txt using `_(...)` but they should be removed.

Style in this project is `char *str`, not `char* str`. Also, drop spaces around function arguments:

    char *str = malloc(length + 1);

Same.

xstrfmt() from strbuf.h can help simplify this entire block:

    char *s = xstrfmt("%ld", c->password_expiry_utc);
    credential_write_item(fp, "password_expiry_utc", str, 0);
    free(s);

Thanks Eric for the review

On Mon, 30 Jan 2023 at 00:59, Eric Sunshine <sunshine@sunshineco.com> wrote:

Done in patch v3.

Neat. Done in patch v3.

From: M Hickford <mirth.hickford@gmail.com>

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@gmail.com>
---
    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@gmail.com>
      
     @@ 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(-)

On Wed, Feb 01, 2023 at 09:39:51AM +0000, M Hickford via GitGitGadget wrote:

This "should not" seems weird to me. The logic you have here throws out entries that have expired when they pass through Git. But wouldn't helpers which store things want to know about and act on the expiration, too?

For example, if Git learns about a credential that expires in 60 seconds and passes it to credential-cache which is configured --timeout=300, wouldn't it want to set its internal ttl on the credential to 60, rather than 300?

I think your plan here is that Git would then reject the credential if a request is made at time now+65. But the cache is holding onto it much longer than necessary.

Likewise, wouldn't anything that stores credentials at least want to be able to store and regurgitate the expiration? For instance, even credential-store would want to do this. I'm OK if it doesn't, and we can consider it a quality-of-implementation issue and see if anybody cares enough to implement it. But I'd think most "real" helpers would want to do so.

So it seems like helpers really do need to support this "expiration" notion. And it's actually Git itself which doesn't need to care about it, assuming the helpers are doing something sensible (though it is OK if Git _also_ throws away expired credentials to support helpers which don't).

Is there a particular reason to use TIME_MAX as the sentinel value here, and not just "0"? It's not that big a deal either way, but it's more usual in our code base to use "0" if there's no reason not to (and it seems like nothing should be expiring in 1970 these days).

This is comparing a timestamp_t to a time_t, which may mix signed/unsigned. I can't offhand think of anything that would go too wrong there before 2038, so it's probably OK, but I wanted to call it out.

Do we need this logic? It seems weird that a helper would output an expiration but not a password in the first place. I guess ignoring the expiration is probably a reasonable outcome, but I wonder if a helper would ever want to just add an expiration to the data coming from another helper.

I.e., could we just read the value directly into c->password_expiry_utc as we do with other fields?

-Peff

Jeff King <peff@peff.net> writes:

This is my fault ;-). Here, there is no difference between 0 and TIME_MAX, but elsewhere the code needed

	if (expiry != 0 && expiry < time(NULL))

to see if the entry has expired. If the sentinel for an entry that will never expire were TIME_MAX, you do not need the first half of the expression.

I am OK either way.

On Wed, Feb 01, 2023 at 09:12:01AM -0800, Junio C Hamano wrote:

Ah. That at least is a compelling reason to use TIME_MAX. I'm OK with it.

-Peff

On 2023-02-01 04:10, Jeff King wrote:

I have often wondered about how, and if, Git should handle expiring credentials where the expiration is known. In my opinion I think Git should be doing *less* decision making with credentials and authentication in general, and leave that up to credential helpers.

The original design of credential helpers from what I can see (and Peff can correct me here of course!) is that they were really only thought about as storage-style helpers. Helpers are consulted for a known credential, and told about bad (erase) or good (store) credentials, all without any context about the request or remote responses.

If no credential helper can respond then Git itself prompts for a user/pass; so Git, or rather the user, is the 'generator'.

Of course that's not to say that credential generating helpers don't exist or are wrong - Git Credential Manager being of course one example rather close to home for me! However the current model, even with generating helpers, is still that Git will try and make the request given the details included in the helper response.

It doesn't make sense that a generating helper that knows about expiration would instead choose to respond with an expired credential rather than just try and generate a new credential.

Now in the case of a simple storage helper without such logic, after returning an expired credential should Git not be calling 'erase' back to the same helper to inform it that it has a stale credential and should be deleted? This would also require some affinity between calls to get/erase/store.

On Wed, Feb 01, 2023 at 12:02:16PM -0800, Matthew John Cheetham wrote:

FWIW, that is my general philosophy, too.

They were always intended to be generators, too. In the early days we discussed having more fancy graphical prompts via helpers, though most people simply use the askpass interface for this.

My personal config for the last, say, 12 years, has been to pull the password out of a read-only store, and ignore "store" and "erase" requests entirely. Which I think counts as a generator. :)

But yeah, if there is more context that we can be giving the helpers to let them make a better decision, I'm all for it. I think your www-authenticate patches are a good step in that direction. I'm not sure what other context would be useful.

Yeah, agreed.

That's a good point. I think in practice it would mostly happen that Git would throw away the expired credential, generate a new one (either from another helper or via prompting), and then if that works, overwrite the old one with a 'store' request.

If the new credential doesn't work (or the user aborts), the expired credential is left in the helper. But in theory that doesn't matter. It will still be expired when it's served up again later. And it's not a security problem to hold onto it longer, since it's no longer valid. Still, it feels a bit clunky compared to having the helper realize it's holding garbage.

-Peff

On Wed, 1 Feb 2023 at 20:02, Matthew John Cheetham <mjcheetham@outlook.com> wrote:

GCM would benefit from being able to store expiry too. Whenever GCM retrieves an OAuth credential from storage, it queries the server to check whether the access token has expired [1]. This would become unnecessary. I've added more about this in patch v3 commit message.

Further, it solves a problem if GCM is configured after another storage helper:

```
[credential]
    helper = storage  # eg. osx-keychain or exotic
    helper = manager
```

Currently this may return an expired credential from storage.

Background for others: GCM is typically configured as the *only* helper, with its own internal storage configuration [2]. These reimplement or wrap popular Git storage helpers [3][4][5].

```
[credential]
    helper = manager
    credentialStore = keychain
```

[1] https://github.com/GitCredentialManager/git-credential-manager/blob/main/src/shared/GitLab/GitLabHostProvider.cs [2] https://github.com/GitCredentialManager/git-credential-manager/blob/main/docs/credstores.md [3] https://github.com/GitCredentialManager/git-credential-manager/blob/main/src/shared/Core/Interop/MacOS/MacOSKeychain.cs [4] https://github.com/GitCredentialManager/git-credential-manager/blob/main/src/shared/Core/Interop/Linux/SecretServiceCollection.cs [5] https://github.com/GitCredentialManager/git-credential-manager/blob/main/src/shared/Core/CredentialCacheStore.cs

On 2023-02-04 22:45, M Hickford wrote:

One of the things that concerns me about the credential helper system today is the lack of 'affinity' across calls, and I think that we may disagree on this here.

A desire I have for the future of Git auth is that helpers can negotiate to start a more long-lived or complicated converstation, with retry and detailed feedback on the auth responses. I'm increasingly feeling that the get/store/erase model is not sufficient for this.

Imagine the scenario where the auth mechanism has a nonce that is updated on each successful (or failed) response. Here we'd want the helper that offered credentials to be told about the successful response for book-keeping, and not have that message 'stolen' by a simpler storage helper.

Another scenario would be multiple user accounts; a helper has credentials that would potentially be valid for the request (same host/forge or IdP for example), and we're wanting to avoid unnecessary user prompts as the user has not specified explicitly the account to 'bind' to that clone or specific remote. Why could we not return credentials to Git, also indicating that we have an ability to retry on a 401/403?

Increasingly, modern auth schemes have credentials that are so short lived (maybe bound to the exact specific request.. a one time use) that it really doesn't make sense to store any credentials at all. With such one-time-use credentials, and a simple storage helper ahead of a generating helper every other request would fail and the user would need to retry the command. Imagine the case that the OS/hardware security device holds the 'real' key or credential that is used to derive a one-time-use credential.

Strong auth mechamisms often tie credential generation strongly to storage.

Thanks Jeff for the review

On Wed, 1 Feb 2023 at 12:10, Jeff King <peff@peff.net> wrote:

I wanted to keep the helper contract as simple as possible "here is an attribute to store and retrieve like any other". It doesn't matter if a helper stores a password beyond expiry, because Git will erase or replace it when it fails authentication or another succeeds. Relax the language in the patch v3 commit message to describe "self-pruning of expired passwords is unnecessary but harmless".

Even after the password expires, there's value to storing other attributes such as username and (perhaps in future) oauth-refresh-token. Hence I named the attribute 'password_expiry_utc' explicitly rather than 'credential_expiry_utc'.

Absolutely. Eventually I'd like to see support in git-credential-osxkeychain, git-credential-wincred, git-credential-libsecret etc. Mentioned this in patch v3 description.

Junio made a persuasive argument for readability https://lore.kernel.org/git/CAPig+cQPLMrUKp0aqLCknSYCs5TAso-VSBYsQbGZ8g8wgY2Liw@mail.gmail.com/T/#mf66955cf3f53c073c68ad5ade7213617907bab63

Done in patch v3. The logic that remains (moved to credential_fill) is a simple sanity check.

From: M Hickford <mirth.hickford@gmail.com>

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

As a workaround, whenever monolithic helper Git Credential Manager (GCM) retrieves an OAuth credential from its storage, it makes a HTTP request to check whether the OAuth token has expired [1]. This complicates and slows the authentication happy path.

Worse is the case that a storage helper and a credential-generating helper are configured together:

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

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

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 credential and a fresh credential is generated. If authentication succeeds, `credential approve` replaces the expired credential 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 storage helpers.

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

[1] https://github.com/GitCredentialManager/git-credential-manager/blob/66b94e489ad8cc1982836355493e369770b30211/src/shared/GitLab/GitLabHostProvider.cs#L217

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
---
    credential: new attribute password_expiry_utc
    
    details in commit message
    
    Changes in patch v3:
    
     * Tests
     * Simplified credential_read, moved expiry logic to credential_fill

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1443%2Fhickford%2Fpassword-expiry-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1443/hickford/password-expiry-v3
Pull-Request: https://github.com/git/git/pull/1443

Range-diff vs v2:

 1:  b9ee729ee4d ! 1:  1846815a5c1 credential: new attribute password_expiry_utc
     @@ Commit message
          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 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.
      
     -    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.
     +    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.)
      
     -    ```
     -    [credential]
     -            helper = storage  # eg. cache or osxkeychain
     -            helper = generate  # eg. oauth
     -    ```
     +    As a workaround, whenever monolithic helper Git Credential Manager (GCM)
     +    retrieves an OAuth credential from its storage, it makes a HTTP request
     +    to check whether the OAuth token has expired [1]. This complicates and
     +    slows the authentication happy path.
      
     -    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.
     +    Worse is the case that a storage helper and a credential-generating
     +    helper are configured together:
      
     -    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.
     +            [credential]
     +                    helper = storage  # eg. cache or osxkeychain
     +                    helper = generate  # eg. oauth or manager
      
     -    This patch adds support for the new attribute to cache.
     +    `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.
     +
     +    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 credential
     +    and a fresh credential is generated. If authentication succeeds,
     +    `credential approve` replaces the expired credential 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 storage helpers.
      
          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
     +    [1] https://github.com/GitCredentialManager/git-credential-manager/blob/66b94e489ad8cc1982836355493e369770b30211/src/shared/GitLab/GitLabHostProvider.cs#L217
      
          Signed-off-by: M Hickford <mirth.hickford@gmail.com>
      
     @@ 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.
     ++	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
      
     + ## Documentation/gitcredentials.txt ##
     +@@ Documentation/gitcredentials.txt: 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.
     ++unexpired 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
     +
       ## builtin/credential-cache--daemon.c ##
      @@ builtin/credential-cache--daemon.c: static void serve_one_client(FILE *in, FILE *out)
       		if (e) {
     @@ credential.c
       
       void credential_init(struct credential *c)
       {
     -@@ 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;
     - 
     -+	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")) {
     -+			this_password_expiry = parse_timestamp(value, NULL, 10);
     -+			if (this_password_expiry == 0 || errno) {
     -+				this_password_expiry = TIME_MAX;
     -+			}
     ++			c->password_expiry_utc = parse_timestamp(value, NULL, 10);
     ++			if (c->password_expiry_utc == 0 || errno)
     ++				c->password_expiry_utc = TIME_MAX;
       		} else if (!strcmp(key, "url")) {
       			credential_from_url(c, value);
       		} else if (!strcmp(key, "quit")) {
     -@@ credential.c: int credential_read(struct credential *c, FILE *fp)
     - 		 */
     - 	}
     - 
     -+	if (password_updated)
     -+		c->password_expiry_utc = this_password_expiry;
     -+
     - 	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.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)
     ++		if (c->password_expiry_utc < time(NULL)) {
     ++			FREE_AND_NULL(c->password);
     ++			c->password_expiry_utc = TIME_MAX;
     ++		}
     + 		if (c->username && c->password)
       			return;
       		if (c->quit)
     - 			die("credential helper '%s' told us to quit",
     +@@ credential.c: 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);
     +@@ credential.c: 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;
     + }
     + 
      
       ## credential.h ##
      @@ credential.h: struct credential {
     @@ credential.h: struct credential {
       }
       
       /* Initialize a credential structure, setting all fields to empty. */
     +
     + ## t/t0300-credentials.sh ##
     +@@ t/t0300-credentials.sh: 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"
     + '
     + 
     +@@ t/t0300-credentials.sh: 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 continues through 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
     +@@ t/t0300-credentials.sh: 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
     +@@ t/t0300-credentials.sh: test_expect_success 'do not bother storing password-less credential' '
     + 	EOF
     + '
     + 
     ++test_expect_success 'credential_approve does not store expired credential' '
     ++	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
     +@@ t/t0300-credentials.sh: test_expect_success 'credential_reject calls all helpers' '
     + 	EOF
     + '
     + 
     ++test_expect_success 'credential_reject erases expired credential' '
     ++	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

 Documentation/git-credential.txt   |  6 ++
 Documentation/gitcredentials.txt   |  2 +-
 builtin/credential-cache--daemon.c |  3 +
 credential.c                       | 17 +++++-
 credential.h                       |  2 +
 t/t0300-credentials.sh             | 94 ++++++++++++++++++++++++++++++
 6 files changed, 122 insertions(+), 2 deletions(-)

"M Hickford via GitGitGadget" <gitgitgadget@gmail.com> writes:

https://github.com/git/git/actions/runs/4169057114/jobs/7217377625

Other platforms seem to be OK, but Windows test seems to be unhappy with it when this topic gets merged to 'seen'.

On Tue, 14 Feb 2023 at 01:59, Junio C Hamano <gitster@pobox.com> wrote:

Curious, let me take a look. I see that the tests failed on freebsd too.

I don't have a Windows machine to debug. If anyone reading has a hypothesis, please share.

I shall try changing the default value for a password without expiry from TIME_MAX to 0, see if that works any better [2].

[1] https://github.com/git/git/pull/1443/checks?check_run_id=11112315291 [2] https://github.com/git/git/pull/1443/checks?check_run_id=11343677685

On 2/14/23 3:36 PM, M Hickford wrote:

I was curious about this as well and took a look on my Windows machine. It appears that errno will be set to 'No such file or directory' unless you explicitly set it before checking it. This is odd, given that the helper script I wrote worked just fine without this requirement. However, I took a look through the rest of the codebase and noticed that `errno = 0` seems to always be declared before this type of conditional check. That did indeed fix the issue - tests all pass on Windows with this patch. I have not confirmed on freebsd, though, as a heads up.

Thanks, Lessley

---

Lessley Dennington <lessleydennington@gmail.com> writes:

Ah, that is quite understandable. Successful library function calls would not _clera_ errno, so if there were a failure before the control reaches this codepath, errno may have been set, and then parse_timestamp() call, which would be a strto$some_integral_type() call, may succeed and will leave errno as-is. Your fix is absolutely correct as long as we want to use "errno" after the call returns.

When strtoumax() etc. wants to report overflow or underflow, the returned value must be UINTMAX_MAX/UINTMAX_MIN and errno would be ERANGE, so it would probably want to check that errno is that value, and/or what c->password_expiry_utc has these overflow/underflow values.

On Fri, 17 Feb 2023 at 21:59, Junio C Hamano <gitster@pobox.com> wrote:

That's a subtle one! Thanks Lessley very much for your help debugging. I shall send a patch v4 with this and some minor changes discussed in review club.

On Sat, 4 Feb 2023 at 23:03, M Hickford via GitGitGadget <gitgitgadget@gmail.com> wrote:

s/a unexpired/an unexpired/ (or "a non-expired", perhaps)

Martin

I see you null out c->password in the expiry if block so that the following c->password check in the following if statement fails. While I think it's neat little trick, I wonder if others on list think it's better to be more explicit with how the logic should work (eg. adding the c->passowrd_expiry_utc check as an inner block inside of the c->username && c->password block).

On Thu, 16 Feb 2023 at 19:16, Calvin Wan <calvinwan@google.com> wrote:

It's important to reset the expiry date as well as discard the expired password so that fill accepts a later password without expiry (see test cases). I'll add a comment in patch v4.

From: M Hickford <mirth.hickford@gmail.com>

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>
---
    credential: new attribute password_expiry_utc
    
    details in commit message
    
    Changes in patch v4:
    
     * Set errno = 0 to fix tests on Windows and FreeBSD (thanks Lessley
       Dennington)
     * Clarify rationale as discussed at review club

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1443%2Fhickford%2Fpassword-expiry-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1443/hickford/password-expiry-v4
Pull-Request: https://github.com/git/git/pull/1443

Range-diff vs v3:

 1:  1846815a5c1 ! 1:  ae8bddbb30a credential: new attribute password_expiry_utc
     @@ Commit message
          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
     +    store expiry information. Even if a helper returned an improvised
          expiry attribute, git credential discards unrecognised attributes
     -    between operations and between helpers.)
     +    between operations and between helpers.
      
     -    As a workaround, whenever monolithic helper Git Credential Manager (GCM)
     -    retrieves an OAuth credential from its storage, it makes a HTTP request
     -    to check whether the OAuth token has expired [1]. This complicates and
     -    slows the authentication happy path.
     -
     -    Worse is the case that a storage helper and a credential-generating
     -    helper are configured together:
     +    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 or manager
     +                    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.
     +    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 credential
     +    In the example above, `credential fill` ignores the expired password
          and a fresh credential is generated. If authentication succeeds,
     -    `credential approve` replaces the expired credential in storage.
     +    `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 storage helpers.
     +    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
      
     -    [1] https://github.com/GitCredentialManager/git-credential-manager/blob/66b94e489ad8cc1982836355493e369770b30211/src/shared/GitLab/GitLabHostProvider.cs#L217
     -
          Signed-off-by: M Hickford <mirth.hickford@gmail.com>
      
       ## Documentation/git-credential.txt ##
     @@ Documentation/gitcredentials.txt: helper::
       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.
     -+unexpired 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
     @@ credential.c: int credential_read(struct credential *c, FILE *fp)
       			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)
     ++			if (c->password_expiry_utc == 0 || errno == ERANGE)
      +				c->password_expiry_utc = TIME_MAX;
       		} else if (!strcmp(key, "url")) {
       			credential_from_url(c, value);
     @@ 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->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)
     @@ t/t0300-credentials.sh: test_expect_success 'credential_fill continues through p
      +	EOF
      +'
      +
     -+test_expect_success 'credential_fill continues through expired password' '
     ++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
     @@ t/t0300-credentials.sh: test_expect_success 'do not bother storing password-less
       	EOF
       '
       
     -+test_expect_success 'credential_approve does not store expired credential' '
     ++test_expect_success 'credential_approve does not store expired password' '
      +	check approve useless <<-\EOF
      +	protocol=http
      +	host=example.com
     @@ t/t0300-credentials.sh: test_expect_success 'credential_reject calls all helpers
       	EOF
       '
       
     -+test_expect_success 'credential_reject erases expired credential' '
     ++test_expect_success 'credential_reject erases credential regardless of expiry' '
      +	check reject useless <<-\EOF
      +	protocol=http
      +	host=example.com

 Documentation/git-credential.txt   |  6 ++
 Documentation/gitcredentials.txt   |  2 +-
 builtin/credential-cache--daemon.c |  3 +
 credential.c                       | 20 ++++++-
 credential.h                       |  2 +
 t/t0300-credentials.sh             | 94 ++++++++++++++++++++++++++++++
 6 files changed, 125 insertions(+), 2 deletions(-)

Thanks for clarifying this block!

Overall, this patch is additive and shouldn't cause any regressions for current users of credential/credential-helper so I'm all for adding an expiry attribute to alleviate the use case pains you described above.

Reviewed-by: Calvin Wan <calvinwan@google.com>