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 | 18 ++---
credential.c | 11 +--
credential.h | 2 +-
t/lib-credential.sh | 103 +++++++++++++++++++++++++++++
7 files changed, 131 insertions(+), 24 deletions(-)
base-commit: d7d8841f67f29e6ecbad85a11805c907d0f00d5d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1525%2Fhickford%2Ferase-test-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1525/hickford/erase-test-v3
Pull-Request: https://github.com/git/git/pull/1525
Range-diff vs v2:
1: 35ee1795bcd ! 1: df3c8a15bf8 credential: avoid erasing distinct password
@@ builtin/credential-cache--daemon.c: static void cache_credential(struct credenti
return &entries[i];
}
return NULL;
-@@ builtin/credential-cache--daemon.c: static void remove_credential(const struct credential *c)
+ }
+
+-static void remove_credential(const struct credential *c)
++static void remove_credential(const struct credential *c, int match_password)
{
struct credential_cache_entry *e;
- e = lookup_credential(c);
-+ e = lookup_credential(c, c->password != NULL);
++ e = lookup_credential(c, match_password);
if (e)
e->expiration = 0;
}
@@ builtin/credential-cache--daemon.c: static void serve_one_client(FILE *in, FILE
if (e) {
fprintf(out, "username=%s\n", e->item.username);
fprintf(out, "password=%s\n", e->item.password);
+@@ builtin/credential-cache--daemon.c: static void serve_one_client(FILE *in, FILE *out)
+ exit(0);
+ }
+ else if (!strcmp(action.buf, "erase"))
+- remove_credential(&c);
++ remove_credential(&c, 1);
+ else if (!strcmp(action.buf, "store")) {
+ if (timeout < 0)
+ warning("cache client didn't specify a timeout");
+ else if (!c.username || !c.password)
+ warning("cache client gave us a partial credential");
+ else {
+- remove_credential(&c);
++ remove_credential(&c, 0);
+ cache_credential(&c, timeout);
+ }
+ }
## builtin/credential-store.c ##
@@ builtin/credential-store.c: static struct lock_file credential_lock;
@@ builtin/credential-store.c: static struct lock_file credential_lock;
void (*match_cb)(struct credential *),
- void (*other_cb)(struct strbuf *))
+ void (*other_cb)(struct strbuf *),
-+ const char* op)
++ int match_password)
{
FILE *fh;
struct strbuf line = STRBUF_INIT;
- struct credential entry = CREDENTIAL_INIT;
- int found_credential = 0;
-+ const int match_password = !strcmp(op, "erase") && c->password != NULL;
-
- fh = fopen(fn, "r");
- if (!fh) {
@@ builtin/credential-store.c: static int parse_credential_file(const char *fn,
while (strbuf_getline_lf(&line, fh) != EOF) {
@@ builtin/credential-store.c: static void print_line(struct strbuf *buf)
static void rewrite_credential_file(const char *fn, struct credential *c,
- struct strbuf *extra)
-+ struct strbuf *extra, const char *op)
++ struct strbuf *extra, int match_password)
{
int timeout_ms = 1000;
@@ builtin/credential-store.c: static void rewrite_credential_file(const char *fn,
if (extra)
print_line(extra);
- parse_credential_file(fn, c, NULL, print_line);
-+ parse_credential_file(fn, c, NULL, print_line, op);
++ parse_credential_file(fn, c, NULL, print_line, match_password);
if (commit_lock_file(&credential_lock) < 0)
die_errno("unable to write credential store");
}
@@ builtin/credential-store.c: static void store_credential_file(const char *fn, st
}
- rewrite_credential_file(fn, c, &buf);
-+ rewrite_credential_file(fn, c, &buf, "store");
++ rewrite_credential_file(fn, c, &buf, 0);
strbuf_release(&buf);
}
@@ builtin/credential-store.c: static void remove_credential(const struct string_li
for_each_string_list_item(fn, fns)
if (!access(fn->string, F_OK))
- rewrite_credential_file(fn->string, c, NULL);
-+ rewrite_credential_file(fn->string, c, NULL, "erase");
++ rewrite_credential_file(fn->string, c, NULL, 1);
}
static void lookup_credential(const struct string_list *fns, struct credential *c)
@@ builtin/credential-store.c: static void lookup_credential(const struct string_li
for_each_string_list_item(fn, fns)
- if (parse_credential_file(fn->string, c, print_entry, NULL))
-+ if (parse_credential_file(fn->string, c, print_entry, NULL, "get"))
++ if (parse_credential_file(fn->string, c, print_entry, NULL, 0))
return; /* Found credential */
}
@@ 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 user8
reject $1 http path.tld user
reject $1 https timeout.tld user
reject $1 https sso.tld
@@ t/lib-credential.sh: helper_test() {
EOF
'
++ test_expect_success "helper ($HELPER) overwrites on store" '
++ check approve $HELPER <<-\EOF &&
++ protocol=https
++ host=example.com
++ username=user8
++ password=pass1
++ EOF
++ check approve $HELPER <<-\EOF &&
++ protocol=https
++ host=example.com
++ username=user8
++ password=pass2
++ EOF
++ check fill $HELPER <<-\EOF &&
++ protocol=https
++ host=example.com
++ username=user8
++ --
++ protocol=https
++ host=example.com
++ username=user8
++ password=pass2
++ EOF
++ check reject $HELPER <<-\EOF &&
++ protocol=https
++ host=example.com
++ username=user8
++ password=pass2
++ EOF
++ check fill $HELPER <<-\EOF
++ protocol=https
++ host=example.com
++ username=user8
++ --
++ protocol=https
++ host=example.com
++ username=user8
++ password=askpass-password
++ --
++ askpass: Password for '\''https://user8@example.com'\'':
++ EOF
++ '
++
+ test_expect_success "helper ($HELPER) can forget host" '
+ check reject $HELPER <<-\EOF &&
+ protocol=https
+@@ t/lib-credential.sh: helper_test() {
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) does not erase a password distinct from input" '
+ check approve $HELPER <<-\EOF &&
+ protocol=https
2: 9b12f17dc7e ! 2: e06d80e99a0 credential: erase all matching credentials
@@ Commit message
Signed-off-by: M Hickford <mirth.hickford@gmail.com>
## Documentation/git-credential.txt ##
-@@ Documentation/git-credential.txt: to any configured credential helpers, which may store the credential
- for later use.
+@@ Documentation/git-credential.txt: for later use.
If the action is `reject`, git-credential will send the description to
--any configured credential helpers, which may erase any stored
+ 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.
++credentials matching the description.
If the action is `approve` or `reject`, no output should be emitted.
@@ builtin/credential-cache--daemon.c: static void cache_credential(struct credenti
return &entries[i];
}
return NULL;
-@@ builtin/credential-cache--daemon.c: static void remove_credential(const struct credential *c)
+@@ builtin/credential-cache--daemon.c: static void remove_credential(const struct credential *c, int match_password)
{
struct credential_cache_entry *e;
-- e = lookup_credential(c, c->password != NULL);
+- 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, c->password != NULL))
++ if (credential_match(c, &e->item, match_password))
+ e->expiration = 0;
+ }
}
@@ builtin/credential-store.c: static int parse_credential_file(const char *fn,
if (match_cb) {
match_cb(&entry);
- break;
-+ if (strcmp(op, "erase"))
-+ break;
}
}
else if (other_cb)
@@ t/lib-credential.sh: helper_test_clean() {
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 http path.tld user
reject $1 https timeout.tld user
- reject $1 https sso.tld
@@ t/lib-credential.sh: helper_test() {
EOF
'M Hickford via GitGitGadget <gitgitgadget@gmail.com>