Skip to content

Commit 6bfcb0c

Browse files
committed
feat(auth): improve unauthorized error handling for refresh and auto-refresh
- Added `isUnauthorizedError` and `hasUnauthorizedAuthFailure` to classify and handle unauthorized errors. - Introduced `refreshErrorFromError` to map errors to standardized unauthorized responses. - Modified refresh logic to stop auto-refresh retries for unauthorized errors. - Updated tests to verify unauthorized error handling and refresh retry prevention.
1 parent bd8c05a commit 6bfcb0c

5 files changed

Lines changed: 131 additions & 3 deletions

File tree

internal/runtime/executor/helps/home_refresh.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func RefreshAuthViaHome(ctx context.Context, cfg *config.Config, auth *cliproxya
7878
if msg == "" {
7979
msg = "home returned error"
8080
}
81-
return nil, true, homeStatusErr{code: http.StatusBadGateway, msg: msg}
81+
return nil, true, homeStatusErr{code: statusFromHomeErrorCode(code), msg: msg}
8282
}
8383

8484
var updated cliproxyauth.Auth
@@ -89,3 +89,14 @@ func RefreshAuthViaHome(ctx context.Context, cfg *config.Config, auth *cliproxya
8989
updated.EnsureIndex()
9090
return &updated, true, nil
9191
}
92+
93+
func statusFromHomeErrorCode(code string) int {
94+
switch strings.ToLower(strings.TrimSpace(code)) {
95+
case "authentication_error", "unauthorized":
96+
return http.StatusUnauthorized
97+
case "model_not_found":
98+
return http.StatusNotFound
99+
default:
100+
return http.StatusBadGateway
101+
}
102+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package helps
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
)
7+
8+
func TestStatusFromHomeErrorCodeMapsAuthenticationErrorToUnauthorized(t *testing.T) {
9+
if got := statusFromHomeErrorCode("authentication_error"); got != http.StatusUnauthorized {
10+
t.Fatalf("statusFromHomeErrorCode(authentication_error) = %d, want %d", got, http.StatusUnauthorized)
11+
}
12+
if got := statusFromHomeErrorCode("unauthorized"); got != http.StatusUnauthorized {
13+
t.Fatalf("statusFromHomeErrorCode(unauthorized) = %d, want %d", got, http.StatusUnauthorized)
14+
}
15+
}

sdk/cliproxy/auth/auto_refresh_loop.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,9 @@ func nextRefreshCheckAt(now time.Time, auth *Auth, interval time.Duration) (time
339339
if auth == nil {
340340
return time.Time{}, false
341341
}
342+
if hasUnauthorizedAuthFailure(auth) {
343+
return time.Time{}, false
344+
}
342345

343346
accountType, _ := auth.AccountInfo()
344347
if accountType == "api_key" {

sdk/cliproxy/auth/conductor.go

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2486,6 +2486,40 @@ func statusCodeFromError(err error) int {
24862486
return 0
24872487
}
24882488

2489+
func isUnauthorizedError(err error) bool {
2490+
if err == nil {
2491+
return false
2492+
}
2493+
if statusCodeFromError(err) == http.StatusUnauthorized {
2494+
return true
2495+
}
2496+
raw := strings.ToLower(err.Error())
2497+
return strings.Contains(raw, "status 401") || strings.Contains(raw, "401 unauthorized")
2498+
}
2499+
2500+
func hasUnauthorizedAuthFailure(auth *Auth) bool {
2501+
if auth == nil || auth.LastError == nil {
2502+
return false
2503+
}
2504+
return auth.LastError.StatusCode() == http.StatusUnauthorized || strings.EqualFold(auth.LastError.Code, "unauthorized")
2505+
}
2506+
2507+
func refreshErrorFromError(err error) *Error {
2508+
if err == nil {
2509+
return nil
2510+
}
2511+
statusCode := statusCodeFromError(err)
2512+
if statusCode == 0 && isUnauthorizedError(err) {
2513+
statusCode = http.StatusUnauthorized
2514+
}
2515+
authErr := &Error{Message: err.Error(), HTTPStatus: statusCode}
2516+
if statusCode == http.StatusUnauthorized {
2517+
authErr.Code = "unauthorized"
2518+
authErr.Retryable = false
2519+
}
2520+
return authErr
2521+
}
2522+
24892523
func retryAfterFromError(err error) *time.Duration {
24902524
if err == nil {
24912525
return nil
@@ -3680,6 +3714,9 @@ func (m *Manager) shouldRefresh(a *Auth, now time.Time) bool {
36803714
if a == nil {
36813715
return false
36823716
}
3717+
if hasUnauthorizedAuthFailure(a) {
3718+
return false
3719+
}
36833720
if !a.NextRefreshAfter.IsZero() && now.Before(a.NextRefreshAfter) {
36843721
return false
36853722
}
@@ -3924,11 +3961,19 @@ func (m *Manager) refreshAuth(ctx context.Context, id string) {
39243961
log.Debugf("refreshed %s, %s, %v", auth.Provider, auth.ID, err)
39253962
now := time.Now()
39263963
if err != nil {
3964+
unauthorized := isUnauthorizedError(err)
39273965
shouldReschedule := false
39283966
m.mu.Lock()
39293967
if current := m.auths[id]; current != nil {
3930-
current.NextRefreshAfter = now.Add(refreshFailureBackoff)
3931-
current.LastError = &Error{Message: err.Error()}
3968+
current.LastError = refreshErrorFromError(err)
3969+
if unauthorized {
3970+
current.NextRefreshAfter = time.Time{}
3971+
current.Unavailable = true
3972+
current.Status = StatusError
3973+
current.StatusMessage = "unauthorized"
3974+
} else {
3975+
current.NextRefreshAfter = now.Add(refreshFailureBackoff)
3976+
}
39323977
m.auths[id] = current
39333978
shouldReschedule = true
39343979
if m.scheduler != nil {

sdk/cliproxy/auth/conductor_scheduler_refresh_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"net/http"
77
"testing"
8+
"time"
89

910
"github.com/router-for-me/CLIProxyAPI/v7/internal/registry"
1011
cliproxyexecutor "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/executor"
@@ -36,6 +37,59 @@ func (e schedulerProviderTestExecutor) HttpRequest(ctx context.Context, auth *Au
3637
return nil, nil
3738
}
3839

40+
type unauthorizedRefreshTestExecutor struct {
41+
schedulerProviderTestExecutor
42+
}
43+
44+
func (e unauthorizedRefreshTestExecutor) Refresh(ctx context.Context, auth *Auth) (*Auth, error) {
45+
return nil, errors.New("token refresh failed with status 401: invalid_grant")
46+
}
47+
48+
func TestManager_RefreshAuthUnauthorizedFailureStopsAutoRefreshRetry(t *testing.T) {
49+
ctx := context.Background()
50+
manager := NewManager(nil, &RoundRobinSelector{}, nil)
51+
manager.RegisterExecutor(unauthorizedRefreshTestExecutor{
52+
schedulerProviderTestExecutor: schedulerProviderTestExecutor{provider: "codex"},
53+
})
54+
55+
auth := &Auth{
56+
ID: "unauthorized-refresh",
57+
Provider: "codex",
58+
Metadata: map[string]any{
59+
"email": "x@example.com",
60+
},
61+
}
62+
if _, errRegister := manager.Register(ctx, auth); errRegister != nil {
63+
t.Fatalf("register auth: %v", errRegister)
64+
}
65+
66+
manager.refreshAuth(ctx, auth.ID)
67+
68+
updated, ok := manager.GetByID(auth.ID)
69+
if !ok {
70+
t.Fatalf("expected auth %q after refresh", auth.ID)
71+
}
72+
if updated.LastError == nil {
73+
t.Fatal("expected unauthorized refresh failure to be recorded")
74+
}
75+
if got := updated.LastError.StatusCode(); got != http.StatusUnauthorized {
76+
t.Fatalf("LastError.StatusCode() = %d, want %d", got, http.StatusUnauthorized)
77+
}
78+
if updated.LastError.Code != "unauthorized" {
79+
t.Fatalf("LastError.Code = %q, want unauthorized", updated.LastError.Code)
80+
}
81+
if !updated.NextRefreshAfter.IsZero() {
82+
t.Fatalf("NextRefreshAfter = %s, want zero for unauthorized refresh failure", updated.NextRefreshAfter)
83+
}
84+
now := time.Now()
85+
if manager.shouldRefresh(updated, now) {
86+
t.Fatal("expected unauthorized auth to stop refresh attempts")
87+
}
88+
if _, shouldSchedule := nextRefreshCheckAt(now, updated, time.Second); shouldSchedule {
89+
t.Fatal("expected unauthorized auth to be removed from the auto-refresh schedule")
90+
}
91+
}
92+
3993
func TestManager_RefreshSchedulerEntry_RebuildsSupportedModelSetAfterModelRegistration(t *testing.T) {
4094
ctx := context.Background()
4195

0 commit comments

Comments
 (0)