From 0ae55991bb8241302b3a5d88561efc6a595266d7 Mon Sep 17 00:00:00 2001 From: thotam <78630736+thotam@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:27:44 +0700 Subject: [PATCH] feat(mcp): MCP OAuth 2.1 client for tool servers (#1196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(mcp): MCP OAuth 2.1 client — full implementation with tests Implements a complete MCP OAuth 2.1 authorization flow for tool servers that require user-delegated access, covering all layers from DB to UI. - discovery.go: RFC 9728 protected-resource → RFC 8414 AS metadata → OIDC fallback chain with 5-min in-memory cache and InvalidateCache() - dcr.go: RFC 7591 Dynamic Client Registration with response size guard - flow.go: PKCE (S256) authorization code flow — StartFlow(), ExchangeCode(), ClientCredentials(), auto-cleanup of expired flows; carries AS issuer through PendingFlow for status display - refresher.go: OAuthTokenProvider with in-memory token cache, automatic refresh on expiry, per-user vs global slot isolation, InvalidateCache/InvalidateServer - migrations/000074 + SQLite schema: mcp_oauth_tokens with AES-256-GCM encrypted access/refresh tokens, partial unique index for global vs per-user rows, ON DELETE CASCADE from mcp_servers - store.MCPOAuthTokenStore: Upsert, Get/GetUser, Delete/DeleteUser, and DeleteServerOAuthTokens (purge all rows for a server) - PostgreSQL + SQLite implementations - POST /v1/mcp/oauth/start — discovery + optional DCR + PKCE redirect URL; client_credentials completes server-side (no redirect) and returns completed=true - GET /v1/mcp/oauth/callback — exchange code, persist token, publish WS event; payload built via json.Marshal (no reflected XSS via error_description) - GET /v1/mcp/oauth/status/{id}, DELETE /v1/mcp/oauth/token/{id} — admin-gated - POST /v1/mcp/oauth/discover/{id} — on-demand discovery probe - All outbound calls go through the SSRF-safe client with pinned IPs - pkg/protocol/mcp_events.go: EventMCPOAuthComplete routed only to the initiating user (admins in-tenant included); fail-closed across tenants - getUserMCPTools() injects Authorization: Bearer from OAuthTokenProvider; on a 401 for OAuth servers it purges the cached token so the next turn re-resolves - handleUpdateServer purges all OAuth tokens (global + per-user), drops the refresher cache, and evicts the pool when a server's URL or OAuth config (client_id / endpoints / grant_type / scope / auth_type) changes — so the status UI and agent never use a token minted for the old resource/AS - MCPOAuthDialog (WS-driven), unified user-credentials dialog, OAuth settings fields; handles the no-redirect client_credentials completion - internal/mcp/oauth/*_test.go: discovery cache, PKCE, DCR, refresher - internal/http/mcp_oauth_test.go + mcp_update_oauth_purge_test.go: routes, auth gating, WS event, purge-on-URL/OAuth-config-change - tests/integration: store + encryption + tenant isolation, E2E start→callback, DeleteServerOAuthTokens - internal/gateway/event_filter_test.go, internal/agent/loop_mcp_user_test.go * fix(mcp): return 400 on OAuth callback with code but missing state The callback handler rendered a 200 HTML page whenever code or state was absent. An auth code WITH a missing state is a malformed / CSRF-risk callback (state is the CSRF token), so reject that case with HTTP 400. A bare hit with neither code nor state (user opening the URL directly), provider errors, and exchange failures keep their 200 HTML popup page. Adds a status code parameter to writeCallbackHTML. Fixes the TestOAuthCallbackMissingState integration regression while keeping TestHandleCallbackMissingCodeAndState (no params -> 200) green. * fix(mcp): scope-based OAuth auth + honor manual OAuth endpoints Addresses the two MCP/OAuth security-review findings. Finding 1 — authorization. mcp_oauth_tokens is tenant-scoped, but start/status/revoke were gated only by requireAuth(RoleAdmin), an RBAC role check, not tenant membership, so a RoleAdmin caller could act on a tenant they don't administer. A blanket requireTenantAdmin would have broken per-user self-service, which the UI exposes (the per-user MCPUserCredentialsDialog shows an "Authorize" button to regular users for their own credentials). Instead mirror the existing per-user MCP credentials model (resolveTargetUserID in mcp_user_credentials.go): - start/status/revoke accept any authenticated user; each handler calls authorizeOAuthScope. - a caller may manage their OWN per-user token (self-service); the global/server token (user_id="") and other users' tokens require tenant-admin (owner bypass), so a RoleAdmin that is not a tenant admin is rejected. - discover stays admin-only (it only previews AS metadata for a server). Add a TenantStore dependency. Tests cover self-service, on-behalf-of- another (403), and global-by-non-tenant-admin (403). Finding 2 — honor manual OAuth config end-to-end. The UI sent use_dcr / auth_endpoint / token_endpoint and the update path fingerprinted them for purge, but handleStart always discovered + DCR'd and ignored them. Now: - use_dcr=false (a *bool, so legacy/absent stays discover+DCR) skips discovery/registration and uses the operator endpoints, SSRF-validated. - token_endpoint is always required; auth_endpoint only for auth-code grants — client_credentials needs no authorization URL, matching the UI which hides that field for that grant. - the refresher already refreshes against the stored token_endpoint and the callback persists it, so manual-mode tokens refresh correctly. - oauthFingerprint includes use_dcr (nil normalized to true) so toggling DCR mode purges stale tokens. - the web form only serializes manual endpoints when use_dcr is off. Audited all MCP dialogs (form, global OAuth, per-user credentials, grants, tools): OAuth dialogs handle completed/auth_url identically and read config from stored server settings; runtime connect uses the stored token via the refresher (no re-discovery). Tests: manual auth-code + client_credentials endpoints, missing/SSRF endpoints, and the full self/global/on-behalf authorization matrix. --- cmd/gateway.go | 42 +- cmd/gateway_http_handlers.go | 2 +- cmd/gateway_http_wiring.go | 7 + cmd/gateway_managed.go | 9 +- internal/agent/loop_mcp_user.go | 89 +- internal/agent/loop_mcp_user_test.go | 315 ++++- internal/agent/loop_types.go | 21 +- internal/agent/resolver.go | 7 + internal/config/config_channels.go | 1 + internal/gateway/event_filter.go | 9 + internal/gateway/event_filter_test.go | 67 + internal/gateway/server.go | 1 + internal/http/mcp.go | 127 +- internal/http/mcp_oauth.go | 717 +++++++++++ internal/http/mcp_oauth_test.go | 1122 +++++++++++++++++ internal/http/mcp_tools.go | 56 +- internal/http/mcp_tools_test.go | 119 ++ internal/http/mcp_update_oauth_purge_test.go | 131 ++ internal/http/mcp_user_credentials.go | 29 +- internal/http/mcp_user_credentials_test.go | 82 ++ internal/http/validate.go | 2 +- internal/i18n/catalog_en.go | 6 +- internal/i18n/catalog_vi.go | 6 +- internal/i18n/catalog_zh.go | 6 +- internal/i18n/keys.go | 6 +- internal/mcp/manager.go | 70 +- internal/mcp/manager_oauth_test.go | 94 ++ internal/mcp/oauth/dcr.go | 76 ++ internal/mcp/oauth/dcr_test.go | 137 ++ internal/mcp/oauth/discovery.go | 226 ++++ internal/mcp/oauth/discovery_test.go | 362 ++++++ internal/mcp/oauth/flow.go | 282 +++++ internal/mcp/oauth/flow_test.go | 313 +++++ internal/mcp/oauth/refresher.go | 201 +++ internal/mcp/oauth/refresher_test.go | 357 ++++++ internal/mcp/pool.go | 69 + internal/store/mcp_store.go | 46 +- internal/store/pg/factory.go | 1 + internal/store/pg/mcp_oauth_tokens.go | 226 ++++ internal/store/sqlitestore/factory.go | 1 + .../store/sqlitestore/mcp_oauth_tokens.go | 198 +++ internal/store/sqlitestore/schema.go | 28 +- internal/store/sqlitestore/schema.sql | 37 + internal/store/stores.go | 1 + internal/tools/policy.go | 20 + internal/upgrade/version.go | 2 +- migrations/000084_mcp_oauth_tokens.down.sql | 1 + migrations/000084_mcp_oauth_tokens.up.sql | 36 + pkg/protocol/events.go | 4 + pkg/protocol/mcp_events.go | 11 + tests/integration/v3_fixture_builders.go | 42 + tests/integration/v3_mcp_oauth_flow_test.go | 329 +++++ tests/integration/v3_mcp_oauth_tokens_test.go | 513 ++++++++ .../components/shared/key-value-editor.tsx | 2 +- ui/web/src/i18n/locales/en/mcp.json | 34 +- ui/web/src/i18n/locales/vi/mcp.json | 34 +- ui/web/src/i18n/locales/zh/mcp.json | 34 +- ui/web/src/pages/mcp/hooks/use-mcp.ts | 35 +- ui/web/src/pages/mcp/mcp-form-dialog.tsx | 185 ++- ui/web/src/pages/mcp/mcp-oauth-dialog.tsx | 222 ++++ ui/web/src/pages/mcp/mcp-page.tsx | 25 +- ui/web/src/pages/mcp/mcp-settings-fields.tsx | 151 ++- .../pages/mcp/mcp-user-credentials-dialog.tsx | 206 ++- ui/web/src/schemas/mcp.schema.ts | 9 + ui/web/src/types/mcp.ts | 45 +- 65 files changed, 7512 insertions(+), 132 deletions(-) create mode 100644 internal/http/mcp_oauth.go create mode 100644 internal/http/mcp_oauth_test.go create mode 100644 internal/http/mcp_tools_test.go create mode 100644 internal/http/mcp_update_oauth_purge_test.go create mode 100644 internal/http/mcp_user_credentials_test.go create mode 100644 internal/mcp/manager_oauth_test.go create mode 100644 internal/mcp/oauth/dcr.go create mode 100644 internal/mcp/oauth/dcr_test.go create mode 100644 internal/mcp/oauth/discovery.go create mode 100644 internal/mcp/oauth/discovery_test.go create mode 100644 internal/mcp/oauth/flow.go create mode 100644 internal/mcp/oauth/flow_test.go create mode 100644 internal/mcp/oauth/refresher.go create mode 100644 internal/mcp/oauth/refresher_test.go create mode 100644 internal/store/pg/mcp_oauth_tokens.go create mode 100644 internal/store/sqlitestore/mcp_oauth_tokens.go create mode 100644 migrations/000084_mcp_oauth_tokens.down.sql create mode 100644 migrations/000084_mcp_oauth_tokens.up.sql create mode 100644 pkg/protocol/mcp_events.go create mode 100644 tests/integration/v3_mcp_oauth_flow_test.go create mode 100644 tests/integration/v3_mcp_oauth_tokens_test.go create mode 100644 ui/web/src/pages/mcp/mcp-oauth-dialog.tsx diff --git a/cmd/gateway.go b/cmd/gateway.go index 53846be5..30916757 100644 --- a/cmd/gateway.go +++ b/cmd/gateway.go @@ -41,9 +41,11 @@ import ( httpapi "github.com/nextlevelbuilder/goclaw/internal/http" kg "github.com/nextlevelbuilder/goclaw/internal/knowledgegraph" mcpbridge "github.com/nextlevelbuilder/goclaw/internal/mcp" + mcpoauth "github.com/nextlevelbuilder/goclaw/internal/mcp/oauth" "github.com/nextlevelbuilder/goclaw/internal/media" "github.com/nextlevelbuilder/goclaw/internal/providers" "github.com/nextlevelbuilder/goclaw/internal/scheduler" + "github.com/nextlevelbuilder/goclaw/internal/security" "github.com/nextlevelbuilder/goclaw/internal/skills" "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/nextlevelbuilder/goclaw/internal/tools" @@ -366,10 +368,16 @@ func runGateway() { server.SetAgentStore(pgStores.Agents) } + // Build OAuth token refresher before wireExtras so the resolver can inject tokens. + var mcpOAuthRefresher mcpbridge.OAuthTokenProvider + if pgStores != nil && pgStores.MCPOAuthTokens != nil { + mcpOAuthRefresher = mcpoauth.NewRefresher(pgStores.MCPOAuthTokens, security.NewSafeClient(15*time.Second)) + } + var mcpPool *mcpbridge.Pool var mediaStore *media.Store var postTurn tools.PostTurnProcessor - contextFileInterceptor, mcpPool, mediaStore, postTurn = wireExtras(pgStores, agentRouter, providerRegistry, modelReg, msgBus, pgStores.Sessions, toolsReg, toolPE, skillsLoader, hasMemory, traceCollector, workspace, cfg.Gateway.InjectionAction, cfg, sandboxMgr, redisClient, domainBus, usageCapSvc) + contextFileInterceptor, mcpPool, mediaStore, postTurn = wireExtras(pgStores, agentRouter, providerRegistry, modelReg, msgBus, pgStores.Sessions, toolsReg, toolPE, skillsLoader, hasMemory, traceCollector, workspace, cfg.Gateway.InjectionAction, cfg, sandboxMgr, redisClient, domainBus, usageCapSvc, mcpOAuthRefresher) if mcpPool != nil { defer mcpPool.Stop() } @@ -421,6 +429,37 @@ func runGateway() { wakeH.SetPostTurnProcessor(postTurn) } + // MCP OAuth handler — per-server OAuth 2.1 client flows. + var mcpOAuthH *httpapi.MCPOAuthHandler + if pgStores != nil && pgStores.MCP != nil && pgStores.MCPOAuthTokens != nil { + safeHTTPClient := security.NewSafeClient(15 * time.Second) + var oauthRefresher *mcpoauth.Refresher + if r, ok := mcpOAuthRefresher.(*mcpoauth.Refresher); ok { + oauthRefresher = r + } + mcpOAuthH = httpapi.NewMCPOAuthHandler(httpapi.MCPOAuthHandlerDeps{ + MCPStore: pgStores.MCP, + OAuthStore: pgStores.MCPOAuthTokens, + Discoverer: mcpoauth.NewDiscoverer(safeHTTPClient), + FlowMgr: mcpoauth.NewFlowManager(safeHTTPClient), + Refresher: oauthRefresher, + EventBus: msgBus, + PublicURL: cfg.Gateway.PublicURL, + Port: cfg.Gateway.Port, + TenantStore: pgStores.Tenants, + }) + // Inject OAuth token provider into MCP tools handler so on-demand tool + // discovery can authenticate against OAuth-protected MCP servers. + if mcpH != nil && mcpOAuthRefresher != nil { + mcpH.SetOAuthProvider(mcpOAuthRefresher) + } + // Inject the OAuth token store so the update handler can purge stale tokens + // when a server's URL or OAuth config changes. + if mcpH != nil { + mcpH.SetOAuthStore(pgStores.MCPOAuthTokens) + } + } + // Wire all server.Set*Handler() calls via extracted helper. deps.wireHTTPHandlersOnServer( httpHandlers{ @@ -436,6 +475,7 @@ func runGateway() { secureCLI: secureCLIH, secureCLIGrant: secureCLIGrantH, mcpUserCreds: mcpUserCredsH, + mcpOAuth: mcpOAuthH, }, wakeH, mcpPool, diff --git a/cmd/gateway_http_handlers.go b/cmd/gateway_http_handlers.go index d01ab5a1..231fe225 100644 --- a/cmd/gateway_http_handlers.go +++ b/cmd/gateway_http_handlers.go @@ -65,7 +65,7 @@ func wireHTTP(stores *store.Stores, defaultWorkspace, dataDir, bundledSkillsDir } var mcpUserCredsH *httpapi.MCPUserCredentialsHandler if stores != nil && stores.MCP != nil { - mcpUserCredsH = httpapi.NewMCPUserCredentialsHandler(stores.MCP, stores.Tenants) + mcpUserCredsH = httpapi.NewMCPUserCredentialsHandler(stores.MCP, stores.Tenants, msgBus) } if stores != nil && stores.ChannelInstances != nil { diff --git a/cmd/gateway_http_wiring.go b/cmd/gateway_http_wiring.go index c5e8db77..b403e7f2 100644 --- a/cmd/gateway_http_wiring.go +++ b/cmd/gateway_http_wiring.go @@ -32,6 +32,7 @@ type httpHandlers struct { secureCLI *httpapi.SecureCLIHandler secureCLIGrant *httpapi.SecureCLIGrantHandler mcpUserCreds *httpapi.MCPUserCredentialsHandler + mcpOAuth *httpapi.MCPOAuthHandler } // wireHTTPHandlersOnServer registers all HTTP handler objects onto the gateway server. @@ -66,6 +67,12 @@ func (d *gatewayDeps) wireHTTPHandlersOnServer( if h.mcpUserCreds != nil { d.server.SetMCPUserCredentialsHandler(h.mcpUserCreds) } + if h.mcpOAuth != nil { + if mcpPool != nil { + h.mcpOAuth.SetEvictor(mcpPool) + } + d.server.SetMCPOAuthHandler(h.mcpOAuth) + } if h.channelInstances != nil { d.server.SetChannelInstancesHandler(h.channelInstances) } diff --git a/cmd/gateway_managed.go b/cmd/gateway_managed.go index 5494a462..dabdfd9d 100644 --- a/cmd/gateway_managed.go +++ b/cmd/gateway_managed.go @@ -61,6 +61,7 @@ func wireExtras( redisClient any, // nil when built without -tags redis or when Redis is unconfigured domainBus eventbus.DomainEventBus, usageCapSvc *usagecaps.Service, + mcpOAuthProvider mcpbridge.OAuthTokenProvider, // nil = OAuth injection disabled ) (*tools.ContextFileInterceptor, *mcpbridge.Pool, *media.Store, tools.PostTurnProcessor) { // 1. Build cache instances (in-memory or Redis depending on build tags) agentCtxCache, userCtxCache := makeCaches(redisClient) @@ -243,6 +244,7 @@ func wireExtras( MCPStore: stores.MCP, MCPPool: mcpPool, MCPGrantChecker: mcpGrantChecker, + MCPOAuthTokenProvider: mcpOAuthProvider, ConfigPermStore: stores.ConfigPermissions, MediaStore: mediaStore, ModelPricing: appCfg.Telemetry.ModelPricing, @@ -535,7 +537,9 @@ func wireExtras( agentRouter.InvalidateAll() }) - // MCP cache: invalidate all agent caches when MCP servers/grants change + // MCP cache: invalidate all agent caches + per-user pool connections when MCP servers/grants change. + // Per-user pool connections hold stale credentials/headers; evicting them forces a fresh + // AcquireUser on next request so new OAuth tokens and grant changes take effect immediately. msgBus.Subscribe(bus.TopicCacheMCP, func(event bus.Event) { if event.Name != protocol.EventCacheInvalidate { return @@ -545,6 +549,9 @@ func wireExtras( return } agentRouter.InvalidateAll() + if mcpPool != nil { + mcpPool.EvictAllUsers() + } }) // Cron cache: invalidate job cache on cron changes diff --git a/internal/agent/loop_mcp_user.go b/internal/agent/loop_mcp_user.go index f2c83d50..96916ba6 100644 --- a/internal/agent/loop_mcp_user.go +++ b/internal/agent/loop_mcp_user.go @@ -2,12 +2,15 @@ package agent import ( "context" + "encoding/json" "log/slog" "maps" "strings" "time" + "github.com/google/uuid" mcpbridge "github.com/nextlevelbuilder/goclaw/internal/mcp" + "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/nextlevelbuilder/goclaw/internal/tools" ) @@ -121,13 +124,38 @@ func (l *Loop) getUserMCPTools(ctx context.Context, userID string) []tools.Tool for _, info := range l.mcpUserCredSrvs { srv := info.Server - // Check if user has credentials for this server - uc, err := l.mcpStore.GetUserCredentials(ctx, srv.ID, userID) - if err != nil || uc == nil || (uc.APIKey == "" && len(uc.Headers) == 0 && len(uc.Env) == 0) { + // OAuth-active servers authenticate ONLY via the user's OAuth token. Static + // per-user credentials (api_key/headers/env) are deliberately NOT consulted for + // them — otherwise a user who set static creds but never authorized OAuth could + // call tools, bypassing OAuth entirely. + oauthActive := false + if len(srv.Settings) > 0 { + var settingsObj struct { + OAuth struct { + AuthType string `json:"auth_type"` + } `json:"oauth"` + } + _ = json.Unmarshal(srv.Settings, &settingsObj) + oauthActive = settingsObj.OAuth.AuthType == "oauth" + } + + // Static per-user credentials — only for NON-OAuth servers. + var uc *store.MCPUserCredentials + hasStaticCreds := false + if !oauthActive { + if c, e := l.mcpStore.GetUserCredentials(ctx, srv.ID, userID); e == nil && c != nil && + (c.APIKey != "" || len(c.Headers) > 0 || len(c.Env) > 0) { + uc = c + hasStaticCreds = true + } + } + hasOAuthCreds := oauthActive && l.mcpOAuthTokenProvider != nil + + if !hasStaticCreds && !hasOAuthCreds { continue } - // Resolve connection params: server defaults merged with user overrides + // Resolve connection params: server defaults merged with user overrides. args := mcpbridge.ParseJSONBytesToStringSlice(srv.Args) env := mcpbridge.ParseJSONBytesToStringMap(srv.Env) if env == nil { @@ -138,23 +166,47 @@ func (l *Loop) getUserMCPTools(ctx context.Context, userID string) []tools.Tool headers = make(map[string]string) } - // Inject server-level API key into headers if present - if srv.APIKey != "" && headers["Authorization"] == "" { + // Inject server-level API key into headers — ONLY for non-OAuth servers. + // For OAuth servers the Authorization MUST come from the user's OAuth token; + // falling back to the shared server-level api_key/headers would let a user who + // hasn't authorized still call tools with the common credential (isolation leak). + if !hasOAuthCreds && srv.APIKey != "" && headers["Authorization"] == "" { headers["Authorization"] = "Bearer " + srv.APIKey } - // Merge user credentials (user overrides server defaults) - if uc.APIKey != "" { - headers["Authorization"] = "Bearer " + uc.APIKey + // Merge user credentials (user overrides server defaults). + if hasStaticCreds { + if uc.APIKey != "" { + headers["Authorization"] = "Bearer " + uc.APIKey + } + maps.Copy(headers, uc.Headers) + maps.Copy(env, uc.Env) + } + + // Inject OAuth Bearer token for OAuth-enabled servers. The token is the ONLY + // source of Authorization for these servers — if none is available (user not + // authorized yet / refresh failed), skip the server entirely so no tools are + // exposed, rather than connecting with the shared server-level credential. + if hasOAuthCreds { + token, err2 := l.mcpOAuthTokenProvider.GetValidToken(ctx, srv.ID, l.tenantID, userID) + if err2 != nil || token == "" { + if err2 != nil { + slog.Warn("mcp.oauth_token_unavailable", "server", srv.Name, "user", userID, "tenant", l.tenantID, "error", err2) + } else { + slog.Debug("mcp.oauth_token_empty", "server", srv.Name, "user", userID, "tenant", l.tenantID) + } + // Strip any inherited Authorization and skip — OAuth not authorized. + continue + } + headers["Authorization"] = "Bearer " + token + slog.Debug("mcp.oauth_token_injected", "server", srv.Name, "user", userID, "tenant", l.tenantID) } - maps.Copy(headers, uc.Headers) - maps.Copy(env, uc.Env) // Acquire user-keyed pool connection entry, err := l.mcpPool.AcquireUser(ctx, l.tenantID, srv.Name, userID, srv.Transport, srv.Command, args, env, srv.URL, headers, srv.TimeoutSec) if err != nil { - if isUnauthorized401(err) { + if isUnauthorized401(err) && uc != nil { expiresAt := strings.TrimSpace(uc.Env["BITRIX_EXPIRES_AT"]) expired := false if expiresAt != "" { @@ -174,6 +226,19 @@ func (l *Loop) getUserMCPTools(ctx context.Context, userID string) []tools.Tool _ = l.mcpStore.DeleteUserCredentials(ctx, srv.ID, userID) slog.Warn("mcp.user_credentials_purged", "server", srv.Name, "user", userID, "reason", "unauthorized_401") } + // OAuth servers (uc == nil, hasOAuthCreds) carry no static credentials + // to purge, but a 401 still means the injected Bearer token was rejected. + // Evict the in-memory OAuth token cache so the next turn reloads from the + // store (picking up a token refreshed out-of-band) instead of replaying the + // same rejected token from cache. + if isUnauthorized401(err) && hasOAuthCreds { + if inv, ok := l.mcpOAuthTokenProvider.(interface { + InvalidateCache(uuid.UUID, string) + }); ok { + inv.InvalidateCache(srv.ID, userID) + slog.Warn("mcp.oauth_token_cache_purged", "server", srv.Name, "user", userID, "tenant", l.tenantID, "reason", "unauthorized_401") + } + } slog.Warn("mcp.user_pool_acquire_failed", "server", srv.Name, "user", userID, "error", err) continue } diff --git a/internal/agent/loop_mcp_user_test.go b/internal/agent/loop_mcp_user_test.go index 26ad8398..4720a97d 100644 --- a/internal/agent/loop_mcp_user_test.go +++ b/internal/agent/loop_mcp_user_test.go @@ -1,6 +1,137 @@ package agent -import "testing" +import ( + "context" + "encoding/json" + "sync" + "testing" + "time" + + "github.com/google/uuid" + mcpbridge "github.com/nextlevelbuilder/goclaw/internal/mcp" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// ---- mock helpers for getUserMCPTools OAuth tests ---- + +// minimalMCPServerStore implements store.MCPServerStore with panics for all +// methods except GetUserCredentials, which is the only method called in +// getUserMCPTools. +type minimalMCPServerStore struct { + userCreds *store.MCPUserCredentials + userCredsErr error + getUserCredsCalls int +} + +func (m *minimalMCPServerStore) GetUserCredentials(_ context.Context, _ uuid.UUID, _ string) (*store.MCPUserCredentials, error) { + m.getUserCredsCalls++ + return m.userCreds, m.userCredsErr +} + +func (m *minimalMCPServerStore) CreateServer(_ context.Context, _ *store.MCPServerData) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) GetServer(_ context.Context, _ uuid.UUID) (*store.MCPServerData, error) { + panic("not implemented") +} +func (m *minimalMCPServerStore) GetServerByName(_ context.Context, _ string) (*store.MCPServerData, error) { + panic("not implemented") +} +func (m *minimalMCPServerStore) ListServers(_ context.Context) ([]store.MCPServerData, error) { + panic("not implemented") +} +func (m *minimalMCPServerStore) UpdateServer(_ context.Context, _ uuid.UUID, _ map[string]any) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) DeleteServer(_ context.Context, _ uuid.UUID) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) GrantToAgent(_ context.Context, _ *store.MCPAgentGrant) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) RevokeFromAgent(_ context.Context, _, _ uuid.UUID) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) ListAgentGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { + panic("not implemented") +} +func (m *minimalMCPServerStore) ListServerGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { + panic("not implemented") +} +func (m *minimalMCPServerStore) GrantToUser(_ context.Context, _ *store.MCPUserGrant) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) RevokeFromUser(_ context.Context, _ uuid.UUID, _ string) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) CountAgentGrantsByServer(_ context.Context) (map[uuid.UUID]int, error) { + panic("not implemented") +} +func (m *minimalMCPServerStore) ListAccessible(_ context.Context, _ uuid.UUID, _ string) ([]store.MCPAccessInfo, error) { + panic("not implemented") +} +func (m *minimalMCPServerStore) CreateRequest(_ context.Context, _ *store.MCPAccessRequest) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) ListPendingRequests(_ context.Context) ([]store.MCPAccessRequest, error) { + panic("not implemented") +} +func (m *minimalMCPServerStore) ReviewRequest(_ context.Context, _ uuid.UUID, _ bool, _, _ string) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) SetUserCredentials(_ context.Context, _ uuid.UUID, _ string, _ store.MCPUserCredentials) error { + panic("not implemented") +} +func (m *minimalMCPServerStore) DeleteUserCredentials(_ context.Context, _ uuid.UUID, _ string) error { + panic("not implemented") +} + +// recordingOAuthProvider records calls to GetValidToken for assertion. +type recordingOAuthProvider struct { + mu sync.Mutex + callCount int + returnToken string + returnErr error +} + +func (r *recordingOAuthProvider) GetValidToken(_ context.Context, _ uuid.UUID, _ uuid.UUID, _ string) (string, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.callCount++ + return r.returnToken, r.returnErr +} + +// newTestPool creates a real *mcpbridge.Pool with short timeouts for unit tests. +// The pool's AcquireUser will fail (no real MCP server), but token injection +// happens before the pool call. +func newTestPool(t *testing.T) *mcpbridge.Pool { + t.Helper() + pool := mcpbridge.NewPool(mcpbridge.PoolConfig{ + MaxSize: 10, + UserAcquireTimeout: 10 * time.Millisecond, + }) + t.Cleanup(func() { pool.Stop() }) + return pool +} + +// oauthSettings returns JSON settings with auth_type: oauth. +func oauthSettings() json.RawMessage { + return json.RawMessage(`{"oauth":{"auth_type":"oauth"}}`) +} + +// oauthServerData returns a minimal MCPServerData for an OAuth-configured HTTP +// server. Transport=http + unreachable URL causes AcquireUser to fail quickly +// (connection refused) without panicking — unlike stdio with empty command +// which would spawn a goroutine reading from a nil pipe. +func oauthServerData() store.MCPServerData { + return store.MCPServerData{ + BaseModel: store.BaseModel{ID: uuid.New()}, + Name: "oauth-server-" + uuid.New().String()[:8], + Transport: "http", + URL: "http://127.0.0.1:1", // port 1 = connection refused immediately + Settings: oauthSettings(), + } +} // TestResolveActorUserID locks the actor-vs-context user-id resolution semantics // that gate per-user MCP credential lookup (and other per-actor resources). @@ -159,3 +290,185 @@ func TestResolveActorUserID(t *testing.T) { }) } } + +// ---- getUserMCPTools OAuth token injection tests ---- +// +// These tests verify that mcpOAuthTokenProvider.GetValidToken is called (or +// not) based on whether the MCP server settings declare auth_type=oauth. +// Pool.AcquireUser fails (no real MCP server), but GetValidToken is injected +// BEFORE the pool call, so we can assert on call count without needing a +// working MCP connection. + +// TestGetUserMCPToolsOAuthTokenProviderCalled verifies that when a server's +// settings declare auth_type=oauth, GetValidToken is called for the user. +func TestGetUserMCPToolsOAuthTokenProviderCalled(t *testing.T) { + pool := newTestPool(t) + provider := &recordingOAuthProvider{returnToken: "test-bearer-token"} + + ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) + defer cancel() + + l := &Loop{ + tenantID: uuid.New(), + mcpPool: pool, + mcpStore: &minimalMCPServerStore{}, + mcpUserCredSrvs: []store.MCPAccessInfo{{Server: oauthServerData()}}, + mcpOAuthTokenProvider: provider, + } + + _ = l.getUserMCPTools(ctx, "user-1") + + provider.mu.Lock() + callCount := provider.callCount + provider.mu.Unlock() + + if callCount == 0 { + t.Error("expected GetValidToken to be called for OAuth-configured server, but it was not") + } +} + +// TestGetUserMCPToolsOAuthTokenNotCalledForNonOAuth verifies that when a +// server has no OAuth settings, the token provider is never invoked and the +// server is skipped (neither static nor OAuth creds). +func TestGetUserMCPToolsOAuthTokenNotCalledForNonOAuth(t *testing.T) { + pool := newTestPool(t) + provider := &recordingOAuthProvider{} + + l := &Loop{ + tenantID: uuid.New(), + mcpPool: pool, + mcpStore: &minimalMCPServerStore{}, + mcpUserCredSrvs: []store.MCPAccessInfo{ + {Server: store.MCPServerData{ + BaseModel: store.BaseModel{ID: uuid.New()}, + Name: "plain-server", + Transport: "http", + URL: "http://127.0.0.1:1", + // No settings → auth_type defaults to "" (not "oauth") + }}, + }, + mcpOAuthTokenProvider: provider, + } + + _ = l.getUserMCPTools(context.Background(), "user-1") + + provider.mu.Lock() + callCount := provider.callCount + provider.mu.Unlock() + + if callCount != 0 { + t.Errorf("GetValidToken must not be called for non-OAuth server, called %d times", callCount) + } +} + +// TestGetUserMCPToolsOAuthNoTokenSkipsServer verifies that when GetValidToken +// returns no valid token (expired / not authorized yet), the OAuth server is +// SKIPPED entirely — it must NOT fall back to the shared server-level +// headers/api_key as Authorization (which would let an unauthorized user call +// tools with the common credential). GetValidToken is still consulted (proving +// we entered the OAuth branch) but no tools are produced. +func TestGetUserMCPToolsOAuthNoTokenSkipsServer(t *testing.T) { + pool := newTestPool(t) + expiredErr := errTokenExpiredForTest("oauth token expired") + provider := &recordingOAuthProvider{ + returnToken: "", + returnErr: expiredErr, + } + + ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) + defer cancel() + + // Server carries a server-level API key — must NOT be used as a fallback. + srv := oauthServerData() + srv.APIKey = "shared-server-key" + + l := &Loop{ + tenantID: uuid.New(), + mcpPool: pool, + mcpStore: &minimalMCPServerStore{}, + mcpUserCredSrvs: []store.MCPAccessInfo{{Server: srv}}, + mcpOAuthTokenProvider: provider, + } + + result := l.getUserMCPTools(ctx, "user-1") + if result != nil { + t.Errorf("expected nil tools when OAuth has no token (skip, no fallback), got %d tools", len(result)) + } + provider.mu.Lock() + calls := provider.callCount + provider.mu.Unlock() + if calls == 0 { + t.Error("expected GetValidToken to be consulted before skipping the OAuth server") + } +} + +// TestGetUserMCPToolsOAuthIgnoresStaticCreds verifies that for an OAuth server +// the user's static per-user credentials are NOT consulted or used: even when the +// user has set static creds, an OAuth server without a valid token is skipped +// (the OAuth token is the sole credential). This closes the bypass where a user +// with static headers could call OAuth tools without authorizing. +func TestGetUserMCPToolsOAuthIgnoresStaticCreds(t *testing.T) { + pool := newTestPool(t) + provider := &recordingOAuthProvider{returnToken: ""} // not authorized yet + + ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) + defer cancel() + + // Store has valid static creds for the user — must be ignored for OAuth servers. + st := &minimalMCPServerStore{ + userCreds: &store.MCPUserCredentials{ + APIKey: "user-static-key", + Headers: map[string]string{"Authorization": "Bearer user-static-key"}, + }, + } + + l := &Loop{ + tenantID: uuid.New(), + mcpPool: pool, + mcpStore: st, + mcpUserCredSrvs: []store.MCPAccessInfo{{Server: oauthServerData()}}, + mcpOAuthTokenProvider: provider, + } + + result := l.getUserMCPTools(ctx, "user-1") + if result != nil { + t.Errorf("expected nil tools: OAuth server without token must be skipped even with static creds, got %d tools", len(result)) + } + if st.getUserCredsCalls != 0 { + t.Errorf("static GetUserCredentials must NOT be consulted for OAuth servers, called %d times", st.getUserCredsCalls) + } + provider.mu.Lock() + calls := provider.callCount + provider.mu.Unlock() + if calls == 0 { + t.Error("expected GetValidToken to be consulted for the OAuth server") + } +} + +// TestGetUserMCPToolsNilPoolEarlyReturn verifies that a nil pool causes the +// function to return immediately without calling the token provider. +func TestGetUserMCPToolsNilPoolEarlyReturn(t *testing.T) { + provider := &recordingOAuthProvider{} + + l := &Loop{ + tenantID: uuid.New(), + mcpPool: nil, // triggers early return + mcpStore: &minimalMCPServerStore{}, + mcpUserCredSrvs: []store.MCPAccessInfo{{Server: oauthServerData()}}, + mcpOAuthTokenProvider: provider, + } + + result := l.getUserMCPTools(context.Background(), "user-1") + if result != nil { + t.Errorf("expected nil when pool is nil, got %v", result) + } + if provider.callCount != 0 { + t.Error("GetValidToken must not be called when pool is nil (early return path)") + } +} + +// errTokenExpiredForTest is a simple error type used to simulate ErrTokenExpired +// without importing the oauth package (avoids potential import cycles in tests). +type errTokenExpiredForTest string + +func (e errTokenExpiredForTest) Error() string { return string(e) } diff --git a/internal/agent/loop_types.go b/internal/agent/loop_types.go index 991a2f22..f0be3305 100644 --- a/internal/agent/loop_types.go +++ b/internal/agent/loop_types.go @@ -139,11 +139,12 @@ type Loop struct { userSetups sync.Map // userID → *userSetup (workspace + seeding state, per Loop instance) // Per-user MCP tools: servers requiring user credentials get connected per-request. - mcpStore store.MCPServerStore // for credential lookup - mcpPool *mcpbridge.Pool // user-keyed connection pool - mcpUserCredSrvs []store.MCPAccessInfo // servers needing per-user creds - mcpUserTools sync.Map // userID → []tools.Tool (cached per-user tools) - mcpGrantChecker mcpbridge.GrantChecker // runtime grant verification (nil = skip) + mcpStore store.MCPServerStore // for credential lookup + mcpPool *mcpbridge.Pool // user-keyed connection pool + mcpUserCredSrvs []store.MCPAccessInfo // servers needing per-user creds + mcpUserTools sync.Map // userID → []tools.Tool (cached per-user tools) + mcpGrantChecker mcpbridge.GrantChecker // runtime grant verification (nil = skip) + mcpOAuthTokenProvider mcpbridge.OAuthTokenProvider // OAuth Bearer token injection (nil = disabled) // Compaction config (memory flush settings) compactionCfg *config.CompactionConfig @@ -448,10 +449,11 @@ type LoopConfig struct { MemoryStore store.MemoryStore // Per-user MCP tools (servers requiring per-user credentials) - MCPStore store.MCPServerStore // for credential lookup - MCPPool *mcpbridge.Pool // user-keyed connection pool - MCPUserCredSrvs []store.MCPAccessInfo // servers needing per-user creds - MCPGrantChecker mcpbridge.GrantChecker // runtime grant verification (nil = skip) + MCPStore store.MCPServerStore // for credential lookup + MCPPool *mcpbridge.Pool // user-keyed connection pool + MCPUserCredSrvs []store.MCPAccessInfo // servers needing per-user creds + MCPGrantChecker mcpbridge.GrantChecker // runtime grant verification (nil = skip) + MCPOAuthTokenProvider mcpbridge.OAuthTokenProvider // OAuth Bearer token injection (nil = disabled) // V3 orchestration mode (resolved by resolver, controls tool visibility) OrchMode OrchestrationMode @@ -594,6 +596,7 @@ func NewLoop(cfg LoopConfig) *Loop { mcpPool: cfg.MCPPool, mcpUserCredSrvs: cfg.MCPUserCredSrvs, mcpGrantChecker: cfg.MCPGrantChecker, + mcpOAuthTokenProvider: cfg.MCPOAuthTokenProvider, orchMode: cfg.OrchMode, delegateTargets: cfg.DelegateTargets, evolutionMetricsStore: cfg.EvolutionMetricsStore, diff --git a/internal/agent/resolver.go b/internal/agent/resolver.go index 12298249..7c1e649a 100644 --- a/internal/agent/resolver.go +++ b/internal/agent/resolver.go @@ -84,6 +84,9 @@ type ResolverDeps struct { // MCP grant checker — for runtime grant verification at BridgeTool.Execute MCPGrantChecker mcpbridge.GrantChecker + // MCP OAuth token provider — injects Bearer tokens for OAuth-enabled MCP servers + MCPOAuthTokenProvider mcpbridge.OAuthTokenProvider + // Skill access store — for per-agent skill visibility filtering SkillAccessStore store.SkillAccessStore SkillStore store.SkillStore @@ -319,6 +322,9 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc { if deps.MCPGrantChecker != nil { mcpOpts = append(mcpOpts, mcpbridge.WithGrantChecker(deps.MCPGrantChecker)) } + if deps.MCPOAuthTokenProvider != nil { + mcpOpts = append(mcpOpts, mcpbridge.WithOAuthTokenProvider(deps.MCPOAuthTokenProvider)) + } mcpMgr := mcpbridge.NewManager(toolsReg, mcpOpts...) if err := mcpMgr.LoadForAgent(ctx, ag.ID, ""); err != nil { slog.Warn("failed to load MCP servers for agent", "agent", agentKey, "error", err) @@ -543,6 +549,7 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc { MCPPool: deps.MCPPool, MCPUserCredSrvs: mcpUserCredSrvs, MCPGrantChecker: deps.MCPGrantChecker, + MCPOAuthTokenProvider: deps.MCPOAuthTokenProvider, OrchMode: orchMode, DelegateTargets: delegateTargets, EvolutionMetricsStore: evoMetricsStore, diff --git a/internal/config/config_channels.go b/internal/config/config_channels.go index 51cf2b62..c95ea267 100644 --- a/internal/config/config_channels.go +++ b/internal/config/config_channels.go @@ -428,6 +428,7 @@ type GatewayConfig struct { TaskRecoveryIntervalSec int `json:"task_recovery_interval_sec,omitempty"` // team task recovery ticker interval in seconds (default 300 = 5min) BackgroundProvider string `json:"background_provider,omitempty"` // LLM provider for background workers (vault enrichment, consolidation) BackgroundModel string `json:"background_model,omitempty"` // LLM model for background workers + PublicURL string `json:"public_url,omitempty"` // public base URL for OAuth callbacks (e.g. "https://goclaw.example.com") } // ToolsConfig controls tool availability, policy, and web search. diff --git a/internal/gateway/event_filter.go b/internal/gateway/event_filter.go index 3d1dacda..202ae5df 100644 --- a/internal/gateway/event_filter.go +++ b/internal/gateway/event_filter.go @@ -108,6 +108,15 @@ func clientCanReceiveEvent(c *Client, event bus.Event) bool { return false // non-admin clients don't receive these } + // MCP OAuth completion: deliver to the user who initiated the flow. + // Admins already receive this via the admin check above. + if event.Name == protocol.EventMCPOAuthComplete { + if uid := extractMapField(event.Payload, "initiatingUserId"); uid != "" { + return uid == c.userID + } + return false + } + // Exec approval events: scoped to the requesting user. if strings.HasPrefix(event.Name, "exec.approval.") { if uid := extractMapField(event.Payload, "userId"); uid != "" { diff --git a/internal/gateway/event_filter_test.go b/internal/gateway/event_filter_test.go index 88c2edf9..9dd897d2 100644 --- a/internal/gateway/event_filter_test.go +++ b/internal/gateway/event_filter_test.go @@ -251,3 +251,70 @@ func TestExtractMapField_StructPayload_JSONFallback(t *testing.T) { t.Errorf("extractMapField JSON fallback = %q, want %q", got, "from-struct") } } + +// ---- MCP OAuth Complete events ---- + +func TestClientCanReceiveEvent_MCPOAuthComplete_RoutedToInitiatingUser(t *testing.T) { + userA := makeClient(permissions.RoleOperator, "user-a", masterTenant) + userB := makeClient(permissions.RoleOperator, "user-b", masterTenant) + + evt := makeEvent(protocol.EventMCPOAuthComplete, masterTenant, map[string]any{ + "serverId": "server-1", + "initiatingUserId": "user-a", + "status": "success", + }) + + if !clientCanReceiveEvent(userA, evt) { + t.Error("initiating user should receive mcp.oauth_complete") + } + if clientCanReceiveEvent(userB, evt) { + t.Error("non-initiating user should NOT receive mcp.oauth_complete") + } +} + +func TestClientCanReceiveEvent_MCPOAuthComplete_NotLeakAcrossTenant(t *testing.T) { + // Same user ID, different tenants. + userAMaster := makeClient(permissions.RoleOperator, "user-a", masterTenant) + userAOther := makeClient(permissions.RoleOperator, "user-a", otherTenant) + + evt := makeEvent(protocol.EventMCPOAuthComplete, masterTenant, map[string]any{ + "serverId": "server-1", + "initiatingUserId": "user-a", + "status": "success", + }) + + if !clientCanReceiveEvent(userAMaster, evt) { + t.Error("user-a in masterTenant should receive the event they initiated") + } + if clientCanReceiveEvent(userAOther, evt) { + t.Error("user-a in otherTenant must NOT receive masterTenant event (tenant isolation)") + } +} + +func TestClientCanReceiveEvent_MCPOAuthComplete_AdminReceivesInTenant(t *testing.T) { + admin := makeClient(permissions.RoleAdmin, "admin", masterTenant) + + evt := makeEvent(protocol.EventMCPOAuthComplete, masterTenant, map[string]any{ + "serverId": "server-1", + "initiatingUserId": "other-user", + "status": "success", + }) + + if !clientCanReceiveEvent(admin, evt) { + t.Error("admin should receive mcp.oauth_complete events for any user in their tenant") + } +} + +func TestClientCanReceiveEvent_MCPOAuthComplete_NoInitiatingUserIDBlocked(t *testing.T) { + c := makeClient(permissions.RoleOperator, "user-x", masterTenant) + + // Event with no initiatingUserId — blocked for non-admin (fail-closed). + evt := makeEvent(protocol.EventMCPOAuthComplete, masterTenant, map[string]any{ + "serverId": "server-1", + "status": "success", + }) + + if clientCanReceiveEvent(c, evt) { + t.Error("mcp.oauth_complete without initiatingUserId should be blocked for non-admin (fail-closed)") + } +} diff --git a/internal/gateway/server.go b/internal/gateway/server.go index 717f7a45..92ca99be 100644 --- a/internal/gateway/server.go +++ b/internal/gateway/server.go @@ -471,6 +471,7 @@ func (s *Server) SetWakeHandler(h *httpapi.WakeHandler) { s.handlers = append(s. // SetMCPHandler sets the MCP server management handler. func (s *Server) SetMCPHandler(h *httpapi.MCPHandler) { s.handlers = append(s.handlers, h) } +func (s *Server) SetMCPOAuthHandler(h *httpapi.MCPOAuthHandler) { s.handlers = append(s.handlers, h) } func (s *Server) SetMCPUserCredentialsHandler(h *httpapi.MCPUserCredentialsHandler) { s.handlers = append(s.handlers, h) } diff --git a/internal/http/mcp.go b/internal/http/mcp.go index 497ef0f5..0d15eaee 100644 --- a/internal/http/mcp.go +++ b/internal/http/mcp.go @@ -1,10 +1,12 @@ package http import ( + "context" "database/sql" "encoding/json" "log/slog" "net/http" + "strings" "github.com/google/uuid" @@ -24,15 +26,26 @@ type MCPToolLister interface { // MCPPoolEvictor evicts pooled connections for a tenant+server (called on credential rotation). type MCPPoolEvictor interface { Evict(tenantID uuid.UUID, serverName string) + // EvictServer evicts both the shared and all per-user connections for a server. + // Called when OAuth tokens or grants change so next request reconnects with fresh credentials. + EvictServer(tenantID uuid.UUID, serverName string) } // MCPHandler handles MCP server management HTTP endpoints. type MCPHandler struct { - store store.MCPServerStore - msgBus *bus.MessageBus - mgr MCPToolLister // optional, nil when Manager not available - poolEvictor MCPPoolEvictor // optional, nil when pool not available - db *sql.DB // for export/import direct queries + store store.MCPServerStore + msgBus *bus.MessageBus + mgr MCPToolLister // optional, nil when Manager not available + poolEvictor MCPPoolEvictor // optional, nil when pool not available + db *sql.DB // for export/import direct queries + oauthProvider MCPOAuthTokenProvider // optional, nil when OAuth not configured + oauthStore store.MCPOAuthTokenStore // optional, nil when OAuth not configured +} + +// MCPOAuthTokenProvider retrieves a valid OAuth Bearer token for an MCP server. +// Mirrors mcp.OAuthTokenProvider to avoid a circular import. +type MCPOAuthTokenProvider interface { + GetValidToken(ctx context.Context, serverID, tenantID uuid.UUID, userID string) (string, error) } // NewMCPHandler creates a handler for MCP server management endpoints. @@ -43,6 +56,15 @@ func NewMCPHandler(s store.MCPServerStore, msgBus *bus.MessageBus, mgr MCPToolLi // SetPoolEvictor sets the pool evictor for credential rotation handling. func (h *MCPHandler) SetPoolEvictor(e MCPPoolEvictor) { h.poolEvictor = e } +// SetOAuthProvider wires in an OAuth token provider so the tool-list endpoint +// can inject a Bearer token when doing on-demand discovery for OAuth servers. +func (h *MCPHandler) SetOAuthProvider(p MCPOAuthTokenProvider) { h.oauthProvider = p } + +// SetOAuthStore wires in the OAuth token store so the update handler can purge +// stored tokens when a server's URL or OAuth config changes (the old tokens +// were minted for a different resource/AS and can no longer be used). +func (h *MCPHandler) SetOAuthStore(s store.MCPOAuthTokenStore) { h.oauthStore = s } + func (h *MCPHandler) emitCacheInvalidate() { if h.msgBus == nil { return @@ -194,6 +216,56 @@ func (h *MCPHandler) handleGetServer(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, srv) } +// oauthFingerprint captures the MCP-server OAuth settings that, when changed, +// invalidate previously minted tokens (different client, AS, grant, or scope). +type oauthFingerprint struct { + AuthType string + ClientID string + AuthEndpoint string + TokenEndpoint string + GrantType string + Scope string + UseDCR bool // toggling DCR changes how tokens are obtained → must purge +} + +// extractOAuthFingerprint parses settings.oauth into a comparable fingerprint. +// An empty/absent oauth block yields the zero value, so toggling OAuth on/off is +// correctly detected as a change (UpdateServer replaces the whole settings blob). +func extractOAuthFingerprint(settings json.RawMessage) oauthFingerprint { + if len(settings) == 0 { + return oauthFingerprint{} + } + var s struct { + OAuth struct { + AuthType string `json:"auth_type"` + ClientID string `json:"client_id"` + AuthEndpoint string `json:"auth_endpoint"` + TokenEndpoint string `json:"token_endpoint"` + GrantType string `json:"grant_type"` + Scope string `json:"scope"` + UseDCR *bool `json:"use_dcr"` + } `json:"oauth"` + } + if err := json.Unmarshal(settings, &s); err != nil { + return oauthFingerprint{} + } + // Absent use_dcr means discover+DCR (matching handleStart), so normalize nil→true + // to avoid a spurious purge when a legacy server gains an explicit use_dcr=true. + useDCR := true + if s.OAuth.UseDCR != nil { + useDCR = *s.OAuth.UseDCR + } + return oauthFingerprint{ + AuthType: s.OAuth.AuthType, + ClientID: s.OAuth.ClientID, + AuthEndpoint: s.OAuth.AuthEndpoint, + TokenEndpoint: s.OAuth.TokenEndpoint, + GrantType: s.OAuth.GrantType, + Scope: s.OAuth.Scope, + UseDCR: useDCR, + } +} + func (h *MCPHandler) handleUpdateServer(w http.ResponseWriter, r *http.Request) { locale := store.LocaleFromContext(r.Context()) id, err := uuid.Parse(r.PathValue("id")) @@ -273,14 +345,51 @@ func (h *MCPHandler) handleUpdateServer(w http.ResponseWriter, r *http.Request) return } - // Evict pool connections when credentials change (force reconnect with new creds) - if h.poolEvictor != nil && serverName != "" { + // When the URL or OAuth config changes, the previously minted OAuth tokens + // were issued for a different resource/AS and are no longer usable. Purge them + // (global + per-user), drop the refresher's in-memory cache, and force the pool + // to reconnect so a stale Bearer token is never replayed against the new target. + tid := store.TenantIDFromContext(r.Context()) + oauthInvalidated := false + if existingSrv != nil { + urlChanged := false + if newURL, ok := updates["url"].(string); ok && strings.TrimSpace(newURL) != existingSrv.URL { + urlChanged = true + } + oauthChanged := false + if rawSettings, ok := updates["settings"]; ok { + newSettings, _ := json.Marshal(rawSettings) + if extractOAuthFingerprint(newSettings) != extractOAuthFingerprint(existingSrv.Settings) { + oauthChanged = true + } + } + if urlChanged || oauthChanged { + if h.oauthStore != nil { + if err := h.oauthStore.DeleteServerOAuthTokens(r.Context(), id, tid); err != nil { + slog.Warn("mcp.oauth_purge_on_config_change_failed", "server_id", id, "error", err) + } + } + if inv, ok := h.oauthProvider.(interface{ InvalidateServer(uuid.UUID) }); ok { + inv.InvalidateServer(id) + } + if h.poolEvictor != nil && serverName != "" { + h.poolEvictor.EvictServer(tid, serverName) + } + oauthInvalidated = true + slog.Info("mcp.oauth_tokens_purged_on_config_change", "server_id", id, "url_changed", urlChanged, "oauth_changed", oauthChanged) + } + } + + // Evict pool connections when credentials change (force reconnect with new creds). + // Use EvictServer (shared + all per-user connections) so per-user connections that + // inherited the server-level headers/api_key as their base don't keep stale values — + // matching the OAuth-config-change path above. Skipped when EvictServer already ran. + if !oauthInvalidated && h.poolEvictor != nil && serverName != "" { _, hasKey := updates["api_key"] _, hasHeaders := updates["headers"] _, hasEnv := updates["env"] if hasKey || hasHeaders || hasEnv { - tid := store.TenantIDFromContext(r.Context()) - h.poolEvictor.Evict(tid, serverName) + h.poolEvictor.EvictServer(tid, serverName) } } diff --git a/internal/http/mcp_oauth.go b/internal/http/mcp_oauth.go new file mode 100644 index 00000000..4f2b15fb --- /dev/null +++ b/internal/http/mcp_oauth.go @@ -0,0 +1,717 @@ +package http + +import ( + "context" + "encoding/json" + "fmt" + "html" + "log/slog" + "net/http" + "strings" + "time" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/bus" + "github.com/nextlevelbuilder/goclaw/internal/i18n" + mcpoauth "github.com/nextlevelbuilder/goclaw/internal/mcp/oauth" + "github.com/nextlevelbuilder/goclaw/internal/permissions" + "github.com/nextlevelbuilder/goclaw/internal/security" + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/pkg/protocol" +) + +// MCPOAuthHandler exposes OAuth endpoints for MCP server authentication. +type MCPOAuthHandler struct { + mcpStore store.MCPServerStore + oauthStore store.MCPOAuthTokenStore + discoverer *mcpoauth.Discoverer + flowMgr *mcpoauth.FlowManager + refresher *mcpoauth.Refresher + eventBus bus.EventPublisher + publicURL string // e.g. "https://goclaw.example.com" + port int + evictor MCPPoolEvictor + // tenantStore gates tenant-scoped writes (token mint/revoke/admin status) with a + // tenant-admin membership check — RoleAdmin alone is not a tenant check. + tenantStore store.TenantStore +} + +// MCPOAuthHandlerDeps contains all dependencies for the OAuth handler. +type MCPOAuthHandlerDeps struct { + MCPStore store.MCPServerStore + OAuthStore store.MCPOAuthTokenStore + Discoverer *mcpoauth.Discoverer + FlowMgr *mcpoauth.FlowManager + Refresher *mcpoauth.Refresher + EventBus bus.EventPublisher + PublicURL string + Port int + Evictor MCPPoolEvictor + TenantStore store.TenantStore +} + +// NewMCPOAuthHandler creates an MCPOAuthHandler. +func NewMCPOAuthHandler(deps MCPOAuthHandlerDeps) *MCPOAuthHandler { + return &MCPOAuthHandler{ + mcpStore: deps.MCPStore, + oauthStore: deps.OAuthStore, + discoverer: deps.Discoverer, + flowMgr: deps.FlowMgr, + refresher: deps.Refresher, + eventBus: deps.EventBus, + publicURL: deps.PublicURL, + port: deps.Port, + evictor: deps.Evictor, + tenantStore: deps.TenantStore, + } +} + +// SetEvictor sets the MCP pool evictor (called after construction when pool is available). +func (h *MCPOAuthHandler) SetEvictor(e MCPPoolEvictor) { h.evictor = e } + +// RegisterRoutes registers all MCP OAuth routes on the given mux. +func (h *MCPOAuthHandler) RegisterRoutes(mux *http.ServeMux) { + // Authorization is scope-based, mirroring the per-user MCP credentials flow + // (resolveTargetUserID in mcp_user_credentials.go): start/status/revoke are + // open to any authenticated user, and each handler then calls authorizeOAuthScope — + // a user may manage their OWN per-user token (self-service, matching the + // MCPUserCredentialsDialog "Authorize" button), while the global/server token + // (user_id="") and other users' tokens require tenant-admin. RoleAdmin alone is + // not a tenant check; SQL scoping (WHERE tenant_id=$N) is the final layer. + // discover only previews AS metadata for a server config — keep it admin-only. + mux.HandleFunc("POST /v1/mcp/oauth/start", requireAuth("", h.handleStart)) + mux.HandleFunc("GET /v1/mcp/oauth/callback", h.handleCallback) + mux.HandleFunc("GET /v1/mcp/oauth/status/{id}", requireAuth("", h.handleStatus)) + mux.HandleFunc("DELETE /v1/mcp/oauth/token/{id}", requireAuth("", h.handleRevoke)) + mux.HandleFunc("POST /v1/mcp/oauth/discover/{id}", requireAuth(permissions.RoleAdmin, h.handleDiscover)) +} + +// callbackURL derives the OAuth redirect URI from the incoming request. +// +// Priority: +// 1. Config public_url — explicit admin override (wins when set) +// 2. X-Forwarded-Proto + X-Forwarded-Host — nginx / reverse proxy +// 3. r.Host — the backend's actual host:port (correct for local dev even when +// the frontend runs on a different port than the backend) +// 4. localhost:port — final fallback +// +// Using r.Host instead of the Origin header is intentional: Origin carries the +// *frontend* origin (e.g. Vite dev server on :5173), which differs from the +// backend address that OAuth providers must redirect back to. +func (h *MCPOAuthHandler) callbackURL(r *http.Request) string { + const path = "/v1/mcp/oauth/callback" + + // 1. Explicit config override (highest priority). + if h.publicURL != "" { + return strings.TrimRight(h.publicURL, "/") + path + } + + // 2. Nginx / reverse-proxy forwarded headers. + if fwdHost := r.Header.Get("X-Forwarded-Host"); fwdHost != "" { + proto := r.Header.Get("X-Forwarded-Proto") + if proto == "" { + proto = "https" + } + return proto + "://" + strings.TrimRight(fwdHost, "/") + path + } + + // 3. Backend's own host:port from the request (works for localhost:18790 and + // any custom host — correct even in split-port dev setups). + if host := r.Host; host != "" { + scheme := "http" + if r.TLS != nil { + scheme = "https" + } + return scheme + "://" + host + path + } + + // 4. Fallback: localhost + configured port. + port := h.port + if port == 0 { + port = 8080 + } + return fmt.Sprintf("http://localhost:%d%s", port, path) +} + +// --- POST /v1/mcp/oauth/start --- + +type startOAuthReq struct { + ServerID string `json:"server_id"` + MCPURL string `json:"mcp_url"` + UserID string `json:"user_id,omitempty"` // empty = global token +} + +type startOAuthResp struct { + AuthURL string `json:"auth_url"` + State string `json:"state"` + ClientID string `json:"client_id"` + Issuer string `json:"issuer"` + Completed bool `json:"completed,omitempty"` // true for client_credentials — token already minted, no redirect needed +} + +// authorizeOAuthScope enforces who may operate on a given OAuth token scope. +// A per-user token (non-empty targetUserID) may be managed by its owner without +// admin rights — self-service, matching the per-user MCP credentials flow and the +// MCPUserCredentialsDialog "Authorize" button. The global/server token +// (targetUserID="") and another user's per-user token require tenant-admin, so a +// RoleAdmin who is not a tenant admin cannot mint/revoke tenant-scoped tokens. +func (h *MCPOAuthHandler) authorizeOAuthScope(w http.ResponseWriter, r *http.Request, targetUserID string) bool { + // Self-service: a caller may manage their own per-user token. Only applies to a + // non-empty target (the global/server token is shared, never "self"). + if targetUserID != "" { + if callerID := store.UserIDFromContext(r.Context()); callerID != "" && targetUserID == callerID { + return true + } + } + // Global token, or a per-user token on behalf of another user → tenant-admin. + return requireTenantAdmin(w, r, h.tenantStore) +} + +func (h *MCPOAuthHandler) handleStart(w http.ResponseWriter, r *http.Request) { + var req startOAuthReq + if !bindJSON(w, r, store.LocaleFromContext(r.Context()), &req) { + return + } + if !h.authorizeOAuthScope(w, r, req.UserID) { + return + } + if req.ServerID == "" || req.MCPURL == "" { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "server_id and mcp_url are required"}) + return + } + + serverID, err := uuid.Parse(req.ServerID) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid server_id"}) + return + } + + ctx := r.Context() + tenantID := store.TenantIDFromContext(ctx) + + // Verify server belongs to tenant. + srv, err := h.mcpStore.GetServer(ctx, serverID) + if err != nil || srv == nil { + writeJSON(w, http.StatusNotFound, map[string]string{"error": "MCP server not found"}) + return + } + + // Parse server OAuth settings up front — they decide whether to auto-discover the + // AS + dynamically register (use_dcr) or to use operator-supplied endpoints. + // use_dcr is a pointer so a legacy/absent value (nil) keeps the discover+DCR path. + var oauthSettings struct { + OAuth struct { + UseDCR *bool `json:"use_dcr"` + AuthEndpoint string `json:"auth_endpoint"` + TokenEndpoint string `json:"token_endpoint"` + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + Scope string `json:"scope"` + GrantType string `json:"grant_type"` + } `json:"oauth"` + } + if len(srv.Settings) > 0 { + _ = json.Unmarshal(srv.Settings, &oauthSettings) + } + + safeClient := security.NewSafeClient(15 * time.Second) + callbackURI := h.callbackURL(r) + + var disc *mcpoauth.DiscoveryResult + if oauthSettings.OAuth.UseDCR != nil && !*oauthSettings.OAuth.UseDCR { + // Manual mode: the operator disabled DCR and supplied the AS endpoints + // directly. We do NOT auto-discover or dynamically register. The endpoints + // are SSRF-validated because the token endpoint is dialed server-side and the + // authorization endpoint is handed to the browser. A manual client_id is + // required (enforced by the no-client_id guard below, since no DCR runs). + ae := strings.TrimSpace(oauthSettings.OAuth.AuthEndpoint) + te := strings.TrimSpace(oauthSettings.OAuth.TokenEndpoint) + // The token endpoint is always required (it is dialed for every grant). + // The authorization endpoint is only needed for browser/auth-code grants — + // client_credentials has no authorization redirect, matching the UI which + // hides the authorization URL field for that grant. + if te == "" { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "manual OAuth (use_dcr=false) requires token_endpoint"}) + return + } + if oauthSettings.OAuth.GrantType != "client_credentials" && ae == "" { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "manual OAuth (use_dcr=false) requires auth_endpoint for this grant type"}) + return + } + // SSRF-validate the token endpoint (always dialed) and the authorization + // endpoint when present (handed to the browser). + if _, _, verr := security.Validate(te); verr != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid token_endpoint: " + verr.Error()}) + return + } + if ae != "" { + if _, _, verr := security.Validate(ae); verr != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid auth_endpoint: " + verr.Error()}) + return + } + } + disc = &mcpoauth.DiscoveryResult{AuthorizationEndpoint: ae, TokenEndpoint: te} + } else { + // Discovery path. Discovery and the resulting resource indicator must bind to + // the server's own registered URL, not a client-supplied one. Trusting + // req.MCPURL would let the caller mint a token whose resource_uri / AS differ + // from the server the agent actually connects to. Fall back to req.MCPURL only + // when the server has no stored URL (legacy rows). + mcpURL := srv.URL + if mcpURL == "" { + mcpURL = req.MCPURL + } + // Reuse the shared, cache-backed discoverer when wired (5-min metadata cache); + // fall back to a request-local one only when none was injected (e.g. tests). + discoverer := h.discoverer + if discoverer == nil { + discoverer = mcpoauth.NewDiscoverer(safeClient) + } + var derr error + disc, derr = discoverer.Discover(ctx, mcpURL) + if derr != nil { + slog.Warn("mcpoauth.discover_failed", "server_id", serverID, "url", mcpURL, "error", derr) + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "OAuth discovery failed: " + derr.Error()}) + return + } + } + + // Determine client credentials — priority: + // 1. Manual client_id in server settings (DCR OFF): use as-is. + // 2. DCR: always register fresh — avoids reusing stale credentials when MCP URL changes. + // 3. Fallback: existing token credentials (legacy / no-DCR servers without manual config). + clientID := oauthSettings.OAuth.ClientID + clientSecret := oauthSettings.OAuth.ClientSecret + + if clientID == "" { + if disc.RegistrationEndpoint != "" { + dcrResp, err2 := mcpoauth.RegisterClient(ctx, safeClient, disc.RegistrationEndpoint, callbackURI) + if err2 != nil { + slog.Warn("mcpoauth.dcr_failed", "server_id", serverID, "error", err2) + writeJSON(w, http.StatusBadGateway, map[string]string{"error": "Dynamic Client Registration failed: " + err2.Error()}) + return + } + clientID = dcrResp.ClientID + clientSecret = dcrResp.ClientSecret + } else { + // No DCR endpoint — fall back to existing token's credentials if available. + var existingTok *store.MCPOAuthToken + if req.UserID == "" { + existingTok, _ = h.oauthStore.GetOAuthToken(ctx, serverID, tenantID) + } else { + existingTok, _ = h.oauthStore.GetUserOAuthToken(ctx, serverID, tenantID, req.UserID) + } + if existingTok != nil { + clientID = existingTok.DCRClientID + clientSecret = existingTok.DCRClientSecret + } + } + } + + if clientID == "" { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "no client_id: DCR not supported and no manual credentials configured"}) + return + } + + // Scope: use manual setting, fall back to AS-advertised scopes when DCR is used. + scopes := oauthSettings.OAuth.Scope + if scopes == "" && len(disc.ScopesSupported) > 0 { + scopes = strings.Join(disc.ScopesSupported, " ") + } + + // client_credentials is a non-interactive grant: there is no browser redirect. + // Obtain the token server-side now and persist it, returning Completed=true so + // the UI skips the popup. Routing it through StartFlow (authorization-code) would + // produce a bogus response_type=code authorization URL. + if oauthSettings.OAuth.GrantType == "client_credentials" { + if clientSecret == "" { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "client_credentials grant requires a client_secret"}) + return + } + tokens, ccErr := h.flowMgr.ClientCredentials(ctx, disc.TokenEndpoint, clientID, clientSecret, scopes, disc.ResourceURI) + if ccErr != nil { + slog.Warn("mcpoauth.client_credentials_failed", "server_id", serverID, "error", ccErr) + writeJSON(w, http.StatusBadGateway, map[string]string{"error": "client credentials grant failed: " + ccErr.Error()}) + return + } + now := time.Now() + var expiresAt *time.Time + if tokens.ExpiresIn > 0 { + t := now.Add(time.Duration(tokens.ExpiresIn) * time.Second) + expiresAt = &t + } + tok := &store.MCPOAuthToken{ + ServerID: serverID, + TenantID: tenantID, + UserID: req.UserID, + AccessToken: tokens.AccessToken, + RefreshToken: tokens.RefreshToken, + TokenType: tokens.TokenType, + Scopes: tokens.Scope, + ExpiresAt: expiresAt, + IssuedAt: &now, + DCRClientID: clientID, + DCRClientSecret: clientSecret, + DCRIssuer: disc.Issuer, + TokenEndpoint: disc.TokenEndpoint, + ResourceURI: disc.ResourceURI, + } + if err := h.activateToken(ctx, tok, srv.Name); err != nil { + slog.Error("mcpoauth.save_token", "error", err, "server_id", serverID) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "failed to save token"}) + return + } + writeJSON(w, http.StatusOK, startOAuthResp{Completed: true, ClientID: clientID, Issuer: disc.Issuer}) + return + } + + authURL, state, err := h.flowMgr.StartFlow(ctx, mcpoauth.StartFlowParams{ + ServerID: serverID, + TenantID: tenantID, + UserID: req.UserID, + InitiatingUserID: store.UserIDFromContext(ctx), + DiscoveryResult: disc, + ClientID: clientID, + ClientSecret: clientSecret, + RedirectURI: callbackURI, + Scopes: scopes, + GrantType: oauthSettings.OAuth.GrantType, + }) + if err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + return + } + + writeJSON(w, http.StatusOK, startOAuthResp{ + AuthURL: authURL, + State: state, + ClientID: clientID, + Issuer: disc.Issuer, + }) +} + +// --- GET /v1/mcp/oauth/callback --- + +func (h *MCPOAuthHandler) handleCallback(w http.ResponseWriter, r *http.Request) { + code := r.URL.Query().Get("code") + state := r.URL.Query().Get("state") + oauthErr := r.URL.Query().Get("error") + oauthErrDesc := r.URL.Query().Get("error_description") + + locale := extractLocale(r) + + if oauthErr != "" { + h.writeCallbackHTML(w, http.StatusOK, locale, false, oauthErrDesc) + return + } + if code != "" && state == "" { + // An auth code with no state is a malformed / CSRF-risk callback (state is + // the CSRF token), so reject with 400. A bare hit with neither code nor state + // (e.g. the user opening the URL directly) falls through to the 200 HTML page. + h.writeCallbackHTML(w, http.StatusBadRequest, locale, false, "missing state") + return + } + if code == "" || state == "" { + h.writeCallbackHTML(w, http.StatusOK, locale, false, "missing code or state") + return + } + + ctx := r.Context() + tokens, flow, err := h.flowMgr.ExchangeCode(ctx, state, code) + if err != nil { + slog.Warn("mcpoauth.exchange_failed", "error", err) + h.writeCallbackHTML(w, http.StatusOK, locale, false, err.Error()) + return + } + + now := time.Now() + var expiresAt *time.Time + if tokens.ExpiresIn > 0 { + t := now.Add(time.Duration(tokens.ExpiresIn) * time.Second) + expiresAt = &t + } + + tok := &store.MCPOAuthToken{ + ServerID: flow.ServerID, + TenantID: flow.TenantID, + UserID: flow.UserID, + AccessToken: tokens.AccessToken, + RefreshToken: tokens.RefreshToken, + TokenType: tokens.TokenType, + Scopes: tokens.Scope, + ExpiresAt: expiresAt, + IssuedAt: &now, + DCRClientID: flow.ClientID, + DCRClientSecret: flow.ClientSecret, + DCRIssuer: flow.Issuer, + TokenEndpoint: flow.TokenEndpoint, + ResourceURI: flow.ResourceURI, + } + srv, _ := h.mcpStore.GetServer(ctx, flow.ServerID) + serverName := "" + if srv != nil { + serverName = srv.Name + } + if err := h.activateToken(ctx, tok, serverName); err != nil { + slog.Error("mcpoauth.save_token", "error", err, "server_id", flow.ServerID) + h.publishOAuthComplete(flow, "error", "failed to save token") + h.writeCallbackHTML(w, http.StatusOK, locale, false, "failed to save token") + return + } + + // Notify the initiating WS client that OAuth completed successfully. + h.publishOAuthComplete(flow, "success", "") + h.writeCallbackHTML(w, http.StatusOK, locale, true, "") +} + +// activateToken persists a freshly obtained OAuth token and makes it live: +// it upserts the row, evicts the refresher's in-memory cache, and drops the +// pooled MCP connection (shared + per-user) so the next request reconnects +// with the new Bearer token. Shared by the authorization-code callback and +// the client_credentials path in handleStart. +func (h *MCPOAuthHandler) activateToken(ctx context.Context, tok *store.MCPOAuthToken, serverName string) error { + if err := h.oauthStore.UpsertOAuthToken(ctx, tok); err != nil { + return err + } + if h.refresher != nil { + h.refresher.InvalidateCache(tok.ServerID, tok.UserID) + } + if h.evictor != nil && serverName != "" { + h.evictor.EvictServer(tok.TenantID, serverName) + } + // Broadcast an MCP cache-invalidate so all per-user pool connections are + // evicted and agent Loop caches reload — picks up the new token for both the + // shared (global) and per-user paths on the next request. + h.emitMCPCacheInvalidate() + return nil +} + +// emitMCPCacheInvalidate broadcasts an MCP cache-invalidate event. The subscriber +// (cmd/gateway_managed.go) responds with agentRouter.InvalidateAll() + +// mcpPool.EvictAllUsers(), so freshly authorized/revoked OAuth tokens take effect +// immediately instead of waiting for idle TTL. Mirrors MCPHandler.emitCacheInvalidate. +func (h *MCPOAuthHandler) emitMCPCacheInvalidate() { + if h.eventBus == nil { + return + } + h.eventBus.Broadcast(bus.Event{ + Name: protocol.EventCacheInvalidate, + Payload: bus.CacheInvalidatePayload{Kind: bus.CacheKindMCP}, + }) +} + +func (h *MCPOAuthHandler) writeCallbackHTML(w http.ResponseWriter, statusCode int, locale string, success bool, errMsg string) { + status := "success" + if !success { + status = "error" + } + + var bodyMsg string + if success { + bodyMsg = i18n.T(locale, i18n.MsgOAuthCallbackSuccess) + } else { + bodyMsg = i18n.T(locale, i18n.MsgOAuthCallbackFailed) + } + + // Build the postMessage payload with json.Marshal. errMsg is attacker-controlled + // (reflected from the OAuth provider's `error_description` query param on this + // unauthenticated callback endpoint), so it must never be concatenated into the + // page. encoding/json escapes <, >, & to >/& by default, which + // neutralises breakout attempts inside the +
%s
+