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>

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index 0e6d9e85ec7074986f5b081abc48a98d27fa5200..a220afed4f35ba2a2ea1d7bd68dd07b1353d3522 100644 (file)
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -39,7 +39,7 @@ for later use.
 
 If the action is `reject`, git-credential will send the description to
 any configured credential helpers, which may erase any stored
-credential matching the description.
+credentials matching the description.
 
 If the action is `approve` or `reject`, no output should be emitted.
 

diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index 100f045bb1a0918e51cc3834a015913f9286dbbb..65d652dc40e63f70f9eab3debb9cf335470bbcef 100644 (file)
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -260,7 +260,7 @@ appended to its command line, which is one of:
 
 `erase`::
 
-       Remove a matching credential, if any, from the helper's storage.
+       Remove matching credentials, if any, from the helper's storage.
 
 The details of the credential will be provided on the helper's stdin
 stream. The exact format is the same as the input/output format of the

diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c
index f64dd21d335b3f2c9c6f3f0fc0c1488bb5c733a6..dc1cf2d25fc04c6f08c54f14589704f426bdd3d7 100644 (file)
--- a/builtin/credential-cache--daemon.c
+++ b/builtin/credential-cache--daemon.c
@@ -33,12 +33,12 @@ static void cache_credential(struct credential *c, int timeout)
        e->expiration = time(NULL) + timeout;
 }
 
-static struct credential_cache_entry *lookup_credential(const struct credential *c, int match_password)
+static struct credential_cache_entry *lookup_credential(const struct credential *c)
 {
        int i;
        for (i = 0; i < entries_nr; i++) {
                struct credential *e = &entries[i].item;
-               if (credential_match(c, e, match_password))
+               if (credential_match(c, e, 0))
                        return &entries[i];
        }
        return NULL;
@@ -48,9 +48,12 @@ static void remove_credential(const struct credential *c, int match_password)
 {
        struct credential_cache_entry *e;
 
-       e = lookup_credential(c, match_password);
-       if (e)
-               e->expiration = 0;
+       int i;
+       for (i = 0; i < entries_nr; i++) {
+               e = &entries[i];
+               if (credential_match(c, &e->item, match_password))
+                       e->expiration = 0;
+       }
 }
 
 static timestamp_t check_expirations(void)
@@ -127,7 +130,7 @@ static void serve_one_client(FILE *in, FILE *out)
        if (read_request(in, &c, &action, &timeout) < 0)
                /* ignore error */ ;
        else if (!strcmp(action.buf, "get")) {
-               struct credential_cache_entry *e = lookup_credential(&c, 0);
+               struct credential_cache_entry *e = lookup_credential(&c);
                if (e) {
                        fprintf(out, "username=%s\n", e->item.username);
                        fprintf(out, "password=%s\n", e->item.password);

diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 77baec53b6a58c2af23f5804ef0f1a6f4cac32a3..032b2d8fcc48aa2cd72c60fa05f9b1299680f92c 100644 (file)
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -46,6 +46,8 @@ helper_test_clean() {
        reject $1 https example.com user4
        reject $1 https example.com user-distinct-pass
        reject $1 https example.com user-overwrite
+       reject $1 https example.com user-erase1
+       reject $1 https example.com user-erase2
        reject $1 http path.tld user
        reject $1 https timeout.tld user
        reject $1 https sso.tld
@@ -342,6 +344,37 @@ helper_test() {
                EOF
        '
 
+       test_expect_success "helper ($HELPER) erases all matching credentials" '
+               check approve $HELPER <<-\EOF &&
+               protocol=https
+               host=example.com
+               username=user-erase1
+               password=pass1
+               EOF
+               check approve $HELPER <<-\EOF &&
+               protocol=https
+               host=example.com
+               username=user-erase2
+               password=pass1
+               EOF
+               check reject $HELPER <<-\EOF &&
+               protocol=https
+               host=example.com
+               EOF
+               check fill $HELPER <<-\EOF
+               protocol=https
+               host=example.com
+               --
+               protocol=https
+               host=example.com
+               username=askpass-username
+               password=askpass-password
+               --
+               askpass: Username for '\''https://example.com'\'':
+               askpass: Password for '\''https://askpass-username@example.com'\'':
+               EOF
+       '
+
        : ${GIT_TEST_LONG_CRED_BUFFER:=1024}
        # 23 bytes accounts for "wwwauth[]=basic realm=" plus NUL
        LONG_VALUE_LEN=$((GIT_TEST_LONG_CRED_BUFFER - 23))