pass: base64 encoded username with backward compatility

Signed-off-by: CrazyMax <[email protected]>

Author: crazy-max

pass/pass.go

@@ -89,14 +89,9 @@ func (p Pass) Add(creds *credentials.Credentials) error {
 		return errors.New("missing credentials")
 	}
 
-	encoded := base64.URLEncoding.EncodeToString([]byte(creds.ServerURL))
-
-	username := creds.Username
-	if strings.Contains(username, "/") {
-		username = base64.URLEncoding.EncodeToString([]byte(creds.Username))
-	}
-
-	_, err := p.runPass(creds.Secret, "insert", "-f", "-m", path.Join(PASS_FOLDER, encoded, username))
+	encodedServerURL := base64.URLEncoding.EncodeToString([]byte(creds.ServerURL))
+	encodedUsername := base64.URLEncoding.EncodeToString([]byte(creds.Username))
+	_, err := p.runPass(creds.Secret, "insert", "-f", "-m", path.Join(PASS_FOLDER, encodedServerURL, encodedUsername))
 	return err
 }
 
@@ -106,8 +101,8 @@ func (p Pass) Delete(serverURL string) error {
 		return errors.New("missing server url")
 	}
 
-	encoded := base64.URLEncoding.EncodeToString([]byte(serverURL))
-	_, err := p.runPass("", "rm", "-rf", path.Join(PASS_FOLDER, encoded))
+	encodedServerURL := base64.URLEncoding.EncodeToString([]byte(serverURL))
+	_, err := p.runPass("", "rm", "-rf", path.Join(PASS_FOLDER, encodedServerURL))
 	return err
 }
 
@@ -149,17 +144,16 @@ func (p Pass) Get(serverURL string) (string, string, error) {
 		return "", "", errors.New("missing server url")
 	}
 
-	encoded := base64.URLEncoding.EncodeToString([]byte(serverURL))
-
-	if _, err := os.Stat(path.Join(getPassDir(), PASS_FOLDER, encoded)); err != nil {
+	encodedServerURL := base64.URLEncoding.EncodeToString([]byte(serverURL))
+	if _, err := os.Stat(path.Join(getPassDir(), PASS_FOLDER, encodedServerURL)); err != nil {
 		if os.IsNotExist(err) {
 			return "", "", credentials.NewErrCredentialsNotFound()
 		}
 
 		return "", "", err
 	}
 
-	usernames, err := listPassDir(encoded)
+	usernames, err := listPassDir(encodedServerURL)
 	if err != nil {
 		return "", "", err
 	}
@@ -170,11 +164,10 @@ func (p Pass) Get(serverURL string) (string, string, error) {
 
 	actual := strings.TrimSuffix(usernames[0].Name(), ".gpg")
 	username := actual
-	decodedUsername, err := base64.URLEncoding.DecodeString(actual)
-	if err == nil {
+	if decodedUsername, err := base64.URLEncoding.DecodeString(actual); err == nil {
 		username = string(decodedUsername)
 	}
-	secret, err := p.runPass("", "show", path.Join(PASS_FOLDER, encoded, actual))
+	secret, err := p.runPass("", "show", path.Join(PASS_FOLDER, encodedServerURL, actual))
 	return username, secret, err
 }
 
@@ -206,9 +199,9 @@ func (p Pass) List() (map[string]string, error) {
 			return nil, fmt.Errorf("no usernames for %s", serverURL)
 		}
 
-		resp[string(serverURL)] = strings.TrimSuffix(usernames[0].Name(), ".gpg")
-		decodedUsername, err := base64.URLEncoding.DecodeString(strings.TrimSuffix(usernames[0].Name(), ".gpg"))
-		if err == nil {
+		username := strings.TrimSuffix(usernames[0].Name(), ".gpg")
+		resp[string(serverURL)] = username
+		if decodedUsername, err := base64.URLEncoding.DecodeString(username); err == nil {
 			resp[string(serverURL)] = string(decodedUsername)
 		}
 	}

pass/pass_test.go

@@ -4,13 +4,67 @@
 package pass
 
 import (
+	"encoding/base64"
+	"path"
 	"strings"
 	"testing"
 
 	"github.com/docker/docker-credential-helpers/credentials"
 )
 
 func TestPassHelper(t *testing.T) {
+	tests := []struct {
+		name  string
+		creds *credentials.Credentials
+	}{
+		{
+			name: "create nothing",
+			creds: &credentials.Credentials{
+				ServerURL: "https://foobar.docker.io:2376/v1",
+				Username:  "nothing",
+				Secret:    "isthebestmeshuggahalbum",
+			},
+		},
+		{
+			name: "create foo/bar",
+			creds: &credentials.Credentials{
+				ServerURL: "https://foobar.docker.io:2376/v1",
+				Username:  "foo/bar",
+				Secret:    "foobarbaz",
+			},
+		},
+	}
+
+	helper := Pass{}
+	_ = helper.CheckInitialized()
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if err := helper.Add(tc.creds); err != nil {
+				t.Error(err)
+			}
+			u, s, err := helper.Get(tc.creds.ServerURL)
+			if err != nil {
+				t.Error(err)
+			}
+			if u != tc.creds.Username {
+				t.Errorf("invalid username %s", u)
+			}
+			if s != tc.creds.Secret {
+				t.Errorf("invalid secret: %s", s)
+			}
+			if err = helper.Delete(tc.creds.ServerURL); err != nil {
+				t.Error(err)
+			}
+			if _, _, err = helper.Get(tc.creds.ServerURL); !credentials.IsErrCredentialsNotFound(err) {
+				t.Errorf("expected credentials not found, actual: %v", err)
+			}
+		})
+	}
+}
+
+func TestPassHelperBackwardCompat(t *testing.T) {
 	creds := &credentials.Credentials{
 		ServerURL: "https://foobar.docker.io:2376/v1",
 		Username:  "nothing",
@@ -20,26 +74,32 @@ func TestPassHelper(t *testing.T) {
 	helper := Pass{}
 	_ = helper.CheckInitialized()
 
+	// add a credential with the old format
+	encodedServerURL := base64.URLEncoding.EncodeToString([]byte(creds.ServerURL))
+	if _, err := helper.runPass(creds.Secret, "insert", "-f", "-m", path.Join(PASS_FOLDER, encodedServerURL, creds.Username)); err != nil {
+		t.Error(err)
+	}
+
 	if err := helper.Add(creds); err != nil {
-		t.Fatal(err)
+		t.Error(err)
 	}
 
 	u, s, err := helper.Get(creds.ServerURL)
 	if err != nil {
-		t.Fatal(err)
+		t.Error(err)
 	}
 	if u != creds.Username {
-		t.Fatalf("invalid username %s", u)
+		t.Errorf("invalid username %s", u)
 	}
 	if s != creds.Secret {
-		t.Fatalf("invalid secret: %s", s)
+		t.Errorf("invalid secret: %s", s)
 	}
 
 	if err = helper.Delete(creds.ServerURL); err != nil {
-		t.Fatal(err)
+		t.Error(err)
 	}
 	if _, _, err = helper.Get(creds.ServerURL); !credentials.IsErrCredentialsNotFound(err) {
-		t.Fatalf("expected credentials not found, actual: %v", err)
+		t.Errorf("expected credentials not found, actual: %v", err)
 	}
 }