Merge pull request #364 from akerouanton/fix-regression-v0.9.0

osxkeychain: list: return full server URIs
docker · Mar 4, 2025 · 9d6cddd · 9d6cddd
2 parents 833d2c3 + d8e34f8
commit 9d6cddd
Showing 2 changed files with 84 additions and 14 deletions.
diff --git a/osxkeychain/osxkeychain.go b/osxkeychain/osxkeychain.go
@@ -12,6 +12,8 @@ import "C"
 
 import (
 	"errors"
+	"net"
+	"net/url"
 	"strconv"
 
 	"github.com/docker/docker-credential-helpers/credentials"
@@ -121,10 +123,20 @@ func (h Osxkeychain) List() (map[string]string, error) {
 
 	resp := make(map[string]string)
 	for _, r := range res {
-		if r.Path == "" {
-			continue
+		proto := "http"
+		if r.Protocol == kSecProtocolTypeHTTPS {
+			proto = "https"
 		}
-		resp[r.Path] = r.Account
+		host := r.Server
+		if r.Port != 0 {
+			host = net.JoinHostPort(host, strconv.Itoa(int(r.Port)))
+		}
+		u := url.URL{
+			Scheme: proto,
+			Host:   host,
+			Path:   r.Path,
+		}
+		resp[u.String()] = r.Account
 	}
 	return resp, nil
 }

diff --git a/osxkeychain/osxkeychain_test.go b/osxkeychain/osxkeychain_test.go
@@ -15,11 +15,6 @@ func TestOSXKeychainHelper(t *testing.T) {
 		Username:  "foobar",
 		Secret:    "foobarbaz",
 	}
-	creds1 := &credentials.Credentials{
-		ServerURL: "https://foobar.example.com:2376/v2",
-		Username:  "foobarbaz",
-		Secret:    "foobar",
-	}
 	helper := Osxkeychain{}
 	if err := helper.Add(creds); err != nil {
 		t.Fatal(err)
@@ -43,19 +38,49 @@ func TestOSXKeychainHelper(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	if _, ok := auths[creds.ServerURL]; !ok {
+		t.Fatalf("server %s not found in list, got: %+v", creds.ServerURL, auths)
+	}
+
+	// Insert another token and check if it is in the list
+	creds1 := &credentials.Credentials{
+		ServerURL: "https://foobar.example.com:2376/v2",
+		Username:  "foobarbaz",
+		Secret:    "foobar",
+	}
 	helper.Add(creds1)
 	defer helper.Delete(creds1.ServerURL)
-	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))
-		}
-		t.Fatalf("Error: len(newauths): %d, len(auths): %d\n Error= %v", len(newauths), len(auths), err)
+
+	auths, err = helper.List()
+	if err != nil {
+		t.Fatalf("operation List failed: %+v", err)
+	}
+
+	if _, ok := auths[creds.ServerURL]; !ok {
+		t.Fatalf("server %s not found in list, got: %+v", creds.ServerURL, auths)
+	}
+	if _, ok := auths[creds1.ServerURL]; !ok {
+		t.Fatalf("server %s not found in list, got: %+v", creds1.ServerURL, auths)
 	}
 
+	// Delete the 1st token inserted
 	if err := helper.Delete(creds.ServerURL); err != nil {
 		t.Fatal(err)
 	}
+
+	auths, err = helper.List()
+	if err != nil {
+		t.Fatalf("operation List failed: %+v", err)
+	}
+
+	// First token should have been deleted
+	if _, ok := auths[creds.ServerURL]; ok {
+		t.Fatalf("server %s was not deleted, got: %+v", creds.ServerURL, auths)
+	}
+	// Second token should still be there
+	if _, ok := auths[creds1.ServerURL]; !ok {
+		t.Fatalf("server %s not found in list, got: %+v", creds1.ServerURL, auths)
+	}
 }
 
 // TestOSXKeychainHelperRetrieveAliases verifies that secrets can be accessed
@@ -116,6 +141,39 @@ func TestOSXKeychainHelperRetrieveAliases(t *testing.T) {
 	}
 }
 
+func TestOSXKeychainHelperStoreWithUncleanPath(t *testing.T) {
+	helper := Osxkeychain{}
+	creds := &credentials.Credentials{
+		ServerURL: "https://::1:8080//////location/../../hello",
+		Username:  "testuser",
+		Secret:    "testsecret",
+	}
+
+	// Clean store before and after the test.
+	defer helper.Delete(creds.ServerURL)
+	if err := helper.Delete(creds.ServerURL); err != nil && !credentials.IsErrCredentialsNotFound(err) {
+		t.Errorf("prepare: failed to delete '%s': %v", creds.ServerURL, err)
+	}
+
+	// Store the credentials
+	if err := helper.Add(creds); err != nil {
+		t.Fatalf("Error: failed to store credentials with unclean path %q: %s", creds.ServerURL, err)
+	}
+
+	// Retrieve and verify credentials
+	username, secret, err := helper.Get(creds.ServerURL)
+	if err != nil {
+		t.Fatalf("Error: failed to retrieve credentials with unclean path %q: %s", creds.ServerURL, err)
+	}
+
+	if username != creds.Username {
+		t.Errorf("Error: expected username %s, got %s", creds.Username, username)
+	}
+	if secret != creds.Secret {
+		t.Errorf("Error: expected secret %s, got %s", creds.Secret, secret)
+	}
+}
+
 // TestOSXKeychainHelperRetrieveStrict verifies that only matching secrets are
 // returned.
 func TestOSXKeychainHelperRetrieveStrict(t *testing.T) {