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 <jsand@google.com>

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.
+// List executes a program to list server credentials in the native store.
-func List(program ProgramFunc) error {
+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
 }

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)
+	}
 }

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{}
+	return json.NewEncoder(writer).Encode(accts)
-	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
 }

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) {

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
+	// List returns the stored serverURLs and their associated usernames.
-	// associated usernames from the OS store as a
+	List() (map[string]string, error)
-	// list of strings
-	List() ([]string, []string, error)
 }

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)
+	resp := make(map[string]string)
-	accts := make([]string, listLen)
-	at := 0
 	for i := 0; i < listLen; i++ {
 		if C.GoString(pathTmp[i]) == "0" {
 			continue
 		}
-		paths[at] = C.GoString(pathTmp[i])
+		resp[C.GoString(pathTmp[i])] = C.GoString(acctTmp[i])
-		accts[at] = C.GoString(acctTmp[i])
-		at = at + 1
 	}
-	paths = paths[:at]
+	return resp, nil
-	accts = accts[:at]
-	C.freeListData(&pathsC, listLenC)
-	C.freeListData(&acctsC, listLenC)
-	return paths, accts, nil
 }
 
 func splitServer(serverURL string) (*C.struct_Server, error) {

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()
+	auths, err := helper.List()
-	if err != nil || len(paths) == 0 || len(accts) == 0 {
+	if err != nil || len(auths) == 0 {
 		t.Fatal(err)
 	}
 
 	helper.Add(creds1)
 	defer helper.Delete(creds1.ServerURL)
-	newpaths, newaccts, err := helper.List()
+	newauths, err := helper.List()
-	if len(newpaths)-len(paths) != 1 || len(newaccts)-len(accts) != 1 {
+	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 {

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)
+	resp := make(map[string]string)
-	accts := make([]string, listLen)
 	for i := 0; i < listLen; i++ {
-		paths[i] = C.GoString(pathTmp[i])
+		resp[C.GoString(pathTmp[i])] = C.GoString(acctTmp[i])
-		accts[i] = C.GoString(acctTmp[i])
 	}
-	C.freeListData(&pathsC, listLenC)
+
-	C.freeListData(&acctsC, listLenC)
+	return resp, nil
-	return paths, accts, nil
 }

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()
+	auths, err := helper.List()
-	if err != nil || len(paths) == 0 || len(accts) == 0 {
+	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)
 	}
 }

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
+		resp[creds[i].TargetName] = creds[i].UserName
-		accts[i] = creds[i].UserName
 	}
-	return paths, accts, nil
+	return resp, nil
 }

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()
+	auths, err := helper.List()
-	if err != nil || len(paths) == 0 || len(accts) == 0 {
+	if err != nil || len(auths) == 0 {
 		t.Fatal(err)
 	}
 
 	helper.Add(creds1)
 	defer helper.Delete(creds1.ServerURL)
-	newpaths, newaccts, err := helper.List()
+	newauths, err := helper.List()
-	if len(newpaths)-len(paths) != 1 || len(newaccts)-len(accts) != 1 {
+	if err != nil {
-		if err == nil {
+		t.Fatal(err)
-			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(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())
+
+	if len(newauths)-len(auths) != 1 {
+		if err == nil {
+			t.Fatalf("Error: len(newauths): %d, len(auths): %d", len(newauths), len(auths))
+		}
+		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 {