diff --git a/internal/commands/account/login.go b/internal/commands/account/login.go index d1e560059..a0ee13780 100644 --- a/internal/commands/account/login.go +++ b/internal/commands/account/login.go @@ -9,6 +9,7 @@ import ( "github.com/UpCloudLtd/upcloud-cli/v3/internal/config" "github.com/UpCloudLtd/upcloud-cli/v3/internal/output" "github.com/spf13/pflag" + "github.com/spf13/viper" ) // LoginCommand creates the "account login" command @@ -57,14 +58,42 @@ func (s *loginCommand) executeWithToken(exec commands.Executor) (output.Output, return nil, fmt.Errorf("failed to read token from standard input: %w", err) } - msg := "Saving provided token to the system keyring." - exec.PushProgressStarted(msg) - err = config.SaveTokenToKeyring(strings.TrimSpace(token)) - if err != nil { - return commands.HandleError(exec, msg, err) - } + token = strings.TrimSpace(token) - exec.PushProgressSuccess(msg) + // Check if keyring is disabled via global flag or config + // Note: We check the flag directly because the config isn't loaded for offline commands + noKeyring, _ := s.Cobra().Root().PersistentFlags().GetBool("no-keyring") + // Also check if it was set in config via viper (if available) + if !noKeyring && s.Cobra().Root().PersistentFlags().Changed("config") { + // Config file was specified, check if no-keyring is set there + v := viper.New() + configFile, _ := s.Cobra().Root().PersistentFlags().GetString("config") + if configFile != "" { + v.SetConfigFile(configFile) + v.SetConfigType("yaml") + _ = v.ReadInConfig() + noKeyring = v.GetBool("no-keyring") + } + } + if noKeyring { + // Get config file path from flag if specified + configFile, _ := s.Cobra().Root().PersistentFlags().GetString("config") + msg := "Saving token to configuration file (keyring disabled)." + exec.PushProgressStarted(msg) + err = config.SaveTokenToConfigFile(token, configFile) + if err != nil { + return commands.HandleError(exec, msg, err) + } + exec.PushProgressSuccess(msg) + } else { + msg := "Saving provided token to the system keyring." + exec.PushProgressStarted(msg) + err = config.SaveTokenToKeyring(token) + if err != nil { + return commands.HandleError(exec, msg, err) + } + exec.PushProgressSuccess(msg) + } return output.None{}, nil } diff --git a/internal/config/config.go b/internal/config/config.go index 0649d22d0..de5f39c8a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -61,6 +61,7 @@ type GlobalFlags struct { OutputFormat string `valid:"in(human|json|yaml)"` NoColours OptionalBoolean ForceColours OptionalBoolean + NoKeyring bool `valid:"-"` } // Config holds the configuration for running upctl @@ -98,15 +99,26 @@ func (s *Config) Load() error { } } - creds, err := credentials.Parse(credentials.Credentials{ - Username: v.GetString("username"), - Password: v.GetString("password"), - Token: v.GetString("token"), - }) - if err == nil { - v.Set("username", creds.Username) - v.Set("password", creds.Password) - v.Set("token", creds.Token) + // Check for no-keyring setting from both flag and config file + noKeyring := s.GlobalFlags.NoKeyring || v.GetBool("no-keyring") + + // Only attempt to parse credentials with keyring if no-keyring is not set + if !noKeyring { + creds, err := credentials.Parse(credentials.Credentials{ + Username: v.GetString("username"), + Password: v.GetString("password"), + Token: v.GetString("token"), + }) + if err == nil { + v.Set("username", creds.Username) + v.Set("password", creds.Password) + v.Set("token", creds.Token) + } else { + // Log keyring error but don't fail - credentials might be provided differently + // Wrap the error with helpful hints for debugging + wrappedErr := WrapKeyringError(err) + logger.Debug("Failed to parse credentials from keyring", "error", wrappedErr) + } } v.Set("config", v.ConfigFileUsed()) @@ -144,6 +156,11 @@ func (s *Config) GetString(key string) string { return s.viper.GetString(key) } +// GetBool is a convenience method of getting a configuration value in the current namespace as a boolean +func (s *Config) GetBool(key string) bool { + return s.viper.GetBool(key) +} + // FlagByKey returns pflag.Flag associated with a key in config func (s *Config) FlagByKey(key string) *pflag.Flag { if s.flagSet == nil { @@ -242,7 +259,64 @@ func GetVersion() string { } func SaveTokenToKeyring(token string) error { - return keyring.Set(credentials.KeyringServiceName, credentials.KeyringTokenUser, token) + err := keyring.Set(credentials.KeyringServiceName, credentials.KeyringTokenUser, token) + if err != nil { + return WrapKeyringError(err) + } + return nil +} + +// WrapKeyringError adds helpful hints to keyring errors +func WrapKeyringError(err error) error { + if err == nil { + return nil + } + + // Check for common keyring error patterns + errStr := err.Error() + if strings.Contains(errStr, "failed to unlock") || + strings.Contains(errStr, "keyring") || + strings.Contains(errStr, "secret") || + strings.Contains(errStr, "dbus") || + strings.Contains(errStr, "collection") { + return fmt.Errorf("%w\n\nHint: If you're experiencing keyring issues, you can:\n 1. Use the --no-keyring flag to save credentials to config file instead\n 2. Set 'no-keyring: true' in your upctl.yaml config file\n 3. Use environment variables UPCLOUD_TOKEN for authentication", err) + } + + return err +} + +func SaveTokenToConfigFile(token string, configFile string) error { + // Read existing config or create new one + v := viper.New() + v.SetConfigType("yaml") + + if configFile != "" { + // Use the specified config file + v.SetConfigFile(configFile) + } else { + // Use default locations + v.SetConfigName("upctl") + v.AddConfigPath(xdg.ConfigHome) + v.AddConfigPath("$HOME/.config") + } + + // Try to read existing config (ignore errors if not found) + _ = v.ReadInConfig() + + // Set the token + v.Set("token", token) + + // Determine config file path for writing + if configFile == "" { + configFile = v.ConfigFileUsed() + if configFile == "" { + // Config file doesn't exist, create it in XDG config home + configFile = filepath.Join(xdg.ConfigHome, "upctl.yaml") + } + } + + // Write the config file + return v.WriteConfigAs(configFile) } func getVersion() string { diff --git a/internal/core/core.go b/internal/core/core.go index e50aef7e7..6b3a64319 100644 --- a/internal/core/core.go +++ b/internal/core/core.go @@ -102,6 +102,10 @@ func BuildRootCmd(conf *config.Config) cobra.Command { &conf.GlobalFlags.Debug, "debug", false, "Print out more verbose debug logs.", ) + flags.BoolVar( + &conf.GlobalFlags.NoKeyring, "no-keyring", false, + "Disable system keyring integration for credential storage.", + ) flags.DurationVarP( &conf.GlobalFlags.ClientTimeout, "client-timeout", "t", 0,