From 14062807fb765048491cee1389379d647223c6c9 Mon Sep 17 00:00:00 2001 From: wesleymccollam Date: Tue, 22 Apr 2025 14:17:09 -0400 Subject: [PATCH 1/3] fix: validate profile when using feedback command, disallow the use of activeprofile as a profile name, validate profile value before active profile value --- cmd/config/add_profile_test.go | 10 +++++++++ cmd/feedback/feedback_test.go | 7 +++++++ internal/commands/config/set_internal.go | 4 ++++ internal/commands/config/set_internal_test.go | 15 +++++++++++++ internal/profiles/koanf.go | 4 ++++ internal/profiles/validate.go | 21 +++++++++++++++---- 6 files changed, 57 insertions(+), 4 deletions(-) diff --git a/cmd/config/add_profile_test.go b/cmd/config/add_profile_test.go index ddb57223..8280a73c 100644 --- a/cmd/config/add_profile_test.go +++ b/cmd/config/add_profile_test.go @@ -68,3 +68,13 @@ func TestConfigAddProfileCmd_InvalidSetActiveValue(t *testing.T) { "--set-active=invalid-value") testutils.CheckExpectedError(t, err, &expectedErrorPattern) } + +// Test config add profile command fails when using activeprofile as the profile name +func TestConfigSetCmd_InvalidAddActiveProfile(t *testing.T) { + expectedErrorPattern := `^failed to add profile: invalid profile name: '.*'. name cannot be the same as the active profile key$` + err := testutils_cobra.ExecutePingcli(t, "config", "add-profile", + "--name", "activeprofile", + "--description", "test description", + "--set-active=true") + testutils.CheckExpectedError(t, err, &expectedErrorPattern) +} diff --git a/cmd/feedback/feedback_test.go b/cmd/feedback/feedback_test.go index 45903fc5..45b97d07 100644 --- a/cmd/feedback/feedback_test.go +++ b/cmd/feedback/feedback_test.go @@ -5,6 +5,7 @@ package feedback_test import ( "testing" + "github.com/pingidentity/pingcli/internal/configuration/options" "github.com/pingidentity/pingcli/internal/testing/testutils" "github.com/pingidentity/pingcli/internal/testing/testutils_cobra" ) @@ -37,3 +38,9 @@ func TestFeedbackCmd_InvalidFlag(t *testing.T) { err := testutils_cobra.ExecutePingcli(t, "feedback", "--invalid") testutils.CheckExpectedError(t, err, &expectedErrorPattern) } + +// Test Feedback Command with valid profile +func TestFeedbackCmd_Profile(t *testing.T) { + err := testutils_cobra.ExecutePingcli(t, "feedback", "--"+options.RootProfileOption.CobraParamName, "default") + testutils.CheckExpectedError(t, err, nil) +} diff --git a/internal/commands/config/set_internal.go b/internal/commands/config/set_internal.go index 53e7034a..a6e08889 100644 --- a/internal/commands/config/set_internal.go +++ b/internal/commands/config/set_internal.go @@ -106,6 +106,10 @@ func parseKeyValuePair(kvPair string) (string, string, error) { return "", "", fmt.Errorf("invalid assignment format '%s'. Expect 'key=value' format", kvPair) } + if strings.EqualFold(parsedInput[0], options.RootActiveProfileOption.KoanfKey) { + return "", "", fmt.Errorf("invalid assignment. Please use the 'pingcli config set active-profile ' command to set the active profile") + } + return parsedInput[0], parsedInput[1], nil } diff --git a/internal/commands/config/set_internal_test.go b/internal/commands/config/set_internal_test.go index f4b55ac7..b30e8450 100644 --- a/internal/commands/config/set_internal_test.go +++ b/internal/commands/config/set_internal_test.go @@ -21,6 +21,21 @@ func Test_RunInternalConfigSet(t *testing.T) { } } +// Test RunInternalConfigSet function fails when active profile is set +func Test_RunInternalConfigSet_InvalidActiveProfileUse(t *testing.T) { + testutils_koanf.InitKoanfs(t) + + var ( + profileName = customtypes.String("default") + ) + + options.RootProfileOption.Flag.Changed = true + options.RootProfileOption.CobraParamValue = &profileName + expectedErrorPattern := `^failed to set configuration: invalid assignment. Please use the 'pingcli config set active-profile ' command to set the active profile` + err := RunInternalConfigSet("activeProfile=myNewProfile") + testutils.CheckExpectedError(t, err, &expectedErrorPattern) +} + // Test RunInternalConfigSet function fails with invalid key func Test_RunInternalConfigSet_InvalidKey(t *testing.T) { testutils_koanf.InitKoanfs(t) diff --git a/internal/profiles/koanf.go b/internal/profiles/koanf.go index 91a28282..682c5fd7 100644 --- a/internal/profiles/koanf.go +++ b/internal/profiles/koanf.go @@ -155,6 +155,10 @@ func (k KoanfConfig) ValidateProfileNameFormat(pName string) (err error) { return fmt.Errorf("invalid profile name: '%s'. name must contain only alphanumeric characters, underscores, and dashes", pName) } + if strings.EqualFold(pName, options.RootActiveProfileOption.KoanfKey) { + return fmt.Errorf("invalid profile name: '%s'. name cannot be the same as the active profile key", pName) + } + return nil } diff --git a/internal/profiles/validate.go b/internal/profiles/validate.go index a404ff8a..6e662112 100644 --- a/internal/profiles/validate.go +++ b/internal/profiles/validate.go @@ -22,15 +22,28 @@ func Validate() (err error) { return err } - // Make sure selected active profile is in the configuration file activeProfileName, err := GetOptionValue(options.RootActiveProfileOption) if err != nil { return fmt.Errorf("failed to validate Ping CLI configuration: %w", err) } - if !slices.Contains(profileNames, activeProfileName) { - return fmt.Errorf("failed to validate Ping CLI configuration: active profile '%s' not found in configuration "+ - "file %s", activeProfileName, GetKoanfConfig().GetKoanfConfigFile()) + profileName, err := GetOptionValue(options.RootProfileOption) + if err != nil { + return fmt.Errorf("failed to validate Ping CLI configuration: %w", err) + } + + if profileName != "" { + // Make sure selected profile is in the configuration file + if !slices.Contains(profileNames, profileName) { + return fmt.Errorf("failed to validate Ping CLI configuration: '%s' profile not found in configuration "+ + "file %s", profileName, GetKoanfConfig().GetKoanfConfigFile()) + } + } else { + // Make sure selected active profile is in the configuration file + if !slices.Contains(profileNames, activeProfileName) { + return fmt.Errorf("failed to validate Ping CLI configuration: active profile '%s' not found in configuration "+ + "file %s", activeProfileName, GetKoanfConfig().GetKoanfConfigFile()) + } } // for each profile key, validate the profile koanf From e30eb4602e3bc469266f48bc874afcc77839b1d1 Mon Sep 17 00:00:00 2001 From: wesleymccollam Date: Tue, 22 Apr 2025 15:59:18 -0400 Subject: [PATCH 2/3] improve validation logic, fix typo --- internal/profiles/validate.go | 14 ++++++++------ internal/testing/testutils_koanf/koanf_utils.go | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/profiles/validate.go b/internal/profiles/validate.go index 6e662112..9aa36b7a 100644 --- a/internal/profiles/validate.go +++ b/internal/profiles/validate.go @@ -22,11 +22,6 @@ func Validate() (err error) { return err } - activeProfileName, err := GetOptionValue(options.RootActiveProfileOption) - if err != nil { - return fmt.Errorf("failed to validate Ping CLI configuration: %w", err) - } - profileName, err := GetOptionValue(options.RootProfileOption) if err != nil { return fmt.Errorf("failed to validate Ping CLI configuration: %w", err) @@ -38,7 +33,14 @@ func Validate() (err error) { return fmt.Errorf("failed to validate Ping CLI configuration: '%s' profile not found in configuration "+ "file %s", profileName, GetKoanfConfig().GetKoanfConfigFile()) } - } else { + } + + activeProfileName, err := GetOptionValue(options.RootActiveProfileOption) + if err != nil { + return fmt.Errorf("failed to validate Ping CLI configuration: %w", err) + } + + if activeProfileName != "" { // Make sure selected active profile is in the configuration file if !slices.Contains(profileNames, activeProfileName) { return fmt.Errorf("failed to validate Ping CLI configuration: active profile '%s' not found in configuration "+ diff --git a/internal/testing/testutils_koanf/koanf_utils.go b/internal/testing/testutils_koanf/koanf_utils.go index f6249233..66862f43 100644 --- a/internal/testing/testutils_koanf/koanf_utils.go +++ b/internal/testing/testutils_koanf/koanf_utils.go @@ -83,7 +83,7 @@ func configureMainKoanf(t *testing.T) { mainKoanf.SetKoanfConfigFile(configFilePath) if err := mainKoanf.KoanfInstance().Load(file.Provider(configFilePath), yaml.Parser()); err != nil { - t.Fatalf("Failed to load configurationhere from file '%s': %v", configFilePath, err) + t.Fatalf("Failed to load configuration from file '%s': %v", configFilePath, err) } } From 2336005abfe162360465d61d81085659fbbcd393 Mon Sep 17 00:00:00 2001 From: wesleymccollam Date: Tue, 22 Apr 2025 16:10:04 -0400 Subject: [PATCH 3/3] remove unneeded if condition on active profile name validation --- internal/profiles/validate.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/internal/profiles/validate.go b/internal/profiles/validate.go index 9aa36b7a..1d2c7664 100644 --- a/internal/profiles/validate.go +++ b/internal/profiles/validate.go @@ -40,12 +40,10 @@ func Validate() (err error) { return fmt.Errorf("failed to validate Ping CLI configuration: %w", err) } - if activeProfileName != "" { - // Make sure selected active profile is in the configuration file - if !slices.Contains(profileNames, activeProfileName) { - return fmt.Errorf("failed to validate Ping CLI configuration: active profile '%s' not found in configuration "+ - "file %s", activeProfileName, GetKoanfConfig().GetKoanfConfigFile()) - } + // Make sure selected active profile is in the configuration file + if !slices.Contains(profileNames, activeProfileName) { + return fmt.Errorf("failed to validate Ping CLI configuration: active profile '%s' not found in configuration "+ + "file %s", activeProfileName, GetKoanfConfig().GetKoanfConfigFile()) } // for each profile key, validate the profile koanf