From 4b0d2dcb09401b4eded91e3f6c81d84ff79509c3 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 10:31:41 +0100 Subject: [PATCH 01/16] add deprecated note --- docs/command/atlas-config-delete.txt | 103 --------------------------- docs/command/atlas-config.txt | 2 - internal/cli/config/delete.go | 9 +-- 3 files changed, 5 insertions(+), 109 deletions(-) delete mode 100644 docs/command/atlas-config-delete.txt diff --git a/docs/command/atlas-config-delete.txt b/docs/command/atlas-config-delete.txt deleted file mode 100644 index 767cc09906..0000000000 --- a/docs/command/atlas-config-delete.txt +++ /dev/null @@ -1,103 +0,0 @@ -.. _atlas-config-delete: - -=================== -atlas config delete -=================== - -.. default-domain:: mongodb - -.. contents:: On this page - :local: - :backlinks: none - :depth: 1 - :class: singlecol - -Delete a profile. - -Syntax ------- - -.. code-block:: - :caption: Command Syntax - - atlas config delete [options] - -.. Code end marker, please don't delete this comment - -Arguments ---------- - -.. list-table:: - :header-rows: 1 - :widths: 20 10 10 60 - - * - Name - - Type - - Required - - Description - * - name - - string - - true - - Name of the profile. - -Options -------- - -.. list-table:: - :header-rows: 1 - :widths: 20 10 10 60 - - * - Name - - Type - - Required - - Description - * - --force - - - - false - - Flag that indicates whether to skip the confirmation prompt before proceeding with the requested action. - * - -h, --help - - - - false - - help for delete - -Inherited Options ------------------ - -.. list-table:: - :header-rows: 1 - :widths: 20 10 10 60 - - * - Name - - Type - - Required - - Description - * - -P, --profile - - string - - false - - Name of the profile to use from your configuration file. To learn about profiles for the Atlas CLI, see https://dochub.mongodb.org/core/atlas-cli-save-connection-settings. - -Output ------- - -If the command succeeds, the CLI returns output similar to the following sample. Values in brackets represent your values. - -.. code-block:: - - Profile '' deleted - - -Examples --------- - -.. code-block:: - :copyable: false - - # Delete the default profile configuration: - atlas config delete default - - -.. code-block:: - :copyable: false - - # Skip the confirmation question and delete the default profile configuration: - atlas config delete default --force diff --git a/docs/command/atlas-config.txt b/docs/command/atlas-config.txt index 7beaa190c0..c301105a8f 100644 --- a/docs/command/atlas-config.txt +++ b/docs/command/atlas-config.txt @@ -56,7 +56,6 @@ Inherited Options Related Commands ---------------- -* :ref:`atlas-config-delete` - Delete a profile. * :ref:`atlas-config-describe` - Return the profile you specify. * :ref:`atlas-config-edit` - Opens the config file with the default text editor. * :ref:`atlas-config-list` - Return a list of available profiles by name. @@ -67,7 +66,6 @@ Related Commands .. toctree:: :titlesonly: - delete describe edit list diff --git a/internal/cli/config/delete.go b/internal/cli/config/delete.go index 15c40c20a8..118d85529d 100644 --- a/internal/cli/config/delete.go +++ b/internal/cli/config/delete.go @@ -50,10 +50,11 @@ func DeleteBuilder() *cobra.Command { DeleteOpts: cli.NewDeleteOpts("Profile '%s' deleted\n", "Profile not deleted"), } cmd := &cobra.Command{ - Use: "delete ", - Aliases: []string{"rm"}, - Short: "Delete a profile.", - Args: require.ExactArgs(1), + Use: "delete ", + Aliases: []string{"rm"}, + Short: "Delete a profile.", + Args: require.ExactArgs(1), + Deprecated: "Please use the 'atlas auth logout' command instead.", Example: ` # Delete the default profile configuration: atlas config delete default From c24d496778be12dc0cbc3119c41e21da9e8e5b4c Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 11:45:24 +0100 Subject: [PATCH 02/16] CLOUDP-329800-alias: reuse logout and deprecate config delete --- internal/cli/auth/logout.go | 36 +++++++++++++++---- internal/cli/auth/whoami.go | 2 ++ internal/cli/config/delete.go | 23 ++++++++++++ .../deployments/options/deployment_opts.go | 2 +- internal/config/migrate.go | 9 +++-- internal/config/migrate_test.go | 9 +++-- internal/config/profile.go | 1 + 7 files changed, 66 insertions(+), 16 deletions(-) diff --git a/internal/cli/auth/logout.go b/internal/cli/auth/logout.go index aeedc7d1d8..8617539505 100644 --- a/internal/cli/auth/logout.go +++ b/internal/cli/auth/logout.go @@ -74,20 +74,30 @@ func (opts *logoutOpts) Run(ctx context.Context) error { fallthrough case config.UserAccount: // revoking a refresh token revokes the access token - if _, err := opts.flow.RevokeToken(ctx, config.RefreshToken(), "refresh_token"); err != nil { - return err + if refreshToken := config.RefreshToken(); refreshToken != "" && opts.flow != nil { + if _, err := opts.flow.RevokeToken(ctx, refreshToken, "refresh_token"); err != nil { + return err + } } + opts.config.SetAccessToken("") + opts.config.SetRefreshToken("") + case config.NoAuth: + // Handle profiles with no authentication configured + // Just clear any potential leftover credentials + opts.config.SetPublicAPIKey("") + opts.config.SetPrivateAPIKey("") opts.config.SetAccessToken("") opts.config.SetRefreshToken("") } + opts.config.SetProjectID("") + opts.config.SetOrgID("") + if !opts.keepConfig { return opts.Delete(opts.config.Delete) } - opts.config.SetProjectID("") - opts.config.SetOrgID("") return opts.config.Save() } @@ -105,16 +115,24 @@ func LogoutBuilder() *cobra.Command { PreRunE: func(cmd *cobra.Command, _ []string) error { opts.OutWriter = cmd.OutOrStdout() opts.config = config.Default() - return opts.initFlow() + // Only initialize OAuth flow if we have OAuth-based auth + if opts.config.AuthType() == config.UserAccount || opts.config.AuthType() == config.ServiceAccount { + return opts.initFlow() + } + return nil }, RunE: func(cmd *cobra.Command, _ []string) error { var message, entry string var err error - if opts.config.AuthType() == config.APIKeys { + switch opts.config.AuthType() { + case config.APIKeys: entry = opts.config.PublicAPIKey() + if entry == "" { + entry = "API key account" + } message = "Are you sure you want to log out of account with public API key %s?" - } else { + case config.ServiceAccount, config.UserAccount: entry, err = config.AccessTokenSubject() if err != nil { return err @@ -125,6 +143,10 @@ func LogoutBuilder() *cobra.Command { } message = "Are you sure you want to log out of account %s?" + case config.NoAuth: + // Handle profiles with no authentication configured + entry = "profile" + message = "Are you sure you want to clear profile %s?" } opts.Entry = entry diff --git a/internal/cli/auth/whoami.go b/internal/cli/auth/whoami.go index a30db67891..9725515902 100644 --- a/internal/cli/auth/whoami.go +++ b/internal/cli/auth/whoami.go @@ -47,6 +47,8 @@ func authTypeAndSubject() (string, string, error) { case config.UserAccount: subject, _ := config.AccessTokenSubject() return "account", subject, nil + case config.NoAuth: + return "", "", ErrUnauthenticated } return "", "", ErrUnauthenticated diff --git a/internal/cli/config/delete.go b/internal/cli/config/delete.go index 118d85529d..4212d956dd 100644 --- a/internal/cli/config/delete.go +++ b/internal/cli/config/delete.go @@ -16,6 +16,8 @@ package config import ( "fmt" + "os" + "os/exec" "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli" "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli/require" @@ -29,11 +31,32 @@ type DeleteOpts struct { *cli.DeleteOpts } +func (opts *DeleteOpts) executeLogout() error { + // Get the current executable path + executable, err := os.Executable() + if err != nil { + return fmt.Errorf("failed to get executable path: %w", err) + } + + // Execute: atlas auth logout --profile --force + cmd := exec.Command(executable, "auth", "logout", "--profile", opts.Entry, "--force") + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + // Execute logout - this now handles all profile types gracefully + return cmd.Run() +} + func (opts *DeleteOpts) Run() error { if !opts.Confirm { return nil } + // Execute logout command for the profile before deletion + if err := opts.executeLogout(); err != nil { + return err + } + if err := config.SetName(opts.Entry); err != nil { return err } diff --git a/internal/cli/deployments/options/deployment_opts.go b/internal/cli/deployments/options/deployment_opts.go index 2a9dbe910e..64b84a975a 100644 --- a/internal/cli/deployments/options/deployment_opts.go +++ b/internal/cli/deployments/options/deployment_opts.go @@ -231,7 +231,7 @@ func (opts *DeploymentOpts) IsCliAuthenticated() bool { if opts.CredStore == nil { opts.CredStore = config.Default() } - return opts.CredStore.AuthType() != "" + return opts.CredStore.AuthType() != "" && opts.CredStore.AuthType() != config.NoAuth } func (opts *DeploymentOpts) GetLocalContainers(ctx context.Context) ([]container.Container, error) { diff --git a/internal/config/migrate.go b/internal/config/migrate.go index 0fcf37ebbc..3045ab4eda 100644 --- a/internal/config/migrate.go +++ b/internal/config/migrate.go @@ -32,15 +32,14 @@ func MigrateVersions(store Store) error { } // setAuthTypes sets the auth type for each profile based on the credentials available. -// Nothing is set if no credentials are found. +// Sets NoAuth for profiles without authentication. func setAuthTypes(store Store, getAuthType func(*Profile) AuthMechanism) { profileNames := store.GetProfileNames() for _, name := range profileNames { profile := NewProfile(name, store) authType := getAuthType(profile) - if authType != "" { - profile.SetAuthType(authType) - } + // Always set the auth type, including NoAuth for profiles without authentication + profile.SetAuthType(authType) } } @@ -53,5 +52,5 @@ func getAuthType(profile *Profile) AuthMechanism { case profile.ClientID() != "" && profile.ClientSecret() != "": return ServiceAccount } - return AuthMechanism("") // This should not happen unless profile is not properly initialized. + return NoAuth // Profile has no authentication configured } diff --git a/internal/config/migrate_test.go b/internal/config/migrate_test.go index 9bc2e5c653..5fd79b4cfa 100644 --- a/internal/config/migrate_test.go +++ b/internal/config/migrate_test.go @@ -118,13 +118,16 @@ func Test_SetAuthTypes(t *testing.T) { GetProfileNames(). Return([]string{"test"}). Times(1) + mockStore.EXPECT(). + SetProfileValue("test", "auth_type", "no_auth"). + Times(1) mockStore.EXPECT(). GetHierarchicalValue("test", "auth_type"). - Return(""). + Return("no_auth"). AnyTimes() }, setupProfile: func(*Profile) {}, - expectedAuthType: AuthMechanism(""), + expectedAuthType: NoAuth, }, } @@ -177,7 +180,7 @@ func Test_GetAuthType(t *testing.T) { { name: "Empty Profile", setup: func(*Profile) {}, - expectedAuthType: AuthMechanism(""), + expectedAuthType: NoAuth, }, } diff --git a/internal/config/profile.go b/internal/config/profile.go index e15bc26f91..a3043abe7c 100644 --- a/internal/config/profile.go +++ b/internal/config/profile.go @@ -281,6 +281,7 @@ const ( APIKeys AuthMechanism = "api_keys" UserAccount AuthMechanism = "user_account" ServiceAccount AuthMechanism = "service_account" + NoAuth AuthMechanism = "no_auth" ) // AuthType gets the configured auth type. From 25a40712b63e47b3399eda749c5662c527e98fc3 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 13:36:15 +0100 Subject: [PATCH 03/16] add noAuth --- internal/cli/auth/logout_test.go | 119 +++++++++++++++++++ internal/cli/auth/whoami_test.go | 170 +++++++++++++++++++++++++++ internal/cli/organizations/create.go | 2 + internal/cli/setup/setup_cmd.go | 7 +- internal/store/store.go | 2 + internal/telemetry/event.go | 2 + 6 files changed, 301 insertions(+), 1 deletion(-) diff --git a/internal/cli/auth/logout_test.go b/internal/cli/auth/logout_test.go index edff9bb1c5..4b6556038e 100644 --- a/internal/cli/auth/logout_test.go +++ b/internal/cli/auth/logout_test.go @@ -18,6 +18,7 @@ package auth import ( "bytes" + "context" "testing" "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli" @@ -395,3 +396,121 @@ func Test_LogoutBuilder_Integration(t *testing.T) { assert.True(t, keepFlag.Hidden) }) } + +func Test_LogoutBuilder_NoAuth_Handling(t *testing.T) { + t.Run("handles NoAuth profiles gracefully", func(t *testing.T) { + // Save original config state + originalAuthType := config.AuthType() + originalPublicKey := config.PublicAPIKey() + originalPrivateKey := config.PrivateAPIKey() + originalAccessToken := config.AccessToken() + originalRefreshToken := config.RefreshToken() + originalProjectID := config.ProjectID() + originalOrgID := config.OrgID() + + defer func() { + config.SetAuthType(originalAuthType) + config.SetPublicAPIKey(originalPublicKey) + config.SetPrivateAPIKey(originalPrivateKey) + config.SetAccessToken(originalAccessToken) + config.SetRefreshToken(originalRefreshToken) + config.SetProjectID(originalProjectID) + config.SetOrgID(originalOrgID) + }() + + // Set up NoAuth configuration + config.SetAuthType(config.NoAuth) + config.SetPublicAPIKey("some-leftover-key") + config.SetPrivateAPIKey("some-leftover-secret") + config.SetAccessToken("some-leftover-token") + config.SetRefreshToken("some-leftover-refresh") + config.SetProjectID("some-project-id") + config.SetOrgID("some-org-id") + + cmd := LogoutBuilder() + + // Create a test command context + testCmd := &cobra.Command{} + buf := new(bytes.Buffer) + testCmd.SetOut(buf) + + // Execute PreRunE first - should not initialize OAuth flow for NoAuth + err := cmd.PreRunE(testCmd, []string{}) + assert.NoError(t, err) + + // Create logout opts for testing Run method directly + opts := &logoutOpts{ + DeleteOpts: cli.NewDeleteOpts("test success", "test fail"), + config: config.Default(), + keepConfig: true, // Don't delete config, just clear credentials + } + + // Execute Run method + ctx := context.Background() + err = opts.Run(ctx) + assert.NoError(t, err) + + // Verify all credentials are cleared + assert.Equal(t, "", config.PublicAPIKey()) + assert.Equal(t, "", config.PrivateAPIKey()) + assert.Equal(t, "", config.AccessToken()) + assert.Equal(t, "", config.RefreshToken()) + assert.Equal(t, "", config.ProjectID()) + assert.Equal(t, "", config.OrgID()) + }) + + t.Run("RunE handles NoAuth with appropriate message", func(t *testing.T) { + // Save original config state + originalAuthType := config.AuthType() + defer func() { + config.SetAuthType(originalAuthType) + }() + + // Set up NoAuth configuration + config.SetAuthType(config.NoAuth) + + cmd := LogoutBuilder() + + // Create a test command context + testCmd := &cobra.Command{} + buf := new(bytes.Buffer) + testCmd.SetOut(buf) + + // Execute PreRunE first + err := cmd.PreRunE(testCmd, []string{}) + assert.NoError(t, err) + + // We can't easily test the full RunE without mocking the prompt, + // but we can verify the command structure handles NoAuth + assert.NotNil(t, cmd.RunE) + }) + + t.Run("PreRunE does not initialize OAuth flow for NoAuth", func(t *testing.T) { + // Save original config state + originalAuthType := config.AuthType() + defer func() { + config.SetAuthType(originalAuthType) + }() + + // Set up NoAuth configuration + config.SetAuthType(config.NoAuth) + + opts := &logoutOpts{} + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + // Simulate PreRunE logic + opts.config = config.Default() + + var flowInitialized bool + if opts.config.AuthType() == config.UserAccount || opts.config.AuthType() == config.ServiceAccount { + flowInitialized = true + } + + // Verify OAuth flow is not initialized for NoAuth + assert.False(t, flowInitialized) + assert.Nil(t, opts.flow) + }) +} diff --git a/internal/cli/auth/whoami_test.go b/internal/cli/auth/whoami_test.go index 20a6d8439f..6e642cbfcb 100644 --- a/internal/cli/auth/whoami_test.go +++ b/internal/cli/auth/whoami_test.go @@ -20,6 +20,7 @@ import ( "bytes" "testing" + "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -34,3 +35,172 @@ func Test_whoOpts_Run(t *testing.T) { require.NoError(t, opts.Run()) assert.Equal(t, "Logged in as test@test.com account\n", buf.String()) } + +func Test_authTypeAndSubject(t *testing.T) { + tests := []struct { + name string + setupConfig func() + expectedType string + expectedSubject string + expectedError error + }{ + { + name: "API Keys authentication", + setupConfig: func() { + config.SetAuthType(config.APIKeys) + config.SetPublicAPIKey("test-public-key") + }, + expectedType: "key", + expectedSubject: "test-public-key", + expectedError: nil, + }, + { + name: "Service Account authentication", + setupConfig: func() { + config.SetAuthType(config.ServiceAccount) + config.SetClientID("test-client-id") + }, + expectedType: "service account", + expectedSubject: "test-client-id", + expectedError: nil, + }, + { + name: "User Account authentication", + setupConfig: func() { + config.SetAuthType(config.UserAccount) + config.SetAccessToken("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0QGV4YW1wbGUuY29tIiwibmFtZSI6IlRlc3QgVXNlciIsImlhdCI6MTUxNjIzOTAyMn0.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c") + }, + expectedType: "account", + expectedSubject: "test@example.com", + expectedError: nil, + }, + { + name: "NoAuth authentication", + setupConfig: func() { + config.SetAuthType(config.NoAuth) + }, + expectedType: "", + expectedSubject: "", + expectedError: ErrUnauthenticated, + }, + { + name: "Empty authentication type", + setupConfig: func() { + config.SetAuthType(config.AuthMechanism("")) + }, + expectedType: "", + expectedSubject: "", + expectedError: ErrUnauthenticated, + }, + { + name: "Unknown authentication type", + setupConfig: func() { + config.SetAuthType(config.AuthMechanism("unknown")) + }, + expectedType: "", + expectedSubject: "", + expectedError: ErrUnauthenticated, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save original config state + originalAuthType := config.AuthType() + originalPublicKey := config.PublicAPIKey() + originalClientID := config.ClientID() + originalAccessToken := config.AccessToken() + + defer func() { + config.SetAuthType(originalAuthType) + config.SetPublicAPIKey(originalPublicKey) + config.SetClientID(originalClientID) + config.SetAccessToken(originalAccessToken) + }() + + // Clear config state + config.SetAuthType(config.AuthMechanism("")) + config.SetPublicAPIKey("") + config.SetClientID("") + config.SetAccessToken("") + + // Setup test configuration + tt.setupConfig() + + // Execute the function + authType, authSubject, err := authTypeAndSubject() + + // Verify results + assert.Equal(t, tt.expectedType, authType) + assert.Equal(t, tt.expectedSubject, authSubject) + + if tt.expectedError != nil { + assert.ErrorIs(t, err, tt.expectedError) + } else { + assert.NoError(t, err) + } + }) + } +} + +func Test_authTypeAndSubject_EdgeCases(t *testing.T) { + t.Run("API Keys with empty public key", func(t *testing.T) { + // Save original config state + originalAuthType := config.AuthType() + originalPublicKey := config.PublicAPIKey() + + defer func() { + config.SetAuthType(originalAuthType) + config.SetPublicAPIKey(originalPublicKey) + }() + + config.SetAuthType(config.APIKeys) + config.SetPublicAPIKey("") + + authType, authSubject, err := authTypeAndSubject() + + assert.Equal(t, "key", authType) + assert.Equal(t, "", authSubject) + assert.NoError(t, err) + }) + + t.Run("Service Account with empty client ID", func(t *testing.T) { + // Save original config state + originalAuthType := config.AuthType() + originalClientID := config.ClientID() + + defer func() { + config.SetAuthType(originalAuthType) + config.SetClientID(originalClientID) + }() + + config.SetAuthType(config.ServiceAccount) + config.SetClientID("") + + authType, authSubject, err := authTypeAndSubject() + + assert.Equal(t, "service account", authType) + assert.Equal(t, "", authSubject) + assert.NoError(t, err) + }) + + t.Run("User Account with malformed access token", func(t *testing.T) { + // Save original config state + originalAuthType := config.AuthType() + originalAccessToken := config.AccessToken() + + defer func() { + config.SetAuthType(originalAuthType) + config.SetAccessToken(originalAccessToken) + }() + + config.SetAuthType(config.UserAccount) + config.SetAccessToken("invalid-token") + + authType, authSubject, err := authTypeAndSubject() + + assert.Equal(t, "account", authType) + assert.Equal(t, "", authSubject) // AccessTokenSubject will return empty for invalid token + assert.NoError(t, err) + }) +} diff --git a/internal/cli/organizations/create.go b/internal/cli/organizations/create.go index 2d2a78c419..1cdf361930 100644 --- a/internal/cli/organizations/create.go +++ b/internal/cli/organizations/create.go @@ -127,6 +127,8 @@ func (opts *CreateOpts) validateAuthType() error { return opts.validateOAuthRequirements() case config.ServiceAccount: return opts.validateOAuthRequirements() + case config.NoAuth: + fallthrough default: return nil } diff --git a/internal/cli/setup/setup_cmd.go b/internal/cli/setup/setup_cmd.go index 9f72a1e568..3c20d29504 100644 --- a/internal/cli/setup/setup_cmd.go +++ b/internal/cli/setup/setup_cmd.go @@ -586,8 +586,13 @@ func (opts *Opts) PreRun(ctx context.Context) error { if err := opts.login.RefreshAccessToken(ctx); !commonerrors.IsInvalidRefreshToken(err) { return nil } + case config.NoAuth: + fallthrough + default: + opts.skipLogin = false + return nil } - opts.skipLogin = false + return nil } diff --git a/internal/store/store.go b/internal/store/store.go index 58150fabdd..b0dbbb0d20 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -73,6 +73,8 @@ func HTTPClient(c CredentialsGetter, httpTransport http.RoundTripper) (*http.Cli return &http.Client{Transport: tr}, nil case config.ServiceAccount: return transport.NewServiceAccountClient(c.ClientID(), c.ClientSecret()), nil + case config.NoAuth: + fallthrough default: return &http.Client{Transport: httpTransport}, nil } diff --git a/internal/telemetry/event.go b/internal/telemetry/event.go index 5ea8169a91..a0554f53d3 100644 --- a/internal/telemetry/event.go +++ b/internal/telemetry/event.go @@ -220,6 +220,8 @@ func withAuthMethod(c Authenticator) EventOpt { event.Properties["auth_method"] = "oauth" case config.ServiceAccount: event.Properties["auth_method"] = "service_account" + case config.NoAuth: + event.Properties["auth_method"] = "no_auth" } } } From 1905f17279209abca896bb2e5fcfe58c044bc01b Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 14:36:39 +0100 Subject: [PATCH 04/16] cleanup --- internal/cli/auth/logout.go | 23 +- internal/cli/auth/logout_test.go | 355 +++++++------------------------ 2 files changed, 88 insertions(+), 290 deletions(-) diff --git a/internal/cli/auth/logout.go b/internal/cli/auth/logout.go index 8617539505..8eed86f8ce 100644 --- a/internal/cli/auth/logout.go +++ b/internal/cli/auth/logout.go @@ -70,18 +70,6 @@ func (opts *logoutOpts) Run(ctx context.Context) error { case config.APIKeys: opts.config.SetPublicAPIKey("") opts.config.SetPrivateAPIKey("") - case config.ServiceAccount: - fallthrough - case config.UserAccount: - // revoking a refresh token revokes the access token - if refreshToken := config.RefreshToken(); refreshToken != "" && opts.flow != nil { - if _, err := opts.flow.RevokeToken(ctx, refreshToken, "refresh_token"); err != nil { - return err - } - } - - opts.config.SetAccessToken("") - opts.config.SetRefreshToken("") case config.NoAuth: // Handle profiles with no authentication configured // Just clear any potential leftover credentials @@ -89,6 +77,14 @@ func (opts *logoutOpts) Run(ctx context.Context) error { opts.config.SetPrivateAPIKey("") opts.config.SetAccessToken("") opts.config.SetRefreshToken("") + case config.ServiceAccount: + fallthrough + case config.UserAccount: + if _, err := opts.flow.RevokeToken(ctx, config.RefreshToken(), "refresh_token"); err != nil { + return err + } + opts.config.SetAccessToken("") + opts.config.SetRefreshToken("") } opts.config.SetProjectID("") @@ -144,8 +140,7 @@ func LogoutBuilder() *cobra.Command { message = "Are you sure you want to log out of account %s?" case config.NoAuth: - // Handle profiles with no authentication configured - entry = "profile" + entry = config.Name() message = "Are you sure you want to clear profile %s?" } diff --git a/internal/cli/auth/logout_test.go b/internal/cli/auth/logout_test.go index 4b6556038e..3a443a4bcc 100644 --- a/internal/cli/auth/logout_test.go +++ b/internal/cli/auth/logout_test.go @@ -18,13 +18,10 @@ package auth import ( "bytes" - "context" "testing" "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli" "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/config" - "github.com/spf13/cobra" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" ) @@ -45,6 +42,7 @@ func Test_logoutOpts_Run_UserAccount(t *testing.T) { }, } ctx := t.Context() + mockConfig. EXPECT(). AuthType(). @@ -57,14 +55,8 @@ func Test_logoutOpts_Run_UserAccount(t *testing.T) { Return(nil, nil). Times(1) - mockConfig. - EXPECT(). - SetAccessToken(""). - Times(1) - mockConfig. - EXPECT(). - SetRefreshToken(""). - Times(1) + mockTokenCleanUp(mockConfig) + mockProjectAndOrgCleanUp(mockConfig) mockConfig. EXPECT(). @@ -96,14 +88,9 @@ func Test_logoutOpts_Run_APIKeys(t *testing.T) { Return(config.APIKeys). Times(1) - mockConfig. - EXPECT(). - SetPublicAPIKey(""). - Times(1) - mockConfig. - EXPECT(). - SetPrivateAPIKey(""). - Times(1) + mockApiKeysCleanUp(mockConfig) + mockProjectAndOrgCleanUp(mockConfig) + mockConfig. EXPECT(). Delete(). @@ -133,12 +120,18 @@ func Test_logoutOpts_Run_ServiceAccount(t *testing.T) { AuthType(). Return(config.ServiceAccount). Times(1) - mockConfig. EXPECT(). Delete(). Return(nil). Times(1) + mockFlow. + EXPECT(). + RevokeToken(ctx, gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) + mockTokenCleanUp(mockConfig) + mockProjectAndOrgCleanUp(mockConfig) require.NoError(t, opts.Run(ctx)) } @@ -171,22 +164,8 @@ func Test_logoutOpts_Run_Keep_UserAccount(t *testing.T) { Return(nil, nil). Times(1) - mockConfig. - EXPECT(). - SetAccessToken(""). - Times(1) - mockConfig. - EXPECT(). - SetRefreshToken(""). - Times(1) - mockConfig. - EXPECT(). - SetProjectID(""). - Times(1) - mockConfig. - EXPECT(). - SetOrgID(""). - Times(1) + mockTokenCleanUp(mockConfig) + mockProjectAndOrgCleanUp(mockConfig) mockConfig. EXPECT(). Save(). @@ -219,22 +198,8 @@ func Test_logoutOpts_Run_Keep_APIKeys(t *testing.T) { Return(config.APIKeys). Times(1) - mockConfig. - EXPECT(). - SetPublicAPIKey(""). - Times(1) - mockConfig. - EXPECT(). - SetPrivateAPIKey(""). - Times(1) - mockConfig. - EXPECT(). - SetProjectID(""). - Times(1) - mockConfig. - EXPECT(). - SetOrgID(""). - Times(1) + mockApiKeysCleanUp(mockConfig) + mockProjectAndOrgCleanUp(mockConfig) mockConfig. EXPECT(). Save(). @@ -267,14 +232,14 @@ func Test_logoutOpts_Run_Keep_ServiceAccount(t *testing.T) { Return(config.ServiceAccount). Times(1) - mockConfig. - EXPECT(). - SetProjectID(""). - Times(1) - mockConfig. + mockFlow. EXPECT(). - SetOrgID(""). + RevokeToken(ctx, gomock.Any(), gomock.Any()). + Return(nil, nil). Times(1) + + mockTokenCleanUp(mockConfig) + mockProjectAndOrgCleanUp(mockConfig) mockConfig. EXPECT(). Save(). @@ -284,233 +249,71 @@ func Test_logoutOpts_Run_Keep_ServiceAccount(t *testing.T) { require.NoError(t, opts.Run(ctx)) } -func Test_LogoutBuilder_PreRunE(t *testing.T) { - t.Run("successful prerun", func(t *testing.T) { - cmd := LogoutBuilder() - - // Create a test command context - testCmd := &cobra.Command{} - buf := new(bytes.Buffer) - testCmd.SetOut(buf) +func Test_logoutOpts_Run_NoAuth(t *testing.T) { + ctrl := gomock.NewController(t) + mockFlow := NewMockRevoker(ctrl) + mockConfig := NewMockConfigDeleter(ctrl) - // Execute PreRunE - err := cmd.PreRunE(testCmd, []string{}) + buf := new(bytes.Buffer) - // Should not return an error - assert.NoError(t, err) - }) -} + opts := logoutOpts{ + OutWriter: buf, + config: mockConfig, + flow: mockFlow, + DeleteOpts: &cli.DeleteOpts{ + Confirm: true, + }, + keepConfig: false, + } + ctx := t.Context() + mockConfig. + EXPECT(). + AuthType(). + Return(config.NoAuth). + Times(1) -func Test_logoutOpts_initFlow(t *testing.T) { - t.Run("successful flow initialization", func(t *testing.T) { - opts := &logoutOpts{} + mockTokenCleanUp(mockConfig) + mockProjectAndOrgCleanUp(mockConfig) + mockApiKeysCleanUp(mockConfig) - err := opts.initFlow() + mockConfig. + EXPECT(). + Delete(). + Return(nil). + Times(1) - // Should not return an error under normal conditions - assert.NoError(t, err) - assert.NotNil(t, opts.flow) - }) + require.NoError(t, opts.Run(ctx)) } -func Test_LogoutBuilder_RunE_ErrorHandling(t *testing.T) { - t.Run("no refresh token error for user account", func(t *testing.T) { - // Save original config state - originalRefreshToken := config.RefreshToken() - originalAuthType := config.AuthType() - defer func() { - config.SetRefreshToken(originalRefreshToken) - config.SetAuthType(originalAuthType) - }() - - // Set up UserAccount auth type but clear refresh token to trigger error - config.SetAuthType(config.UserAccount) - config.SetRefreshToken("") - - cmd := LogoutBuilder() - - // Create a test command context - testCmd := &cobra.Command{} - buf := new(bytes.Buffer) - testCmd.SetOut(buf) - - // Execute PreRunE first - err := cmd.PreRunE(testCmd, []string{}) - assert.NoError(t, err) - - // Execute RunE - should return ErrUnauthenticated - err = cmd.RunE(testCmd, []string{}) - assert.ErrorIs(t, err, ErrUnauthenticated) - }) - - t.Run("api keys flow validates properly", func(t *testing.T) { - // Save original config state - originalAuthType := config.AuthType() - originalPublicKey := config.PublicAPIKey() - defer func() { - config.SetAuthType(originalAuthType) - config.SetPublicAPIKey(originalPublicKey) - }() - - // Set up API key configuration - config.SetAuthType(config.APIKeys) - config.SetPublicAPIKey("test-public-key") - - cmd := LogoutBuilder() - - // Create a test command context - testCmd := &cobra.Command{} - buf := new(bytes.Buffer) - testCmd.SetOut(buf) - - // Execute PreRunE first - err := cmd.PreRunE(testCmd, []string{}) - assert.NoError(t, err) - - // For API keys, the RunE should work without refresh token - // Note: This would normally prompt for confirmation, but we're just testing structure - assert.NotNil(t, cmd.RunE) - }) +func mockApiKeysCleanUp(mockConfig *MockConfigDeleter) { + mockConfig. + EXPECT(). + SetPublicAPIKey(""). + Times(1) + mockConfig. + EXPECT(). + SetPrivateAPIKey(""). + Times(1) } -func Test_LogoutBuilder_Integration(t *testing.T) { - t.Run("command structure validation", func(t *testing.T) { - cmd := LogoutBuilder() - - // Verify command metadata - assert.Equal(t, "logout", cmd.Use) - assert.Contains(t, cmd.Short, "Log out") - assert.NotEmpty(t, cmd.Example) - - // Verify command functions are set - assert.NotNil(t, cmd.PreRunE) - assert.NotNil(t, cmd.RunE) - - // Verify flags are configured - assert.True(t, cmd.Flags().Lookup("force") != nil) - assert.True(t, cmd.Flags().Lookup("keep") != nil) - - // Verify the keep flag is hidden - keepFlag := cmd.Flags().Lookup("keep") - assert.NotNil(t, keepFlag) - assert.True(t, keepFlag.Hidden) - }) +func mockTokenCleanUp(mockConfig *MockConfigDeleter) { + mockConfig. + EXPECT(). + SetRefreshToken(""). + Times(1) + mockConfig. + EXPECT(). + SetAccessToken(""). + Times(1) } -func Test_LogoutBuilder_NoAuth_Handling(t *testing.T) { - t.Run("handles NoAuth profiles gracefully", func(t *testing.T) { - // Save original config state - originalAuthType := config.AuthType() - originalPublicKey := config.PublicAPIKey() - originalPrivateKey := config.PrivateAPIKey() - originalAccessToken := config.AccessToken() - originalRefreshToken := config.RefreshToken() - originalProjectID := config.ProjectID() - originalOrgID := config.OrgID() - - defer func() { - config.SetAuthType(originalAuthType) - config.SetPublicAPIKey(originalPublicKey) - config.SetPrivateAPIKey(originalPrivateKey) - config.SetAccessToken(originalAccessToken) - config.SetRefreshToken(originalRefreshToken) - config.SetProjectID(originalProjectID) - config.SetOrgID(originalOrgID) - }() - - // Set up NoAuth configuration - config.SetAuthType(config.NoAuth) - config.SetPublicAPIKey("some-leftover-key") - config.SetPrivateAPIKey("some-leftover-secret") - config.SetAccessToken("some-leftover-token") - config.SetRefreshToken("some-leftover-refresh") - config.SetProjectID("some-project-id") - config.SetOrgID("some-org-id") - - cmd := LogoutBuilder() - - // Create a test command context - testCmd := &cobra.Command{} - buf := new(bytes.Buffer) - testCmd.SetOut(buf) - - // Execute PreRunE first - should not initialize OAuth flow for NoAuth - err := cmd.PreRunE(testCmd, []string{}) - assert.NoError(t, err) - - // Create logout opts for testing Run method directly - opts := &logoutOpts{ - DeleteOpts: cli.NewDeleteOpts("test success", "test fail"), - config: config.Default(), - keepConfig: true, // Don't delete config, just clear credentials - } - - // Execute Run method - ctx := context.Background() - err = opts.Run(ctx) - assert.NoError(t, err) - - // Verify all credentials are cleared - assert.Equal(t, "", config.PublicAPIKey()) - assert.Equal(t, "", config.PrivateAPIKey()) - assert.Equal(t, "", config.AccessToken()) - assert.Equal(t, "", config.RefreshToken()) - assert.Equal(t, "", config.ProjectID()) - assert.Equal(t, "", config.OrgID()) - }) - - t.Run("RunE handles NoAuth with appropriate message", func(t *testing.T) { - // Save original config state - originalAuthType := config.AuthType() - defer func() { - config.SetAuthType(originalAuthType) - }() - - // Set up NoAuth configuration - config.SetAuthType(config.NoAuth) - - cmd := LogoutBuilder() - - // Create a test command context - testCmd := &cobra.Command{} - buf := new(bytes.Buffer) - testCmd.SetOut(buf) - - // Execute PreRunE first - err := cmd.PreRunE(testCmd, []string{}) - assert.NoError(t, err) - - // We can't easily test the full RunE without mocking the prompt, - // but we can verify the command structure handles NoAuth - assert.NotNil(t, cmd.RunE) - }) - - t.Run("PreRunE does not initialize OAuth flow for NoAuth", func(t *testing.T) { - // Save original config state - originalAuthType := config.AuthType() - defer func() { - config.SetAuthType(originalAuthType) - }() - - // Set up NoAuth configuration - config.SetAuthType(config.NoAuth) - - opts := &logoutOpts{} - - cmd := &cobra.Command{} - buf := new(bytes.Buffer) - cmd.SetOut(buf) - - // Simulate PreRunE logic - opts.config = config.Default() - - var flowInitialized bool - if opts.config.AuthType() == config.UserAccount || opts.config.AuthType() == config.ServiceAccount { - flowInitialized = true - } - - // Verify OAuth flow is not initialized for NoAuth - assert.False(t, flowInitialized) - assert.Nil(t, opts.flow) - }) +func mockProjectAndOrgCleanUp(mockConfig *MockConfigDeleter) { + mockConfig. + EXPECT(). + SetProjectID(""). + Times(1) + mockConfig. + EXPECT(). + SetOrgID(""). + Times(1) } From bf529c81e98f39c6e80dcc46ecc73214e2170a8a Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 14:39:10 +0100 Subject: [PATCH 05/16] update test --- internal/cli/auth/logout.go | 2 +- test/e2e/config/config_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/cli/auth/logout.go b/internal/cli/auth/logout.go index 8eed86f8ce..929e952a84 100644 --- a/internal/cli/auth/logout.go +++ b/internal/cli/auth/logout.go @@ -99,7 +99,7 @@ func (opts *logoutOpts) Run(ctx context.Context) error { func LogoutBuilder() *cobra.Command { opts := &logoutOpts{ - DeleteOpts: cli.NewDeleteOpts("Successfully logged out of account %s\n", " "), + DeleteOpts: cli.NewDeleteOpts("Successfully logged out of '%s'\n", " "), } cmd := &cobra.Command{ diff --git a/test/e2e/config/config_test.go b/test/e2e/config/config_test.go index 40ef4a907b..6784ecf0f1 100644 --- a/test/e2e/config/config_test.go +++ b/test/e2e/config/config_test.go @@ -201,7 +201,7 @@ func TestConfig(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v, resp: %v", err, string(resp)) } - const expected = "Profile 'renamed' deleted\n" + const expected = "Successfully logged out of 'renamed'\n" if string(resp) != expected { t.Errorf("expected %s, got %s\n", expected, string(resp)) } From 9d1a7abb39b41324a06286f1f34e11dbb036975c Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 14:44:30 +0100 Subject: [PATCH 06/16] fix prompts --- internal/cli/auth/logout.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/cli/auth/logout.go b/internal/cli/auth/logout.go index 929e952a84..88b4efafc3 100644 --- a/internal/cli/auth/logout.go +++ b/internal/cli/auth/logout.go @@ -77,9 +77,7 @@ func (opts *logoutOpts) Run(ctx context.Context) error { opts.config.SetPrivateAPIKey("") opts.config.SetAccessToken("") opts.config.SetRefreshToken("") - case config.ServiceAccount: - fallthrough - case config.UserAccount: + case config.ServiceAccount, config.UserAccount: if _, err := opts.flow.RevokeToken(ctx, config.RefreshToken(), "refresh_token"); err != nil { return err } @@ -124,9 +122,6 @@ func LogoutBuilder() *cobra.Command { switch opts.config.AuthType() { case config.APIKeys: entry = opts.config.PublicAPIKey() - if entry == "" { - entry = "API key account" - } message = "Are you sure you want to log out of account with public API key %s?" case config.ServiceAccount, config.UserAccount: entry, err = config.AccessTokenSubject() From 981e6a2ed588d68ded9472a43e7308997cb7fd8e Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 14:47:58 +0100 Subject: [PATCH 07/16] remove test --- internal/cli/auth/whoami_test.go | 170 ------------------------------- 1 file changed, 170 deletions(-) diff --git a/internal/cli/auth/whoami_test.go b/internal/cli/auth/whoami_test.go index 6e642cbfcb..20a6d8439f 100644 --- a/internal/cli/auth/whoami_test.go +++ b/internal/cli/auth/whoami_test.go @@ -20,7 +20,6 @@ import ( "bytes" "testing" - "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -35,172 +34,3 @@ func Test_whoOpts_Run(t *testing.T) { require.NoError(t, opts.Run()) assert.Equal(t, "Logged in as test@test.com account\n", buf.String()) } - -func Test_authTypeAndSubject(t *testing.T) { - tests := []struct { - name string - setupConfig func() - expectedType string - expectedSubject string - expectedError error - }{ - { - name: "API Keys authentication", - setupConfig: func() { - config.SetAuthType(config.APIKeys) - config.SetPublicAPIKey("test-public-key") - }, - expectedType: "key", - expectedSubject: "test-public-key", - expectedError: nil, - }, - { - name: "Service Account authentication", - setupConfig: func() { - config.SetAuthType(config.ServiceAccount) - config.SetClientID("test-client-id") - }, - expectedType: "service account", - expectedSubject: "test-client-id", - expectedError: nil, - }, - { - name: "User Account authentication", - setupConfig: func() { - config.SetAuthType(config.UserAccount) - config.SetAccessToken("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0QGV4YW1wbGUuY29tIiwibmFtZSI6IlRlc3QgVXNlciIsImlhdCI6MTUxNjIzOTAyMn0.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c") - }, - expectedType: "account", - expectedSubject: "test@example.com", - expectedError: nil, - }, - { - name: "NoAuth authentication", - setupConfig: func() { - config.SetAuthType(config.NoAuth) - }, - expectedType: "", - expectedSubject: "", - expectedError: ErrUnauthenticated, - }, - { - name: "Empty authentication type", - setupConfig: func() { - config.SetAuthType(config.AuthMechanism("")) - }, - expectedType: "", - expectedSubject: "", - expectedError: ErrUnauthenticated, - }, - { - name: "Unknown authentication type", - setupConfig: func() { - config.SetAuthType(config.AuthMechanism("unknown")) - }, - expectedType: "", - expectedSubject: "", - expectedError: ErrUnauthenticated, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Save original config state - originalAuthType := config.AuthType() - originalPublicKey := config.PublicAPIKey() - originalClientID := config.ClientID() - originalAccessToken := config.AccessToken() - - defer func() { - config.SetAuthType(originalAuthType) - config.SetPublicAPIKey(originalPublicKey) - config.SetClientID(originalClientID) - config.SetAccessToken(originalAccessToken) - }() - - // Clear config state - config.SetAuthType(config.AuthMechanism("")) - config.SetPublicAPIKey("") - config.SetClientID("") - config.SetAccessToken("") - - // Setup test configuration - tt.setupConfig() - - // Execute the function - authType, authSubject, err := authTypeAndSubject() - - // Verify results - assert.Equal(t, tt.expectedType, authType) - assert.Equal(t, tt.expectedSubject, authSubject) - - if tt.expectedError != nil { - assert.ErrorIs(t, err, tt.expectedError) - } else { - assert.NoError(t, err) - } - }) - } -} - -func Test_authTypeAndSubject_EdgeCases(t *testing.T) { - t.Run("API Keys with empty public key", func(t *testing.T) { - // Save original config state - originalAuthType := config.AuthType() - originalPublicKey := config.PublicAPIKey() - - defer func() { - config.SetAuthType(originalAuthType) - config.SetPublicAPIKey(originalPublicKey) - }() - - config.SetAuthType(config.APIKeys) - config.SetPublicAPIKey("") - - authType, authSubject, err := authTypeAndSubject() - - assert.Equal(t, "key", authType) - assert.Equal(t, "", authSubject) - assert.NoError(t, err) - }) - - t.Run("Service Account with empty client ID", func(t *testing.T) { - // Save original config state - originalAuthType := config.AuthType() - originalClientID := config.ClientID() - - defer func() { - config.SetAuthType(originalAuthType) - config.SetClientID(originalClientID) - }() - - config.SetAuthType(config.ServiceAccount) - config.SetClientID("") - - authType, authSubject, err := authTypeAndSubject() - - assert.Equal(t, "service account", authType) - assert.Equal(t, "", authSubject) - assert.NoError(t, err) - }) - - t.Run("User Account with malformed access token", func(t *testing.T) { - // Save original config state - originalAuthType := config.AuthType() - originalAccessToken := config.AccessToken() - - defer func() { - config.SetAuthType(originalAuthType) - config.SetAccessToken(originalAccessToken) - }() - - config.SetAuthType(config.UserAccount) - config.SetAccessToken("invalid-token") - - authType, authSubject, err := authTypeAndSubject() - - assert.Equal(t, "account", authType) - assert.Equal(t, "", authSubject) // AccessTokenSubject will return empty for invalid token - assert.NoError(t, err) - }) -} From 5552918cc7c4f96ef6b391063c466443681fb82b Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 15:03:14 +0100 Subject: [PATCH 08/16] remove double-text --- internal/cli/config/delete.go | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/internal/cli/config/delete.go b/internal/cli/config/delete.go index 4212d956dd..a3e0bb20b1 100644 --- a/internal/cli/config/delete.go +++ b/internal/cli/config/delete.go @@ -43,7 +43,6 @@ func (opts *DeleteOpts) executeLogout() error { cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - // Execute logout - this now handles all profile types gracefully return cmd.Run() } @@ -51,21 +50,7 @@ func (opts *DeleteOpts) Run() error { if !opts.Confirm { return nil } - - // Execute logout command for the profile before deletion - if err := opts.executeLogout(); err != nil { - return err - } - - if err := config.SetName(opts.Entry); err != nil { - return err - } - - if err := config.Delete(); err != nil { - return err - } - fmt.Printf(opts.SuccessMessage(), opts.Entry) - return nil + return opts.executeLogout() } func DeleteBuilder() *cobra.Command { From 26e5930cf05fbc9bdfd2ad7774535b58662ff4f1 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 15:10:12 +0100 Subject: [PATCH 09/16] init default profile with noauth --- internal/config/profile.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/config/profile.go b/internal/config/profile.go index a3043abe7c..b13f1950af 100644 --- a/internal/config/profile.go +++ b/internal/config/profile.go @@ -159,6 +159,7 @@ func IsTrue(s string) bool { } func Default() *Profile { + defaultProfile.SetAuthType(NoAuth) return defaultProfile } From 734f6d88467b32a080672556392d8c3cb7a0a1e0 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 15:11:49 +0100 Subject: [PATCH 10/16] update --- internal/config/profile.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/config/profile.go b/internal/config/profile.go index b13f1950af..ffc75f382e 100644 --- a/internal/config/profile.go +++ b/internal/config/profile.go @@ -159,7 +159,9 @@ func IsTrue(s string) bool { } func Default() *Profile { - defaultProfile.SetAuthType(NoAuth) + if defaultProfile.AuthType() == "" { + defaultProfile.SetAuthType(NoAuth) + } return defaultProfile } From e16a4c8d8b9c204a164266e167bccbc1d2749eea Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 16:12:11 +0100 Subject: [PATCH 11/16] update --- internal/cli/auth/logout.go | 2 +- internal/config/profile.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/cli/auth/logout.go b/internal/cli/auth/logout.go index 88b4efafc3..1e407cfa30 100644 --- a/internal/cli/auth/logout.go +++ b/internal/cli/auth/logout.go @@ -134,7 +134,7 @@ func LogoutBuilder() *cobra.Command { } message = "Are you sure you want to log out of account %s?" - case config.NoAuth: + case config.NoAuth, "": entry = config.Name() message = "Are you sure you want to clear profile %s?" } diff --git a/internal/config/profile.go b/internal/config/profile.go index ffc75f382e..a3043abe7c 100644 --- a/internal/config/profile.go +++ b/internal/config/profile.go @@ -159,9 +159,6 @@ func IsTrue(s string) bool { } func Default() *Profile { - if defaultProfile.AuthType() == "" { - defaultProfile.SetAuthType(NoAuth) - } return defaultProfile } From f3f7037b8c685afd567ea0d20d7875d5a152b4d4 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 8 Aug 2025 18:31:12 +0100 Subject: [PATCH 12/16] fix bug --- internal/cli/auth/logout.go | 32 ++++++++++++++++++++++---------- internal/config/viper_store.go | 2 +- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/internal/cli/auth/logout.go b/internal/cli/auth/logout.go index 1e407cfa30..5afedffff5 100644 --- a/internal/cli/auth/logout.go +++ b/internal/cli/auth/logout.go @@ -16,6 +16,7 @@ package auth import ( "context" + "fmt" "io" "net/http" @@ -34,6 +35,7 @@ import ( type ConfigDeleter interface { Delete() error + Name() string SetAccessToken(string) SetRefreshToken(string) SetProjectID(string) @@ -51,6 +53,7 @@ type Revoker interface { type logoutOpts struct { *cli.DeleteOpts + cli.DefaultSetterOpts OutWriter io.Writer config ConfigDeleter flow Revoker @@ -66,23 +69,25 @@ func (opts *logoutOpts) initFlow() error { } func (opts *logoutOpts) Run(ctx context.Context) error { + if !opts.Confirm { + return nil + } + switch opts.config.AuthType() { + case config.ServiceAccount, config.UserAccount: + if _, err := opts.flow.RevokeToken(ctx, config.RefreshToken(), "refresh_token"); err != nil { + return err + } + opts.config.SetAccessToken("") + opts.config.SetRefreshToken("") case config.APIKeys: opts.config.SetPublicAPIKey("") opts.config.SetPrivateAPIKey("") - case config.NoAuth: - // Handle profiles with no authentication configured - // Just clear any potential leftover credentials + case config.NoAuth, "": // Just clear any potential leftover credentials opts.config.SetPublicAPIKey("") opts.config.SetPrivateAPIKey("") opts.config.SetAccessToken("") opts.config.SetRefreshToken("") - case config.ServiceAccount, config.UserAccount: - if _, err := opts.flow.RevokeToken(ctx, config.RefreshToken(), "refresh_token"); err != nil { - return err - } - opts.config.SetAccessToken("") - opts.config.SetRefreshToken("") } opts.config.SetProjectID("") @@ -107,12 +112,19 @@ func LogoutBuilder() *cobra.Command { atlas auth logout `, PreRunE: func(cmd *cobra.Command, _ []string) error { + // Check if profile was provided and if it exists + profile := cmd.Flag(flag.Profile).Value.String() + if profile != "" && !config.Exists(profile) { + return fmt.Errorf("profile %v does not exist", profile) + } opts.OutWriter = cmd.OutOrStdout() opts.config = config.Default() + // Only initialize OAuth flow if we have OAuth-based auth if opts.config.AuthType() == config.UserAccount || opts.config.AuthType() == config.ServiceAccount { return opts.initFlow() } + return nil }, RunE: func(cmd *cobra.Command, _ []string) error { @@ -135,7 +147,7 @@ func LogoutBuilder() *cobra.Command { message = "Are you sure you want to log out of account %s?" case config.NoAuth, "": - entry = config.Name() + entry = opts.config.Name() message = "Are you sure you want to clear profile %s?" } diff --git a/internal/config/viper_store.go b/internal/config/viper_store.go index 1d8bbf6340..266b61c73f 100644 --- a/internal/config/viper_store.go +++ b/internal/config/viper_store.go @@ -165,7 +165,7 @@ func (s *ViperConfigStore) RenameProfile(oldProfileName string, newProfileName s func (s *ViperConfigStore) DeleteProfile(profileName string) error { // Configuration needs to be deleted from toml, as viper doesn't support this yet. // FIXME :: change when https://github.com/spf13/viper/pull/519 is merged. - settings := viper.AllSettings() + settings := s.viper.AllSettings() t, err := toml.TreeFromMap(settings) if err != nil { From 170daf23aa1d64f4240823830c84f0c92e408853 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Mon, 11 Aug 2025 10:44:26 +0100 Subject: [PATCH 13/16] update mocks --- internal/cli/auth/logout_mock_test.go | 38 +++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/internal/cli/auth/logout_mock_test.go b/internal/cli/auth/logout_mock_test.go index 4004b80aa9..85b06f39d6 100644 --- a/internal/cli/auth/logout_mock_test.go +++ b/internal/cli/auth/logout_mock_test.go @@ -118,6 +118,44 @@ func (c *MockConfigDeleterDeleteCall) DoAndReturn(f func() error) *MockConfigDel return c } +// Name mocks base method. +func (m *MockConfigDeleter) Name() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Name") + ret0, _ := ret[0].(string) + return ret0 +} + +// Name indicates an expected call of Name. +func (mr *MockConfigDeleterMockRecorder) Name() *MockConfigDeleterNameCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Name", reflect.TypeOf((*MockConfigDeleter)(nil).Name)) + return &MockConfigDeleterNameCall{Call: call} +} + +// MockConfigDeleterNameCall wrap *gomock.Call +type MockConfigDeleterNameCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockConfigDeleterNameCall) Return(arg0 string) *MockConfigDeleterNameCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockConfigDeleterNameCall) Do(f func() string) *MockConfigDeleterNameCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockConfigDeleterNameCall) DoAndReturn(f func() string) *MockConfigDeleterNameCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // PublicAPIKey mocks base method. func (m *MockConfigDeleter) PublicAPIKey() string { m.ctrl.T.Helper() From ec122dd2bb3b2b0ed7c93b04738372827fd35732 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Mon, 11 Aug 2025 10:55:50 +0100 Subject: [PATCH 14/16] add unit test --- internal/cli/auth/logout_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/cli/auth/logout_test.go b/internal/cli/auth/logout_test.go index 3a443a4bcc..20a8641849 100644 --- a/internal/cli/auth/logout_test.go +++ b/internal/cli/auth/logout_test.go @@ -22,6 +22,7 @@ import ( "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli" "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/config" + "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/flag" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" ) @@ -317,3 +318,21 @@ func mockProjectAndOrgCleanUp(mockConfig *MockConfigDeleter) { SetOrgID(""). Times(1) } + +func TestLogoutBuilder_PreRunE_ProfileNotExists(t *testing.T) { + cmd := LogoutBuilder() + + // Since profile is a persistent flag from the root command, we need to add it to test + cmd.Flags().StringP(flag.Profile, flag.ProfileShort, "", "profile to use") + + // Set the profile flag to a non-existing profile with a very unique name + // that's highly unlikely to exist in any test environment + nonExistentProfile := "definitely-non-existent-profile-12345" + err := cmd.Flags().Set(flag.Profile, nonExistentProfile) + require.NoError(t, err) + + // Test that PreRunE returns an error for non-existing profile + err = cmd.PreRunE(cmd, []string{}) + require.Error(t, err, "PreRunE should return error when profile does not exist") + require.Contains(t, err.Error(), "profile "+nonExistentProfile+" does not exist") +} From f831d9d1a2c6396fcb77fd25f55ad083ce9eb334 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Mon, 11 Aug 2025 10:56:34 +0100 Subject: [PATCH 15/16] update logout --- internal/cli/auth/logout.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/cli/auth/logout.go b/internal/cli/auth/logout.go index 5afedffff5..d8b5523f2f 100644 --- a/internal/cli/auth/logout.go +++ b/internal/cli/auth/logout.go @@ -112,12 +112,13 @@ func LogoutBuilder() *cobra.Command { atlas auth logout `, PreRunE: func(cmd *cobra.Command, _ []string) error { + opts.OutWriter = cmd.OutOrStdout() // Check if profile was provided and if it exists profile := cmd.Flag(flag.Profile).Value.String() if profile != "" && !config.Exists(profile) { return fmt.Errorf("profile %v does not exist", profile) } - opts.OutWriter = cmd.OutOrStdout() + opts.config = config.Default() // Only initialize OAuth flow if we have OAuth-based auth From 6baebdafb74294a0a77e933b7786070023692369 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Mon, 11 Aug 2025 11:01:24 +0100 Subject: [PATCH 16/16] remove check --- internal/cli/auth/logout.go | 7 ------- internal/cli/auth/logout_test.go | 19 ------------------- 2 files changed, 26 deletions(-) diff --git a/internal/cli/auth/logout.go b/internal/cli/auth/logout.go index d8b5523f2f..9eb13f34e8 100644 --- a/internal/cli/auth/logout.go +++ b/internal/cli/auth/logout.go @@ -16,7 +16,6 @@ package auth import ( "context" - "fmt" "io" "net/http" @@ -113,12 +112,6 @@ func LogoutBuilder() *cobra.Command { `, PreRunE: func(cmd *cobra.Command, _ []string) error { opts.OutWriter = cmd.OutOrStdout() - // Check if profile was provided and if it exists - profile := cmd.Flag(flag.Profile).Value.String() - if profile != "" && !config.Exists(profile) { - return fmt.Errorf("profile %v does not exist", profile) - } - opts.config = config.Default() // Only initialize OAuth flow if we have OAuth-based auth diff --git a/internal/cli/auth/logout_test.go b/internal/cli/auth/logout_test.go index 20a8641849..3a443a4bcc 100644 --- a/internal/cli/auth/logout_test.go +++ b/internal/cli/auth/logout_test.go @@ -22,7 +22,6 @@ import ( "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli" "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/config" - "github.com/mongodb/mongodb-atlas-cli/atlascli/internal/flag" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" ) @@ -318,21 +317,3 @@ func mockProjectAndOrgCleanUp(mockConfig *MockConfigDeleter) { SetOrgID(""). Times(1) } - -func TestLogoutBuilder_PreRunE_ProfileNotExists(t *testing.T) { - cmd := LogoutBuilder() - - // Since profile is a persistent flag from the root command, we need to add it to test - cmd.Flags().StringP(flag.Profile, flag.ProfileShort, "", "profile to use") - - // Set the profile flag to a non-existing profile with a very unique name - // that's highly unlikely to exist in any test environment - nonExistentProfile := "definitely-non-existent-profile-12345" - err := cmd.Flags().Set(flag.Profile, nonExistentProfile) - require.NoError(t, err) - - // Test that PreRunE returns an error for non-existing profile - err = cmd.PreRunE(cmd, []string{}) - require.Error(t, err, "PreRunE should return error when profile does not exist") - require.Contains(t, err.Error(), "profile "+nonExistentProfile+" does not exist") -}