secrets: scope keyring by endpoint

Author: burmudar

internal/oauth/flow.go

@@ -34,9 +34,7 @@ const (
 	ScopeOfflineAccess string = "offline_access"
 	ScopeUserAll       string = "user:all"
 
-	// storeKeyFmt is the format of the key name that will be used to store a value
-	// typically the last element is the endpoint the value is for ie. src:oauth:https://sourcegraph.sourcegraph.com
-	storeKeyFmt string = "src:oauth:%s"
+	oauthKey string = "src:oauth"
 )
 
 var defaultScopes = []string{ScopeEmail, ScopeOfflineAccess, ScopeOpenID, ScopeProfile, ScopeUserAll}
@@ -394,12 +392,12 @@ func (t *Token) ExpiringIn(d time.Duration) bool {
 	return future.After(t.ExpiresAt)
 }
 
-func oauthKey(endpoint string) string {
-	return fmt.Sprintf(storeKeyFmt, endpoint)
-}
-
 func StoreToken(ctx context.Context, token *Token) error {
-	store, err := secrets.Open(ctx)
+	if token.Endpoint == "" {
+		return errors.New("token endpoint cannot be empty when storing the token")
+	}
+
+	store, err := secrets.Open(ctx, token.Endpoint)
 	if err != nil {
 		return err
 	}
@@ -408,21 +406,16 @@ func StoreToken(ctx context.Context, token *Token) error {
 		return errors.Wrap(err, "failed to marshal token")
 	}
 
-	if token.Endpoint == "" {
-		return errors.New("token endpoint cannot be empty when storing the token")
-	}
-
-	return store.Put(oauthKey(token.Endpoint), data)
+	return store.Put(oauthKey, data)
 }
 
 func LoadToken(ctx context.Context, endpoint string) (*Token, error) {
-	store, err := secrets.Open(ctx)
+	store, err := secrets.Open(ctx, endpoint)
 	if err != nil {
 		return nil, err
 	}
 
-	key := oauthKey(endpoint)
-	data, err := store.Get(key)
+	data, err := store.Get(oauthKey)
 	if err != nil {
 		return nil, err
 	}

internal/secrets/keyring.go

@@ -2,21 +2,31 @@ package secrets
 
 import (
 	"context"
+	"strings"
 
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 	"github.com/zalando/go-keyring"
 )
 
 var ErrSecretNotFound = errors.New("secret not found")
 
+const serviceNamePrefix = "Sourcegraph CLI"
+
 type keyringStore struct {
 	ctx         context.Context
 	serviceName string
 }
 
 // Open opens the system keyring for the Sourcegraph CLI.
-func Open(ctx context.Context) (*keyringStore, error) {
-	return &keyringStore{ctx: ctx, serviceName: "Sourcegraph CLI"}, nil
+func Open(ctx context.Context, endpoint string) (*keyringStore, error) {
+	endpoint = strings.TrimRight(strings.TrimSpace(endpoint), "/")
+	if endpoint == "" {
+		return nil, errors.New("endpoint cannot be empty")
+	}
+
+	serviceName := serviceNamePrefix + " <" + endpoint + ">"
+
+	return &keyringStore{ctx: ctx, serviceName: serviceName}, nil
 }
 
 // withContext runs fn in a goroutine and returns its result, or ctx.Err() if the context is cancelled first.

internal/secrets/keyring_test.go

@@ -0,0 +1,58 @@
+package secrets
+
+import (
+	"context"
+	"testing"
+)
+
+func TestOpen(t *testing.T) {
+	tests := []struct {
+		name            string
+		endpoint        string
+		wantServiceName string
+		wantErr         bool
+	}{
+		{
+			name:            "normalized endpoint",
+			endpoint:        " https://sourcegraph.example.com/ ",
+			wantServiceName: "Sourcegraph CLI <https://sourcegraph.example.com>",
+		},
+		{
+			name:            "normalized endpoint with path",
+			endpoint:        " https://sourcegraph.example.com/sourcegraph/ ",
+			wantServiceName: "Sourcegraph CLI <https://sourcegraph.example.com/sourcegraph>",
+		},
+		{
+			name:            "normalized endpoint with nested path",
+			endpoint:        "https://sourcegraph.example.com/custom/path///",
+			wantServiceName: "Sourcegraph CLI <https://sourcegraph.example.com/custom/path>",
+		},
+		{
+			name:     "empty endpoint",
+			endpoint: " / ",
+			wantErr:  true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			store, err := Open(context.Background(), test.endpoint)
+			if test.wantErr {
+				if err == nil {
+					t.Fatal("Open() error = nil, want non-nil")
+				}
+				if store != nil {
+					t.Fatalf("Open() store = %v, want nil", store)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("Open() error = %v, want nil", err)
+			}
+			if got := store.serviceName; got != test.wantServiceName {
+				t.Fatalf("Open() serviceName = %q, want %q", got, test.wantServiceName)
+			}
+		})
+	}
+}