diff --git a/apps/mesh/src/api/app.ts b/apps/mesh/src/api/app.ts index 1381912f8f..628a4532dd 100644 --- a/apps/mesh/src/api/app.ts +++ b/apps/mesh/src/api/app.ts @@ -346,6 +346,20 @@ const oauthProxyHandler: MiddlewareHandler = async (c) => { let originAuthServer: string | undefined; const connUrl = new URL(connection.connection_url); + // RFC 8707 resource indicator forwarded to the downstream authorization + // server on the authorize/token legs. Defaults to the connection's MCP + // endpoint URL — what most servers validate against (e.g. Supabase requires + // the exact endpoint). Some servers only accept the *origin* and reject a + // path-bearing resource (e.g. Pipedream returns "Invalid or unauthorized + // resource parameter" for ".../v2"). Allow a per-connection override via + // `metadata.oauthResource` for those, falling back to the connection URL. + const resourceOverride = + typeof connection.metadata?.oauthResource === "string" && + connection.metadata.oauthResource.length > 0 + ? connection.metadata.oauthResource + : undefined; + const resourceIndicator = resourceOverride ?? connection.connection_url; + if (resourceRes.ok) { // Origin has Protected Resource Metadata - use authorization_servers from it const resourceData = (await resourceRes.json()) as { @@ -441,7 +455,7 @@ const oauthProxyHandler: MiddlewareHandler = async (c) => { // Some auth servers (like Supabase) validate that the resource is their actual endpoint, // not our proxy. We keep the proxy URL for redirect_uri since that's where we handle the callback. if (targetUrl.searchParams.has("resource")) { - targetUrl.searchParams.set("resource", connection.connection_url); + targetUrl.searchParams.set("resource", resourceIndicator); } // Add smart OAuth params for deco-hosted MCPs to skip org/project selection @@ -514,7 +528,7 @@ const oauthProxyHandler: MiddlewareHandler = async (c) => { // Parse form body and rewrite resource if present const formData = await c.req.formData(); if (formData.has("resource")) { - formData.set("resource", connection.connection_url); + formData.set("resource", resourceIndicator); } const cidRaw = formData.get("client_id"); const csRaw = formData.get("client_secret"); @@ -1758,10 +1772,17 @@ export async function createApp(options: CreateAppOptions = {}) { // Require either user or API key authentication if (!meshContext.auth.user?.id && !meshContext.auth.apiKey?.id) { const url = new URL(c.req.url); + // Behind a TLS-terminating reverse proxy (e.g. Caddy/nginx) the request + // reaches us over http, so `url.origin` would advertise an http:// + // resource_metadata URL and OAuth-capable clients that require https + // (e.g. Claude) reject it. Honor X-Forwarded-Proto so the advertised + // URL matches the public scheme. + const fwdProto = c.req.header("x-forwarded-proto")?.split(",")[0]?.trim(); + const origin = fwdProto ? `${fwdProto}://${url.host}` : url.origin; return (c.res = new Response(null, { status: 401, headers: { - "WWW-Authenticate": `Bearer realm="mcp",resource_metadata="${url.origin}${url.pathname}/.well-known/oauth-protected-resource"`, + "WWW-Authenticate": `Bearer realm="mcp",resource_metadata="${origin}${url.pathname}/.well-known/oauth-protected-resource"`, }, })); } diff --git a/apps/mesh/src/api/routes/oauth-proxy.ts b/apps/mesh/src/api/routes/oauth-proxy.ts index ca6dbe5fea..5756e0e8de 100644 --- a/apps/mesh/src/api/routes/oauth-proxy.ts +++ b/apps/mesh/src/api/routes/oauth-proxy.ts @@ -13,8 +13,10 @@ */ import { Hono } from "hono"; +import { oAuthProtectedResourceMetadata } from "better-auth/plugins"; import { ContextFactory } from "../../core/context-factory"; import type { StudioContext } from "../../core/studio-context"; +import { auth } from "../../auth"; import { retry, RetryError } from "@decocms/std"; import { authorizationServerMetadataUrls, @@ -390,6 +392,20 @@ export const protectedResourceMetadataHandler = async (c: { return c.json({ error: "Connection not found" }, 404); } + // Virtual MCPs (`virtual://`) are Studio-native aggregators with no + // downstream OAuth server to proxy — trying to fetch protected-resource + // metadata from `virtual://` fails ("protocol must be http/https/s3"). Their + // OAuth resource is Studio itself: the Better Auth MCP authorization server, + // which supports Dynamic Client Registration and therefore accepts an + // external MCP client's own `redirect_uri` (e.g. Claude Desktop). The + // connection `oauth-proxy` only accepts Studio's own origin, so it can't + // serve external clients. Hand back Better Auth's metadata instead. + if (connectionUrl.startsWith("virtual://")) { + const res = await oAuthProtectedResourceMetadata(auth)(c.req.raw); + const data = await res.json(); + return Response.json(data, res); + } + const prefix = buildPathPrefix(orgSlug); const proxyResourceUrl = `${requestUrl.origin}${prefix}/mcp/${connectionId}`; // Auth-server URL (the value advertised in `authorization_servers`) stays on diff --git a/apps/mesh/src/api/routes/org-scoped.ts b/apps/mesh/src/api/routes/org-scoped.ts index c262a06e30..3db683620a 100644 --- a/apps/mesh/src/api/routes/org-scoped.ts +++ b/apps/mesh/src/api/routes/org-scoped.ts @@ -134,6 +134,18 @@ export const createOrgScopedApi = (deps: OrgScopedDeps) => { deps.betterAuthProtectedResourceHandler, ); + // Aggregate (Decopilot) MCP endpoint at `/api/:org/mcp` has no connectionId, + // so its OAuth resource is Studio itself — the Better Auth MCP authorization + // server (with Dynamic Client Registration). This lets external MCP clients + // (e.g. Claude Desktop) register their own redirect_uri and log in against + // Studio, instead of the connection `oauth-proxy` which only accepts Studio's + // own origin. Mounted BEFORE the proxy catch-all so the well-known suffix is + // not swallowed as a `:connectionId`. + app.get( + "/mcp/.well-known/oauth-protected-resource", + deps.betterAuthProtectedResourceHandler, + ); + app.route("/mcp", createVirtualMcpRoutes()); app.route("/mcp/self", createSelfRoutes()); app.route("/mcp", createProxyRoutes()); diff --git a/apps/mesh/src/api/routes/proxy.ts b/apps/mesh/src/api/routes/proxy.ts index caf6e3652a..9208c2b99b 100644 --- a/apps/mesh/src/api/routes/proxy.ts +++ b/apps/mesh/src/api/routes/proxy.ts @@ -412,6 +412,15 @@ export const createProxyRoutes = () => { if (error instanceof CircuitOpenError) { throw error; } + // Per-user authorization required is an EXPECTED state for + // `auth_mode: "per_user"` connections when the caller has no token yet + // — not a downstream outage. Return the 401 challenge (with authorize + // URL) WITHOUT tripping the breaker or auto-disabling the connection. + // Handled before recordFailure below, which would otherwise open the + // circuit and 503 the connection (hiding the OAuth the error asks for). + if (isPerUserAuthorizationRequiredError(error)) { + return renderPerUserAuthorizationRequired(error); + } // Check if this is an auth error - if so, return appropriate 401 // Note: This only applies to HTTP connections const connection = await ctx.storage.connections.findById( diff --git a/apps/mesh/src/core/context-factory.ts b/apps/mesh/src/core/context-factory.ts index 0e06651fbf..87612362a4 100644 --- a/apps/mesh/src/core/context-factory.ts +++ b/apps/mesh/src/core/context-factory.ts @@ -670,6 +670,21 @@ async function authenticateRequest( // ctx.organization on every request that doesn't target their first org. const orgIdHint = req.headers.get("x-org-id"); const orgSlugHint = req.headers.get("x-org-slug"); + // External MCP clients (Claude Desktop/Code) authenticate via OAuth and + // do NOT send x-org-* headers — the org they target is in the request + // path (`/api/:org/mcp/...`). Without honoring it, a multi-org member + // falls through to the single-membership guard below, resolves to NO + // role, and loses the admin/owner bypass (every connection tool call + // 403s "Access denied"). Derive the slug from the path as a hint. + const pathOrgSlug = (() => { + try { + const segs = new URL(req.url).pathname.split("/").filter(Boolean); + if (segs[0] === "api" && segs[1]) return decodeURIComponent(segs[1]); + } catch { + // Malformed URL — fall through to header/single-membership logic. + } + return undefined; + })(); const membership = await timings.measure("auth_query_membership", () => { const base = db @@ -695,6 +710,11 @@ async function authenticateRequest( .where("organization.slug", "=", orgSlugHint) .executeTakeFirst(); } + if (pathOrgSlug) { + return base + .where("organization.slug", "=", pathOrgSlug) + .executeTakeFirst(); + } // No org hint — only resolve when the user has exactly one membership. // For multi-org users without a hint, return undefined so callers get // no org context instead of a non-deterministic pick (the previous diff --git a/apps/mesh/src/mcp-clients/lazy-client.ts b/apps/mesh/src/mcp-clients/lazy-client.ts index bb8483eeda..b04bfd3540 100644 --- a/apps/mesh/src/mcp-clients/lazy-client.ts +++ b/apps/mesh/src/mcp-clients/lazy-client.ts @@ -29,6 +29,7 @@ import { recordSuccess, } from "./circuit-breaker"; import { clientFromConnection } from "./client"; +import { isPerUserAuthorizationRequiredError } from "./outbound/errors"; import { invalidateConnectionCaches } from "./mcp-cache-invalidation"; import { fetchWithCache, @@ -119,7 +120,14 @@ export function createLazyClient( // Clear cached promise so transient failures don't permanently // break the client — next call will retry the connection. realClientPromise = null; - recordFailure(connection.id); + // A per-user authorization prompt is an expected state for + // `auth_mode: "per_user"` connections without a token for the caller, + // NOT a downstream outage. Counting it as a failure trips the circuit + // breaker, which then 503s the connection — hiding the "Connect your + // account" UI and blocking the very OAuth the error is asking for. + if (!isPerUserAuthorizationRequiredError(err)) { + recordFailure(connection.id); + } throw err; }); } diff --git a/apps/mesh/src/web/components/details/connection/index.tsx b/apps/mesh/src/web/components/details/connection/index.tsx index cd86c90928..b43c9986f8 100644 --- a/apps/mesh/src/web/components/details/connection/index.tsx +++ b/apps/mesh/src/web/components/details/connection/index.tsx @@ -304,10 +304,19 @@ function ConnectionInspectorViewWithConnection({ }; const handleAuthenticateForId = async (connId: string) => { + // Only request scopes the connection has explicitly configured. Do NOT + // hardcode "offline_access": it's an OIDC-ism many MCP providers don't + // advertise, and passing it into Dynamic Client Registration makes strict + // servers reject the registration outright (e.g. Pipedrive returns HTTP 400 + // on /register). Refresh tokens are already requested via grant_types, so + // omitting scope lets such servers grant their default scope set. + const configuredScopes = connection.configuration_scopes?.length + ? connection.configuration_scopes.join(" ") + : undefined; const { token, tokenInfo, error } = await authenticateMcp({ connectionId: connId, orgSlug: projectOrg.slug, - scope: "offline_access", + ...(configuredScopes ? { scope: configuredScopes } : {}), }); if (error || !token) { toast.error(`Authentication failed: ${error}`); diff --git a/packages/mcp-utils/src/aggregate/gateway-client.test.ts b/packages/mcp-utils/src/aggregate/gateway-client.test.ts index f46ee9634b..688b47bd1f 100644 --- a/packages/mcp-utils/src/aggregate/gateway-client.test.ts +++ b/packages/mcp-utils/src/aggregate/gateway-client.test.ts @@ -3,10 +3,14 @@ import type { IClient } from "../client-like.ts"; import { GatewayClient, displayToolName, + namespaceCode, slugify, stripToolNamespace, } from "./gateway-client.ts"; +// Namespaced tool name for a client key, mirroring GatewayClient's scheme. +const ns = (key: string, tool: string) => `${namespaceCode(key)}_${tool}`; + function createMockClient( tools: { name: string }[] = [], resources: { uri: string; name: string }[] = [], @@ -74,7 +78,7 @@ describe("slugify", () => { describe("stripToolNamespace", () => { it("strips clientId prefix", () => { - expect(stripToolNamespace("my-conn_SOME_TOOL", "my-conn")).toBe( + expect(stripToolNamespace(ns("my-conn", "SOME_TOOL"), "my-conn")).toBe( "SOME_TOOL", ); }); @@ -90,18 +94,16 @@ describe("stripToolNamespace", () => { }); it("strips real connection ID prefix", () => { - expect( - stripToolNamespace( - "conn-dvitqc2ooobdzmrd5ky24_hello_world", - "conn-dvitqc2ooobdzmrd5ky24", - ), - ).toBe("hello_world"); + const cid = "conn-dvitqc2ooobdzmrd5ky24"; + expect(stripToolNamespace(ns(cid, "hello_world"), cid)).toBe("hello_world"); }); }); describe("displayToolName", () => { it("strips clientId prefix and formats for display", () => { - expect(displayToolName("my-conn_SOME_TOOL", "my-conn")).toBe("some tool"); + expect(displayToolName(ns("my-conn", "SOME_TOOL"), "my-conn")).toBe( + "some tool", + ); }); it("returns formatted name when no clientId", () => { @@ -111,7 +113,7 @@ describe("displayToolName", () => { describe("GatewayClient", () => { describe("tool namespacing", () => { - it("prefixes tool names with slugified client key", async () => { + it("prefixes tool names with the client key namespace code", async () => { const clientA = createMockClient([{ name: "toolA" }]); const clientB = createMockClient([{ name: "toolB" }]); @@ -123,8 +125,17 @@ describe("GatewayClient", () => { expect(result.tools).toHaveLength(2); const names = result.tools.map((t) => t.name); - expect(names).toContain("a_toolA"); - expect(names).toContain("b_toolB"); + expect(names).toContain(ns("a", "toolA")); + expect(names).toContain(ns("b", "toolB")); + }); + + it("keeps namespaced tool names short (fits 64-char limit under a second prefix)", () => { + // conn ids are ~26 chars; the namespace code must be short so a + // downstream client can add its own prefix and still stay <= 64. + expect(namespaceCode("conn_MMGhTBDv1JmlGbsdHmSn5").length).toBeLessThan( + 9, + ); + expect(namespaceCode("conn_MMGhTBDv1JmlGbsdHmSn5")).not.toContain("_"); }); it("tags tools with _meta.gatewayClientId", async () => { @@ -132,7 +143,7 @@ describe("GatewayClient", () => { const gw = new GatewayClient({ myKey: { client: clientA } }); const result = await gw.listTools(); - expect(result.tools[0].name).toBe("mykey_toolA"); + expect(result.tools[0].name).toBe(ns("myKey", "toolA")); expect((result.tools[0]._meta as any).gatewayClientId).toBe("myKey"); }); @@ -147,18 +158,15 @@ describe("GatewayClient", () => { const result = await gw.listTools(); expect(result.tools).toHaveLength(2); - expect(result.tools.map((t) => t.name)).toEqual(["a_search", "b_search"]); + expect(result.tools.map((t) => t.name)).toEqual([ + ns("a", "search"), + ns("b", "search"), + ]); }); - it("throws on duplicate slugified keys", () => { - const client = createMockClient(); - expect( - () => - new GatewayClient({ - "My Server": { client }, - "my--server": { client }, - }), - ).toThrow(/duplicate slug/); + it("gives distinct keys distinct namespace codes", () => { + expect(namespaceCode("alpha")).not.toBe(namespaceCode("beta")); + expect(namespaceCode("conn_a")).not.toBe(namespaceCode("conn_b")); }); }); @@ -168,7 +176,7 @@ describe("GatewayClient", () => { const gw = new GatewayClient({ server: { client } }); const result = await gw.listPrompts(); - expect(result.prompts[0].name).toBe("server_greet"); + expect(result.prompts[0].name).toBe(ns("server", "greet")); }); }); @@ -182,7 +190,7 @@ describe("GatewayClient", () => { b: { client: clientB }, }); - await gw.callTool({ name: "b_toolB", arguments: {} }); + await gw.callTool({ name: ns("b", "toolB"), arguments: {} }); expect(clientB.callTool).toHaveBeenCalledWith( { name: "toolB", arguments: {} }, undefined, @@ -195,7 +203,7 @@ describe("GatewayClient", () => { const client = createMockClient([{ name: "doStuff" }]); const gw = new GatewayClient({ srv: { client } }); - await gw.callTool({ name: "srv_doStuff", arguments: { x: 1 } }); + await gw.callTool({ name: ns("srv", "doStuff"), arguments: { x: 1 } }); expect(client.callTool).toHaveBeenCalledWith( { name: "doStuff", arguments: { x: 1 } }, undefined, @@ -233,7 +241,7 @@ describe("GatewayClient", () => { b: { client: clientB }, }); - await gw.getPrompt({ name: "b_promptB", arguments: {} }); + await gw.getPrompt({ name: ns("b", "promptB"), arguments: {} }); expect(clientB.getPrompt).toHaveBeenCalledWith({ name: "promptB", arguments: {}, @@ -304,7 +312,7 @@ describe("GatewayClient", () => { const result = await gw.listTools(); expect(result.tools).toHaveLength(1); - expect(result.tools[0].name).toBe("async_async_tool"); + expect(result.tools[0].name).toBe(ns("async", "async_tool")); }); }); @@ -323,9 +331,9 @@ describe("GatewayClient", () => { const result = await gw.listTools(); expect(result.tools).toHaveLength(2); const names = result.tools.map((t) => t.name); - expect(names).toContain("c_toolA"); - expect(names).toContain("c_toolC"); - expect(names).not.toContain("c_toolB"); + expect(names).toContain(ns("c", "toolA")); + expect(names).toContain(ns("c", "toolC")); + expect(names).not.toContain(ns("c", "toolB")); }); it("empty tools array blocks all tools", async () => { @@ -338,7 +346,7 @@ describe("GatewayClient", () => { }); const result = await gw.listTools(); - expect(result.tools.map((t) => t.name)).toEqual(["b_t2"]); + expect(result.tools.map((t) => t.name)).toEqual([ns("b", "t2")]); }); it("filters resources by selected URIs", async () => { @@ -386,7 +394,7 @@ describe("GatewayClient", () => { const result = await gw.listPrompts(); expect(result.prompts).toHaveLength(1); - expect(result.prompts[0].name).toBe("c_p2"); + expect(result.prompts[0].name).toBe(ns("c", "p2")); }); it("per-client selection across multiple clients", async () => { @@ -402,7 +410,10 @@ describe("GatewayClient", () => { }); const result = await gw.listTools(); - expect(result.tools.map((t) => t.name)).toEqual(["a_a1", "b_b2"]); + expect(result.tools.map((t) => t.name)).toEqual([ + ns("a", "a1"), + ns("b", "b2"), + ]); }); }); diff --git a/packages/mcp-utils/src/aggregate/gateway-client.ts b/packages/mcp-utils/src/aggregate/gateway-client.ts index b5e52f6cd8..e3ac617624 100644 --- a/packages/mcp-utils/src/aggregate/gateway-client.ts +++ b/packages/mcp-utils/src/aggregate/gateway-client.ts @@ -63,6 +63,29 @@ export function slugify(input: string): string { .replace(/^-|-$/g, ""); } +/** + * Short, stable namespace code for a connection key. + * + * The aggregated tool name is `${namespaceCode(key)}_${toolName}`. Downstream + * clients often add their OWN prefix on top (e.g. Hermes prepends + * `mcp__`, ~21 chars), and tool names are capped at 64 chars by the + * Anthropic API (`^[A-Za-z0-9_-]{1,64}$`). Using the full slugified connection + * id (~26 chars) as the prefix blew the budget once a second prefix was added. + * + * This produces a 7-char code (`a` + 6 base36 chars of an FNV-1a hash) with NO + * underscore, so `resolveToolTarget`'s split on the first `_` still cleanly + * separates the prefix from the (possibly `_`-containing) tool name. Collisions + * are astronomically unlikely for a handful of connections and, if they ever + * happen, the GatewayClient constructor throws on a duplicate code. + */ +export function namespaceCode(input: string): string { + let h = 0x811c9dc5; + for (let i = 0; i < input.length; i++) { + h = Math.imul(h ^ input.charCodeAt(i), 0x01000193); + } + return `a${(h >>> 0).toString(36).padStart(6, "0").slice(-6)}`; +} + /** * Extract `gatewayClientId` from an item's `_meta` object. * Returns `undefined` when the field is absent or not a string. @@ -89,7 +112,7 @@ export function stripToolNamespace( clientId?: string, ): string { if (!clientId) return namespacedName; - const prefix = `${slugify(clientId)}_`; + const prefix = `${namespaceCode(clientId)}_`; return namespacedName.startsWith(prefix) ? namespacedName.slice(prefix.length) : namespacedName; @@ -151,10 +174,10 @@ export class GatewayClient extends Client { }); this.clients = clients; for (const key of Object.keys(clients)) { - const slug = slugify(key); + const slug = namespaceCode(key); if (this.slugToKey.has(slug)) { throw new Error( - `GatewayClient: duplicate slug "${slug}" from keys "${this.slugToKey.get(slug)}" and "${key}"`, + `GatewayClient: duplicate namespace code "${slug}" from keys "${this.slugToKey.get(slug)}" and "${key}"`, ); } this.slugToKey.set(slug, key); @@ -166,7 +189,7 @@ export class GatewayClient extends Client { // --------------------------------------------------------------------------- private namespace(clientKey: string, name: string): string { - return `${slugify(clientKey)}_${name}`; + return `${namespaceCode(clientKey)}_${name}`; } /**