Add username to target name of GitHub credential

[gaborcsardi]

Currently it is cumbersome to handle multiple GitHub credentials with the github provider. One solution to this would be to include the username in the target name, just like it is included for the generic provider.

Relevant discussion: microsoft/Git-Credential-Manager-for-Windows#749

PR to fix the same issue in Git Credential Manager for Windows: microsoft/Git-Credential-Manager-for-Windows#891

Thanks much for considering this!

[mjcheetham]

Hi @gaborcsardi! Thanks for opening this issue and your interest in GCM Core.

We are looking at changing the way we store and recall stored credentials in general in GCM Core, specifically such that multi-user scenarios like this are supported better. Unfortunately to get the best experience a change in how Git interacts with credential helpers (like GCM Core) is likely to be required.

We are currently looking at submitting such a change to Git, as well as an overhaul in GCM Core to better support scenarios as best we can without breaking backwards compatibility when used on down-level versions of Git (without our proposed patches), and to take advantage of a Git that may support our patches to 100% support all scenarios.

Note: the change we're looking at introducing to Git is to have Git echo all key=value pairs that are returned over stdout, not just the ones known to Git internally. This will allow us to preserve some state between the independent calls Git makes to the helpers (get followed by a store/erase) in an authentication flow.

I will update this issue with more information once we have it 😄

[gaborcsardi]

@mjcheetham Thanks a lot for the update! Makes sense.

FWIW I did a quick review on how the current (commonly used) helpers support multiple credentials, it is here: https://github.com/r-lib/gitcreds/blob/master/vignettes/gitcreds-advanced.Rmd

It will great to be able to switch to manager-core, can't wait. :)

[mjcheetham]

Awesome write up! That's a useful summary of the differing behaviours of helpers in the wild.
As I mentioned we're hoping to match more closely with the wincred or osxkeychain models, but handling multi-user accounts a little better.

[mjcheetham]

Hi @gaborcsardi! I have an update for you here.

We've got a pre-release out now which should allow multiple users for the same host provider, such as GitHub.

https://github.com/microsoft/Git-Credential-Manager-Core/releases/tag/v2.0.246-beta

If you include a username in the remote URL (e.g. https://<account>@github.com/<owner>/<repo>) then we'll only look for existing creds for <account>. Credentials are also stored with the real username/account now, in the target name (if there are multiple users).

If no user account has been explicitly specified we'll just pick the first matching credential for that host – the same as wincred and osxkeychain today.

It's set as a pre-release at the moment and not yet bundled with Git for Windows.

[gaborcsardi]

@mjcheetham Awesome, thanks very much! I'll give the pre-release a go.

Btw. IDK if the next git release will still default to manager on Windows, but if it will, then it might make sense to fix this there as well. FWIW there is a PR: microsoft/Git-Credential-Manager-for-Windows#891 (Disclaimer: I haven't actually tested this PR.)

[gaborcsardi]

@mjcheetham I tried the new pre-release on macOS, and it seems to work well, so thanks for the update!

One potential catch for existing users is that AFAICT now manager-core does not include the username in the service name for the generic provider. This is a change of behavior, and the new pre-release will not find the credentials that were set with the old version. (FYI, if this is a concern at all. :) )

E.g. if I run this with the older version from brew (2.0.194-beta+819e6bc120):

❯ GCM_PROVIDER=generic git credential approve
url=https://user@github.com
password=useruser


❯ GCM_PROVIDER=generic git credential fill
url=https://user@github.com

protocol=https
host=github.com
username=user
password=useruser

and then update to the new pre-release (2.0.249-beta+85e4125669):

❯ GCM_PROVIDER=generic git credential fill
url=https://user@github.com

Enter basic credentials for 'https://github.com/':
Username: user
Password:

It will not find the credentials any more. This is because the old version used git:https://user@github.com/ as a service name, and the new version is looking for git:https://github.com only.

Maybe for compatibility it is worth looking for git:https://user@github.com/ as well? But of course it is also OK to change the behavior. :)

[mjcheetham]

@mjcheetham I tried the new pre-release on macOS, and it seems to work well, so thanks for the update!

Awesome! Thanks for confirming 🎉

One potential catch for existing users is that AFAICT now manager-core does not include the username in the service name for the generic provider. This is a change of behavior, and the new pre-release will not find the credentials that were set with the old version. (FYI, if this is a concern at all. :) )

It will not find the credentials any more. This is because the old version used git:https://user@github.com/ as a service name, and the new version is looking for git:https://github.com only.

Maybe for compatibility it is worth looking for git:https://user@github.com/ as well? But of course it is also OK to change the behavior. :)

This is something we might look at adding if we get more complaints, but to be honest I think the breakage here is "acceptable", and I'd rather this than adding more codepaths to the credential lookup to support older versions. The worst an upgrader will see is one more credential prompt, and then nothing else. Also this would only be for users not using GitHub, Bitbucket, or Azure Repos (probably a small minority).

To be honest, I knew the credential lookup/store was inadequate and probably too simplistic, but it was "good enough for now" then. I hope we're now at a stable place where we won't be changing behaviour anymore, and if we do, then back-compat will be a concern going forwards.

[gaborcsardi]

Sounds good to me. :)

[gaborcsardi]

@mjcheetham Actually, I spoke too soon, there are still problems on Windows. Since we still don't add the username to the target name (TargetName) and the target name is a primary key on Windows, we cannot add multiple users, still. E.g.

❯ git config credential.helper
manager-core
❯ git-credential-manager-core.exe --version
Git Credential Manager version 2.0.249-beta+85e4125669 (Windows, .NET Framework 4.0.30319.42000)

Add first user:

❯ git credential approve
url=https://user1@github.com
password=pass1

❯ git credential fill
url=https://user1@github.com

protocol=https
host=github.com
username=user1
password=pass1

So far so good. Add second user:

❯ git credential approve
url=https://user2@github.com
password=pass2

❯ git credential fill
url=https://user1@github.com

protocol=https
host=github.com
username=user2
password=pass2

Oh, this is not good, the first user's credentials were overwritten.

Based on your description above, this is not the expected behavior, and the user name should be stored in the target name, right?

[mjcheetham]

Based on your description above, this is not the expected behavior, and the user name should be stored in the target name, right?

Correct. This should not happen. There is an integration test for this scenario, so not sure what's going on.. I'll reactivate this issue to track the bug.