pass: return correct error, and ignore empty stores on list

commit 2fc2313 changed the errors returned
by the pass credentials-helper to use a errCredentialsNotFound. This error
string is used in the client to distinguish a "not found" error from other
errors. (see [client.Get][1]).

However, there were additional second code-paths that returned a custom error,
which would not be detected as a "not found" error, resulting in an error when
logging out;

    Removing login credentials for https://index.docker.io/v1/
    WARNING: could not erase credentials:
    https://index.docker.io/v1/: error erasing credentials - err: exit status 1, out: `error getting credentials - err: exit status 1, out: `no usernames for https://index.docker.io/v1/``

This patch:

- updates Pass.Get() to return a errCredentialsNotFound if no credentials
  were found
- updates Pass.List() to not return an error if any of the domains had no
  credentials stored.

[1]: https://github.com/docker/docker-credential-helpers/blob/73b9e5d51f8dc9f598e08a0f2171c5d5a828e76b/client/client.go#L51-L55

Signed-off-by: Sebastiaan van Stijn <[email protected]>

Author: thaJeztah

pass/pass.go

@@ -158,7 +158,7 @@ func (p Pass) Get(serverURL string) (string, string, error) {
 	}
 
 	if len(usernames) < 1 {
-		return "", "", fmt.Errorf("no usernames for %s", serverURL)
+		return "", "", credentials.NewErrCredentialsNotFound()
 	}
 
 	actual := strings.TrimSuffix(usernames[0].Name(), ".gpg")
@@ -191,7 +191,7 @@ func (p Pass) List() (map[string]string, error) {
 		}
 
 		if len(usernames) < 1 {
-			return nil, fmt.Errorf("no usernames for %s", serverURL)
+			continue
 		}
 
 		resp[string(serverURL)] = strings.TrimSuffix(usernames[0].Name(), ".gpg")

pass/pass_test.go

@@ -3,6 +3,9 @@
 package pass
 
 import (
+	"encoding/base64"
+	"os"
+	"path"
 	"strings"
 	"testing"
 
@@ -116,6 +119,75 @@ func TestPassHelperList(t *testing.T) {
 	}
 }
 
+// TestPassHelperWithEmptyServer verifies that empty directories (servers
+// without credentials) are ignored, but still returns credentials for other
+// servers.
+func TestPassHelperWithEmptyServer(t *testing.T) {
+	helper := Pass{}
+	if err := helper.checkInitialized(); err != nil {
+		t.Error(err)
+	}
+
+	creds := []*credentials.Credentials{
+		{
+			ServerURL: "https://myreqistry.example.com:2375/v1",
+			Username:  "foo",
+			Secret:    "isthebestmeshuggahalbum",
+		},
+		{
+			ServerURL: "https://index.example.com/v1//access-token",
+		},
+	}
+
+	t.Cleanup(func() {
+		for _, cred := range creds {
+			_ = helper.Delete(cred.ServerURL)
+		}
+	})
+
+	for _, cred := range creds {
+		if cred.Username != "" {
+			if err := helper.Add(cred); err != nil {
+				t.Error(err)
+			}
+		} else {
+			// No credentials; create an empty directory for this server.
+			serverURL := base64.URLEncoding.EncodeToString([]byte(cred.ServerURL))
+			p := path.Join(getPassDir(), PASS_FOLDER, serverURL)
+			if err := os.Mkdir(p, 0o755); err != nil {
+				t.Error(err)
+			}
+		}
+	}
+
+	credsList, err := helper.List()
+	if err != nil {
+		t.Error(err)
+	}
+	if len(credsList) == 0 {
+		t.Error("expected credentials to be returned, but got none")
+	}
+	for _, cred := range creds {
+		if cred.Username != "" {
+			userName, secret, err := helper.Get(cred.ServerURL)
+			if err != nil {
+				t.Error(err)
+			}
+			if userName != cred.Username {
+				t.Errorf("expected username %q, actual: %q", cred.Username, userName)
+			}
+			if secret != cred.Secret {
+				t.Errorf("expected secret %q, actual: %q", cred.Secret, secret)
+			}
+		} else {
+			_, _, err := helper.Get(cred.ServerURL)
+			if !credentials.IsErrCredentialsNotFound(err) {
+				t.Errorf("expected credentials not found, actual: %v", err)
+			}
+		}
+	}
+}
+
 func TestMissingCred(t *testing.T) {
 	helper := Pass{}
 	if _, _, err := helper.Get("garbage"); !credentials.IsErrCredentialsNotFound(err) {