diff --git a/cmd/auth/check.go b/cmd/auth/check.go index 5f0bd0f4..b6f99797 100644 --- a/cmd/auth/check.go +++ b/cmd/auth/check.go @@ -46,8 +46,7 @@ func authCheckRun(opts *CheckOptions) error { required := strings.Fields(opts.Scope) if len(required) == 0 { - output.PrintJson(f.IOStreams.Out, map[string]interface{}{"ok": true, "granted": []string{}, "missing": []string{}}) - return nil + return output.ErrValidation("--scope cannot be empty") } config, err := f.Config() diff --git a/cmd/auth/check_test.go b/cmd/auth/check_test.go new file mode 100644 index 00000000..6822438d --- /dev/null +++ b/cmd/auth/check_test.go @@ -0,0 +1,41 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "strings" + "testing" + + "github.com/larksuite/cli/internal/cmdutil" +) + +func TestAuthCheckRun_EmptyScopeReturnsValidationError(t *testing.T) { + tests := []struct { + name string + scope string + }{ + {name: "empty string", scope: ""}, + {name: "whitespace only", scope: " "}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, stdout, _, _ := cmdutil.TestFactory(t, nil) + + err := authCheckRun(&CheckOptions{ + Factory: f, + Scope: tt.scope, + }) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "--scope cannot be empty") { + t.Fatalf("unexpected error: %v", err) + } + if stdout.Len() != 0 { + t.Fatalf("expected no stdout, got %q", stdout.String()) + } + }) + } +} diff --git a/cmd/config/remove.go b/cmd/config/remove.go index 52c5eb0c..3172f1b7 100644 --- a/cmd/config/remove.go +++ b/cmd/config/remove.go @@ -4,23 +4,36 @@ package config import ( + "errors" "fmt" + "os" "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/keychain" "github.com/larksuite/cli/internal/output" "github.com/spf13/cobra" ) // ConfigRemoveOptions holds all inputs for config remove. type ConfigRemoveOptions struct { - Factory *cmdutil.Factory + Factory *cmdutil.Factory + LoadConfig func() (*core.MultiAppConfig, error) + SaveConfig func(*core.MultiAppConfig) error + RemoveSecret func(core.SecretInput, keychain.KeychainAccess) + RemoveStoredToken func(string, string) error } // NewCmdConfigRemove creates the config remove subcommand. func NewCmdConfigRemove(f *cmdutil.Factory, runF func(*ConfigRemoveOptions) error) *cobra.Command { - opts := &ConfigRemoveOptions{Factory: f} + opts := &ConfigRemoveOptions{ + Factory: f, + LoadConfig: core.LoadMultiAppConfig, + SaveConfig: core.SaveMultiAppConfig, + RemoveSecret: core.RemoveSecretStore, + RemoveStoredToken: auth.RemoveStoredToken, + } cmd := &cobra.Command{ Use: "remove", @@ -37,35 +50,62 @@ func NewCmdConfigRemove(f *cmdutil.Factory, runF func(*ConfigRemoveOptions) erro } func configRemoveRun(opts *ConfigRemoveOptions) error { + if opts == nil || opts.Factory == nil || + opts.LoadConfig == nil || opts.SaveConfig == nil || + opts.RemoveSecret == nil || opts.RemoveStoredToken == nil { + return output.Errorf(output.ExitInternal, "internal", "config remove options not initialized") + } + f := opts.Factory - config, err := core.LoadMultiAppConfig() - if err != nil || config == nil || len(config.Apps) == 0 { + config, err := opts.LoadConfig() + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return output.ErrValidation("not configured yet") + } + return output.Errorf(output.ExitInternal, "internal", "failed to load config: %v", err) + } + if config == nil || len(config.Apps) == 0 { return output.ErrValidation("not configured yet") } // Save empty config first. If this fails, keep secrets and tokens intact so the // existing config can still be retried instead of ending up half-removed. empty := &core.MultiAppConfig{Apps: []core.AppConfig{}} - if err := core.SaveMultiAppConfig(empty); err != nil { + if err := opts.SaveConfig(empty); err != nil { return output.Errorf(output.ExitInternal, "internal", "failed to save config: %v", err) } // Clean up keychain entries for all apps after config is cleared. + tokenTargets := 0 + tokenRemoved := 0 + tokenFailures := 0 for _, app := range config.Apps { - core.RemoveSecretStore(app.AppSecret, f.Keychain) + opts.RemoveSecret(app.AppSecret, f.Keychain) for _, user := range app.Users { - _ = auth.RemoveStoredToken(app.AppId, user.UserOpenId) + tokenTargets++ + if err := opts.RemoveStoredToken(app.AppId, user.UserOpenId); err != nil { + tokenFailures++ + fmt.Fprintf(f.IOStreams.ErrOut, "warning: failed to remove a stored token for app %q: %v\n", app.AppId, err) + continue + } + tokenRemoved++ } } output.PrintSuccess(f.IOStreams.ErrOut, "Configuration removed") - userCount := 0 - for _, app := range config.Apps { - userCount += len(app.Users) - } - if userCount > 0 { - fmt.Fprintf(f.IOStreams.ErrOut, "Cleared tokens for %d users\n", userCount) + if tokenTargets > 0 { + if tokenFailures == 0 { + fmt.Fprintf(f.IOStreams.ErrOut, "Cleared tokens for %d users\n", tokenRemoved) + } else { + fmt.Fprintf( + f.IOStreams.ErrOut, + "Token cleanup attempted for %d users: removed %d, failed %d\n", + tokenTargets, + tokenRemoved, + tokenFailures, + ) + } } return nil } diff --git a/cmd/config/remove_test.go b/cmd/config/remove_test.go new file mode 100644 index 00000000..e904a75f --- /dev/null +++ b/cmd/config/remove_test.go @@ -0,0 +1,203 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package config + +import ( + "errors" + "os" + "strings" + "testing" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/keychain" +) + +func TestConfigRemoveRun_UsesInjectedCallbacksAndSavesBeforeCleanup(t *testing.T) { + f, _, stderr, _ := cmdutil.TestFactory(t, nil) + + cfg := &core.MultiAppConfig{ + Apps: []core.AppConfig{ + { + AppId: "cli_app_a", + AppSecret: core.SecretInput{Ref: &core.SecretRef{Source: "keychain", ID: "appsecret:cli_app_a"}}, + Users: []core.AppUser{{UserOpenId: "ou_a"}}, + }, + { + AppId: "cli_app_b", + AppSecret: core.SecretInput{Ref: &core.SecretRef{Source: "keychain", ID: "appsecret:cli_app_b"}}, + Users: []core.AppUser{{UserOpenId: "ou_b1"}, {UserOpenId: "ou_b2"}}, + }, + }, + } + + callOrder := []string{} + saveConfig := func(next *core.MultiAppConfig) error { + callOrder = append(callOrder, "save") + if len(next.Apps) != 0 { + t.Fatalf("expected empty config, got %+v", next.Apps) + } + return nil + } + + var secretRemovals []string + removeSecret := func(input core.SecretInput, kc keychain.KeychainAccess) { + callOrder = append(callOrder, "secret") + if input.Ref != nil { + secretRemovals = append(secretRemovals, input.Ref.ID) + } + } + + var tokenRemovals []string + removeStoredToken := func(appID, userOpenID string) error { + callOrder = append(callOrder, "token") + tokenRemovals = append(tokenRemovals, appID+":"+userOpenID) + return nil + } + + if err := configRemoveRun(&ConfigRemoveOptions{ + Factory: f, + LoadConfig: func() (*core.MultiAppConfig, error) { return cfg, nil }, + SaveConfig: saveConfig, + RemoveSecret: removeSecret, + RemoveStoredToken: removeStoredToken, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(callOrder) == 0 || callOrder[0] != "save" { + t.Fatalf("expected save to happen first, order=%v", callOrder) + } + if len(secretRemovals) != 2 { + t.Fatalf("secret removals = %v, want 2 entries", secretRemovals) + } + if len(tokenRemovals) != 3 { + t.Fatalf("token removals = %v, want 3 entries", tokenRemovals) + } + got := stderr.String() + if !strings.Contains(got, "Configuration removed") { + t.Fatalf("expected success message on stderr, got %q", got) + } + if !strings.Contains(got, "Cleared tokens for 3 users") { + t.Fatalf("expected exact cleanup summary, got %q", got) + } +} + +func TestConfigRemoveRun_WarnsWhenTokenCleanupFails(t *testing.T) { + f, _, stderr, _ := cmdutil.TestFactory(t, nil) + + loadConfig := func() (*core.MultiAppConfig, error) { + return &core.MultiAppConfig{ + Apps: []core.AppConfig{{ + AppId: "cli_app", + AppSecret: core.SecretInput{Ref: &core.SecretRef{Source: "keychain", ID: "appsecret:cli_app"}}, + Users: []core.AppUser{{UserOpenId: "ou_123"}}, + }}, + }, nil + } + + if err := configRemoveRun(&ConfigRemoveOptions{ + Factory: f, + LoadConfig: loadConfig, + SaveConfig: func(*core.MultiAppConfig) error { return nil }, + RemoveSecret: func(core.SecretInput, keychain.KeychainAccess) {}, + RemoveStoredToken: func(appID, userOpenID string) error { + return errors.New("keychain unavailable") + }, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + got := stderr.String() + if !strings.Contains(got, `warning: failed to remove a stored token for app "cli_app": keychain unavailable`) { + t.Fatalf("expected token cleanup warning, got %q", got) + } + if strings.Contains(got, "ou_123") { + t.Fatalf("warning should not leak the user identifier, got %q", got) + } + if strings.Contains(got, "Cleared tokens for 1 users") { + t.Fatalf("cleanup summary should not claim full success, got %q", got) + } + if !strings.Contains(got, "Token cleanup attempted for 1 users: removed 0, failed 1") { + t.Fatalf("expected partial cleanup summary, got %q", got) + } +} + +func TestConfigRemoveRun_DoesNotCleanupWhenSaveFails(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, nil) + + cfg := &core.MultiAppConfig{ + Apps: []core.AppConfig{{ + AppId: "cli_app", + AppSecret: core.SecretInput{Ref: &core.SecretRef{Source: "keychain", ID: "appsecret:cli_app"}}, + Users: []core.AppUser{{UserOpenId: "ou_123"}}, + }}, + } + + secretCalls := 0 + tokenCalls := 0 + err := configRemoveRun(&ConfigRemoveOptions{ + Factory: f, + LoadConfig: func() (*core.MultiAppConfig, error) { return cfg, nil }, + SaveConfig: func(*core.MultiAppConfig) error { return errors.New("disk full") }, + RemoveSecret: func(core.SecretInput, keychain.KeychainAccess) { + secretCalls++ + }, + RemoveStoredToken: func(string, string) error { + tokenCalls++ + return nil + }, + }) + if err == nil { + t.Fatal("expected save failure") + } + if !strings.Contains(err.Error(), "failed to save config: disk full") { + t.Fatalf("unexpected error: %v", err) + } + if secretCalls != 0 { + t.Fatalf("expected no secret cleanup on save failure, got %d calls", secretCalls) + } + if tokenCalls != 0 { + t.Fatalf("expected no token cleanup on save failure, got %d calls", tokenCalls) + } +} + +func TestConfigRemoveRun_DistinguishesLoadErrors(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, nil) + + notConfiguredErr := configRemoveRun(&ConfigRemoveOptions{ + Factory: f, + LoadConfig: func() (*core.MultiAppConfig, error) { return nil, os.ErrNotExist }, + SaveConfig: func(*core.MultiAppConfig) error { return nil }, + RemoveSecret: func(core.SecretInput, keychain.KeychainAccess) {}, + RemoveStoredToken: func(string, string) error { return nil }, + }) + if notConfiguredErr == nil || !strings.Contains(notConfiguredErr.Error(), "not configured yet") { + t.Fatalf("expected not configured error, got %v", notConfiguredErr) + } + + loadErr := configRemoveRun(&ConfigRemoveOptions{ + Factory: f, + LoadConfig: func() (*core.MultiAppConfig, error) { return nil, errors.New("permission denied") }, + SaveConfig: func(*core.MultiAppConfig) error { return nil }, + RemoveSecret: func(core.SecretInput, keychain.KeychainAccess) {}, + RemoveStoredToken: func(string, string) error { return nil }, + }) + if loadErr == nil { + t.Fatal("expected load failure") + } + if !strings.Contains(loadErr.Error(), "failed to load config: permission denied") { + t.Fatalf("unexpected load error: %v", loadErr) + } +} + +func TestConfigRemoveRun_RejectsUninitializedOptions(t *testing.T) { + err := configRemoveRun(&ConfigRemoveOptions{}) + if err == nil { + t.Fatal("expected initialization error") + } + if !strings.Contains(err.Error(), "config remove options not initialized") { + t.Fatalf("unexpected error: %v", err) + } +}