Skip to content

Commit 49112be

Browse files
author
Dylan Lerch
committed
GetResponseWithRedirects only adds credentials to cache for matching 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.
1 parent 7c78387 commit 49112be

File tree

1 file changed

+26
-12
lines changed

1 file changed

+26
-12
lines changed

LibGit2Sharp/Core/ManagedHttpSmartSubtransport.cs

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ private class ManagedHttpSmartSubtransportStream : SmartSubtransportStream
4747
{
4848
private static int MAX_REDIRECTS = 7;
4949

50+
private static readonly string[] DEFAULT_CREDENTIAL_SUPPORTED_SCHEMES = { "Negotiate" };
51+
private static readonly string[] USERNAME_PASSWORD_CREDENTIAL_SUPPORTED_SCHEMES = { "NTLM", "Basic" };
52+
5053
#if NETCOREAPP
5154
private static readonly SocketsHttpHandler httpHandler;
5255
#else
@@ -189,20 +192,14 @@ private HttpResponseMessage GetResponseWithRedirects()
189192
throw new InvalidOperationException("authentication cancelled");
190193
}
191194

192-
var scheme = response.Headers.WwwAuthenticate.First().Scheme;
193-
194-
if (cred is DefaultCredentials)
195+
foreach (var authenticationHeader in response.Headers.WwwAuthenticate)
195196
{
196-
lock (credentialCache)
197+
if (CredentialsAreValidForScheme(cred, authenticationHeader.Scheme, out var networkCredential))
197198
{
198-
credentialCache.Add(url, scheme, CredentialCache.DefaultNetworkCredentials);
199-
}
200-
}
201-
else if (cred is UsernamePasswordCredentials userpass)
202-
{
203-
lock (credentialCache)
204-
{
205-
credentialCache.Add(url, scheme, new NetworkCredential(userpass.Username, userpass.Password));
199+
lock (credentialCache)
200+
{
201+
credentialCache.Add(url, authenticationHeader.Scheme, networkCredential);
202+
}
206203
}
207204
}
208205

@@ -221,6 +218,23 @@ private HttpResponseMessage GetResponseWithRedirects()
221218
throw new Exception("too many redirects or authentication replays");
222219
}
223220

221+
bool CredentialsAreValidForScheme(Credentials credentials, string scheme, out NetworkCredential networkCredential)
222+
{
223+
if (credentials is DefaultCredentials && DEFAULT_CREDENTIAL_SUPPORTED_SCHEMES.Contains(scheme, StringComparer.InvariantCultureIgnoreCase))
224+
{
225+
networkCredential = CredentialCache.DefaultNetworkCredentials;
226+
return true;
227+
}
228+
else if (credentials is UsernamePasswordCredentials userpass && USERNAME_PASSWORD_CREDENTIAL_SUPPORTED_SCHEMES.Contains(scheme, StringComparer.InvariantCultureIgnoreCase))
229+
{
230+
networkCredential = new NetworkCredential(userpass.Username, userpass.Password);
231+
return true;
232+
}
233+
234+
networkCredential = null;
235+
return false;
236+
}
237+
224238
public override int Read(Stream dataStream, long length, out long readTotal)
225239
{
226240
byte[] buffer = new byte[4096];

0 commit comments

Comments
 (0)