Skip to content

Support using credentials from multiple accounts against the LFS endpoint#554

Merged
technoweenie merged 3 commits intogit-lfs:masterfrom
clareliguori:master
Aug 4, 2015
Merged

Support using credentials from multiple accounts against the LFS endpoint#554
technoweenie merged 3 commits intogit-lfs:masterfrom
clareliguori:master

Conversation

@clareliguori
Copy link
Contributor

Support credentials from multiple accounts when useHttpPath=true is set in the git config

Provide the URL path to 'git credential fill' for credential helpers that need the path to return the right credentials
Use the objects API URL for credentials so there is a stable path passed to the credential helper for caching

…et in the git config

Provide the URL path to 'git credential fill' for credential helpers that need the path to return the right credentials
Use the objects API URL for credentials so there is a stable path passed to the credential helper for caching
@technoweenie
Copy link
Contributor

Ah, good call. I have a few questions though:

Will passing the path to the credential fill command (https://github.com/github/git-lfs/pull/554/files#diff-1eed123d34cdc2b60cc1a63521a9ca88R20) cause any problems for those of us without useHttpPath enabled? I don't think so, but I'll setup my laptop to use the osxkeychain helper instead of boxen's to see firsthand.

Do you by chance know what path git uses for pushes and pulls? We don't want end users to have to enter their password again for /{user}/{repo}.git/info/lfs/objects when they've already entered it for /{user}/{repo}.git.

EDIT: Found it. Git doesn't try to do anything intelligent.

git clone https://github.com/github/git-lfs-test
Cloning into 'git-lfs-test'...
Username for 'https://github.com/github/git-lfs-test': 

So, I think this should attempt to use the same path that the remote uses IF apiUrl's scheme and host matches the remote.

$ git remote show origin
* remote origin
  Fetch URL: https://github.com/github/git-lfs-test
  Push  URL: https://github.com/github/git-lfs-test

In this case, getCreds() should pass a path of "github/git-lfs-test" so that it's using the same user/pass as the Git repo.

@technoweenie
Copy link
Contributor

I think this needs to strip the leading / from the path:

Downloading droidtocat.gif (24.65 KB)
trace git-lfs: run_command: 'git' config -l
trace git-lfs: run_command: 'git' config -l -f .git/config
Username for 'https://github.com//technoweenie/glowing-octo-lana.git/info/lfs/objects':

The Username prompt looks correct here:

$ echo "protocol=https\nhost=github.com\npath=technoweenie/glowing-octo-lana.git/info/lfs/objects" | git credential fill 
Username for 'https://github.com/technoweenie/glowing-octo-lana.git/info/lfs/objects':

@technoweenie technoweenie mentioned this pull request Jul 31, 2015
13 tasks
@clareliguori
Copy link
Contributor Author

Added trimming the leading slash, so you get this now:

$ git clone https://github.com/technoweenie/glowing-octo-lana
Downloading droidtocat.gif (24.65 KB)
Username for 'https://github.com/technoweenie/glowing-octo-lana.git/info/lfs/objects'

Good idea on re-using the base remote URL, I'll work on that

…t users don't have to cache the same credentials twice
@clareliguori
Copy link
Contributor Author

Patch will now use the git remote's credentials if possible.

$ git clone https://github.com/technoweenie/glowing-octo-lana
Downloading droidtocat.gif (24.65 KB)
Password for 'https://clareliguori@github.com/technoweenie/glowing-octo-lana':

@technoweenie
Copy link
Contributor

This looks great. There are a couple more things:

  1. The code to set the auth from the git remote is directly copied from earlier in the function. It could be extracted into a small, private helper function. I'll probably just make a small refactoring PR after merging this.
  2. I'd really like new tests for this. Unfortunately, we don't have adequate credential tests at ALL. So I'll probably tackle this on my own.

This will likely make it into Git LFS v0.6.0, and a v0.5.5 if another bug fix release is necessary. Thanks for sending this patch!

This was referenced Aug 4, 2015
@technoweenie technoweenie merged commit 5bcb160 into git-lfs:master Aug 4, 2015
@technoweenie
Copy link
Contributor

Thanks again for the patch. I made a private helper and added some shell tests in #561.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants