diff --git a/cmd/auth/auth.go b/cmd/auth/auth.go index 71fb58db7..0020eeb2f 100644 --- a/cmd/auth/auth.go +++ b/cmd/auth/auth.go @@ -24,6 +24,16 @@ func NewCmdAuth(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "auth", Short: "OAuth credentials and authorization management", + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + // Replicate rootCmd's PersistentPreRun behaviour: cobra stops at the first + // PersistentPreRun[E] found walking up the chain, so the root-level + // SilenceUsage=true would be skipped without this line. + cmd.SilenceUsage = true + // cmd.Name() returns the subcommand name (e.g. "login"), not "auth". + // Pass "auth" as a literal so the error message reads + // `"auth" is not supported: ...` + return f.RequireBuiltinCredentialProvider(cmd.Context(), "auth") + }, } cmdutil.DisableAuthCheck(cmd) diff --git a/cmd/auth/auth_test.go b/cmd/auth/auth_test.go index 3de6267ea..41a775145 100644 --- a/cmd/auth/auth_test.go +++ b/cmd/auth/auth_test.go @@ -5,15 +5,19 @@ package auth import ( "context" + "errors" + "io" "net/http" "sort" "strings" "testing" + extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/registry" ) @@ -303,3 +307,72 @@ func (r *authScopesTokenResolver) ResolveToken(ctx context.Context, req credenti return &credential.TokenResult{Token: "unexpected-token"}, nil } } + +// stubExternalProvider is a minimal extcred.Provider that always reports an account, +// simulating env/sidecar mode for guard tests. +type stubExternalProvider struct{ name string } + +func (s *stubExternalProvider) Name() string { return s.name } +func (s *stubExternalProvider) ResolveAccount(_ context.Context) (*extcred.Account, error) { + return &extcred.Account{AppID: "test-app"}, nil +} +func (s *stubExternalProvider) ResolveToken(_ context.Context, _ extcred.TokenSpec) (*extcred.Token, error) { + return nil, nil +} + +// newFactoryWithExternalProvider creates a Factory whose Credential uses a stub +// extension provider, simulating env/sidecar credential mode. +func newFactoryWithExternalProvider(t *testing.T) *cmdutil.Factory { + t.Helper() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + stub := &stubExternalProvider{name: "env"} + cred := credential.NewCredentialProvider([]extcred.Provider{stub}, nil, nil, nil) + f, _, _, _ := cmdutil.TestFactory(t, nil) + f.Credential = cred + return f +} + +func TestAuthBlockedByExternalProvider(t *testing.T) { + f := newFactoryWithExternalProvider(t) + + tests := []struct { + name string + args []string + }{ + {"login", []string{"login"}}, + {"logout", []string{"logout"}}, + {"status", []string{"status"}}, + {"check", []string{"check", "--scope", "calendar:read"}}, // --scope is required + {"list", []string{"list"}}, + {"scopes", []string{"scopes"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := NewCmdAuth(f) + cmd.SilenceErrors = true + cmd.SetErr(io.Discard) + cmd.SetArgs(tt.args) + + // Locate the subcommand before execution (PersistentPreRunE receives it as cmd). + matched, _, _ := cmd.Find(tt.args) + + err := cmd.Execute() + + // PersistentPreRunE sets SilenceUsage on the matched subcommand, not the parent. + if matched != nil && matched != cmd && !matched.SilenceUsage { + t.Error("expected PersistentPreRunE to set SilenceUsage on matched subcommand") + } + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected *output.ExitError, got %T: %v", err, err) + } + if exitErr.Code != output.ExitValidation { + t.Errorf("exit code = %d, want %d", exitErr.Code, output.ExitValidation) + } + if exitErr.Detail == nil || exitErr.Detail.Type != "external_provider" { + t.Errorf("error type = %v, want %q", exitErr.Detail, "external_provider") + } + }) + } +} diff --git a/cmd/config/config.go b/cmd/config/config.go index 275309609..d62c6b150 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -14,6 +14,14 @@ func NewCmdConfig(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "config", Short: "Global CLI configuration management", + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + // Replicate rootCmd's PersistentPreRun behaviour: cobra stops at the first + // PersistentPreRun[E] found walking up the chain, so the root-level + // SilenceUsage=true would be skipped without this line. + cmd.SilenceUsage = true + // Pass "config" as a literal — cmd.Name() would return the subcommand name. + return f.RequireBuiltinCredentialProvider(cmd.Context(), "config") + }, } cmdutil.DisableAuthCheck(cmd) diff --git a/cmd/config/config_test.go b/cmd/config/config_test.go index 2644467db..300865548 100644 --- a/cmd/config/config_test.go +++ b/cmd/config/config_test.go @@ -6,13 +6,16 @@ package config import ( "context" "errors" + "io" "os" "path/filepath" "strings" "testing" + extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/keychain" "github.com/larksuite/cli/internal/output" ) @@ -340,3 +343,68 @@ func TestUpdateExistingProfileWithoutSecret_RejectsAppIDChange(t *testing.T) { t.Fatalf("error = %v, want mention of App Secret", err) } } + +// stubConfigExtProvider simulates env/sidecar credential mode for config guard tests. +type stubConfigExtProvider struct{ name string } + +func (s *stubConfigExtProvider) Name() string { return s.name } +func (s *stubConfigExtProvider) ResolveAccount(_ context.Context) (*extcred.Account, error) { + return &extcred.Account{AppID: "test-app"}, nil +} +func (s *stubConfigExtProvider) ResolveToken(_ context.Context, _ extcred.TokenSpec) (*extcred.Token, error) { + return nil, nil +} + +func newConfigFactoryWithExternalProvider(t *testing.T) *cmdutil.Factory { + t.Helper() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + stub := &stubConfigExtProvider{name: "env"} + cred := credential.NewCredentialProvider([]extcred.Provider{stub}, nil, nil, nil) + f, _, _, _ := cmdutil.TestFactory(t, nil) + f.Credential = cred + return f +} + +func TestConfigBlockedByExternalProvider(t *testing.T) { + f := newConfigFactoryWithExternalProvider(t) + + tests := []struct { + name string + args []string + }{ + {"init", []string{"init", "--app-id", "x", "--app-secret-stdin"}}, + {"remove", []string{"remove"}}, + {"show", []string{"show"}}, + {"default-as", []string{"default-as", "user"}}, + {"strict-mode", []string{"strict-mode", "off"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := NewCmdConfig(f) + cmd.SilenceErrors = true + cmd.SetErr(io.Discard) + cmd.SetArgs(tt.args) + + // Locate the subcommand before execution (PersistentPreRunE receives it as cmd). + matched, _, _ := cmd.Find(tt.args) + + err := cmd.Execute() + + // PersistentPreRunE sets SilenceUsage on the matched subcommand, not the parent. + if matched != nil && matched != cmd && !matched.SilenceUsage { + t.Error("expected PersistentPreRunE to set SilenceUsage on matched subcommand") + } + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected *output.ExitError, got %T: %v", err, err) + } + if exitErr.Code != output.ExitValidation { + t.Errorf("exit code = %d, want %d", exitErr.Code, output.ExitValidation) + } + if exitErr.Detail == nil || exitErr.Detail.Type != "external_provider" { + t.Errorf("error type = %v, want %q", exitErr.Detail, "external_provider") + } + }) + } +} diff --git a/internal/cmdutil/factory.go b/internal/cmdutil/factory.go index 3f4759838..7f96eb7b7 100644 --- a/internal/cmdutil/factory.go +++ b/internal/cmdutil/factory.go @@ -199,3 +199,29 @@ func (f *Factory) NewAPIClientWithConfig(cfg *core.CliConfig) (*client.APIClient Credential: f.Credential, }, nil } + +// RequireBuiltinCredentialProvider returns a structured error (exit 2, code +// "external_provider") when an extension provider is actively managing credentials. +// Intended for use as PersistentPreRunE on the auth and config parent commands. +// +// Returns nil when: +// - f.Credential is nil (test environments without credential setup) +// - No extension provider is active (built-in keychain/config path is used) +func (f *Factory) RequireBuiltinCredentialProvider(ctx context.Context, command string) error { + if f.Credential == nil { + return nil + } + provName, err := f.Credential.ActiveExtensionProviderName(ctx) + if err != nil { + return err + } + if provName == "" { + return nil + } + return output.ErrWithHint( + output.ExitValidation, + "external_provider", + fmt.Sprintf("%q is not supported: credentials are provided externally and do not support interactive management", command), + "If another tool or method for authorization is available in this environment, try that. Otherwise, ask the user to set up credentials through the appropriate channel.", + ) +} diff --git a/internal/cmdutil/factory_test.go b/internal/cmdutil/factory_test.go index a0eec24f8..ff888b547 100644 --- a/internal/cmdutil/factory_test.go +++ b/internal/cmdutil/factory_test.go @@ -5,13 +5,17 @@ package cmdutil import ( "context" + "errors" "strings" "testing" "github.com/spf13/cobra" + extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/internal/output" ) // newCmdWithAsFlag creates a cobra.Command with a --as string flag for testing. @@ -355,3 +359,79 @@ func TestResolveAs_StrictModeBot_IgnoresDefaultAsUser(t *testing.T) { t.Errorf("bot mode should override default-as user, got %s", got) } } + +// stubExtProvider is a minimal extcred.Provider for testing external-provider guards. +type stubExtProvider struct { + name string + acct *extcred.Account + err error +} + +func (s *stubExtProvider) Name() string { return s.name } +func (s *stubExtProvider) ResolveAccount(_ context.Context) (*extcred.Account, error) { + return s.acct, s.err +} +func (s *stubExtProvider) ResolveToken(_ context.Context, _ extcred.TokenSpec) (*extcred.Token, error) { + return nil, nil +} + +func TestRequireBuiltinCredentialProvider_BlocksExternalProvider(t *testing.T) { + stub := &stubExtProvider{name: "env", acct: &extcred.Account{AppID: "app"}} + cred := credential.NewCredentialProvider([]extcred.Provider{stub}, nil, nil, nil) + f, _, _, _ := TestFactory(t, nil) + f.Credential = cred + + err := f.RequireBuiltinCredentialProvider(context.Background(), "auth") + if err == nil { + t.Fatal("expected error, got nil") + } + + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("error type = %T, want *output.ExitError", err) + } + if exitErr.Code != output.ExitValidation { + t.Errorf("exit code = %d, want %d", exitErr.Code, output.ExitValidation) + } + if exitErr.Detail == nil || exitErr.Detail.Type != "external_provider" { + t.Errorf("error type field = %v, want %q", exitErr.Detail, "external_provider") + } + if exitErr.Detail.Message == "" { + t.Error("expected non-empty message") + } + if exitErr.Detail.Hint == "" { + t.Error("expected non-empty hint") + } +} + +func TestRequireBuiltinCredentialProvider_AllowsBuiltinProvider(t *testing.T) { + // No extension providers → built-in path → no error + f, _, _, _ := TestFactory(t, nil) + err := f.RequireBuiltinCredentialProvider(context.Background(), "auth") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestRequireBuiltinCredentialProvider_NilCredential(t *testing.T) { + f, _, _, _ := TestFactory(t, nil) + f.Credential = nil + err := f.RequireBuiltinCredentialProvider(context.Background(), "auth") + if err != nil { + t.Fatalf("unexpected error with nil Credential: %v", err) + } +} + +func TestRequireBuiltinCredentialProvider_PropagatesProviderError(t *testing.T) { + sentinel := errors.New("provider unavailable") + stub := &stubExtProvider{name: "env", err: sentinel} + cred := credential.NewCredentialProvider([]extcred.Provider{stub}, nil, nil, nil) + + f, _, _, _ := TestFactory(t, nil) + f.Credential = cred + + err := f.RequireBuiltinCredentialProvider(context.Background(), "auth") + if !errors.Is(err, sentinel) { + t.Fatalf("error = %v, want sentinel", err) + } +} diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index 5d28e2314..442468d01 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -331,6 +331,43 @@ func (p *CredentialProvider) ResolveToken(ctx context.Context, req TokenSpec) (* return nil, &TokenUnavailableError{Type: req.Type} } +// ActiveExtensionProviderName reports whether an extension provider is managing +// credentials. It probes p.providers (extension providers only, not defaultAcct) +// and returns the name of the first engaged provider. +// +// "Engaged" means: ResolveAccount returns a non-nil account, OR returns a +// *extcred.BlockError (provider configured but misconfigured — still counts as +// external). Any other error is propagated to the caller. +// +// Returns ("", nil) when no extension provider is active (built-in keychain path). +// Safe to call multiple times — probes providers directly without the sync.Once cache. +func (p *CredentialProvider) ActiveExtensionProviderName(ctx context.Context) (string, error) { + for _, prov := range p.providers { + acct, err := prov.ResolveAccount(ctx) + if err != nil { + var blockErr *extcred.BlockError + if errors.As(err, &blockErr) { + name := blockErr.Provider + if name == "" { + name = prov.Name() + } + if name == "" { + name = "external" + } + return name, nil + } + return "", err + } + if acct != nil { + if name := prov.Name(); name != "" { + return name, nil + } + return "external", nil + } + } + return "", nil +} + func convertAccount(ext *extcred.Account) *Account { return &Account{ AppID: ext.AppID, diff --git a/internal/credential/credential_provider_test.go b/internal/credential/credential_provider_test.go index 509e83a7d..6dd13a985 100644 --- a/internal/credential/credential_provider_test.go +++ b/internal/credential/credential_provider_test.go @@ -422,3 +422,72 @@ func TestCredentialProvider_ResolveTokenDoesNotBypassFailedDefaultAccountResolut t.Fatalf("ResolveToken() error = %v, want config unavailable", err) } } + +func TestActiveExtensionProviderName_ExtActive(t *testing.T) { + cp := NewCredentialProvider( + []extcred.Provider{&mockExtProvider{name: "env", account: &extcred.Account{AppID: "app"}}}, + nil, nil, nil, + ) + name, err := cp.ActiveExtensionProviderName(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if name != "env" { + t.Errorf("got %q, want %q", name, "env") + } +} + +func TestActiveExtensionProviderName_BlockError(t *testing.T) { + cp := NewCredentialProvider( + []extcred.Provider{&mockExtProvider{ + name: "env", + accountErr: &extcred.BlockError{Provider: "env", Reason: "APP_ID missing"}, + }}, + nil, nil, nil, + ) + name, err := cp.ActiveExtensionProviderName(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if name != "env" { + t.Errorf("got %q, want %q", name, "env") + } +} + +func TestActiveExtensionProviderName_NoExtProvider(t *testing.T) { + cp := NewCredentialProvider(nil, nil, nil, nil) + name, err := cp.ActiveExtensionProviderName(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if name != "" { + t.Errorf("got %q, want empty string", name) + } +} + +func TestActiveExtensionProviderName_UnexpectedError(t *testing.T) { + sentinel := errors.New("network timeout") + cp := NewCredentialProvider( + []extcred.Provider{&mockExtProvider{name: "env", accountErr: sentinel}}, + nil, nil, nil, + ) + _, err := cp.ActiveExtensionProviderName(context.Background()) + if !errors.Is(err, sentinel) { + t.Errorf("got %v, want sentinel error", err) + } +} + +func TestActiveExtensionProviderName_SkipsNilProvider(t *testing.T) { + // nil account + nil error = provider not applicable; fallback returns "" + cp := NewCredentialProvider( + []extcred.Provider{&mockExtProvider{name: "sidecar"}}, // no account set → returns nil, nil + nil, nil, nil, + ) + name, err := cp.ActiveExtensionProviderName(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if name != "" { + t.Errorf("got %q, want empty string", name) + } +}