From ea9eb27614005c153c40ac642014fb9cad097c53 Mon Sep 17 00:00:00 2001 From: kiraWangRuilong Date: Thu, 30 Jul 2026 22:54:56 +0800 Subject: [PATCH 1/4] refactor: optimize refresh token error handling --- internal/auth/uat_client.go | 349 +++++++++++++++++++++-------- internal/errclass/codemeta.go | 20 +- internal/errclass/codemeta_test.go | 18 +- 3 files changed, 289 insertions(+), 98 deletions(-) diff --git a/internal/auth/uat_client.go b/internal/auth/uat_client.go index d1a0683927..1f08789d10 100644 --- a/internal/auth/uat_client.go +++ b/internal/auth/uat_client.go @@ -4,17 +4,18 @@ package auth import ( + "bytes" "context" "encoding/json" "fmt" "io" "net/http" - "net/url" + "net/http/httptrace" "os" "path/filepath" "regexp" - "strings" "sync" + "sync/atomic" "time" "github.com/gofrs/flock" @@ -170,6 +171,51 @@ func refreshWithLock(httpClient *http.Client, opts UATCallOptions, stored *Store return doRefreshToken(httpClient, opts, stored) } +const refreshMaxAttempts = 2 + +type refreshRequest struct { + GrantType string `json:"grant_type"` + RefreshToken string `json:"refresh_token"` + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` +} + +// refreshResponse contains only fields documented by the OAuth token endpoint. +// Pointers distinguish an omitted numeric field from a real zero value. +type refreshResponse struct { + Code *int `json:"code"` + AccessToken string `json:"access_token"` + ExpiresIn *int64 `json:"expires_in"` + RefreshToken string `json:"refresh_token"` + RefreshTokenExpiresIn *int64 `json:"refresh_token_expires_in"` + TokenType string `json:"token_type"` + Scope string `json:"scope"` + Error string `json:"error"` + ErrorDescription string `json:"error_description"` +} + +// refreshAction describes both retry behavior and local token disposition. +type refreshAction uint8 + +const ( + // refreshSaveResponse saves a successful response. + refreshSaveResponse refreshAction = iota + // refreshRetryAndPreserve retries, preserving the stored token if retry fails. + refreshRetryAndPreserve + // refreshRetryAndClear retries, clearing the stored token if retry fails. + refreshRetryAndClear + // refreshStopAndPreserve stops without clearing the stored token. + refreshStopAndPreserve + // refreshStopAndClear stops and clears the stored token. + refreshStopAndClear +) + +type refreshResult struct { + action refreshAction + response refreshResponse + err error +} + // doRefreshToken performs the actual HTTP request to refresh the token. func doRefreshToken(httpClient *http.Client, opts UATCallOptions, stored *StoredUAToken) (*StoredUAToken, error) { errOut := opts.ErrOut @@ -177,8 +223,7 @@ func doRefreshToken(httpClient *http.Client, opts UATCallOptions, stored *Stored errOut = os.Stderr } - now := time.Now().UnixMilli() - if now >= stored.RefreshExpiresAt { + if time.Now().UnixMilli() >= stored.RefreshExpiresAt { fmt.Fprintf(errOut, "[lark-cli] uat-client: refresh_token expired for %s, clearing\n", opts.UserOpenId) if err := RemoveStoredToken(opts.AppId, opts.UserOpenId); err != nil { fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: failed to remove expired token: %v\n", err) @@ -186,132 +231,246 @@ func doRefreshToken(httpClient *http.Client, opts UATCallOptions, stored *Stored return nil, nil } - endpoints := ResolveOAuthEndpoints(opts.Domain) + endpoint := ResolveOAuthEndpoints(opts.Domain).Token + uncertain := false + for attempt := 1; attempt <= refreshMaxAttempts; attempt++ { + result := refreshOnce(httpClient, endpoint, opts, stored) + if result.action == refreshSaveResponse { + return saveRefreshResponse(opts, stored, result.response) + } - callEndpoint := func() (map[string]interface{}, error) { - form := url.Values{} - form.Set("grant_type", "refresh_token") - form.Set("refresh_token", stored.RefreshToken) - form.Set("client_id", opts.AppId) - form.Set("client_secret", opts.AppSecret) + switch result.action { + case refreshRetryAndPreserve, refreshRetryAndClear: + if result.action == refreshRetryAndClear { + uncertain = true + } + if attempt < refreshMaxAttempts { + fmt.Fprintf(errOut, + "[lark-cli] [WARN] uat-client: refresh attempt %d/%d failed for %s: %v; retrying\n", + attempt, refreshMaxAttempts, opts.UserOpenId, result.err) + continue + } + case refreshStopAndPreserve, refreshStopAndClear: + default: + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "unrecognized token refresh action %d", result.action) + } - req, err := http.NewRequest("POST", endpoints.Token, strings.NewReader(form.Encode())) - if err != nil { - return nil, err + clearToken := result.action == refreshStopAndClear || + result.action == refreshRetryAndClear || + (result.action == refreshRetryAndPreserve && uncertain) + if !clearToken { + fmt.Fprintf(errOut, + "[lark-cli] [WARN] uat-client: refresh failed for %s, preserving token: %v\n", + opts.UserOpenId, result.err) + return nil, result.err } - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := httpClient.Do(req) - if err != nil { - return nil, err + if problem, ok := errs.ProblemOf(result.err); ok { + problem.Retryable = false + } + if err := RemoveStoredToken(opts.AppId, opts.UserOpenId); err != nil { + fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: failed to remove token: %v\n", err) + if _, ok := errs.ProblemOf(err); ok { + return nil, err + } + return nil, errs.NewInternalError(errs.SubtypeStorage, + "failed to remove refreshed user token for user %q: %v", opts.UserOpenId, err). + WithCause(err) } - defer resp.Body.Close() - logHTTPResponse(resp) + fmt.Fprintf(errOut, + "[lark-cli] [WARN] uat-client: refresh failed for %s, token cleared: %v\n", + opts.UserOpenId, result.err) + return nil, result.err + } - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("token refresh read error: %v", err) + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "token refresh exhausted attempts without a result") +} + +func refreshOnce(httpClient *http.Client, endpoint string, opts UATCallOptions, stored *StoredUAToken) refreshResult { + payload, err := json.Marshal(refreshRequest{ + GrantType: "refresh_token", + RefreshToken: stored.RefreshToken, + ClientID: opts.AppId, + ClientSecret: opts.AppSecret, + }) + if err != nil { + return refreshResult{ + action: refreshStopAndPreserve, + err: errs.NewInternalError(errs.SubtypeSDKError, + "failed to encode token refresh request: %v", err). + WithCause(err), } - var data map[string]interface{} - if err := json.Unmarshal(body, &data); err != nil { - return nil, fmt.Errorf("token refresh parse error: %w", err) + } + + var wroteRequest atomic.Bool + trace := &httptrace.ClientTrace{ + WroteRequest: func(httptrace.WroteRequestInfo) { + wroteRequest.Store(true) + }, + } + ctx := httptrace.WithClientTrace(context.Background(), trace) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(payload)) + if err != nil { + return refreshResult{ + action: refreshStopAndPreserve, + err: errs.NewInternalError(errs.SubtypeSDKError, + "failed to create token refresh request: %v", err). + WithCause(err), } - return data, nil } + req.Header.Set("Content-Type", "application/json; charset=utf-8") - data, err := callEndpoint() + resp, err := httpClient.Do(req) if err != nil { - return nil, err + action := refreshRetryAndPreserve + if wroteRequest.Load() { + action = refreshRetryAndClear + } + return refreshResult{action: action, err: err} } + defer resp.Body.Close() + logHTTPResponse(resp) - code := getInt(data, "code", -1) - meta, metaOK := errclass.LookupCodeMeta(code) - if metaOK && meta.Category == errs.CategoryPolicy { - challengeUrl := getStr(data, "challenge_url") - cliHint := getStr(data, "cli_hint") - msg := getStr(data, "error_description") - - return nil, &errs.SecurityPolicyError{ - Problem: errs.Problem{ - Category: errs.CategoryPolicy, - Subtype: meta.Subtype, - Code: code, - Message: msg, - Hint: cliHint, - }, - ChallengeURL: challengeUrl, + body, err := io.ReadAll(resp.Body) + if err != nil { + return refreshResult{ + action: refreshRetryAndClear, + err: errs.NewNetworkError(errs.SubtypeNetworkTransport, + "token refresh response read failed: %v", err). + WithRetryable(). + WithCause(err), } } - errStr := getStr(data, "error") - - if (code != -1 && code != 0) || errStr != "" { - // Retryable server error: retry once, then clear token on second failure. - if metaOK && meta.Category == errs.CategoryAuthentication && meta.Retryable { - fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: refresh transient error (code=%d) for %s, retrying once\n", code, opts.UserOpenId) - data, err = callEndpoint() - if err != nil { - fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: refresh retry network error for %s, clearing token\n", opts.UserOpenId) - if err := RemoveStoredToken(opts.AppId, opts.UserOpenId); err != nil { - fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: failed to remove token: %v\n", err) - } - return nil, nil - } - code = getInt(data, "code", -1) - errStr = getStr(data, "error") - if (code != -1 && code != 0) || errStr != "" { - fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: refresh failed after retry (code=%d) for %s, clearing token\n", code, opts.UserOpenId) - if err := RemoveStoredToken(opts.AppId, opts.UserOpenId); err != nil { - fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: failed to remove token: %v\n", err) - } - return nil, nil + var parsed refreshResponse + if err := json.Unmarshal(body, &parsed); err != nil { + return refreshResult{ + action: refreshRetryAndClear, + err: errs.NewInternalError(errs.SubtypeInvalidResponse, + "token refresh returned invalid JSON: %v", err). + WithRetryable(). + WithCause(err), + } + } + if parsed.Code == nil { + return refreshResult{ + action: refreshRetryAndClear, + err: errs.NewInternalError(errs.SubtypeInvalidResponse, + "token refresh response is missing required field code"). + WithRetryable(), + } + } + + code := *parsed.Code + if code != 0 { + if meta, ok := errclass.LookupCodeMeta(code); ok && meta.Category == errs.CategoryPolicy { + var policyFields struct { + ChallengeURL string `json:"challenge_url"` + CLIHint string `json:"cli_hint"` } - // Retry succeeded, fall through to parse token below. - } else { - // All other errors: clear token, require re-authorization. - fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: refresh failed (code=%d), clearing token for %s\n", code, opts.UserOpenId) - if err := RemoveStoredToken(opts.AppId, opts.UserOpenId); err != nil { - fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: failed to remove token: %v\n", err) + _ = json.Unmarshal(body, &policyFields) + return refreshResult{ + action: refreshStopAndPreserve, + err: &errs.SecurityPolicyError{ + Problem: errs.Problem{ + Category: errs.CategoryPolicy, + Subtype: meta.Subtype, + Code: code, + Message: parsed.ErrorDescription, + Hint: policyFields.CLIHint, + }, + ChallengeURL: policyFields.ChallengeURL, + }, } - return nil, nil } + + message := parsed.ErrorDescription + if message == "" { + message = parsed.Error + } + // BuildAPIError accepts the common OpenAPI message key; OAuth names + // the same value error_description. + apiErr := errclass.BuildAPIError(map[string]any{ + "code": code, + "msg": message, + }, errclass.ClassifyContext{ + Brand: string(opts.Domain), + AppID: opts.AppId, + Identity: "user", + }) + if authErr, ok := apiErr.(*errs.AuthenticationError); ok { + authErr.UserOpenID = opts.UserOpenId + } + return refreshResult{action: refreshActionForCode(code), err: apiErr} } - accessToken := getStr(data, "access_token") - if accessToken == "" { - return nil, fmt.Errorf("Token refresh returned no access_token") + if parsed.RefreshToken == "" { + parsed.RefreshToken = stored.RefreshToken } - refreshToken := getStr(data, "refresh_token") - if refreshToken == "" { - refreshToken = stored.RefreshToken + if parsed.AccessToken == "" { + return refreshResult{ + action: refreshStopAndPreserve, + err: errs.NewInternalError(errs.SubtypeInvalidResponse, + "token refresh response is missing required field access_token"). + WithRetryable(), + } } - expiresIn := getInt(data, "expires_in", 7200) - refreshExpiresIn := getInt(data, "refresh_token_expires_in", 0) - refreshExpiresAt := stored.RefreshExpiresAt - if refreshExpiresIn > 0 { - refreshExpiresAt = now + int64(refreshExpiresIn)*1000 + if parsed.ExpiresIn == nil || *parsed.ExpiresIn <= 0 { + parsed.ExpiresIn = new(int64) + *parsed.ExpiresIn = 7200 // 2 hours } - scope := getStr(data, "scope") - if scope == "" { - scope = stored.Scope + if parsed.RefreshTokenExpiresIn == nil || *parsed.RefreshTokenExpiresIn <= 0 { + parsed.RefreshTokenExpiresIn = new(int64) + if stored.RefreshExpiresAt <= 0 { + *parsed.RefreshTokenExpiresIn = 2592000 // 30 days + } else { + now := time.Now().UnixMilli() + *parsed.RefreshTokenExpiresIn = (stored.RefreshExpiresAt - now) / 1000 + } } + return refreshResult{action: refreshSaveResponse, response: parsed} +} + +func refreshActionForCode(code int) refreshAction { + meta, ok := errclass.LookupCodeMeta(code) + switch { + case !ok: + return refreshRetryAndClear + case meta.Category == errs.CategoryPolicy: + return refreshStopAndPreserve + case meta.Retryable: + return refreshRetryAndPreserve + default: + return refreshStopAndClear + } +} + +func saveRefreshResponse(opts UATCallOptions, stored *StoredUAToken, response refreshResponse) (*StoredUAToken, error) { + now := time.Now().UnixMilli() + updated := &StoredUAToken{ UserOpenId: stored.UserOpenId, AppId: opts.AppId, - AccessToken: accessToken, - RefreshToken: refreshToken, - ExpiresAt: now + int64(expiresIn)*1000, - RefreshExpiresAt: refreshExpiresAt, - Scope: scope, + AccessToken: response.AccessToken, + RefreshToken: response.RefreshToken, + ExpiresAt: now + *response.ExpiresIn*1000, + RefreshExpiresAt: now + *response.RefreshTokenExpiresIn*1000, + Scope: response.Scope, GrantedAt: stored.GrantedAt, } - if err := SetStoredToken(updated); err != nil { - return nil, err + if _, ok := errs.ProblemOf(err); ok { + return nil, err + } + return nil, errs.NewInternalError(errs.SubtypeStorage, + "failed to store refreshed user token for user %q: %v", opts.UserOpenId, err). + WithCause(err) } return updated, nil } diff --git a/internal/errclass/codemeta.go b/internal/errclass/codemeta.go index d0b1d76441..f35ba87fbc 100644 --- a/internal/errclass/codemeta.go +++ b/internal/errclass/codemeta.go @@ -38,11 +38,13 @@ var codeMeta = map[int]CodeMeta{ 99991668: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeTokenInvalid}, // UAT invalid/expired (server does not distinguish) 99991663: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeTokenInvalid}, // access_token invalid 99991677: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeTokenExpired}, // UAT expired - 20026: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeRefreshTokenInvalid}, // refresh_token v1 legacy format + 20024: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeRefreshTokenInvalid}, // authorization code or refresh_token does not match client_id + 20026: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeRefreshTokenInvalid}, // refresh_token is invalid or v1 legacy format 20037: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeRefreshTokenExpired}, // refresh_token expired + 20050: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeRefreshServerError, Retryable: true}, // refresh endpoint transient error 20064: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeRefreshTokenRevoked}, // refresh_token revoked + 20072: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeRefreshServerError}, // refresh endpoint temporarily unavailable 20073: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeRefreshTokenReused}, // refresh_token already used - 20050: {Category: errs.CategoryAuthentication, Subtype: errs.SubtypeRefreshServerError, Retryable: true}, // refresh endpoint transient error // CategoryAuthorization 99991672: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeAppScopeNotApplied}, @@ -51,6 +53,13 @@ var codeMeta = map[int]CodeMeta{ 230027: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeUserUnauthorized}, // user never authorized the app 99991673: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeAppUnavailable}, // app status unavailable 99991662: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeAppDisabled}, // app currently disabled in tenant + 20008: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeUserUnauthorized}, // user does not exist + 20009: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeAppUnavailable}, // app specified is not installed + 20010: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeUserUnauthorized}, // user does not have permission to use this app + 20048: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeAppUnavailable}, // app specified is not exist + 20066: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeUserUnauthorized}, // user staus is not normal + 20069: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeAppDisabled}, // app specified is disabled + 20074: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypeAppUnavailable}, // app specified not allows for refresh token // CategoryAPI 99991400: {Category: errs.CategoryAPI, Subtype: errs.SubtypeRateLimit, Retryable: true}, @@ -62,10 +71,17 @@ var codeMeta = map[int]CodeMeta{ 1063006: {Category: errs.CategoryAPI, Subtype: errs.SubtypeRateLimit}, // drive perm-apply quota; 5/day, not short-term retryable 1063007: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, 231205: {Category: errs.CategoryAPI, Subtype: errs.SubtypeOwnershipMismatch}, + 20001: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // request missing required parameter + 20036: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // grant_type not supported + 20063: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // request format error + 20067: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // scope list contains duplicated items + 20068: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // scope list contains forbidden permissions + 20070: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // request provide multiple authorization methods // CategoryConfig 99991543: {Category: errs.CategoryConfig, Subtype: errs.SubtypeInvalidClient}, // RFC 6749 §5.2 — app_id / app_secret incorrect (Open API) 10014: {Category: errs.CategoryConfig, Subtype: errs.SubtypeInvalidClient}, // legacy TAT endpoint — "app secret invalid" (pre-v3 variant of 99991543; CLI now reports invalid_client) + 20002: {Category: errs.CategoryConfig, Subtype: errs.SubtypeInvalidClient}, // client secret invalid // CategoryPolicy 21000: {Category: errs.CategoryPolicy, Subtype: errs.SubtypeChallengeRequired}, diff --git a/internal/errclass/codemeta_test.go b/internal/errclass/codemeta_test.go index 0d766f3de6..e419439157 100644 --- a/internal/errclass/codemeta_test.go +++ b/internal/errclass/codemeta_test.go @@ -23,11 +23,27 @@ func TestLookupCodeMeta_CredentialCodes(t *testing.T) { {99991668, errs.CategoryAuthentication, errs.SubtypeTokenInvalid, false}, {99991663, errs.CategoryAuthentication, errs.SubtypeTokenInvalid, false}, {99991677, errs.CategoryAuthentication, errs.SubtypeTokenExpired, false}, + {20024, errs.CategoryAuthentication, errs.SubtypeRefreshTokenInvalid, false}, {20026, errs.CategoryAuthentication, errs.SubtypeRefreshTokenInvalid, false}, {20037, errs.CategoryAuthentication, errs.SubtypeRefreshTokenExpired, false}, + {20050, errs.CategoryAuthentication, errs.SubtypeRefreshServerError, true}, {20064, errs.CategoryAuthentication, errs.SubtypeRefreshTokenRevoked, false}, + {20072, errs.CategoryAuthentication, errs.SubtypeRefreshServerError, false}, {20073, errs.CategoryAuthentication, errs.SubtypeRefreshTokenReused, false}, - {20050, errs.CategoryAuthentication, errs.SubtypeRefreshServerError, true}, + {20008, errs.CategoryAuthorization, errs.SubtypeUserUnauthorized, false}, + {20009, errs.CategoryAuthorization, errs.SubtypeAppUnavailable, false}, + {20010, errs.CategoryAuthorization, errs.SubtypeUserUnauthorized, false}, + {20048, errs.CategoryAuthorization, errs.SubtypeAppUnavailable, false}, + {20066, errs.CategoryAuthorization, errs.SubtypeUserUnauthorized, false}, + {20069, errs.CategoryAuthorization, errs.SubtypeAppDisabled, false}, + {20074, errs.CategoryAuthorization, errs.SubtypeAppUnavailable, false}, + {20001, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + {20036, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + {20063, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + {20067, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + {20068, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + {20070, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + {20002, errs.CategoryConfig, errs.SubtypeInvalidClient, false}, } for _, tc := range cases { t.Run(fmt.Sprintf("%d", tc.code), func(t *testing.T) { From eedaea24085f9da396de387301e1a59c07ed7b47 Mon Sep 17 00:00:00 2001 From: kiraWangRuilong Date: Fri, 31 Jul 2026 18:01:28 +0800 Subject: [PATCH 2/4] feat: add file lock directory writable checking logic --- internal/auth/token_store.go | 64 ++++++++++++++- internal/auth/uat_client.go | 152 +++++++++++++++++++++++++++-------- 2 files changed, 179 insertions(+), 37 deletions(-) diff --git a/internal/auth/token_store.go b/internal/auth/token_store.go index bbf4b98758..687d0524b6 100644 --- a/internal/auth/token_store.go +++ b/internal/auth/token_store.go @@ -40,15 +40,23 @@ func MaskToken(token string) string { // GetStoredToken reads the stored UAT for a given (appId, userOpenId) pair. func GetStoredToken(appId, userOpenId string) *StoredUAToken { + token, _ := readStoredToken(appId, userOpenId) + return token +} + +func readStoredToken(appId, userOpenId string) (*StoredUAToken, error) { jsonStr, err := keychain.Get(keychain.LarkCliService, accountKey(appId, userOpenId)) - if err != nil || jsonStr == "" { - return nil + if err != nil { + return nil, err + } + if jsonStr == "" { + return nil, nil } var token StoredUAToken if err := json.Unmarshal([]byte(jsonStr), &token); err != nil { - return nil + return nil, err } - return &token + return &token, nil } // SetStoredToken persists a UAT. @@ -66,6 +74,54 @@ func RemoveStoredToken(appId, userOpenId string) error { return keychain.Remove(keychain.LarkCliService, accountKey(appId, userOpenId)) } +// sameStoredTokenGeneration reports whether two snapshots represent the same +// refresh-token generation. Access tokens are used only for case that does not +// contain a refresh token. +func isSameStoredTokenGeneration(current, expected *StoredUAToken) bool { + if current == nil || expected == nil || + current.AppId != expected.AppId || + current.UserOpenId != expected.UserOpenId { + return false + } + if current.RefreshToken != "" || expected.RefreshToken != "" { + return current.RefreshToken == expected.RefreshToken + } + return current.AccessToken == expected.AccessToken +} + +// setStoredTokenIfCurrent stores updated only when expected is still the +// current token generation. It returns the token present after the check and +// whether the update was applied. +func setStoredTokenIfCurrent(expected, updated *StoredUAToken) (*StoredUAToken, bool, error) { + current, err := readStoredToken(expected.AppId, expected.UserOpenId) + if err != nil { + return nil, false, err + } + if !isSameStoredTokenGeneration(current, expected) { + return current, false, nil + } + if err := SetStoredToken(updated); err != nil { + return current, false, err + } + return updated, true, nil +} + +// removeStoredTokenIfCurrent removes expected only when it is still the +// current token generation. It returns the token retained on a mismatch. +func removeStoredTokenIfCurrent(expected *StoredUAToken) (*StoredUAToken, bool, error) { + current, err := readStoredToken(expected.AppId, expected.UserOpenId) + if err != nil { + return nil, false, err + } + if !isSameStoredTokenGeneration(current, expected) { + return current, false, nil + } + if err := RemoveStoredToken(expected.AppId, expected.UserOpenId); err != nil { + return current, false, err + } + return nil, true, nil +} + // TokenStatus determines the freshness of a stored token. func TokenStatus(token *StoredUAToken) string { now := time.Now().UnixMilli() diff --git a/internal/auth/uat_client.go b/internal/auth/uat_client.go index 1f08789d10..4e2ae86ced 100644 --- a/internal/auth/uat_client.go +++ b/internal/auth/uat_client.go @@ -82,7 +82,7 @@ func GetValidAccessToken(httpClient *http.Client, opts UATCallOptions) (string, } if status == "needs_refresh" { - refreshed, err := refreshWithLock(httpClient, opts, stored) + refreshed, err := refreshWithLock(httpClient, opts) if err != nil { return "", err } @@ -104,7 +104,7 @@ func GetValidAccessToken(httpClient *http.Client, opts UATCallOptions) (string, } // refreshWithLock acquires a file lock before attempting to refresh the token. -func refreshWithLock(httpClient *http.Client, opts UATCallOptions, stored *StoredUAToken) (*StoredUAToken, error) { +func refreshWithLock(httpClient *http.Client, opts UATCallOptions) (*StoredUAToken, error) { key := fmt.Sprintf("%s:%s", opts.AppId, opts.UserOpenId) // 1. Process-level lock (prevents multiple goroutines in the same process) @@ -126,12 +126,9 @@ func refreshWithLock(httpClient *http.Client, opts UATCallOptions, stored *Store refreshLocks.Delete(key) }() - // 2. Cross-process lock using flock - // We use the same underlying storage directory resolution as keychain_other.go - // to ensure locks are isolated properly alongside other sensitive data. - configDir := core.GetConfigDir() - - lockDir := filepath.Join(configDir, "locks") + // 2. Cross-process lock using the global config directory so all + // workspaces sharing the same token also share the same lock. + lockDir := filepath.Join(core.GetBaseConfigDir(), "locks") if err := vfs.MkdirAll(lockDir, 0700); err != nil { return nil, fmt.Errorf("failed to create lock directory: %w", err) } @@ -154,21 +151,46 @@ func refreshWithLock(httpClient *http.Client, opts UATCallOptions, stored *Store } defer fileLock.Unlock() - // 3. Double-checked locking: Check if another process has already refreshed the token - freshStored := GetStoredToken(opts.AppId, opts.UserOpenId) - if freshStored != nil { - status := TokenStatus(freshStored) - if status == "valid" { - // Another process refreshed it, we can just use the new token - if opts.ErrOut != nil { - fmt.Fprintf(opts.ErrOut, "[lark-cli] uat-client: token already refreshed by another process\n") - } - return freshStored, nil + // 3. Re-read under the global lock and use only the current generation. + freshStored, err := readStoredToken(opts.AppId, opts.UserOpenId) + if err != nil { + return nil, err + } + if freshStored == nil { + return nil, nil + } + + switch TokenStatus(freshStored) { + case "valid": + if opts.ErrOut != nil { + fmt.Fprintf(opts.ErrOut, "[lark-cli] uat-client: token already refreshed by another process\n") + } + return freshStored, nil + case "expired": + retained, removed, err := removeStoredTokenIfCurrent(freshStored) + if err != nil { + return nil, err + } + if !removed { + return storedTokenAfterGenerationChange(retained, opts.UserOpenId) + } + if opts.ErrOut != nil { + fmt.Fprintf(opts.ErrOut, "[lark-cli] uat-client: refresh_token expired for %s, clearing\n", opts.UserOpenId) } + return nil, nil + } + + if err := ensureDirWritable(lockDir, "tmp_writetest-*"); err != nil { + if opts.ErrOut != nil { + fmt.Fprintf(opts.ErrOut, + "[lark-cli] [WARN] uat-client: refresh lock directory is not writable while refreshing: %v\n", + err) + } + return nil, err } // 4. Actually perform the refresh - return doRefreshToken(httpClient, opts, stored) + return doRefreshToken(httpClient, opts, freshStored) } const refreshMaxAttempts = 2 @@ -225,8 +247,13 @@ func doRefreshToken(httpClient *http.Client, opts UATCallOptions, stored *Stored if time.Now().UnixMilli() >= stored.RefreshExpiresAt { fmt.Fprintf(errOut, "[lark-cli] uat-client: refresh_token expired for %s, clearing\n", opts.UserOpenId) - if err := RemoveStoredToken(opts.AppId, opts.UserOpenId); err != nil { + retained, removed, err := removeStoredTokenIfCurrent(stored) + if err != nil { fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: failed to remove expired token: %v\n", err) + return nil, err + } + if !removed { + return storedTokenAfterGenerationChange(retained, opts.UserOpenId) } return nil, nil } @@ -269,14 +296,16 @@ func doRefreshToken(httpClient *http.Client, opts UATCallOptions, stored *Stored if problem, ok := errs.ProblemOf(result.err); ok { problem.Retryable = false } - if err := RemoveStoredToken(opts.AppId, opts.UserOpenId); err != nil { + retained, removed, err := removeStoredTokenIfCurrent(stored) + if err != nil { fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: failed to remove token: %v\n", err) - if _, ok := errs.ProblemOf(err); ok { - return nil, err - } - return nil, errs.NewInternalError(errs.SubtypeStorage, - "failed to remove refreshed user token for user %q: %v", opts.UserOpenId, err). - WithCause(err) + return nil, err + } + if !removed { + fmt.Fprintf(errOut, + "[lark-cli] [WARN] uat-client: stored token changed during refresh for %s, preserving current token\n", + opts.UserOpenId) + return storedTokenAfterGenerationChange(retained, opts.UserOpenId) } fmt.Fprintf(errOut, "[lark-cli] [WARN] uat-client: refresh failed for %s, token cleared: %v\n", @@ -464,13 +493,70 @@ func saveRefreshResponse(opts UATCallOptions, stored *StoredUAToken, response re Scope: response.Scope, GrantedAt: stored.GrantedAt, } - if err := SetStoredToken(updated); err != nil { - if _, ok := errs.ProblemOf(err); ok { - return nil, err + current, saved, err := setStoredTokenIfCurrent(stored, updated) + if err != nil { + return nil, err + } + if !saved { + if opts.ErrOut != nil { + fmt.Fprintf(opts.ErrOut, + "[lark-cli] [WARN] uat-client: stored token changed during refresh for %s, preserving current token\n", + opts.UserOpenId) } - return nil, errs.NewInternalError(errs.SubtypeStorage, - "failed to store refreshed user token for user %q: %v", opts.UserOpenId, err). - WithCause(err) + return storedTokenAfterGenerationChange(current, opts.UserOpenId) } return updated, nil } + +func storedTokenAfterGenerationChange(current *StoredUAToken, userOpenId string) (*StoredUAToken, error) { + if current == nil { + return nil, nil + } + if TokenStatus(current) == "valid" { + return current, nil + } + return nil, errs.NewInternalError(errs.SubtypeStorage, + "stored refresh token changed while refreshing user %q", userOpenId). + WithRetryable(). + WithHint("retry the command") +} + +func ensureDirWritable(dir, tempPrefix string) error { + if dir == "" { + return nil + } + + if err := vfs.MkdirAll(dir, 0700); err != nil { + return errs.NewInternalError(errs.SubtypeFileIO, + "failed to access refresh lock directory %q", dir). + WithCause(err). + WithHint("If running in a sandbox or read-only workspace, grant write access for this directory and retry.") + } + + tmp, err := vfs.CreateTemp(dir, tempPrefix) + if err != nil { + return errs.NewInternalError(errs.SubtypeFileIO, + "failed to create temporary file in refresh lock directory %q", dir). + WithCause(err). + WithHint("If running in a sandbox or read-only workspace, grant write access for this directory and retry.") + } + + tmpName := tmp.Name() + closeErr := tmp.Close() + if removeErr := vfs.Remove(tmpName); removeErr != nil { + err := fmt.Errorf("%v", removeErr) + if closeErr != nil { + err = fmt.Errorf("%v; also failed to close temp file: %v", removeErr, closeErr) + } + return errs.NewInternalError(errs.SubtypeFileIO, + "failed to clean up refresh lock write-check file %q", tmpName). + WithCause(err) + } + if closeErr != nil { + return errs.NewInternalError(errs.SubtypeFileIO, + "failed to close refresh lock write-check file %q", tmpName). + WithCause(closeErr) + } + + return nil +} From 8af089a8878ebe7d278725893e0fd082ffe7ad02 Mon Sep 17 00:00:00 2001 From: kiraWangRuilong Date: Mon, 3 Aug 2026 11:27:57 +0800 Subject: [PATCH 3/4] feat: add token storage writable check before refresh token logic --- internal/auth/uat_client.go | 39 ++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/internal/auth/uat_client.go b/internal/auth/uat_client.go index 4e2ae86ced..dd272e5cbf 100644 --- a/internal/auth/uat_client.go +++ b/internal/auth/uat_client.go @@ -189,6 +189,15 @@ func refreshWithLock(httpClient *http.Client, opts UATCallOptions) (*StoredUATok return nil, err } + if err := ensureTokenStorageWritable(opts.AppId, opts.UserOpenId); err != nil { + if opts.ErrOut != nil { + fmt.Fprintf(opts.ErrOut, + "[lark-cli] [WARN] uat-client: token storage is not writable while refreshing: %v\n", + err) + } + return nil, err + } + // 4. Actually perform the refresh return doRefreshToken(httpClient, opts, freshStored) } @@ -482,6 +491,10 @@ func refreshActionForCode(code int) refreshAction { func saveRefreshResponse(opts UATCallOptions, stored *StoredUAToken, response refreshResponse) (*StoredUAToken, error) { now := time.Now().UnixMilli() + scope := response.Scope + if scope == "" { + scope = stored.Scope + } updated := &StoredUAToken{ UserOpenId: stored.UserOpenId, @@ -490,7 +503,7 @@ func saveRefreshResponse(opts UATCallOptions, stored *StoredUAToken, response re RefreshToken: response.RefreshToken, ExpiresAt: now + *response.ExpiresIn*1000, RefreshExpiresAt: now + *response.RefreshTokenExpiresIn*1000, - Scope: response.Scope, + Scope: scope, GrantedAt: stored.GrantedAt, } current, saved, err := setStoredTokenIfCurrent(stored, updated) @@ -560,3 +573,27 @@ func ensureDirWritable(dir, tempPrefix string) error { return nil } + +func ensureTokenStorageWritable(appID, userOpenID string) error { + if appID == "" || userOpenID == "" { + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "cannot validate refresh token storage without user identity"). + WithParam("app-id/user-open-id") + } + + probeUserOpenID := fmt.Sprintf("%s:%s:%d", appID, userOpenID, time.Now().UnixNano()) + probeToken := &StoredUAToken{ + AppId: appID, + UserOpenId: probeUserOpenID, + AccessToken: "refresh-storage-probe", + Scope: "", + } + + if err := SetStoredToken(probeToken); err != nil { + return err + } + if err := RemoveStoredToken(appID, probeUserOpenID); err != nil { + return err + } + return nil +} From 5290b584d4711c4192a03d7574b90cefff360b3f Mon Sep 17 00:00:00 2001 From: kiraWangRuilong Date: Mon, 3 Aug 2026 11:54:42 +0800 Subject: [PATCH 4/4] fix: ci test --- internal/auth/token_store.go | 4 ++-- internal/auth/uat_client.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/auth/token_store.go b/internal/auth/token_store.go index 687d0524b6..0a99e04375 100644 --- a/internal/auth/token_store.go +++ b/internal/auth/token_store.go @@ -74,8 +74,8 @@ func RemoveStoredToken(appId, userOpenId string) error { return keychain.Remove(keychain.LarkCliService, accountKey(appId, userOpenId)) } -// sameStoredTokenGeneration reports whether two snapshots represent the same -// refresh-token generation. Access tokens are used only for case that does not +// isSameStoredTokenGeneration reports whether two snapshots represent the same +// refresh-token generation. Access tokens are used only for case that does not // contain a refresh token. func isSameStoredTokenGeneration(current, expected *StoredUAToken) bool { if current == nil || expected == nil || diff --git a/internal/auth/uat_client.go b/internal/auth/uat_client.go index dd272e5cbf..3e721d79b3 100644 --- a/internal/auth/uat_client.go +++ b/internal/auth/uat_client.go @@ -197,7 +197,7 @@ func refreshWithLock(httpClient *http.Client, opts UATCallOptions) (*StoredUATok } return nil, err } - + // 4. Actually perform the refresh return doRefreshToken(httpClient, opts, freshStored) } @@ -581,7 +581,7 @@ func ensureTokenStorageWritable(appID, userOpenID string) error { WithParam("app-id/user-open-id") } - probeUserOpenID := fmt.Sprintf("%s:%s:%d", appID, userOpenID, time.Now().UnixNano()) + probeUserOpenID := fmt.Sprintf("%s:%s:refresh-storage-probe", appID, userOpenID) probeToken := &StoredUAToken{ AppId: appID, UserOpenId: probeUserOpenID,