Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
519 changes: 211 additions & 308 deletions internal/commands/auth/credentials.go

Large diffs are not rendered by default.

17 changes: 9 additions & 8 deletions internal/commands/auth/credentials_storage_fallback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/pingidentity/pingcli/internal/constants"
"github.com/pingidentity/pingcli/internal/customtypes"
"github.com/pingidentity/pingcli/internal/testing/testutils_koanf"
"github.com/stretchr/testify/mock"
"golang.org/x/oauth2"
Expand Down Expand Up @@ -84,11 +85,11 @@ func TestSaveTokenForMethod_FallsBackToFileWhenKeychainSaveFails(t *testing.T) {
if err != nil {
t.Fatalf("expected no error, got %v", err)
}
if location.Keychain {
t.Fatalf("expected Keychain=false, got true")
if location == customtypes.StorageLocationKeychain {
t.Fatalf("expected storage location to not be %s", customtypes.StorageLocationKeychain)
}
if !location.File {
t.Fatalf("expected File=true, got false")
if location != customtypes.StorageLocationFile {
t.Fatalf("expected storage location %s, got %s", customtypes.StorageLocationFile, location)
}

// Verify it actually wrote the expected file under HOME
Expand Down Expand Up @@ -137,11 +138,11 @@ func TestSaveTokenForMethod_UsesKeychainWhenAvailable(t *testing.T) {

mockStorage.AssertExpectations(t)

if !location.Keychain {
t.Fatalf("expected Keychain=true")
if location != customtypes.StorageLocationKeychain {
t.Fatalf("expected storage location %s, got %s", customtypes.StorageLocationKeychain, location)
}
if location.File {
t.Fatalf("expected File=false")
if location == customtypes.StorageLocationFile {
t.Fatalf("expected storage location to not be %s", customtypes.StorageLocationFile)
}

// File should not be written when keychain save succeeds
Expand Down
7 changes: 4 additions & 3 deletions internal/commands/auth/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
auth_internal "github.com/pingidentity/pingcli/internal/commands/auth"
"github.com/pingidentity/pingcli/internal/configuration/options"
"github.com/pingidentity/pingcli/internal/constants"
"github.com/pingidentity/pingcli/internal/customtypes"
"github.com/pingidentity/pingcli/internal/profiles"
"github.com/pingidentity/pingcli/internal/testing/testutils_koanf"
"golang.org/x/oauth2"
Expand Down Expand Up @@ -264,7 +265,7 @@ func TestClearToken(t *testing.T) {

// Test that ClearTokenForMethod doesn't panic when no token exists
// This should handle the case where keychain entry doesn't exist
_, err := auth_internal.ClearToken(testKey)
err := auth_internal.ClearToken(testKey)

// Should not error when no token exists (handles ErrNotFound)
if err != nil {
Expand Down Expand Up @@ -775,10 +776,10 @@ func TestSaveTokenForMethod_StorageTypeNone(t *testing.T) {
}

// Verify neither File nor Keychain storage was used
if location.File {
if location == customtypes.StorageLocationFile {
t.Error("Expected File storage to be false, got true")
}
if location.Keychain {
if location == customtypes.StorageLocationKeychain {
t.Error("Expected Keychain storage to be false, got true")
}

Expand Down
19 changes: 15 additions & 4 deletions internal/commands/auth/login_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,20 @@ func AuthLoginRunE(cmd *cobra.Command, args []string) error {
switch provider {
case customtypes.ENUM_AUTH_PROVIDER_PINGONE:
// Determine desired authentication method
deviceCodeStr, _ := profiles.GetOptionValue(options.AuthMethodDeviceCodeOption)
clientCredentialsStr, _ := profiles.GetOptionValue(options.AuthMethodClientCredentialsOption)
authorizationCodeStr, _ := profiles.GetOptionValue(options.AuthMethodAuthorizationCodeOption)
deviceCodeStr, err := profiles.GetOptionValue(options.AuthMethodDeviceCodeOption)
if err != nil {
return &errs.PingCLIError{Prefix: loginErrorPrefix, Err: err}
}

clientCredentialsStr, err := profiles.GetOptionValue(options.AuthMethodClientCredentialsOption)
if err != nil {
return &errs.PingCLIError{Prefix: loginErrorPrefix, Err: err}
}

authorizationCodeStr, err := profiles.GetOptionValue(options.AuthMethodAuthorizationCodeOption)
if err != nil {
return &errs.PingCLIError{Prefix: loginErrorPrefix, Err: err}
}

flagProvided := deviceCodeStr == "true" || clientCredentialsStr == "true" || authorizationCodeStr == "true"

Expand Down Expand Up @@ -186,7 +197,7 @@ func performLoginByConfiguredType(ctx context.Context, authType, profileName str
}

// displayLoginSuccess displays the successful login message
func displayLoginSuccess(token *oauth2.Token, newAuth bool, location StorageLocation, selectedMethod, profileName string) {
func displayLoginSuccess(token *oauth2.Token, newAuth bool, location customtypes.StorageLocationType, selectedMethod, profileName string) {
if newAuth {
// Build storage location message
storageMsg := formatStorageLocation(location)
Expand Down
33 changes: 24 additions & 9 deletions internal/commands/auth/logout_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,29 @@ import (
"github.com/spf13/cobra"
)

var (
logoutErrorPrefix = "failed to logout"
)

// AuthLogoutRunE implements the logout command logic, clearing credentials from both
// keychain and file storage. If no grant type flag is provided, clears all tokens.
// If a specific grant type flag is provided, clears only that method's token.
func AuthLogoutRunE(cmd *cobra.Command, args []string) error {
// Check if any grant type flags were provided
deviceCodeStr, _ := profiles.GetOptionValue(options.AuthMethodDeviceCodeOption)
clientCredentialsStr, _ := profiles.GetOptionValue(options.AuthMethodClientCredentialsOption)
authorizationCodeStr, _ := profiles.GetOptionValue(options.AuthMethodAuthorizationCodeOption)
deviceCodeStr, err := profiles.GetOptionValue(options.AuthMethodDeviceCodeOption)
if err != nil {
return &errs.PingCLIError{Prefix: logoutErrorPrefix, Err: err}
}

clientCredentialsStr, err := profiles.GetOptionValue(options.AuthMethodClientCredentialsOption)
if err != nil {
return &errs.PingCLIError{Prefix: logoutErrorPrefix, Err: err}
}

authorizationCodeStr, err := profiles.GetOptionValue(options.AuthMethodAuthorizationCodeOption)
if err != nil {
return &errs.PingCLIError{Prefix: logoutErrorPrefix, Err: err}
}

flagProvided := deviceCodeStr == "true" || clientCredentialsStr == "true" || authorizationCodeStr == "true"

Expand Down Expand Up @@ -71,9 +86,9 @@ func AuthLogoutRunE(cmd *cobra.Command, args []string) error {
}

// Clear only the token for the specified grant type
location, err := ClearToken(tokenKey)
err = ClearToken(tokenKey)
if err != nil {
return &errs.PingCLIError{Prefix: credentialsErrorPrefix, Err: fmt.Errorf("failed to clear %s credentials. in %s: %w", authType, formatStorageLocation(location), err)}
return &errs.PingCLIError{Prefix: credentialsErrorPrefix, Err: fmt.Errorf("failed to clear %s credentials: %w", authType, err)}
}

output.Success(fmt.Sprintf("Successfully logged out and cleared credentials from %s for service '%s' using profile '%s'.", authType, providerName, profileName), nil)
Expand Down Expand Up @@ -147,12 +162,12 @@ func GetAuthMethodKey(authMethod string) (string, error) {
}

// Build suffix to disambiguate across provider/grant/profile for both keychain and files
profileName, _ := profiles.GetOptionValue(options.RootActiveProfileOption)
if profileName == "" {
profileName, err := profiles.GetOptionValue(options.RootActiveProfileOption)
if err != nil || profileName == "" {
profileName = "default"
}
providerName, _ := profiles.GetOptionValue(options.AuthProviderOption)
if strings.TrimSpace(providerName) == "" {
providerName, err := profiles.GetOptionValue(options.AuthProviderOption)
if err != nil || strings.TrimSpace(providerName) == "" {
providerName = "pingone"
}
suffix := fmt.Sprintf("_%s_%s_%s", providerName, string(grantType), profileName)
Expand Down
33 changes: 17 additions & 16 deletions internal/commands/auth/use_keychain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

"github.com/pingidentity/pingcli/internal/customtypes"
"github.com/pingidentity/pingcli/internal/testing/testutils_koanf"
svcOAuth2 "github.com/pingidentity/pingone-go-client/oauth2"
"golang.org/x/oauth2"
Expand Down Expand Up @@ -40,8 +41,8 @@ func TestSaveTokenForMethod_WithKeychainDisabled(t *testing.T) {
}

// Verify location indicates file storage only
if !location.File || location.Keychain {
t.Errorf("Expected file storage only (File=true, Keychain=false), got %+v", location)
if location != customtypes.StorageLocationFile {
t.Errorf("Expected file storage only, got %+v", location)
}

// Verify token was saved to file
Expand Down Expand Up @@ -72,7 +73,7 @@ func TestSaveTokenForMethod_WithKeychainEnabled(t *testing.T) {
authMethod := "test-keychain-enabled"

t.Cleanup(func() {
_, _ = ClearToken(authMethod)
_ = ClearToken(authMethod)
})

// Save token - should try keychain first
Expand All @@ -84,9 +85,9 @@ func TestSaveTokenForMethod_WithKeychainEnabled(t *testing.T) {
} else {
t.Logf("Token saved to: %v", location)

// If it's expected to be in keychain (location.Keychain=true), we must manually save it there
// If it's expected to be in keychain (location == StorageLocationKeychain), we must manually save it there
// because SaveTokenForMethod doesn't actually perform the save (it assumes SDK did it)
if location.Keychain {
if location == customtypes.StorageLocationKeychain {
storage, sErr := svcOAuth2.NewKeychainStorage("pingcli", authMethod)
if sErr != nil {
if strings.Contains(sErr.Error(), "keychain") || strings.Contains(sErr.Error(), "freedesktop") {
Expand Down Expand Up @@ -174,7 +175,7 @@ func TestLoadTokenForMethod_FallbackToFileStorage(t *testing.T) {

t.Cleanup(func() {
_ = clearTokenFromFile(authMethod)
_, _ = ClearToken(authMethod)
_ = ClearToken(authMethod)
})

// Save token only to file storage (keychain disabled)
Expand Down Expand Up @@ -214,7 +215,7 @@ func TestShouldUseKeychain_Default(t *testing.T) {
authMethod := "test-default-keychain"

t.Cleanup(func() {
_, _ = ClearToken(authMethod)
_ = ClearToken(authMethod)
})

// Save token - should try keychain by default
Expand All @@ -224,8 +225,8 @@ func TestShouldUseKeychain_Default(t *testing.T) {
} else {
t.Logf("Token saved with default settings to: %v", location)

// If it's expected to be in keychain (location.Keychain=true), we must manually save it there
if location.Keychain {
// If it's expected to be in keychain (location == StorageLocationKeychain), we must manually save it there
if location == customtypes.StorageLocationKeychain {
storage, sErr := svcOAuth2.NewKeychainStorage("pingcli", authMethod)
if sErr != nil {
if strings.Contains(sErr.Error(), "keychain") || strings.Contains(sErr.Error(), "freedesktop") {
Expand Down Expand Up @@ -270,7 +271,7 @@ func TestClearToken_ClearsBothStorages(t *testing.T) {
authMethod := "test-clear-both-storages"

t.Cleanup(func() {
_, _ = ClearToken(authMethod)
_ = ClearToken(authMethod)
})

// Save to file storage directly
Expand All @@ -286,7 +287,7 @@ func TestClearToken_ClearsBothStorages(t *testing.T) {
}

// Clear token - should remove from both keychain and file storage
_, err = ClearToken(authMethod)
err = ClearToken(authMethod)
if err != nil {
t.Logf("ClearToken returned error (may be expected if keychain not available): %v", err)
}
Expand Down Expand Up @@ -334,7 +335,7 @@ func TestSaveTokenForMethod_FileStorageFallback(t *testing.T) {
authMethod := "test-save-fallback"

t.Cleanup(func() {
_, _ = ClearToken(authMethod)
_ = ClearToken(authMethod)
})

// Save token - will try keychain first (may succeed or fail depending on environment)
Expand All @@ -344,8 +345,8 @@ func TestSaveTokenForMethod_FileStorageFallback(t *testing.T) {
} else {
t.Logf("Token saved - fallback test to: %v", location)

// If it's expected to be in keychain (location.Keychain=true), we must manually save it there
if location.Keychain {
// If it's expected to be in keychain (location == StorageLocationKeychain), we must manually save it there
if location == customtypes.StorageLocationKeychain {
storage, sErr := svcOAuth2.NewKeychainStorage("pingcli", authMethod)
if sErr == nil {
// Don't skip here if save fails, because we are testing fallback?
Expand Down Expand Up @@ -427,8 +428,8 @@ func TestEnvironmentVariable_FileStorage(t *testing.T) {
}

// Verify location indicates file storage
if !location.File || location.Keychain {
t.Errorf("Expected file storage with env var (File=true, Keychain=false), got %+v", location)
if location != customtypes.StorageLocationFile {
t.Errorf("Expected file storage with env var, got %+v", location)
}

// Verify token was saved to file (since file-storage is true)
Expand Down
22 changes: 8 additions & 14 deletions internal/commands/auth/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package auth_internal

import (
"fmt"
"strings"

"github.com/pingidentity/pingcli/internal/configuration/options"
"github.com/pingidentity/pingcli/internal/customtypes"
Expand Down Expand Up @@ -44,19 +43,14 @@ func applyRegionConfiguration(cfg *config.Configuration) (*config.Configuration,
}

// formatStorageLocation returns a human-friendly message for where credentials were cleared
// based on StorageLocation.
func formatStorageLocation(location StorageLocation) string {
var locs []string
if location.Keychain {
locs = append(locs, "keychain")
}
if location.File {
locs = append(locs, "file storage")
}

if len(locs) == 0 {
// based on StorageLocationType.
func formatStorageLocation(location customtypes.StorageLocationType) string {
switch location {
case customtypes.StorageLocationKeychain:
return "keychain"
case customtypes.StorageLocationFile:
return "file storage"
default:
return "storage"
}

return strings.Join(locs, " and ")
}
20 changes: 16 additions & 4 deletions internal/commands/platform/export_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,18 @@ func initPingOneApiClient(ctx context.Context, pingcliVersion string) (err error
return &errs.PingCLIError{Prefix: exportErrorPrefix, Err: ErrNilContext}
}

workerClientID, _ := profiles.GetOptionValue(options.PingOneAuthenticationWorkerClientIDOption)
workerClientSecret, _ := profiles.GetOptionValue(options.PingOneAuthenticationWorkerClientSecretOption)
workerEnvironmentID, _ := profiles.GetOptionValue(options.PingOneAuthenticationWorkerEnvironmentIDOption)
workerClientID, err := profiles.GetOptionValue(options.PingOneAuthenticationWorkerClientIDOption)
if err != nil {
return &errs.PingCLIError{Prefix: exportErrorPrefix, Err: err}
}
workerClientSecret, err := profiles.GetOptionValue(options.PingOneAuthenticationWorkerClientSecretOption)
if err != nil {
return &errs.PingCLIError{Prefix: exportErrorPrefix, Err: err}
}
workerEnvironmentID, err := profiles.GetOptionValue(options.PingOneAuthenticationWorkerEnvironmentIDOption)
if err != nil {
return &errs.PingCLIError{Prefix: exportErrorPrefix, Err: err}
}
regionCode, err := profiles.GetOptionValue(options.PingOneRegionCodeOption)
if err != nil {
return &errs.PingCLIError{Prefix: exportErrorPrefix, Err: err}
Expand All @@ -357,7 +366,10 @@ func initPingOneApiClient(ctx context.Context, pingcliVersion string) (err error
return &errs.PingCLIError{Prefix: exportErrorPrefix, Err: ErrRegionCodeRequired}
}

authType, _ := profiles.GetOptionValue(options.PingOneAuthenticationTypeOption)
authType, err := profiles.GetOptionValue(options.PingOneAuthenticationTypeOption)
if err != nil {
return &errs.PingCLIError{Prefix: exportErrorPrefix, Err: err}
}

userAgent := fmt.Sprintf("pingcli/%s", pingcliVersion)
if v := strings.TrimSpace(os.Getenv("PINGCLI_PINGONE_APPEND_USER_AGENT")); v != "" {
Expand Down
2 changes: 2 additions & 0 deletions internal/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ const (
PingCliName = "pingcli"
PingCliDirName = ".pingcli"
CredentialsDirName = "credentials"
DefaultProfileName = "default"
OpenIDScope = "openid"
)
11 changes: 11 additions & 0 deletions internal/customtypes/storage_location_type.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright © 2026 Ping Identity Corporation

package customtypes

// StorageLocationType defines the type of storage where credentials are saved
type StorageLocationType string

const (
StorageLocationKeychain StorageLocationType = "keychain"
StorageLocationFile StorageLocationType = "file"
)