From 053134f66ec8f97d730adf581d5b6d1b80ffb5a2 Mon Sep 17 00:00:00 2001 From: Luis Pater Date: Mon, 22 Sep 2025 21:11:53 +0800 Subject: [PATCH] refactor(auth): remove unused `Refresh` methods from authenticators - Deleted `Refresh` implementations in Codex, Claude, Gemini, Qwen, and Gemini-web authenticators. - Updated the `Authenticator` interface to exclude `Refresh` for cleaner design. - Revised `Manager` and related components to handle refresh logic improvements. - Simplified token refresh behavior and eliminated redundant code paths. --- .../runtime/executor/gemini_cli_executor.go | 3 +- sdk/auth/claude.go | 33 ----------------- sdk/auth/codex.go | 32 ----------------- sdk/auth/gemini-web.go | 7 ---- sdk/auth/gemini.go | 4 --- sdk/auth/interfaces.go | 1 - sdk/auth/manager.go | 26 -------------- sdk/auth/qwen.go | 35 ------------------- sdk/cliproxy/auth/manager.go | 34 +++++++++++------- sdk/cliproxy/auth/types.go | 8 ++--- 10 files changed, 27 insertions(+), 156 deletions(-) diff --git a/internal/runtime/executor/gemini_cli_executor.go b/internal/runtime/executor/gemini_cli_executor.go index 8568169d..ff850b0d 100644 --- a/internal/runtime/executor/gemini_cli_executor.go +++ b/internal/runtime/executor/gemini_cli_executor.go @@ -15,6 +15,7 @@ import ( cliproxyauth "github.com/router-for-me/CLIProxyAPI/v6/sdk/cliproxy/auth" cliproxyexecutor "github.com/router-for-me/CLIProxyAPI/v6/sdk/cliproxy/executor" sdktranslator "github.com/router-for-me/CLIProxyAPI/v6/sdk/translator" + log "github.com/sirupsen/logrus" "github.com/tidwall/sjson" "golang.org/x/oauth2" "golang.org/x/oauth2/google" @@ -255,7 +256,7 @@ func (e *GeminiCLIExecutor) ExecuteStream(ctx context.Context, auth *cliproxyaut } func (e *GeminiCLIExecutor) Refresh(ctx context.Context, auth *cliproxyauth.Auth) (*cliproxyauth.Auth, error) { - // log.Debugf("gemini cli executor: refresh called") + log.Debugf("gemini cli executor: refresh called") _ = ctx return auth, nil } diff --git a/sdk/auth/claude.go b/sdk/auth/claude.go index 1dbeafdb..1856d61f 100644 --- a/sdk/auth/claude.go +++ b/sdk/auth/claude.go @@ -143,36 +143,3 @@ func (a *ClaudeAuthenticator) Login(ctx context.Context, cfg *config.Config, opt Metadata: metadata, }, nil } - -func (a *ClaudeAuthenticator) Refresh(ctx context.Context, cfg *config.Config, record *TokenRecord) (*TokenRecord, error) { - if record == nil || record.Storage == nil { - return nil, fmt.Errorf("cliproxy auth: empty token record for claude refresh") - } - if cfg == nil { - return nil, fmt.Errorf("cliproxy auth: configuration is required") - } - if ctx == nil { - ctx = context.Background() - } - - storage, ok := record.Storage.(*claude.ClaudeTokenStorage) - if !ok { - return nil, fmt.Errorf("cliproxy auth: unexpected token storage type for claude refresh") - } - - // Refresh via auth service directly (no legacy client) - svc := claude.NewClaudeAuth(cfg) - td, err := svc.RefreshTokensWithRetry(ctx, storage.RefreshToken, 3) - if err != nil { - return nil, err - } - svc.UpdateTokenStorage(storage, td) - - result := &TokenRecord{ - Provider: a.Provider(), - FileName: record.FileName, - Storage: storage, - Metadata: record.Metadata, - } - return result, nil -} diff --git a/sdk/auth/codex.go b/sdk/auth/codex.go index e60b85a5..c95a7705 100644 --- a/sdk/auth/codex.go +++ b/sdk/auth/codex.go @@ -142,35 +142,3 @@ func (a *CodexAuthenticator) Login(ctx context.Context, cfg *config.Config, opts Metadata: metadata, }, nil } - -func (a *CodexAuthenticator) Refresh(ctx context.Context, cfg *config.Config, record *TokenRecord) (*TokenRecord, error) { - if record == nil || record.Storage == nil { - return nil, fmt.Errorf("cliproxy auth: empty token record for codex refresh") - } - if cfg == nil { - return nil, fmt.Errorf("cliproxy auth: configuration is required") - } - if ctx == nil { - ctx = context.Background() - } - - storage, ok := record.Storage.(*codex.CodexTokenStorage) - if !ok { - return nil, fmt.Errorf("cliproxy auth: unexpected token storage type for codex refresh") - } - - svc := codex.NewCodexAuth(cfg) - td, err := svc.RefreshTokensWithRetry(ctx, storage.RefreshToken, 3) - if err != nil { - return nil, err - } - svc.UpdateTokenStorage(storage, td) - - result := &TokenRecord{ - Provider: a.Provider(), - FileName: record.FileName, - Storage: storage, - Metadata: record.Metadata, - } - return result, nil -} diff --git a/sdk/auth/gemini-web.go b/sdk/auth/gemini-web.go index fb766f88..bb5c24f4 100644 --- a/sdk/auth/gemini-web.go +++ b/sdk/auth/gemini-web.go @@ -23,13 +23,6 @@ func (a *GeminiWebAuthenticator) Login(ctx context.Context, cfg *config.Config, return nil, fmt.Errorf("gemini-web authenticator does not support scripted login; use CLI --gemini-web-auth") } -func (a *GeminiWebAuthenticator) Refresh(ctx context.Context, cfg *config.Config, record *TokenRecord) (*TokenRecord, error) { - _ = ctx - _ = cfg - _ = record - return nil, ErrRefreshNotSupported -} - func (a *GeminiWebAuthenticator) RefreshLead() *time.Duration { d := 9 * time.Minute return &d diff --git a/sdk/auth/gemini.go b/sdk/auth/gemini.go index 4f6b71a5..d080d20e 100644 --- a/sdk/auth/gemini.go +++ b/sdk/auth/gemini.go @@ -66,7 +66,3 @@ func (a *GeminiAuthenticator) Login(ctx context.Context, cfg *config.Config, opt Metadata: metadata, }, nil } - -func (a *GeminiAuthenticator) Refresh(ctx context.Context, cfg *config.Config, record *TokenRecord) (*TokenRecord, error) { - return nil, ErrRefreshNotSupported -} diff --git a/sdk/auth/interfaces.go b/sdk/auth/interfaces.go index f6e5051c..7e6a268e 100644 --- a/sdk/auth/interfaces.go +++ b/sdk/auth/interfaces.go @@ -37,6 +37,5 @@ type TokenStore interface { type Authenticator interface { Provider() string Login(ctx context.Context, cfg *config.Config, opts *LoginOptions) (*TokenRecord, error) - Refresh(ctx context.Context, cfg *config.Config, record *TokenRecord) (*TokenRecord, error) RefreshLead() *time.Duration } diff --git a/sdk/auth/manager.go b/sdk/auth/manager.go index 951ec37f..2e7e39b6 100644 --- a/sdk/auth/manager.go +++ b/sdk/auth/manager.go @@ -67,29 +67,3 @@ func (m *Manager) Login(ctx context.Context, provider string, cfg *config.Config } return record, savedPath, nil } - -// Refresh delegates to the provider-specific refresh implementation and persists the result. -func (m *Manager) Refresh(ctx context.Context, provider string, cfg *config.Config, record *TokenRecord) (*TokenRecord, string, error) { - auth, ok := m.authenticators[provider] - if !ok { - return nil, "", fmt.Errorf("cliproxy auth: authenticator %s not registered", provider) - } - - updated, err := auth.Refresh(ctx, cfg, record) - if err != nil { - return nil, "", err - } - if updated == nil { - updated = record - } - - if m.store == nil { - return updated, "", nil - } - - savedPath, err := m.store.Save(ctx, cfg, updated) - if err != nil { - return updated, "", err - } - return updated, savedPath, nil -} diff --git a/sdk/auth/qwen.go b/sdk/auth/qwen.go index 41362b69..7d9ab828 100644 --- a/sdk/auth/qwen.go +++ b/sdk/auth/qwen.go @@ -110,38 +110,3 @@ func (a *QwenAuthenticator) Login(ctx context.Context, cfg *config.Config, opts Metadata: metadata, }, nil } - -func (a *QwenAuthenticator) Refresh(ctx context.Context, cfg *config.Config, record *TokenRecord) (*TokenRecord, error) { - if record == nil || record.Storage == nil { - return nil, fmt.Errorf("cliproxy auth: empty token record for qwen refresh") - } - if cfg == nil { - return nil, fmt.Errorf("cliproxy auth: configuration is required") - } - if ctx == nil { - ctx = context.Background() - } - - storage, ok := record.Storage.(*qwen.QwenTokenStorage) - if !ok { - return nil, fmt.Errorf("cliproxy auth: unexpected token storage type for qwen refresh") - } - - svc := qwen.NewQwenAuth(cfg) - td, err := svc.RefreshTokens(ctx, storage.RefreshToken) - if err != nil { - return nil, err - } - storage.AccessToken = td.AccessToken - storage.RefreshToken = td.RefreshToken - storage.ResourceURL = td.ResourceURL - storage.Expire = td.Expire - - result := &TokenRecord{ - Provider: a.Provider(), - FileName: record.FileName, - Storage: storage, - Metadata: record.Metadata, - } - return result, nil -} diff --git a/sdk/cliproxy/auth/manager.go b/sdk/cliproxy/auth/manager.go index f4291d20..96bee16a 100644 --- a/sdk/cliproxy/auth/manager.go +++ b/sdk/cliproxy/auth/manager.go @@ -585,17 +585,21 @@ func (m *Manager) checkRefreshes(ctx context.Context) { now := time.Now() snapshot := m.snapshotAuths() for _, a := range snapshot { - log.Debugf("checking refresh for %s, %s", a.Provider, a.ID) - if !m.shouldRefresh(a, now) { - continue + typ, _ := a.AccountInfo() + if typ != "api_key" { + if !m.shouldRefresh(a, now) { + continue + } + log.Debugf("checking refresh for %s, %s, %s", a.Provider, a.ID, typ) + + if exec := m.executorFor(a.Provider); exec == nil { + continue + } + if !m.markRefreshPending(a.ID, now) { + continue + } + go m.refreshAuth(ctx, a.ID) } - if exec := m.executorFor(a.Provider); exec == nil { - continue - } - if !m.markRefreshPending(a.ID, now) { - continue - } - go m.refreshAuth(ctx, a.ID) } } @@ -646,17 +650,20 @@ func (m *Manager) shouldRefresh(a *Auth, now time.Time) bool { provider := strings.ToLower(a.Provider) lead := ProviderRefreshLead(provider, a.Runtime) - if lead <= 0 { + if lead == nil { + return false + } + if *lead <= 0 { if hasExpiry && !expiry.IsZero() { return now.After(expiry) } return false } if hasExpiry && !expiry.IsZero() { - return time.Until(expiry) <= lead + return time.Until(expiry) <= *lead } if !lastRefresh.IsZero() { - return now.Sub(lastRefresh) >= lead + return now.Sub(lastRefresh) >= *lead } return true } @@ -839,6 +846,7 @@ func (m *Manager) refreshAuth(ctx context.Context, id string) { } cloned := auth.Clone() updated, err := exec.Refresh(ctx, cloned) + log.Debugf("refreshed %s, %s, %v", auth.Provider, auth.ID, err) now := time.Now() if err != nil { m.mu.Lock() diff --git a/sdk/cliproxy/auth/types.go b/sdk/cliproxy/auth/types.go index bbaba60b..33081aef 100644 --- a/sdk/cliproxy/auth/types.go +++ b/sdk/cliproxy/auth/types.go @@ -169,23 +169,23 @@ func expirationFromMap(meta map[string]any) (time.Time, bool) { return time.Time{}, false } -func ProviderRefreshLead(provider string, runtime any) time.Duration { +func ProviderRefreshLead(provider string, runtime any) *time.Duration { provider = strings.ToLower(provider) if runtime != nil { if eval, ok := runtime.(interface{ RefreshLead() *time.Duration }); ok { if lead := eval.RefreshLead(); lead != nil && *lead > 0 { - return *lead + return lead } } } if factory, ok := defaultAuthenticatorFactories[provider]; ok { if auth := factory(); auth != nil { if lead := auth.RefreshLead(); lead != nil && *lead > 0 { - return *lead + return lead } } } - return 0 + return nil } func parseTimeValue(v any) (time.Time, bool) {