@matheusbsbs / matheus-bagliano / commits / d208bfdfef

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>

M Hickford committed Feb 18, 2023 at 06:32 UTC d208bfdfef97a1e8fb746763b5057e0ad91e283b
6 files changed +125 -2
Documentation/git-credential.txt
+6
@@ -144,6 +144,12 @@ Git understands the following attributes:
144
145 The credential's password, if we are asking it to be stored.
146
147 +`password_expiry_utc`::
148 +
149 + Generated passwords such as an OAuth access token may have an expiry date.
150 + When reading credentials from helpers, `git credential fill` ignores expired
151 + passwords. Represented as Unix time UTC, seconds since 1970.
152 +
153 `url`::
154
155 When this special attribute is read by `git credential`, the
Documentation/gitcredentials.txt
+1 -1
@@ -167,7 +167,7 @@ helper::
167 If there are multiple instances of the `credential.helper` configuration
168 variable, each helper will be tried in turn, and may provide a username,
169 password, or nothing. Once Git has acquired both a username and a
170 -password, no more helpers will be tried.
170 +non-expired password, no more helpers will be tried.
171 +
172 If `credential.helper` is configured to the empty string, this resets
173 the helper list to empty (so you may override a helper set by a
builtin/credential-cache--daemon.c
+3
@@ -127,6 +127,9 @@ static void serve_one_client(FILE *in, FILE *out)
127 if (e) {
128 fprintf(out, "username=%s\n", e->item.username);
129 fprintf(out, "password=%s\n", e->item.password);
130 + if (e->item.password_expiry_utc != TIME_MAX)
131 + fprintf(out, "password_expiry_utc=%"PRItime"\n",
132 + e->item.password_expiry_utc);
133 }
134 }
135 else if (!strcmp(action.buf, "exit")) {
credential.c
+19 -1
@@ -7,6 +7,7 @@
7 #include "prompt.h"
8 #include "sigchain.h"
9 #include "urlmatch.h"
10 +#include "git-compat-util.h"
11
12 void credential_init(struct credential *c)
13 {
@@ -234,6 +235,11 @@ int credential_read(struct credential *c, FILE *fp)
235 } else if (!strcmp(key, "path")) {
236 free(c->path);
237 c->path = xstrdup(value);
238 + } else if (!strcmp(key, "password_expiry_utc")) {
239 + errno = 0;
240 + c->password_expiry_utc = parse_timestamp(value, NULL, 10);
241 + if (c->password_expiry_utc == 0 || errno == ERANGE)
242 + c->password_expiry_utc = TIME_MAX;
243 } else if (!strcmp(key, "url")) {
244 credential_from_url(c, value);
245 } else if (!strcmp(key, "quit")) {
@@ -269,6 +275,11 @@ void credential_write(const struct credential *c, FILE *fp)
275 credential_write_item(fp, "path", c->path, 0);
276 credential_write_item(fp, "username", c->username, 0);
277 credential_write_item(fp, "password", c->password, 0);
278 + if (c->password_expiry_utc != TIME_MAX) {
279 + char *s = xstrfmt("%"PRItime, c->password_expiry_utc);
280 + credential_write_item(fp, "password_expiry_utc", s, 0);
281 + free(s);
282 + }
283 }
284
285 static int run_credential_helper(struct credential *c,
@@ -342,6 +353,12 @@ void credential_fill(struct credential *c)
353
354 for (i = 0; i < c->helpers.nr; i++) {
355 credential_do(c, c->helpers.items[i].string, "get");
356 + if (c->password_expiry_utc < time(NULL)) {
357 + /* Discard expired password */
358 + FREE_AND_NULL(c->password);
359 + /* Reset expiry to maintain consistency */
360 + c->password_expiry_utc = TIME_MAX;
361 + }
362 if (c->username && c->password)
363 return;
364 if (c->quit)
@@ -360,7 +377,7 @@ void credential_approve(struct credential *c)
377
378 if (c->approved)
379 return;
363 - if (!c->username || !c->password)
380 + if (!c->username || !c->password || c->password_expiry_utc < time(NULL))
381 return;
382
383 credential_apply_config(c);
@@ -381,6 +398,7 @@ void credential_reject(struct credential *c)
398
399 FREE_AND_NULL(c->username);
400 FREE_AND_NULL(c->password);
401 + c->password_expiry_utc = TIME_MAX;
402 c->approved = 0;
403 }
404
credential.h
+2
@@ -126,10 +126,12 @@ struct credential {
126 char *protocol;
127 char *host;
128 char *path;
129 + timestamp_t password_expiry_utc;
130 };
131
132 #define CREDENTIAL_INIT { \
133 .helpers = STRING_LIST_INIT_DUP, \
134 + .password_expiry_utc = TIME_MAX, \
135 }
136
137 /* Initialize a credential structure, setting all fields to empty. */
t/t0300-credentials.sh
+94
@@ -35,6 +35,16 @@ test_expect_success 'setup helper scripts' '
35 test -z "$pass" || echo password=$pass
36 EOF
37
38 + write_script git-credential-verbatim-with-expiry <<-\EOF &&
39 + user=$1; shift
40 + pass=$1; shift
41 + pexpiry=$1; shift
42 + . ./dump
43 + test -z "$user" || echo username=$user
44 + test -z "$pass" || echo password=$pass
45 + test -z "$pexpiry" || echo password_expiry_utc=$pexpiry
46 + EOF
47 +
48 PATH="$PWD:$PATH"
49 '
50
@@ -109,6 +119,43 @@ test_expect_success 'credential_fill continues through partial response' '
119 EOF
120 '
121
122 +test_expect_success 'credential_fill populates password_expiry_utc' '
123 + check fill "verbatim-with-expiry one two 9999999999" <<-\EOF
124 + protocol=http
125 + host=example.com
126 + --
127 + protocol=http
128 + host=example.com
129 + username=one
130 + password=two
131 + password_expiry_utc=9999999999
132 + --
133 + verbatim-with-expiry: get
134 + verbatim-with-expiry: protocol=http
135 + verbatim-with-expiry: host=example.com
136 + EOF
137 +'
138 +
139 +test_expect_success 'credential_fill ignores expired password' '
140 + check fill "verbatim-with-expiry one two 5" "verbatim three four" <<-\EOF
141 + protocol=http
142 + host=example.com
143 + --
144 + protocol=http
145 + host=example.com
146 + username=three
147 + password=four
148 + --
149 + verbatim-with-expiry: get
150 + verbatim-with-expiry: protocol=http
151 + verbatim-with-expiry: host=example.com
152 + verbatim: get
153 + verbatim: protocol=http
154 + verbatim: host=example.com
155 + verbatim: username=one
156 + EOF
157 +'
158 +
159 test_expect_success 'credential_fill passes along metadata' '
160 check fill "verbatim one two" <<-\EOF
161 protocol=ftp
@@ -149,6 +196,24 @@ test_expect_success 'credential_approve calls all helpers' '
196 EOF
197 '
198
199 +test_expect_success 'credential_approve stores password expiry' '
200 + check approve useless <<-\EOF
201 + protocol=http
202 + host=example.com
203 + username=foo
204 + password=bar
205 + password_expiry_utc=9999999999
206 + --
207 + --
208 + useless: store
209 + useless: protocol=http
210 + useless: host=example.com
211 + useless: username=foo
212 + useless: password=bar
213 + useless: password_expiry_utc=9999999999
214 + EOF
215 +'
216 +
217 test_expect_success 'do not bother storing password-less credential' '
218 check approve useless <<-\EOF
219 protocol=http
@@ -159,6 +224,17 @@ test_expect_success 'do not bother storing password-less credential' '
224 EOF
225 '
226
227 +test_expect_success 'credential_approve does not store expired password' '
228 + check approve useless <<-\EOF
229 + protocol=http
230 + host=example.com
231 + username=foo
232 + password=bar
233 + password_expiry_utc=5
234 + --
235 + --
236 + EOF
237 +'
238
239 test_expect_success 'credential_reject calls all helpers' '
240 check reject useless "verbatim one two" <<-\EOF
@@ -181,6 +257,24 @@ test_expect_success 'credential_reject calls all helpers' '
257 EOF
258 '
259
260 +test_expect_success 'credential_reject erases credential regardless of expiry' '
261 + check reject useless <<-\EOF
262 + protocol=http
263 + host=example.com
264 + username=foo
265 + password=bar
266 + password_expiry_utc=5
267 + --
268 + --
269 + useless: erase
270 + useless: protocol=http
271 + useless: host=example.com
272 + useless: username=foo
273 + useless: password=bar
274 + useless: password_expiry_utc=5
275 + EOF
276 +'
277 +
278 test_expect_success 'usernames can be preserved' '
279 check fill "verbatim \"\" three" <<-\EOF
280 protocol=http