From f060f0dc16a16418d54541d1b0296c3bb8f644e7 Mon Sep 17 00:00:00 2001 From: rick olson Date: Thu, 26 Oct 2017 18:34:33 -0600 Subject: [PATCH] lfsapi: replace old credential cache and CredentialHelpers types --- lfsapi/creds.go | 232 +++++++++----------------------------- lfsapi/creds_test.go | 260 ------------------------------------------- lfsapi/lfsapi.go | 38 +++++-- 3 files changed, 83 insertions(+), 447 deletions(-) diff --git a/lfsapi/creds.go b/lfsapi/creds.go index eaf79186..ec817c81 100644 --- a/lfsapi/creds.go +++ b/lfsapi/creds.go @@ -12,6 +12,27 @@ import ( "github.com/rubyist/tracerx" ) +type CredentialHelper interface { + Fill(Creds) (Creds, error) + Reject(Creds) error + Approve(Creds) error +} + +type Creds map[string]string + +func bufferCreds(c Creds) *bytes.Buffer { + buf := new(bytes.Buffer) + + for k, v := range c { + buf.Write([]byte(k)) + buf.Write([]byte("=")) + buf.Write([]byte(v)) + buf.Write([]byte("\n")) + } + + return buf +} + // getCredentialHelper parses a 'credsConfig' from the git and OS environments, // returning the appropriate CredentialHelper to authenticate requests with. // @@ -30,94 +51,18 @@ func (c *Client) getCredentialHelper(u *url.URL) (CredentialHelper, Creds) { return c.Credentials, input } - askpass, ok := c.osEnv.Get("GIT_ASKPASS") - if !ok { - askpass, ok = c.gitEnv.Get("core.askpass") + helpers := make([]CredentialHelper, 0, 3) + if c.cachingCredHelper != nil { + helpers = append(helpers, c.cachingCredHelper) } - if !ok { - askpass, ok = c.osEnv.Get("SSH_ASKPASS") - } - helper, _ := c.gitEnv.Get("credential.helper") - cached := c.gitEnv.Bool("lfs.cachecredentials", true) - skipPrompt := c.osEnv.Bool("GIT_TERMINAL_PROMPT", false) - - var hs []CredentialHelper - if len(helper) == 0 && len(askpass) > 0 { - hs = append(hs, &AskPassCredentialHelper{ - Program: askpass, - }) - } - - var h CredentialHelper - h = &commandCredentialHelper{ - SkipPrompt: skipPrompt, - } - - if cached { - h = withCredentialCache(h) - } - hs = append(hs, h) - - switch len(hs) { - case 0: - return defaultCredentialHelper, input - case 1: - return hs[0], input - } - return CredentialHelpers(hs), input -} - -// CredentialHelpers is a []CredentialHelper that iterates through each -// credential helper to fill, reject, or approve credentials. -type CredentialHelpers []CredentialHelper - -// Fill implements CredentialHelper.Fill by asking each CredentialHelper in -// order to fill the credentials. -// -// If a fill was successful, it is returned immediately, and no other -// `CredentialHelper`s are consulted. If any CredentialHelper returns an error, -// it is returned immediately. -func (h CredentialHelpers) Fill(what Creds) (Creds, error) { - for _, c := range h { - creds, err := c.Fill(what) - if err != nil { - return nil, err - } - - if creds != nil { - return creds, nil + if c.askpassCredHelper != nil { + helper, _ := c.gitEnv.Get("credential.helper") + if len(helper) == 0 { + helpers = append(helpers, c.askpassCredHelper) } } - return nil, nil -} - -// Reject implements CredentialHelper.Reject and rejects the given Creds "what" -// amongst all knonw CredentialHelpers. If any `CredentialHelper`s returned a -// non-nil error, no further `CredentialHelper`s are notified, so as to prevent -// inconsistent state. -func (h CredentialHelpers) Reject(what Creds) error { - for _, c := range h { - if err := c.Reject(what); err != nil { - return err - } - } - - return nil -} - -// Approve implements CredentialHelper.Approve and approves the given Creds -// "what" amongst all known CredentialHelpers. If any `CredentialHelper`s -// returned a non-nil error, no further `CredentialHelper`s are notified, so as -// to prevent inconsistent state. -func (h CredentialHelpers) Approve(what Creds) error { - for _, c := range h { - if err := c.Approve(what); err != nil { - return err - } - } - - return nil + return NewCredentialHelpers(append(helpers, c.commandCredHelper)), input } // AskPassCredentialHelper implements the CredentialHelper type for GIT_ASKPASS @@ -211,88 +156,6 @@ func (a *AskPassCredentialHelper) args(prompt string) []string { return []string{prompt} } -type CredentialHelper interface { - Fill(Creds) (Creds, error) - Reject(Creds) error - Approve(Creds) error -} - -type Creds map[string]string - -func bufferCreds(c Creds) *bytes.Buffer { - buf := new(bytes.Buffer) - - for k, v := range c { - buf.Write([]byte(k)) - buf.Write([]byte("=")) - buf.Write([]byte(v)) - buf.Write([]byte("\n")) - } - - return buf -} - -func withCredentialCache(helper CredentialHelper) CredentialHelper { - return &credentialCacher{ - cmu: new(sync.Mutex), - creds: make(map[string]Creds), - helper: helper, - } -} - -type credentialCacher struct { - // cmu guards creds - cmu *sync.Mutex - 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) - - c.cmu.Lock() - defer c.cmu.Unlock() - - 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 { - c.cmu.Lock() - defer c.cmu.Unlock() - - 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.cmu.Lock() - c.creds[credCacheKey(creds)] = creds - c.cmu.Unlock() - } - return err -} - type commandCredentialHelper struct { SkipPrompt bool } @@ -363,16 +226,25 @@ func (h *commandCredentialHelper) exec(subcommand string, input Creds) (Creds, e return creds, nil } -type credentialCacher2 struct { +type credentialCacher struct { creds map[string]Creds mu sync.Mutex } -func newCredentialCacher() *credentialCacher2 { - return &credentialCacher2{creds: make(map[string]Creds)} +func newCredentialCacher() *credentialCacher { + return &credentialCacher{creds: make(map[string]Creds)} } -func (c credentialCacher2) Fill(what Creds) (Creds, error) { +func credCacheKey(creds Creds) string { + parts := []string{ + creds["protocol"], + creds["host"], + creds["path"], + } + return strings.Join(parts, "//") +} + +func (c credentialCacher) Fill(what Creds) (Creds, error) { key := credCacheKey(what) c.mu.Lock() cached, ok := c.creds[key] @@ -385,7 +257,7 @@ func (c credentialCacher2) Fill(what Creds) (Creds, error) { return nil, credHelperNoOp } -func (c credentialCacher2) Approve(what Creds) error { +func (c credentialCacher) Approve(what Creds) error { key := credCacheKey(what) c.mu.Lock() @@ -399,7 +271,7 @@ func (c credentialCacher2) Approve(what Creds) error { return credHelperNoOp } -func (c credentialCacher2) Reject(what Creds) error { +func (c credentialCacher) Reject(what Creds) error { key := credCacheKey(what) c.mu.Lock() delete(c.creds, key) @@ -407,16 +279,16 @@ func (c credentialCacher2) Reject(what Creds) error { return credHelperNoOp } -// CredentialHelperSet is a []CredentialHelper that iterates through each +// CredentialHelpers is a []CredentialHelper that iterates through each // credential helper to fill, reject, or approve credentials. -type CredentialHelperSet struct { +type CredentialHelpers struct { helpers []CredentialHelper skippedHelpers map[int]bool mu sync.Mutex } func NewCredentialHelpers(helpers []CredentialHelper) CredentialHelper { - return &CredentialHelperSet{ + return &CredentialHelpers{ helpers: helpers, skippedHelpers: make(map[int]bool), } @@ -430,7 +302,7 @@ var credHelperNoOp = errors.New("no-op!") // If a fill was successful, it is returned immediately, and no other // `CredentialHelper`s are consulted. If any CredentialHelper returns an error, // it is returned immediately. -func (s CredentialHelperSet) Fill(what Creds) (Creds, error) { +func (s CredentialHelpers) Fill(what Creds) (Creds, error) { errs := make([]string, 0, len(s.helpers)) for i, h := range s.helpers { if s.skipped(i) { @@ -463,7 +335,7 @@ func (s CredentialHelperSet) Fill(what Creds) (Creds, error) { // amongst all knonw CredentialHelpers. If any `CredentialHelper`s returned a // non-nil error, no further `CredentialHelper`s are notified, so as to prevent // inconsistent state. -func (s CredentialHelperSet) Reject(what Creds) error { +func (s CredentialHelpers) Reject(what Creds) error { for i, h := range s.helpers { if s.skipped(i) { continue @@ -481,7 +353,7 @@ func (s CredentialHelperSet) Reject(what Creds) error { // "what" amongst all known CredentialHelpers. If any `CredentialHelper`s // returned a non-nil error, no further `CredentialHelper`s are notified, so as // to prevent inconsistent state. -func (s CredentialHelperSet) Approve(what Creds) error { +func (s CredentialHelpers) Approve(what Creds) error { skipped := make(map[int]bool) for i, h := range s.helpers { if s.skipped(i) { @@ -504,13 +376,13 @@ func (s CredentialHelperSet) Approve(what Creds) error { return errors.New("no valid credential helpers to approve") } -func (s CredentialHelperSet) skip(i int) { +func (s CredentialHelpers) skip(i int) { s.mu.Lock() s.skippedHelpers[i] = true s.mu.Unlock() } -func (s CredentialHelperSet) skipped(i int) bool { +func (s CredentialHelpers) skipped(i int) bool { s.mu.Lock() skipped := s.skippedHelpers[i] s.mu.Unlock() diff --git a/lfsapi/creds_test.go b/lfsapi/creds_test.go index a74cbac2..4e78e066 100644 --- a/lfsapi/creds_test.go +++ b/lfsapi/creds_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) type testCredHelper struct { @@ -265,262 +264,3 @@ func TestCredHelperSetAllFillErrors(t *testing.T) { assert.Equal(t, 0, len(helper1.reject)) assert.Equal(t, 0, len(helper2.reject)) } - -// 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 -} diff --git a/lfsapi/lfsapi.go b/lfsapi/lfsapi.go index 426809b5..411a8203 100644 --- a/lfsapi/lfsapi.go +++ b/lfsapi/lfsapi.go @@ -47,9 +47,12 @@ type Client struct { LoggingStats bool // DEPRECATED - gitEnv config.Environment - osEnv config.Environment - uc *config.URLConfig + commandCredHelper *commandCredentialHelper + askpassCredHelper *AskPassCredentialHelper + cachingCredHelper *credentialCacher + gitEnv config.Environment + osEnv config.Environment + uc *config.URLConfig } type Context interface { @@ -70,8 +73,9 @@ func NewClient(ctx Context) (*Client, error) { return nil, errors.Wrap(err, fmt.Sprintf("bad netrc file %s", netrcfile)) } + cacheCreds := gitEnv.Bool("lfs.cachecredentials", true) var sshResolver SSHResolver = &sshAuthClient{os: osEnv} - if gitEnv.Bool("lfs.cachecredentials", true) { + if cacheCreds { sshResolver = withSSHCache(sshResolver) } @@ -86,9 +90,29 @@ func NewClient(ctx Context) (*Client, error) { SkipSSLVerify: !gitEnv.Bool("http.sslverify", true) || osEnv.Bool("GIT_SSL_NO_VERIFY", false), Verbose: osEnv.Bool("GIT_CURL_VERBOSE", false), DebuggingVerbose: osEnv.Bool("LFS_DEBUG_HTTP", false), - gitEnv: gitEnv, - osEnv: osEnv, - uc: config.NewURLConfig(gitEnv), + commandCredHelper: &commandCredentialHelper{ + SkipPrompt: osEnv.Bool("GIT_TERMINAL_PROMPT", false), + }, + gitEnv: gitEnv, + osEnv: osEnv, + uc: config.NewURLConfig(gitEnv), + } + + askpass, ok := osEnv.Get("GIT_ASKPASS") + if !ok { + askpass, ok = gitEnv.Get("core.askpass") + } + if !ok { + askpass, _ = osEnv.Get("SSH_ASKPASS") + } + if len(askpass) > 0 { + c.askpassCredHelper = &AskPassCredentialHelper{ + Program: askpass, + } + } + + if cacheCreds { + c.cachingCredHelper = newCredentialCacher() } return c, nil