From 37ad6dbc0cf86759f2a329cec5e959fc2aee0808 Mon Sep 17 00:00:00 2001 From: zhaojunchang Date: Wed, 8 Apr 2026 14:58:22 +0800 Subject: [PATCH 1/8] feat(auth): improve scope handling and output in login flow - Add scope validation to check for missing requested scopes - Implement detailed scope breakdown in login success output - Add new message strings for scope-related output - Refactor login success output to handle both JSON and text formats - Add tests for scope validation and output scenarios --- cmd/auth/login.go | 31 +-- cmd/auth/login_messages.go | 60 ++++-- cmd/auth/login_result.go | 222 ++++++++++++++++++++ cmd/auth/login_test.go | 409 +++++++++++++++++++++++++++++++++++++ 4 files changed, 692 insertions(+), 30 deletions(-) create mode 100644 cmd/auth/login_result.go diff --git a/cmd/auth/login.go b/cmd/auth/login.go index 467755bf1..7d73142c6 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -284,6 +284,9 @@ func authLoginRun(opts *LoginOptions) error { } return output.ErrAuth("authorization failed: %s", result.Message) } + if result.Token == nil { + return output.ErrAuth("authorization succeeded but no token returned") + } // Step 6: Get user info log(msg.AuthSuccess) @@ -296,6 +299,8 @@ func authLoginRun(opts *LoginOptions) error { return output.ErrAuth("failed to get user info: %v", err) } + scopeSummary := loadLoginScopeSummary(config.AppID, openId, finalScope, result.Token.Scope) + // Step 7: Store token now := time.Now().UnixMilli() storedToken := &larkauth.StoredUAToken{ @@ -318,21 +323,11 @@ func authLoginRun(opts *LoginOptions) error { return output.Errorf(output.ExitInternal, "internal", "failed to update login profile: %v", err) } - if opts.JSON { - b, _ := json.Marshal(map[string]interface{}{ - "event": "authorization_complete", - "user_open_id": openId, - "user_name": userName, - "scope": result.Token.Scope, - }) - fmt.Fprintln(f.IOStreams.Out, string(b)) - } else { - fmt.Fprintln(f.IOStreams.ErrOut) - output.PrintSuccess(f.IOStreams.ErrOut, fmt.Sprintf(msg.LoginSuccess, userName, openId)) - if result.Token.Scope != "" { - fmt.Fprintf(f.IOStreams.ErrOut, msg.GrantedScopes, result.Token.Scope) - } + if issue := ensureRequestedScopesGranted(finalScope, result.Token.Scope, msg, scopeSummary); issue != nil { + return handleLoginScopeIssue(opts, msg, f, issue, openId, userName) } + + writeLoginSuccess(opts, msg, f, openId, userName, scopeSummary) return nil } @@ -367,6 +362,8 @@ func authLoginPollDeviceCode(opts *LoginOptions, config *core.CliConfig, msg *lo return output.ErrAuth("failed to get user info: %v", err) } + scopeSummary := loadLoginScopeSummary(config.AppID, openId, "", result.Token.Scope) + // Store token now := time.Now().UnixMilli() storedToken := &larkauth.StoredUAToken{ @@ -389,7 +386,11 @@ func authLoginPollDeviceCode(opts *LoginOptions, config *core.CliConfig, msg *lo return output.Errorf(output.ExitInternal, "internal", "failed to update login profile: %v", err) } - output.PrintSuccess(f.IOStreams.ErrOut, fmt.Sprintf(msg.LoginSuccess, userName, openId)) + if issue := ensureRequestedScopesGranted("", result.Token.Scope, msg, scopeSummary); issue != nil { + return handleLoginScopeIssue(opts, msg, f, issue, openId, userName) + } + + writeLoginSuccess(opts, msg, f, openId, userName, scopeSummary) return nil } diff --git a/cmd/auth/login_messages.go b/cmd/auth/login_messages.go index f16349679..ad5956dd0 100644 --- a/cmd/auth/login_messages.go +++ b/cmd/auth/login_messages.go @@ -20,11 +20,21 @@ type loginMsg struct { ConfirmAuth string // Non-interactive prompts (login.go) - OpenURL string - WaitingAuth string - AuthSuccess string - LoginSuccess string - GrantedScopes string + OpenURL string + WaitingAuth string + AuthSuccess string + LoginSuccess string + GrantedScopes string + ScopeMismatch string + ScopeHint string + ScopeHintShort string + PendingScopes string + RequestedScopes string + NewlyGrantedScopes string + AlreadyGrantedScopes string + MissingScopes string + FinalGrantedScopes string + NoScopes string // Non-interactive hint (no flags) HintHeader string @@ -50,11 +60,21 @@ var loginMsgZh = &loginMsg{ ErrNoDomain: "请至少选择一个业务域", ConfirmAuth: "确认授权?", - OpenURL: "在浏览器中打开以下链接进行认证:\n\n", - WaitingAuth: "等待用户授权...", - AuthSuccess: "授权成功，正在获取用户信息...", - LoginSuccess: "登录成功! 用户: %s (%s)", - GrantedScopes: " 已授权 scopes: %s\n", + OpenURL: "在浏览器中打开以下链接进行认证:\n\n", + WaitingAuth: "等待用户授权...", + AuthSuccess: "授权成功，正在获取用户信息...", + LoginSuccess: "登录成功! 用户: %s (%s)", + GrantedScopes: " 已授权 scopes: %s\n", + ScopeMismatch: "授权完成，但以下请求 scopes 未被授予: %s", + ScopeHint: "实际已授予 scopes: %s。请检查应用在飞书开发者后台是否已启用这些 scopes，并确认授权页已同意对应权限。可执行 `lark-cli auth scopes` 查看应用已启用 scopes。", + ScopeHintShort: "请检查应用在飞书开发者后台是否已启用这些 scopes，并确认授权页已同意对应权限。可执行 `lark-cli auth scopes` 查看应用已启用 scopes。", + PendingScopes: "（授权后确认）", + RequestedScopes: " 本次请求 scopes: %s\n", + NewlyGrantedScopes: " 本次新增 scopes: %s\n", + AlreadyGrantedScopes: " 已有 scopes: %s\n", + MissingScopes: " 未授权 scopes: %s\n", + FinalGrantedScopes: " 最终已授权 scopes: %s\n", + NoScopes: "（空）", HintHeader: "请指定要授权的权限:\n", HintCommon1: " --recommend 授权推荐权限", @@ -79,11 +99,21 @@ var loginMsgEn = &loginMsg{ ErrNoDomain: "please select at least one domain", ConfirmAuth: "Confirm authorization?", - OpenURL: "Open this URL in your browser to authenticate:\n\n", - WaitingAuth: "Waiting for user authorization...", - AuthSuccess: "Authorization successful, fetching user info...", - LoginSuccess: "Login successful! User: %s (%s)", - GrantedScopes: " Granted scopes: %s\n", + OpenURL: "Open this URL in your browser to authenticate:\n\n", + WaitingAuth: "Waiting for user authorization...", + AuthSuccess: "Authorization successful, fetching user info...", + LoginSuccess: "Login successful! User: %s (%s)", + GrantedScopes: " Granted scopes: %s\n", + ScopeMismatch: "authorization completed, but these requested scopes were not granted: %s", + ScopeHint: "Granted scopes: %s. Check whether the app has enabled these scopes in the developer console and whether the user approved them on the authorization page. Run `lark-cli auth scopes` to inspect the app's enabled scopes.", + ScopeHintShort: "Check whether the app has enabled these scopes in the developer console and whether the user approved them on the authorization page. Run `lark-cli auth scopes` to inspect the app's enabled scopes.", + PendingScopes: "(determined after authorization)", + RequestedScopes: " Requested scopes: %s\n", + NewlyGrantedScopes: " Newly granted scopes: %s\n", + AlreadyGrantedScopes: " Already granted scopes: %s\n", + MissingScopes: " Missing scopes: %s\n", + FinalGrantedScopes: " Final granted scopes: %s\n", + NoScopes: "(none)", HintHeader: "Please specify the scopes to authorize:\n", HintCommon1: " --recommend authorize recommended scopes", diff --git a/cmd/auth/login_result.go b/cmd/auth/login_result.go new file mode 100644 index 000000000..978a343b7 --- /dev/null +++ b/cmd/auth/login_result.go @@ -0,0 +1,222 @@ +package auth + +import ( + "encoding/json" + "fmt" + "strings" + + larkauth "github.com/larksuite/cli/internal/auth" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/output" +) + +type loginScopeSummary struct { + Requested []string + NewlyGranted []string + AlreadyGranted []string + Granted []string + Missing []string +} + +type loginScopeIssue struct { + Message string + Hint string + ShortHint string + Summary *loginScopeSummary +} + +func ensureRequestedScopesGranted(requestedScope, grantedScope string, msg *loginMsg, summary *loginScopeSummary) *loginScopeIssue { + requested := uniqueScopeList(requestedScope) + if len(requested) == 0 { + return nil + } + + missing := larkauth.MissingScopes(grantedScope, requested) + if len(missing) == 0 { + return nil + } + + granted := strings.Fields(grantedScope) + grantedDisplay := msg.NoScopes + if len(granted) > 0 { + grantedDisplay = strings.Join(granted, " ") + } + + if summary == nil { + summary = &loginScopeSummary{ + Requested: requested, + Granted: granted, + Missing: missing, + } + } + return &loginScopeIssue{ + Message: fmt.Sprintf(msg.ScopeMismatch, strings.Join(missing, " ")), + Hint: fmt.Sprintf(msg.ScopeHint, grantedDisplay), + ShortHint: msg.ScopeHintShort, + Summary: summary, + } +} + +func loadLoginScopeSummary(appID, openId, requestedScope, grantedScope string) *loginScopeSummary { + previousScope := "" + if previous := larkauth.GetStoredToken(appID, openId); previous != nil { + previousScope = previous.Scope + } + return buildLoginScopeSummary(requestedScope, previousScope, grantedScope) +} + +func buildLoginScopeSummary(requestedScope, previousScope, grantedScope string) *loginScopeSummary { + requested := uniqueScopeList(requestedScope) + previous := uniqueScopeList(previousScope) + granted := uniqueScopeList(grantedScope) + previousSet := make(map[string]bool, len(previous)) + for _, scope := range previous { + previousSet[scope] = true + } + grantedSet := make(map[string]bool, len(granted)) + for _, scope := range granted { + grantedSet[scope] = true + } + + summary := &loginScopeSummary{ + Requested: requested, + Granted: granted, + } + for _, scope := range requested { + if !grantedSet[scope] { + summary.Missing = append(summary.Missing, scope) + continue + } + if previousSet[scope] { + summary.AlreadyGranted = append(summary.AlreadyGranted, scope) + continue + } + summary.NewlyGranted = append(summary.NewlyGranted, scope) + } + return summary +} + +func uniqueScopeList(scope string) []string { + seen := make(map[string]bool) + var result []string + for _, item := range strings.Fields(scope) { + if seen[item] { + continue + } + seen[item] = true + result = append(result, item) + } + return result +} + +func formatScopeList(scopes []string, empty string) string { + if len(scopes) == 0 { + return empty + } + return strings.Join(scopes, " ") +} + +func writeLoginScopeBreakdown(errOut *cmdutil.IOStreams, msg *loginMsg, summary *loginScopeSummary, pendingLabel string) { + if summary == nil { + summary = &loginScopeSummary{} + } + fmt.Fprintf(errOut.ErrOut, msg.RequestedScopes, formatScopeList(summary.Requested, msg.NoScopes)) + if pendingLabel != "" { + fmt.Fprintf(errOut.ErrOut, msg.NewlyGrantedScopes, pendingLabel) + fmt.Fprintf(errOut.ErrOut, msg.MissingScopes, pendingLabel) + return + } + fmt.Fprintf(errOut.ErrOut, msg.NewlyGrantedScopes, formatScopeList(summary.NewlyGranted, msg.NoScopes)) + if len(summary.Missing) > 0 { + fmt.Fprintf(errOut.ErrOut, msg.MissingScopes, formatScopeList(summary.Missing, msg.NoScopes)) + } + fmt.Fprintf(errOut.ErrOut, msg.FinalGrantedScopes, formatScopeList(summary.Granted, msg.NoScopes)) +} + +func writeLoginSuccess(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory, openId, userName string, summary *loginScopeSummary) { + if summary == nil { + summary = &loginScopeSummary{} + } + if opts.JSON { + b, _ := json.Marshal(authorizationCompletePayload(openId, userName, summary, nil)) + fmt.Fprintln(f.IOStreams.Out, string(b)) + return + } + + fmt.Fprintln(f.IOStreams.ErrOut) + output.PrintSuccess(f.IOStreams.ErrOut, fmt.Sprintf(msg.LoginSuccess, userName, openId)) + writeLoginScopeBreakdown(f.IOStreams, msg, summary, "") +} + +func handleLoginScopeIssue(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory, issue *loginScopeIssue, openId, userName string) error { + if issue == nil { + return nil + } + loginSucceeded := openId != "" + if opts.JSON { + if loginSucceeded { + b, _ := json.Marshal(authorizationCompletePayload(openId, userName, issue.Summary, issue)) + fmt.Fprintln(f.IOStreams.Out, string(b)) + return nil + } + detail := map[string]interface{}{ + "requested": issue.Summary.Requested, + "granted": issue.Summary.Granted, + "missing": issue.Summary.Missing, + } + return &output.ExitError{ + Code: output.ExitAuth, + Detail: &output.ErrDetail{ + Type: "missing_scope", + Message: issue.Message, + Hint: issue.Hint, + Detail: detail, + }, + } + } + + fmt.Fprintln(f.IOStreams.ErrOut) + if loginSucceeded { + output.PrintSuccess(f.IOStreams.ErrOut, fmt.Sprintf(msg.LoginSuccess, userName, openId)) + } else { + fmt.Fprintln(f.IOStreams.ErrOut, issue.Message) + } + if loginSucceeded { + fmt.Fprintln(f.IOStreams.ErrOut, issue.Message) + } + writeLoginScopeBreakdown(f.IOStreams, msg, issue.Summary, "") + if issue.ShortHint != "" { + fmt.Fprintln(f.IOStreams.ErrOut, issue.ShortHint) + } else if issue.Hint != "" { + fmt.Fprintln(f.IOStreams.ErrOut, issue.Hint) + } + if loginSucceeded { + return nil + } + return output.ErrBare(output.ExitAuth) +} + +func authorizationCompletePayload(openId, userName string, summary *loginScopeSummary, issue *loginScopeIssue) map[string]interface{} { + if summary == nil { + summary = &loginScopeSummary{} + } + payload := map[string]interface{}{ + "event": "authorization_complete", + "user_open_id": openId, + "user_name": userName, + "scope": strings.Join(summary.Granted, " "), + "requested": summary.Requested, + "newly_granted": summary.NewlyGranted, + "already_granted": summary.AlreadyGranted, + "missing": summary.Missing, + "granted": summary.Granted, + } + if issue != nil { + payload["warning"] = map[string]interface{}{ + "type": "missing_scope", + "message": issue.Message, + "hint": issue.Hint, + } + } + return payload +} diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go index 06f9717e8..1f06758df 100644 --- a/cmd/auth/login_test.go +++ b/cmd/auth/login_test.go @@ -5,16 +5,27 @@ package auth import ( "context" + "encoding/json" + "errors" "sort" "strings" "testing" + larkauth "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/internal/registry" "github.com/larksuite/cli/shortcuts/common" + "github.com/zalando/go-keyring" ) +type failWriter struct{} + +func (failWriter) Write([]byte) (int, error) { + return 0, errors.New("write failed") +} + func TestSuggestDomain_PrefixMatch(t *testing.T) { known := map[string]bool{ "calendar": true, @@ -282,6 +293,404 @@ func TestAuthLoginRun_NonTerminal_NoFlags_RejectsWithHint(t *testing.T) { } } +func TestEnsureRequestedScopesGranted(t *testing.T) { + issue := ensureRequestedScopesGranted("im:message:send im:message:reply", "im:message:reply", getLoginMsg("en"), nil) + if issue == nil { + t.Fatal("expected missing scope issue") + } + if !strings.Contains(issue.Message, "im:message:send") { + t.Fatalf("message %q missing requested scope", issue.Message) + } + if !strings.Contains(issue.Hint, "Granted scopes: im:message:reply") { + t.Fatalf("hint %q missing granted scope context", issue.Hint) + } + if got := strings.Join(issue.Summary.Missing, " "); got != "im:message:send" { + t.Fatalf("Missing = %q", got) + } +} + +func TestBuildLoginScopeSummary(t *testing.T) { + summary := buildLoginScopeSummary("im:message:send im:message:reply im:message:send", "im:message:reply", "im:message:send im:message:reply im:chat:read") + if got := strings.Join(summary.Requested, " "); got != "im:message:send im:message:reply" { + t.Fatalf("Requested = %q", got) + } + if got := strings.Join(summary.NewlyGranted, " "); got != "im:message:send" { + t.Fatalf("NewlyGranted = %q", got) + } + if got := strings.Join(summary.AlreadyGranted, " "); got != "im:message:reply" { + t.Fatalf("AlreadyGranted = %q", got) + } + if len(summary.Missing) != 0 { + t.Fatalf("Missing = %v, want empty", summary.Missing) + } + if got := strings.Join(summary.Granted, " "); got != "im:message:send im:message:reply im:chat:read" { + t.Fatalf("Granted = %q", got) + } +} + +func TestWriteLoginSuccess_JSONIncludesScopeDiff(t *testing.T) { + f, stdout, _, _ := cmdutil.TestFactory(t, nil) + + writeLoginSuccess(&LoginOptions{JSON: true}, getLoginMsg("en"), f, "ou_user", "tester", &loginScopeSummary{ + Requested: []string{"im:message:send", "im:message:reply"}, + NewlyGranted: []string{"im:message:send"}, + AlreadyGranted: []string{"im:message:reply"}, + Granted: []string{"im:message:send", "im:message:reply"}, + }) + + var data map[string]interface{} + if err := json.Unmarshal(stdout.Bytes(), &data); err != nil { + t.Fatalf("Unmarshal(stdout) error = %v, stdout=%s", err, stdout.String()) + } + if data["event"] != "authorization_complete" { + t.Fatalf("event = %v", data["event"]) + } + if data["scope"] != "im:message:send im:message:reply" { + t.Fatalf("scope = %v", data["scope"]) + } + if len(data["newly_granted"].([]interface{})) != 1 { + t.Fatalf("newly_granted = %#v", data["newly_granted"]) + } + if len(data["already_granted"].([]interface{})) != 1 { + t.Fatalf("already_granted = %#v", data["already_granted"]) + } +} + +func TestHandleLoginScopeIssue_NonJSONAlignsWithLoginSuccess(t *testing.T) { + f, _, stderr, _ := cmdutil.TestFactory(t, nil) + err := handleLoginScopeIssue(&LoginOptions{}, getLoginMsg("zh"), f, &loginScopeIssue{ + Message: "授权完成，但以下请求 scopes 未被授予: im:message:send", + Hint: "实际已授予 scopes: base:app:copy。请检查后台配置", + ShortHint: "请检查后台配置", + Summary: &loginScopeSummary{ + Requested: []string{"im:message:send"}, + Missing: []string{"im:message:send"}, + Granted: []string{"base:app:copy"}, + }, + }, "ou_user", "tester") + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + got := stderr.String() + for _, want := range []string{ + "OK: 登录成功! 用户: tester (ou_user)", + "授权完成，但以下请求 scopes 未被授予: im:message:send", + "本次请求 scopes: im:message:send", + "本次新增 scopes: （空）", + "未授权 scopes: im:message:send", + "最终已授权 scopes: base:app:copy", + "请检查后台配置", + } { + if !strings.Contains(got, want) { + t.Fatalf("stderr missing %q, got:\n%s", want, got) + } + } + if strings.Contains(got, "ERROR:") { + t.Fatalf("stderr should not contain error prefix, got:\n%s", got) + } +} + +func TestHandleLoginScopeIssue_JSONAlignsWithLoginSuccess(t *testing.T) { + f, stdout, _, _ := cmdutil.TestFactory(t, nil) + err := handleLoginScopeIssue(&LoginOptions{JSON: true}, getLoginMsg("en"), f, &loginScopeIssue{ + Message: "authorization completed, but these requested scopes were not granted: im:message:send", + Hint: "Granted scopes: base:app:copy. Check app scopes.", + Summary: &loginScopeSummary{ + Requested: []string{"im:message:send"}, + Missing: []string{"im:message:send"}, + Granted: []string{"base:app:copy"}, + }, + }, "ou_user", "tester") + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + var data map[string]interface{} + if err := json.Unmarshal(stdout.Bytes(), &data); err != nil { + t.Fatalf("Unmarshal(stdout) error = %v, stdout=%s", err, stdout.String()) + } + if data["event"] != "authorization_complete" { + t.Fatalf("event = %v", data["event"]) + } + if data["user_open_id"] != "ou_user" { + t.Fatalf("user_open_id = %v", data["user_open_id"]) + } + warning, ok := data["warning"].(map[string]interface{}) + if !ok { + t.Fatalf("warning = %#v", data["warning"]) + } + if warning["type"] != "missing_scope" { + t.Fatalf("warning.type = %v", warning["type"]) + } +} + +func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { + tests := []struct { + name string + summary *loginScopeSummary + expectedPresent []string + expectedAbsent []string + }{ + { + name: "mixed newly granted and already granted", + summary: &loginScopeSummary{ + Requested: []string{"im:message:send", "im:message:reply"}, + NewlyGranted: []string{"im:message:send"}, + AlreadyGranted: []string{"im:message:reply"}, + Granted: []string{"im:message:send", "im:message:reply"}, + }, + expectedPresent: []string{ + "登录成功! 用户: tester (ou_user)", + "本次请求 scopes: im:message:send im:message:reply", + "本次新增 scopes: im:message:send", + "最终已授权 scopes: im:message:send im:message:reply", + }, + expectedAbsent: []string{ + "已有 scopes:", + "未授权 scopes:", + }, + }, + { + name: "all already granted", + summary: &loginScopeSummary{ + Requested: []string{"im:message:send"}, + AlreadyGranted: []string{"im:message:send"}, + Granted: []string{"im:message:send", "contact:user.base:readonly"}, + }, + expectedPresent: []string{ + "本次请求 scopes: im:message:send", + "本次新增 scopes: （空）", + "最终已授权 scopes: im:message:send contact:user.base:readonly", + }, + expectedAbsent: []string{ + "已有 scopes:", + "未授权 scopes:", + }, + }, + { + name: "missing scopes are shown", + summary: &loginScopeSummary{ + Requested: []string{"im:message:send", "im:message:reply"}, + Missing: []string{"im:message:send"}, + Granted: []string{"im:message:reply"}, + }, + expectedPresent: []string{ + "本次请求 scopes: im:message:send im:message:reply", + "本次新增 scopes: （空）", + "未授权 scopes: im:message:send", + "最终已授权 scopes: im:message:reply", + }, + expectedAbsent: []string{ + "已有 scopes:", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, _, stderr, _ := cmdutil.TestFactory(t, nil) + writeLoginSuccess(&LoginOptions{}, getLoginMsg("zh"), f, "ou_user", "tester", tt.summary) + + got := stderr.String() + for _, want := range tt.expectedPresent { + if !strings.Contains(got, want) { + t.Fatalf("stderr missing %q, got:\n%s", want, got) + } + } + for _, unwanted := range tt.expectedAbsent { + if strings.Contains(got, unwanted) { + t.Fatalf("stderr should not contain %q, got:\n%s", unwanted, got) + } + } + }) + } +} + +func TestBuildLoginScopeSummary_WithMissingScopes(t *testing.T) { + summary := buildLoginScopeSummary("im:message:send im:message:reply", "im:message:reply", "im:message:reply") + if got := strings.Join(summary.NewlyGranted, " "); got != "" { + t.Fatalf("NewlyGranted = %q, want empty", got) + } + if got := strings.Join(summary.AlreadyGranted, " "); got != "im:message:reply" { + t.Fatalf("AlreadyGranted = %q", got) + } + if got := strings.Join(summary.Missing, " "); got != "im:message:send" { + t.Fatalf("Missing = %q", got) + } +} + +func TestAuthLoginRun_MissingRequestedScopeAlignsWithLoginSuccess(t *testing.T) { + keyring.MockInit() + setupLoginConfigDir(t) + t.Setenv("HOME", t.TempDir()) + + multi := &core.MultiAppConfig{ + CurrentApp: "default", + Apps: []core.AppConfig{ + {Name: "default", AppId: "cli_test"}, + }, + } + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig() error = %v", err) + } + + f, _, stderr, reg := cmdutil.TestFactory(t, &core.CliConfig{ + ProfileName: "default", + AppID: "cli_test", + AppSecret: "secret", + Brand: core.BrandFeishu, + }) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: larkauth.PathDeviceAuthorization, + Body: map[string]interface{}{ + "device_code": "device-code", + "user_code": "user-code", + "verification_uri": "https://example.com/verify", + "verification_uri_complete": "https://example.com/verify?code=123", + "expires_in": 240, + "interval": 0, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: larkauth.PathOAuthTokenV2, + Body: map[string]interface{}{ + "access_token": "user-access-token", + "refresh_token": "refresh-token", + "expires_in": 7200, + "refresh_token_expires_in": 604800, + "scope": "offline_access", + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: larkauth.PathUserInfoV1, + Body: map[string]interface{}{ + "code": 0, + "msg": "ok", + "data": map[string]interface{}{ + "open_id": "ou_user", + "name": "tester", + }, + }, + }) + + err := authLoginRun(&LoginOptions{ + Factory: f, + Ctx: context.Background(), + Scope: "im:message:send", + }) + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + got := stderr.String() + for _, want := range []string{ + "OK: 登录成功! 用户: tester (ou_user)", + "授权完成，但以下请求 scopes 未被授予: im:message:send", + "本次请求 scopes: im:message:send", + "未授权 scopes: im:message:send", + "最终已授权 scopes: offline_access", + } { + if !strings.Contains(got, want) { + t.Fatalf("stderr missing %q, got:\n%s", want, got) + } + } + if strings.Contains(got, "ERROR:") { + t.Fatalf("stderr should not contain error prefix, got:\n%s", got) + } + stored := larkauth.GetStoredToken("cli_test", "ou_user") + if stored == nil { + t.Fatal("expected token to be stored when authorization succeeds with missing scopes") + } + if stored.Scope != "offline_access" { + t.Fatalf("stored scope = %q", stored.Scope) + } + cfg, err := core.LoadMultiAppConfig() + if err != nil { + t.Fatalf("LoadMultiAppConfig() error = %v", err) + } + if len(cfg.Apps) != 1 || len(cfg.Apps[0].Users) != 1 { + t.Fatalf("unexpected users in config: %#v", cfg.Apps) + } + if cfg.Apps[0].Users[0].UserOpenId != "ou_user" { + t.Fatalf("stored user open id = %q", cfg.Apps[0].Users[0].UserOpenId) + } + if cfg.Apps[0].Users[0].UserName != "tester" { + t.Fatalf("stored user name = %q", cfg.Apps[0].Users[0].UserName) + } +} + +func TestAuthLoginRun_JSONWriteFailure_NoWaitIgnoresWriterError(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ + ProfileName: "default", + AppID: "cli_test", + AppSecret: "secret", + Brand: core.BrandFeishu, + }) + f.IOStreams.Out = failWriter{} + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: larkauth.PathDeviceAuthorization, + Body: map[string]interface{}{ + "device_code": "device-code", + "user_code": "user-code", + "verification_uri": "https://example.com/verify", + "verification_uri_complete": "https://example.com/verify?code=123", + "expires_in": 240, + "interval": 5, + }, + }) + + err := authLoginRun(&LoginOptions{ + Factory: f, + Ctx: context.Background(), + Scope: "im:message:send", + NoWait: true, + JSON: true, + }) + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } +} + +func TestAuthLoginRun_JSONWriteFailure_DeviceAuthorizationStillPolls(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ + ProfileName: "default", + AppID: "cli_test", + AppSecret: "secret", + Brand: core.BrandFeishu, + }) + f.IOStreams.Out = failWriter{} + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: larkauth.PathDeviceAuthorization, + Body: map[string]interface{}{ + "device_code": "device-code", + "user_code": "user-code", + "verification_uri": "https://example.com/verify", + "verification_uri_complete": "https://example.com/verify?code=123", + "expires_in": 240, + "interval": 5, + }, + }) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + err := authLoginRun(&LoginOptions{ + Factory: f, + Ctx: ctx, + Scope: "im:message:send", + JSON: true, + }) + if err == nil { + t.Fatal("expected error") + } +} + func TestGetDomainMetadata_ExcludesEvent(t *testing.T) { domains := getDomainMetadata("zh") for _, dm := range domains { From 983a0e06a7f79b544b10ecf658612dbee5e30ceb Mon Sep 17 00:00:00 2001 From: zhaojunchang Date: Wed, 8 Apr 2026 15:25:28 +0800 Subject: [PATCH 2/8] feat(auth): add requested scope caching for device code login Implement caching of requested scopes during device code login flow to ensure proper scope validation after authorization. The cache is stored in JSON files under config directory and automatically cleaned up after successful or failed authorization. Add tests for scope caching functionality and verify proper integration with existing login flow. --- cmd/auth/login.go | 20 +++++- cmd/auth/login_scope_cache.go | 81 ++++++++++++++++++++++++ cmd/auth/login_scope_cache_test.go | 48 +++++++++++++++ cmd/auth/login_test.go | 98 ++++++++++++++++++++++++++++++ 4 files changed, 245 insertions(+), 2 deletions(-) create mode 100644 cmd/auth/login_scope_cache.go create mode 100644 cmd/auth/login_scope_cache_test.go diff --git a/cmd/auth/login.go b/cmd/auth/login.go index 7d73142c6..e490e239a 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -235,6 +235,9 @@ func authLoginRun(opts *LoginOptions) error { // --no-wait: return immediately with device code and URL if opts.NoWait { + if err := saveLoginRequestedScope(authResp.DeviceCode, finalScope); err != nil { + fmt.Fprintf(f.IOStreams.ErrOut, "[lark-cli] [WARN] auth login: failed to cache requested scopes: %v\n", err) + } data := map[string]interface{}{ "verification_url": authResp.VerificationUriComplete, "device_code": authResp.DeviceCode, @@ -340,16 +343,29 @@ func authLoginPollDeviceCode(opts *LoginOptions, config *core.CliConfig, msg *lo if err != nil { return err } + requestedScope, err := loadLoginRequestedScope(opts.DeviceCode) + if err != nil { + fmt.Fprintf(f.IOStreams.ErrOut, "[lark-cli] [WARN] auth login: failed to load cached requested scopes: %v\n", err) + } + cleanupRequestedScope := func() { + if err := removeLoginRequestedScope(opts.DeviceCode); err != nil { + fmt.Fprintf(f.IOStreams.ErrOut, "[lark-cli] [WARN] auth login: failed to remove cached requested scopes: %v\n", err) + } + } log(msg.WaitingAuth) result := larkauth.PollDeviceToken(opts.Ctx, httpClient, config.AppID, config.AppSecret, config.Brand, opts.DeviceCode, 5, 180, f.IOStreams.ErrOut) if !result.OK { + if shouldRemoveLoginRequestedScope(result) { + cleanupRequestedScope() + } return output.ErrAuth("authorization failed: %s", result.Message) } if result.Token == nil { return output.ErrAuth("authorization succeeded but no token returned") } + defer cleanupRequestedScope() // Get user info log(msg.AuthSuccess) @@ -362,7 +378,7 @@ func authLoginPollDeviceCode(opts *LoginOptions, config *core.CliConfig, msg *lo return output.ErrAuth("failed to get user info: %v", err) } - scopeSummary := loadLoginScopeSummary(config.AppID, openId, "", result.Token.Scope) + scopeSummary := loadLoginScopeSummary(config.AppID, openId, requestedScope, result.Token.Scope) // Store token now := time.Now().UnixMilli() @@ -386,7 +402,7 @@ func authLoginPollDeviceCode(opts *LoginOptions, config *core.CliConfig, msg *lo return output.Errorf(output.ExitInternal, "internal", "failed to update login profile: %v", err) } - if issue := ensureRequestedScopesGranted("", result.Token.Scope, msg, scopeSummary); issue != nil { + if issue := ensureRequestedScopesGranted(requestedScope, result.Token.Scope, msg, scopeSummary); issue != nil { return handleLoginScopeIssue(opts, msg, f, issue, openId, userName) } diff --git a/cmd/auth/login_scope_cache.go b/cmd/auth/login_scope_cache.go new file mode 100644 index 000000000..c9db737fb --- /dev/null +++ b/cmd/auth/login_scope_cache.go @@ -0,0 +1,81 @@ +package auth + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "regexp" + + larkauth "github.com/larksuite/cli/internal/auth" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/internal/vfs" +) + +var loginScopeCacheSafeChars = regexp.MustCompile(`[^a-zA-Z0-9._-]`) + +type loginScopeCacheRecord struct { + RequestedScope string `json:"requested_scope"` +} + +func loginScopeCacheDir() string { + return filepath.Join(core.GetConfigDir(), "cache", "auth_login_scopes") +} + +func loginScopeCachePath(deviceCode string) string { + return filepath.Join(loginScopeCacheDir(), sanitizeLoginScopeCacheKey(deviceCode)+".json") +} + +func sanitizeLoginScopeCacheKey(deviceCode string) string { + sanitized := loginScopeCacheSafeChars.ReplaceAllString(deviceCode, "_") + if sanitized == "" { + return "default" + } + return sanitized +} + +func saveLoginRequestedScope(deviceCode, requestedScope string) error { + if err := vfs.MkdirAll(loginScopeCacheDir(), 0700); err != nil { + return err + } + data, err := json.Marshal(loginScopeCacheRecord{RequestedScope: requestedScope}) + if err != nil { + return err + } + return validate.AtomicWrite(loginScopeCachePath(deviceCode), data, 0600) +} + +func loadLoginRequestedScope(deviceCode string) (string, error) { + data, err := vfs.ReadFile(loginScopeCachePath(deviceCode)) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return "", nil + } + return "", err + } + var record loginScopeCacheRecord + if err := json.Unmarshal(data, &record); err != nil { + _ = vfs.Remove(loginScopeCachePath(deviceCode)) + return "", err + } + return record.RequestedScope, nil +} + +func removeLoginRequestedScope(deviceCode string) error { + err := vfs.Remove(loginScopeCachePath(deviceCode)) + if errors.Is(err, os.ErrNotExist) { + return nil + } + return err +} + +func shouldRemoveLoginRequestedScope(result *larkauth.DeviceFlowResult) bool { + if result == nil { + return false + } + if result.OK || result.Error == "access_denied" { + return true + } + return result.Error == "expired_token" && result.Message != "Polling was cancelled" +} diff --git a/cmd/auth/login_scope_cache_test.go b/cmd/auth/login_scope_cache_test.go new file mode 100644 index 000000000..4d0bea91c --- /dev/null +++ b/cmd/auth/login_scope_cache_test.go @@ -0,0 +1,48 @@ +package auth + +import ( + "errors" + "os" + "testing" + + "github.com/larksuite/cli/internal/vfs" +) + +func TestLoginRequestedScopeCache_RoundTrip(t *testing.T) { + setupLoginConfigDir(t) + + deviceCode := "device/code:123" + requestedScope := "im:message:send im:message:reply" + + if err := saveLoginRequestedScope(deviceCode, requestedScope); err != nil { + t.Fatalf("saveLoginRequestedScope() error = %v", err) + } + got, err := loadLoginRequestedScope(deviceCode) + if err != nil { + t.Fatalf("loadLoginRequestedScope() error = %v", err) + } + if got != requestedScope { + t.Fatalf("requestedScope = %q, want %q", got, requestedScope) + } + if _, err := vfs.Stat(loginScopeCachePath(deviceCode)); err != nil { + t.Fatalf("Stat(cachePath) error = %v", err) + } + if err := removeLoginRequestedScope(deviceCode); err != nil { + t.Fatalf("removeLoginRequestedScope() error = %v", err) + } + if _, err := vfs.Stat(loginScopeCachePath(deviceCode)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("Stat(cachePath) error = %v, want not exist", err) + } +} + +func TestLoadLoginRequestedScope_MissingReturnsEmpty(t *testing.T) { + setupLoginConfigDir(t) + + got, err := loadLoginRequestedScope("missing-device-code") + if err != nil { + t.Fatalf("loadLoginRequestedScope() error = %v", err) + } + if got != "" { + t.Fatalf("requestedScope = %q, want empty", got) + } +} diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go index 1f06758df..432c4d61c 100644 --- a/cmd/auth/login_test.go +++ b/cmd/auth/login_test.go @@ -622,6 +622,104 @@ func TestAuthLoginRun_MissingRequestedScopeAlignsWithLoginSuccess(t *testing.T) } } +func TestAuthLoginRun_DeviceCodeUsesCachedRequestedScopes(t *testing.T) { + keyring.MockInit() + setupLoginConfigDir(t) + t.Setenv("HOME", t.TempDir()) + + multi := &core.MultiAppConfig{ + CurrentApp: "default", + Apps: []core.AppConfig{ + {Name: "default", AppId: "cli_test"}, + }, + } + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig() error = %v", err) + } + + f, stdout, stderr, reg := cmdutil.TestFactory(t, &core.CliConfig{ + ProfileName: "default", + AppID: "cli_test", + AppSecret: "secret", + Brand: core.BrandFeishu, + }) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: larkauth.PathDeviceAuthorization, + Body: map[string]interface{}{ + "device_code": "device-code", + "user_code": "user-code", + "verification_uri": "https://example.com/verify", + "verification_uri_complete": "https://example.com/verify?code=123", + "expires_in": 240, + "interval": 0, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: larkauth.PathOAuthTokenV2, + Body: map[string]interface{}{ + "access_token": "user-access-token", + "refresh_token": "refresh-token", + "expires_in": 7200, + "refresh_token_expires_in": 604800, + "scope": "im:message:send offline_access", + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: larkauth.PathUserInfoV1, + Body: map[string]interface{}{ + "code": 0, + "msg": "ok", + "data": map[string]interface{}{ + "open_id": "ou_user", + "name": "tester", + }, + }, + }) + + err := authLoginRun(&LoginOptions{ + Factory: f, + Ctx: context.Background(), + Scope: "im:message:send", + NoWait: true, + }) + if err != nil { + t.Fatalf("no-wait authLoginRun() error = %v", err) + } + if got, err := loadLoginRequestedScope("device-code"); err != nil || got != "im:message:send" { + t.Fatalf("loadLoginRequestedScope() = (%q, %v), want requested scope", got, err) + } + + stdout.Reset() + stderr.Reset() + + err = authLoginRun(&LoginOptions{ + Factory: f, + Ctx: context.Background(), + DeviceCode: "device-code", + }) + if err != nil { + t.Fatalf("device-code authLoginRun() error = %v", err) + } + got := stderr.String() + for _, want := range []string{ + "OK: 登录成功! 用户: tester (ou_user)", + "本次请求 scopes: im:message:send", + "本次新增 scopes: im:message:send", + "最终已授权 scopes: im:message:send offline_access", + } { + if !strings.Contains(got, want) { + t.Fatalf("stderr missing %q, got:\n%s", want, got) + } + } + if got, err := loadLoginRequestedScope("device-code"); err != nil || got != "" { + t.Fatalf("loadLoginRequestedScope() after cleanup = (%q, %v), want empty", got, err) + } +} + func TestAuthLoginRun_JSONWriteFailure_NoWaitIgnoresWriterError(t *testing.T) { f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ ProfileName: "default", From 9666dd9b54363746f8eb59c4e459a6677d2f2c9f Mon Sep 17 00:00:00 2001 From: zhaojunchang Date: Wed, 8 Apr 2026 15:41:52 +0800 Subject: [PATCH 3/8] docs(auth): add function comments for login scope handling Add detailed doc comments to all functions in login scope cache and result handling files to improve code documentation and maintainability. --- cmd/auth/login_result.go | 17 +++++++++++++++++ cmd/auth/login_scope_cache.go | 10 ++++++++++ 2 files changed, 27 insertions(+) diff --git a/cmd/auth/login_result.go b/cmd/auth/login_result.go index 978a343b7..b4198c5a7 100644 --- a/cmd/auth/login_result.go +++ b/cmd/auth/login_result.go @@ -25,6 +25,8 @@ type loginScopeIssue struct { Summary *loginScopeSummary } +// ensureRequestedScopesGranted checks whether all requested scopes were granted +// and returns a structured issue when any requested scope is missing. func ensureRequestedScopesGranted(requestedScope, grantedScope string, msg *loginMsg, summary *loginScopeSummary) *loginScopeIssue { requested := uniqueScopeList(requestedScope) if len(requested) == 0 { @@ -57,6 +59,8 @@ func ensureRequestedScopesGranted(requestedScope, grantedScope string, msg *logi } } +// loadLoginScopeSummary builds a scope summary by comparing the requested scopes, +// previously stored scopes, and the newly granted scopes from the current login. func loadLoginScopeSummary(appID, openId, requestedScope, grantedScope string) *loginScopeSummary { previousScope := "" if previous := larkauth.GetStoredToken(appID, openId); previous != nil { @@ -65,6 +69,8 @@ func loadLoginScopeSummary(appID, openId, requestedScope, grantedScope string) * return buildLoginScopeSummary(requestedScope, previousScope, grantedScope) } +// buildLoginScopeSummary classifies requested scopes into newly granted, +// already granted, and missing buckets while preserving the final granted list. func buildLoginScopeSummary(requestedScope, previousScope, grantedScope string) *loginScopeSummary { requested := uniqueScopeList(requestedScope) previous := uniqueScopeList(previousScope) @@ -96,6 +102,7 @@ func buildLoginScopeSummary(requestedScope, previousScope, grantedScope string) return summary } +// uniqueScopeList splits a scope string into a de-duplicated ordered slice. func uniqueScopeList(scope string) []string { seen := make(map[string]bool) var result []string @@ -109,6 +116,8 @@ func uniqueScopeList(scope string) []string { return result } +// formatScopeList joins scopes for display and falls back to the provided empty +// label when the input slice is empty. func formatScopeList(scopes []string, empty string) string { if len(scopes) == 0 { return empty @@ -116,6 +125,8 @@ func formatScopeList(scopes []string, empty string) string { return strings.Join(scopes, " ") } +// writeLoginScopeBreakdown renders the requested/newly granted/missing/final +// granted scope breakdown to stderr. func writeLoginScopeBreakdown(errOut *cmdutil.IOStreams, msg *loginMsg, summary *loginScopeSummary, pendingLabel string) { if summary == nil { summary = &loginScopeSummary{} @@ -133,6 +144,8 @@ func writeLoginScopeBreakdown(errOut *cmdutil.IOStreams, msg *loginMsg, summary fmt.Fprintf(errOut.ErrOut, msg.FinalGrantedScopes, formatScopeList(summary.Granted, msg.NoScopes)) } +// writeLoginSuccess emits the successful login payload in either JSON or text +// format together with the computed scope breakdown. func writeLoginSuccess(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory, openId, userName string, summary *loginScopeSummary) { if summary == nil { summary = &loginScopeSummary{} @@ -148,6 +161,8 @@ func writeLoginSuccess(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory, op writeLoginScopeBreakdown(f.IOStreams, msg, summary, "") } +// handleLoginScopeIssue prints or returns a structured missing-scope result +// while preserving a successful login outcome when authorization completed. func handleLoginScopeIssue(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory, issue *loginScopeIssue, openId, userName string) error { if issue == nil { return nil @@ -196,6 +211,8 @@ func handleLoginScopeIssue(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory return output.ErrBare(output.ExitAuth) } +// authorizationCompletePayload builds the JSON payload for a completed login, +// optionally attaching a warning when requested scopes are missing. func authorizationCompletePayload(openId, userName string, summary *loginScopeSummary, issue *loginScopeIssue) map[string]interface{} { if summary == nil { summary = &loginScopeSummary{} diff --git a/cmd/auth/login_scope_cache.go b/cmd/auth/login_scope_cache.go index c9db737fb..2549c0e35 100644 --- a/cmd/auth/login_scope_cache.go +++ b/cmd/auth/login_scope_cache.go @@ -19,14 +19,18 @@ type loginScopeCacheRecord struct { RequestedScope string `json:"requested_scope"` } +// loginScopeCacheDir returns the directory used to persist auth login --no-wait +// requested scopes keyed by device_code. func loginScopeCacheDir() string { return filepath.Join(core.GetConfigDir(), "cache", "auth_login_scopes") } +// loginScopeCachePath returns the cache file path for a given device_code. func loginScopeCachePath(deviceCode string) string { return filepath.Join(loginScopeCacheDir(), sanitizeLoginScopeCacheKey(deviceCode)+".json") } +// sanitizeLoginScopeCacheKey converts a device_code into a safe filename token. func sanitizeLoginScopeCacheKey(deviceCode string) string { sanitized := loginScopeCacheSafeChars.ReplaceAllString(deviceCode, "_") if sanitized == "" { @@ -35,6 +39,7 @@ func sanitizeLoginScopeCacheKey(deviceCode string) string { return sanitized } +// saveLoginRequestedScope persists the requested scope string for a device_code. func saveLoginRequestedScope(deviceCode, requestedScope string) error { if err := vfs.MkdirAll(loginScopeCacheDir(), 0700); err != nil { return err @@ -46,6 +51,8 @@ func saveLoginRequestedScope(deviceCode, requestedScope string) error { return validate.AtomicWrite(loginScopeCachePath(deviceCode), data, 0600) } +// loadLoginRequestedScope loads the cached requested scope string for a device_code. +// It returns an empty string if no cache entry exists. func loadLoginRequestedScope(deviceCode string) (string, error) { data, err := vfs.ReadFile(loginScopeCachePath(deviceCode)) if err != nil { @@ -62,6 +69,7 @@ func loadLoginRequestedScope(deviceCode string) (string, error) { return record.RequestedScope, nil } +// removeLoginRequestedScope deletes the cache entry for a device_code. func removeLoginRequestedScope(deviceCode string) error { err := vfs.Remove(loginScopeCachePath(deviceCode)) if errors.Is(err, os.ErrNotExist) { @@ -70,6 +78,8 @@ func removeLoginRequestedScope(deviceCode string) error { return err } +// shouldRemoveLoginRequestedScope indicates whether the requested-scope cache +// should be removed after polling finishes. func shouldRemoveLoginRequestedScope(result *larkauth.DeviceFlowResult) bool { if result == nil { return false From 85160a031741626c5b1a03441d22ffce7ea47408 Mon Sep 17 00:00:00 2001 From: zhaojunchang Date: Wed, 8 Apr 2026 16:02:33 +0800 Subject: [PATCH 4/8] refactor(auth): remove pending scopes and improve json output stability - Remove PendingScopes field and related logic as it's no longer needed - Add emptyIfNil helper to ensure nil slices are normalized to empty slices in JSON output - Update tests to verify JSON output stability and fix expected text outputs --- cmd/auth/login_messages.go | 3 --- cmd/auth/login_result.go | 32 +++++++++++++++++++------------- cmd/auth/login_test.go | 26 ++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 18 deletions(-) diff --git a/cmd/auth/login_messages.go b/cmd/auth/login_messages.go index ad5956dd0..3dc8fc431 100644 --- a/cmd/auth/login_messages.go +++ b/cmd/auth/login_messages.go @@ -28,7 +28,6 @@ type loginMsg struct { ScopeMismatch string ScopeHint string ScopeHintShort string - PendingScopes string RequestedScopes string NewlyGrantedScopes string AlreadyGrantedScopes string @@ -68,7 +67,6 @@ var loginMsgZh = &loginMsg{ ScopeMismatch: "授权完成，但以下请求 scopes 未被授予: %s", ScopeHint: "实际已授予 scopes: %s。请检查应用在飞书开发者后台是否已启用这些 scopes，并确认授权页已同意对应权限。可执行 `lark-cli auth scopes` 查看应用已启用 scopes。", ScopeHintShort: "请检查应用在飞书开发者后台是否已启用这些 scopes，并确认授权页已同意对应权限。可执行 `lark-cli auth scopes` 查看应用已启用 scopes。", - PendingScopes: "（授权后确认）", RequestedScopes: " 本次请求 scopes: %s\n", NewlyGrantedScopes: " 本次新增 scopes: %s\n", AlreadyGrantedScopes: " 已有 scopes: %s\n", @@ -107,7 +105,6 @@ var loginMsgEn = &loginMsg{ ScopeMismatch: "authorization completed, but these requested scopes were not granted: %s", ScopeHint: "Granted scopes: %s. Check whether the app has enabled these scopes in the developer console and whether the user approved them on the authorization page. Run `lark-cli auth scopes` to inspect the app's enabled scopes.", ScopeHintShort: "Check whether the app has enabled these scopes in the developer console and whether the user approved them on the authorization page. Run `lark-cli auth scopes` to inspect the app's enabled scopes.", - PendingScopes: "(determined after authorization)", RequestedScopes: " Requested scopes: %s\n", NewlyGrantedScopes: " Newly granted scopes: %s\n", AlreadyGrantedScopes: " Already granted scopes: %s\n", diff --git a/cmd/auth/login_result.go b/cmd/auth/login_result.go index b4198c5a7..9dc023686 100644 --- a/cmd/auth/login_result.go +++ b/cmd/auth/login_result.go @@ -125,19 +125,25 @@ func formatScopeList(scopes []string, empty string) string { return strings.Join(scopes, " ") } +// emptyIfNil normalizes nil slices to empty slices for stable JSON output. +func emptyIfNil(s []string) []string { + if s == nil { + return []string{} + } + return s +} + // writeLoginScopeBreakdown renders the requested/newly granted/missing/final // granted scope breakdown to stderr. -func writeLoginScopeBreakdown(errOut *cmdutil.IOStreams, msg *loginMsg, summary *loginScopeSummary, pendingLabel string) { +func writeLoginScopeBreakdown(errOut *cmdutil.IOStreams, msg *loginMsg, summary *loginScopeSummary) { if summary == nil { summary = &loginScopeSummary{} } fmt.Fprintf(errOut.ErrOut, msg.RequestedScopes, formatScopeList(summary.Requested, msg.NoScopes)) - if pendingLabel != "" { - fmt.Fprintf(errOut.ErrOut, msg.NewlyGrantedScopes, pendingLabel) - fmt.Fprintf(errOut.ErrOut, msg.MissingScopes, pendingLabel) - return - } fmt.Fprintf(errOut.ErrOut, msg.NewlyGrantedScopes, formatScopeList(summary.NewlyGranted, msg.NoScopes)) + if len(summary.AlreadyGranted) > 0 { + fmt.Fprintf(errOut.ErrOut, msg.AlreadyGrantedScopes, formatScopeList(summary.AlreadyGranted, msg.NoScopes)) + } if len(summary.Missing) > 0 { fmt.Fprintf(errOut.ErrOut, msg.MissingScopes, formatScopeList(summary.Missing, msg.NoScopes)) } @@ -158,7 +164,7 @@ func writeLoginSuccess(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory, op fmt.Fprintln(f.IOStreams.ErrOut) output.PrintSuccess(f.IOStreams.ErrOut, fmt.Sprintf(msg.LoginSuccess, userName, openId)) - writeLoginScopeBreakdown(f.IOStreams, msg, summary, "") + writeLoginScopeBreakdown(f.IOStreams, msg, summary) } // handleLoginScopeIssue prints or returns a structured missing-scope result @@ -199,7 +205,7 @@ func handleLoginScopeIssue(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory if loginSucceeded { fmt.Fprintln(f.IOStreams.ErrOut, issue.Message) } - writeLoginScopeBreakdown(f.IOStreams, msg, issue.Summary, "") + writeLoginScopeBreakdown(f.IOStreams, msg, issue.Summary) if issue.ShortHint != "" { fmt.Fprintln(f.IOStreams.ErrOut, issue.ShortHint) } else if issue.Hint != "" { @@ -222,11 +228,11 @@ func authorizationCompletePayload(openId, userName string, summary *loginScopeSu "user_open_id": openId, "user_name": userName, "scope": strings.Join(summary.Granted, " "), - "requested": summary.Requested, - "newly_granted": summary.NewlyGranted, - "already_granted": summary.AlreadyGranted, - "missing": summary.Missing, - "granted": summary.Granted, + "requested": emptyIfNil(summary.Requested), + "newly_granted": emptyIfNil(summary.NewlyGranted), + "already_granted": emptyIfNil(summary.AlreadyGranted), + "missing": emptyIfNil(summary.Missing), + "granted": emptyIfNil(summary.Granted), } if issue != nil { payload["warning"] = map[string]interface{}{ diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go index 432c4d61c..a42fa4c6f 100644 --- a/cmd/auth/login_test.go +++ b/cmd/auth/login_test.go @@ -424,6 +424,28 @@ func TestHandleLoginScopeIssue_JSONAlignsWithLoginSuccess(t *testing.T) { } } +func TestWriteLoginSuccess_JSONEmptySlicesNotNull(t *testing.T) { + f, stdout, _, _ := cmdutil.TestFactory(t, nil) + + writeLoginSuccess(&LoginOptions{JSON: true}, getLoginMsg("en"), f, "ou_user", "tester", &loginScopeSummary{ + Granted: []string{"offline_access"}, + }) + + var data map[string]interface{} + if err := json.Unmarshal(stdout.Bytes(), &data); err != nil { + t.Fatalf("Unmarshal(stdout) error = %v, stdout=%s", err, stdout.String()) + } + for _, k := range []string{"requested", "newly_granted", "already_granted", "missing", "granted"} { + v, ok := data[k] + if !ok { + t.Fatalf("missing key %q in payload: %v", k, data) + } + if _, ok := v.([]interface{}); !ok { + t.Fatalf("%s = %#v, want JSON array", k, v) + } + } +} + func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { tests := []struct { name string @@ -443,10 +465,10 @@ func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { "登录成功! 用户: tester (ou_user)", "本次请求 scopes: im:message:send im:message:reply", "本次新增 scopes: im:message:send", + "已有 scopes: im:message:reply", "最终已授权 scopes: im:message:send im:message:reply", }, expectedAbsent: []string{ - "已有 scopes:", "未授权 scopes:", }, }, @@ -460,10 +482,10 @@ func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { expectedPresent: []string{ "本次请求 scopes: im:message:send", "本次新增 scopes: （空）", + "已有 scopes: im:message:send", "最终已授权 scopes: im:message:send contact:user.base:readonly", }, expectedAbsent: []string{ - "已有 scopes:", "未授权 scopes:", }, }, From a194ec274dc3a4927c0c26c34b6648e7ca845e95 Mon Sep 17 00:00:00 2001 From: zhaojunchang Date: Wed, 8 Apr 2026 16:20:27 +0800 Subject: [PATCH 5/8] refactor(auth): extract device token polling function for testability Move device token polling to a package-level variable to enable mocking in tests Add test case for scope cleanup when token is nil --- cmd/auth/login.go | 8 +++++--- cmd/auth/login_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/cmd/auth/login.go b/cmd/auth/login.go index e490e239a..ae51ac279 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -34,6 +34,8 @@ type LoginOptions struct { DeviceCode string } +var pollDeviceToken = larkauth.PollDeviceToken + // NewCmdAuthLogin creates the auth login subcommand. func NewCmdAuthLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command { opts := &LoginOptions{Factory: f} @@ -273,7 +275,7 @@ func authLoginRun(opts *LoginOptions) error { // Step 3: Poll for token log(msg.WaitingAuth) - result := larkauth.PollDeviceToken(opts.Ctx, httpClient, config.AppID, config.AppSecret, config.Brand, + result := pollDeviceToken(opts.Ctx, httpClient, config.AppID, config.AppSecret, config.Brand, authResp.DeviceCode, authResp.Interval, authResp.ExpiresIn, f.IOStreams.ErrOut) if !result.OK { @@ -353,7 +355,7 @@ func authLoginPollDeviceCode(opts *LoginOptions, config *core.CliConfig, msg *lo } } log(msg.WaitingAuth) - result := larkauth.PollDeviceToken(opts.Ctx, httpClient, config.AppID, config.AppSecret, config.Brand, + result := pollDeviceToken(opts.Ctx, httpClient, config.AppID, config.AppSecret, config.Brand, opts.DeviceCode, 5, 180, f.IOStreams.ErrOut) if !result.OK { @@ -362,10 +364,10 @@ func authLoginPollDeviceCode(opts *LoginOptions, config *core.CliConfig, msg *lo } return output.ErrAuth("authorization failed: %s", result.Message) } + defer cleanupRequestedScope() if result.Token == nil { return output.ErrAuth("authorization succeeded but no token returned") } - defer cleanupRequestedScope() // Get user info log(msg.AuthSuccess) diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go index a42fa4c6f..a83b8a55e 100644 --- a/cmd/auth/login_test.go +++ b/cmd/auth/login_test.go @@ -7,6 +7,8 @@ import ( "context" "encoding/json" "errors" + "io" + "net/http" "sort" "strings" "testing" @@ -742,6 +744,43 @@ func TestAuthLoginRun_DeviceCodeUsesCachedRequestedScopes(t *testing.T) { } } +func TestAuthLoginRun_DeviceCodeTokenNilCleansScopeCache(t *testing.T) { + keyring.MockInit() + setupLoginConfigDir(t) + + if err := saveLoginRequestedScope("device-code", "im:message:send"); err != nil { + t.Fatalf("saveLoginRequestedScope() error = %v", err) + } + + original := pollDeviceToken + t.Cleanup(func() { pollDeviceToken = original }) + pollDeviceToken = func(ctx context.Context, httpClient *http.Client, appId, appSecret string, brand core.LarkBrand, deviceCode string, interval, expiresIn int, errOut io.Writer) *larkauth.DeviceFlowResult { + return &larkauth.DeviceFlowResult{OK: true, Token: nil} + } + + f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ + ProfileName: "default", + AppID: "cli_test", + AppSecret: "secret", + Brand: core.BrandFeishu, + }) + + err := authLoginRun(&LoginOptions{ + Factory: f, + Ctx: context.Background(), + DeviceCode: "device-code", + }) + if err == nil { + t.Fatal("expected error for nil token") + } + if !strings.Contains(err.Error(), "authorization succeeded but no token returned") { + t.Fatalf("error = %v, want nil token error", err) + } + if got, err := loadLoginRequestedScope("device-code"); err != nil || got != "" { + t.Fatalf("loadLoginRequestedScope() after nil token = (%q, %v), want empty", got, err) + } +} + func TestAuthLoginRun_JSONWriteFailure_NoWaitIgnoresWriterError(t *testing.T) { f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ ProfileName: "default", From 4ab384aecc5a0a85a2e294617b72364f543cec4e Mon Sep 17 00:00:00 2001 From: zhaojunchang Date: Wed, 8 Apr 2026 17:23:53 +0800 Subject: [PATCH 6/8] fix(auth): return JSON write errors instead of ignoring them Previously, JSON write errors were only logged to stderr but not returned, causing tests to pass when they should fail. Now properly propagate these errors to callers and update tests to verify error handling. --- cmd/auth/login.go | 13 ++++++++----- cmd/auth/login_test.go | 14 ++++++++++---- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/cmd/auth/login.go b/cmd/auth/login.go index ae51ac279..0398cc368 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -249,7 +249,7 @@ func authLoginRun(opts *LoginOptions) error { encoder := json.NewEncoder(f.IOStreams.Out) encoder.SetEscapeHTML(false) if err := encoder.Encode(data); err != nil { - fmt.Fprintf(f.IOStreams.ErrOut, "error: failed to write JSON output: %v\n", err) + return output.Errorf(output.ExitInternal, "internal", "failed to write JSON output: %v", err) } return nil } @@ -266,7 +266,7 @@ func authLoginRun(opts *LoginOptions) error { encoder := json.NewEncoder(f.IOStreams.Out) encoder.SetEscapeHTML(false) if err := encoder.Encode(data); err != nil { - fmt.Fprintf(f.IOStreams.ErrOut, "error: failed to write JSON output: %v\n", err) + return output.Errorf(output.ExitInternal, "internal", "failed to write JSON output: %v", err) } } else { fmt.Fprintf(f.IOStreams.ErrOut, msg.OpenURL) @@ -280,11 +280,14 @@ func authLoginRun(opts *LoginOptions) error { if !result.OK { if opts.JSON { - b, _ := json.Marshal(map[string]interface{}{ + encoder := json.NewEncoder(f.IOStreams.Out) + encoder.SetEscapeHTML(false) + if err := encoder.Encode(map[string]interface{}{ "event": "authorization_failed", "error": result.Message, - }) - fmt.Fprintln(f.IOStreams.Out, string(b)) + }); err != nil { + return output.Errorf(output.ExitInternal, "internal", "failed to write JSON output: %v", err) + } return output.ErrBare(output.ExitAuth) } return output.ErrAuth("authorization failed: %s", result.Message) diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go index a83b8a55e..28d0b50f4 100644 --- a/cmd/auth/login_test.go +++ b/cmd/auth/login_test.go @@ -781,7 +781,7 @@ func TestAuthLoginRun_DeviceCodeTokenNilCleansScopeCache(t *testing.T) { } } -func TestAuthLoginRun_JSONWriteFailure_NoWaitIgnoresWriterError(t *testing.T) { +func TestAuthLoginRun_JSONWriteFailure_NoWaitReturnsWriterError(t *testing.T) { f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ ProfileName: "default", AppID: "cli_test", @@ -810,12 +810,15 @@ func TestAuthLoginRun_JSONWriteFailure_NoWaitIgnoresWriterError(t *testing.T) { NoWait: true, JSON: true, }) - if err != nil { - t.Fatalf("expected nil error, got %v", err) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "failed to write JSON output") { + t.Fatalf("error = %v, want JSON write failure", err) } } -func TestAuthLoginRun_JSONWriteFailure_DeviceAuthorizationStillPolls(t *testing.T) { +func TestAuthLoginRun_JSONWriteFailure_DeviceAuthorizationReturnsWriterError(t *testing.T) { f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ ProfileName: "default", AppID: "cli_test", @@ -848,6 +851,9 @@ func TestAuthLoginRun_JSONWriteFailure_DeviceAuthorizationStillPolls(t *testing. if err == nil { t.Fatal("expected error") } + if !strings.Contains(err.Error(), "failed to write JSON output") { + t.Fatalf("error = %v, want JSON write failure", err) + } } func TestGetDomainMetadata_ExcludesEvent(t *testing.T) { From d34147cb10703e6fca005c14d16688c50a73a86c Mon Sep 17 00:00:00 2001 From: zhaojunchang Date: Wed, 8 Apr 2026 19:34:08 +0800 Subject: [PATCH 7/8] refactor(auth): simplify scope handling and improve user messaging remove redundant scope display and consolidate hint messages to focus on actionable guidance --- cmd/auth/login_messages.go | 75 +++++++++++++++------------------ cmd/auth/login_messages_test.go | 6 --- cmd/auth/login_result.go | 18 ++------ cmd/auth/login_test.go | 39 +++++++++++------ 4 files changed, 63 insertions(+), 75 deletions(-) diff --git a/cmd/auth/login_messages.go b/cmd/auth/login_messages.go index 3dc8fc431..d734a0816 100644 --- a/cmd/auth/login_messages.go +++ b/cmd/auth/login_messages.go @@ -20,20 +20,17 @@ type loginMsg struct { ConfirmAuth string // Non-interactive prompts (login.go) - OpenURL string - WaitingAuth string - AuthSuccess string - LoginSuccess string - GrantedScopes string - ScopeMismatch string - ScopeHint string - ScopeHintShort string - RequestedScopes string - NewlyGrantedScopes string - AlreadyGrantedScopes string - MissingScopes string - FinalGrantedScopes string - NoScopes string + OpenURL string + WaitingAuth string + AuthSuccess string + LoginSuccess string + ScopeMismatch string + ScopeHint string + ScopeHintShort string + RequestedScopes string + NewlyGrantedScopes string + MissingScopes string + NoScopes string // Non-interactive hint (no flags) HintHeader string @@ -59,20 +56,17 @@ var loginMsgZh = &loginMsg{ ErrNoDomain: "请至少选择一个业务域", ConfirmAuth: "确认授权?", - OpenURL: "在浏览器中打开以下链接进行认证:\n\n", - WaitingAuth: "等待用户授权...", - AuthSuccess: "授权成功，正在获取用户信息...", - LoginSuccess: "登录成功! 用户: %s (%s)", - GrantedScopes: " 已授权 scopes: %s\n", - ScopeMismatch: "授权完成，但以下请求 scopes 未被授予: %s", - ScopeHint: "实际已授予 scopes: %s。请检查应用在飞书开发者后台是否已启用这些 scopes，并确认授权页已同意对应权限。可执行 `lark-cli auth scopes` 查看应用已启用 scopes。", - ScopeHintShort: "请检查应用在飞书开发者后台是否已启用这些 scopes，并确认授权页已同意对应权限。可执行 `lark-cli auth scopes` 查看应用已启用 scopes。", - RequestedScopes: " 本次请求 scopes: %s\n", - NewlyGrantedScopes: " 本次新增 scopes: %s\n", - AlreadyGrantedScopes: " 已有 scopes: %s\n", - MissingScopes: " 未授权 scopes: %s\n", - FinalGrantedScopes: " 最终已授权 scopes: %s\n", - NoScopes: "（空）", + OpenURL: "在浏览器中打开以下链接进行认证:\n\n", + WaitingAuth: "等待用户授权...", + AuthSuccess: "授权成功，正在获取用户信息...", + LoginSuccess: "登录成功! 用户: %s (%s)", + ScopeMismatch: "授权完成，但以下请求 scopes 未被授予: %s", + ScopeHint: "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试。可执行 `lark-cli auth status` 查看当前账号实际已授权 scopes，执行 `lark-cli auth scopes` 查看应用已启用 scopes；如果仍需这些权限，请检查应用在飞书开发者后台是否已启用对应 scopes，并确认用户在授权页已同意相关权限。", + ScopeHintShort: "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试。可执行 `lark-cli auth status` 查看当前账号实际已授权 scopes，执行 `lark-cli auth scopes` 查看应用已启用 scopes。", + RequestedScopes: " 本次请求 scopes: %s\n", + NewlyGrantedScopes: " 本次新增 scopes: %s\n", + MissingScopes: " 未授权 scopes: %s\n", + NoScopes: "（空）", HintHeader: "请指定要授权的权限:\n", HintCommon1: " --recommend 授权推荐权限", @@ -97,20 +91,17 @@ var loginMsgEn = &loginMsg{ ErrNoDomain: "please select at least one domain", ConfirmAuth: "Confirm authorization?", - OpenURL: "Open this URL in your browser to authenticate:\n\n", - WaitingAuth: "Waiting for user authorization...", - AuthSuccess: "Authorization successful, fetching user info...", - LoginSuccess: "Login successful! User: %s (%s)", - GrantedScopes: " Granted scopes: %s\n", - ScopeMismatch: "authorization completed, but these requested scopes were not granted: %s", - ScopeHint: "Granted scopes: %s. Check whether the app has enabled these scopes in the developer console and whether the user approved them on the authorization page. Run `lark-cli auth scopes` to inspect the app's enabled scopes.", - ScopeHintShort: "Check whether the app has enabled these scopes in the developer console and whether the user approved them on the authorization page. Run `lark-cli auth scopes` to inspect the app's enabled scopes.", - RequestedScopes: " Requested scopes: %s\n", - NewlyGrantedScopes: " Newly granted scopes: %s\n", - AlreadyGrantedScopes: " Already granted scopes: %s\n", - MissingScopes: " Missing scopes: %s\n", - FinalGrantedScopes: " Final granted scopes: %s\n", - NoScopes: "(none)", + OpenURL: "Open this URL in your browser to authenticate:\n\n", + WaitingAuth: "Waiting for user authorization...", + AuthSuccess: "Authorization successful, fetching user info...", + LoginSuccess: "Login successful! User: %s (%s)", + ScopeMismatch: "authorization completed, but these requested scopes were not granted: %s", + ScopeHint: "These missing scopes are the final outcome of this authorization and should not be retried continuously. Run `lark-cli auth status` to inspect the scopes currently granted to this account, and run `lark-cli auth scopes` to inspect the scopes enabled for the app. If these scopes are still required, verify the app configuration in the developer console and confirm that the user approved them on the authorization page.", + ScopeHintShort: "These missing scopes are the final outcome of this authorization and should not be retried continuously. Run `lark-cli auth status` to inspect the scopes currently granted to this account, and run `lark-cli auth scopes` to inspect the scopes enabled for the app.", + RequestedScopes: " Requested scopes: %s\n", + NewlyGrantedScopes: " Newly granted scopes: %s\n", + MissingScopes: " Missing scopes: %s\n", + NoScopes: "(none)", HintHeader: "Please specify the scopes to authorize:\n", HintCommon1: " --recommend authorize recommended scopes", diff --git a/cmd/auth/login_messages_test.go b/cmd/auth/login_messages_test.go index 500866ded..f0c8808b7 100644 --- a/cmd/auth/login_messages_test.go +++ b/cmd/auth/login_messages_test.go @@ -69,12 +69,6 @@ func TestLoginMsg_FormatStrings(t *testing.T) { t.Errorf("%s LoginSuccess has no format verb", lang) } - // GrantedScopes should contain %s - got = fmt.Sprintf(msg.GrantedScopes, "scope1 scope2") - if got == msg.GrantedScopes { - t.Errorf("%s GrantedScopes has no format verb", lang) - } - // SummaryDomains should contain %s got = fmt.Sprintf(msg.SummaryDomains, "calendar, task") if got == msg.SummaryDomains { diff --git a/cmd/auth/login_result.go b/cmd/auth/login_result.go index 9dc023686..d6329a356 100644 --- a/cmd/auth/login_result.go +++ b/cmd/auth/login_result.go @@ -38,22 +38,16 @@ func ensureRequestedScopesGranted(requestedScope, grantedScope string, msg *logi return nil } - granted := strings.Fields(grantedScope) - grantedDisplay := msg.NoScopes - if len(granted) > 0 { - grantedDisplay = strings.Join(granted, " ") - } - if summary == nil { summary = &loginScopeSummary{ Requested: requested, - Granted: granted, + Granted: strings.Fields(grantedScope), Missing: missing, } } return &loginScopeIssue{ Message: fmt.Sprintf(msg.ScopeMismatch, strings.Join(missing, " ")), - Hint: fmt.Sprintf(msg.ScopeHint, grantedDisplay), + Hint: msg.ScopeHint, ShortHint: msg.ScopeHintShort, Summary: summary, } @@ -133,21 +127,17 @@ func emptyIfNil(s []string) []string { return s } -// writeLoginScopeBreakdown renders the requested/newly granted/missing/final -// granted scope breakdown to stderr. +// writeLoginScopeBreakdown renders the requested/newly granted/missing scope +// breakdown to stderr. func writeLoginScopeBreakdown(errOut *cmdutil.IOStreams, msg *loginMsg, summary *loginScopeSummary) { if summary == nil { summary = &loginScopeSummary{} } fmt.Fprintf(errOut.ErrOut, msg.RequestedScopes, formatScopeList(summary.Requested, msg.NoScopes)) fmt.Fprintf(errOut.ErrOut, msg.NewlyGrantedScopes, formatScopeList(summary.NewlyGranted, msg.NoScopes)) - if len(summary.AlreadyGranted) > 0 { - fmt.Fprintf(errOut.ErrOut, msg.AlreadyGrantedScopes, formatScopeList(summary.AlreadyGranted, msg.NoScopes)) - } if len(summary.Missing) > 0 { fmt.Fprintf(errOut.ErrOut, msg.MissingScopes, formatScopeList(summary.Missing, msg.NoScopes)) } - fmt.Fprintf(errOut.ErrOut, msg.FinalGrantedScopes, formatScopeList(summary.Granted, msg.NoScopes)) } // writeLoginSuccess emits the successful login payload in either JSON or text diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go index 28d0b50f4..79ee87f1f 100644 --- a/cmd/auth/login_test.go +++ b/cmd/auth/login_test.go @@ -303,8 +303,10 @@ func TestEnsureRequestedScopesGranted(t *testing.T) { if !strings.Contains(issue.Message, "im:message:send") { t.Fatalf("message %q missing requested scope", issue.Message) } - if !strings.Contains(issue.Hint, "Granted scopes: im:message:reply") { - t.Fatalf("hint %q missing granted scope context", issue.Hint) + for _, want := range []string{"should not be retried continuously", "lark-cli auth status", "lark-cli auth scopes"} { + if !strings.Contains(issue.Hint, want) { + t.Fatalf("hint %q missing %q", issue.Hint, want) + } } if got := strings.Join(issue.Summary.Missing, " "); got != "im:message:send" { t.Fatalf("Missing = %q", got) @@ -362,8 +364,8 @@ func TestHandleLoginScopeIssue_NonJSONAlignsWithLoginSuccess(t *testing.T) { f, _, stderr, _ := cmdutil.TestFactory(t, nil) err := handleLoginScopeIssue(&LoginOptions{}, getLoginMsg("zh"), f, &loginScopeIssue{ Message: "授权完成，但以下请求 scopes 未被授予: im:message:send", - Hint: "实际已授予 scopes: base:app:copy。请检查后台配置", - ShortHint: "请检查后台配置", + Hint: "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试。可执行 `lark-cli auth status` 查看当前账号实际已授权 scopes，执行 `lark-cli auth scopes` 查看应用已启用 scopes；如果仍需这些权限，请检查应用在飞书开发者后台是否已启用对应 scopes，并确认用户在授权页已同意相关权限。", + ShortHint: "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试。可执行 `lark-cli auth status` 查看当前账号实际已授权 scopes，执行 `lark-cli auth scopes` 查看应用已启用 scopes。", Summary: &loginScopeSummary{ Requested: []string{"im:message:send"}, Missing: []string{"im:message:send"}, @@ -380,13 +382,17 @@ func TestHandleLoginScopeIssue_NonJSONAlignsWithLoginSuccess(t *testing.T) { "本次请求 scopes: im:message:send", "本次新增 scopes: （空）", "未授权 scopes: im:message:send", - "最终已授权 scopes: base:app:copy", - "请检查后台配置", + "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试", + "lark-cli auth status", + "lark-cli auth scopes", } { if !strings.Contains(got, want) { t.Fatalf("stderr missing %q, got:\n%s", want, got) } } + if strings.Contains(got, "最终已授权 scopes:") { + t.Fatalf("stderr should not contain final granted scopes, got:\n%s", got) + } if strings.Contains(got, "ERROR:") { t.Fatalf("stderr should not contain error prefix, got:\n%s", got) } @@ -467,11 +473,11 @@ func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { "登录成功! 用户: tester (ou_user)", "本次请求 scopes: im:message:send im:message:reply", "本次新增 scopes: im:message:send", - "已有 scopes: im:message:reply", - "最终已授权 scopes: im:message:send im:message:reply", }, expectedAbsent: []string{ "未授权 scopes:", + "最终已授权 scopes:", + "已有 scopes:", }, }, { @@ -484,11 +490,11 @@ func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { expectedPresent: []string{ "本次请求 scopes: im:message:send", "本次新增 scopes: （空）", - "已有 scopes: im:message:send", - "最终已授权 scopes: im:message:send contact:user.base:readonly", }, expectedAbsent: []string{ "未授权 scopes:", + "最终已授权 scopes:", + "已有 scopes:", }, }, { @@ -502,10 +508,10 @@ func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { "本次请求 scopes: im:message:send im:message:reply", "本次新增 scopes: （空）", "未授权 scopes: im:message:send", - "最终已授权 scopes: im:message:reply", }, expectedAbsent: []string{ "已有 scopes:", + "最终已授权 scopes:", }, }, } @@ -615,12 +621,17 @@ func TestAuthLoginRun_MissingRequestedScopeAlignsWithLoginSuccess(t *testing.T) "授权完成，但以下请求 scopes 未被授予: im:message:send", "本次请求 scopes: im:message:send", "未授权 scopes: im:message:send", - "最终已授权 scopes: offline_access", + "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试", + "lark-cli auth status", + "lark-cli auth scopes", } { if !strings.Contains(got, want) { t.Fatalf("stderr missing %q, got:\n%s", want, got) } } + if strings.Contains(got, "最终已授权 scopes:") { + t.Fatalf("stderr should not contain final granted scopes, got:\n%s", got) + } if strings.Contains(got, "ERROR:") { t.Fatalf("stderr should not contain error prefix, got:\n%s", got) } @@ -733,12 +744,14 @@ func TestAuthLoginRun_DeviceCodeUsesCachedRequestedScopes(t *testing.T) { "OK: 登录成功! 用户: tester (ou_user)", "本次请求 scopes: im:message:send", "本次新增 scopes: im:message:send", - "最终已授权 scopes: im:message:send offline_access", } { if !strings.Contains(got, want) { t.Fatalf("stderr missing %q, got:\n%s", want, got) } } + if strings.Contains(got, "最终已授权 scopes:") { + t.Fatalf("stderr should not contain final granted scopes, got:\n%s", got) + } if got, err := loadLoginRequestedScope("device-code"); err != nil || got != "" { t.Fatalf("loadLoginRequestedScope() after cleanup = (%q, %v), want empty", got, err) } From d6499273d99dffad0576323b484b35989d0ac226 Mon Sep 17 00:00:00 2001 From: zhaojunchang Date: Wed, 8 Apr 2026 20:50:34 +0800 Subject: [PATCH 8/8] refactor(auth): improve scope handling and messaging in login flow remove ShortHint field and simplify scope hint messages always display missing scopes section with consistent formatting add StatusHint for successful login with no missing scopes update tests to reflect new message structure and content --- cmd/auth/login_messages.go | 16 +++++----- cmd/auth/login_result.go | 25 +++++++-------- cmd/auth/login_test.go | 62 +++++++++++++++++++++++++++----------- 3 files changed, 63 insertions(+), 40 deletions(-) diff --git a/cmd/auth/login_messages.go b/cmd/auth/login_messages.go index d734a0816..9de3361d8 100644 --- a/cmd/auth/login_messages.go +++ b/cmd/auth/login_messages.go @@ -26,11 +26,11 @@ type loginMsg struct { LoginSuccess string ScopeMismatch string ScopeHint string - ScopeHintShort string RequestedScopes string NewlyGrantedScopes string MissingScopes string NoScopes string + StatusHint string // Non-interactive hint (no flags) HintHeader string @@ -61,12 +61,12 @@ var loginMsgZh = &loginMsg{ AuthSuccess: "授权成功，正在获取用户信息...", LoginSuccess: "登录成功! 用户: %s (%s)", ScopeMismatch: "授权完成，但以下请求 scopes 未被授予: %s", - ScopeHint: "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试。可执行 `lark-cli auth status` 查看当前账号实际已授权 scopes，执行 `lark-cli auth scopes` 查看应用已启用 scopes；如果仍需这些权限，请检查应用在飞书开发者后台是否已启用对应 scopes，并确认用户在授权页已同意相关权限。", - ScopeHintShort: "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试。可执行 `lark-cli auth status` 查看当前账号实际已授权 scopes，执行 `lark-cli auth scopes` 查看应用已启用 scopes。", + ScopeHint: "以上结果是本次授权请求用户最终确认后的结果，请勿持续重试；Scopes 未授予的原因是多样的，如 scope 被禁用；具体原因已通过授权页提示用户。可执行 `lark-cli auth status` 查看账号当前已授予的全部 scopes；", RequestedScopes: " 本次请求 scopes: %s\n", - NewlyGrantedScopes: " 本次新增 scopes: %s\n", - MissingScopes: " 未授权 scopes: %s\n", + NewlyGrantedScopes: " 本次新授予 scopes: %s\n", + MissingScopes: " 本次未授予 scopes: %s\n", NoScopes: "（空）", + StatusHint: "可执行 `lark-cli auth status` 查看账号当前已授予的全部 scopes；", HintHeader: "请指定要授权的权限:\n", HintCommon1: " --recommend 授权推荐权限", @@ -96,12 +96,12 @@ var loginMsgEn = &loginMsg{ AuthSuccess: "Authorization successful, fetching user info...", LoginSuccess: "Login successful! User: %s (%s)", ScopeMismatch: "authorization completed, but these requested scopes were not granted: %s", - ScopeHint: "These missing scopes are the final outcome of this authorization and should not be retried continuously. Run `lark-cli auth status` to inspect the scopes currently granted to this account, and run `lark-cli auth scopes` to inspect the scopes enabled for the app. If these scopes are still required, verify the app configuration in the developer console and confirm that the user approved them on the authorization page.", - ScopeHintShort: "These missing scopes are the final outcome of this authorization and should not be retried continuously. Run `lark-cli auth status` to inspect the scopes currently granted to this account, and run `lark-cli auth scopes` to inspect the scopes enabled for the app.", + ScopeHint: "The result above is the user's final confirmation for this authorization request. Do not retry continuously. Scopes may be not granted for various reasons, such as a scope being disabled. The specific reason has already been shown to the user on the authorization page. Run `lark-cli auth status` to inspect all scopes currently granted to the account.", RequestedScopes: " Requested scopes: %s\n", NewlyGrantedScopes: " Newly granted scopes: %s\n", - MissingScopes: " Missing scopes: %s\n", + MissingScopes: " Not granted scopes: %s\n", NoScopes: "(none)", + StatusHint: "Run `lark-cli auth status` to inspect all scopes currently granted to the account.", HintHeader: "Please specify the scopes to authorize:\n", HintCommon1: " --recommend authorize recommended scopes", diff --git a/cmd/auth/login_result.go b/cmd/auth/login_result.go index d6329a356..ef288d45e 100644 --- a/cmd/auth/login_result.go +++ b/cmd/auth/login_result.go @@ -19,10 +19,9 @@ type loginScopeSummary struct { } type loginScopeIssue struct { - Message string - Hint string - ShortHint string - Summary *loginScopeSummary + Message string + Hint string + Summary *loginScopeSummary } // ensureRequestedScopesGranted checks whether all requested scopes were granted @@ -46,10 +45,9 @@ func ensureRequestedScopesGranted(requestedScope, grantedScope string, msg *logi } } return &loginScopeIssue{ - Message: fmt.Sprintf(msg.ScopeMismatch, strings.Join(missing, " ")), - Hint: msg.ScopeHint, - ShortHint: msg.ScopeHintShort, - Summary: summary, + Message: fmt.Sprintf(msg.ScopeMismatch, strings.Join(missing, " ")), + Hint: msg.ScopeHint, + Summary: summary, } } @@ -135,9 +133,7 @@ func writeLoginScopeBreakdown(errOut *cmdutil.IOStreams, msg *loginMsg, summary } fmt.Fprintf(errOut.ErrOut, msg.RequestedScopes, formatScopeList(summary.Requested, msg.NoScopes)) fmt.Fprintf(errOut.ErrOut, msg.NewlyGrantedScopes, formatScopeList(summary.NewlyGranted, msg.NoScopes)) - if len(summary.Missing) > 0 { - fmt.Fprintf(errOut.ErrOut, msg.MissingScopes, formatScopeList(summary.Missing, msg.NoScopes)) - } + fmt.Fprintf(errOut.ErrOut, msg.MissingScopes, formatScopeList(summary.Missing, msg.NoScopes)) } // writeLoginSuccess emits the successful login payload in either JSON or text @@ -155,6 +151,9 @@ func writeLoginSuccess(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory, op fmt.Fprintln(f.IOStreams.ErrOut) output.PrintSuccess(f.IOStreams.ErrOut, fmt.Sprintf(msg.LoginSuccess, userName, openId)) writeLoginScopeBreakdown(f.IOStreams, msg, summary) + if len(summary.Missing) == 0 && msg.StatusHint != "" { + fmt.Fprintln(f.IOStreams.ErrOut, msg.StatusHint) + } } // handleLoginScopeIssue prints or returns a structured missing-scope result @@ -196,9 +195,7 @@ func handleLoginScopeIssue(opts *LoginOptions, msg *loginMsg, f *cmdutil.Factory fmt.Fprintln(f.IOStreams.ErrOut, issue.Message) } writeLoginScopeBreakdown(f.IOStreams, msg, issue.Summary) - if issue.ShortHint != "" { - fmt.Fprintln(f.IOStreams.ErrOut, issue.ShortHint) - } else if issue.Hint != "" { + if issue.Hint != "" { fmt.Fprintln(f.IOStreams.ErrOut, issue.Hint) } if loginSucceeded { diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go index 79ee87f1f..fa6942b3e 100644 --- a/cmd/auth/login_test.go +++ b/cmd/auth/login_test.go @@ -303,7 +303,7 @@ func TestEnsureRequestedScopesGranted(t *testing.T) { if !strings.Contains(issue.Message, "im:message:send") { t.Fatalf("message %q missing requested scope", issue.Message) } - for _, want := range []string{"should not be retried continuously", "lark-cli auth status", "lark-cli auth scopes"} { + for _, want := range []string{"Do not retry continuously", "scope being disabled", "lark-cli auth status"} { if !strings.Contains(issue.Hint, want) { t.Fatalf("hint %q missing %q", issue.Hint, want) } @@ -363,9 +363,8 @@ func TestWriteLoginSuccess_JSONIncludesScopeDiff(t *testing.T) { func TestHandleLoginScopeIssue_NonJSONAlignsWithLoginSuccess(t *testing.T) { f, _, stderr, _ := cmdutil.TestFactory(t, nil) err := handleLoginScopeIssue(&LoginOptions{}, getLoginMsg("zh"), f, &loginScopeIssue{ - Message: "授权完成，但以下请求 scopes 未被授予: im:message:send", - Hint: "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试。可执行 `lark-cli auth status` 查看当前账号实际已授权 scopes，执行 `lark-cli auth scopes` 查看应用已启用 scopes；如果仍需这些权限，请检查应用在飞书开发者后台是否已启用对应 scopes，并确认用户在授权页已同意相关权限。", - ShortHint: "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试。可执行 `lark-cli auth status` 查看当前账号实际已授权 scopes，执行 `lark-cli auth scopes` 查看应用已启用 scopes。", + Message: "授权完成，但以下请求 scopes 未被授予: im:message:send", + Hint: "以上结果是本次授权请求用户最终确认后的结果，请勿持续重试；Scopes 未授予的原因是多样的，如 scope 被禁用；具体原因已通过授权页提示用户。可执行 `lark-cli auth status` 查看账号当前已授予的全部 scopes；", Summary: &loginScopeSummary{ Requested: []string{"im:message:send"}, Missing: []string{"im:message:send"}, @@ -380,11 +379,11 @@ func TestHandleLoginScopeIssue_NonJSONAlignsWithLoginSuccess(t *testing.T) { "OK: 登录成功! 用户: tester (ou_user)", "授权完成，但以下请求 scopes 未被授予: im:message:send", "本次请求 scopes: im:message:send", - "本次新增 scopes: （空）", - "未授权 scopes: im:message:send", - "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试", + "本次新授予 scopes: （空）", + "本次未授予 scopes: im:message:send", + "以上结果是本次授权请求用户最终确认后的结果，请勿持续重试", + "scope 被禁用", "lark-cli auth status", - "lark-cli auth scopes", } { if !strings.Contains(got, want) { t.Fatalf("stderr missing %q, got:\n%s", want, got) @@ -472,10 +471,11 @@ func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { expectedPresent: []string{ "登录成功! 用户: tester (ou_user)", "本次请求 scopes: im:message:send im:message:reply", - "本次新增 scopes: im:message:send", + "本次新授予 scopes: im:message:send", + "本次未授予 scopes: （空）", + "可执行 `lark-cli auth status` 查看账号当前已授予的全部 scopes；", }, expectedAbsent: []string{ - "未授权 scopes:", "最终已授权 scopes:", "已有 scopes:", }, @@ -489,10 +489,11 @@ func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { }, expectedPresent: []string{ "本次请求 scopes: im:message:send", - "本次新增 scopes: （空）", + "本次新授予 scopes: （空）", + "本次未授予 scopes: （空）", + "可执行 `lark-cli auth status` 查看账号当前已授予的全部 scopes；", }, expectedAbsent: []string{ - "未授权 scopes:", "最终已授权 scopes:", "已有 scopes:", }, @@ -506,12 +507,13 @@ func TestWriteLoginSuccess_TextOutputScenarios(t *testing.T) { }, expectedPresent: []string{ "本次请求 scopes: im:message:send im:message:reply", - "本次新增 scopes: （空）", - "未授权 scopes: im:message:send", + "本次新授予 scopes: （空）", + "本次未授予 scopes: im:message:send", }, expectedAbsent: []string{ "已有 scopes:", "最终已授权 scopes:", + "可执行 `lark-cli auth status` 查看账号当前已授予的全部 scopes；", }, }, } @@ -620,10 +622,10 @@ func TestAuthLoginRun_MissingRequestedScopeAlignsWithLoginSuccess(t *testing.T) "OK: 登录成功! 用户: tester (ou_user)", "授权完成，但以下请求 scopes 未被授予: im:message:send", "本次请求 scopes: im:message:send", - "未授权 scopes: im:message:send", - "以上未授权 scopes 是用户本次授权的最终结果，请不要持续重试", + "本次未授予 scopes: im:message:send", + "以上结果是本次授权请求用户最终确认后的结果，请勿持续重试", + "scope 被禁用", "lark-cli auth status", - "lark-cli auth scopes", } { if !strings.Contains(got, want) { t.Fatalf("stderr missing %q, got:\n%s", want, got) @@ -743,7 +745,8 @@ func TestAuthLoginRun_DeviceCodeUsesCachedRequestedScopes(t *testing.T) { for _, want := range []string{ "OK: 登录成功! 用户: tester (ou_user)", "本次请求 scopes: im:message:send", - "本次新增 scopes: im:message:send", + "本次新授予 scopes: im:message:send", + "可执行 `lark-cli auth status` 查看账号当前已授予的全部 scopes；", } { if !strings.Contains(got, want) { t.Fatalf("stderr missing %q, got:\n%s", want, got) @@ -757,6 +760,29 @@ func TestAuthLoginRun_DeviceCodeUsesCachedRequestedScopes(t *testing.T) { } } +func TestWriteLoginSuccess_TextOutputEnglishIncludesStatusHintWhenNoMissingScopes(t *testing.T) { + f, _, stderr, _ := cmdutil.TestFactory(t, nil) + + writeLoginSuccess(&LoginOptions{}, getLoginMsg("en"), f, "ou_user", "tester", &loginScopeSummary{ + Requested: []string{"im:message:send"}, + NewlyGranted: []string{"im:message:send"}, + Granted: []string{"im:message:send"}, + }) + + got := stderr.String() + for _, want := range []string{ + "Login successful! User: tester (ou_user)", + "Requested scopes: im:message:send", + "Newly granted scopes: im:message:send", + "Not granted scopes: (none)", + "Run `lark-cli auth status` to inspect all scopes currently granted to the account.", + } { + if !strings.Contains(got, want) { + t.Fatalf("stderr missing %q, got:\n%s", want, got) + } + } +} + func TestAuthLoginRun_DeviceCodeTokenNilCleansScopeCache(t *testing.T) { keyring.MockInit() setupLoginConfigDir(t)