From c45d9e9e282a9309666d687da5323dc175c155d3 Mon Sep 17 00:00:00 2001 From: Jake Sanders Date: Fri, 2 Sep 2016 15:12:18 -0700 Subject: [PATCH] Implement client.List, change list API []string, []string -> map[string]string because the other APIs assume a 1:1 correspondence Signed-off-by: Jake Sanders --- client/client.go | 16 +++++++++++----- client/client_test.go | 10 ++++++++-- credentials/credentials.go | 22 ++-------------------- credentials/credentials_test.go | 4 ++-- credentials/helper.go | 6 ++---- osxkeychain/osxkeychain_darwin.go | 23 ++++++++++------------- osxkeychain/osxkeychain_darwin_test.go | 12 ++++++------ secretservice/secretservice_linux.go | 19 ++++++++++--------- secretservice/secretservice_linux_test.go | 6 +++--- wincred/wincred_windows.go | 12 +++++++----- wincred/wincred_windows_test.go | 16 ++++++++++------ 11 files changed, 71 insertions(+), 75 deletions(-) diff --git a/client/client.go b/client/client.go index 469754c..d081396 100644 --- a/client/client.go +++ b/client/client.go @@ -68,14 +68,20 @@ func Erase(program ProgramFunc, serverURL string) error { return nil } -// List executes a program to remove the server credentials from the native store. -func List(program ProgramFunc) error { +// List executes a program to list server credentials in the native store. +func List(program ProgramFunc) (map[string]string, error) { cmd := program("list") - cmd.Input(strings.NewReader("garbage")) + cmd.Input(strings.NewReader("unused")) out, err := cmd.Output() if err != nil { t := strings.TrimSpace(string(out)) - return fmt.Errorf("error listing credentials - err: %v, out: `%s`", err, t) + return nil, fmt.Errorf("error listing credentials - err: %v, out: `%s`", err, t) } - return nil + + var resp map[string]string + if err = json.NewDecoder(bytes.NewReader(out)).Decode(&resp); err != nil { + return nil, err + } + + return resp, nil } diff --git a/client/client_test.go b/client/client_test.go index 1f8b4bd..e5f1bf2 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -13,6 +13,7 @@ import ( const ( validServerAddress = "https://index.docker.io/v1" + validUsername = "linus" validServerAddress2 = "https://example.com:5002" invalidServerAddress = "https://foobar.example.com" missingCredsAddress = "https://missing.docker.io/v1" @@ -71,7 +72,7 @@ func (m *mockProgram) Output() ([]byte, error) { return []byte("error storing credentials"), errProgramExited } case "list": - return []byte(`{"Path":"e237574ae22fd53ddb9490dc1f72139946fd5372d42ba54d1eeb3ae5068fd22b","Username":"http://example.com/collections\u003cnotary_key\u003eSnapshot"}`), nil + return []byte(fmt.Sprintf(`{"%s": "%s"}`, validServerAddress, validUsername)), nil } @@ -195,7 +196,12 @@ func TestErase(t *testing.T) { } func TestList(t *testing.T) { - if err := List(mockProgramFn); err != nil { + auths, err := List(mockProgramFn) + if err != nil { t.Fatal(err) } + + if username, exists := auths[validServerAddress]; !exists || username != validUsername { + t.Fatalf("auths[%s] returned %s, %t; expected %s, %t", validServerAddress, username, exists, validUsername, true) + } } diff --git a/credentials/credentials.go b/credentials/credentials.go index 6d7259b..3c4eec7 100644 --- a/credentials/credentials.go +++ b/credentials/credentials.go @@ -17,11 +17,6 @@ type Credentials struct { Secret string } -type KeyData struct { - Path string - Username string -} - // Serve initializes the credentials helper and parses the action argument. // This function is designed to be called from a command line interface. // It uses os.Args[1] as the key for the action. @@ -138,22 +133,9 @@ 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 { - paths, accts, err := helper.List() + accts, err := helper.List() if err != nil { return err } - keyDataList := []KeyData{} - for index := 0; index < len(paths); index++ { - keyDataObj := KeyData{ - Path: paths[index], - Username: accts[index], - } - keyDataList = append([]KeyData{keyDataObj}, keyDataList...) - } - buffer := new(bytes.Buffer) - if err := json.NewEncoder(buffer).Encode(keyDataList); err != nil { - return err - } - fmt.Fprint(writer, buffer.String()) - return nil + return json.NewEncoder(writer).Encode(accts) } diff --git a/credentials/credentials_test.go b/credentials/credentials_test.go index 6476012..6b8bbe1 100644 --- a/credentials/credentials_test.go +++ b/credentials/credentials_test.go @@ -36,9 +36,9 @@ func (m *memoryStore) Get(serverURL string) (string, string, error) { return c.Username, c.Secret, nil } -func (m *memoryStore) List() ([]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, nil + return nil, nil } func TestStore(t *testing.T) { diff --git a/credentials/helper.go b/credentials/helper.go index 785fcd7..135acd2 100644 --- a/credentials/helper.go +++ b/credentials/helper.go @@ -9,8 +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 serverURLs of keys and their - // associated usernames from the OS store as a - // list of strings - List() ([]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 28c316d..bd3874c 100644 --- a/osxkeychain/osxkeychain_darwin.go +++ b/osxkeychain/osxkeychain_darwin.go @@ -94,7 +94,8 @@ func (h Osxkeychain) Get(serverURL string) (string, string, error) { return user, pass, nil } -func (h Osxkeychain) List() ([]string, []string, error) { +// List returns the stored URLs and corresponding usernames. +func (h Osxkeychain) List() (map[string]string, error) { var pathsC **C.char defer C.free(unsafe.Pointer(pathsC)) var acctsC **C.char @@ -104,29 +105,25 @@ func (h Osxkeychain) List() ([]string, []string, error) { if errMsg != nil { defer C.free(unsafe.Pointer(errMsg)) goMsg := C.GoString(errMsg) - return nil, nil, errors.New(goMsg) + return nil, errors.New(goMsg) } + + defer C.freeListData(&pathsC, listLenC) + defer C.freeListData(&acctsC, listLenC) + var listLen int listLen = int(listLenC) pathTmp := (*[1 << 30]*C.char)(unsafe.Pointer(pathsC))[:listLen:listLen] acctTmp := (*[1 << 30]*C.char)(unsafe.Pointer(acctsC))[:listLen:listLen] //taking the array of c strings into go while ignoring all the stuff irrelevant to credentials-helper - paths := make([]string, listLen) - accts := make([]string, listLen) - at := 0 + resp := make(map[string]string) for i := 0; i < listLen; i++ { if C.GoString(pathTmp[i]) == "0" { continue } - paths[at] = C.GoString(pathTmp[i]) - accts[at] = C.GoString(acctTmp[i]) - at = at + 1 + resp[C.GoString(pathTmp[i])] = C.GoString(acctTmp[i]) } - paths = paths[:at] - accts = accts[:at] - C.freeListData(&pathsC, listLenC) - C.freeListData(&acctsC, listLenC) - return paths, accts, nil + return resp, nil } func splitServer(serverURL string) (*C.struct_Server, error) { diff --git a/osxkeychain/osxkeychain_darwin_test.go b/osxkeychain/osxkeychain_darwin_test.go index 7dcfb06..406fe9b 100644 --- a/osxkeychain/osxkeychain_darwin_test.go +++ b/osxkeychain/osxkeychain_darwin_test.go @@ -34,19 +34,19 @@ func TestOSXKeychainHelper(t *testing.T) { t.Fatalf("expected %s, got %s\n", "foobarbaz", secret) } - paths, accts, err := helper.List() - if err != nil || len(paths) == 0 || len(accts) == 0 { + auths, err := helper.List() + if err != nil || len(auths) == 0 { t.Fatal(err) } helper.Add(creds1) defer helper.Delete(creds1.ServerURL) - newpaths, newaccts, err := helper.List() - if len(newpaths)-len(paths) != 1 || len(newaccts)-len(accts) != 1 { + newauths, err := helper.List() + if len(newauths)-len(auths) != 1 { if err == nil { - t.Fatalf("Error: len(newpaths): %d, len(paths): %d\n len(newaccts): %d, len(accts): %d\n Error= %s", len(newpaths), len(paths), len(newaccts), len(accts), "") + t.Fatalf("Error: len(newauths): %d, len(auths): %d", len(newauths), len(auths)) } - t.Fatalf("Error: len(newpaths): %d, len(paths): %d\n len(newaccts): %d, len(accts): %d\n Error= %s", len(newpaths), len(paths), len(newaccts), len(accts), err.Error()) + t.Fatalf("Error: len(newauths): %d, len(auths): %d\n Error= %v", len(newauths), len(auths), err) } if err := helper.Delete(creds.ServerURL); err != nil { diff --git a/secretservice/secretservice_linux.go b/secretservice/secretservice_linux.go index 9178f90..f3264ce 100644 --- a/secretservice/secretservice_linux.go +++ b/secretservice/secretservice_linux.go @@ -79,7 +79,8 @@ func (h Secretservice) Get(serverURL string) (string, string, error) { return user, pass, nil } -func (h Secretservice) List() ([]string, []string, error) { +// List returns the stored URLs and corresponding usernames. +func (h Secretservice) List() (map[string]string, error) { var pathsC **C.char defer C.free(unsafe.Pointer(pathsC)) var acctsC **C.char @@ -88,18 +89,18 @@ func (h Secretservice) List() ([]string, []string, error) { err := C.list(&pathsC, &acctsC, &listLenC) if err != nil { defer C.free(unsafe.Pointer(err)) - return nil, nil, errors.New("Error from list function in secretservice_linux.c likely due to error in secretservice library") + return nil, errors.New("Error from list function in secretservice_linux.c likely due to error in secretservice library") } + defer C.freeListData(&pathsC, listLenC) + defer C.freeListData(&acctsC, listLenC) + listLen := int(listLenC) pathTmp := (*[1 << 30]*C.char)(unsafe.Pointer(pathsC))[:listLen:listLen] acctTmp := (*[1 << 30]*C.char)(unsafe.Pointer(acctsC))[:listLen:listLen] - paths := make([]string, listLen) - accts := make([]string, listLen) + resp := make(map[string]string) for i := 0; i < listLen; i++ { - paths[i] = C.GoString(pathTmp[i]) - accts[i] = C.GoString(acctTmp[i]) + resp[C.GoString(pathTmp[i])] = C.GoString(acctTmp[i]) } - C.freeListData(&pathsC, listLenC) - C.freeListData(&acctsC, listLenC) - return paths, accts, nil + + return resp, nil } diff --git a/secretservice/secretservice_linux_test.go b/secretservice/secretservice_linux_test.go index 89bd196..bd0caf3 100644 --- a/secretservice/secretservice_linux_test.go +++ b/secretservice/secretservice_linux_test.go @@ -36,12 +36,12 @@ func TestSecretServiceHelper(t *testing.T) { if err := helper.Delete(creds.ServerURL); err != nil { t.Fatal(err) } - paths, accts, err := helper.List() - if err != nil || len(paths) == 0 || len(accts) == 0 { + auths, err := helper.List() + if err != nil || len(auths) == 0 { t.Fatal(err) } helper.Add(creds) - if newpaths, newaccts, err := helper.List(); (len(newpaths)-len(paths)) != 1 || (len(newaccts)-len(accts)) != 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 8b36814..fc8da99 100644 --- a/wincred/wincred_windows.go +++ b/wincred/wincred_windows.go @@ -38,16 +38,18 @@ func (h Wincred) Get(serverURL string) (string, string, error) { return g.UserName, string(g.CredentialBlob), nil } -func (h Wincred) List() ([]string, []string, error) { +// List returns the stored URLs and corresponding usernames. +func (h Wincred) List() (map[string]string, error) { creds, err := winc.List() paths := make([]string, len(creds)) accts := make([]string, len(creds)) if err != nil { - return nil, nil, err + return nil, err } + + resp := make(map[string]string) for i := range creds { - paths[i] = creds[i].TargetName - accts[i] = creds[i].UserName + resp[creds[i].TargetName] = creds[i].UserName } - return paths, accts, nil + return resp, nil } diff --git a/wincred/wincred_windows_test.go b/wincred/wincred_windows_test.go index 1634156..01f6e59 100644 --- a/wincred/wincred_windows_test.go +++ b/wincred/wincred_windows_test.go @@ -36,19 +36,23 @@ func TestWinCredHelper(t *testing.T) { t.Fatalf("expected %s, got %s\n", "foobarbaz", secret) } - paths, accts, err := helper.List() - if err != nil || len(paths) == 0 || len(accts) == 0 { + auths, err := helper.List() + if err != nil || len(auths) == 0 { t.Fatal(err) } helper.Add(creds1) defer helper.Delete(creds1.ServerURL) - newpaths, newaccts, err := helper.List() - if len(newpaths)-len(paths) != 1 || len(newaccts)-len(accts) != 1 { + newauths, err := helper.List() + if err != nil { + t.Fatal(err) + } + + if len(newauths)-len(auths) != 1 { if err == nil { - t.Fatalf("Error: len(newpaths): %d, len(paths): %d\n len(newaccts): %d, len(accts): %d\n Error= %s", len(newpaths), len(paths), len(newaccts), len(accts), "") + t.Fatalf("Error: len(newauths): %d, len(auths): %d", len(newauths), len(auths)) } - t.Fatalf("Error: len(newpaths): %d, len(paths): %d\n len(newaccts): %d, len(accts): %d\n Error= %s", len(newpaths), len(paths), len(newaccts), len(accts), err.Error()) + t.Fatalf("Error: len(newauths): %d, len(auths): %d\n Error= %v", len(newauths), len(auths), err) } if err := helper.Delete(creds.ServerURL); err != nil {