diff --git a/internal/http/oauth.go b/internal/http/oauth.go index 76a8440c..fa901647 100644 --- a/internal/http/oauth.go +++ b/internal/http/oauth.go @@ -59,27 +59,34 @@ func NewOAuthHandler(provStore store.ProviderStore, secretStore store.ConfigSecr // RegisterRoutes registers OAuth routes on the given mux. func (h *OAuthHandler) RegisterRoutes(mux *http.ServeMux) { - mux.HandleFunc("GET /v1/auth/chatgpt/{provider}/status", h.auth(h.handleStatus)) - mux.HandleFunc("GET /v1/auth/chatgpt/{provider}/quota", h.auth(h.handleQuota)) + // Read-only status/quota — needed by /setup for browser-paired Operators + // (issue #1075). Managing OAuth (start/callback/logout) stays Admin-only. + mux.HandleFunc("GET /v1/auth/chatgpt/{provider}/status", h.readAuth(h.handleStatus)) + mux.HandleFunc("GET /v1/auth/chatgpt/{provider}/quota", h.readAuth(h.handleQuota)) mux.HandleFunc("POST /v1/auth/chatgpt/{provider}/start", h.auth(h.handleStart)) mux.HandleFunc("POST /v1/auth/chatgpt/{provider}/callback", h.auth(h.handleManualCallback)) mux.HandleFunc("POST /v1/auth/chatgpt/{provider}/logout", h.auth(h.handleLogout)) - mux.HandleFunc("GET /v1/auth/openai/status", h.auth(h.handleStatus)) - mux.HandleFunc("GET /v1/auth/openai/quota", h.auth(h.handleQuota)) + mux.HandleFunc("GET /v1/auth/openai/status", h.readAuth(h.handleStatus)) + mux.HandleFunc("GET /v1/auth/openai/quota", h.readAuth(h.handleQuota)) mux.HandleFunc("POST /v1/auth/openai/start", h.auth(h.handleStart)) mux.HandleFunc("POST /v1/auth/openai/callback", h.auth(h.handleManualCallback)) mux.HandleFunc("POST /v1/auth/openai/logout", h.auth(h.handleLogout)) } -// auth requires RoleAdmin for all OAuth endpoints. -// Breaking change from pre-#450: previously used requireAuth("", next) which -// allowed any authenticated user including operators. Now only admins can -// manage OAuth providers. +// auth requires RoleAdmin for OAuth management (start/callback/logout). +// Since #450 these mutating flows are admin-only; #1075 keeps that for writes +// but splits reads out via readAuth so setup can check connection status. func (h *OAuthHandler) auth(next http.HandlerFunc) http.HandlerFunc { return requireAuth(permissions.RoleAdmin, next) } +// readAuth gates read-only OAuth status/quota at the method-derived minimum +// (GET→Viewer). Responses expose only connection state and quota, never tokens. +func (h *OAuthHandler) readAuth(next http.HandlerFunc) http.HandlerFunc { + return requireAuth("", next) +} + func oauthTenantID(ctx context.Context) uuid.UUID { if tid := store.TenantIDFromContext(ctx); tid != uuid.Nil { return tid diff --git a/internal/http/oauth_test.go b/internal/http/oauth_test.go index 22cb3068..5eb2d113 100644 --- a/internal/http/oauth_test.go +++ b/internal/http/oauth_test.go @@ -420,8 +420,11 @@ func TestOAuthHandlerProviderLogoutRoute(t *testing.T) { } } -func TestProvidersHandlerRequiresAdmin(t *testing.T) { - token := "operator-key" +// operatorAPIKeyRequest builds a request authenticated as a browser-pairing-style +// Operator (operator.write scope → RoleOperator). +func operatorAPIKeyRequest(t *testing.T, method, path string) *http.Request { + t.Helper() + token := "operator-key-" + method + path setupTestCache(t, map[string]*store.APIKeyData{ crypto.HashAPIKey(token): { ID: uuid.New(), @@ -429,41 +432,49 @@ func TestProvidersHandlerRequiresAdmin(t *testing.T) { TenantID: store.MasterTenantID, }, }) + req := httptest.NewRequest(method, path, nil) + req.Header.Set("Authorization", "Bearer "+token) + return req +} +// TestProvidersHandler_OperatorReadsButCannotMutate is a regression for issue #1075: +// browser-paired Operators must read providers so /setup can proceed, while +// create/update/delete stay Admin-only. +func TestProvidersHandler_OperatorReadsButCannotMutate(t *testing.T) { h := NewProvidersHandler(newMockProviderStore(), newMockSecretsStore(), nil, "") mux := http.NewServeMux() h.RegisterRoutes(mux) - req := httptest.NewRequest("GET", "/v1/providers", nil) - req.Header.Set("Authorization", "Bearer "+token) - w := httptest.NewRecorder() - mux.ServeHTTP(w, req) + wGet := httptest.NewRecorder() + mux.ServeHTTP(wGet, operatorAPIKeyRequest(t, "GET", "/v1/providers")) + if wGet.Code != http.StatusOK { + t.Fatalf("operator GET /v1/providers = %d, want 200", wGet.Code) + } - if w.Code != http.StatusForbidden { - t.Fatalf("status code = %d, want %d", w.Code, http.StatusForbidden) + wPost := httptest.NewRecorder() + mux.ServeHTTP(wPost, operatorAPIKeyRequest(t, "POST", "/v1/providers")) + if wPost.Code != http.StatusForbidden { + t.Fatalf("operator POST /v1/providers = %d, want 403", wPost.Code) } } -func TestOAuthHandlerRequiresAdmin(t *testing.T) { - token := "operator-key" - setupTestCache(t, map[string]*store.APIKeyData{ - crypto.HashAPIKey(token): { - ID: uuid.New(), - Scopes: []string{"operator.write"}, - TenantID: store.MasterTenantID, - }, - }) +// TestOAuthHandler_OperatorReadsStatusButCannotManage: Operator can read OAuth +// status for setup; managing OAuth (start) remains Admin-only (issue #1075, #450). +func TestOAuthHandler_OperatorReadsStatusButCannotManage(t *testing.T) { h := newTestOAuthHandler(t, "") mux := http.NewServeMux() h.RegisterRoutes(mux) - req := httptest.NewRequest("GET", "/v1/auth/openai/status", nil) - req.Header.Set("Authorization", "Bearer "+token) - w := httptest.NewRecorder() - mux.ServeHTTP(w, req) + wGet := httptest.NewRecorder() + mux.ServeHTTP(wGet, operatorAPIKeyRequest(t, "GET", "/v1/auth/openai/status")) + if wGet.Code != http.StatusOK { + t.Fatalf("operator GET oauth status = %d, want 200", wGet.Code) + } - if w.Code != http.StatusForbidden { - t.Fatalf("status code = %d, want %d", w.Code, http.StatusForbidden) + wPost := httptest.NewRecorder() + mux.ServeHTTP(wPost, operatorAPIKeyRequest(t, "POST", "/v1/auth/openai/start")) + if wPost.Code != http.StatusForbidden { + t.Fatalf("operator POST oauth start = %d, want 403", wPost.Code) } } diff --git a/internal/http/providers.go b/internal/http/providers.go index 9539d631..b35848d1 100644 --- a/internal/http/providers.go +++ b/internal/http/providers.go @@ -158,35 +158,45 @@ func providerCacheTenantID(ctx context.Context, tenantID uuid.UUID) uuid.UUID { // RegisterRoutes registers all provider management routes on the given mux. func (h *ProvidersHandler) RegisterRoutes(mux *http.ServeMux) { - // Provider CRUD - mux.HandleFunc("GET /v1/providers", h.auth(h.handleListProviders)) + // Provider CRUD — reads are needed by /setup for browser-paired Operators + // (issue #1075), so GET routes use read-level auth (GET→Viewer) while + // mutations stay Admin-only. + mux.HandleFunc("GET /v1/providers", h.readAuth(h.handleListProviders)) mux.HandleFunc("POST /v1/providers", h.auth(h.handleCreateProvider)) - mux.HandleFunc("GET /v1/providers/{id}", h.auth(h.handleGetProvider)) + mux.HandleFunc("GET /v1/providers/{id}", h.readAuth(h.handleGetProvider)) mux.HandleFunc("PUT /v1/providers/{id}", h.auth(h.handleUpdateProvider)) mux.HandleFunc("DELETE /v1/providers/{id}", h.auth(h.handleDeleteProvider)) // Model listing (proxied to upstream provider API) - mux.HandleFunc("GET /v1/providers/{id}/models", h.auth(h.handleListProviderModels)) + mux.HandleFunc("GET /v1/providers/{id}/models", h.readAuth(h.handleListProviderModels)) - // Provider + model verification (pre-flight check) + // Provider + model verification (pre-flight check) — mutating actions, Admin. mux.HandleFunc("POST /v1/providers/{id}/reconnect", h.auth(h.handleReconnectProvider)) mux.HandleFunc("POST /v1/providers/{id}/verify", h.auth(h.handleVerifyProvider)) mux.HandleFunc("POST /v1/providers/{id}/verify-embedding", h.auth(h.handleVerifyEmbedding)) - // Provider-scoped Codex pool activity monitor - mux.HandleFunc("GET /v1/providers/{id}/codex-pool-activity", h.auth(h.handleProviderCodexPoolActivity)) + // Provider-scoped Codex pool activity monitor (read-only status) + mux.HandleFunc("GET /v1/providers/{id}/codex-pool-activity", h.readAuth(h.handleProviderCodexPoolActivity)) - // Embedding system status - mux.HandleFunc("GET /v1/embedding/status", h.auth(h.handleEmbeddingStatus)) + // Embedding system status (read-only) + mux.HandleFunc("GET /v1/embedding/status", h.readAuth(h.handleEmbeddingStatus)) - // Claude CLI auth status (global — not per-provider) - mux.HandleFunc("GET /v1/providers/claude-cli/auth-status", h.auth(h.handleClaudeCLIAuthStatus)) + // Claude CLI auth status (global — not per-provider; read-only status) + mux.HandleFunc("GET /v1/providers/claude-cli/auth-status", h.readAuth(h.handleClaudeCLIAuthStatus)) } +// auth gates provider mutations at Admin. func (h *ProvidersHandler) auth(next http.HandlerFunc) http.HandlerFunc { return requireAuth(permissions.RoleAdmin, next) } +// readAuth gates read-only provider endpoints at the method-derived minimum +// (GET→Viewer), so browser-paired Operators can complete /setup (issue #1075). +// Responses already mask API keys, and provider queries stay tenant-scoped. +func (h *ProvidersHandler) readAuth(next http.HandlerFunc) http.HandlerFunc { + return requireAuth("", next) +} + // maskAPIKey replaces non-empty API keys with "***". func maskAPIKey(p *store.LLMProviderData) { if p.APIKey != "" {