libgit2sharp icon indicating copy to clipboard operation
libgit2sharp copied to clipboard

Credentials being cached between requests to separate remotes with the same URL

Open dylanlerch opened this issue 4 years ago • 4 comments

The current implementation of ManagedHttpSmartSubtransportStream uses a static CredentialCache. The credentials in this cache are only differentiated by URL and authentication scheme, if you have two separate repositories configured with the same URL, it's possible to authenticate to a repository without providing valid credentials.

Reproduction steps

using System;
using System.Net;
using LibGit2Sharp;

namespace Example
{
    class Program
    {
        static void Main(string[] args)
        {
            var uri = "<repository URL>";

            var repo1 = Repository.Clone(uri, "directory1", new CloneOptions
            {
                BranchName = "main",
                CredentialsProvider = CredentialsHandlerThatReturnsValidCredentials
            }); // This will succeed

            var repo2 = Repository.Clone(uri, "directory2", new CloneOptions
            {
                BranchName = "main",
                CredentialsProvider = CredentialHandlerThatReturnsInvalidCredentials
            }); // Invalid credential provider is never called, cached credentials from the first request are used
        }

        public static Credentials CredentialsHandlerThatReturnsValidCredentials(string url, string usernameFromUrl,
            SupportedCredentialTypes types)
        {
            return new UsernamePasswordCredentials
            {
                Username = "<valid username>",
                Password = "<valid password>"
            };
        }

        public static Credentials CredentialHandlerThatReturnsInvalidCredentials(string url, string usernameFromUrl,
            SupportedCredentialTypes types)
        {
            return new UsernamePasswordCredentials
            {
                Username = "invalid username",
                Password = "invalid password"
            };
        }
    }
}

Expected behavior

Second clone should not authenticate, invalid credentials supplied.

Actual behavior

Authenticates with repository successfully, as there has already been a successful authentication to that repository URL.

Version of LibGit2Sharp (release number or SHA1)

0.27.0-preview-0102

Operating system(s) tested; .NET runtime tested

macOS, .NET 5

dylanlerch avatar Jul 19 '21 07:07 dylanlerch

This is a bit tricky to solve. I see you opened a PR, but that approach won't work. The SocketsHttpHandler/HttpClientHandler has to be static to avoid port exhaustion, which is why it is designed the way it is.

What is the use case for what you're trying to do? Why are you cloning the same repo more than once and wanting to use different credentials?

bording avatar Jul 19 '21 22:07 bording

It's definitely not a fun one. Understand around the socket exhaustion. I had some brief thoughts about using IHttpClientFactory but I'm yet to use it outside an ASP.NET app with default DI and all that, so haven't even begun to go down that road.

The case is multi-user server applications. A user can configure a connection to a repository with a url, username, and password and connect successfully (which is the expected behaviour). A completely separate user could then connect to that same repository with invalid credentials.

dylanlerch avatar Jul 19 '21 22:07 dylanlerch

. I had some brief thoughts about using IHttpClientFactory but I'm yet to use it outside an ASP.NET app with default DI and all that, so haven't even begun to go down that road.

Yeah, that's also not an option for this library since it's not designed to be used outside of ASP.NET Core.

The only possibility that's coming to mind would be to always make the SmartTransport.AcquireCredentials call and update the credentials cache before the SendAsync call, but if multiple requests are being made on different threads, even that wouldn't really fix the issue.

You'd likely have to make entire request behind some sort of synchronization primitives to avoid concurrent access at that point. 🤮

bording avatar Jul 19 '21 22:07 bording

Yeah, that's also not an option for this library since it's not designed to be used outside of ASP.NET Core.

I saw some vague mentions of it being possible, but no evidence of anyone actually using it outside of ASP.NET Core. I thought: Well, there is my answer! 🙂


I'll keep digging and see what we can make work. Working with keeping HttpClientHandler single instance (which I probably should have anyway), I'll see if there is a way to set credentials for each transport without having to do any sort of locking dances. 🤞

dylanlerch avatar Jul 19 '21 23:07 dylanlerch