From ece4336b4d614c7a4eb9e3cf293300d64da8807f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20J=2E=20Gajda?= Date: Wed, 31 Dec 2025 11:42:36 +0100 Subject: [PATCH 1/3] feat(auth): add --no-keyring flag to skip keyring access Addresses issue #643 where users on WSL and ARM experience keyring access issues. Changes: - Add --no-keyring global flag to disable system keyring integration - Support no-keyring configuration option in upctl.yaml - When --no-keyring is set, save tokens to config file instead of keyring - Skip credentials.Parse() when keyring is disabled to avoid access attempts - Update account login command to save tokens to config when keyring disabled This allows users experiencing keyring issues to still use token authentication by storing tokens in the configuration file instead. --- internal/commands/account/login.go | 43 ++++++++++++++++---- internal/config/config.go | 64 +++++++++++++++++++++++++----- internal/core/core.go | 4 ++ 3 files changed, 95 insertions(+), 16 deletions(-) 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..05b86680a 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,21 @@ 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) + } } v.Set("config", v.ConfigFileUsed()) @@ -144,6 +151,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 { @@ -245,6 +257,40 @@ func SaveTokenToKeyring(token string) error { return keyring.Set(credentials.KeyringServiceName, credentials.KeyringTokenUser, token) } +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 { // Version was overridden during the build if Version != "dev" { 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, From 75ff2af37a9fac303b73595b51ecec6443888414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20J=2E=20Gajda?= Date: Wed, 31 Dec 2025 11:46:54 +0100 Subject: [PATCH 2/3] feat(auth): improve keyring error messages with helpful hints When keyring operations fail, provide clear hints about alternative options: - Using --no-keyring flag - Setting no-keyring in config file - Using environment variables This helps users quickly resolve keyring access issues on WSL, ARM, and other environments with problematic keyring support. --- internal/config/config.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 05b86680a..6d01a50c3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -113,6 +113,11 @@ func (s *Config) Load() error { 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) } } @@ -254,7 +259,30 @@ 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 { From 1273d770e137b7d1232438ebad8e3ae22c69edcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20J=2E=20Gajda?= Date: Wed, 31 Dec 2025 11:59:54 +0100 Subject: [PATCH 3/3] fix: apply go fmt formatting --- internal/config/config.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 6d01a50c3..de5f39c8a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -61,7 +61,7 @@ type GlobalFlags struct { OutputFormat string `valid:"in(human|json|yaml)"` NoColours OptionalBoolean ForceColours OptionalBoolean - NoKeyring bool `valid:"-"` + NoKeyring bool `valid:"-"` } // Config holds the configuration for running upctl @@ -275,10 +275,10 @@ func WrapKeyringError(err error) error { // 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") { + 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) }