diff --git a/internal/auth/store.go b/internal/auth/store.go index f215013a..2d580c49 100644 --- a/internal/auth/store.go +++ b/internal/auth/store.go @@ -2,17 +2,38 @@ package auth import ( "encoding/json" + "errors" "fmt" "os" "path/filepath" "runtime" "sync" - "github.com/zalando/go-keyring" + keyringlib "github.com/zalando/go-keyring" ) const serviceName = "hey" +type credentialKeyring interface { + Get(service, user string) (string, error) + Set(service, user, password string) error + Delete(service, user string) error +} + +type systemCredentialKeyring struct{} + +func (systemCredentialKeyring) Get(service, user string) (string, error) { + return keyringlib.Get(service, user) +} + +func (systemCredentialKeyring) Set(service, user, password string) error { + return keyringlib.Set(service, user, password) +} + +func (systemCredentialKeyring) Delete(service, user string) error { + return keyringlib.Delete(service, user) +} + // Credentials holds OAuth tokens and metadata. type Credentials struct { AccessToken string `json:"access_token"` //nolint:gosec // G117: legitimate credential field @@ -29,6 +50,7 @@ type Store struct { useKeyring bool noKeyring bool fallbackDir string + keyring credentialKeyring } // NewStore creates a credential store. Keyring availability is probed lazily @@ -37,6 +59,7 @@ func NewStore(fallbackDir string) *Store { return &Store{ fallbackDir: fallbackDir, noKeyring: os.Getenv("HEY_NO_KEYRING") != "", + keyring: systemCredentialKeyring{}, } } @@ -45,10 +68,8 @@ func (s *Store) ensureInit() { if s.noKeyring { return } - testKey := "hey::test" - err := keyring.Set(serviceName, testKey, "test") - if err == nil { - _ = keyring.Delete(serviceName, testKey) + _, err := s.keyring.Get(serviceName, "hey::availability") + if err == nil || errors.Is(err, keyringlib.ErrNotFound) { s.useKeyring = true return } @@ -83,13 +104,13 @@ func (s *Store) Save(origin string, creds *Credentials) error { func (s *Store) Delete(origin string) error { s.ensureInit() if s.useKeyring { - return keyring.Delete(serviceName, key(origin)) + return s.keyring.Delete(serviceName, key(origin)) } return s.deleteFile(origin) } func (s *Store) loadFromKeyring(origin string) (*Credentials, error) { - data, err := keyring.Get(serviceName, key(origin)) + data, err := s.keyring.Get(serviceName, key(origin)) if err != nil { return nil, fmt.Errorf("credentials not found: %w", err) } @@ -106,7 +127,7 @@ func (s *Store) saveToKeyring(origin string, creds *Credentials) error { if err != nil { return err } - return keyring.Set(serviceName, key(origin), string(data)) + return s.keyring.Set(serviceName, key(origin), string(data)) } func (s *Store) credentialsPath() string { diff --git a/internal/auth/store_test.go b/internal/auth/store_test.go index 26c92fca..1f17a1c1 100644 --- a/internal/auth/store_test.go +++ b/internal/auth/store_test.go @@ -1,11 +1,36 @@ package auth import ( + "errors" "os" "path/filepath" "testing" + + keyringlib "github.com/zalando/go-keyring" ) +type recordingKeyring struct { + getCalls int + setCalls int + deleteCalls int + getErr error +} + +func (k *recordingKeyring) Get(_, _ string) (string, error) { + k.getCalls++ + return "", k.getErr +} + +func (k *recordingKeyring) Set(_, _, _ string) error { + k.setCalls++ + return nil +} + +func (k *recordingKeyring) Delete(_, _ string) error { + k.deleteCalls++ + return nil +} + func testStore(t *testing.T) *Store { t.Helper() t.Setenv("HEY_NO_KEYRING", "1") @@ -53,6 +78,34 @@ func TestLoadNotFound(t *testing.T) { } } +func TestKeyringAvailabilityCheckIsReadOnly(t *testing.T) { + t.Setenv("HEY_NO_KEYRING", "") + keyring := &recordingKeyring{getErr: keyringlib.ErrNotFound} + store := NewStore(t.TempDir()) + store.keyring = keyring + + if !store.UsingKeyring() { + t.Fatal("expected missing availability key to confirm keyring access") + } + if keyring.getCalls != 1 { + t.Errorf("Get calls = %d, want 1", keyring.getCalls) + } + if keyring.setCalls != 0 || keyring.deleteCalls != 0 { + t.Errorf("availability check mutated keyring: Set = %d, Delete = %d", keyring.setCalls, keyring.deleteCalls) + } +} + +func TestKeyringAvailabilityErrorUsesFileStore(t *testing.T) { + t.Setenv("HEY_NO_KEYRING", "") + keyring := &recordingKeyring{getErr: errors.New("keyring unavailable")} + store := NewStore(t.TempDir()) + store.keyring = keyring + + if store.UsingKeyring() { + t.Fatal("expected unavailable keyring to use file store") + } +} + func TestDeleteCredentials(t *testing.T) { s := testStore(t) origin := "https://app.hey.com"