| Message ID | pull.1525.v2.git.git.1686778838.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | credential: improvements to erase in helpers | expand |
"M Hickford via GitGitGadget" <gitgitgadget@gmail.com> writes: > M Hickford (2): > credential: avoid erasing distinct password > credential: erase all matching credentials Will queue. Comments by folks with more experience (than me) who have worked on the credential subsystem are very much welcomed. Thanks. > > Documentation/git-credential.txt | 4 +- > Documentation/gitcredentials.txt | 2 +- > builtin/credential-cache--daemon.c | 11 ++++-- > builtin/credential-store.c | 21 ++++++----- > credential.c | 11 +++--- > credential.h | 2 +- > t/lib-credential.sh | 59 ++++++++++++++++++++++++++++++ > 7 files changed, 88 insertions(+), 22 deletions(-) > > > base-commit: fe86abd7511a9a6862d5706c6fa1d9b57a63ba09 > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1525%2Fhickford%2Ferase-test-v2 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1525/hickford/erase-test-v2 > Pull-Request: https://github.com/git/git/pull/1525 > > Range-diff vs v1: > > 1: 35ee1795bcd = 1: 35ee1795bcd credential: avoid erasing distinct password > 2: fcdb579263f ! 2: 9b12f17dc7e credential: erase all matching credentials > @@ Metadata > ## Commit message ## > credential: erase all matching credentials > > - 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. > + `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. > > - Fixes for credential-cache and credential-store. > + Test that helpers erase all matching credentials. This behaviour is > + easiest to reason about. 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. > + > + Fix credential-cache and credential-store. > > Signed-off-by: M Hickford <mirth.hickford@gmail.com>
On Wed, Jun 14, 2023 at 02:56:11PM -0700, Junio C Hamano wrote: > "M Hickford via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > M Hickford (2): > > credential: avoid erasing distinct password > > credential: erase all matching credentials > > Will queue. Comments by folks with more experience (than me) who > have worked on the credential subsystem are very much welcomed. I think the direction is good, but there are a few small bits in each patch to tweak, and we'll want a v3. -Peff
M Hickford (2): credential: avoid erasing distinct password credential: erase all matching credentials Documentation/git-credential.txt | 4 +- Documentation/gitcredentials.txt | 2 +- builtin/credential-cache--daemon.c | 11 ++++-- builtin/credential-store.c | 21 ++++++----- credential.c | 11 +++--- credential.h | 2 +- t/lib-credential.sh | 59 ++++++++++++++++++++++++++++++ 7 files changed, 88 insertions(+), 22 deletions(-) base-commit: fe86abd7511a9a6862d5706c6fa1d9b57a63ba09 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1525%2Fhickford%2Ferase-test-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1525/hickford/erase-test-v2 Pull-Request: https://github.com/git/git/pull/1525 Range-diff vs v1: 1: 35ee1795bcd = 1: 35ee1795bcd credential: avoid erasing distinct password 2: fcdb579263f ! 2: 9b12f17dc7e credential: erase all matching credentials @@ Metadata ## Commit message ## credential: erase all matching credentials - 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. + `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. - Fixes for credential-cache and credential-store. + Test that helpers erase all matching credentials. This behaviour is + easiest to reason about. 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. + + Fix credential-cache and credential-store. Signed-off-by: M Hickford <mirth.hickford@gmail.com>