-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add username to target name of GitHub credential #160
Comments
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 I will update this issue with more information once we have it 😄 |
@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. :) |
Awesome write up! That's a useful summary of the differing behaviours of helpers in the wild. |
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. If no user account has been explicitly specified we'll just pick the first matching credential for that host – the same as It's set as a pre-release at the moment and not yet bundled with Git for Windows. |
@mjcheetham Awesome, thanks very much! I'll give the pre-release a go. Btw. IDK if the next git release will still default to |
@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 E.g. if I run this with the older version from brew (2.0.194-beta+819e6bc120):
and then update to the new pre-release (2.0.249-beta+85e4125669):
It will not find the credentials any more. This is because the old version used Maybe for compatibility it is worth looking for |
Awesome! Thanks for confirming 🎉
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. |
Sounds good to me. :) |
@mjcheetham Actually, I spoke too soon, there are still problems on Windows. Since we still don't add the username to the target name (
Add first user:
So far so good. Add second user:
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? |
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. |
The logic that should be followed here is we'll try and locate an existing target name without-user ( If a target name without-user already exists, and it's not for the same user, we will store the credential against a target name value that includes the user ( |
Hey @gaborcsardi I've just tried reproducing the behaviour you saw above, but I cannot: Credentials are stored here as expected, and recalled correctly. Is there something I've missed in the repro steps here? |
Ah, my bad, sorry. It seems that the |
I was able to get this to work but I was expecting a pop-up when I used a previously-unused account name, instead I had to add the credentials via the command line. Not a major issue, but a better UX would have saved me several minutes 🙂 |
🎉
What remote URL did you use for the second user? Did you include the user handle in the remote URL (for example: If the user is absent from the remote URL, we will pick "the first" GitHub credential we have stored. If a specific user has been specified in the remote URL, then we will look exactly for this user credential - or prompt. (at least, that's the design - perhaps there's an issue) |
I used the form |
Actually... I must have still done something wrong because now I lost access to my other account 🙄 |
I'm definitely missing a step somewhere. No matter what I've tried, it only saves a single Github credential and uses it everywhere. I have to use |
@RyanLamansky try setting |
@gaborcsardi Everything I know about the GCM command line I know from the discussion here, in other words, I have basically no idea what I'm doing. I can set those environment variables but I don't know how to confirm the version of GCM used or what steps you intend me to take next. |
Well, set them and then use |
With the following sequence,
then
and finally
It shows credentials for the second one instead of the first. |
From a command prompt window (cmd.exe) run the following: set GCM_TRACE=1
git credential fill
protocol=https
host=github.com ..and report back the output (please redact any private information). Hopefully you see something that starts with:
Can you tell me exactly how you're expecting to use Git and GCM here to have multiple users? Ideally you should not be calling
If you have need a specific user account, you must include the username in your query to |
The expectation is that any repo I clone using the The repository is private, so if the wrong user is configured, |
Hi @RyanLamansky, Looking at the trace you posted (excerpt below) shows that Git is not giving GCM the user portion of the remote URL.
There should be another entry passed by Git to us: Can you run with
|
@mjcheetham I believe I'm seeing the same behaviour, the user portion of the repo url is being ignored and credentials always end up in windows credential manager as I'm using Git Credential Manager version 2.0.252-beta+fe025c12fc (Windows, .NET Framework 4.0.30319.42000)
None of the following config options are set in my repo:
|
👋 hey folks, I'm doing a little repo hygiene, and it looks like this issue has a storied past. It began as a feature request, turned into a debugging session, and now is an open investigation into a possible (different) bug. I'm going to reset the title on this back to its prior feature-request state and then close it. I'll move the portions that contain @RyanLamansky and @nero120 's bug reports into a new issue. |
The originally-tracked issue has been completed! Subsequent bug tracking now happening in #284. |
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!
The text was updated successfully, but these errors were encountered: