diff --git a/internal/tiger/common/connection.go b/internal/tiger/common/connection.go index 8b55ac40..901a80a3 100644 --- a/internal/tiger/common/connection.go +++ b/internal/tiger/common/connection.go @@ -100,6 +100,8 @@ func GetPassword(service api.Service, role string) (string, error) { return "", fmt.Errorf("no password found in keyring for this service") case *PgpassStorage: return "", fmt.Errorf("no password found in ~/.pgpass for this service") + case *AutoFallbackStorage: + return "", fmt.Errorf("no password found for this service (checked keyring and ~/.pgpass)") default: return "", fmt.Errorf("failed to retrieve password: %w", err) } diff --git a/internal/tiger/common/password_storage.go b/internal/tiger/common/password_storage.go index 89d2dc0b..41413288 100644 --- a/internal/tiger/common/password_storage.go +++ b/internal/tiger/common/password_storage.go @@ -2,6 +2,7 @@ package common import ( "bufio" + "errors" "fmt" "os" "path/filepath" @@ -330,6 +331,97 @@ func (n *NoStorage) GetStorageResult(err error, password string) PasswordStorage } } +// AutoFallbackStorage tries keyring first, falls back to pgpass on any error +type AutoFallbackStorage struct { + keyring *KeyringStorage + pgpass *PgpassStorage + lastMethod string // tracks which method was used for GetStorageResult +} + +func (a *AutoFallbackStorage) Save(service api.Service, password string, role string) error { + // Try keyring first + a.lastMethod = "keyring" + keyringErr := a.keyring.Save(service, password, role) + if keyringErr == nil { + return nil + } + + // Any keyring error -> fall back to pgpass + a.lastMethod = "pgpass" + pgpassErr := a.pgpass.Save(service, password, role) + if pgpassErr == nil { + return nil + } + + // Both failed - return combined error + return errors.Join( + fmt.Errorf("keyring: %w", keyringErr), + fmt.Errorf("pgpass: %w", pgpassErr), + ) +} + +func (a *AutoFallbackStorage) Get(service api.Service, role string) (string, error) { + // Try keyring first + password, keyringErr := a.keyring.Get(service, role) + if keyringErr == nil { + return password, nil + } + + // Any keyring error -> try pgpass + password, pgpassErr := a.pgpass.Get(service, role) + if pgpassErr == nil { + return password, nil + } + + // Both failed + return "", errors.Join( + fmt.Errorf("keyring: %w", keyringErr), + fmt.Errorf("pgpass: %w", pgpassErr), + ) +} + +func (a *AutoFallbackStorage) Remove(service api.Service, role string) error { + // Try to remove from both (best effort) + keyringErr := a.keyring.Remove(service, role) + pgpassErr := a.pgpass.Remove(service, role) + + // Success if removed from at least one location + if keyringErr == nil || pgpassErr == nil { + return nil + } + // Both failed + return errors.Join( + fmt.Errorf("keyring: %w", keyringErr), + fmt.Errorf("pgpass: %w", pgpassErr), + ) +} + +func (a *AutoFallbackStorage) GetStorageResult(err error, password string) PasswordStorageResult { + if err != nil { + return PasswordStorageResult{ + Success: false, + Method: a.lastMethod, + Message: fmt.Sprintf("Failed to save password: %s", sanitizeErrorMessage(err, password)), + } + } + + // Return method-specific success message + switch a.lastMethod { + case "keyring": + return PasswordStorageResult{ + Success: true, + Method: "keyring", + Message: "Password saved to system keyring for automatic authentication", + } + default: + return PasswordStorageResult{ + Success: true, + Method: "pgpass", + Message: "Password saved to ~/.pgpass for automatic authentication", + } + } +} + // GetPasswordStorage returns the appropriate PasswordStorage implementation based on configuration func GetPasswordStorage() PasswordStorage { storageMethod := viper.GetString("password_storage") @@ -341,7 +433,10 @@ func GetPasswordStorage() PasswordStorage { case "none": return &NoStorage{} default: - return &KeyringStorage{} // Default to keyring + return &AutoFallbackStorage{ + keyring: &KeyringStorage{}, + pgpass: &PgpassStorage{}, + } } } diff --git a/internal/tiger/common/password_storage_test.go b/internal/tiger/common/password_storage_test.go index b04d28f2..fe70221e 100644 --- a/internal/tiger/common/password_storage_test.go +++ b/internal/tiger/common/password_storage_test.go @@ -295,8 +295,9 @@ func TestGetPasswordStorage(t *testing.T) { {"keyring", "keyring", "*common.KeyringStorage"}, {"pgpass", "pgpass", "*common.PgpassStorage"}, {"none", "none", "*common.NoStorage"}, - {"default", "", "*common.KeyringStorage"}, // Default case - {"invalid", "invalid", "*common.KeyringStorage"}, // Falls back to default + {"auto", "auto", "*common.AutoFallbackStorage"}, + {"default", "", "*common.AutoFallbackStorage"}, + {"invalid", "invalid", "*common.AutoFallbackStorage"}, // Falls back to auto } for _, tt := range tests { @@ -937,3 +938,55 @@ func TestSanitizeErrorMessage(t *testing.T) { }) } } + +// Test AutoFallbackStorage using pgpass as fallback (since keyring may not be available in CI) +func TestAutoFallbackStorage_Integration(t *testing.T) { + // Set up test service name for keyring + config.SetTestServiceName(t) + + // Create a temporary directory for pgpass + tempDir := t.TempDir() + originalHome := os.Getenv("HOME") + os.Setenv("HOME", tempDir) + defer os.Setenv("HOME", originalHome) + + storage := &AutoFallbackStorage{ + keyring: &KeyringStorage{}, + pgpass: &PgpassStorage{}, + } + service := createTestService("auto-fallback-test-service") + password := "test-password-auto" + role := "tsdbadmin" + + // Test Save - should succeed using either keyring or pgpass + err := storage.Save(service, password, role) + if err != nil { + t.Fatalf("AutoFallbackStorage.Save() failed: %v", err) + } + + // Verify lastMethod is set to either keyring or pgpass + if storage.lastMethod != "keyring" && storage.lastMethod != "pgpass" { + t.Errorf("AutoFallbackStorage.lastMethod = %q, want 'keyring' or 'pgpass'", storage.lastMethod) + } + + // Test Get - should retrieve the password + retrieved, err := storage.Get(service, role) + if err != nil { + t.Fatalf("AutoFallbackStorage.Get() failed: %v", err) + } + if retrieved != password { + t.Errorf("AutoFallbackStorage.Get() = %q, want %q", retrieved, password) + } + + // Test Remove - should succeed + err = storage.Remove(service, role) + if err != nil { + t.Fatalf("AutoFallbackStorage.Remove() failed: %v", err) + } + + // Verify password is gone + _, err = storage.Get(service, role) + if err == nil { + t.Error("AutoFallbackStorage.Get() should fail after Remove()") + } +} diff --git a/internal/tiger/config/config.go b/internal/tiger/config/config.go index ad261725..d843a747 100644 --- a/internal/tiger/config/config.go +++ b/internal/tiger/config/config.go @@ -66,7 +66,7 @@ const ( DefaultDocsMCPURL = "https://mcp.tigerdata.com/docs" DefaultGatewayURL = "https://console.cloud.timescale.com/api" DefaultOutput = "table" - DefaultPasswordStorage = "keyring" + DefaultPasswordStorage = "auto" DefaultReleasesURL = "https://cli.tigerdata.com" DefaultVersionCheckInterval = 24 * time.Hour ) @@ -368,8 +368,8 @@ func (c *Config) UpdateField(key string, value any) (any, error) { if !ok { return nil, fmt.Errorf("password_storage must be string, got %T", value) } - if s != "keyring" && s != "pgpass" && s != "none" { - return nil, fmt.Errorf("invalid password_storage value: %s (must be keyring, pgpass, or none)", s) + if s != "auto" && s != "keyring" && s != "pgpass" && s != "none" { + return nil, fmt.Errorf("invalid password_storage value: %s (must be auto, keyring, pgpass, or none)", s) } c.PasswordStorage = s validated = s