credential: improvements to erase in helpers
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v4 Latest2 patches 4 repliesIn review
- v32 patches 2 repliesSuperseded
- v22 patches 6 repliesSuperseded
Earlier versions · 1
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v1 | 2 patches | 2 replies | Superseded |
Patches in v4 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/2 | credential: avoid erasing distinct passwordbuiltin/ builtin/ credential.c +2 more | +90 | 0 replies | |
| 2/2 | credential: erase all matching credentialsDocumentation/ Documentation/ builtin/ +1 more | +44 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v4 yet.
Cover letter [PATCH v4 0/2] credential: improvements to erase in helpers
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://[email protected]'\'':
++ askpass: Password for '\''https://[email protected]'\'':
+ 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.
@@ 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 &&
Signature · 1 line
gitgitgadget