feat: block auth/config when external credential provider is active (#627)

* feat(credential): add ActiveExtensionProviderName to detect external providers

Change-Id: Ie17a4b714e5eca17ae574ac188d570721790107d

* feat(cmdutil): add RequireBuiltinCredentialProvider guard for external credential providers

Change-Id: I8f2ea0af6fe6506b29beb69264b04c21c0f75da1

* feat(config): block all config subcommands when external credential provider is active

Change-Id: If215cb8f0a53cc92d623dd3d842e4465124af2be

* feat(auth): block all auth subcommands when external credential provider is active

Change-Id: Ia61184fb2daeb6a7a38d122c647b7cb67eaf8b1f

* fix(auth,config): silence usage in PersistentPreRunE to match root command behaviour

Change-Id: I6d4b3c7d9d9c7b10fc2482fdc80252bf051771ee

* test(auth,config,credential): address CodeRabbit review comments

- Use cmd.Find() to assert SilenceUsage on matched subcommand (not parent)
- Add TestRequireBuiltinCredentialProvider_PropagatesProviderError for error path
- Add 'external' fallback sentinel in ActiveExtensionProviderName

Change-Id: Iba35779ad2ed9807556264ba23db7096541e2bf3
This commit is contained in:
MaxHuang22
2026-04-24 18:45:31 +08:00
committed by GitHub
parent fd4c35b10e
commit 7d0ceb5d58
8 changed files with 371 additions and 0 deletions

View File

@@ -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)

View File

@@ -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")
}
})
}
}

View File

@@ -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)

View File

@@ -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")
}
})
}
}