Skip to content
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

Credential handling in 0.27.0-preview-0096 fails against VSTS/Azure #1871

Open
curea opened this issue Feb 25, 2021 · 7 comments
Open

Credential handling in 0.27.0-preview-0096 fails against VSTS/Azure #1871

curea opened this issue Feb 25, 2021 · 7 comments

Comments

@curea
Copy link

curea commented Feb 25, 2021

Between versions preview-0034 and preview-0096 the credential handling changed and no longer works with VSTS/Azure

Include="LibGit2Sharp" Version="0.27.0-preview-0034" - works correctly

Include="LibGit2Sharp" Version="0.27.0-preview-0096" - authentication fails

Reproduction steps

string surl = "https://testsite.visualstudio.com/testsite/_git/Test";
string un = "pat";
string pwd = "";

var credentialsProvider = new CredentialsHandler((_url, _user, _cred) =>
new UsernamePasswordCredentials
{
Username = un,
Password = pwd
}
);

var references = Repository.ListRemoteReferences(surl, credentialsProvider);

Expected behavior

Should return references

Actual behavior

preview-0096 throws "this remote has never connected" error when authentication fails.

Version of LibGit2Sharp (release number or SHA1)

0.27.0-preview-0096

Operating system(s) tested; .NET runtime tested

OSx, .net core 5.0

@dylanlerch
Copy link

I've run in to this same issue connecting to Azure DevOps repositories. Looks like it's specifically for Azure AD backed organisations (if your Azure DevOps account was just created with a public Microsoft account you should be fine for this one).

Looks to be an edge case with the way that the ManagedHttpSmartSubtransportStream loads credentials in to the CredentialCache. The credentials are loaded in to the cache with the first scheme that is returned from WWW-Authenticate header. For Azure AD-backed Azure DevOps repositories this is Bearer, Basic credentials aren't going to work there, and 💥

List of WWW-Authenticate headers for HttpResponseMessage showing Bearer as scheme for first value, Basic as scheme for second value, and TFS-Federated as scheme for third value

For normal Azure DevOps accounts (created with a standard Microsoft account), Basic is the first one returned in that list so it all works as expected. This seems to be the case for most repositories.

List of WWW-Authenticate headers for HttpResponseMessage showing Basic as scheme for first value, Bearer as scheme for second value, and TFS-Federated as scheme for third value

It's a sneaky one. The libgit2 implementation looks to have a specific set of schemes that it supports, and ignores all challenges for unsupported schemes, so that seems like it could be a valid way forward for this implementation as well. 🙂

@curea
Copy link
Author

curea commented Aug 25, 2021

If you can point me to the code that handles this, I'd be happy to take a stab at a PR.

@X-TiMe
Copy link

X-TiMe commented Aug 31, 2021

If you can point me to the code that handles this, I'd be happy to take a stab at a PR.

@curea after converging to the same diagnostic here is the problematic line

var scheme = response.Headers.WwwAuthenticate.First().Scheme;

It gets the first entry but for some reasons the first one is not basic. So bearer is completed with basic informations, an gets discarded by native api call.

image

Hope it helps 😄

Best Regards

@dylanlerch
Copy link

I put together a fix in a fork that seemed to fix this specific issue for us: OctopusDeploy#1.

There may be issues with it, we're not currently using it in production anywhere, but could be a good starting point. :)

@bording
Copy link
Member

bording commented Sep 1, 2021

FYI there's a decent chance this will all be fixed once I pull in a libgit2 that has the new optional/dynamic OpenSSL dependency, which will let me go back to using the native HTTPS support and get rid of the managed implementation.

@X-TiMe
Copy link

X-TiMe commented Sep 1, 2021

Hello everyone,

@dylanlerch I've seen your code and it looks neat and corrects the issue. Can you please issue a PR to the main libgit2sharp repository ?

On my side i'm informing Cake.git team there is an issue with libgit2sharp with Azure/VSTS that will be corrected soon (I hope 😄 ).

Best regards

@X-TiMe
Copy link

X-TiMe commented Sep 27, 2021

FYI there's a decent chance this will all be fixed once I pull in a libgit2 that has the new optional/dynamic OpenSSL dependency, which will let me go back to using the native HTTPS support and get rid of the managed implementation.

Hello @bording, I just compiled your last commit from the 19/09 with your Native HTTPS integration and now IT WORKS in a Windows Machine.

But We've got another issue if it's using a Mac computer (Azure Pipelines to build Xamarin Mobile (iOS) applications use a Mac computer too). "Unable to load shared library 'git2-6777db8' or one of its dependencies" it seems related to issue #1914

Any comment to this topic will be greatly appreciated. If you need I can make some tests on my Mac for you if you don't have a Mac computer.

We had to create a nuget in our private repository to use it.

Do you expect to release a new Nuget in pre-release status at least ? (last official nuget package is in 2019)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants