Add support for wwwauth[] to credential helpers#5381
Merged
bk2204 merged 2 commits intogit-lfs:mainfrom Jun 5, 2023
Merged
Conversation
0c5d118 to
17c256a
Compare
chrisd8088
approved these changes
Jun 1, 2023
In the upcoming changes to the credential helper protocol, we'll be able to specify multiple values for the same key. In order to make this work, let's make the credential structure able to handle multiple values. Note that if there are multiple values, we'll only use the first one for most keys. That simplifies the logic and allows us to gracefully handle future extensions to the protocol.
Git recently added a new field to the credential helper, `wwwauth[]`, which may be repeated and includes all of the `WWW-Authenticate` headers so that the credential helper may choose an appropriate set of credentials and extract any sort of necessary data from the field (such as challenge). In Git LFS, we also want to do this with the `LFS-Authenticate` headers as well, since those are often used for the same purpose, so include both these headers in that field when passing them to `git credential fill`. Note that `git credential fill` only honours this value and passes it to the credential helper in Git 2.41 and newer (including the latest `HEAD`). However, just to be safe, let's add an undocumented and experimental option (`credential.*.skipwwwauth`) that users can use to control this, which we can remove in a few releases if it turns out it's not needed. Similarly, skip our new tests if we have an older version of Git where this doesn't work, since they'll otherwise fail.
17c256a to
badde87
Compare
chrisd8088
added a commit
that referenced
this pull request
Jan 14, 2025
The Creds structure in our "creds" package was first introduced in commit e8de7a3, where it was part of a different package in an early pre-alpha version of the Git LFS client. A single method was defined for the structure in that commit, the Buffer() method, which formatted lines of key/value pairs to be sent to the git-credential(1) command. In commit 3a271b1 of PR #1839 this method was converted into the current bufferCreds() function, a change made to avoid exporting the method out of the "lfsapi" package, where the structure was defined at that time. We next moved the Creds structure (which was then a simple map of strings to strings) and its one function into the current "creds" package in commit c752dcd of PR #3270. Later, in commits 5d5f90e of PR #5381 and 6783078 of PR #5803 we revised the Creds structure so its map contains arrays of strings, and added the IsMultistage() method, so we again have a single method defined for this structure. In subsequent commits in this PR we will refactor the bufferCreds() function to accept and return additional values. As a first step, in order to keep our code as consistent as possible, we convert it back into a method, as it was originally defined, except with the lowercase name of buffer() so it is not exported outside of the "creds" package. This retains the intent of commit 3a271b1 from PR #1839, but uses a naming pattern that is more idiomatic for the Go language. As well, we add a test function named TestCredsBufferFormat() to the accompanying creds/creds_test.go source file. This test function simply verifies the behaviour of the former bufferCreds() function that we have now renamed to the buffer() method. To permit comparison of the contents of the method's output buffer with the lines of key/value pairs expected by the test function, we also add an assertCredsLinesMatch() helper function which splits and sorts the lines in the buffer and compares them with the sorted set of expected output lines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Git recently added a new field to the credential helper,
wwwauth[], which may be repeated and includes all of theWWW-Authenticateheaders so that the credential helper may choose an appropriate set of credentials and extract any sort of necessary data from the field (such as challenge).In Git LFS, we also want to do this with the
LFS-Authenticateheaders as well, since those are often used for the same purpose, so include both these headers in that field when passing them togit credential fill.Note that
git credential fillonly honours this value and passes it to the credential helper in Git 2.41 and newer (including the latestHEAD). However, just to be safe, let's add an undocumented and experimental option (credential.*.skipwwwauth) that users can use to control this, which we can remove in a few releases if it turns out it's not needed. Similarly, skip our new tests if we have an older version of Git where this doesn't work, since they'll otherwise fail.In addition, perform some preparatory work to allow multiple values for a key, which we'll use in the second commit.
Fixes: #5222