fix(identity): correct identity diagnosis under external credential providers (#1693)

2026-07-03 14:02:43 +08:00 · 2026-06-30 21:56:03 +08:00
parent 75926f9744
commit 6f2cddfce1
7 changed files with 522 additions and 131 deletions
--- a/internal/identitydiag/diagnostics.go
+++ b/internal/identitydiag/diagnostics.go
@@ -13,6 +13,7 @@ import (
 	"strings"
 	"time"

+	extcred "github.com/larksuite/cli/extension/credential"
 	larkauth "github.com/larksuite/cli/internal/auth"
 	"github.com/larksuite/cli/internal/cmdutil"
 	"github.com/larksuite/cli/internal/core"
@@ -61,12 +62,131 @@ func Diagnose(ctx context.Context, f *cmdutil.Factory, cfg *core.CliConfig, veri
 	if ctx == nil {
 		ctx = context.Background()
 	}
+	// An external provider mints tokens on demand and blocks interactive auth,
+	// so the built-in keychain heuristics and "auth login" hints don't apply.
+	if provider := activeExternalProvider(ctx, f); provider != "" {
+		return diagnoseExternal(ctx, f, cfg, provider, verify)
+	}
 	return Result{
 		Bot:  diagnoseBot(ctx, f, cfg, verify),
 		User: diagnoseUser(ctx, f, cfg, verify),
 	}
 }

+// activeExternalProvider returns the active extension provider name, or "".
+// An error degrades to the built-in path: an unreachable provider would already
+// have failed the f.Config() that produced cfg.
+func activeExternalProvider(ctx context.Context, f *cmdutil.Factory) string {
+	if f == nil || f.Credential == nil {
+		return ""
+	}
+	name, err := f.Credential.ActiveExtensionProviderName(ctx)
+	if err != nil {
+		return ""
+	}
+	return name
+}
+
+func diagnoseExternal(ctx context.Context, f *cmdutil.Factory, cfg *core.CliConfig, provider string, verify bool) Result {
+	if cfg == nil || cfg.AppID == "" {
+		notConfigured := Identity{
+			Status:  StatusNotConfigured,
+			Message: "not configured (missing app config)",
+			Hint:    externalCredentialHint(provider),
+		}
+		return Result{Bot: notConfigured, User: notConfigured}
+	}
+	// SupportedIdentities == 0 is "unspecified" — treat as both, per CanBot.
+	ids := extcred.IdentitySupport(cfg.SupportedIdentities)
+	supportsBot := cfg.SupportedIdentities == 0 || ids.Has(extcred.SupportsBot)
+	supportsUser := cfg.SupportedIdentities == 0 || ids.Has(extcred.SupportsUser)
+	return Result{
+		Bot:  diagnoseExternalBot(ctx, f, cfg, provider, supportsBot, verify),
+		User: diagnoseExternalUser(ctx, f, cfg, provider, supportsUser, verify),
+	}
+}
+
+func diagnoseExternalBot(ctx context.Context, f *cmdutil.Factory, cfg *core.CliConfig, provider string, supported, verify bool) Identity {
+	if !supported {
+		return notProvidedExternally("Bot", provider)
+	}
+	id := Identity{Status: StatusReady, Available: true, Message: "Bot identity: ready (provided by " + provider + ")"}
+	if !verify {
+		return id
+	}
+	token, err := resolveBotToken(ctx, f, cfg)
+	if err != nil {
+		return externalVerifyFailed(id, "Bot", provider, err)
+	}
+	info, err := fetchBotInfo(ctx, f, cfg, token)
+	if err != nil {
+		return externalVerifyFailed(id, "Bot", provider, err)
+	}
+	id.Verified = boolPtr(true)
+	id.OpenID = info.OpenID
+	id.AppName = info.AppName
+	return id
+}
+
+func diagnoseExternalUser(ctx context.Context, f *cmdutil.Factory, cfg *core.CliConfig, provider string, supported, verify bool) Identity {
+	if !supported {
+		return notProvidedExternally("User", provider)
+	}
+	// enrichUserInfo populates UserOpenId only after the provider returns and
+	// verifies a UAT (and clears it on failure), so a resolved open id is the
+	// external analogue of a keychain token being present.
+	if cfg.UserOpenId == "" {
+		return Identity{
+			Status:  StatusMissing,
+			Message: "User identity: not signed in via credential source " + provider,
+			Hint:    externalCredentialHint(provider),
+		}
+	}
+	id := Identity{
+		Status:      StatusReady,
+		Available:   true,
+		TokenStatus: StatusReady,
+		UserName:    cfg.UserName,
+		OpenID:      cfg.UserOpenId,
+		Message:     "User identity: ready (provided by " + provider + ")",
+	}
+	if !verify {
+		return id
+	}
+	if _, err := f.Credential.ResolveToken(ctx, credential.NewTokenSpec(core.AsUser, cfg.AppID)); err != nil {
+		return externalVerifyFailed(id, "User", provider, err)
+	}
+	id.Verified = boolPtr(true)
+	return id
+}
+
+func notProvidedExternally(label, provider string) Identity {
+	return Identity{
+		Status:  StatusNotConfigured,
+		Message: label + " identity: not provided by credential source " + provider,
+		Hint:    externalCredentialHint(provider),
+	}
+}
+
+// externalVerifyFailed flips id to verify-failed, keeping any identity fields
+// (open id, user name) already resolved before the probe.
+func externalVerifyFailed(id Identity, label, provider string, err error) Identity {
+	id.Available = false
+	id.Verified = boolPtr(false)
+	id.Status = StatusVerifyFailed
+	id.TokenStatus = ""
+	id.Message = label + " identity: verify failed: " + err.Error()
+	id.Hint = externalCredentialHint(provider)
+	return id
+}
+
+// externalCredentialHint reports the constraint, not a remediation: the
+// identity is the provider's to manage, not lark-cli's to fix. What to do about
+// it is the caller's call — there may be no user to ask.
+func externalCredentialHint(provider string) string {
+	return fmt.Sprintf("managed by the external credential provider %q and cannot be configured via lark-cli", provider)
+}
+
 func diagnoseBot(ctx context.Context, f *cmdutil.Factory, cfg *core.CliConfig, verify bool) Identity {
 	if cfg == nil || cfg.AppID == "" {
 		return Identity{
--- a/internal/identitydiag/diagnostics_test.go
+++ b/internal/identitydiag/diagnostics_test.go
@@ -10,9 +10,11 @@ import (
 	"testing"
 	"time"

+	extcred "github.com/larksuite/cli/extension/credential"
 	larkauth "github.com/larksuite/cli/internal/auth"
 	"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/zalando/go-keyring"
 )
@@ -348,3 +350,136 @@ func TestDiagnose_UserIdentityNeedsRefresh(t *testing.T) {
 		t.Fatalf("token status = %q, want needs_refresh", got.User.TokenStatus)
 	}
 }
+
+// fakeExtProvider is a minimal credential.extcred.Provider for exercising the
+// external-credential diagnosis path. account makes the provider "active";
+// token (when set) satisfies ResolveToken during verify.
+type fakeExtProvider struct {
+	name    string
+	account *extcred.Account
+	token   *extcred.Token
+}
+
+func (p *fakeExtProvider) Name() string { return p.name }
+func (p *fakeExtProvider) ResolveAccount(context.Context) (*extcred.Account, error) {
+	return p.account, nil
+}
+func (p *fakeExtProvider) ResolveToken(context.Context, extcred.TokenSpec) (*extcred.Token, error) {
+	return p.token, nil
+}
+
+func externalFactory(prov *fakeExtProvider, cfg *core.CliConfig) *cmdutil.Factory {
+	cred := credential.NewCredentialProvider(
+		[]extcred.Provider{prov}, nil, nil,
+		func() (*http.Client, error) { return nil, nil },
+	)
+	return &cmdutil.Factory{
+		Config:     func() (*core.CliConfig, error) { return cfg, nil },
+		Credential: cred,
+		IOStreams:  &cmdutil.IOStreams{},
+	}
+}
+
+// assertExternalHint locks the contract that an external-provider hint never
+// points at interactive commands blocked under an external provider.
+func assertExternalHint(t *testing.T, hint string) {
+	t.Helper()
+	if hint == "" {
+		t.Fatalf("hint empty, want external guidance")
+	}
+	for _, blocked := range []string{"auth login", "config --help"} {
+		if strings.Contains(hint, blocked) {
+			t.Fatalf("hint %q must not point at %q (blocked under external provider)", hint, blocked)
+		}
+	}
+	if !strings.Contains(hint, "external") {
+		t.Fatalf("hint %q should explain credentials are external", hint)
+	}
+}
+
+func TestDiagnose_External_UserReady(t *testing.T) {
+	cfg := &core.CliConfig{AppID: "cli_x", Brand: core.BrandFeishu, SupportedIdentities: uint8(extcred.SupportsAll), UserOpenId: "ou_x", UserName: "Alice"}
+	f := externalFactory(&fakeExtProvider{name: "corp-sso", account: &extcred.Account{AppID: "cli_x"}}, cfg)
+
+	got := Diagnose(context.Background(), f, cfg, false)
+	// The bug this guards: the built-in path read the keychain (empty under an
+	// external provider) and reported the user as missing. Now availability
+	// follows the resolved account, so a signed-in user reads as ready.
+	if !got.User.Available || got.User.Status != StatusReady || got.User.TokenStatus != StatusReady {
+		t.Fatalf("user = %#v, want ready/available", got.User)
+	}
+	if got.User.OpenID != "ou_x" || got.User.UserName != "Alice" {
+		t.Fatalf("user identity = %#v", got.User)
+	}
+	if got.User.Hint != "" {
+		t.Fatalf("hint = %q, want empty when available", got.User.Hint)
+	}
+	if !got.Bot.Available || got.Bot.Status != StatusReady {
+		t.Fatalf("bot = %#v, want ready/available", got.Bot)
+	}
+}
+
+func TestDiagnose_External_UserNotSignedIn(t *testing.T) {
+	cfg := &core.CliConfig{AppID: "cli_x", Brand: core.BrandFeishu, SupportedIdentities: uint8(extcred.SupportsAll)}
+	f := externalFactory(&fakeExtProvider{name: "corp-sso", account: &extcred.Account{AppID: "cli_x"}}, cfg)
+
+	got := Diagnose(context.Background(), f, cfg, false)
+	if got.User.Available || got.User.Status != StatusMissing {
+		t.Fatalf("user = %#v, want missing/unavailable", got.User)
+	}
+	assertExternalHint(t, got.User.Hint)
+}
+
+func TestDiagnose_External_BotOnly(t *testing.T) {
+	cfg := &core.CliConfig{AppID: "cli_x", Brand: core.BrandFeishu, SupportedIdentities: uint8(extcred.SupportsBot), UserOpenId: "ou_x"}
+	f := externalFactory(&fakeExtProvider{name: "corp-sso", account: &extcred.Account{AppID: "cli_x"}}, cfg)
+
+	got := Diagnose(context.Background(), f, cfg, false)
+	if !got.Bot.Available || got.Bot.Status != StatusReady {
+		t.Fatalf("bot = %#v, want ready/available", got.Bot)
+	}
+	// Provider declares bot-only: user is unavailable even though an open id is
+	// present, and the hint is external (not "auth login").
+	if got.User.Available || got.User.Status != StatusNotConfigured {
+		t.Fatalf("user = %#v, want not_configured/unavailable", got.User)
+	}
+	assertExternalHint(t, got.User.Hint)
+}
+
+func TestDiagnose_External_UserOnly(t *testing.T) {
+	cfg := &core.CliConfig{AppID: "cli_x", Brand: core.BrandLark, SupportedIdentities: uint8(extcred.SupportsUser), UserOpenId: "ou_x", UserName: "Bob"}
+	f := externalFactory(&fakeExtProvider{name: "corp-sso", account: &extcred.Account{AppID: "cli_x"}}, cfg)
+
+	got := Diagnose(context.Background(), f, cfg, false)
+	if !got.User.Available || got.User.Status != StatusReady {
+		t.Fatalf("user = %#v, want ready/available", got.User)
+	}
+	if got.Bot.Available || got.Bot.Status != StatusNotConfigured {
+		t.Fatalf("bot = %#v, want not_configured/unavailable", got.Bot)
+	}
+	assertExternalHint(t, got.Bot.Hint)
+}
+
+func TestDiagnose_External_VerifyUserResolvesToken(t *testing.T) {
+	cfg := &core.CliConfig{AppID: "cli_x", Brand: core.BrandFeishu, SupportedIdentities: uint8(extcred.SupportsUser), UserOpenId: "ou_x", UserName: "Alice"}
+	f := externalFactory(&fakeExtProvider{name: "corp-sso", account: &extcred.Account{AppID: "cli_x"}, token: &extcred.Token{Value: "ext-uat"}}, cfg)
+
+	got := Diagnose(context.Background(), f, cfg, true)
+	if !got.User.Available || got.User.Verified == nil || !*got.User.Verified {
+		t.Fatalf("user = %#v, want available and verified", got.User)
+	}
+}
+
+func TestDiagnose_External_VerifyUserTokenUnavailable(t *testing.T) {
+	cfg := &core.CliConfig{AppID: "cli_x", Brand: core.BrandFeishu, SupportedIdentities: uint8(extcred.SupportsUser), UserOpenId: "ou_x"}
+	f := externalFactory(&fakeExtProvider{name: "corp-sso", account: &extcred.Account{AppID: "cli_x"}}, cfg)
+
+	got := Diagnose(context.Background(), f, cfg, true)
+	if got.User.Available || got.User.Status != StatusVerifyFailed {
+		t.Fatalf("user = %#v, want verify_failed/unavailable", got.User)
+	}
+	if got.User.Verified == nil || *got.User.Verified {
+		t.Fatalf("verified = %v, want false", got.User.Verified)
+	}
+	assertExternalHint(t, got.User.Hint)
+}