GetResponseWithRedirects only adds credentials to cache for matching schemes
#1
Conversation
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
…g schemes The previous implementation of `ManagedHttpSmartSubtransportStream.GetResponseWithRedirects` was not checking that the scheme matched the provided credentials before adding to the credential cache (instead defaulting to the scheme of the first value in the WWW-Authenticate header). That could cause issues in some cases where the first challenge contained an unsupported scheme (such as Bearer). In this case, the old implemenation would add the credentials to the cache with Bearer scheme - regardless of the credential type - and all additional (potentially valid) schemes in the header would be discarded. Credentials are now added to the cache for every supported scheme that matches the given credential type. The new implementation copies the one used in the libgit2 library: Default credentials are used for the Negotiate scheme, username/password credentials are used for the NTLM and Basic schemes, and all others are ignored. The error in the current implementation can be reproduced when attempting to connect to a repository in an Azure AD-backed Azure DevOps organisation.
dylanlerch
force-pushed
the
dylan/credential-schemes
branch
from
49112be to
0eb7b94
Compare
|
No longer necessary, master now uses the LibGit2 transport |
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.
The previous implementation of
ManagedHttpSmartSubtransportStream.GetResponseWithRedirectswas not checking that the scheme matched the provided credentials before adding to the credential cache (instead defaulting to the scheme of the first value in theWWW-Authenticateheader).That could cause issues in some cases where the first challenge contained an unsupported scheme (such as Bearer). In this case, the old implemenation would add the credentials to the cache with Bearer scheme - regardless of the credential type - and all additional (potentially valid) schemes in the header would be discarded.
Credentials are now added to the cache for every supported scheme that matches the given credential type. This new implementation copies the one used in the libgit2 library; default credentials are used for the Negotiate scheme, username/password credentials are used for the NTLM and Basic schemes, and all others are ignored.
The error in the current implementation can be reproduced when attempting to connect to a repository in an Azure AD-backed Azure DevOps organisation.