From 2a8670e0da11b26e54bcccf815afbf7d851073c6 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Thu, 9 Mar 2017 01:33:48 +0100 Subject: [PATCH] Cleanup original modifications to the exposed APIs Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client_test.go | 10 +++++----- credentials/credentials.go | 10 ++++------ credentials/credentials_test.go | 2 +- credentials/helper.go | 5 ++--- osxkeychain/osxkeychain_darwin.go | 6 +++--- osxkeychain/osxkeychain_darwin_test.go | 6 ++---- secretservice/secretservice_linux.go | 6 +++--- secretservice/secretservice_linux_test.go | 5 ++--- wincred/wincred_windows.go | 4 ++-- wincred/wincred_windows_test.go | 6 ++---- 10 files changed, 26 insertions(+), 34 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index c1665c4..e5f1bf2 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -106,8 +106,8 @@ func ExampleStore() { func TestStore(t *testing.T) { valid := []credentials.Credentials{ - {credentials.CredsLabel, validServerAddress, "foo", "bar"}, - {credentials.CredsLabel, validServerAddress2, "", "abcd1234"}, + {validServerAddress, "foo", "bar"}, + {validServerAddress2, "", "abcd1234"}, } for _, v := range valid { @@ -117,7 +117,7 @@ func TestStore(t *testing.T) { } invalid := []credentials.Credentials{ - {credentials.CredsLabel, invalidServerAddress, "foo", "bar"}, + {invalidServerAddress, "foo", "bar"}, } for _, v := range invalid { @@ -140,8 +140,8 @@ func ExampleGet() { func TestGet(t *testing.T) { valid := []credentials.Credentials{ - {credentials.CredsLabel, validServerAddress, "foo", "bar"}, - {credentials.CredsLabel, validServerAddress2, "", "abcd1234"}, + {validServerAddress, "foo", "bar"}, + {validServerAddress2, "", "abcd1234"}, } for _, v := range valid { diff --git a/credentials/credentials.go b/credentials/credentials.go index 539a5a4..ec1aff8 100644 --- a/credentials/credentials.go +++ b/credentials/credentials.go @@ -12,14 +12,14 @@ import ( // Credentials holds the information shared between docker and the credentials store. type Credentials struct { - Label string ServerURL string Username string Secret string } -// Docker credentials should be labeled as such in credential stores, this label -// allow us to filter out non-Docker credentials at lookup +// Docker credentials should be labeled as such in credentials stores that allow labelling. +// That label allows to filter out non-Docker credentials too at lookup/search in macOS keychain, +// Windows credentials manager and Linux libsecret. const CredsLabel = "Docker Credentials" // Serve initializes the credentials helper and parses the action argument. @@ -77,8 +77,6 @@ func Store(helper Helper, reader io.Reader) error { return err } - creds.Label = CredsLabel - return helper.Add(&creds) } @@ -140,7 +138,7 @@ func Erase(helper Helper, reader io.Reader) error { //List returns all the serverURLs of keys in //the OS store as a list of strings func List(helper Helper, writer io.Writer) error { - accts, err := helper.List(CredsLabel) + accts, err := helper.List() if err != nil { return err } diff --git a/credentials/credentials_test.go b/credentials/credentials_test.go index 3c2ca52..6b8bbe1 100644 --- a/credentials/credentials_test.go +++ b/credentials/credentials_test.go @@ -36,7 +36,7 @@ func (m *memoryStore) Get(serverURL string) (string, string, error) { return c.Username, c.Secret, nil } -func (m *memoryStore) List(credsLabel string) (map[string]string, error) { +func (m *memoryStore) List() (map[string]string, error) { //Simply a placeholder to let memoryStore be a valid implementation of Helper interface return nil, nil } diff --git a/credentials/helper.go b/credentials/helper.go index fad53ee..135acd2 100644 --- a/credentials/helper.go +++ b/credentials/helper.go @@ -9,7 +9,6 @@ type Helper interface { // Get retrieves credentials from the store. // It returns username and secret as strings. Get(serverURL string) (string, string, error) - // List returns the stored serverURLs and their associated usernames - // for a given credentials label. - List(credsLabel string) (map[string]string, error) + // List returns the stored serverURLs and their associated usernames. + List() (map[string]string, error) } diff --git a/osxkeychain/osxkeychain_darwin.go b/osxkeychain/osxkeychain_darwin.go index 137b894..a3f5da8 100644 --- a/osxkeychain/osxkeychain_darwin.go +++ b/osxkeychain/osxkeychain_darwin.go @@ -35,7 +35,7 @@ func (h Osxkeychain) Add(creds *credentials.Credentials) error { } defer freeServer(s) - label := C.CString(creds.Label) + label := C.CString(credentials.CredsLabel) defer C.free(unsafe.Pointer(label)) username := C.CString(creds.Username) defer C.free(unsafe.Pointer(username)) @@ -100,8 +100,8 @@ func (h Osxkeychain) Get(serverURL string) (string, string, error) { } // List returns the stored URLs and corresponding usernames. -func (h Osxkeychain) List(credsLabel string) (map[string]string, error) { - credsLabelC := C.CString(credsLabel) +func (h Osxkeychain) List() (map[string]string, error) { + credsLabelC := C.CString(credentials.CredsLabel) defer C.free(unsafe.Pointer(credsLabelC)) var pathsC **C.char diff --git a/osxkeychain/osxkeychain_darwin_test.go b/osxkeychain/osxkeychain_darwin_test.go index 9cfb108..406fe9b 100644 --- a/osxkeychain/osxkeychain_darwin_test.go +++ b/osxkeychain/osxkeychain_darwin_test.go @@ -7,13 +7,11 @@ import ( func TestOSXKeychainHelper(t *testing.T) { creds := &credentials.Credentials{ - Label: credentials.CredsLabel, ServerURL: "https://foobar.docker.io:2376/v1", Username: "foobar", Secret: "foobarbaz", } creds1 := &credentials.Credentials{ - Label: credentials.CredsLabel, ServerURL: "https://foobar.docker.io:2376/v2", Username: "foobarbaz", Secret: "foobar", @@ -36,14 +34,14 @@ func TestOSXKeychainHelper(t *testing.T) { t.Fatalf("expected %s, got %s\n", "foobarbaz", secret) } - auths, err := helper.List(credentials.CredsLabel) + auths, err := helper.List() if err != nil || len(auths) == 0 { t.Fatal(err) } helper.Add(creds1) defer helper.Delete(creds1.ServerURL) - newauths, err := helper.List(credentials.CredsLabel) + newauths, err := helper.List() if len(newauths)-len(auths) != 1 { if err == nil { t.Fatalf("Error: len(newauths): %d, len(auths): %d", len(newauths), len(auths)) diff --git a/secretservice/secretservice_linux.go b/secretservice/secretservice_linux.go index 26760bf..6b82a07 100644 --- a/secretservice/secretservice_linux.go +++ b/secretservice/secretservice_linux.go @@ -22,7 +22,7 @@ func (h Secretservice) Add(creds *credentials.Credentials) error { if creds == nil { return errors.New("missing credentials") } - credsLabel := C.CString(creds.Label) + credsLabel := C.CString(credentials.CredsLabel) defer C.free(unsafe.Pointer(credsLabel)) server := C.CString(creds.ServerURL) defer C.free(unsafe.Pointer(server)) @@ -82,8 +82,8 @@ func (h Secretservice) Get(serverURL string) (string, string, error) { } // List returns the stored URLs and corresponding usernames for a given credentials label -func (h Secretservice) List(credsLabel string) (map[string]string, error) { - credsLabelC := C.CString(credsLabel) +func (h Secretservice) List() (map[string]string, error) { + credsLabelC := C.CString(credentials.CredsLabel) defer C.free(unsafe.Pointer(credsLabelC)) var pathsC **C.char diff --git a/secretservice/secretservice_linux_test.go b/secretservice/secretservice_linux_test.go index 057efca..bd0caf3 100644 --- a/secretservice/secretservice_linux_test.go +++ b/secretservice/secretservice_linux_test.go @@ -10,7 +10,6 @@ func TestSecretServiceHelper(t *testing.T) { t.Skip("test requires gnome-keyring but travis CI doesn't have it") creds := &credentials.Credentials{ - Label: credentials.CredsLabel, ServerURL: "https://foobar.docker.io:2376/v1", Username: "foobar", Secret: "foobarbaz", @@ -37,12 +36,12 @@ func TestSecretServiceHelper(t *testing.T) { if err := helper.Delete(creds.ServerURL); err != nil { t.Fatal(err) } - auths, err := helper.List(credentials.CredsLabel) + auths, err := helper.List() if err != nil || len(auths) == 0 { t.Fatal(err) } helper.Add(creds) - if newauths, err := helper.List(credentials.CredsLabel); (len(newauths) - len(auths)) != 1 { + if newauths, err := helper.List(); (len(newauths) - len(auths)) != 1 { t.Fatal(err) } } diff --git a/wincred/wincred_windows.go b/wincred/wincred_windows.go index ce42d91..18e442e 100644 --- a/wincred/wincred_windows.go +++ b/wincred/wincred_windows.go @@ -16,7 +16,7 @@ func (h Wincred) Add(creds *credentials.Credentials) error { g.UserName = creds.Username g.CredentialBlob = []byte(creds.Secret) g.Persist = winc.PersistLocalMachine - g.Attributes = []winc.CredentialAttribute{{"label", []byte(creds.Label)}} + g.Attributes = []winc.CredentialAttribute{{"label", []byte(credentials.CredsLabel)}} return g.Write() } @@ -43,7 +43,7 @@ func (h Wincred) Get(serverURL string) (string, string, error) { } // List returns the stored URLs and corresponding usernames for a given credentials label. -func (h Wincred) List(credsLabel string) (map[string]string, error) { +func (h Wincred) List() (map[string]string, error) { creds, err := winc.List() if err != nil { return nil, err diff --git a/wincred/wincred_windows_test.go b/wincred/wincred_windows_test.go index 0c115e9..01f6e59 100644 --- a/wincred/wincred_windows_test.go +++ b/wincred/wincred_windows_test.go @@ -8,13 +8,11 @@ import ( func TestWinCredHelper(t *testing.T) { creds := &credentials.Credentials{ - Label: credentials.CredsLabel, ServerURL: "https://foobar.docker.io:2376/v1", Username: "foobar", Secret: "foobarbaz", } creds1 := &credentials.Credentials{ - Label: credentials.CredsLabel, ServerURL: "https://foobar.docker.io:2376/v2", Username: "foobarbaz", Secret: "foobar", @@ -38,14 +36,14 @@ func TestWinCredHelper(t *testing.T) { t.Fatalf("expected %s, got %s\n", "foobarbaz", secret) } - auths, err := helper.List(credentials.CredsLabel) + auths, err := helper.List() if err != nil || len(auths) == 0 { t.Fatal(err) } helper.Add(creds1) defer helper.Delete(creds1.ServerURL) - newauths, err := helper.List(credentials.CredsLabel) + newauths, err := helper.List() if err != nil { t.Fatal(err) }