From d25b6b41e85e91b8f22e79ba3469a5b5e5508b32 Mon Sep 17 00:00:00 2001 From: seakee Date: Thu, 2 Jul 2026 12:55:45 +0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix(management):=20filter=20auth?= =?UTF-8?q?=20files=20by=20identity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add name and auth_index filtering to the auth-files management API and allow status updates to verify auth_index before mutating an auth entry. This lets downstream tools target one auth file without fetching the entire list, and avoids toggling a same-name auth when an auth_index snapshot is available. --- .../api/handlers/management/auth_files.go | 91 ++++-- .../management/auth_files_filter_test.go | 260 ++++++++++++++++++ 2 files changed, 335 insertions(+), 16 deletions(-) create mode 100644 internal/api/handlers/management/auth_files_filter_test.go diff --git a/internal/api/handlers/management/auth_files.go b/internal/api/handlers/management/auth_files.go index a960b5861..3325f45b8 100644 --- a/internal/api/handlers/management/auth_files.go +++ b/internal/api/handlers/management/auth_files.go @@ -62,6 +62,7 @@ type codexOAuthService interface { var ( callbackForwardersMu sync.Mutex callbackForwarders = make(map[int]*callbackForwarder) + authFileEntryMu sync.Mutex errAuthFileMustBeJSON = errors.New("auth file must be .json") errAuthFileNotFound = errors.New("auth file not found") errPluginVirtualAuth = errors.New("plugin virtual auth cannot be modified directly; edit or delete the source auth file") @@ -325,9 +326,14 @@ func (h *Handler) ListAuthFiles(c *gin.Context) { h.listAuthFilesFromDisk(c) return } + nameFilter := strings.TrimSpace(c.Query("name")) + authIndexFilter := strings.TrimSpace(c.Query("auth_index")) auths := h.authManager.List() files := make([]gin.H, 0, len(auths)) for _, auth := range auths { + if !matchesAuthFileLookup(auth, nameFilter, authIndexFilter) { + continue + } if entry := h.buildAuthFileEntry(auth); entry != nil { files = append(files, entry) } @@ -340,6 +346,55 @@ func (h *Handler) ListAuthFiles(c *gin.Context) { c.JSON(200, gin.H{"files": files}) } +func lockedAuthIndex(auth *coreauth.Auth) string { + if auth == nil { + return "" + } + authFileEntryMu.Lock() + defer authFileEntryMu.Unlock() + return strings.TrimSpace(auth.EnsureIndex()) +} + +func matchesAuthFileLookup(auth *coreauth.Auth, name string, authIndex string) bool { + if auth == nil { + return false + } + if name != "" && strings.TrimSpace(auth.ID) != name && strings.TrimSpace(auth.FileName) != name { + return false + } + if authIndex != "" && lockedAuthIndex(auth) != authIndex { + return false + } + return true +} + +func (h *Handler) lookupAuthFile(name string, authIndex string) (*coreauth.Auth, bool) { + name = strings.TrimSpace(name) + authIndex = strings.TrimSpace(authIndex) + if h == nil || h.authManager == nil || name == "" { + return nil, false + } + if authIndex == "" { + if auth, ok := h.authManager.GetByID(name); ok { + return auth, true + } + auths := h.authManager.List() + for _, auth := range auths { + if auth != nil && strings.TrimSpace(auth.FileName) == name { + return auth, true + } + } + return nil, false + } + auths := h.authManager.List() + for _, auth := range auths { + if matchesAuthFileLookup(auth, name, authIndex) { + return auth, true + } + } + return nil, false +} + // GetAuthFileModels returns the models supported by a specific auth file func (h *Handler) GetAuthFileModels(c *gin.Context) { name := c.Query("name") @@ -390,17 +445,26 @@ func (h *Handler) GetAuthFileModels(c *gin.Context) { // List auth files from disk when the auth manager is unavailable. func (h *Handler) listAuthFilesFromDisk(c *gin.Context) { + nameFilter := strings.TrimSpace(c.Query("name")) + authIndexFilter := strings.TrimSpace(c.Query("auth_index")) entries, err := os.ReadDir(h.cfg.AuthDir) if err != nil { c.JSON(500, gin.H{"error": fmt.Sprintf("failed to read auth dir: %v", err)}) return } files := make([]gin.H, 0) + if authIndexFilter != "" { + c.JSON(200, gin.H{"files": files}) + return + } for _, e := range entries { if e.IsDir() { continue } name := e.Name() + if nameFilter != "" && name != nameFilter { + continue + } if !strings.HasSuffix(strings.ToLower(name), ".json") { continue } @@ -453,6 +517,12 @@ func (h *Handler) listAuthFilesFromDisk(c *gin.Context) { } func (h *Handler) buildAuthFileEntry(auth *coreauth.Auth) gin.H { + authFileEntryMu.Lock() + defer authFileEntryMu.Unlock() + return h.buildAuthFileEntryLocked(auth) +} + +func (h *Handler) buildAuthFileEntryLocked(auth *coreauth.Auth) gin.H { if auth == nil { return nil } @@ -1242,8 +1312,9 @@ func (h *Handler) PatchAuthFileStatus(c *gin.Context) { } var req struct { - Name string `json:"name"` - Disabled *bool `json:"disabled"` + Name string `json:"name"` + AuthIndex string `json:"auth_index"` + Disabled *bool `json:"disabled"` } if err := c.ShouldBindJSON(&req); err != nil { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"}) @@ -1251,6 +1322,7 @@ func (h *Handler) PatchAuthFileStatus(c *gin.Context) { } name := strings.TrimSpace(req.Name) + authIndex := strings.TrimSpace(req.AuthIndex) if name == "" { c.JSON(http.StatusBadRequest, gin.H{"error": "name is required"}) return @@ -1262,20 +1334,7 @@ func (h *Handler) PatchAuthFileStatus(c *gin.Context) { ctx := c.Request.Context() - // Find auth by name or ID - var targetAuth *coreauth.Auth - if auth, ok := h.authManager.GetByID(name); ok { - targetAuth = auth - } else { - auths := h.authManager.List() - for _, auth := range auths { - if auth.FileName == name { - targetAuth = auth - break - } - } - } - + targetAuth, _ := h.lookupAuthFile(name, authIndex) if targetAuth == nil { c.JSON(http.StatusNotFound, gin.H{"error": "auth file not found"}) return diff --git a/internal/api/handlers/management/auth_files_filter_test.go b/internal/api/handlers/management/auth_files_filter_test.go new file mode 100644 index 000000000..ea08d36cc --- /dev/null +++ b/internal/api/handlers/management/auth_files_filter_test.go @@ -0,0 +1,260 @@ +package management + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/gin-gonic/gin" + "github.com/router-for-me/CLIProxyAPI/v7/internal/config" + coreauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/auth" +) + +func TestListAuthFilesFiltersByNameAndAuthIndex(t *testing.T) { + t.Setenv("MANAGEMENT_PASSWORD", "") + + authDir := t.TempDir() + fileName := "shared-codex.json" + filePath := filepath.Join(authDir, fileName) + if errWrite := os.WriteFile(filePath, []byte(`{"type":"codex"}`), 0o600); errWrite != nil { + t.Fatalf("failed to write auth file: %v", errWrite) + } + + manager := coreauth.NewManager(nil, nil, nil) + registerAuthForLookupTest(t, manager, &coreauth.Auth{ + ID: "auth-a", + Index: "idx-a", + FileName: fileName, + Provider: "codex", + Status: coreauth.StatusActive, + Attributes: map[string]string{ + "path": filePath, + }, + }) + registerAuthForLookupTest(t, manager, &coreauth.Auth{ + ID: "auth-b", + Index: "idx-b", + FileName: fileName, + Provider: "codex", + Status: coreauth.StatusActive, + Attributes: map[string]string{ + "path": filePath, + }, + }) + + h := NewHandlerWithoutConfigFilePath(&config.Config{AuthDir: authDir}, manager) + rec := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(rec) + req := httptest.NewRequest(http.MethodGet, "/v0/management/auth-files?name=shared-codex.json&auth_index=idx-b", nil) + ctx.Request = req + + h.ListAuthFiles(ctx) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + var payload struct { + Files []map[string]any `json:"files"` + } + if errDecode := json.Unmarshal(rec.Body.Bytes(), &payload); errDecode != nil { + t.Fatalf("decode response: %v", errDecode) + } + if len(payload.Files) != 1 { + t.Fatalf("files len = %d, want 1 payload=%s", len(payload.Files), rec.Body.String()) + } + if got := payload.Files[0]["id"]; got != "auth-b" { + t.Fatalf("id = %#v, want auth-b", got) + } + if got := payload.Files[0]["auth_index"]; got != "idx-b" { + t.Fatalf("auth_index = %#v, want idx-b", got) + } +} + +func TestListAuthFilesFromDiskFiltersByNameAndRejectsAuthIndex(t *testing.T) { + t.Setenv("MANAGEMENT_PASSWORD", "") + + authDir := t.TempDir() + for _, file := range []struct { + name string + body string + }{ + {name: "alpha.json", body: `{"type":"codex","email":"alpha@example.com"}`}, + {name: "beta.json", body: `{"type":"codex","email":"beta@example.com"}`}, + } { + if errWrite := os.WriteFile(filepath.Join(authDir, file.name), []byte(file.body), 0o600); errWrite != nil { + t.Fatalf("failed to write auth file %s: %v", file.name, errWrite) + } + } + + h := NewHandlerWithoutConfigFilePath(&config.Config{AuthDir: authDir}, nil) + rec := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(rec) + ctx.Request = httptest.NewRequest(http.MethodGet, "/v0/management/auth-files?name=beta.json", nil) + + h.ListAuthFiles(ctx) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + var payload struct { + Files []map[string]any `json:"files"` + } + if errDecode := json.Unmarshal(rec.Body.Bytes(), &payload); errDecode != nil { + t.Fatalf("decode response: %v", errDecode) + } + if len(payload.Files) != 1 || payload.Files[0]["name"] != "beta.json" { + t.Fatalf("files = %#v, want only beta.json", payload.Files) + } + + rec = httptest.NewRecorder() + ctx, _ = gin.CreateTestContext(rec) + ctx.Request = httptest.NewRequest(http.MethodGet, "/v0/management/auth-files?name=beta.json&auth_index=idx-b", nil) + + h.ListAuthFiles(ctx) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + payload.Files = nil + if errDecode := json.Unmarshal(rec.Body.Bytes(), &payload); errDecode != nil { + t.Fatalf("decode auth_index response: %v", errDecode) + } + if len(payload.Files) != 0 { + t.Fatalf("files = %#v, want no disk fallback matches for auth_index", payload.Files) + } +} + +func TestPatchAuthFileStatusVerifiesAuthIndex(t *testing.T) { + t.Setenv("MANAGEMENT_PASSWORD", "") + + manager := coreauth.NewManager(nil, nil, nil) + registerAuthForLookupTest(t, manager, &coreauth.Auth{ + ID: "auth-a", + Index: "idx-a", + FileName: "shared-codex.json", + Provider: "codex", + Status: coreauth.StatusActive, + }) + registerAuthForLookupTest(t, manager, &coreauth.Auth{ + ID: "auth-b", + Index: "idx-b", + FileName: "shared-codex.json", + Provider: "codex", + Status: coreauth.StatusActive, + }) + + h := NewHandlerWithoutConfigFilePath(&config.Config{AuthDir: t.TempDir()}, manager) + rec := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(rec) + req := httptest.NewRequest(http.MethodPatch, "/v0/management/auth-files/status", strings.NewReader(`{"name":"shared-codex.json","auth_index":"idx-b","disabled":true}`)) + req.Header.Set("Content-Type", "application/json") + ctx.Request = req + + h.PatchAuthFileStatus(ctx) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + authA, okA := manager.GetByID("auth-a") + authB, okB := manager.GetByID("auth-b") + if !okA || !okB { + t.Fatalf("expected both auth records to exist") + } + if authA.Disabled || authA.Status == coreauth.StatusDisabled { + t.Fatalf("auth-a was modified: %+v", authA) + } + if !authB.Disabled || authB.Status != coreauth.StatusDisabled { + t.Fatalf("auth-b was not disabled: %+v", authB) + } +} + +func TestPatchAuthFileStatusRejectsMismatchedAuthIndex(t *testing.T) { + t.Setenv("MANAGEMENT_PASSWORD", "") + + manager := coreauth.NewManager(nil, nil, nil) + registerAuthForLookupTest(t, manager, &coreauth.Auth{ + ID: "auth-a", + Index: "idx-a", + FileName: "shared-codex.json", + Provider: "codex", + Status: coreauth.StatusActive, + }) + + h := NewHandlerWithoutConfigFilePath(&config.Config{AuthDir: t.TempDir()}, manager) + rec := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(rec) + req := httptest.NewRequest(http.MethodPatch, "/v0/management/auth-files/status", strings.NewReader(`{"name":"shared-codex.json","auth_index":"idx-missing","disabled":true}`)) + req.Header.Set("Content-Type", "application/json") + ctx.Request = req + + h.PatchAuthFileStatus(ctx) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want %d body=%s", rec.Code, http.StatusNotFound, rec.Body.String()) + } + authA, ok := manager.GetByID("auth-a") + if !ok { + t.Fatalf("expected auth-a to exist") + } + if authA.Disabled || authA.Status == coreauth.StatusDisabled { + t.Fatalf("auth-a was modified: %+v", authA) + } +} + +func TestAuthFileLookupAndEntryBuildConcurrentEnsureIndex(t *testing.T) { + t.Setenv("MANAGEMENT_PASSWORD", "") + + authDir := t.TempDir() + fileName := "concurrent-codex.json" + filePath := filepath.Join(authDir, fileName) + if errWrite := os.WriteFile(filePath, []byte(`{"type":"codex"}`), 0o600); errWrite != nil { + t.Fatalf("failed to write auth file: %v", errWrite) + } + + h := NewHandlerWithoutConfigFilePath(&config.Config{AuthDir: authDir}, nil) + auth := &coreauth.Auth{ + ID: "auth-concurrent", + Index: "idx-concurrent", + FileName: fileName, + Provider: "codex", + Status: coreauth.StatusActive, + Attributes: map[string]string{ + "path": filePath, + }, + } + + var wg sync.WaitGroup + for i := 0; i < 32; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 100; j++ { + if !matchesAuthFileLookup(auth, fileName, "idx-concurrent") { + t.Errorf("auth lookup did not match") + } + entry := h.buildAuthFileEntry(auth) + if entry == nil { + t.Errorf("entry is nil") + continue + } + if got := entry["auth_index"]; got != "idx-concurrent" { + t.Errorf("auth_index = %#v, want idx-concurrent", got) + } + } + }() + } + wg.Wait() +} + +func registerAuthForLookupTest(t *testing.T, manager *coreauth.Manager, auth *coreauth.Auth) { + t.Helper() + if _, errRegister := manager.Register(context.Background(), auth); errRegister != nil { + t.Fatalf("register auth %q: %v", auth.ID, errRegister) + } +}