From ce584f88b31fe1b864ec18949504c5d7820fa993 Mon Sep 17 00:00:00 2001 From: Alex Collins Date: Sat, 22 Nov 2025 09:59:35 -0800 Subject: [PATCH] refactor: replace internal keyring wrapper with direct library usage - Update go.mod with replace directive for github.com/kitproj/go-keyring v0.2.10 - Replace internal/keyring package with direct usage of github.com/zalando/go-keyring - Remove internal/keyring package and all platform-specific implementations - Fix test to handle nil error case properly --- go.mod | 3 +- go.sum | 8 +- internal/config/config.go | 2 +- internal/keyring/keyring.go | 22 ---- internal/keyring/keyring_darwin.go | 24 ----- internal/keyring/keyring_linux.go | 96 ----------------- internal/keyring/keyring_test.go | 153 ---------------------------- internal/keyring/keyring_windows.go | 24 ----- mcp_test.go | 1 + 9 files changed, 6 insertions(+), 327 deletions(-) delete mode 100644 internal/keyring/keyring.go delete mode 100644 internal/keyring/keyring_darwin.go delete mode 100644 internal/keyring/keyring_linux.go delete mode 100644 internal/keyring/keyring_test.go delete mode 100644 internal/keyring/keyring_windows.go diff --git a/go.mod b/go.mod index 9450c35..2ad4e45 100644 --- a/go.mod +++ b/go.mod @@ -9,8 +9,9 @@ require ( golang.org/x/term v0.36.0 ) +replace github.com/zalando/go-keyring => github.com/kitproj/go-keyring v0.2.10 + require ( - al.essio.dev/pkg/shellescape v1.5.1 // indirect github.com/bahlo/generic-list-go v0.2.0 // indirect github.com/buger/jsonparser v1.1.1 // indirect github.com/danieljoos/wincred v1.2.2 // indirect diff --git a/go.sum b/go.sum index b35d652..a18d94a 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho= -al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890= github.com/andygrunwald/go-jira v1.17.0 h1:bbu5H676l6MaNcV6A7VDIAjIOQVgzNGEhNAwNI/Cjgo= github.com/andygrunwald/go-jira v1.17.0/go.mod h1:tiZsPUu9824bwcI2BUXatE4hJbs9rUOif0nv1lkq1hQ= github.com/bahlo/generic-list-go v0.2.0 h1:5sz/EEAK+ls5wF+NeqDpk5+iNdMDXrh3z3nPnH1Wvgk= @@ -23,13 +21,13 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= -github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= -github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/invopop/jsonschema v0.13.0 h1:KvpoAJWEjR3uD9Kbm2HWJmqsEaHt8lBUpd0qHcIi21E= github.com/invopop/jsonschema v0.13.0/go.mod h1:ffZ5Km5SWWRAIN6wbDXItl95euhFz2uON45H2qjYt+0= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= +github.com/kitproj/go-keyring v0.2.10 h1:ZjwJOV8mIG8pYrWSGxtdGj62sYg6uAdMVa8kPbeMicQ= +github.com/kitproj/go-keyring v0.2.10/go.mod h1:yhtJhnoQt44WWzPtoc44uANw82MILytMeDc01iKC8cM= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -56,8 +54,6 @@ github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/ github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= -github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s= -github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI= golang.org/x/sys v0.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ= golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/term v0.36.0 h1:zMPR+aF8gfksFprF/Nc/rd1wRS1EI6nDBGyWAvDzx2Q= diff --git a/internal/config/config.go b/internal/config/config.go index af3c65a..6933469 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,7 +6,7 @@ import ( "os" "path/filepath" - "github.com/kitproj/jira-cli/internal/keyring" + "github.com/zalando/go-keyring" ) const ( diff --git a/internal/keyring/keyring.go b/internal/keyring/keyring.go deleted file mode 100644 index 9306010..0000000 --- a/internal/keyring/keyring.go +++ /dev/null @@ -1,22 +0,0 @@ -package keyring - -// Provider defines the interface for token storage -type Provider interface { - // Set stores a token for the given service and user - Set(service, user, token string) error - // Get retrieves a token for the given service and user - Get(service, user string) (string, error) -} - -// provider is the platform-specific implementation -var provider Provider - -// Set stores a token using the platform-specific provider -func Set(service, user, token string) error { - return provider.Set(service, user, token) -} - -// Get retrieves a token using the platform-specific provider -func Get(service, user string) (string, error) { - return provider.Get(service, user) -} diff --git a/internal/keyring/keyring_darwin.go b/internal/keyring/keyring_darwin.go deleted file mode 100644 index 9c1e751..0000000 --- a/internal/keyring/keyring_darwin.go +++ /dev/null @@ -1,24 +0,0 @@ -//go:build darwin - -package keyring - -import ( - "github.com/zalando/go-keyring" -) - -func init() { - provider = &systemKeyringProvider{} -} - -// systemKeyringProvider implements token storage using the system keyring -type systemKeyringProvider struct{} - -// Set stores a token in the system keyring -func (s *systemKeyringProvider) Set(service, user, token string) error { - return keyring.Set(service, user, token) -} - -// Get retrieves a token from the system keyring -func (s *systemKeyringProvider) Get(service, user string) (string, error) { - return keyring.Get(service, user) -} diff --git a/internal/keyring/keyring_linux.go b/internal/keyring/keyring_linux.go deleted file mode 100644 index e3ea3ad..0000000 --- a/internal/keyring/keyring_linux.go +++ /dev/null @@ -1,96 +0,0 @@ -//go:build linux - -package keyring - -import ( - "encoding/json" - "fmt" - "os" - "path/filepath" -) - -const tokenFile = "token" - -func init() { - provider = &fileProvider{} -} - -// fileProvider implements token storage using files -type fileProvider struct{} - -// Set stores a token in a file -func (f *fileProvider) Set(service, user, token string) error { - tokenPath, err := getTokenFilePath() - if err != nil { - return err - } - - // Create config directory if it doesn't exist - configDirPath := filepath.Dir(tokenPath) - if err := os.MkdirAll(configDirPath, 0700); err != nil { - return fmt.Errorf("failed to create config directory: %w", err) - } - - // Store as a simple key-value map - tokens := make(map[string]string) - - // Try to load existing tokens - if data, err := os.ReadFile(tokenPath); err == nil { - _ = json.Unmarshal(data, &tokens) - } - - // Add or update token for this user - tokens[user] = token - - // Save to file - data, err := json.MarshalIndent(tokens, "", " ") - if err != nil { - return fmt.Errorf("failed to marshal tokens: %w", err) - } - - // Write with 0600 permissions (only owner can read/write) - if err := os.WriteFile(tokenPath, data, 0600); err != nil { - return fmt.Errorf("failed to write token file: %w", err) - } - - return nil -} - -// Get retrieves a token from a file -func (f *fileProvider) Get(service, user string) (string, error) { - tokenPath, err := getTokenFilePath() - if err != nil { - return "", err - } - - data, err := os.ReadFile(tokenPath) - if err != nil { - if os.IsNotExist(err) { - return "", fmt.Errorf("token not found") - } - return "", fmt.Errorf("failed to read token file: %w", err) - } - - tokens := make(map[string]string) - if err := json.Unmarshal(data, &tokens); err != nil { - return "", fmt.Errorf("failed to parse token file: %w", err) - } - - token, ok := tokens[user] - if !ok { - return "", fmt.Errorf("token not found for user: %s", user) - } - - return token, nil -} - -// getTokenFilePath returns the path to the token file -func getTokenFilePath() (string, error) { - configDirPath, err := os.UserConfigDir() - if err != nil { - return "", fmt.Errorf("failed to get config directory: %w", err) - } - - tokenPath := filepath.Join(configDirPath, "jira-cli", tokenFile) - return tokenPath, nil -} diff --git a/internal/keyring/keyring_test.go b/internal/keyring/keyring_test.go deleted file mode 100644 index 867a0d5..0000000 --- a/internal/keyring/keyring_test.go +++ /dev/null @@ -1,153 +0,0 @@ -package keyring - -import ( - "os" - "path/filepath" - "testing" -) - -// TestSetGet tests basic set and get operations -func TestSetGet(t *testing.T) { - // Create a temporary directory for testing - tmpDir := t.TempDir() - - // Override the config directory - origConfigDir := os.Getenv("XDG_CONFIG_HOME") - os.Setenv("XDG_CONFIG_HOME", tmpDir) - defer func() { - if origConfigDir != "" { - os.Setenv("XDG_CONFIG_HOME", origConfigDir) - } else { - os.Unsetenv("XDG_CONFIG_HOME") - } - }() - - testService := "test-service" - testUser := "test.atlassian.net" - testToken := "test-token-12345" - - // Test Set - err := Set(testService, testUser, testToken) - if err != nil { - t.Fatalf("Failed to set token: %v", err) - } - - // Test Get - retrievedToken, err := Get(testService, testUser) - if err != nil { - t.Fatalf("Failed to get token: %v", err) - } - - if retrievedToken != testToken { - t.Errorf("Expected token %q, got %q", testToken, retrievedToken) - } -} - -// TestMultipleUsers tests storing tokens for multiple users -func TestMultipleUsers(t *testing.T) { - // Create a temporary directory for testing - tmpDir := t.TempDir() - - // Override the config directory - origConfigDir := os.Getenv("XDG_CONFIG_HOME") - os.Setenv("XDG_CONFIG_HOME", tmpDir) - defer func() { - if origConfigDir != "" { - os.Setenv("XDG_CONFIG_HOME", origConfigDir) - } else { - os.Unsetenv("XDG_CONFIG_HOME") - } - }() - - testService := "test-service" - users := map[string]string{ - "host1.atlassian.net": "token1", - "host2.atlassian.net": "token2", - "host3.atlassian.net": "token3", - } - - // Set all tokens - for user, token := range users { - err := Set(testService, user, token) - if err != nil { - t.Fatalf("Failed to set token for %s: %v", user, err) - } - } - - // Get and verify all tokens - for user, expectedToken := range users { - retrievedToken, err := Get(testService, user) - if err != nil { - t.Fatalf("Failed to get token for %s: %v", user, err) - } - if retrievedToken != expectedToken { - t.Errorf("For user %s, expected token %q, got %q", user, expectedToken, retrievedToken) - } - } -} - -// TestGetNotFound tests error handling when token doesn't exist -func TestGetNotFound(t *testing.T) { - // Create a temporary directory for testing - tmpDir := t.TempDir() - - // Override the config directory - origConfigDir := os.Getenv("XDG_CONFIG_HOME") - os.Setenv("XDG_CONFIG_HOME", tmpDir) - defer func() { - if origConfigDir != "" { - os.Setenv("XDG_CONFIG_HOME", origConfigDir) - } else { - os.Unsetenv("XDG_CONFIG_HOME") - } - }() - - // Try to get from non-existent file - _, err := Get("test-service", "nonexistent.atlassian.net") - if err == nil { - t.Error("Expected error when getting non-existent token, got nil") - } -} - -// TestFilePermissions tests that token file has correct permissions (Linux only) -func TestFilePermissions(t *testing.T) { - // Skip on non-Linux platforms since file implementation is Linux-specific - if provider == nil { - t.Skip("Skipping file permissions test on non-Linux platform") - } - - // Check if provider is fileProvider (Linux) - if _, ok := provider.(*fileProvider); !ok { - t.Skip("Skipping file permissions test on non-Linux platform") - } - - tmpDir := t.TempDir() - - origConfigDir := os.Getenv("XDG_CONFIG_HOME") - os.Setenv("XDG_CONFIG_HOME", tmpDir) - defer func() { - if origConfigDir != "" { - os.Setenv("XDG_CONFIG_HOME", origConfigDir) - } else { - os.Unsetenv("XDG_CONFIG_HOME") - } - }() - - // Set a token - err := Set("test-service", "test.atlassian.net", "test-token") - if err != nil { - t.Fatalf("Failed to set token: %v", err) - } - - // Check file permissions - tokenPath := filepath.Join(tmpDir, "jira-cli", "token") - info, err := os.Stat(tokenPath) - if err != nil { - t.Fatalf("Failed to stat token file: %v", err) - } - - expectedPerm := os.FileMode(0600) - if info.Mode().Perm() != expectedPerm { - t.Errorf("Expected file permissions %v, got %v", expectedPerm, info.Mode().Perm()) - } -} diff --git a/internal/keyring/keyring_windows.go b/internal/keyring/keyring_windows.go deleted file mode 100644 index 5e3a410..0000000 --- a/internal/keyring/keyring_windows.go +++ /dev/null @@ -1,24 +0,0 @@ -//go:build windows - -package keyring - -import ( - "github.com/zalando/go-keyring" -) - -func init() { - provider = &systemKeyringProvider{} -} - -// systemKeyringProvider implements token storage using the system keyring -type systemKeyringProvider struct{} - -// Set stores a token in the system keyring -func (s *systemKeyringProvider) Set(service, user, token string) error { - return keyring.Set(service, user, token) -} - -// Get retrieves a token from the system keyring -func (s *systemKeyringProvider) Get(service, user string) (string, error) { - return keyring.Get(service, user) -} diff --git a/mcp_test.go b/mcp_test.go index 01b1074..9f3084d 100644 --- a/mcp_test.go +++ b/mcp_test.go @@ -50,6 +50,7 @@ func TestRun_MCPServerMissingConfig(t *testing.T) { if err == nil { t.Error("Expected error for missing configuration, got nil") + return } if !strings.Contains(err.Error(), "JIRA host must be configured") {