From: "M Hickford via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>,
Matthew John Cheetham <mjcheetham@outlook.com>,
M Hickford <mirth.hickford@gmail.com>
Subject: [PATCH v4 0/2] credential: improvements to erase in helpers
Date: Thu, 15 Jun 2023 19:19:31 +0000 [thread overview]
Message-ID: <pull.1525.v4.git.git.1686856773.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1525.v3.git.git.1686809004.gitgitgadget@gmail.com>
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@example.com'\'':
++ askpass: Password for '\''https://user-overwrite@example.com'\'':
+ 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@gmail.com>
@@ 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
next prev parent reply other threads:[~2023-06-15 19:19 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-14 11:23 [PATCH 0/2] credential: improvements to erase in helpers M Hickford via GitGitGadget
2023-06-14 11:23 ` [PATCH 1/2] credential: avoid erasing distinct password M Hickford via GitGitGadget
2023-06-14 11:23 ` [PATCH 2/2] credential: erase all matching credentials M Hickford via GitGitGadget
2023-06-14 16:00 ` Junio C Hamano
2023-06-14 21:35 ` M Hickford
2023-06-14 21:40 ` [PATCH v2 0/2] credential: improvements to erase in helpers M Hickford via GitGitGadget
2023-06-14 21:40 ` [PATCH v2 1/2] credential: avoid erasing distinct password M Hickford via GitGitGadget
2023-06-14 22:43 ` Jeff King
2023-06-15 4:51 ` M Hickford
2023-06-14 21:40 ` [PATCH v2 2/2] credential: erase all matching credentials M Hickford via GitGitGadget
2023-06-14 22:51 ` Jeff King
2023-06-15 4:57 ` M Hickford
2023-06-14 21:56 ` [PATCH v2 0/2] credential: improvements to erase in helpers Junio C Hamano
2023-06-14 22:51 ` Jeff King
2023-06-15 6:03 ` [PATCH v3 " M Hickford via GitGitGadget
2023-06-15 6:03 ` [PATCH v3 1/2] credential: avoid erasing distinct password M Hickford via GitGitGadget
2023-06-15 7:08 ` Jeff King
2023-06-15 6:03 ` [PATCH v3 2/2] credential: erase all matching credentials M Hickford via GitGitGadget
2023-06-15 7:09 ` Jeff King
2023-06-15 19:19 ` M Hickford via GitGitGadget [this message]
2023-06-15 19:19 ` [PATCH v4 1/2] credential: avoid erasing distinct password M Hickford via GitGitGadget
2023-06-15 19:19 ` [PATCH v4 2/2] credential: erase all matching credentials M Hickford via GitGitGadget
2023-06-15 21:09 ` [PATCH v4 0/2] credential: improvements to erase in helpers Junio C Hamano
2023-06-15 21:21 ` Jeff King
2023-06-15 21:52 ` Junio C Hamano
2023-06-16 16:54 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1525.v4.git.git.1686856773.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=mirth.hickford@gmail.com \
--cc=mjcheetham@outlook.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).