refactor git-credentials handling for http requests

[technoweenie]

This changes Git LFS so it only checks git-credential for LFS API requests. The Git LFS API is intended to be mounted next to the Git server. So it makes sense to use git credentials, since it can take advantage of existing credentials for Git users that use HTTPS remotes.

https://github.com/foo/bar.git # git remote
https://github.com/foo/bar.git/info/lfs/... # lfs remote

However, the client shouldn't have to check git credentials for the "storage api" requests. These are the requests to download or upload raw objects from URLs linked from the LFS API response. The Storage API may not require any authorization for public objects in public repositories. Basically, the Git LFS client should trust the LFS server to return enough meta data to authenticate the requests.

1. doApiRequestWithRedirects() is the low level function that handles API communications. It now fetches the credentials if needed, and saves the credentials on success.
2. With the Batch API, there's now a distinction between public and private repos. Initially a repo is considered public, unless the batch API returns a 401. Then a git config value is set so future requests treat the repo as private. doApiBatchRequest() now tells doApiRequestWithRedirects() if credentials are needed

/cc @rubyist @sinbad

[WillHipschman]

WTF is a fun trace message :)

[rubyist]

Surely it means "Waiting To Fetch"!

[technoweenie]

Gah, 2nd debug message that I left in here.

force pushes

These are not the commits you are looking for.

[technoweenie]

I think we can fix #612 in this PR:

In client.go in getCreds if we make a request against mymachinename but the remote is localhost then we will not authenticate correctly.

if req.URL.Scheme != apiUrl.Scheme ||
   req.URL.Host != apiUrl.Host {

I believe that check was added so that the LFS client doesn't apply credentials from your LFS server at https://lfs-server.com to the storage requests hosted on a 3rd party system like S3. But since this PR only uses git credentials for LFS API requests, I think we can remove this check.

[WillHipschman]

Sounds good to me 👍

[sinbad]

While personally I would always get the API server to generate sufficient authentication tokens to pass to the storage server, I think dropping support for storage credentials entirely might be a step too far. How about supporting 401 responses from the storage server and fetching credentials only then, just like the API calls?

Even though the storage server might not be hosted in the same place as the git server, git-credentials is still a pretty solid way to cache authentication data, and it would be nice to retain the ability to use basic auth on storage calls, for systems that don't support tokens.

[ttaylorr]

Could probably push this value onto creds itself, since we return anyway if there's an non-nil value in err.

[technoweenie]

I don't understand, push what into creds. The error?

[ttaylorr]

Whoops, I see what you were doing here, ignore my suggestion.

[technoweenie]

@sinbad: While I think forcing users to maintain two sets of authentication is a poor user experience, I agree that having git-credential track stuff for the storage API could be useful.

[technoweenie]

I've updated this PR based on some feedback:

How about supporting 401 responses from the storage server and fetching credentials only then, just like the API calls?

I've refactored the credential and client methods a bit. All credential related functions are in lfs/credentials.go. Some things were renamed. Some exported types were removed to simplify the code. I also moved a lot of the manual Creds handling to doHttpRequest(). doHttpRequest() now takes an optional Creds arg. If given, it will accept or reject it based on the response. There are now three entry points to run HTTP requests:

• doAPIRequest() talks to the LFS API using credentials pulled from getCredsForAPI(). This is only directly used for the "verify" API calls.
• doApiBatchRequest() and doLegacyApiRequest() both use doAPIRequest(), but also parse the response JSON.
• doStorageRequest() directly uploads or downloads files using credentials pulled from getCreds().

One potential problem. Storage requests may add a lot of entries to the git credential helper if useHttpPath is set. I wonder if we can disable this for a specific git credential call, regardless of the git config?

getCreds incorrectly compares a requests URL scheme when against localhost #612

There are now two functions for filling an HTTP request with an Authorization header:

• getCreds() asks git credential for the request URL.
• getCredsForAPI() tries to get the credentials from the configured LFS or Git remote URLs before asking git credential. If the either the host or scheme of the request URL does not match the LFS or Git remote URLs, then it still asks git credential (instead of bailing).

getCreds fails when CredentialManagers don't return optional fields #614

It now ignores any exit code related errors from git credential fill. It still raises problems if the reject or accept subcommands exit unsuccessfully, however.

[technoweenie]

This is looking good in my QA tests 🤘