diff --git a/internal/iam/command_rbac_role_binding_list.go b/internal/iam/command_rbac_role_binding_list.go index be7ff740e0..7617d1d11b 100644 --- a/internal/iam/command_rbac_role_binding_list.go +++ b/internal/iam/command_rbac_role_binding_list.go @@ -114,7 +114,7 @@ func (c *roleBindingCommand) newListCommand() *cobra.Command { } cmd.Flags().String("resource", "", `Resource type and identifier using "Prefix:ID" format. If specified with "--role" and no principals, list all principals and role bindings.`) - cmd.Flags().Bool("inclusive", false, "List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list only organization-scoped role bindings.") + cmd.Flags().Bool("inclusive", false, "List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list role bindings across all scopes. Only applies to Confluent Cloud.") pcmd.AddOutputFlag(cmd) return cmd diff --git a/internal/logout/command.go b/internal/logout/command.go index e30fdd6dec..c8b76bcf6f 100644 --- a/internal/logout/command.go +++ b/internal/logout/command.go @@ -1,6 +1,7 @@ package logout import ( + "context" "fmt" "github.com/spf13/cobra" @@ -12,11 +13,12 @@ import ( "github.com/confluentinc/cli/v4/pkg/ccloudv2" pcmd "github.com/confluentinc/cli/v4/pkg/cmd" "github.com/confluentinc/cli/v4/pkg/config" + "github.com/confluentinc/cli/v4/pkg/log" "github.com/confluentinc/cli/v4/pkg/output" ) type command struct { - *pcmd.AuthenticatedCLICommand + *pcmd.CLICommand cfg *config.Config authTokenHandler pauth.AuthTokenHandler } @@ -28,16 +30,18 @@ func New(cfg *config.Config, prerunner pcmd.PreRunner, authTokenHandler pauth.Au } context := "Confluent Cloud or Confluent Platform" - c := &command{ - AuthenticatedCLICommand: pcmd.NewAuthenticatedCLICommand(cmd, prerunner), - cfg: cfg, - authTokenHandler: authTokenHandler, - } if cfg.IsCloudLogin() { context = "Confluent Cloud" } else if cfg.IsOnPremLogin() { context = "Confluent Platform" - c.AuthenticatedCLICommand = pcmd.NewAuthenticatedWithMDSCLICommand(cmd, prerunner) + } + + c := &command{ + // Anonymous (not Authenticated): logout must not require being logged in, and must not + // trigger an auto-login via env-var credentials only to immediately log back out. + CLICommand: pcmd.NewAnonymousCLICommand(cmd, prerunner), + cfg: cfg, + authTokenHandler: authTokenHandler, } cmd.Short = fmt.Sprintf("Log out of %s.", context) @@ -49,11 +53,14 @@ func New(cfg *config.Config, prerunner pcmd.PreRunner, authTokenHandler pauth.Au func (c *command) logout(_ *cobra.Command, _ []string) error { ctx := c.Config.Context() - if ctx != nil { - if ccloudv2.IsCCloudURL(ctx.Platform.Server, c.cfg.IsTest) { - if _, err := c.revokeCCloudRefreshToken(ctx); err != nil { - return err - } + if ctx == nil { + // Already logged out: do nothing. + return nil + } + + if ccloudv2.IsCCloudURL(ctx.Platform.Server, c.cfg.IsTest) { + if _, err := c.revokeCCloudRefreshToken(ctx); err != nil { + return err } } @@ -67,14 +74,28 @@ func (c *command) logout(_ *cobra.Command, _ []string) error { func (c *command) revokeCCloudRefreshToken(ctx *config.Context) (*ccloudv1.AuthenticateReply, error) { contextState := c.Config.ContextStates[ctx.Name] + if contextState == nil { + // Missing or corrupt context state: nothing to revoke, but logout should still succeed. + return nil, nil + } if err := contextState.DecryptAuthToken(ctx.Name); err != nil { return nil, err } + var userAgent string + if c.Version != nil { + userAgent = c.Version.UserAgent + } + client := ccloudv1.NewClientWithJWT(context.Background(), contextState.AuthToken, &ccloudv1.Params{ + BaseURL: ctx.GetPlatformServer(), + Logger: log.CliLogger, + UserAgent: userAgent, + }) + req := &ccloudv1.AuthenticateRequest{IdToken: contextState.AuthToken} if sso.IsOkta(ctx.Platform.Server) { - return c.Client.Auth.OktaLogout(req) + return client.Auth.OktaLogout(req) } else { - return c.Client.Auth.Logout(req) + return client.Auth.Logout(req) } } diff --git a/internal/logout/command_test.go b/internal/logout/command_test.go index a4a865251f..54246d6d9f 100644 --- a/internal/logout/command_test.go +++ b/internal/logout/command_test.go @@ -53,6 +53,30 @@ func TestLogout(t *testing.T) { verifyLoggedOutState(t, cfg, contextName) } +func TestRevokeCCloudRefreshTokenNoopWhenContextStateMissing(t *testing.T) { + req := require.New(t) + cfg := config.AuthenticatedConfigMockWithContextName(config.MockContextName) + ctx := cfg.Context() + // Simulate a missing/corrupt context_state entry for an otherwise-valid context. + delete(cfg.ContextStates, ctx.Name) + + c := &command{CLICommand: &pcmd.CLICommand{Config: cfg}, cfg: cfg} + + reply, err := c.revokeCCloudRefreshToken(ctx) + req.NoError(err) + req.Nil(reply) +} + +func TestLogoutNoopWhenAlreadyLoggedOut(t *testing.T) { + req := require.New(t) + cfg := config.New() + prerunner := climock.NewPreRunnerMock(nil, nil, nil, nil, cfg) + logoutCmd := New(cfg, prerunner, AuthTokenHandler) + + _, err := pcmd.ExecuteCommand(logoutCmd) + req.NoError(err) +} + func newLogoutCmd(auth *ccloudv1mock.Auth, userInterface *ccloudv1mock.UserInterface, isCloud bool, req *require.Assertions, authTokenHandler pauth.AuthTokenHandler, contextName string) (*cobra.Command, *config.Config) { config.SetTempHomeDir() cfg := config.AuthenticatedConfigMockWithContextName(contextName) diff --git a/pkg/linter/command_rules.go b/pkg/linter/command_rules.go index 5a8b84fe96..ae8be56eb7 100644 --- a/pkg/linter/command_rules.go +++ b/pkg/linter/command_rules.go @@ -221,29 +221,52 @@ func RequireValidExamples() CommandRule { return func(cmd *cobra.Command) error { requiredFlags := getRequiredFlags(cmd.Flags()) allFlags := getAllFlags(cmd.Flags()) + boolFlags := getBoolFlags(cmd.Flags()) errs := new(multierror.Error) for i, example := range getExampleCodeSnippets(cmd.Example) { - for _, flag := range requiredFlags { - if !strings.Contains(example, flag) { - errs = multierror.Append(errs, fmt.Errorf("%s: required flag `%s` not found in example %d", cmd.CommandPath(), flag, i+1)) - } - } + errs = multierror.Append(errs, requireExampleHasRequiredFlags(cmd, requiredFlags, example, i)...) + errs = multierror.Append(errs, requireExampleFlagsAreKnown(cmd, allFlags, example, i)...) + errs = multierror.Append(errs, requireExampleNoEqualsForNonBoolFlags(cmd, boolFlags, example, i)...) + } - for _, match := range regexp.MustCompile(`--[a-z\-]+`).FindAllString(example, -1) { - if !slices.Contains(allFlags, match) { - errs = multierror.Append(errs, fmt.Errorf("%s: unknown flag `%s` found in example %d", cmd.CommandPath(), match, i+1)) - } - } + return errs + } +} - for _, match := range regexp.MustCompile(`--[a-z\-]+=`).FindAllString(example, -1) { - errs = multierror.Append(errs, fmt.Errorf("%s: flag `%s` must not use \"=\" in example %d", cmd.CommandPath(), strings.TrimSuffix(match, "="), i+1)) - } +func requireExampleHasRequiredFlags(cmd *cobra.Command, requiredFlags []string, example string, i int) []error { + var errs []error + for _, flag := range requiredFlags { + if !strings.Contains(example, flag) { + errs = append(errs, fmt.Errorf("%s: required flag `%s` not found in example %d", cmd.CommandPath(), flag, i+1)) } + } + return errs +} - return errs +func requireExampleFlagsAreKnown(cmd *cobra.Command, allFlags []string, example string, i int) []error { + var errs []error + for _, match := range regexp.MustCompile(`--[a-z\-]+`).FindAllString(example, -1) { + if !slices.Contains(allFlags, match) { + errs = append(errs, fmt.Errorf("%s: unknown flag `%s` found in example %d", cmd.CommandPath(), match, i+1)) + } + } + return errs +} + +func requireExampleNoEqualsForNonBoolFlags(cmd *cobra.Command, boolFlags []string, example string, i int) []error { + matches := regexp.MustCompile(`--[a-z\-]+=`).FindAllString(example, -1) + errs := make([]error, 0, len(matches)) + for _, match := range matches { + flag := strings.TrimSuffix(match, "=") + // Boolean flags legitimately need "=" to set the non-default value, e.g. --flag=false. + if slices.Contains(boolFlags, flag) { + continue + } + errs = append(errs, fmt.Errorf("%s: flag `%s` must not use \"=\" in example %d", cmd.CommandPath(), flag, i+1)) } + return errs } func getExampleCodeSnippets(example string) []string { @@ -274,6 +297,16 @@ func getAllFlags(flags *pflag.FlagSet) []string { return all } +func getBoolFlags(flags *pflag.FlagSet) []string { + var boolFlags []string + flags.VisitAll(func(flag *pflag.Flag) { + if flag.Value.Type() == "bool" { + boolFlags = append(boolFlags, "--"+flag.Name) + } + }) + return boolFlags +} + func getValueByName(obj any, name string) string { return reflect.Indirect(reflect.ValueOf(obj)).FieldByName(name).String() } diff --git a/pkg/linter/rules_test.go b/pkg/linter/rules_test.go index f788dc2b5f..03dab6182a 100644 --- a/pkg/linter/rules_test.go +++ b/pkg/linter/rules_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/client9/gospell" + "github.com/hashicorp/go-multierror" "github.com/spf13/cobra" "github.com/stretchr/testify/require" ) @@ -55,6 +56,30 @@ func TestFlagKebabCase(t *testing.T) { }) } +func TestRequireValidExamplesAllowsEqualsForBoolFlags(t *testing.T) { + rule := RequireValidExamples() + + t.Run("bool flag with \"=\" is allowed", func(t *testing.T) { + cmd := &cobra.Command{Run: func(cmd *cobra.Command, args []string) {}} + cmd.Flags().Bool("force", false, "a bool flag") + cmd.Example = " $ confluent example --force=false\n" + err := cmd.Execute() + require.NoError(t, err) + err = rule(cmd) + require.Nil(t, err.(*multierror.Error).ErrorOrNil()) + }) + + t.Run("non-bool flag with \"=\" is rejected", func(t *testing.T) { + cmd := &cobra.Command{Run: func(cmd *cobra.Command, args []string) {}} + cmd.Flags().String("cluster", "", "a string flag") + cmd.Example = " $ confluent example --cluster=lkc-123456\n" + err := cmd.Execute() + require.NoError(t, err) + err = rule(cmd) + require.Error(t, err) + }) +} + func TestFlagUsageRealWords(t *testing.T) { req := require.New(t) rule := RequireFlagUsageRealWords([]string{}) diff --git a/test/fixtures/output/iam/rbac/role-binding/list-failure-help-cloud.golden b/test/fixtures/output/iam/rbac/role-binding/list-failure-help-cloud.golden index b3f8a39e1a..64027504e7 100644 --- a/test/fixtures/output/iam/rbac/role-binding/list-failure-help-cloud.golden +++ b/test/fixtures/output/iam/rbac/role-binding/list-failure-help-cloud.golden @@ -39,7 +39,7 @@ Flags: --ksql-cluster string ksqlDB cluster name, which specifies the ksqlDB cluster scope. --flink-region string Flink region for the role binding, formatted as "cloud.region". --resource string Resource type and identifier using "Prefix:ID" format. If specified with "--role" and no principals, list all principals and role bindings. - --inclusive List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list only organization-scoped role bindings. + --inclusive List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list role bindings across all scopes. Only applies to Confluent Cloud. -o, --output string Specify the output format as "human", "json", or "yaml". (default "human") Global Flags: diff --git a/test/fixtures/output/iam/rbac/role-binding/list-failure-help-onprem.golden b/test/fixtures/output/iam/rbac/role-binding/list-failure-help-onprem.golden index 3297fc0539..990fbebb3f 100644 --- a/test/fixtures/output/iam/rbac/role-binding/list-failure-help-onprem.golden +++ b/test/fixtures/output/iam/rbac/role-binding/list-failure-help-onprem.golden @@ -38,7 +38,7 @@ Flags: --context string CLI context name. --cluster-name string Cluster name, which specifies the cluster scope. --resource string Resource type and identifier using "Prefix:ID" format. If specified with "--role" and no principals, list all principals and role bindings. - --inclusive List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list only organization-scoped role bindings. + --inclusive List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list role bindings across all scopes. Only applies to Confluent Cloud. -o, --output string Specify the output format as "human", "json", or "yaml". (default "human") Global Flags: diff --git a/test/fixtures/output/iam/rbac/role-binding/list-help-onprem.golden b/test/fixtures/output/iam/rbac/role-binding/list-help-onprem.golden index e2b2adffb0..ddcc7284e3 100644 --- a/test/fixtures/output/iam/rbac/role-binding/list-help-onprem.golden +++ b/test/fixtures/output/iam/rbac/role-binding/list-help-onprem.golden @@ -39,7 +39,7 @@ Flags: --context string CLI context name. --cluster-name string Cluster name, which specifies the cluster scope. --resource string Resource type and identifier using "Prefix:ID" format. If specified with "--role" and no principals, list all principals and role bindings. - --inclusive List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list only organization-scoped role bindings. + --inclusive List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list role bindings across all scopes. Only applies to Confluent Cloud. -o, --output string Specify the output format as "human", "json", or "yaml". (default "human") Global Flags: diff --git a/test/fixtures/output/iam/rbac/role-binding/list-help.golden b/test/fixtures/output/iam/rbac/role-binding/list-help.golden index 28a1de56b8..52febe6b8b 100644 --- a/test/fixtures/output/iam/rbac/role-binding/list-help.golden +++ b/test/fixtures/output/iam/rbac/role-binding/list-help.golden @@ -40,7 +40,7 @@ Flags: --ksql-cluster string ksqlDB cluster name, which specifies the ksqlDB cluster scope. --flink-region string Flink region for the role binding, formatted as "cloud.region". --resource string Resource type and identifier using "Prefix:ID" format. If specified with "--role" and no principals, list all principals and role bindings. - --inclusive List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list only organization-scoped role bindings. + --inclusive List role bindings for specified scopes and nested scopes. Otherwise, list role bindings for the specified scopes. If scopes are unspecified, list role bindings across all scopes. Only applies to Confluent Cloud. -o, --output string Specify the output format as "human", "json", or "yaml". (default "human") Global Flags: diff --git a/test/logout_test.go b/test/logout_test.go index c3d580a667..0c7614f0f6 100644 --- a/test/logout_test.go +++ b/test/logout_test.go @@ -9,6 +9,18 @@ import ( "github.com/confluentinc/cli/v4/pkg/utils" ) +func (s *CLITestSuite) TestLogout_NoopWhenAlreadyLoggedOut() { + cloudUrl := s.TestBackend.GetCloudUrl() + env := []string{fmt.Sprintf("%s=good@user.com", auth.ConfluentCloudEmail), fmt.Sprintf("%s=pass1", auth.ConfluentCloudPassword)} + + runCommand(s.T(), testBin, env, "login -vvvv --save --url "+cloudUrl, 0, "") + runCommand(s.T(), testBin, env, "logout -vvvv", 0, "") + + // Logging out a second time, with no active session, must succeed as a no-op rather than error. + output := runCommand(s.T(), testBin, env, "logout -vvvv", 0, "") + s.NotContains(output, "You are now logged out.") +} + func (s *CLITestSuite) TestLogout_RemoveUsernamePassword() { type saveTest struct { isCloud bool