Merge pull request #2080 from git-lfs/auth-caching

ssh auth and credential helper caching

docs/man/git-lfs-config.5.ronn

@@ -45,6 +45,11 @@ be scoped inside the configuration for a remote.
   Sets the maximum time, in seconds, for the HTTP client to maintain keepalive
   connections. Default: 30 minutes.
 
+* `lfs.cachecredentials`
+
+  Enables in-memory SSH and Git Credential caching for a single 'git lfs'
+  command. Default: false. This will default to true in v2.1.0.
+
 ### Transfer (upload / download) settings
 
   These settings control how the upload and download of LFS content occurs.

lfsapi/client.go

@@ -18,7 +18,7 @@ var UserAgent = "git-lfs"
 const MediaType = "application/vnd.git-lfs+json; charset=utf-8"
 
 func (c *Client) NewRequest(method string, e Endpoint, suffix string, body interface{}) (*http.Request, error) {
-	sshRes, err := c.resolveSSHEndpoint(e, method)
+	sshRes, err := c.SSH.Resolve(e, method)
 	if err != nil {
 		tracerx.Printf("ssh: %s failed, error: %s, message: %s",
 			e.SshUserAndHost, err.Error(), sshRes.Message,

lfsapi/creds.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"os/exec"
 	"strings"
+
+	"github.com/rubyist/tracerx"
 )
 
 type CredentialHelper interface {
@@ -28,11 +30,62 @@ func bufferCreds(c Creds) *bytes.Buffer {
 	return buf
 }
 
+func withCredentialCache(helper CredentialHelper) CredentialHelper {
+	return &credentialCacher{
+		creds:  make(map[string]Creds),
+		helper: helper,
+	}
+}
+
+type credentialCacher struct {
+	creds  map[string]Creds
+	helper CredentialHelper
+}
+
+func credCacheKey(creds Creds) string {
+	parts := []string{
+		creds["protocol"],
+		creds["host"],
+		creds["path"],
+	}
+	return strings.Join(parts, "//")
+}
+
+func (c *credentialCacher) Fill(creds Creds) (Creds, error) {
+	key := credCacheKey(creds)
+	if cache, ok := c.creds[key]; ok {
+		tracerx.Printf("creds: git credential cache (%q, %q, %q)",
+			creds["protocol"], creds["host"], creds["path"])
+		return cache, nil
+	}
+
+	creds, err := c.helper.Fill(creds)
+	if err == nil && len(creds["username"]) > 0 && len(creds["password"]) > 0 {
+		c.creds[key] = creds
+	}
+	return creds, err
+}
+
+func (c *credentialCacher) Reject(creds Creds) error {
+	delete(c.creds, credCacheKey(creds))
+	return c.helper.Reject(creds)
+}
+
+func (c *credentialCacher) Approve(creds Creds) error {
+	err := c.helper.Approve(creds)
+	if err == nil {
+		c.creds[credCacheKey(creds)] = creds
+	}
+	return err
+}
+
 type commandCredentialHelper struct {
 	SkipPrompt bool
 }
 
 func (h *commandCredentialHelper) Fill(creds Creds) (Creds, error) {
+	tracerx.Printf("creds: git credential fill (%q, %q, %q)",
+		creds["protocol"], creds["host"], creds["path"])
 	return h.exec("fill", creds)
 }
 

lfsapi/creds_test.go

@@ -0,0 +1,268 @@
+package lfsapi
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// test that cache satisfies Fill() without looking at creds
+func TestCredsCacheFillFromCache(t *testing.T) {
+	creds := newFakeCreds()
+	cache := withCredentialCache(creds).(*credentialCacher)
+	cache.creds["http//lfs.test//foo/bar"] = Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+		"username": "u",
+		"password": "p",
+	}
+
+	filled, err := cache.Fill(Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+	})
+	assert.Nil(t, err)
+	require.NotNil(t, filled)
+	assert.Equal(t, "u", filled["username"])
+	assert.Equal(t, "p", filled["password"])
+
+	assert.Equal(t, 1, len(cache.creds))
+	cached, ok := cache.creds["http//lfs.test//foo/bar"]
+	assert.True(t, ok)
+	assert.Equal(t, "u", cached["username"])
+	assert.Equal(t, "p", cached["password"])
+}
+
+// test that cache caches Fill() value from creds
+func TestCredsCacheFillFromValidHelperFill(t *testing.T) {
+	creds := newFakeCreds()
+	cache := withCredentialCache(creds).(*credentialCacher)
+
+	creds.list = append(creds.list, Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+		"username": "u",
+		"password": "p",
+	})
+
+	assert.Equal(t, 0, len(cache.creds))
+
+	filled, err := cache.Fill(Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+	})
+	assert.Nil(t, err)
+	require.NotNil(t, filled)
+	assert.Equal(t, "u", filled["username"])
+	assert.Equal(t, "p", filled["password"])
+
+	assert.Equal(t, 1, len(cache.creds))
+	cached, ok := cache.creds["http//lfs.test//foo/bar"]
+	assert.True(t, ok)
+	assert.Equal(t, "u", cached["username"])
+	assert.Equal(t, "p", cached["password"])
+
+	creds.list = make([]Creds, 0)
+	filled2, err := cache.Fill(Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+	})
+	assert.Nil(t, err)
+	require.NotNil(t, filled2)
+	assert.Equal(t, "u", filled2["username"])
+	assert.Equal(t, "p", filled2["password"])
+}
+
+// test that cache ignores Fill() value from creds with missing username+password
+func TestCredsCacheFillFromInvalidHelperFill(t *testing.T) {
+	creds := newFakeCreds()
+	cache := withCredentialCache(creds).(*credentialCacher)
+
+	creds.list = append(creds.list, Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+		"username": "no-password",
+	})
+
+	assert.Equal(t, 0, len(cache.creds))
+
+	filled, err := cache.Fill(Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+		"username": "u",
+		"password": "p",
+	})
+	assert.Nil(t, err)
+	require.NotNil(t, filled)
+	assert.Equal(t, "no-password", filled["username"])
+	assert.Equal(t, "", filled["password"])
+
+	assert.Equal(t, 0, len(cache.creds))
+}
+
+// test that cache ignores Fill() value from creds with error
+func TestCredsCacheFillFromErroringHelperFill(t *testing.T) {
+	creds := newFakeCreds()
+	cache := withCredentialCache(&erroringCreds{creds}).(*credentialCacher)
+
+	creds.list = append(creds.list, Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+		"username": "u",
+		"password": "p",
+	})
+
+	assert.Equal(t, 0, len(cache.creds))
+
+	filled, err := cache.Fill(Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+	})
+	assert.NotNil(t, err)
+	require.NotNil(t, filled)
+	assert.Equal(t, "u", filled["username"])
+	assert.Equal(t, "p", filled["password"])
+
+	assert.Equal(t, 0, len(cache.creds))
+}
+
+func TestCredsCacheRejectWithoutError(t *testing.T) {
+	creds := newFakeCreds()
+	cache := withCredentialCache(creds).(*credentialCacher)
+
+	cache.creds["http//lfs.test//foo/bar"] = Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+		"username": "u",
+		"password": "p",
+	}
+
+	err := cache.Reject(Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+	})
+	assert.Nil(t, err)
+	assert.Equal(t, 0, len(cache.creds))
+}
+
+func TestCredsCacheRejectWithError(t *testing.T) {
+	creds := newFakeCreds()
+	cache := withCredentialCache(&erroringCreds{creds}).(*credentialCacher)
+
+	cache.creds["http//lfs.test//foo/bar"] = Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+		"username": "u",
+		"password": "p",
+	}
+
+	err := cache.Reject(Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+	})
+	assert.NotNil(t, err)
+	assert.Equal(t, 0, len(cache.creds))
+}
+
+func TestCredsCacheApproveWithoutError(t *testing.T) {
+	creds := newFakeCreds()
+	cache := withCredentialCache(creds).(*credentialCacher)
+
+	assert.Equal(t, 0, len(cache.creds))
+
+	err := cache.Approve(Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+		"username": "U",
+		"password": "P",
+	})
+	assert.Nil(t, err)
+	assert.Equal(t, 1, len(cache.creds))
+	cached, ok := cache.creds["http//lfs.test//foo/bar"]
+	assert.True(t, ok)
+	assert.Equal(t, "U", cached["username"])
+	assert.Equal(t, "P", cached["password"])
+}
+
+func TestCredsCacheApproveWithError(t *testing.T) {
+	creds := newFakeCreds()
+	cache := withCredentialCache(&erroringCreds{creds}).(*credentialCacher)
+
+	assert.Equal(t, 0, len(cache.creds))
+
+	err := cache.Approve(Creds{
+		"protocol": "http",
+		"host":     "lfs.test",
+		"path":     "foo/bar",
+		"username": "u",
+		"password": "p",
+	})
+	assert.NotNil(t, err)
+	assert.Equal(t, 0, len(cache.creds))
+}
+
+func newFakeCreds() *fakeCreds {
+	return &fakeCreds{list: make([]Creds, 0)}
+}
+
+type erroringCreds struct {
+	helper CredentialHelper
+}
+
+func (e *erroringCreds) Fill(creds Creds) (Creds, error) {
+	c, _ := e.helper.Fill(creds)
+	return c, errors.New("fill error")
+}
+
+func (e *erroringCreds) Reject(creds Creds) error {
+	e.helper.Reject(creds)
+	return errors.New("reject error")
+}
+
+func (e *erroringCreds) Approve(creds Creds) error {
+	e.helper.Approve(creds)
+	return errors.New("approve error")
+}
+
+type fakeCreds struct {
+	list []Creds
+}
+
+func credsMatch(c1, c2 Creds) bool {
+	return c1["protocol"] == c2["protocol"] &&
+		c1["host"] == c2["host"] &&
+		c1["path"] == c2["path"]
+}
+
+func (f *fakeCreds) Fill(creds Creds) (Creds, error) {
+	for _, saved := range f.list {
+		if credsMatch(creds, saved) {
+			return saved, nil
+		}
+	}
+	return creds, nil
+}
+
+func (f *fakeCreds) Reject(creds Creds) error {
+	return nil
+}
+
+func (f *fakeCreds) Approve(creds Creds) error {
+	return nil
+}

lfsapi/lfsapi.go

@@ -22,6 +22,7 @@ var (
 type Client struct {
 	Endpoints   EndpointFinder
 	Credentials CredentialHelper
+	SSH         SSHResolver
 	Netrc       NetrcFinder
 
 	DialTimeout         int
@@ -70,11 +71,20 @@ func NewClient(osEnv Env, gitEnv Env) (*Client, error) {
 
 	httpsProxy, httpProxy, noProxy := getProxyServers(osEnv, gitEnv)
 
+	var creds CredentialHelper = &commandCredentialHelper{
+		SkipPrompt: !osEnv.Bool("GIT_TERMINAL_PROMPT", true),
+	}
+	var sshResolver SSHResolver = &sshAuthClient{os: osEnv}
+
+	if gitEnv.Bool("lfs.cachecredentials", false) {
+		creds = withCredentialCache(creds)
+		sshResolver = withSSHCache(sshResolver)
+	}
+
 	c := &Client{
 		Endpoints:           NewEndpointFinder(gitEnv),
-		Credentials: &commandCredentialHelper{
+		Credentials:         creds,
-			SkipPrompt: !osEnv.Bool("GIT_TERMINAL_PROMPT", true),
+		SSH:                 sshResolver,
-		},
 		Netrc:               netrc,
 		DialTimeout:         gitEnv.Int("lfs.dialtimeout", 0),
 		KeepaliveTimeout:    gitEnv.Int("lfs.keepalive", 0),

lfsapi/ssh.go

@@ -7,18 +7,65 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/git-lfs/git-lfs/tools"
 	"github.com/rubyist/tracerx"
 )
 
-func (c *Client) resolveSSHEndpoint(e Endpoint, method string) (sshAuthResponse, error) {
+type SSHResolver interface {
+	Resolve(Endpoint, string) (sshAuthResponse, error)
+}
+
+func withSSHCache(ssh SSHResolver) SSHResolver {
+	return &sshCache{
+		endpoints: make(map[string]*sshAuthResponse),
+		ssh:       ssh,
+	}
+}
+
+type sshCache struct {
+	endpoints map[string]*sshAuthResponse
+	ssh       SSHResolver
+}
+
+func (c *sshCache) Resolve(e Endpoint, method string) (sshAuthResponse, error) {
+	if len(e.SshUserAndHost) == 0 {
+		return sshAuthResponse{}, nil
+	}
+
+	key := strings.Join([]string{e.SshUserAndHost, e.SshPort, e.SshPath, method}, "//")
+	if res, ok := c.endpoints[key]; ok && (res.ExpiresAt.IsZero() || time.Until(res.ExpiresAt) > 5*time.Second) {
+		tracerx.Printf("ssh cache: %s git-lfs-authenticate %s %s",
+			e.SshUserAndHost, e.SshPath, endpointOperation(e, method))
+		return *res, nil
+	}
+
+	res, err := c.ssh.Resolve(e, method)
+	if err == nil {
+		c.endpoints[key] = &res
+	}
+	return res, err
+}
+
+type sshAuthResponse struct {
+	Message   string            `json:"-"`
+	Href      string            `json:"href"`
+	Header    map[string]string `json:"header"`
+	ExpiresAt time.Time         `json:"expires_at"`
+}
+
+type sshAuthClient struct {
+	os Env
+}
+
+func (c *sshAuthClient) Resolve(e Endpoint, method string) (sshAuthResponse, error) {
 	res := sshAuthResponse{}
 	if len(e.SshUserAndHost) == 0 {
 		return res, nil
 	}
 
-	exe, args := sshGetLFSExeAndArgs(c.osEnv, e, method)
+	exe, args := sshGetLFSExeAndArgs(c.os, e, method)
 	cmd := exec.Command(exe, args...)
 
 	// Save stdout and stderr in separate buffers
@@ -42,13 +89,6 @@ func (c *Client) resolveSSHEndpoint(e Endpoint, method string) (sshAuthResponse,
 	return res, err
 }
 
-type sshAuthResponse struct {
-	Message   string            `json:"-"`
-	Href      string            `json:"href"`
-	Header    map[string]string `json:"header"`
-	ExpiresAt string            `json:"expires_at"`
-}
-
 func sshGetLFSExeAndArgs(osEnv Env, e Endpoint, method string) (string, []string) {
 	operation := endpointOperation(e, method)
 	tracerx.Printf("ssh: %s git-lfs-authenticate %s %s",

lfsapi/ssh_test.go

@@ -1,13 +1,146 @@
 package lfsapi
 
 import (
+	"errors"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestSSHCacheResolveFromCache(t *testing.T) {
+	ssh := newFakeResolver()
+	cache := withSSHCache(ssh).(*sshCache)
+	cache.endpoints["userandhost//1//path//post"] = &sshAuthResponse{
+		Href: "cache",
+	}
+	ssh.responses["userandhost"] = sshAuthResponse{Href: "real"}
+
+	e := Endpoint{
+		SshUserAndHost: "userandhost",
+		SshPort:        "1",
+		SshPath:        "path",
+	}
+
+	res, err := cache.Resolve(e, "post")
+	assert.Nil(t, err)
+	assert.Equal(t, "cache", res.Href)
+}
+
+func TestSSHCacheResolveFromCacheWithFutureExpiresAt(t *testing.T) {
+	ssh := newFakeResolver()
+	cache := withSSHCache(ssh).(*sshCache)
+	cache.endpoints["userandhost//1//path//post"] = &sshAuthResponse{
+		Href:      "cache",
+		ExpiresAt: time.Now().Add(time.Duration(1) * time.Hour),
+	}
+	ssh.responses["userandhost"] = sshAuthResponse{Href: "real"}
+
+	e := Endpoint{
+		SshUserAndHost: "userandhost",
+		SshPort:        "1",
+		SshPath:        "path",
+	}
+
+	res, err := cache.Resolve(e, "post")
+	assert.Nil(t, err)
+	assert.Equal(t, "cache", res.Href)
+}
+
+func TestSSHCacheResolveFromCacheWithPastExpiresAt(t *testing.T) {
+	ssh := newFakeResolver()
+	cache := withSSHCache(ssh).(*sshCache)
+	cache.endpoints["userandhost//1//path//post"] = &sshAuthResponse{
+		Href:      "cache",
+		ExpiresAt: time.Now().Add(time.Duration(-1) * time.Hour),
+	}
+	ssh.responses["userandhost"] = sshAuthResponse{Href: "real"}
+
+	e := Endpoint{
+		SshUserAndHost: "userandhost",
+		SshPort:        "1",
+		SshPath:        "path",
+	}
+
+	res, err := cache.Resolve(e, "post")
+	assert.Nil(t, err)
+	assert.Equal(t, "real", res.Href)
+}
+
+func TestSSHCacheResolveWithoutError(t *testing.T) {
+	ssh := newFakeResolver()
+	cache := withSSHCache(ssh).(*sshCache)
+
+	assert.Equal(t, 0, len(cache.endpoints))
+
+	ssh.responses["userandhost"] = sshAuthResponse{Href: "real"}
+
+	e := Endpoint{
+		SshUserAndHost: "userandhost",
+		SshPort:        "1",
+		SshPath:        "path",
+	}
+
+	res, err := cache.Resolve(e, "post")
+	assert.Nil(t, err)
+	assert.Equal(t, "real", res.Href)
+
+	assert.Equal(t, 1, len(cache.endpoints))
+	cacheres, ok := cache.endpoints["userandhost//1//path//post"]
+	assert.True(t, ok)
+	assert.NotNil(t, cacheres)
+	assert.Equal(t, "real", cacheres.Href)
+
+	delete(ssh.responses, "userandhost")
+	res2, err := cache.Resolve(e, "post")
+	assert.Nil(t, err)
+	assert.Equal(t, "real", res2.Href)
+}
+
+func TestSSHCacheResolveWithError(t *testing.T) {
+	ssh := newFakeResolver()
+	cache := withSSHCache(ssh).(*sshCache)
+
+	assert.Equal(t, 0, len(cache.endpoints))
+
+	ssh.responses["userandhost"] = sshAuthResponse{Message: "resolve error", Href: "real"}
+
+	e := Endpoint{
+		SshUserAndHost: "userandhost",
+		SshPort:        "1",
+		SshPath:        "path",
+	}
+
+	res, err := cache.Resolve(e, "post")
+	assert.NotNil(t, err)
+	assert.Equal(t, "real", res.Href)
+
+	assert.Equal(t, 0, len(cache.endpoints))
+	delete(ssh.responses, "userandhost")
+	res2, err := cache.Resolve(e, "post")
+	assert.Nil(t, err)
+	assert.Equal(t, "", res2.Href)
+}
+
+func newFakeResolver() *fakeResolver {
+	return &fakeResolver{responses: make(map[string]sshAuthResponse)}
+}
+
+type fakeResolver struct {
+	responses map[string]sshAuthResponse
+}
+
+func (r *fakeResolver) Resolve(e Endpoint, method string) (sshAuthResponse, error) {
+	res := r.responses[e.SshUserAndHost]
+	var err error
+	if len(res.Message) > 0 {
+		err = errors.New(res.Message)
+	}
+	return res, err
+}
+
 func TestSSHGetLFSExeAndArgs(t *testing.T) {
 	cli, err := NewClient(TestEnv(map[string]string{}), nil)
 	require.Nil(t, err)