credential: erase all matching credentials
`credential reject` sends the erase action to each helper, but the exact behaviour of erase isn't specified in documentation or tests. Some helpers (such as credential-store and credential-libsecret) delete all matching credentials, others (such as credential-cache) delete at most one matching credential. Test that helpers erase all matching credentials. This behaviour is easiest to reason about. Users expect that `echo "url=https://example.com" | git credential reject` or `echo "url=https://example.com\nusername=tim" | git credential reject` erase all matching credentials. Fix credential-cache. Signed-off-by: M Hickford <mirth.hickford@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
M Hickford committed
Jun 15, 2023 at 19:19 UTC
6c26da8404c8acfed62fa4775b7b591f099bcd33
4 files changed
+44
-8
Documentation/git-credential.txt
+1
-1
@@ -39,7 +39,7 @@ for later use.
39
40
If the action is `reject`, git-credential will send the description to
41
any configured credential helpers, which may erase any stored
42
-credential matching the description.
42
+credentials matching the description.
43
44
If the action is `approve` or `reject`, no output should be emitted.
45
Documentation/gitcredentials.txt
+1
-1
@@ -260,7 +260,7 @@ appended to its command line, which is one of:
260
261
`erase`::
262
263
- Remove a matching credential, if any, from the helper's storage.
263
+ Remove matching credentials, if any, from the helper's storage.
264
265
The details of the credential will be provided on the helper's stdin
266
stream. The exact format is the same as the input/output format of the
builtin/credential-cache--daemon.c
+9
-6
@@ -33,12 +33,12 @@ static void cache_credential(struct credential *c, int timeout)
33
e->expiration = time(NULL) + timeout;
34
}
35
36
-static struct credential_cache_entry *lookup_credential(const struct credential *c, int match_password)
36
+static struct credential_cache_entry *lookup_credential(const struct credential *c)
37
{
38
int i;
39
for (i = 0; i < entries_nr; i++) {
40
struct credential *e = &entries[i].item;
41
- if (credential_match(c, e, match_password))
41
+ if (credential_match(c, e, 0))
42
return &entries[i];
43
}
44
return NULL;
@@ -48,9 +48,12 @@ static void remove_credential(const struct credential *c, int match_password)
48
{
49
struct credential_cache_entry *e;
50
51
- e = lookup_credential(c, match_password);
52
- if (e)
53
- e->expiration = 0;
51
+ int i;
52
+ for (i = 0; i < entries_nr; i++) {
53
+ e = &entries[i];
54
+ if (credential_match(c, &e->item, match_password))
55
+ e->expiration = 0;
56
+ }
57
}
58
59
static timestamp_t check_expirations(void)
@@ -127,7 +130,7 @@ static void serve_one_client(FILE *in, FILE *out)
130
if (read_request(in, &c, &action, &timeout) < 0)
131
/* ignore error */ ;
132
else if (!strcmp(action.buf, "get")) {
130
- struct credential_cache_entry *e = lookup_credential(&c, 0);
133
+ struct credential_cache_entry *e = lookup_credential(&c);
134
if (e) {
135
fprintf(out, "username=%s\n", e->item.username);
136
fprintf(out, "password=%s\n", e->item.password);
t/lib-credential.sh
+33
@@ -46,6 +46,8 @@ helper_test_clean() {
46
reject $1 https example.com user4
47
reject $1 https example.com user-distinct-pass
48
reject $1 https example.com user-overwrite
49
+ reject $1 https example.com user-erase1
50
+ reject $1 https example.com user-erase2
51
reject $1 http path.tld user
52
reject $1 https timeout.tld user
53
reject $1 https sso.tld
@@ -342,6 +344,37 @@ helper_test() {
344
EOF
345
'
346
347
+ test_expect_success "helper ($HELPER) erases all matching credentials" '
348
+ check approve $HELPER <<-\EOF &&
349
+ protocol=https
350
+ host=example.com
351
+ username=user-erase1
352
+ password=pass1
353
+ EOF
354
+ check approve $HELPER <<-\EOF &&
355
+ protocol=https
356
+ host=example.com
357
+ username=user-erase2
358
+ password=pass1
359
+ EOF
360
+ check reject $HELPER <<-\EOF &&
361
+ protocol=https
362
+ host=example.com
363
+ EOF
364
+ check fill $HELPER <<-\EOF
365
+ protocol=https
366
+ host=example.com
367
+ --
368
+ protocol=https
369
+ host=example.com
370
+ username=askpass-username
371
+ password=askpass-password
372
+ --
373
+ askpass: Username for '\''https://example.com'\'':
374
+ askpass: Password for '\''https://askpass-username@example.com'\'':
375
+ EOF
376
+ '
377
+
378
: ${GIT_TEST_LONG_CRED_BUFFER:=1024}
379
# 23 bytes accounts for "wwwauth[]=basic realm=" plus NUL
380
LONG_VALUE_LEN=$((GIT_TEST_LONG_CRED_BUFFER - 23))