From 6f2cddfce1480dd264df9db9ac6f0800b5fbbcbf Mon Sep 17 00:00:00 2001 From: liangshuo-1 Date: Tue, 30 Jun 2026 21:56:03 +0800 Subject: [PATCH] fix(identity): correct identity diagnosis under external credential providers (#1693) --- cmd/doctor/doctor.go | 5 +- cmd/doctor/doctor_test.go | 83 ++++++++- cmd/whoami/whoami.go | 96 +++++----- cmd/whoami/whoami_test.go | 212 ++++++++++++++-------- internal/identitydiag/diagnostics.go | 120 ++++++++++++ internal/identitydiag/diagnostics_test.go | 135 ++++++++++++++ skills/lark-shared/SKILL.md | 2 +- 7 files changed, 522 insertions(+), 131 deletions(-) diff --git a/cmd/doctor/doctor.go b/cmd/doctor/doctor.go index a7ba30a4..bef93d03 100644 --- a/cmd/doctor/doctor.go +++ b/cmd/doctor/doctor.go @@ -129,7 +129,10 @@ func doctorRun(opts *DoctorOptions) error { if diagnostics.Bot.Available || diagnostics.User.Available { checks = append(checks, pass("identity_ready", "at least one identity is available")) } else { - checks = append(checks, fail("identity_ready", "no usable bot or user identity is available", "run: lark-cli auth status --verify")) + // No hint: this only summarizes the two checks above, which already carry + // the source-appropriate remediation. A command here would be redundant, + // or wrong (`auth status` is blocked under an external provider). + checks = append(checks, fail("identity_ready", "no usable bot or user identity is available", "")) } // ── 4 & 5. Endpoint reachability ── diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index 0f4fe8f7..76cfbd77 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -4,14 +4,19 @@ package doctor import ( + "bytes" "context" "encoding/json" + "net/http" + "strings" "testing" "github.com/spf13/cobra" + 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" ) func TestNewCmdDoctor_FlagParsing(t *testing.T) { @@ -140,14 +145,84 @@ func TestDoctorRun_SplitsBotAndMissingUserIdentity(t *testing.T) { } func assertCheck(t *testing.T, checks []checkResult, name, status string) { + t.Helper() + if got := findCheck(t, checks, name); got.Status != status { + t.Fatalf("%s status = %q, want %q", name, got.Status, status) + } +} + +func findCheck(t *testing.T, checks []checkResult, name string) checkResult { t.Helper() for _, check := range checks { if check.Name == name { - if check.Status != status { - t.Fatalf("%s status = %q, want %q", name, check.Status, status) - } - return + return check } } t.Fatalf("check %q not found in %#v", name, checks) + return checkResult{} +} + +type fakeExtProvider struct { + name string + account *extcred.Account +} + +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 nil, nil +} + +// Under an external credential provider with no usable identity, the +// identity_ready hint must not point at `auth status` (blocked there); the +// per-identity checks already carry the source-appropriate escalation. +func TestDoctor_ExternalProvider_IdentityReadyHintNotBlockedCommand(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := core.SaveMultiAppConfig(&core.MultiAppConfig{ + CurrentApp: "default", + Apps: []core.AppConfig{{Name: "default", AppId: "cli_x", AppSecret: core.PlainSecret("secret"), Brand: core.BrandFeishu}}, + }); err != nil { + t.Fatalf("SaveMultiAppConfig() error = %v", err) + } + + // Provider serves neither identity: bot unsupported, user supported but not + // signed in → both unavailable → identity_ready fails. + cfg := &core.CliConfig{AppID: "cli_x", Brand: core.BrandFeishu, SupportedIdentities: uint8(extcred.SupportsUser)} + cred := credential.NewCredentialProvider( + []extcred.Provider{&fakeExtProvider{name: "corp-sso", account: &extcred.Account{AppID: "cli_x"}}}, + nil, nil, + func() (*http.Client, error) { return nil, nil }, + ) + out := &bytes.Buffer{} + f := &cmdutil.Factory{ + Config: func() (*core.CliConfig, error) { return cfg, nil }, + Credential: cred, + IOStreams: &cmdutil.IOStreams{Out: out, ErrOut: &bytes.Buffer{}}, + } + + if err := doctorRun(&DoctorOptions{Factory: f, Ctx: context.Background(), Offline: true}); err == nil { + t.Fatalf("doctorRun() = nil, want failure when no identity is available") + } + var got struct { + Checks []checkResult `json:"checks"` + } + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatalf("json.Unmarshal() error = %v\n%s", err, out.String()) + } + + ready := findCheck(t, got.Checks, "identity_ready") + if ready.Status != "fail" { + t.Fatalf("identity_ready status = %q, want fail", ready.Status) + } + // The summary defers to the per-identity checks; it carries no hint of its + // own (a command here would be wrong under an external provider). + if ready.Hint != "" { + t.Fatalf("identity_ready should carry no hint, got %q", ready.Hint) + } + user := findCheck(t, got.Checks, "user_identity") + if !strings.Contains(user.Hint, "external") || strings.Contains(user.Hint, "auth login") { + t.Fatalf("user_identity hint not external-appropriate: %q", user.Hint) + } } diff --git a/cmd/whoami/whoami.go b/cmd/whoami/whoami.go index 3356cb18..d5b4b387 100644 --- a/cmd/whoami/whoami.go +++ b/cmd/whoami/whoami.go @@ -5,8 +5,6 @@ package whoami import ( "context" - "fmt" - "io" "github.com/spf13/cobra" @@ -17,6 +15,13 @@ import ( ) // whoamiResult is the structured output of `lark-cli whoami`. +// +// The self-vs-delegated distinction is carried by `identity`: a bot identity is +// the app acting as itself; a user identity is the app acting *on behalf of* a +// person (calls are attributed to that user, who is not necessarily present). +// onBehalfOf only *names* that person and so appears only once a user is +// resolved — a user identity that is not signed in still has identity "user" +// but no onBehalfOf yet. Do not read "no onBehalfOf" as "self"; read `identity`. type whoamiResult struct { Profile string `json:"profile"` AppID string `json:"appId"` @@ -26,34 +31,44 @@ type whoamiResult struct { IdentitySource string `json:"identitySource"` Available bool `json:"available"` TokenStatus string `json:"tokenStatus"` - OpenID string `json:"openId,omitempty"` - UserName string `json:"userName,omitempty"` + OnBehalfOf *delegatedUser `json:"onBehalfOf,omitempty"` Hint string `json:"hint,omitempty"` } +// delegatedUser is the user a user-identity acts on behalf of. +type delegatedUser struct { + UserName string `json:"userName,omitempty"` + OpenID string `json:"openId,omitempty"` +} + // Options holds inputs for the whoami command. type Options struct { Factory *cmdutil.Factory As string - JSON bool } // NewCmdWhoami creates the top-level whoami command. It reports the identity // that the next API call would actually use (resolved via Factory.ResolveAs), -// together with the active profile, app, and token status. It is local-only: -// no network calls are made. +// together with the active profile, app, and token status. Output is always +// JSON — whoami is consumed by agents. With the built-in credential path it is +// local-only; when an external credential provider manages tokens, resolving +// the identity may contact that provider. func NewCmdWhoami(f *cmdutil.Factory) *cobra.Command { opts := &Options{Factory: f} cmd := &cobra.Command{ Use: "whoami", - Short: "Show the current effective identity, app, profile, and token status", + Short: "Show the current effective identity, app, profile, and token status (JSON)", RunE: func(cmd *cobra.Command, args []string) error { return whoamiRun(cmd, opts) }, } cmdutil.DisableAuthCheck(cmd) cmdutil.AddAPIIdentityFlag(context.Background(), cmd, f, &opts.As) - cmd.Flags().BoolVar(&opts.JSON, "json", false, "structured JSON output") + // Output is always JSON. Accept (and ignore) --json so existing + // `whoami --json` callers don't break; hide it to avoid implying a non-JSON + // mode exists. + cmd.Flags().Bool("json", true, "deprecated: output is always JSON") + _ = cmd.Flags().MarkHidden("json") cmdutil.SetRisk(cmd, "read") return cmd } @@ -67,10 +82,11 @@ func whoamiRun(cmd *cobra.Command, opts *Options) error { ctx := cmd.Context() flagAs := core.Identity(opts.As) as := f.ResolveAs(ctx, cmd, flagAs) - // Reject an explicit --as that does not resolve to a usable identity, so a - // typo like `--as admin` fails clearly instead of echoing back a bogus - // identity. Keeps the §5.1 invariant (identity is always user or bot) and - // matches how api/service/shortcut commands validate the resolved identity. + // Validate as a real API call does (strict mode, then identity) so whoami + // can't preview an identity the next call would refuse. + if err := f.CheckStrictMode(ctx, as); err != nil { + return err + } if err := f.CheckIdentity(as, []string{"user", "bot"}); err != nil { return err } @@ -82,11 +98,7 @@ func whoamiRun(cmd *cobra.Command, opts *Options) error { ) diag := identitydiag.Diagnose(ctx, f, cfg, false) res := buildResult(cfg, as, source, diag) - if opts.JSON { - output.PrintJson(f.IOStreams.Out, res) - return nil - } - formatPretty(f.IOStreams.Out, res) + output.PrintJson(f.IOStreams.Out, res) return nil } @@ -94,17 +106,18 @@ func whoamiRun(cmd *cobra.Command, opts *Options) error { // Mirrors Factory.ResolveAs precedence: explicit flag wins; otherwise an // auto-detected result means auto-detect; otherwise a strict-mode forced // identity means strict-mode; otherwise it came from configured default-as. +// Values are snake_case to match the other enum fields (e.g. tokenStatus). func resolveSource(changedAs bool, flagAs core.Identity, autoDetected bool, strictForced core.Identity) string { if changedAs && (flagAs == core.AsUser || flagAs == core.AsBot) { return "flag" } if autoDetected { - return "auto-detect" + return "auto_detect" } if strictForced != "" { - return "strict-mode" + return "strict_mode" } - return "default-as" + return "default_as" } // buildResult maps the resolved identity and local diagnostics into the output. @@ -122,46 +135,29 @@ func buildResult(cfg *core.CliConfig, as core.Identity, source string, diag iden Identity: string(as), IdentitySource: source, } + // Use the diagnosed hint as-is: it is tailored to the credential source, so + // it never says "auth login" when that is blocked under an external provider. switch as { case core.AsBot: res.Available = diag.Bot.Available res.TokenStatus = diag.Bot.Status if !diag.Bot.Available { - res.Hint = "Bot identity not configured. Set app secret or bot token (see `lark-cli config --help`)." + res.Hint = diag.Bot.Hint } default: // user res.Available = diag.User.Available - res.OpenID = diag.User.OpenID - res.UserName = diag.User.UserName - res.TokenStatus = diag.User.TokenStatus - if res.TokenStatus == "" { - res.TokenStatus = "missing" + // Use Status (not the raw TokenStatus) so the vocab matches the bot + // branch: "ready" means usable for both. available stays the canonical + // usable signal; tokenStatus is the readable state behind it. + res.TokenStatus = diag.User.Status + // Set onBehalfOf only when a user is actually resolved; an unresolved + // user identity (not signed in) has no one to act on behalf of yet. + if diag.User.UserName != "" || diag.User.OpenID != "" { + res.OnBehalfOf = &delegatedUser{UserName: diag.User.UserName, OpenID: diag.User.OpenID} } if !diag.User.Available { - res.Hint = "No usable user token. Run `lark-cli auth login`." + res.Hint = diag.User.Hint } } return res } - -// formatPretty writes the human-readable one-glance summary. -func formatPretty(w io.Writer, r *whoamiResult) { - fmt.Fprintf(w, "Profile: %s (%s, %s)\n", r.Profile, r.AppID, r.Brand) - fmt.Fprintf(w, "Identity: %s (%s)\n", r.Identity, r.IdentitySource) - if r.Identity == string(core.AsUser) && r.UserName != "" { - if r.OpenID != "" { - fmt.Fprintf(w, "User: %s (%s)\n", r.UserName, r.OpenID) - } else { - fmt.Fprintf(w, "User: %s\n", r.UserName) - } - } - token := r.TokenStatus - if !r.Available && r.Hint != "" { - token = r.TokenStatus + " — " + r.Hint - } - // Write the label and value as separate %s args rather than one combined - // literal. A single label-colon-value literal trips the public-content - // credential scanner as a false-positive credential assignment; splitting - // the args avoids it while producing identical output. - fmt.Fprintf(w, "%s%s\n", "Token: ", token) -} diff --git a/cmd/whoami/whoami_test.go b/cmd/whoami/whoami_test.go index d3a7ec80..08886239 100644 --- a/cmd/whoami/whoami_test.go +++ b/cmd/whoami/whoami_test.go @@ -5,15 +5,19 @@ package whoami import ( "bytes" + "context" "encoding/json" "errors" "fmt" + "net/http" "strings" "testing" "github.com/larksuite/cli/errs" + 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/identitydiag" ) @@ -28,10 +32,10 @@ func TestResolveSource(t *testing.T) { }{ {"explicit flag user", true, core.AsUser, false, "", "flag"}, {"explicit flag bot", true, core.AsBot, false, "", "flag"}, - {"flag auto falls through to auto-detect", true, core.AsAuto, true, "", "auto-detect"}, - {"auto detected", false, "", true, "", "auto-detect"}, - {"strict mode", false, "", false, core.AsBot, "strict-mode"}, - {"default-as", false, "", false, "", "default-as"}, + {"flag auto falls through to auto-detect", true, core.AsAuto, true, "", "auto_detect"}, + {"auto detected", false, "", true, "", "auto_detect"}, + {"strict mode", false, "", false, core.AsBot, "strict_mode"}, + {"default_as", false, "", false, "", "default_as"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -46,18 +50,19 @@ func TestResolveSource(t *testing.T) { func TestBuildResult_UserValid(t *testing.T) { cfg := &core.CliConfig{ProfileName: "my-app", AppID: "cli_x", Brand: core.BrandLark, DefaultAs: core.AsAuto} diag := identitydiag.Result{ - User: identitydiag.Identity{Available: true, TokenStatus: "valid", OpenID: "ou_x", UserName: "Alice"}, + User: identitydiag.Identity{Available: true, Status: "ready", TokenStatus: "valid", OpenID: "ou_x", UserName: "Alice"}, } - r := buildResult(cfg, core.AsUser, "auto-detect", diag) + r := buildResult(cfg, core.AsUser, "auto_detect", diag) - if r.Identity != "user" || r.IdentitySource != "auto-detect" { + if r.Identity != "user" || r.IdentitySource != "auto_detect" { t.Fatalf("identity/source = %q/%q", r.Identity, r.IdentitySource) } - if !r.Available || r.TokenStatus != "valid" { + // tokenStatus mirrors the unified Status vocab ("ready"), not the raw "valid". + if !r.Available || r.TokenStatus != "ready" { t.Fatalf("available=%v status=%q", r.Available, r.TokenStatus) } - if r.OpenID != "ou_x" || r.UserName != "Alice" { - t.Fatalf("openId/userName = %q/%q", r.OpenID, r.UserName) + if r.OnBehalfOf == nil || r.OnBehalfOf.OpenID != "ou_x" || r.OnBehalfOf.UserName != "Alice" { + t.Fatalf("onBehalfOf = %#v, want Alice/ou_x", r.OnBehalfOf) } if r.Hint != "" { t.Fatalf("hint = %q, want empty", r.Hint) @@ -70,9 +75,9 @@ func TestBuildResult_UserValid(t *testing.T) { func TestBuildResult_UserMissingToken(t *testing.T) { cfg := &core.CliConfig{ProfileName: "p", AppID: "cli_x", Brand: core.BrandLark} diag := identitydiag.Result{ - User: identitydiag.Identity{Available: false, TokenStatus: ""}, // never logged in + User: identitydiag.Identity{Available: false, Status: "missing", Hint: "run: lark-cli auth login --help"}, // never logged in } - r := buildResult(cfg, core.AsUser, "auto-detect", diag) + r := buildResult(cfg, core.AsUser, "auto_detect", diag) if r.Available { t.Fatalf("available = true, want false") @@ -80,8 +85,10 @@ func TestBuildResult_UserMissingToken(t *testing.T) { if r.TokenStatus != "missing" { t.Fatalf("tokenStatus = %q, want missing", r.TokenStatus) } - if r.Hint == "" { - t.Fatalf("hint empty, want guidance") + // whoami renders the diagnosed hint verbatim (single source of truth) so it + // stays correct for the external-provider path without whoami knowing about it. + if r.Hint != diag.User.Hint { + t.Fatalf("hint = %q, want propagated %q", r.Hint, diag.User.Hint) } if r.DefaultAs != "auto" { t.Fatalf("defaultAs = %q, want auto (empty normalized)", r.DefaultAs) @@ -93,16 +100,16 @@ func TestBuildResult_BotReady(t *testing.T) { diag := identitydiag.Result{ Bot: identitydiag.Identity{Available: true, Status: "ready"}, } - r := buildResult(cfg, core.AsBot, "default-as", diag) + r := buildResult(cfg, core.AsBot, "default_as", diag) - if r.Identity != "bot" || r.IdentitySource != "default-as" { + if r.Identity != "bot" || r.IdentitySource != "default_as" { t.Fatalf("identity/source = %q/%q", r.Identity, r.IdentitySource) } if !r.Available || r.TokenStatus != "ready" { t.Fatalf("available=%v status=%q", r.Available, r.TokenStatus) } - if r.OpenID != "" || r.UserName != "" { - t.Fatalf("bot must not carry openId/userName: %#v", r) + if r.OnBehalfOf != nil { + t.Fatalf("bot must not carry onBehalfOf: %#v", r.OnBehalfOf) } if r.Hint != "" { t.Fatalf("hint = %q, want empty", r.Hint) @@ -112,9 +119,9 @@ func TestBuildResult_BotReady(t *testing.T) { func TestBuildResult_BotNotConfigured(t *testing.T) { cfg := &core.CliConfig{ProfileName: "p", AppID: "cli_x", Brand: core.BrandFeishu} diag := identitydiag.Result{ - Bot: identitydiag.Identity{Available: false, Status: "not_configured"}, + Bot: identitydiag.Identity{Available: false, Status: "not_configured", Hint: "run: lark-cli config --help"}, } - r := buildResult(cfg, core.AsBot, "auto-detect", diag) + r := buildResult(cfg, core.AsBot, "auto_detect", diag) if r.Available { t.Fatalf("available = true, want false") @@ -122,58 +129,8 @@ func TestBuildResult_BotNotConfigured(t *testing.T) { if r.TokenStatus != "not_configured" { t.Fatalf("tokenStatus = %q, want not_configured", r.TokenStatus) } - if r.Hint == "" { - t.Fatalf("hint empty, want guidance") - } -} - -func TestFormatPretty_User(t *testing.T) { - var buf bytes.Buffer - formatPretty(&buf, &whoamiResult{ - Profile: "my-app", AppID: "cli_x", Brand: core.BrandLark, - Identity: "user", IdentitySource: "auto-detect", - Available: true, TokenStatus: "valid", OpenID: "ou_x", UserName: "Alice", - }) - out := buf.String() - for _, want := range []string{ - "Profile: my-app (cli_x, lark)", - "Identity: user (auto-detect)", - "User: Alice (ou_x)", - "Token: valid", - } { - if !strings.Contains(out, want) { - t.Errorf("output missing %q\n--- got ---\n%s", want, out) - } - } -} - -func TestFormatPretty_BotNoUserLine(t *testing.T) { - var buf bytes.Buffer - formatPretty(&buf, &whoamiResult{ - Profile: "p", AppID: "cli_x", Brand: core.BrandFeishu, - Identity: "bot", IdentitySource: "default-as", - Available: true, TokenStatus: "ready", - }) - out := buf.String() - if strings.Contains(out, "User:") { - t.Errorf("bot output must not contain User: line\n%s", out) - } - if !strings.Contains(out, "Identity: bot (default-as)") || !strings.Contains(out, "Token: ready") { - t.Errorf("unexpected bot output:\n%s", out) - } -} - -func TestFormatPretty_UnavailableShowsHint(t *testing.T) { - var buf bytes.Buffer - formatPretty(&buf, &whoamiResult{ - Profile: "p", AppID: "cli_x", Brand: core.BrandLark, - Identity: "user", IdentitySource: "auto-detect", - Available: false, TokenStatus: "missing", - Hint: "No usable user token. Run `lark-cli auth login`.", - }) - out := buf.String() - if !strings.Contains(out, "Token: missing — No usable user token.") { - t.Errorf("expected token line with hint, got:\n%s", out) + if r.Hint != diag.Bot.Hint { + t.Fatalf("hint = %q, want propagated %q", r.Hint, diag.Bot.Hint) } } @@ -183,7 +140,7 @@ func TestWhoami_BotJSON(t *testing.T) { }) cmd := NewCmdWhoami(f) - cmd.SetArgs([]string{"--json"}) + cmd.SetArgs([]string{}) // bare whoami: output is always JSON, no flag needed if err := cmd.Execute(); err != nil { t.Fatalf("Execute() error = %v", err) } @@ -204,8 +161,8 @@ func TestWhoami_BotJSON(t *testing.T) { if got.IdentitySource == "" { t.Fatalf("identitySource empty") } - if got.OpenID != "" { - t.Fatalf("bot must not carry openId: %q", got.OpenID) + if got.OnBehalfOf != nil { + t.Fatalf("bot (self) must not carry onBehalfOf: %#v", got.OnBehalfOf) } } @@ -256,3 +213,108 @@ func TestWhoami_ConfigErrorPropagates(t *testing.T) { t.Fatalf("Execute() error = %v, want it to wrap %v", err, wantErr) } } + +func TestWhoami_StrictModeRejectsCrossIdentity(t *testing.T) { + // Bot-only account → strict mode bot. A real `--as user` call would be + // rejected by CheckStrictMode; whoami must reject it identically rather than + // previewing a user identity the next call would refuse. + f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ + ProfileName: "p", AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + SupportedIdentities: 2, // bot only + }) + cmd := NewCmdWhoami(f) + cmd.SetArgs([]string{"--as", "user", "--json"}) + err := cmd.Execute() + if err == nil { + t.Fatalf("Execute() with --as user under strict bot = nil, want strict-mode rejection") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError: %v", err, err) + } +} + +type fakeExtProvider struct { + name string + account *extcred.Account +} + +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 nil, nil // no UAT served locally; whoami runs with verify=false +} + +func externalWhoamiFactory(cfg *core.CliConfig) (*cmdutil.Factory, *bytes.Buffer) { + cred := credential.NewCredentialProvider( + []extcred.Provider{&fakeExtProvider{name: "corp-sso", account: &extcred.Account{AppID: cfg.AppID}}}, + nil, nil, + func() (*http.Client, error) { return nil, nil }, + ) + out := &bytes.Buffer{} + f := &cmdutil.Factory{ + Config: func() (*core.CliConfig, error) { return cfg, nil }, + Credential: cred, + IOStreams: &cmdutil.IOStreams{Out: out, ErrOut: &bytes.Buffer{}}, + } + return f, out +} + +// Regression for the external-provider blind spot: with credentials managed by +// an extension provider, a signed-in user must read as available, and an +// unavailable identity must not be told to "auth login" (which is blocked). +func TestWhoami_ExternalProvider_UserReady(t *testing.T) { + cfg := &core.CliConfig{ + ProfileName: "p", AppID: "cli_x", Brand: core.BrandFeishu, + SupportedIdentities: uint8(extcred.SupportsAll), UserOpenId: "ou_x", UserName: "Alice", + } + f, out := externalWhoamiFactory(cfg) + + cmd := NewCmdWhoami(f) + cmd.SetArgs([]string{"--as", "user", "--json"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + var got whoamiResult + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatalf("Unmarshal: %v\n%s", err, out.String()) + } + if got.Identity != "user" || !got.Available || got.TokenStatus != "ready" { + t.Fatalf("got %#v, want user/available/ready", got) + } + if got.OnBehalfOf == nil || got.OnBehalfOf.UserName != "Alice" || got.OnBehalfOf.OpenID != "ou_x" { + t.Fatalf("onBehalfOf = %#v, want Alice/ou_x (delegated)", got.OnBehalfOf) + } + if got.Hint != "" { + t.Fatalf("hint = %q, want empty when available", got.Hint) + } +} + +func TestWhoami_ExternalProvider_UserHintNotKeychain(t *testing.T) { + cfg := &core.CliConfig{ + ProfileName: "p", AppID: "cli_x", Brand: core.BrandFeishu, + SupportedIdentities: uint8(extcred.SupportsUser), // user supported but not signed in + } + f, out := externalWhoamiFactory(cfg) + + cmd := NewCmdWhoami(f) + cmd.SetArgs([]string{"--as", "user", "--json"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + var got whoamiResult + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatalf("Unmarshal: %v\n%s", err, out.String()) + } + if got.Identity != "user" || got.Available { + t.Fatalf("got identity=%q available=%v, want user/false", got.Identity, got.Available) + } + if strings.Contains(got.Hint, "auth login") { + t.Fatalf("hint must not point at auth login under external provider: %q", got.Hint) + } + if !strings.Contains(got.Hint, "external") { + t.Fatalf("hint should explain external management: %q", got.Hint) + } +} diff --git a/internal/identitydiag/diagnostics.go b/internal/identitydiag/diagnostics.go index f8eb648d..0e7fb1cc 100644 --- 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{ diff --git a/internal/identitydiag/diagnostics_test.go b/internal/identitydiag/diagnostics_test.go index 6d288e3b..fa4a3234 100644 --- 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) +} diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index 95f305ae..272b2bae 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -33,7 +33,7 @@ lark-cli config init --new | 按业务域授权 | `lark-cli auth login --domain docs --domain drive --no-wait --json`；`--domain` 可重复，也可用逗号分隔 | | 指定单个 scope 授权 | `lark-cli auth login --scope "" --no-wait --json` | | 检查当前登录态、是谁登录、token 是否有效 | `lark-cli auth status --json --verify`；回答时引用 `identity`、`verified`、`identities.user.status`、`identities.user.userName`、`identities.user.openId`（用户 open id）、`identities.user.tokenStatus`、`identities.user.scope` | -| 快速看当前生效身份（人类可读） | `lark-cli whoami`；聚焦实际生效的那一个身份（走 `--as`/`default-as`/strict-mode 解析，可能与 `auth status` 的「第一个可用身份」不同），含当前 profile。脚本/agent 取结构化结果加 `--json`（字段 `identity`、`identitySource`、`available`、`tokenStatus`）。深度诊断 / 服务器校验仍用 `auth status --json --verify` | +| 快速查看当前身份状态 | `lark-cli whoami`；实际生效的那一个身份 | | 退出当前机器的用户登录态 | `lark-cli auth logout --json`；`loggedOut:true` 表示注销成功 | | bot 缺少权限 | 不要执行 `auth login`；引导用户在开发者后台开通 bot scope，优先复用错误里的 `console_url` | | 取消用户对应用的全部服务端授权 | `auth logout` 只清本机登录态；服务端授权需用户在飞书授权管理页取消 |