-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Support arbitrary HTTP credential schemes for authentication #5779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
chrisd8088
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nifty, thank you!
t/cmd/git-credential-lfstest.go
Outdated
|
|
||
| func credsFromFilename(file string) (string, string, error) { | ||
| func credsFromFilename(file string) (string, string, string, error) { | ||
| userPass, err := os.ReadFile(file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor note that userPass is no longer entirely accurate as a variable name here, since it's expected to hold authorization type, username, and credential fields instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'll fix that.
t/t-credentials.sh
Outdated
| grep "Tracking \"\*.dat\"" track.log | ||
|
|
||
| contents="b" | ||
| contents_oid=$(calc_oid "$contents") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this contents_oid variable is used, so perhaps it could be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'll drop that.
t/t-credentials.sh
Outdated
|
|
||
| GIT_TERMINAL_PROMPT=0 GIT_TRACE=1 GIT_TRANSFER_TRACE=1 GIT_CURL_VERBOSE=1 git push origin new-branch 2>&1 | tee push.log | ||
| grep "Uploading LFS objects: 100% (1/1), 1 B" push.log | ||
| echo "approvals:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor thought: these echo statements could possibly just be inline comments, as I don't think anything output to stdout is usually visible when running the tests.
In newer versions of Git, the credential helper protocol has learned to support capabilities and credentials that aren't based around a username and password. This allows the use of Bearer credentials, and it allows credential helpers to implement complex functionality like Digest and NTLM (both of which require insecure password storage) so we don't have to. We'd like to support this in our testsuite, so extend our credential helper to accept these new capabilities and use them to determine a response. In conformance with the credential helper protocol, we only advertise support for capabilities which we ourselves support. In our files in `CREDSDIR`, instead of looking for two parts separated by colons which are the username and password, accept three parts, which are the authentication type, username, and credential. If the authentication type is empty, then the credential is the password; otherwise, the username is empty, and the credential is sent in the credential field. Right now, we don't make use of this new functionality, but we do update existing credential files to make them compatible. Use of the new functionality will come in a future commit.
Implement some endpoints with Bearer authentication so we can test this elsewhere in the testsuite. If the path starts with `/auth-bearer`, send a `WWW-Authenticate` header with `Bearer` instead of an `LFS-Authenticate` header with `Basic`. This lets us test both types of header as well as the new Bearer authentication.
Now that our test credential helper and test server are capable of supporting the new authtype and credential fields, let's pass the appropriate capability to the helper. We use the fields if they exist, and pass them back to the `approve` call to the helper on success, along with the capability. If these fields don't both exist, we fall back to the username and password fields. This allows us to do thing such as query different accounts with the username if one is specified in the URL, but get the credentials back in a format that contains a non-Basic response. Since we now support more than just Basic and Negotiate, let's make sure we iterate through all of the supported authentication schemes, not just those two. This allows the credential helper to support multiple schemes.
bbba2c4 to
e2cd22c
Compare
In commit 08998d6 of PR git-lfs#5779 we updated our Git credential helper test program to reflect the new support in Git v2.46.0 for authentication schemes other than those using usernames and passwords, specifically by accepting input lines with the "capability[]" key and replying with similar lines as appropriate. In particular, if the client sends one or more "capability[]" lines, we check their values against the list of capabilities our helper program supports, which at the moment is just the "authtype" and "state" capabilities. If any matches are found, we include those in "capability[]" lines in our reply, excluding any capabilities the client announced but which our program does not support. At the moment, we do this by first overwriting the "capability[]" entry in the map of key/value pairs we created from the client's input, and then copying that entry into the "return" map of key/value pairs we'll send back to the client. Finally, we make sure to output the values from the "capability[]" entry in that map before sending the values of all the remaining entries. (Note that each entry in these maps may have multiple values. If a key is multi-valued, we send each value on a separate line in the output.) We can simplify our handling of these "capability[]" lines somewhat by skipping the step where we overwrite the entry in the map of input lines, and also by skipping the copy of the values we intend to return into the "result" map. Instead, we can just return the filtered set of capabilities we support from our discoverCapabilities() function, and then output the values from that set first before sending the values from our "return" map. This does mean that we only pass the filtered set of capabilities to the Serialize() method of our "credential" structure, instead of the full set received from the client. But this change has no functional impact because that method only needs to check for the capability types we support ("authtype" and "state"), so those will always be in the filtered set of capabilities, assuming the client sent them to us.
In commit 08998d6 of PR git-lfs#5779 we updated our Git credential helper test program to reflect the new support in Git v2.46.0 for authentication schemes other than those using usernames and passwords, specifically by accepting input lines with the "capability[]" key and replying with similar lines as appropriate. In particular, if the client sends one or more "capability[]" lines, we check their values against the list of capabilities our helper program supports, which at the moment is just the "authtype" and "state" capabilities. If any matches are found, we include those in "capability[]" lines in our reply, excluding any capabilities the client announced but which our program does not support. At the moment, we do this by first overwriting the "capability[]" entry in the map of key/value pairs we created from the client's input, and then copying that entry into the "return" map of key/value pairs we'll send back to the client. Finally, we make sure to output the values from the "capability[]" entry in that map before sending the values of all the remaining entries. (Note that each entry in these maps may have multiple values. If a key is multi-valued, we send each value on a separate line in the output.) We can simplify our handling of these "capability[]" lines somewhat by skipping the step where we overwrite the entry in the map of input lines, and also by skipping the copy of the values we intend to return into the "result" map. Instead, we can just return the filtered set of capabilities we support from our discoverCapabilities() function, and then output the values from that set first before sending the values from our "return" map. This does mean that we only pass the filtered set of capabilities to the Serialize() method of our "credential" structure, instead of the full set received from the client. But this change has no functional impact because that method only needs to check for the capability types we support ("authtype" and "state"), so those will always be in the filtered set of capabilities, assuming the client sent them to us.
In commit 08998d6 of PR git-lfs#5779 we updated our Git credential helper test program to reflect the new support in Git v2.46.0 for authentication schemes other than those using usernames and passwords, specifically by accepting input lines with the "capability[]" key and replying with similar lines as appropriate. In particular, if the client sends one or more "capability[]" lines, we check their values against the list of capabilities our helper program supports, which at the moment is just the "authtype" and "state" capabilities. If any matches are found, we include those in "capability[]" lines in our reply, excluding any capabilities the client announced but which our program does not support. At the moment, we do this by first overwriting the "capability[]" entry in the map of key/value pairs we created from the client's input, and then copying that entry into the "return" map of key/value pairs we'll send back to the client. Finally, we make sure to output the values from the "capability[]" entry in that map before sending the values of all the remaining entries. (Note that each entry in these maps may have multiple values. If a key is multi-valued, we send each value on a separate line in the output.) We can simplify our handling of these "capability[]" lines somewhat by skipping the step where we overwrite the entry in the map of input lines, and also by skipping the copy of the values we intend to return into the "result" map. Instead, we can just return the filtered set of capabilities we support from our discoverCapabilities() function, and then output the values from that set first before sending the values from our "return" map. This does mean that we only pass the filtered set of capabilities to the Serialize() method of our "credential" structure, instead of the full set received from the client. But this change has no functional impact because that method only needs to check for the capability types we support ("authtype" and "state"), so those will always be in the filtered set of capabilities, assuming the client sent them to us.
In commit 08998d6 of PR git-lfs#5779 we updated our Git credential helper test program to reflect the new support in Git v2.46.0 for authentication schemes other than those using usernames and passwords, specifically by accepting input lines with the "capability[]" key and replying with similar lines as appropriate. In particular, if the client sends one or more "capability[]" lines, we check their values against the list of capabilities our helper program supports, which at the moment is just the "authtype" and "state" capabilities. If any matches are found, we include those in "capability[]" lines in our reply, excluding any capabilities the client announced but which our program does not support. At the moment, we do this by first overwriting the "capability[]" entry in the map of key/value pairs we created from the client's input, and then copying that entry into the "return" map of key/value pairs we will send back to the client. Finally, we make sure to output the values from the "capability[]" entry in that map before sending the values of all the remaining entries. (Note that each entry in these maps may have multiple values. If a key is multi-valued, we send each value on a separate line in the output.) We can simplify our handling of these "capability[]" lines somewhat by skipping the step where we overwrite the entry in the map of input lines, and also by not making a copy of the values we intend to return into the "result" map. Instead, we can just return the filtered set of capabilities we support from our discoverCapabilities() function, and then output the values from that set first before sending the values from our "return" map. This does mean that we only pass the filtered set of capabilities to the Serialize() method of our "credential" structure, instead of the full set received from the client. But this change has no functional impact because that method only needs to check for the capability types we support ("authtype" and "state"), so those will always be in the filtered set of capabilities, assuming the client sent them to us.
Git recently learned to announce capabilities for credential helpers and this functionality will be in Git 2.46. One of the capabilities is the ability for credential helpers to specify arbitrary HTTP credential schemes and the credentials for these schemes. This allows users with a suitable credential helper to implement support for, say, Bearer authentication where the credentials come from a credential helper.
With this capability, Git LFS will advertise the
capability[]=authtypeline, and a suitable version of Git will pass this along to the helper, which can then return something likeauthtype=Bearerandcredential=my-bearer-tokento provide aAuthorization: Bearer my-bearer-tokenheader. It can also return other schemes as well, and thecredentialfield can include any necessary parameters that the scheme may require.In order to implement this in Git LFS, we first need to adjust our test credential helper and the server so that they can handle this new authentication type and return appropriate credentials. Then, we add support in the main codebase and verify that it works as expected.
Note that Git versions which do not support this capability will exit unsuccessfully for
git credential capability(causing our tests to be skipped) and will do nothing with thecapability[]=authtypedeclaration, which means that it will not be passed to the credential helper. At that point, the credential helper can either return the previous username/password pair, or decide to return nothing at all.There is an additional capability to allow multi-stage authentication, such as NTLM or Kerberos, in the credential helper. These both require two round trips for authentication to succeed, which needs additional support from the helper and Git or Git LFS. Supporting NTLM in Git LFS via a suitable credential helper was in fact one of the goals of the upstream series, in addition to discouraging the use of the config file for storing authentication credentials (such as used to be required for Bearer auth). However, the additional capability required for that is not implemented here, and can be implemented in an additional series in due course.