M Hickford (2): credential: avoid erasing distinct password credential: erase all matching credentials Documentation/git-credential.txt | 2 +- Documentation/gitcredentials.txt | 2 +- builtin/credential-cache--daemon.c | 17 +++-- builtin/credential-store.c | 15 +++-- credential.c | 7 +- credential.h | 2 +- t/lib-credential.sh | 103 +++++++++++++++++++++++++++++ 7 files changed, 128 insertions(+), 20 deletions(-) base-commit: d7d8841f67f29e6ecbad85a11805c907d0f00d5d Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1525%2Fhickford%2Ferase-test-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1525/hickford/erase-test-v4 Pull-Request: https://github.com/git/git/pull/1525 Range-diff vs v3: 1: df3c8a15bf8 ! 1: 91d4b04b5e1 credential: avoid erasing distinct password @@ builtin/credential-store.c: static struct lock_file credential_lock; FILE *fh; struct strbuf line = STRBUF_INIT; @@ builtin/credential-store.c: static int parse_credential_file(const char *fn, - while (strbuf_getline_lf(&line, fh) != EOF) { if (!credential_from_url_gently(&entry, line.buf, 1) && -- entry.username && entry.password && + entry.username && entry.password && - credential_match(c, &entry)) { -+ entry.username && entry.password && -+ credential_match(c, &entry, match_password)) { ++ credential_match(c, &entry, match_password)) { found_credential = 1; if (match_cb) { match_cb(&entry); @@ credential.c: void credential_clear(struct credential *c) { #define CHECK(x) (!want->x || (have->x && !strcmp(want->x, have->x))) return CHECK(protocol) && -- CHECK(host) && -- CHECK(path) && + CHECK(host) && + CHECK(path) && - CHECK(username); -+ CHECK(host) && -+ CHECK(path) && -+ CHECK(username) && -+ (!match_password || CHECK(password)); ++ CHECK(username) && ++ (!match_password || CHECK(password)); #undef CHECK } @@ t/lib-credential.sh: helper_test_clean() { reject $1 https example.com user1 reject $1 https example.com user2 reject $1 https example.com user4 -+ reject $1 https example.com user5 -+ reject $1 https example.com user8 ++ reject $1 https example.com user-distinct-pass ++ reject $1 https example.com user-overwrite reject $1 http path.tld user reject $1 https timeout.tld user reject $1 https sso.tld @@ t/lib-credential.sh: helper_test() { + check approve $HELPER <<-\EOF && + protocol=https + host=example.com -+ username=user8 ++ username=user-overwrite + password=pass1 + EOF + check approve $HELPER <<-\EOF && + protocol=https + host=example.com -+ username=user8 ++ username=user-overwrite + password=pass2 + EOF + check fill $HELPER <<-\EOF && + protocol=https + host=example.com -+ username=user8 ++ username=user-overwrite + -- + protocol=https + host=example.com -+ username=user8 ++ username=user-overwrite + password=pass2 + EOF + check reject $HELPER <<-\EOF && + protocol=https + host=example.com -+ username=user8 ++ username=user-overwrite + password=pass2 + EOF + check fill $HELPER <<-\EOF + protocol=https + host=example.com -+ username=user8 ++ username=user-overwrite + -- + protocol=https + host=example.com -+ username=user8 ++ username=user-overwrite + password=askpass-password + -- -+ askpass: Password for '\''https://user8@xxxxxxxxxxx'\'': ++ askpass: Password for '\''https://user-overwrite@xxxxxxxxxxx'\'': + EOF + ' + @@ t/lib-credential.sh: helper_test() { + check approve $HELPER <<-\EOF && + protocol=https + host=example.com -+ username=user5 ++ username=user-distinct-pass + password=pass1 + EOF + check reject $HELPER <<-\EOF && + protocol=https + host=example.com -+ username=user5 ++ username=user-distinct-pass + password=pass2 + EOF + check fill $HELPER <<-\EOF + protocol=https + host=example.com -+ username=user5 ++ username=user-distinct-pass + -- + protocol=https + host=example.com -+ username=user5 ++ username=user-distinct-pass + password=pass1 + EOF + ' 2: e06d80e99a0 ! 2: 42f41b28e6e credential: erase all matching credentials @@ Commit message `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-libsecret) delete all matching - credentials, others (such as credential-cache and credential-store) - delete at most one matching credential. + 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 @@ Commit message "url=https://example.com\nusername=tim"; | git credential reject` erase all matching credentials. - Fix credential-cache and credential-store. + Fix credential-cache. Signed-off-by: M Hickford <mirth.hickford@xxxxxxxxx> @@ builtin/credential-cache--daemon.c: static void serve_one_client(FILE *in, FILE fprintf(out, "username=%s\n", e->item.username); fprintf(out, "password=%s\n", e->item.password); - ## builtin/credential-store.c ## -@@ builtin/credential-store.c: static int parse_credential_file(const char *fn, - found_credential = 1; - if (match_cb) { - match_cb(&entry); -- break; - } - } - else if (other_cb) - ## t/lib-credential.sh ## @@ t/lib-credential.sh: 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 https example.com user8 + 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 @@ t/lib-credential.sh: helper_test() { EOF ' @@ t/lib-credential.sh: helper_test() { + check approve $HELPER <<-\EOF && + protocol=https + host=example.com -+ username=user6 ++ username=user-erase1 + password=pass1 + EOF + check approve $HELPER <<-\EOF && + protocol=https + host=example.com -+ username=user7 ++ username=user-erase2 + password=pass1 + EOF + check reject $HELPER <<-\EOF && -- gitgitgadget
