Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cmd/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
73 changes: 73 additions & 0 deletions cmd/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// 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")
}
})
}
}
8 changes: 8 additions & 0 deletions cmd/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
68 changes: 68 additions & 0 deletions cmd/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// 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")
}
})
}
}
26 changes: 26 additions & 0 deletions internal/cmdutil/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Comment thread
MaxHuang22 marked this conversation as resolved.
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.",
)
}
80 changes: 80 additions & 0 deletions internal/cmdutil/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
}
37 changes: 37 additions & 0 deletions internal/credential/credential_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,43 @@
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()

Check warning on line 352 in internal/credential/credential_provider.go

View check run for this annotation

Codecov / codecov/patch

internal/credential/credential_provider.go#L352

Added line #L352 was not covered by tests
}
if name == "" {
name = "external"

Check warning on line 355 in internal/credential/credential_provider.go

View check run for this annotation

Codecov / codecov/patch

internal/credential/credential_provider.go#L355

Added line #L355 was not covered by tests
}
return name, nil
}
return "", err
}
if acct != nil {
if name := prov.Name(); name != "" {
return name, nil
}
return "external", nil

Check warning on line 365 in internal/credential/credential_provider.go

View check run for this annotation

Codecov / codecov/patch

internal/credential/credential_provider.go#L365

Added line #L365 was not covered by tests
}
}
return "", nil
}

func convertAccount(ext *extcred.Account) *Account {
return &Account{
AppID: ext.AppID,
Expand Down
Loading
Loading