Support multistage authentication with a Git credential helper#5803
Merged
bk2204 merged 7 commits intogit-lfs:mainfrom Jun 26, 2024
Merged
Support multistage authentication with a Git credential helper#5803bk2204 merged 7 commits intogit-lfs:mainfrom
bk2204 merged 7 commits intogit-lfs:mainfrom
Conversation
a14e76e to
82b2acd
Compare
chrisd8088
approved these changes
Jun 19, 2024
In a future commit, we'll want to use the `slices` package, so let's bump the required version of Go to 1.21. This is already the oldest supported version we test with anyway.
We're going to start processing more complex sets of credentials in the future, so to keep our code nice and tidy, move the credential handling into a struct so that we can simplify things. This allows us to return only two values from `credsForHostAndPath` instead of the current four, a number which would only increase in the future.
The new functionality in Git allows persisting state across requests and providing multistage responses. To allow testing that, let's implement the `state[]` and `continue` arguments. The `state[]` entry contains one or more items of state that are prefixed by the credential helper to distinguish them from those from other credential helpers. These are passed back to the credential helper on success, failure, or a second round. The `continue` boolean value indicates whether, upon receiving a 401 response, authentication should continue for another round. This flag allows three-legged auth such as NTLM or Kerberos (via either NTLM or Negotiate schemes) to be successfully implemented by the credential helper. Now that we're writing a serialized credential directly into a variable, let's make sure we copy the relevant pieces from the original credential structure, including host, path, protocol, and capabilities, so that these are passed along. Except for the capabilities, none of these are strictly needed since Git will fill them in, but it's still nicer to provide them in the output for completeness. We document the possible values of a line in the configuration file to make them easier to understand as well.
In the future, we'll want to parse multiple credentials per file, so let's move the parsing of a single credential to its own function.
Now that we're implementing multistage authentication, let's allow multiple credentials in the same file so that we can test the multistage functionality. Add some tests to verify that the individual commands work as expected.
We'd like to support multistage authentication that Git now supports in credential helpers, so let's add some support in our fake backend server. We create a fake authentication scheme, Multistage, that requires two sets of round trips to successfully authenticate. If the appropriate credential is sent in each stage, then succeed; otherwise, fail.
In 2.46, Git will support multistage authentication from credential helpers. This will allow Git and Git LFS to implement functionality like NTLM and Kerberos in the credential helper, which lets this functionality to work even if Git LFS doesn't support it natively. This requires two separate pieces of data. First, it involves a `state[]` field, which each credential helper can add to keep track of state. Second, it allows the usage of a boolean `continue` field, which indicates that the response is multistage and this is not the final stage. In order to make this work, we adjust a few things. First, we advertise the `state` capability. Additionally, we save and pass back the `state[]` fields that the credential helper may send to us. We also don't change the authentication scheme if the helper told us that this was a multistage response. Finally, we add a check to avoid a credential helper getting stuck in an infinite loop if it keeps handing back the same credentials.
82b2acd to
6783078
Compare
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Sep 3, 2024
In commit 8e6641b of PR git-lfs#5803 we refactored our git-credential-lfstest test utility program so as to use a new internal "credential" structure to store the details of a given credential, as part of that PR's changes to support multi-stage authentication in a manner similar to that introduced in Git version 2.46.0. The git-credential-lfstest program acts as a Git credential helper, and therefore accepts "get" and "store" command-line arguments along with input data as described in the Git documentation: https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/gitcredentials.txt#L261-L286 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/git-credential.txt#L103-L267 The Serialize() method of our new "credential" structure now encapsulates the logic that determines which credential fields to output in response to a command invocation with the "get" argument. Before commit 8e6641b, this logic was implemented in the fill() function, which handles "get" requests. When handling "get" requests for which the "authtype" credential field was not supplied (and no "authtype" capability was announced by the caller), and the credential record file read by the helper program does not indicate a response should be skipped, the current logic of the Serialize() method only outputs "username" and "password" fields if they were both found in the record file and are non-empty strings. However, the "clone ClientCert" test in our t/t-clone.sh script depends on credential record files we create in the directory identified by the CREDSDIR variable and which have empty values for the "username" field. These are used to supply the password for the encrypted certificate key file generated by our gitserver-lfstest utility program. In particular, when this test is run to completion, it executes a "git lfs clone" command using the URL on which our gitserver-lfstest program requires a client TLS/SSL certificate. We use the http.<url>.ssl{Cert,Key} configuration options to specify the client certificate files to Git. To decrypt the certificate, Git makes a request to the configured credential helper, sending empty "host" and "username" fields, a "path" field with the path to the certificate file, and a "protocol" field with the "cert" scheme. It expects our git-credential-lfstest helper to then return a "password" field with the passphrase for the encrypted client certificate file. Because the Serialize() method in our helper program now only returns "username" and "password" fields if they are both non-empty, it returns only the "host", "protocol", and "path" fields, without a "password" field. This prevents the test from succeeding, as Git then requests a password from the terminal since no helper provided one. Note, though, that due to the issue described in git-lfs#5658, this test does not actually run to completion at the moment, so we don't experience this problem in our CI jobs. We will address this issue in a subsequent commit in this PR, but as a first step, we need to at least restore the behaviour of our git-credential-lfstest helper program in the case where a credential record's file contains an empty "username" field and a non-empty "password" field. The original version of this code was introduced in 2015 in commit 5914d7d of PR git-lfs#306; it simply returned a "username" field if the input fields contained a "username" field, and the same for the "password" field. Prior to commit 8e6641b of PR git-lfs#5803 the helper program had evolved to handle some credential record files with formats other than simple "username" and "password" fields, but when returning those fields, continued to function like the original code in that it would return each field if there was a matching field among the input key/value pairs. Neither this approach, nor the current one, actually reflects how Git's own basic git-credential-store helper functions, nor others such as the git-credential-osxkeychain helper program. The git-credential-store program uses Git's credential_match() function to determine whether to return "username" and "password" fields for a given request, passing a zero (false) value for the match_password flag. For each of the "protocol", "host", "path", and "username" fields (but not the "password" field, because match_password is false) the credential_match() function checks if the field was provided in the request, and if so, that it matches the corresponding field in the credential record under consideration, assuming that field is also defined. Note that these fields may be defined but empty (i.e., be a pointer to zero-length string). If all the possible matches are met, then the function returns a non-zero (true) value. https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/builtin/credential-store.c#L34 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/credential.c#L87-L98 The git-credential-osxkeychain helper program is similar in that if a "username" field is provided, it must match the stored record before anything is returned, even if both are empty strings. But if a "username" field is not provided, both it and the "password" field will be returned, assuming the other required fields like "host" and "protocol" match. To make our git-credential-lfstest helper more accurately reflect the behaviour of these real credential helpers, and to resolve the problem with our "clone ClientCert" test and Git's inability to retrieve the passphrase for our encrypted test TLS/SSL client certificate, we therefore revise the Serialize() method of our "credential" Go structure so that before returning a "password" field, it checks whether a "username" field was provided, and if it was, that it matches the "username" field of the credential record (even if both are empty). As well, if a "username" field was not provided, then the one in the credential record is returned along with the "password" field. Further, we do not require that either the "username" or "password" fields in the credential record be non-empty. This brings our test credential helper program into closer alignment with the actions of Git's own helpers, and again allows a request for a credential with an empty "username" field in the input to match a credential record with an empty "username" field. One consequence of these changes on their own is that we might now return empty "username" and "password" fields even when the credential record file does not specify either, but had a leading non-empty field and so is a record we parse as one with fields for "authtype" and "credential" data. For example, if the credential record file contains the line "foo::bar", then we understand it has having an "authtype" field with the value "foo" and a "credential" field with the value of "bar". If a request is made which lacks "authtype" and "capability[]" fields, but whose "host" field (and possibly also "path" field) map to this credential record file, we will now return "host", "username", and "password" fields, with the latter having two empty values, and nothing else. This is because the Serialize() method will only return "authtype" and "credential" fields if the request contains an "authtype" field, and a "capability[]" field with the value "authtype", and if the credential record also contains non-empty "authtype" and "credential" fields. When these conditions are not met, the method falls through to the conditional governing whether "username" and "password" fields are output, and with our new conditions for handling those, we now allow for a missing "username" input field. Hence, without additional changes, we would output empty "username" and "password" fields in such a case as described above. To be clear, this is not something which occurs with our test suite as it stands now. Neverthless, we should ensure that we only output "username" and "password" fields if we have parsed the credential record file as having them. Therefore we add a check to the final conditional in the Serialze() method's logic, to ensure that we require an empty "authtype" field (i.e, a record such as ":foo:bar", with a blank leading field) before we will output "username" and "password" fields. In other words, we must have parsed the credential record file as one containing "username" and "password" field data before we will send those fields back to a caller.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Sep 6, 2024
In commit 8e6641b of PR git-lfs#5803 we refactored our git-credential-lfstest test utility program so as to use a new internal "credential" structure to store the details of a given credential, as part of that PR's changes to support multi-stage authentication in a manner similar to that introduced in Git version 2.46.0. The git-credential-lfstest program acts as a Git credential helper, and therefore accepts "get" and "store" command-line arguments along with input data as described in the Git documentation: https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/gitcredentials.txt#L261-L286 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/git-credential.txt#L103-L267 The Serialize() method of our new "credential" structure now encapsulates the logic that determines which credential fields to output in response to a command invocation with the "get" argument. Before commit 8e6641b, this logic was implemented in the fill() function, which handles "get" requests. When handling "get" requests for which the "authtype" credential field was not supplied (and no "authtype" capability was announced by the caller), and the credential record file read by the helper program does not indicate a response should be skipped, the current logic of the Serialize() method only outputs "username" and "password" fields if they were both found in the record file and are non-empty strings. However, the "clone ClientCert" test in our t/t-clone.sh script depends on credential record files we create in the directory identified by the CREDSDIR variable and which have empty values for the "username" field. These are used to supply the password for the encrypted certificate key file generated by our gitserver-lfstest utility program. In particular, when this test is run to completion, it executes a "git lfs clone" command using the URL on which our gitserver-lfstest program requires a client TLS/SSL certificate. We use the http.<url>.ssl{Cert,Key} configuration options to specify the client certificate files to Git. To decrypt the certificate, Git makes a request to the configured credential helper, sending empty "host" and "username" fields, a "path" field with the path to the certificate file, and a "protocol" field with the "cert" scheme. It expects our git-credential-lfstest helper to then return a "password" field with the passphrase for the encrypted client certificate file. Because the Serialize() method in our helper program now only returns "username" and "password" fields if they are both non-empty, it returns only the "host", "protocol", and "path" fields, without a "password" field. This prevents the test from succeeding, as Git then requests a password from the terminal since no helper provided one. Note, though, that due to the issue described in git-lfs#5658, this test does not actually run to completion at the moment, so we don't experience this problem in our CI jobs. We will address this issue in a subsequent commit in this PR, but as a first step, we need to at least restore the behaviour of our git-credential-lfstest helper program in the case where a credential record's file contains an empty "username" field and a non-empty "password" field. The original version of this code was introduced in 2015 in commit 5914d7d of PR git-lfs#306; it simply returned a "username" field if the input fields contained a "username" field, and the same for the "password" field. Prior to commit 8e6641b of PR git-lfs#5803 the helper program had evolved to handle some credential record files with formats other than simple "username" and "password" fields, but when returning those fields, continued to function like the original code in that it would return each field if there was a matching field among the input key/value pairs. Neither this approach, nor the current one, actually reflects how Git's own basic git-credential-store helper functions, nor others such as the git-credential-osxkeychain helper program. The git-credential-store program uses Git's credential_match() function to determine whether to return "username" and "password" fields for a given request, passing a zero (false) value for the match_password flag. For each of the "protocol", "host", "path", and "username" fields (but not the "password" field, because match_password is false) the credential_match() function checks if the field was provided in the request, and if so, that it matches the corresponding field in the credential record under consideration, assuming that field is also defined. Note that these fields may be defined but empty (i.e., be a pointer to zero-length string). If all the possible matches are met, then the function returns a non-zero (true) value. https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/builtin/credential-store.c#L34 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/credential.c#L87-L98 The git-credential-osxkeychain helper program is similar in that if a "username" field is provided, it must match the stored record before anything is returned, even if both are empty strings. But if a "username" field is not provided, both it and the "password" field will be returned, assuming the other required fields like "host" and "protocol" match. To make our git-credential-lfstest helper more accurately reflect the behaviour of these real credential helpers, and to resolve the problem with our "clone ClientCert" test and Git's inability to retrieve the passphrase for our encrypted test TLS/SSL client certificate, we therefore revise the Serialize() method of our "credential" Go structure so that before returning a "password" field, it checks whether a "username" field was provided, and if it was, that it matches the "username" field of the credential record (even if both are empty). As well, if a "username" field was not provided, then the one in the credential record is returned along with the "password" field. Further, we do not require that either the "username" or "password" fields in the credential record be non-empty. This brings our test credential helper program into closer alignment with the actions of Git's own helpers, and again allows a request for a credential with an empty "username" field in the input to match a credential record with an empty "username" field. One consequence of these changes on their own is that we might now return empty "username" and "password" fields even when the credential record file does not specify either, but had a leading non-empty field and so is a record we parse as one with fields for "authtype" and "credential" data. For example, if the credential record file contains the line "foo::bar", then we understand it has having an "authtype" field with the value "foo" and a "credential" field with the value of "bar". If a request is made which lacks "authtype" and "capability[]" fields, but whose "host" field (and possibly also "path" field) map to this credential record file, we will now return "host", "username", and "password" fields, with the latter having two empty values, and nothing else. This is because the Serialize() method will only return "authtype" and "credential" fields if the request contains an "authtype" field, and a "capability[]" field with the value "authtype", and if the credential record also contains non-empty "authtype" and "credential" fields. When these conditions are not met, the method falls through to the conditional governing whether "username" and "password" fields are output, and with our new conditions for handling those, we now allow for a missing "username" input field. Hence, without additional changes, we would output empty "username" and "password" fields in such a case as described above. To be clear, this is not something which occurs with our test suite as it stands now. Neverthless, we should ensure that we only output "username" and "password" fields if we have parsed the credential record file as having them. Therefore we add a check to the final conditional in the Serialze() method's logic, to ensure that we require an empty "authtype" field (i.e, a record such as ":foo:bar", with a blank leading field) before we will output "username" and "password" fields. In other words, we must have parsed the credential record file as one containing "username" and "password" field data before we will send those fields back to a caller.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Sep 8, 2024
In commit 8e6641b of PR git-lfs#5803 we refactored our git-credential-lfstest test utility program so as to use a new internal "credential" structure to store the details of a given credential, as part of that PR's changes to support multi-stage authentication in a manner similar to that introduced in Git version 2.46.0. The git-credential-lfstest program acts as a Git credential helper, and therefore accepts "get" and "store" command-line arguments along with input data as described in the Git documentation: https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/gitcredentials.txt#L261-L286 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/git-credential.txt#L103-L267 The Serialize() method of our new "credential" structure now encapsulates the logic that determines which credential fields to output in response to a command invocation with the "get" argument. Before commit 8e6641b, this logic was implemented in the fill() function, which handles "get" requests. When handling "get" requests for which the "authtype" credential field was not supplied (and no "authtype" capability was announced by the caller), and the credential record file read by the helper program does not indicate a response should be skipped, the current logic of the Serialize() method only outputs "username" and "password" fields if they were both found in the record file and are non-empty strings. However, the "clone ClientCert" test in our t/t-clone.sh script depends on credential record files we create in the directory identified by the CREDSDIR variable and which have empty values for the "username" field. These are used to supply the password for the encrypted certificate key file generated by our gitserver-lfstest utility program. In particular, when this test is run to completion, it executes a "git lfs clone" command using the URL on which our gitserver-lfstest program requires a client TLS/SSL certificate. We use the http.<url>.ssl{Cert,Key} configuration options to specify the client certificate files to Git. To decrypt the certificate, Git makes a request to the configured credential helper, sending empty "host" and "username" fields, a "path" field with the path to the certificate file, and a "protocol" field with the "cert" scheme. It expects our git-credential-lfstest helper to then return a "password" field with the passphrase for the encrypted client certificate file. Because the Serialize() method in our helper program now only returns "username" and "password" fields if they are both non-empty, it returns only the "host", "protocol", and "path" fields, without a "password" field. This prevents the test from succeeding, as Git then requests a password from the terminal since no helper provided one. Note, though, that due to the issue described in git-lfs#5658, this test does not actually run to completion at the moment, so we don't experience this problem in our CI jobs. We will address this issue in a subsequent commit in this PR, but as a first step, we need to at least restore the behaviour of our git-credential-lfstest helper program in the case where a credential record's file contains an empty "username" field and a non-empty "password" field. The original version of this code was introduced in 2015 in commit 5914d7d of PR git-lfs#306; it simply returned a "username" field if the input fields contained a "username" field, and the same for the "password" field. Prior to commit 8e6641b of PR git-lfs#5803 the helper program had evolved to handle some credential record files with formats other than simple "username" and "password" fields, but when returning those fields, continued to function like the original code in that it would return each field if there was a matching field among the input key/value pairs. Neither this approach, nor the current one, actually reflects how Git's own basic git-credential-store helper functions, nor others such as the git-credential-osxkeychain helper program. The git-credential-store program uses Git's credential_match() function to determine whether to return "username" and "password" fields for a given request, passing a zero (false) value for the match_password flag. For each of the "protocol", "host", "path", and "username" fields (but not the "password" field, because match_password is false) the credential_match() function checks if the field was provided in the request, and if so, that it matches the corresponding field in the credential record under consideration, assuming that field is also defined. Note that these fields may be defined but empty (i.e., be a pointer to zero-length string). If all the possible matches are met, then the function returns a non-zero (true) value. https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/builtin/credential-store.c#L34 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/credential.c#L87-L98 The git-credential-osxkeychain helper program is similar in that if a "username" field is provided, it must match the stored record before anything is returned, even if both are empty strings. But if a "username" field is not provided, both it and the "password" field will be returned, assuming the other required fields like "host" and "protocol" match. To make our git-credential-lfstest helper more accurately reflect the behaviour of these real credential helpers, and to resolve the problem with our "clone ClientCert" test and Git's inability to retrieve the passphrase for our encrypted test TLS/SSL client certificate, we therefore revise the Serialize() method of our "credential" Go structure so that before returning a "password" field, it checks whether a "username" field was provided, and if it was, that it matches the "username" field of the credential record (even if both are empty). As well, if a "username" field was not provided, then the one in the credential record is returned along with the "password" field. Further, we do not require that either the "username" or "password" fields in the credential record be non-empty. This brings our test credential helper program into closer alignment with the actions of Git's own helpers, and again allows a request for a credential with an empty "username" field in the input to match a credential record with an empty "username" field. One consequence of these changes on their own is that we might now return empty "username" and "password" fields even when the credential record file does not specify either, but had a leading non-empty field and so is a record we parse as one with fields for "authtype" and "credential" data. For example, if the credential record file contains the line "foo::bar", then we understand it has having an "authtype" field with the value "foo" and a "credential" field with the value of "bar". If a request is made which lacks "authtype" and "capability[]" fields, but whose "host" field (and possibly also "path" field) map to this credential record file, we will now return "host", "username", and "password" fields, with the latter having two empty values, and nothing else. This is because the Serialize() method will only return "authtype" and "credential" fields if the request contains an "authtype" field, and a "capability[]" field with the value "authtype", and if the credential record also contains non-empty "authtype" and "credential" fields. When these conditions are not met, the method falls through to the conditional governing whether "username" and "password" fields are output, and with our new conditions for handling those, we now allow for a missing "username" input field. Hence, without additional changes, we would output empty "username" and "password" fields in such a case as described above. To be clear, this is not something which occurs with our test suite as it stands now. Neverthless, we should ensure that we only output "username" and "password" fields if we have parsed the credential record file as having them. Therefore we add a check to the final conditional in the Serialze() method's logic, to ensure that we require an empty "authtype" field (i.e, a record such as ":foo:bar", with a blank leading field) before we will output "username" and "password" fields. In other words, we must have parsed the credential record file as one containing "username" and "password" field data before we will send those fields back to a caller.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Sep 15, 2024
In commit 8e6641b of PR git-lfs#5803 we refactored our git-credential-lfstest test utility program so as to use a new internal "credential" structure to store the details of a given credential, as part of that PR's changes to support multi-stage authentication in a manner similar to that introduced in Git version 2.46.0. The git-credential-lfstest program acts as a Git credential helper, and therefore accepts "get" and "store" command-line arguments along with input data as described in the Git documentation: https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/gitcredentials.txt#L261-L286 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/git-credential.txt#L103-L267 The Serialize() method of our new "credential" structure now encapsulates the logic that determines which credential fields to output in response to a command invocation with the "get" argument. Before commit 8e6641b, this logic was implemented in the fill() function, which handles "get" requests. When handling "get" requests for which the "authtype" credential field was not supplied (and no "authtype" capability was announced by the caller), and the credential record file read by the helper program does not indicate a response should be skipped, the current logic of the Serialize() method only outputs "username" and "password" fields if they were both found in the record file and are non-empty strings. However, the "clone ClientCert" test in our t/t-clone.sh script depends on credential record files we create in the directory identified by the CREDSDIR variable and which have empty values for the "username" field. These are used to supply the password for the encrypted certificate key file generated by our gitserver-lfstest utility program. In particular, when this test is run to completion, it executes a "git lfs clone" command using the URL on which our gitserver-lfstest program requires a client TLS/SSL certificate. We use the http.<url>.ssl{Cert,Key} configuration options to specify the client certificate files to Git. To decrypt the certificate, Git makes a request to the configured credential helper, sending empty "host" and "username" fields, a "path" field with the path to the certificate file, and a "protocol" field with the "cert" scheme. It expects our git-credential-lfstest helper to then return a "password" field with the passphrase for the encrypted client certificate file. Because the Serialize() method in our helper program now only returns "username" and "password" fields if they are both non-empty, it returns only the "host", "protocol", and "path" fields, without a "password" field. This prevents the test from succeeding, as Git then requests a password from the terminal since no helper provided one. Note, though, that due to the issue described in git-lfs#5658, this test does not actually run to completion at the moment, so we don't experience this problem in our CI jobs. We will address this issue in a subsequent commit in this PR, but as a first step, we need to at least restore the behaviour of our git-credential-lfstest helper program in the case where a credential record's file contains an empty "username" field and a non-empty "password" field. The original version of this code was introduced in 2015 in commit 5914d7d of PR git-lfs#306; it simply returned a "username" field if the input fields contained a "username" field, and the same for the "password" field. Prior to commit 8e6641b of PR git-lfs#5803 the helper program had evolved to handle some credential record files with formats other than simple "username" and "password" fields, but when returning those fields, continued to function like the original code in that it would return each field if there was a matching field among the input key/value pairs. Neither this approach, nor the current one, actually reflects how Git's own basic git-credential-store helper functions, nor others such as the git-credential-osxkeychain helper program. The git-credential-store program uses Git's credential_match() function to determine whether to return "username" and "password" fields for a given request, passing a zero (false) value for the match_password flag. For each of the "protocol", "host", "path", and "username" fields (but not the "password" field, because match_password is false) the credential_match() function checks if the field was provided in the request, and if so, that it matches the corresponding field in the credential record under consideration, assuming that field is also defined. Note that these fields may be defined but empty (i.e., be a pointer to zero-length string). If all the possible matches are met, then the function returns a non-zero (true) value. https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/builtin/credential-store.c#L34 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/credential.c#L87-L98 The git-credential-osxkeychain helper program is similar in that if a "username" field is provided, it must match the stored record before anything is returned, even if both are empty strings. But if a "username" field is not provided, both it and the "password" field will be returned, assuming the other required fields like "host" and "protocol" match. To make our git-credential-lfstest helper more accurately reflect the behaviour of these real credential helpers, and to resolve the problem with our "clone ClientCert" test and Git's inability to retrieve the passphrase for our encrypted test TLS/SSL client certificate, we therefore revise the Serialize() method of our "credential" Go structure so that before returning a "password" field, it checks whether a "username" field was provided, and if it was, that it matches the "username" field of the credential record (even if both are empty). As well, if a "username" field was not provided, then the one in the credential record is returned along with the "password" field. Further, we do not require that either the "username" or "password" fields in the credential record be non-empty. This brings our test credential helper program into closer alignment with the actions of Git's own helpers, and again allows a request for a credential with an empty "username" field in the input to match a credential record with an empty "username" field. One consequence of these changes on their own is that we might now return empty "username" and "password" fields even when the credential record file does not specify either, but had a leading non-empty field and so is a record we parse as one with fields for "authtype" and "credential" data. For example, if the credential record file contains the line "foo::bar", then we understand it has having an "authtype" field with the value "foo" and a "credential" field with the value of "bar". If a request is made which lacks "authtype" and "capability[]" fields, but whose "host" field (and possibly also "path" field) map to this credential record file, we will now return "host", "username", and "password" fields, with the latter having two empty values, and nothing else. This is because the Serialize() method will only return "authtype" and "credential" fields if the request contains an "authtype" field, and a "capability[]" field with the value "authtype", and if the credential record also contains non-empty "authtype" and "credential" fields. When these conditions are not met, the method falls through to the conditional governing whether "username" and "password" fields are output, and with our new conditions for handling those, we now allow for a missing "username" input field. Hence, without additional changes, we would output empty "username" and "password" fields in such a case as described above. To be clear, this is not something which occurs with our test suite as it stands now. Neverthless, we should ensure that we only output "username" and "password" fields if we have parsed the credential record file as having them. Therefore we add a check to the final conditional in the Serialze() method's logic, to ensure that we require an empty "authtype" field (i.e, a record such as ":foo:bar", with a blank leading field) before we will output "username" and "password" fields. In other words, we must have parsed the credential record file as one containing "username" and "password" field data before we will send those fields back to a caller.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Oct 7, 2024
In commit 8e6641b of PR git-lfs#5803 we refactored our git-credential-lfstest test utility program so as to use a new internal "credential" structure to store the details of a given credential, as part of that PR's changes to support multi-stage authentication in a manner similar to that introduced in Git version 2.46.0. The git-credential-lfstest program acts as a Git credential helper, and therefore accepts "get" and "store" command-line arguments along with input data as described in the Git documentation: https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/gitcredentials.txt#L261-L286 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/git-credential.txt#L103-L267 The Serialize() method of our new "credential" structure now encapsulates the logic that determines which credential fields to output in response to a command invocation with the "get" argument. Before commit 8e6641b, this logic was implemented in the fill() function, which handles "get" requests. When handling "get" requests for which the "authtype" credential field was not supplied (and no "authtype" capability was announced by the caller), and the credential record file read by the helper program does not indicate a response should be skipped, the current logic of the Serialize() method only outputs "username" and "password" fields if they were both found in the record file and are non-empty strings. However, the "clone ClientCert" test in our t/t-clone.sh script depends on credential record files we create in the directory identified by the CREDSDIR variable and which have empty values for the "username" field. These are used to supply the password for the encrypted certificate key file generated by our gitserver-lfstest utility program. In particular, when this test is run to completion, it executes a "git lfs clone" command using the URL on which our gitserver-lfstest program requires a client TLS/SSL certificate. We use the http.<url>.ssl{Cert,Key} configuration options to specify the client certificate files to Git. To decrypt the certificate, Git makes a request to the configured credential helper, sending empty "host" and "username" fields, a "path" field with the path to the certificate file, and a "protocol" field with the "cert" scheme. It expects our git-credential-lfstest helper to then return a "password" field with the passphrase for the encrypted client certificate file. Because the Serialize() method in our helper program now only returns "username" and "password" fields if they are both non-empty, it returns only the "host", "protocol", and "path" fields, without a "password" field. This prevents the test from succeeding, as Git then requests a password from the terminal since no helper provided one. Note, though, that due to the issue described in git-lfs#5658, this test does not actually run to completion at the moment, so we don't experience this problem in our CI jobs. We will address this issue in a subsequent commit in this PR, but as a first step, we need to at least restore the behaviour of our git-credential-lfstest helper program in the case where a credential record's file contains an empty "username" field and a non-empty "password" field. The original version of this code was introduced in 2015 in commit 5914d7d of PR git-lfs#306; it simply returned a "username" field if the input fields contained a "username" field, and the same for the "password" field. Prior to commit 8e6641b of PR git-lfs#5803 the helper program had evolved to handle some credential record files with formats other than simple "username" and "password" fields, but when returning those fields, continued to function like the original code in that it would return each field if there was a matching field among the input key/value pairs. Neither this approach, nor the current one, actually reflects how Git's own basic git-credential-store helper functions, nor others such as the git-credential-osxkeychain helper program. The git-credential-store program uses Git's credential_match() function to determine whether to return "username" and "password" fields for a given request, passing a zero (false) value for the match_password flag. For each of the "protocol", "host", "path", and "username" fields (but not the "password" field, because match_password is false) the credential_match() function checks if the field was provided in the request, and if so, that it matches the corresponding field in the credential record under consideration, assuming that field is also defined. Note that these fields may be defined but empty (i.e., be a pointer to zero-length string). If all the possible matches are met, then the function returns a non-zero (true) value. https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/builtin/credential-store.c#L34 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/credential.c#L87-L98 The git-credential-osxkeychain helper program is similar in that if a "username" field is provided, it must match the stored record before anything is returned, even if both are empty strings. But if a "username" field is not provided, both it and the "password" field will be returned, assuming the other required fields like "host" and "protocol" match. To make our git-credential-lfstest helper more accurately reflect the behaviour of these real credential helpers, and to resolve the problem with our "clone ClientCert" test and Git's inability to retrieve the passphrase for our encrypted test TLS/SSL client certificate, we therefore revise the Serialize() method of our "credential" Go structure so that before returning a "password" field, it checks whether a "username" field was provided, and if it was, that it matches the "username" field of the credential record (even if both are empty). As well, if a "username" field was not provided, then the one in the credential record is returned along with the "password" field. Further, we do not require that either the "username" or "password" fields in the credential record be non-empty. This brings our test credential helper program into closer alignment with the actions of Git's own helpers, and again allows a request for a credential with an empty "username" field in the input to match a credential record with an empty "username" field. One consequence of these changes on their own is that we might now return empty "username" and "password" fields even when the credential record file does not specify either, but had a leading non-empty field and so is a record we parse as one with fields for "authtype" and "credential" data. For example, if the credential record file contains the line "foo::bar", then we understand it has having an "authtype" field with the value "foo" and a "credential" field with the value of "bar". If a request is made which lacks "authtype" and "capability[]" fields, but whose "host" field (and possibly also "path" field) map to this credential record file, we will now return "host", "username", and "password" fields, with the latter having two empty values, and nothing else. This is because the Serialize() method will only return "authtype" and "credential" fields if the request contains an "authtype" field, and a "capability[]" field with the value "authtype", and if the credential record also contains non-empty "authtype" and "credential" fields. When these conditions are not met, the method falls through to the conditional governing whether "username" and "password" fields are output, and with our new conditions for handling those, we now allow for a missing "username" input field. Hence, without additional changes, we would output empty "username" and "password" fields in such a case as described above. To be clear, this is not something which occurs with our test suite as it stands now. Neverthless, we should ensure that we only output "username" and "password" fields if we have parsed the credential record file as having them. Therefore we add a check to the final conditional in the Serialze() method's logic, to ensure that we require an empty "authtype" field (i.e, a record such as ":foo:bar", with a blank leading field) before we will output "username" and "password" fields. In other words, we must have parsed the credential record file as one containing "username" and "password" field data before we will send those fields back to a caller.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Oct 9, 2024
In commit 8e6641b of PR git-lfs#5803 we refactored our git-credential-lfstest test utility program so as to use a new internal "credential" structure to store the details of a given credential, as part of that PR's changes to support multi-stage authentication in a manner similar to that introduced in Git version 2.46.0. The git-credential-lfstest program acts as a Git credential helper, and therefore accepts "get" and "store" command-line arguments along with input data as described in the Git documentation: https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/gitcredentials.txt#L261-L286 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/Documentation/git-credential.txt#L103-L267 The Serialize() method of our new "credential" structure now encapsulates the logic that determines which credential fields to output in response to a command invocation with the "get" argument. Before commit 8e6641b, this logic was implemented in the fill() function, which handles "get" requests. When handling "get" requests for which the "authtype" credential field was not supplied (and no "authtype" capability was announced by the caller), and the credential record file read by the helper program does not indicate a response should be skipped, the current logic of the Serialize() method only outputs "username" and "password" fields if they were both found in the record file and are non-empty strings. However, the "clone ClientCert" test in our t/t-clone.sh script depends on credential record files we create in the directory identified by the CREDSDIR variable and which have empty values for the "username" field. These are used to supply the password for the encrypted certificate key file generated by our gitserver-lfstest utility program. In particular, when this test is run to completion, it executes a "git lfs clone" command using the URL on which our gitserver-lfstest program requires a client TLS/SSL certificate. We use the http.<url>.ssl{Cert,Key} configuration options to specify the client certificate files to Git. To decrypt the certificate, Git makes a request to the configured credential helper, sending empty "host" and "username" fields, a "path" field with the path to the certificate file, and a "protocol" field with the "cert" scheme. It expects our git-credential-lfstest helper to then return a "password" field with the passphrase for the encrypted client certificate file. Because the Serialize() method in our helper program now only returns "username" and "password" fields if they are both non-empty, it returns only the "host", "protocol", and "path" fields, without a "password" field. This prevents the test from succeeding, as Git then requests a password from the terminal since no helper provided one. Note, though, that due to the issue described in git-lfs#5658, this test does not actually run to completion at the moment, so we don't experience this problem in our CI jobs. We will address this issue in a subsequent commit in this PR, but as a first step, we need to at least restore the behaviour of our git-credential-lfstest helper program in the case where a credential record's file contains an empty "username" field and a non-empty "password" field. The original version of this code was introduced in 2015 in commit 5914d7d of PR git-lfs#306; it simply returned a "username" field if the input fields contained a "username" field, and the same for the "password" field. Prior to commit 8e6641b of PR git-lfs#5803 the helper program had evolved to handle some credential record files with formats other than simple "username" and "password" fields, but when returning those fields, continued to function like the original code in that it would return each field if there was a matching field among the input key/value pairs. Neither this approach, nor the current one, actually reflects how Git's own basic git-credential-store helper functions, nor others such as the git-credential-osxkeychain helper program. The git-credential-store program uses Git's credential_match() function to determine whether to return "username" and "password" fields for a given request, passing a zero (false) value for the match_password flag. For each of the "protocol", "host", "path", and "username" fields (but not the "password" field, because match_password is false) the credential_match() function checks if the field was provided in the request, and if so, that it matches the corresponding field in the credential record under consideration, assuming that field is also defined. Note that these fields may be defined but empty (i.e., be a pointer to zero-length string). If all the possible matches are met, then the function returns a non-zero (true) value. https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/builtin/credential-store.c#L34 https://github.com/git/git/blob/4590f2e9412378c61eac95966709c78766d326ba/credential.c#L87-L98 The git-credential-osxkeychain helper program is similar in that if a "username" field is provided, it must match the stored record before anything is returned, even if both are empty strings. But if a "username" field is not provided, both it and the "password" field will be returned, assuming the other required fields like "host" and "protocol" match. To make our git-credential-lfstest helper more accurately reflect the behaviour of these real credential helpers, and to resolve the problem with our "clone ClientCert" test and Git's inability to retrieve the passphrase for our encrypted test TLS/SSL client certificate, we therefore revise the Serialize() method of our "credential" Go structure so that before returning a "password" field, it checks whether a "username" field was provided, and if it was, that it matches the "username" field of the credential record (even if both are empty). As well, if a "username" field was not provided, then the one in the credential record is returned along with the "password" field. Further, we do not require that either the "username" or "password" fields in the credential record be non-empty. This brings our test credential helper program into closer alignment with the actions of Git's own helpers, and again allows a request for a credential with an empty "username" field in the input to match a credential record with an empty "username" field. One consequence of these changes on their own is that we might now return empty "username" and "password" fields even when the credential record file does not specify either, but had a leading non-empty field and so is a record we parse as one with fields for "authtype" and "credential" data. For example, if the credential record file contains the line "foo::bar", then we understand it has having an "authtype" field with the value "foo" and a "credential" field with the value of "bar". If a request is made which lacks "authtype" and "capability[]" fields, but whose "host" field (and possibly also "path" field) map to this credential record file, we will now return "host", "username", and "password" fields, with the latter two having empty values, and nothing else. This is because the Serialize() method will only return "authtype" and "credential" fields if the request contains an "authtype" field, and a "capability[]" field with the value "authtype", and if the credential record also contains non-empty "authtype" and "credential" fields. When these conditions are not met, the method falls through to the conditional governing whether "username" and "password" fields are output, and with our new conditions for handling those, we now allow for a missing "username" input field. Hence, without additional changes, we would output empty "username" and "password" fields in such a case as described above. To be clear, this is not something which occurs with our test suite as it stands now. Nevertheless, we should ensure that we only output "username" and "password" fields if we have parsed the credential record file as having them. Therefore we add a check to the final conditional in the Serialize() method's logic, to ensure that we require an empty "authtype" field (i.e, a record such as ":foo:bar", with a blank leading field) before we will output "username" and "password" fields. In other words, we must have parsed the credential record file as one containing "username" and "password" field data before we will send those fields back to a caller.
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.
chrisd8088
added a commit
to manturovDan/git-lfs
that referenced
this pull request
Mar 5, 2026
In prior commits in this PR we revised the DoWithAuth() method of the Client structure in our "lfsapi" package so that it should no longer enter an infinite loop if a Git LFS API implementation repeatedly responds to requests with a 401 Unauthorized status code. To help verify that these changes are effective, we now add a test to our "t/t-credentials.sh" shell test script. This test is similar to the existing "credentials can authenticate with multistage auth" test, which was introduced along with our support for multi-stage authentication schemes in PR git-lfs#5803. Both the existing test and our new test make use of the example "Multistage" authentication scheme used in our shell test suite, which also first appeared in PR git-lfs#5803. Our new test simulates a Git credential helper that is misconfigured or broken in such a way that after a certain stage, it begins to always return the same credentials and state. To establish this intentional misconfiguration we simply create a credential record file for our "git-credential-lfstest" utility that contains two entries, one to define the transition from the initial stage to the "state1" stage, and another to define a self-referential transition from the "state1" stage to the same "state1" stage. Note that the "git-credential-lfstest" utility searches for a matching entry in its credential record files in a sequential fashion, and that entries with an empty fourth field (the "MATCH" field) are considered to match against all states. As a result, we must place the entry for the transition from the initial stage to the "state1" stage at the end of the credential record file, or else it would always match the current stage. To help clarify this detail of our test's setup steps we include a comment which explains why the entry for the first transition must appear last in the file. We also take the opportunity to add similar comments to the two existing tests in the "t/t-credential.sh" script that create credential record files for multi-stage authentication, as they likewise place the entry for their first transitions at the end of these files. (As well, we slightly adjust the whitespace formatting of the existing "credentials can authenticate with multistage auth" test so that it more closely resembles our new "credentials with multistage auth loop fails" test, as well as that of another multi-stage authentication test we expect to introduce in a subsequent commit in this PR.) After our new test establishes its misconfigured credential record file, it performs a "git push" command and then performs the same set of checks as are found in the "credentials with useHttpPath, with wrong password and 401 response" test that we added in a prior commit in this PR, plus one additional check specific to multi-stage authentication. As we described in that previous commit, we expect the Git LFS client to make two sets of requests during the "git push" command, one set to the Locking API and another to the Batch API. The initial request to the Locking API is made without an Authorization header, and so no credentials are retrieved from the local configuration for this request. Because a 401 status code is received after the initial request, the client then retrieves the first stage's credential and state from the "git-credential-lfstest" helper and makes a second request, which also receives a 401 status code response. The client then retrieves the next stage's credential and state from the helper, but due to our intentional misconfiguration, these are the same as for the first state. Our "lfstest-gitserver" test utility's skipIfBadAuth() function, which determines whether to accept or reject the credentials presented in a request's Authorization header, handles our example "Multistage" authentication scheme by checking whether the credentials match either the value "cred1" or the value "cred2". The "cred2" credential value will result in the function returning "false" and accepting the credential, while the "cred1" value causes the function to return "true" and the utility to respond with a 401 status code. Since the client's second request to the Locking API also contains the "cred1" credential, it again receives a 401 status code, which due to the revisions we made to the DoWithAuth() method in prior commits in this PR should cause the client to exit from the method and report that the Locking API is not supported by the Git LFS endpoint. We then expect the client to make an initial request to the Batch API, but unlike the first request to the Locking API, an Authorization header will be included because the access mode required by the Git LFS endpoint has been cached by the requests to the Locking API. When this request receives a 401 status code response, it should then be retried two more times before the DoWithAuth() method reports an error and exits. This expected sequence of events explains why our new test checks that the "git push" command retrieved credentials five times rather than six, and why the test checks for five instances of the "cred1" credential in an Authorization header and not six. Note that we anticipate that we will further refine the behaviour of the DoWithAuth() method in a subsequent commit in this PR so that requests without an Authorization header are not counted towards the total number of attempts to request authentication.
chrisd8088
added a commit
to manturovDan/git-lfs
that referenced
this pull request
Mar 5, 2026
In prior commits in this PR we revised the DoWithAuth() method of the Client structure in our "lfsapi" package so that it should no longer enter an infinite loop if a Git LFS API implementation repeatedly responds to requests with a 401 Unauthorized status code. We have also added tests which demonstrate that our changes to the DoWithAuth() method are effective, including when a server only returns 401 status codes. However, a misbehaving server is not the only condition which might cause the DoWithAuth() method to perform the maximum allowed number of requests without successfully authenticating or being rejected. In PR git-lfs#5803 we introduced support for multi-stage authentication, in which a Git credential helper returns a series of credentials and states, and Git LFS client makes a request for each state using the supplied credentials. The final state is expected to complete the authentication sequence and either be accepted or rejected. We have already added tests in previous commits in this PR which simulate a broken or misconfigured credential helper that never returns the final state in a multi-stage authentication sequence, and instead keeps looping through the intermediate states. Both the new "credentials with multistage auth loop fails" test in our "t/t-credentials.sh" shell script and the new TestDoWithAuthMultistageRetryLimitExceeded() function in our Go test suite simulate a credential helper which never advances past an intermediate authentication state. These tests demonstrate that our changes to the DoWithAuth() method ensure it will return after making the maximum allowed number of requests, even if a multi-stage authentication sequence is not complete. This was the intended behaviour of the DoWithAuth() method and the other Client structure methods at the time when PR git-lfs#5803 was developed. In commit 6783078 of that PR we introduced a "count" variable to the DoWithAuth() method, which the method then passed to the doWithAuth() method as a pointer. The doWithAuth() method incremented the "count" variable after making a multi-stage authentication request, unless the maximum number of permitted requests had been made, in which case it was supposed to report that the request was rejected. The stated intent of this design was "to avoid a credential helper getting stuck in an infinite loop if it keeps handing back the same credentials", per the description in commit 6783078. Unfortunately, this implementation did not work as expected because the DoWithAuth() method called itself recusively if the current request received a 401 status code response, and because each invocation of the method would instantiate a new "count" variable with a zero value. Thus every time the doWithAuth() method was called it could only ever increase the variable's value to one, and so it would never find that the maximum number of allowed requests had been made. Technically, as we noted in a comment on this PR, a rare combination of HTTP redirections and 401 status code responses could result in the "count" variable being incremented past one. With a sufficient number of redirections the "count" variable could reach the maximum request limit and the doWithAuth() method would then inform the credential helper that the current credentials should be rejected. However, because the doWithAuth() method still returned an error indicating that the response to the most-recent request contained a 401 status code, the DoWithAuth() method would call itself recursively and the Git LFS client would enter an infinite loop: git-lfs#6018 (comment) The revisions we have made to the DoWithAuth() and doWithAuth() methods in this PR should resolve the problems described above and ensure that misconfigured multi-stage authentication sequences will not cause the Git LFS client to repeat the same requests without ceasing. Despite these changes, the client may still exhibit unintended and unexpected behaviour if a credential helper is configured with a valid multi-stage authentication sequence, but the number of stages in that sequence exceeds the maximum number of requests we allow during an authentication sequence. In such a case, if the client makes several distinct types of requests to the same Git LFS endpoint, then the client may improperly continue the incomplete authentication sequence across the different types of requests. In particular, various Git LFS commands start by making requests to the Locking API of the remote Git LFS endpoint, but Git LFS services are not required to implement that API. If these requests fail, the client just reports that the remote does not support the Locking API and then proceeds to perform the given command, which may require one or more requests to the Batch API of the endpoint. The client may therefore proceed through the initial stages of a long multi-stage authentication sequence, making requests to the Locking API of an endpoint, but reach the maximum number of requests we allow before being either accepted or rejected because the sequence is not yet complete. However, if the client then begins to make requests to the Batch API of the endpoint, it will continue where it left off in the sequence, and may be able to successfully authenticate. While that may appear to be beneficial, this behaviour was not our intention when we added multi-stage authentication support in PR git-lfs#5803. If we permit the client to continue multi-stage authentication across different types of requests, the client may behave inconsistently for users who have identical configurations except for the number of stages in their authentication sequences. We therefore update the DoWithAuth() method so that when it returns after making the maximum allowed number of requests without successfully authenticating or being rejected, it first resets the current state of any multi-stage authentication sequence so that any subsequent requests will restart the sequence from its first stage. To reset the current state we simply pass a "nil" parameter to the SetStateFields() method of the CredentialHelperContext structure from our "creds" package, which sets the structure's "state" field back to its post-initialization value of "nil". To confirm that our changes are effective we then add a new "credentials with multistage auth above limit fails and resets" test to our "t/t-credentials.sh" shell test script. This test establishes a four-stage authentication sequence by creating a credential record file for our "git-credential-lfstest" utility that contains four entries, each of which defines a transition from one stage of four to the next. For this four-stage authentication sequence to be fully supported by our "lfstest-gitserver" test utility, we have to modify its skipIfBadAuth() function, which determines whether to accept or reject the credentials presented in a request's Authorization header. Since commit a3429c3 of PR git-lfs#5803, the skipIfBadAuth() function handles our example "Multistage" authentication scheme by checking whether the credentials match either the value "cred1" or the value "cred2". The "cred2" credential value will result in the function returning "false" and accepting the credential, while the "cred1" value causes the function to return "true" and the utility to respond with a 401 status code. Several of the existing tests in our "t/t-credentials.sh" script depend on the "lfstest-gitserver" utility accepting the "cred2" value in an Authorization header to complete a multi-stage authentication sequence. As our new test requires the utility to not accept the first three of four credentials in a sequence, we alter the skipIfBadAuth() function so that it expects the values in an Authorization header with a "Multistage" scheme to match the pattern "cred<m>of<n>". If a "Multistage" scheme credential matches this pattern and the "<m>" and "<n>" values are equal, the skipIfBadAuth() function will return "false", indicating that the credentials are valid; otherwise, it will return "true" and cause the "lfstest-gitserver" utility to respond to the request with a 401 status code. We then update our existing tests in our "t/t-credentials.sh" script so that they create credential record files whose entries follow the revised pattern, e.g., "cred1of2" and "cred2of2". Our new "credentials with multistage auth above limit fails and resets" test, however, creates entries in its credential record file of the form "cred1of4", "cred2of4", etc. Because the defaultMaxAuthAttempts variable in the "lfsapi" package is assigned a value of three, we allow at most three requests during an authentication sequence. By defining a four-stage authentication sequence for our new test, we guarantee that each distinct type of request made by a Git LFS command should reach the three-request limit without successfully authenticating. Like the other tests we have introduced in previous commits in this PR, our new test establishes its credential record file and then runs a "git push" command. As we described in those commits, we expect the Git LFS client to make two sets of requests during the "git push" command, one set to the Locking API and another to the Batch API. Because the initial request to the Locking API is made without an Authorization header, we expect that no credentials will be retrieved from the local configuration for this request. The client should then make two more requests to the Locking API, both with credentials, before reaching the per-sequence limit of three requests. The client should then proceed to make three requests to the Batch API, all with credentials, before again reaching the per-sequence limit. This expected sequence of events explains why our new test checks that the "git push" command retrieved credentials five times rather than six, and why the test checks for two instances each of the "cred1of4" and "cred2of4" credentials in Authorization headers, but for only one instance of the "cred3of4" credential and none of the "cred4of4" credential. Note that we anticipate that we will further refine the behaviour of the DoWithAuth() method in a subsequent commit in this PR so that requests without an Authorization header are not counted towards the total number of attempts to request authentication. Further, note that our new test would fail without our change to the DoWithAuth() method in this commit, because the Git LFS client would successfully authenticate on its second request to the Batch API. The client would send the "cred3of4" credential on its first request to that API, and then send the "cred4of4" credential on its next request.
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.
In 2.46, Git will support multistage authentication from credential helpers. This will allow Git and Git LFS to implement functionality like NTLM and Kerberos in the credential helper, which lets this functionality to work even if Git LFS doesn't support it natively.
This requires two separate pieces of data. First, it involves a
state[]field, which each credential helper can add to keep track of state. Second, it allows the usage of a booleancontinuefield, which indicates that the response is multistage and this is not the final stage.In order to make this work, we adjust a few things. First, we advertise the
statecapability. Additionally, we save and pass back thestate[]fields that the credential helper may send to us. We also don't change the authentication scheme if the helper told us that this was a multistage response. Finally, we add a check to avoid a credential helper getting stuck in an infinite loop if it keeps handing back the same credentials.There are also a variety of preparatory steps which are necessary and are present in their own commits for easy review.