"M Hickford via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > From: M Hickford <mirth.hickford@xxxxxxxxx> > > 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. Perhaps that is a sensible expectation. It is unclear from the above description what happens instead without the "fix", though. By the way, I do not think your cc. Jeff King peff@xxxxxxxx cc: Matthew John Cheetham mjcheetham@xxxxxxxxxxx in 0/2 is doing anything; I've manually added them to Cc: of this message and left everything in the patch below, even though I am not commenting on anything there myself, to give them easier reference. Thanks. > Fixes for credential-cache and credential-store. > > Signed-off-by: M Hickford <mirth.hickford@xxxxxxxxx> > --- > Documentation/git-credential.txt | 4 ++-- > Documentation/gitcredentials.txt | 2 +- > builtin/credential-cache--daemon.c | 15 ++++++++------ > builtin/credential-store.c | 3 ++- > t/lib-credential.sh | 33 ++++++++++++++++++++++++++++++ > 5 files changed, 47 insertions(+), 10 deletions(-) > > diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt > index 0e6d9e85ec7..04bfb918de6 100644 > --- a/Documentation/git-credential.txt > +++ b/Documentation/git-credential.txt > @@ -38,8 +38,8 @@ to any configured credential helpers, which may store the credential > 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. > +any configured credential helpers, which may erase stored 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 100f045bb1a..65d652dc40e 100644 > --- 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 82f376d3351..5e3a766e42d 100644 > --- 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) > { > struct credential_cache_entry *e; > > - e = lookup_credential(c, c->password != NULL); > - if (e) > - e->expiration = 0; > + int i; > + for (i = 0; i < entries_nr; i++) { > + e = &entries[i]; > + if (credential_match(c, &e->item, c->password != NULL)) > + 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/builtin/credential-store.c b/builtin/credential-store.c > index e0ae028b1c3..85b147e460f 100644 > --- a/builtin/credential-store.c > +++ b/builtin/credential-store.c > @@ -36,7 +36,8 @@ static int parse_credential_file(const char *fn, > found_credential = 1; > if (match_cb) { > match_cb(&entry); > - break; > + if (strcmp(op, "erase")) > + break; > } > } > else if (other_cb) > diff --git a/t/lib-credential.sh b/t/lib-credential.sh > index f7e4e29c5e1..3f4100b6ce2 100644 > --- a/t/lib-credential.sh > +++ b/t/lib-credential.sh > @@ -45,6 +45,8 @@ helper_test_clean() { > reject $1 https example.com user2 > reject $1 https example.com user4 > reject $1 https example.com user5 > + reject $1 https example.com user6 > + reject $1 https example.com user7 > reject $1 http path.tld user > reject $1 https timeout.tld user > reject $1 https sso.tld > @@ -298,6 +300,37 @@ helper_test() { > EOF > ' > > + test_expect_success "helper ($HELPER) erases all matching credentials" ' > + check approve $HELPER <<-\EOF && > + protocol=https > + host=example.com > + username=user6 > + password=pass1 > + EOF > + check approve $HELPER <<-\EOF && > + protocol=https > + host=example.com > + username=user7 > + 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@xxxxxxxxxxx'\'': > + 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))
