diff --git a/internal/clierrors/missing_credentials.go b/internal/clierrors/missing_credentials.go index 2b0775177..cdeabd34e 100644 --- a/internal/clierrors/missing_credentials.go +++ b/internal/clierrors/missing_credentials.go @@ -1,12 +1,15 @@ package clierrors -import "fmt" +import ( + "strings" +) var _ ClientError = MissingCredentialsError{} type MissingCredentialsError struct { - ConfigFile string - ServiceName string + ConfigFile string + ServiceName string + KeyringError error // New field to track keyring issues } func (err MissingCredentialsError) ErrorCode() int { @@ -14,5 +17,46 @@ func (err MissingCredentialsError) ErrorCode() int { } func (err MissingCredentialsError) Error() string { - return fmt.Sprintf("user credentials not found, these must be set in config file (%s) or via environment variables or in a keyring (%s)", err.ConfigFile, err.ServiceName) + var msg strings.Builder + + msg.WriteString("Authentication credentials not found.\n\n") + + // Check if this is due to keyring issues + if err.KeyringError != nil && isKeyringError(err.KeyringError) { + msg.WriteString("System keyring is not accessible. ") + msg.WriteString("You can:\n") + msg.WriteString(" 1. Save token to file: upctl account login --with-token --save-to-file\n") + msg.WriteString(" 2. Set environment variable: export UPCLOUD_TOKEN=your-token\n") + msg.WriteString(" 3. Add to config file: " + err.ConfigFile + "\n\n") + msg.WriteString("Original error: " + err.KeyringError.Error()) + } else { + msg.WriteString("Please configure authentication using one of these methods:\n") + msg.WriteString(" 1. Login with token: upctl account login --with-token\n") + msg.WriteString(" 2. Set environment variable: export UPCLOUD_TOKEN=your-token\n") + msg.WriteString(" 3. Add to config file: " + err.ConfigFile + "\n") + + if err.ServiceName != "" { + msg.WriteString("\nNote: System keyring service '" + err.ServiceName + "' may not be available.\n") + msg.WriteString("Use --save-to-file flag with login command if keyring fails.") + } + } + + return msg.String() +} + +func isKeyringError(err error) bool { + if err == nil { + return false + } + errStr := strings.ToLower(err.Error()) + keyringIndicators := []string{ + "keyring", "secret", "dbus", "collection", + "unlock", "gnome-keyring", "kwallet", + } + for _, indicator := range keyringIndicators { + if strings.Contains(errStr, indicator) { + return true + } + } + return false } diff --git a/internal/commands/account/login.go b/internal/commands/account/login.go index d1e560059..31bf8b9c3 100644 --- a/internal/commands/account/login.go +++ b/internal/commands/account/login.go @@ -3,6 +3,7 @@ package account import ( "bufio" "fmt" + "os" "strings" "github.com/UpCloudLtd/upcloud-cli/v3/internal/commands" @@ -16,8 +17,9 @@ func LoginCommand() commands.Command { return &loginCommand{ BaseCommand: commands.New( "login", - "Configure an authentication token to the system keyring (EXPERIMENTAL) ", + "Configure an authentication token to the system keyring or credentials file", "upctl account login --with-token", + "upctl account login --with-token --save-to-file", ), } } @@ -25,13 +27,16 @@ func LoginCommand() commands.Command { type loginCommand struct { *commands.BaseCommand - withToken config.OptionalBoolean + withToken config.OptionalBoolean + saveToFile bool } // InitCommand implements Command.InitCommand func (s *loginCommand) InitCommand() { fs := &pflag.FlagSet{} config.AddToggleFlag(fs, &s.withToken, "with-token", false, "Read token from standard input.") + fs.BoolVar(&s.saveToFile, "save-to-file", false, + "Save token to credentials file (~/.config/upcloud/credentials) instead of keyring when keyring is unavailable") s.AddFlags(fs) // Require the with-token flag until we support using browser to authenticate. @@ -56,15 +61,64 @@ func (s *loginCommand) executeWithToken(exec commands.Executor) (output.Output, if err != nil { return nil, fmt.Errorf("failed to read token from standard input: %w", err) } + token = strings.TrimSpace(token) - msg := "Saving provided token to the system keyring." - exec.PushProgressStarted(msg) - err = config.SaveTokenToKeyring(strings.TrimSpace(token)) - if err != nil { + // Try keyring first (unless explicitly using file) + if !s.saveToFile { + msg := "Saving provided token to the system keyring." + exec.PushProgressStarted(msg) + err = config.SaveTokenToKeyring(token) + + if err == nil { + exec.PushProgressSuccess(msg) + return output.None{}, nil + } + + // Keyring failed + if config.IsKeyringError(err) { + var errMsg strings.Builder + errMsg.WriteString("System keyring is not accessible.\n\n") + + // Provide system-specific hints + if os.Getenv("WSL_DISTRO_NAME") != "" { + errMsg.WriteString(" (WSL detected - keyring typically doesn't work in WSL)\n") + } else if os.Getenv("SSH_CONNECTION") != "" { + errMsg.WriteString(" (SSH session detected - keyring may not be available)\n") + } else if os.Getenv("DISPLAY") == "" && os.Getenv("WAYLAND_DISPLAY") == "" { + errMsg.WriteString(" (No display detected - keyring requires GUI or D-Bus)\n") + } + + errMsg.WriteString("\nTo save your token, you can:\n") + errMsg.WriteString(" 1. Retry with file storage:\n") + errMsg.WriteString(" upctl account login --with-token --save-to-file\n\n") + errMsg.WriteString(" 2. Set environment variable:\n") + errMsg.WriteString(" export UPCLOUD_TOKEN=" + token + "\n\n") + errMsg.WriteString(" 3. Manually add to config file (~/.config/upctl.yaml):\n") + errMsg.WriteString(" token: " + token + "\n\n") + errMsg.WriteString("For security, tokens are not automatically saved to files without --save-to-file flag.") + + return commands.HandleError(exec, msg, fmt.Errorf("%s\n\nOriginal error: %w", errMsg.String(), err)) + } + + // Non-keyring error return commands.HandleError(exec, msg, err) } + // User explicitly requested file storage + msg := "Saving token to credentials file." + exec.PushProgressStarted(msg) + + credPath, saveErr := config.SaveTokenToCredentialsFile(token) + if saveErr != nil { + errMsg := fmt.Sprintf("Failed to save token to file: %v\n\nYou can still use environment variable:\n export UPCLOUD_TOKEN=%s", saveErr, token) + return commands.HandleError(exec, msg, fmt.Errorf(errMsg)) + } + exec.PushProgressSuccess(msg) + fmt.Fprintf(s.Cobra().OutOrStderr(), "\nToken saved to: %s\n", credPath) + fmt.Fprintf(s.Cobra().OutOrStderr(), "File permissions: 0600 (read/write for owner only)\n\n") + fmt.Fprintf(s.Cobra().OutOrStderr(), "This credentials file can be shared by other UpCloud tools.\n") + fmt.Fprintf(s.Cobra().OutOrStderr(), "You can now use: upctl account show\n") return output.None{}, nil } diff --git a/internal/config/config.go b/internal/config/config.go index b1237995b..feabcb7eb 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log/slog" + "os" "path/filepath" "regexp" @@ -64,11 +65,12 @@ type GlobalFlags struct { // Config holds the configuration for running upctl type Config struct { - viper *viper.Viper - flagSet *pflag.FlagSet - cancel context.CancelFunc - context context.Context //nolint: containedctx // This is where the top-level context is stored - GlobalFlags GlobalFlags + viper *viper.Viper + flagSet *pflag.FlagSet + cancel context.CancelFunc + context context.Context //nolint: containedctx // This is where the top-level context is stored + GlobalFlags GlobalFlags + keyringError error // Track keyring access errors for better error messages } // Load loads config and sets up service @@ -97,15 +99,43 @@ 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) + // Try to load from credentials file first (before trying keyring) + fileCreds, fileErr := LoadCredentialsFile() + if fileErr != nil && !os.IsNotExist(fileErr) { + logger.Debug("Could not load credentials file", "error", fileErr) + } else if fileCreds.Token != "" || (fileCreds.Username != "" && fileCreds.Password != "") { + // Merge file credentials (config/env takes precedence) + if v.GetString("token") == "" && fileCreds.Token != "" { + v.Set("token", fileCreds.Token) + logger.Debug("Using token from credentials file") + } + if v.GetString("username") == "" && fileCreds.Username != "" { + v.Set("username", fileCreds.Username) + } + if v.GetString("password") == "" && fileCreds.Password != "" { + v.Set("password", fileCreds.Password) + } + } + + // Only try keyring if we don't have credentials yet + needsKeyring := v.GetString("token") == "" && + (v.GetString("username") == "" || v.GetString("password") == "") + + if needsKeyring { + creds, err := credentials.Parse(credentials.Credentials{ + Username: v.GetString("username"), + Password: v.GetString("password"), + Token: v.GetString("token"), + }) + if err != nil { + // Store the keyring error for later use in error messages + s.keyringError = err + logger.Debug("Keyring access failed", "error", err) + } else { + v.Set("username", creds.Username) + v.Set("password", creds.Password) + v.Set("token", creds.Token) + } } v.Set("config", v.ConfigFileUsed()) @@ -228,7 +258,11 @@ func (s *Config) CreateService() (internal.AllServices, error) { if s.GetString("config") != "" { configDetails = fmt.Sprintf("used %s", s.GetString("config")) } - return nil, clierrors.MissingCredentialsError{ConfigFile: configDetails, ServiceName: credentials.KeyringServiceName} + return nil, clierrors.MissingCredentialsError{ + ConfigFile: configDetails, + ServiceName: credentials.KeyringServiceName, + KeyringError: s.keyringError, + } } configs := []client.ConfigFn{ diff --git a/internal/config/credentials_file.go b/internal/config/credentials_file.go new file mode 100644 index 000000000..6462ac098 --- /dev/null +++ b/internal/config/credentials_file.go @@ -0,0 +1,148 @@ +package config + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/adrg/xdg" + "github.com/gemalto/flume" + "gopkg.in/yaml.v3" +) + +const ( + // CredentialsFileName is the standard credentials file name for UpCloud tools + CredentialsFileName = "credentials" + // CredentialsFileMode is the required permissions for credentials file (user read/write only) + CredentialsFileMode = 0600 +) + +// GetCredentialsFilePath returns the standard path for UpCloud credentials file +func GetCredentialsFilePath() string { + // Check XDG_CONFIG_HOME/upcloud/credentials first + xdgPath := filepath.Join(xdg.ConfigHome, "upcloud", CredentialsFileName) + + // Fallback to ~/.upcloud/credentials for compatibility + homePath := filepath.Join(os.Getenv("HOME"), ".upcloud", CredentialsFileName) + + // Use XDG path if directory exists or if neither exists (prefer XDG for new installs) + if _, err := os.Stat(filepath.Dir(xdgPath)); err == nil { + return xdgPath + } + if _, err := os.Stat(filepath.Dir(homePath)); err == nil { + return homePath + } + + // Default to XDG for new installations + return xdgPath +} + +// CredentialsFileData represents the structure of the credentials file +type CredentialsFileData struct { + Token string `yaml:"token,omitempty"` + Username string `yaml:"username,omitempty"` + Password string `yaml:"password,omitempty"` +} + +// SaveTokenToCredentialsFile saves token to the standard credentials file with proper permissions +func SaveTokenToCredentialsFile(token string) (string, error) { + credPath := GetCredentialsFilePath() + credDir := filepath.Dir(credPath) + + // Create directory with secure permissions + if err := os.MkdirAll(credDir, 0700); err != nil { + return "", fmt.Errorf("failed to create credentials directory: %w", err) + } + + // Prepare credentials data + creds := CredentialsFileData{ + Token: token, + } + + data, err := yaml.Marshal(&creds) + if err != nil { + return "", fmt.Errorf("failed to marshal credentials: %w", err) + } + + // Write with secure permissions atomically + tempFile := credPath + ".tmp" + if err := os.WriteFile(tempFile, data, CredentialsFileMode); err != nil { + return "", fmt.Errorf("failed to write credentials file: %w", err) + } + + // Move atomically + if err := os.Rename(tempFile, credPath); err != nil { + os.Remove(tempFile) // Clean up + return "", fmt.Errorf("failed to save credentials file: %w", err) + } + + // Verify permissions (in case umask affected them) + if err := os.Chmod(credPath, CredentialsFileMode); err != nil { + // Log warning but don't fail (only if logger is available) + if logger := flume.FromContext(nil); logger != nil { + logger.Info("Could not set secure permissions on credentials file", "path", credPath) + } + } + + return credPath, nil +} + +// LoadCredentialsFile loads credentials from the standard file if it exists +func LoadCredentialsFile() (CredentialsFileData, error) { + credPath := GetCredentialsFilePath() + + // Check if file exists + info, err := os.Stat(credPath) + if err != nil { + if os.IsNotExist(err) { + return CredentialsFileData{}, nil // File doesn't exist, not an error + } + return CredentialsFileData{}, err + } + + // Warn if permissions are too open (but only if logger is available) + mode := info.Mode().Perm() + if mode != CredentialsFileMode { + // Try to get logger, but don't fail if it's not available + if logger := flume.FromContext(nil); logger != nil { + logger.Info("Credentials file has insecure permissions", + "path", credPath, + "current", fmt.Sprintf("%04o", mode), + "expected", fmt.Sprintf("%04o", CredentialsFileMode)) + } + } + + // Read file + data, err := os.ReadFile(credPath) + if err != nil { + return CredentialsFileData{}, fmt.Errorf("failed to read credentials file: %w", err) + } + + var creds CredentialsFileData + if err := yaml.Unmarshal(data, &creds); err != nil { + return CredentialsFileData{}, fmt.Errorf("failed to parse credentials file: %w", err) + } + + return creds, nil +} + +// IsKeyringError checks if an error is related to keyring access +func IsKeyringError(err error) bool { + if err == nil { + return false + } + errStr := err.Error() + keyringIndicators := []string{ + "keyring", "secret", "dbus", "collection", + "unlock", "gnome-keyring", "kwallet", + "windows credential", "failed to unlock", + } + errLower := strings.ToLower(errStr) + for _, indicator := range keyringIndicators { + if strings.Contains(errLower, indicator) { + return true + } + } + return false +} \ No newline at end of file