From 94d052a024c6d0e3d9bbaec064b61e2bca93ec3a Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 12:42:25 -0700 Subject: [PATCH 1/4] Fix MCP auth ordering and OAuth discovery --- .../[...path]/route.ts | 38 ++- .../oauth-protected-resource/route.ts | 29 +- .../api/agent/[transport]/auth-flow.test.ts | 241 ++++++++++++++++ .../app/api/agent/[transport]/auth-flow.ts | 257 ++++++++++++++++++ .../src/app/api/agent/[transport]/route.ts | 148 +--------- apps/api/src/bun-test.d.ts | 25 ++ apps/api/src/lib/oauth-metadata.test.ts | 51 ++++ apps/api/src/lib/oauth-metadata.ts | 54 ++++ 8 files changed, 679 insertions(+), 164 deletions(-) create mode 100644 apps/api/src/app/api/agent/[transport]/auth-flow.test.ts create mode 100644 apps/api/src/app/api/agent/[transport]/auth-flow.ts create mode 100644 apps/api/src/bun-test.d.ts create mode 100644 apps/api/src/lib/oauth-metadata.test.ts create mode 100644 apps/api/src/lib/oauth-metadata.ts diff --git a/apps/api/src/app/.well-known/oauth-protected-resource/[...path]/route.ts b/apps/api/src/app/.well-known/oauth-protected-resource/[...path]/route.ts index b5e8b532600..1db0c83d84e 100644 --- a/apps/api/src/app/.well-known/oauth-protected-resource/[...path]/route.ts +++ b/apps/api/src/app/.well-known/oauth-protected-resource/[...path]/route.ts @@ -1,19 +1,33 @@ -import { env } from "@/env"; +import { auth } from "@superset/auth/server"; +import { buildProtectedResourceMetadata } from "@/lib/oauth-metadata"; -function getPublicOrigin(req: Request): string { - const host = req.headers.get("x-forwarded-host") ?? new URL(req.url).host; - const proto = - req.headers.get("x-forwarded-proto") ?? - new URL(req.url).protocol.replace(":", ""); - return `${proto}://${host}`; +interface RouteContext { + params: Promise<{ + path: string[]; + }>; } -export function GET(req: Request) { +export async function GET( + request: Request, + { params }: RouteContext, +): Promise { + const { path } = await params; + const authServerMetadata = await auth.api.getOAuthServerConfig({ + headers: request.headers, + }); + const resourcePath = `/${path.join("/")}`; + return Response.json( - { - resource: getPublicOrigin(req), - authorization_servers: [env.NEXT_PUBLIC_API_URL], - }, + buildProtectedResourceMetadata(request, resourcePath, { + authorizationServerUrl: + typeof authServerMetadata.issuer === "string" + ? authServerMetadata.issuer + : undefined, + resourceName: "Superset MCP Server", + scopesSupported: Array.isArray(authServerMetadata.scopes_supported) + ? authServerMetadata.scopes_supported + : undefined, + }), { headers: { "Access-Control-Allow-Origin": "*", diff --git a/apps/api/src/app/.well-known/oauth-protected-resource/route.ts b/apps/api/src/app/.well-known/oauth-protected-resource/route.ts index b5e8b532600..1e370eeb4c8 100644 --- a/apps/api/src/app/.well-known/oauth-protected-resource/route.ts +++ b/apps/api/src/app/.well-known/oauth-protected-resource/route.ts @@ -1,19 +1,22 @@ -import { env } from "@/env"; +import { auth } from "@superset/auth/server"; +import { buildProtectedResourceMetadata } from "@/lib/oauth-metadata"; -function getPublicOrigin(req: Request): string { - const host = req.headers.get("x-forwarded-host") ?? new URL(req.url).host; - const proto = - req.headers.get("x-forwarded-proto") ?? - new URL(req.url).protocol.replace(":", ""); - return `${proto}://${host}`; -} +export async function GET(request: Request): Promise { + const authServerMetadata = await auth.api.getOAuthServerConfig({ + headers: request.headers, + }); -export function GET(req: Request) { return Response.json( - { - resource: getPublicOrigin(req), - authorization_servers: [env.NEXT_PUBLIC_API_URL], - }, + buildProtectedResourceMetadata(request, "/", { + authorizationServerUrl: + typeof authServerMetadata.issuer === "string" + ? authServerMetadata.issuer + : undefined, + resourceName: "Superset MCP Server", + scopesSupported: Array.isArray(authServerMetadata.scopes_supported) + ? authServerMetadata.scopes_supported + : undefined, + }), { headers: { "Access-Control-Allow-Origin": "*", diff --git a/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts b/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts new file mode 100644 index 00000000000..fb774e6790e --- /dev/null +++ b/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts @@ -0,0 +1,241 @@ +import { describe, expect, it, mock } from "bun:test"; +import type { AuthInfo } from "@modelcontextprotocol/sdk/server/auth/types.js"; +import type { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js"; +import { + handleMcpRequest, + isApiKeyBearerToken, + type McpRequestDeps, + unauthorizedResponse, + verifyToken, +} from "./auth-flow"; + +function createRequest(headers?: HeadersInit): Request { + return new Request("https://api.superset.sh/api/agent/mcp", { + method: "POST", + headers, + }); +} + +function createDeps(overrides?: Partial): McpRequestDeps & { + sessionSpy: ReturnType; + apiKeySpy: ReturnType; + oauthSpy: ReturnType; + connectSpy: ReturnType; + transportHandleSpy: ReturnType; +} { + const sessionSpy = mock(async () => null); + const apiKeySpy = mock(async () => ({ valid: false, key: null })); + const oauthSpy = mock(async () => { + throw new Error("invalid token"); + }); + const connectSpy = mock(async () => {}); + const transportHandleSpy = mock( + async (_req: Request, _options?: { authInfo?: AuthInfo }) => + new Response("ok"), + ); + + return { + apiUrl: "https://api.superset.sh", + authApi: { + getSession: sessionSpy, + verifyApiKey: apiKeySpy, + }, + createServer: () => + ({ + connect: connectSpy, + }) as unknown as ReturnType, + createTransport: () => + ({ + handleRequest: transportHandleSpy, + }) as unknown as WebStandardStreamableHTTPServerTransport, + verifyAccessToken: oauthSpy as McpRequestDeps["verifyAccessToken"], + sessionSpy, + apiKeySpy, + oauthSpy, + connectSpy, + transportHandleSpy, + ...overrides, + }; +} + +describe("MCP auth flow", () => { + it("detects API key bearer tokens by prefix", () => { + expect(isApiKeyBearerToken("sk_live_123")).toBe(true); + expect(isApiKeyBearerToken("oauth-token")).toBe(false); + }); + + it("short-circuits invalid API keys without falling through", async () => { + const deps = createDeps(); + + const authInfo = await verifyToken( + createRequest({ authorization: "Bearer sk_live_invalid" }), + deps, + ); + + expect(authInfo).toBeUndefined(); + expect(deps.apiKeySpy).toHaveBeenCalledTimes(1); + expect(deps.oauthSpy).toHaveBeenCalledTimes(0); + expect(deps.sessionSpy).toHaveBeenCalledTimes(0); + }); + + it("accepts valid API keys", async () => { + const deps = createDeps({ + authApi: { + getSession: mock(async () => null), + verifyApiKey: mock(async () => ({ + valid: true, + key: { + userId: "user-1", + metadata: JSON.stringify({ organizationId: "org-1" }), + }, + })), + }, + }); + + const authInfo = await verifyToken( + createRequest({ authorization: "Bearer sk_live_valid" }), + deps, + ); + + expect(authInfo).toEqual({ + token: "api-key", + clientId: "api-key", + scopes: ["mcp:full"], + extra: { + mcpContext: { + userId: "user-1", + organizationId: "org-1", + }, + }, + }); + }); + + it("accepts OAuth access tokens before session lookup", async () => { + const deps = createDeps({ + verifyAccessToken: mock(async () => ({ + sub: "user-2", + organizationId: "org-2", + scope: "profile email", + azp: "client-1", + })) as McpRequestDeps["verifyAccessToken"], + }); + + const authInfo = await verifyToken( + createRequest({ authorization: "Bearer oauth.token.value" }), + deps, + ); + + expect(authInfo).toEqual({ + token: "oauth.token.value", + clientId: "client-1", + scopes: ["profile", "email"], + extra: { + mcpContext: { + userId: "user-2", + organizationId: "org-2", + }, + }, + }); + expect(deps.sessionSpy).toHaveBeenCalledTimes(0); + }); + + it("accepts opaque bearer session tokens without attempting OAuth verification", async () => { + const deps = createDeps({ + authApi: { + getSession: mock(async () => ({ + user: { id: "user-3" }, + session: { activeOrganizationId: "org-3" }, + })), + verifyApiKey: mock(async () => ({ valid: false, key: null })), + }, + }); + + const authInfo = await verifyToken( + createRequest({ authorization: "Bearer session-token" }), + deps, + ); + + expect(authInfo).toEqual({ + token: "session", + clientId: "session", + scopes: ["mcp:full"], + extra: { + mcpContext: { + userId: "user-3", + organizationId: "org-3", + }, + }, + }); + expect(deps.oauthSpy).toHaveBeenCalledTimes(0); + }); + + it("falls back to session auth when JWT bearer token is not a valid OAuth access token", async () => { + const deps = createDeps({ + authApi: { + getSession: mock(async () => ({ + user: { id: "user-3" }, + session: { activeOrganizationId: "org-3" }, + })), + verifyApiKey: mock(async () => ({ valid: false, key: null })), + }, + }); + + const authInfo = await verifyToken( + createRequest({ authorization: "Bearer invalid.jwt.token" }), + deps, + ); + + expect(authInfo).toEqual({ + token: "session", + clientId: "session", + scopes: ["mcp:full"], + extra: { + mcpContext: { + userId: "user-3", + organizationId: "org-3", + }, + }, + }); + expect(deps.oauthSpy).toHaveBeenCalledTimes(1); + }); + + it("returns a path-specific unauthorized challenge", () => { + const response = unauthorizedResponse(createRequest()); + + expect(response.status).toBe(401); + expect(response.headers.get("WWW-Authenticate")).toBe( + 'Bearer resource_metadata="https://api.superset.sh/.well-known/oauth-protected-resource/api/agent/mcp"', + ); + }); + + it("does not start MCP transport when auth fails", async () => { + const deps = createDeps(); + + const response = await handleMcpRequest( + createRequest({ authorization: "Bearer sk_live_invalid" }), + deps, + ); + + expect(response.status).toBe(401); + expect(deps.connectSpy).toHaveBeenCalledTimes(0); + expect(deps.transportHandleSpy).toHaveBeenCalledTimes(0); + }); + + it("starts MCP transport when auth succeeds", async () => { + const deps = createDeps({ + authApi: { + getSession: mock(async () => ({ + user: { id: "user-4" }, + session: { activeOrganizationId: "org-4" }, + })), + verifyApiKey: mock(async () => ({ valid: false, key: null })), + }, + }); + + const response = await handleMcpRequest(createRequest(), deps); + + expect(response.status).toBe(200); + expect(deps.connectSpy).toHaveBeenCalledTimes(1); + expect(deps.transportHandleSpy).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/api/src/app/api/agent/[transport]/auth-flow.ts b/apps/api/src/app/api/agent/[transport]/auth-flow.ts new file mode 100644 index 00000000000..707e4dc3cd9 --- /dev/null +++ b/apps/api/src/app/api/agent/[transport]/auth-flow.ts @@ -0,0 +1,257 @@ +import type { AuthInfo } from "@modelcontextprotocol/sdk/server/auth/types.js"; +import type { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js"; +import type { createMcpServer } from "@superset/mcp"; +import type { McpContext } from "@superset/mcp/auth"; +import type { verifyAccessToken as verifyOAuthAccessToken } from "better-auth/oauth2"; +import { getOAuthProtectedResourceMetadataUrl } from "@/lib/oauth-metadata"; + +interface SessionUser { + id: string; +} + +interface SessionRecord { + activeOrganizationId?: string | null; +} + +interface SessionResponse { + session?: SessionRecord | null; + user: SessionUser; +} + +interface VerifiedApiKey { + userId?: string | null; + metadata?: unknown; +} + +interface VerifyApiKeyResponse { + valid: boolean; + key: VerifiedApiKey | null; +} + +export interface McpRequestDeps { + apiUrl: string; + authApi: { + getSession(args: { + headers: Headers; + }): Promise; + verifyApiKey(args: { + body: { key: string }; + }): Promise; + }; + createServer: typeof createMcpServer; + createTransport: () => WebStandardStreamableHTTPServerTransport; + verifyAccessToken: typeof verifyOAuthAccessToken; +} + +function getBearerToken(req: Request): string | undefined { + const authorization = req.headers.get("authorization"); + + return authorization?.startsWith("Bearer ") + ? authorization.slice(7) + : undefined; +} + +export function isApiKeyBearerToken(token: string): boolean { + return token.startsWith("sk_live_"); +} + +function looksLikeJwt(token: string): boolean { + const parts = token.split("."); + return parts.length === 3 && parts.every(Boolean); +} + +function buildSessionAuthInfo(session: SessionResponse): AuthInfo | undefined { + const organizationId = session.session?.activeOrganizationId; + + if (!organizationId) { + console.error("[mcp/auth] Session missing activeOrganizationId"); + return undefined; + } + + return { + token: "session", + clientId: "session", + scopes: ["mcp:full"], + extra: { + mcpContext: { + userId: session.user.id, + organizationId, + } satisfies McpContext, + }, + }; +} + +function parseApiKeyMetadata( + metadata: unknown, +): Record | null { + if (!metadata) { + return null; + } + + if (typeof metadata === "string") { + try { + const parsed = JSON.parse(metadata); + return parsed && typeof parsed === "object" + ? (parsed as Record) + : null; + } catch (error) { + console.error("[mcp/auth] Failed to parse API key metadata:", error); + return null; + } + } + + return typeof metadata === "object" + ? (metadata as Record) + : null; +} + +function buildApiKeyAuthInfo(apiKey: VerifiedApiKey): AuthInfo | undefined { + const userId = apiKey.userId; + + if (!userId) { + console.error("[mcp/auth] API key missing userId"); + return undefined; + } + + const metadata = parseApiKeyMetadata(apiKey.metadata); + const organizationId = + typeof metadata?.organizationId === "string" + ? metadata.organizationId + : undefined; + + if (!organizationId) { + console.error("[mcp/auth] API key missing organizationId in metadata"); + return undefined; + } + + return { + token: "api-key", + clientId: "api-key", + scopes: ["mcp:full"], + extra: { + mcpContext: { + userId, + organizationId, + } satisfies McpContext, + }, + }; +} + +function buildOAuthAuthInfo( + bearerToken: string, + payload: Record, +): AuthInfo | undefined { + if ( + typeof payload.sub !== "string" || + typeof payload.organizationId !== "string" + ) { + console.error( + "[mcp/auth] Access token missing sub or organizationId claim", + ); + return undefined; + } + + const scopes = Array.isArray(payload.scope) + ? (payload.scope as string[]) + : typeof payload.scope === "string" + ? payload.scope.split(" ") + : []; + + return { + token: bearerToken, + clientId: typeof payload.azp === "string" ? payload.azp : "mcp-client", + scopes, + extra: { + mcpContext: { + userId: payload.sub, + organizationId: payload.organizationId, + } satisfies McpContext, + }, + }; +} + +export async function verifyToken( + req: Request, + deps: McpRequestDeps, +): Promise { + const bearerToken = getBearerToken(req); + let oauthVerificationError: unknown; + + if (bearerToken) { + if (isApiKeyBearerToken(bearerToken)) { + try { + const result = await deps.authApi.verifyApiKey({ + body: { key: bearerToken }, + }); + + if (result.valid && result.key) { + return buildApiKeyAuthInfo(result.key); + } + } catch (error) { + console.error("[mcp/auth] API key verification failed:", error); + } + + return undefined; + } + + if (looksLikeJwt(bearerToken)) { + try { + const payload = (await deps.verifyAccessToken(bearerToken, { + jwksUrl: `${deps.apiUrl}/api/auth/jwks`, + verifyOptions: { + issuer: deps.apiUrl, + audience: [deps.apiUrl, `${deps.apiUrl}/`], + }, + })) as Record; + + const authInfo = buildOAuthAuthInfo(bearerToken, payload); + if (authInfo) { + return authInfo; + } + } catch (error) { + oauthVerificationError = error; + } + } + } + + const session = await deps.authApi.getSession({ headers: req.headers }); + if (session?.session) { + return buildSessionAuthInfo(session); + } + + if (oauthVerificationError) { + console.error( + "[mcp/auth] Access token verification failed:", + oauthVerificationError, + ); + } + + return undefined; +} + +export function unauthorizedResponse(req: Request): Response { + return new Response("Unauthorized", { + status: 401, + headers: { + "WWW-Authenticate": `Bearer resource_metadata="${getOAuthProtectedResourceMetadataUrl( + req, + )}"`, + }, + }); +} + +export async function handleMcpRequest( + req: Request, + deps: McpRequestDeps, +): Promise { + const authInfo = await verifyToken(req, deps); + if (!authInfo) { + return unauthorizedResponse(req); + } + + const transport = deps.createTransport(); + const server = deps.createServer(); + await server.connect(transport); + + return transport.handleRequest(req, { authInfo }); +} diff --git a/apps/api/src/app/api/agent/[transport]/route.ts b/apps/api/src/app/api/agent/[transport]/route.ts index 0bf00ad8f49..676971bb153 100644 --- a/apps/api/src/app/api/agent/[transport]/route.ts +++ b/apps/api/src/app/api/agent/[transport]/route.ts @@ -1,150 +1,20 @@ -import type { AuthInfo } from "@modelcontextprotocol/sdk/server/auth/types.js"; import { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js"; import { auth } from "@superset/auth/server"; import { createMcpServer } from "@superset/mcp"; -import type { McpContext } from "@superset/mcp/auth"; import { verifyAccessToken } from "better-auth/oauth2"; import { env } from "@/env"; +import { handleMcpRequest, type McpRequestDeps } from "./auth-flow"; -async function verifyToken(req: Request): Promise { - const authorization = req.headers.get("authorization"); - const bearerToken = authorization?.startsWith("Bearer ") - ? authorization.slice(7) - : undefined; - - // 1. Try session auth (for desktop/web app) - const session = await auth.api.getSession({ headers: req.headers }); - if (session?.session) { - const extendedSession = session.session as { - activeOrganizationId?: string; - }; - if (!extendedSession.activeOrganizationId) { - console.error("[mcp/auth] Session missing activeOrganizationId"); - return undefined; - } - return { - token: "session", - clientId: "session", - scopes: ["mcp:full"], - extra: { - mcpContext: { - userId: session.user.id, - organizationId: extendedSession.activeOrganizationId, - } satisfies McpContext, - }, - }; - } - - // 2. Try API key verification (for sk_live_ tokens) - if (bearerToken) { - try { - const result = await auth.api.verifyApiKey({ - body: { key: bearerToken }, - }); - if (result.valid && result.key) { - const userId = result.key.userId; - if (!userId) { - console.error("[mcp/auth] API key missing userId"); - return undefined; - } - const metadata = - typeof result.key.metadata === "string" - ? JSON.parse(result.key.metadata) - : result.key.metadata; - const organizationId = metadata?.organizationId as string | undefined; - if (!organizationId) { - console.error( - "[mcp/auth] API key missing organizationId in metadata", - ); - return undefined; - } - return { - token: "api-key", - clientId: "api-key", - scopes: ["mcp:full"], - extra: { - mcpContext: { - userId, - organizationId, - } satisfies McpContext, - }, - }; - } - } catch (error) { - console.error("[mcp/auth] API key verification failed:", error); - } - } - - // 3. Try OAuth access token verification via JWKS - if (bearerToken) { - try { - const payload = await verifyAccessToken(bearerToken, { - jwksUrl: `${env.NEXT_PUBLIC_API_URL}/api/auth/jwks`, - verifyOptions: { - issuer: env.NEXT_PUBLIC_API_URL, - audience: [env.NEXT_PUBLIC_API_URL, `${env.NEXT_PUBLIC_API_URL}/`], - }, - }); - if (!payload?.sub || !payload.organizationId) { - console.error( - "[mcp/auth] Access token missing sub or organizationId claim", - ); - return undefined; - } - - const scopes = Array.isArray(payload.scope) - ? (payload.scope as string[]) - : typeof payload.scope === "string" - ? payload.scope.split(" ") - : []; - - return { - token: bearerToken, - clientId: (payload.azp as string) ?? "mcp-client", - scopes, - extra: { - mcpContext: { - userId: payload.sub, - organizationId: payload.organizationId as string, - } satisfies McpContext, - }, - }; - } catch (error) { - console.error("[mcp/auth] Access token verification failed:", error); - return undefined; - } - } - - return undefined; -} - -function getResourceMetadataUrl(req: Request): string { - const host = req.headers.get("x-forwarded-host") ?? new URL(req.url).host; - const proto = - req.headers.get("x-forwarded-proto") ?? - new URL(req.url).protocol.replace(":", ""); - return `${proto}://${host}/.well-known/oauth-protected-resource`; -} - -function unauthorizedResponse(req: Request): Response { - const metadataUrl = getResourceMetadataUrl(req); - return new Response("Unauthorized", { - status: 401, - headers: { - "WWW-Authenticate": `Bearer resource_metadata="${metadataUrl}"`, - }, - }); -} +const deps: McpRequestDeps = { + apiUrl: env.NEXT_PUBLIC_API_URL, + authApi: auth.api, + createServer: createMcpServer, + createTransport: () => new WebStandardStreamableHTTPServerTransport(), + verifyAccessToken, +}; async function handleRequest(req: Request): Promise { - const authInfo = await verifyToken(req); - if (!authInfo) return unauthorizedResponse(req); - - const transport = new WebStandardStreamableHTTPServerTransport(); - const server = createMcpServer(); - await server.connect(transport); - - return transport.handleRequest(req, { authInfo }); + return handleMcpRequest(req, deps); } export { handleRequest as GET, handleRequest as POST, handleRequest as DELETE }; diff --git a/apps/api/src/bun-test.d.ts b/apps/api/src/bun-test.d.ts new file mode 100644 index 00000000000..93f10d284ea --- /dev/null +++ b/apps/api/src/bun-test.d.ts @@ -0,0 +1,25 @@ +declare module "bun:test" { + export interface MockResult { + calls: unknown[][]; + } + + export type Mock unknown> = T & { + mock: MockResult; + }; + + export function describe( + name: string, + callback: () => void | Promise, + ): void; + + export function it(name: string, callback: () => void | Promise): void; + + export function mock unknown>(fn: T): Mock; + + export function expect(actual: T): { + toBe(expected: unknown): void; + toEqual(expected: unknown): void; + toBeUndefined(): void; + toHaveBeenCalledTimes(expected: number): void; + }; +} diff --git a/apps/api/src/lib/oauth-metadata.test.ts b/apps/api/src/lib/oauth-metadata.test.ts new file mode 100644 index 00000000000..4599d040bcd --- /dev/null +++ b/apps/api/src/lib/oauth-metadata.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "bun:test"; +import { + buildProtectedResourceMetadata, + getOAuthProtectedResourceMetadataUrl, + getRequestOrigin, + normalizeResourcePath, +} from "./oauth-metadata"; + +describe("oauth metadata helpers", () => { + it("prefers forwarded origin headers", () => { + const request = new Request("http://internal/api/agent/mcp", { + headers: { + "x-forwarded-host": "api.superset.sh", + "x-forwarded-proto": "https", + }, + }); + + expect(getRequestOrigin(request)).toBe("https://api.superset.sh"); + }); + + it("builds a path-specific protected resource metadata URL", () => { + const request = new Request("https://api.superset.sh/api/agent/mcp"); + + expect(getOAuthProtectedResourceMetadataUrl(request)).toBe( + "https://api.superset.sh/.well-known/oauth-protected-resource/api/agent/mcp", + ); + }); + + it("normalizes root and nested resource paths", () => { + expect(normalizeResourcePath("/")).toBe(""); + expect(normalizeResourcePath("api/agent/mcp")).toBe("/api/agent/mcp"); + expect(normalizeResourcePath("/api/agent/mcp")).toBe("/api/agent/mcp"); + }); + + it("builds protected resource metadata with optional fields", () => { + const request = new Request("https://api.superset.sh/anything"); + + expect( + buildProtectedResourceMetadata(request, "/api/agent/mcp", { + authorizationServerUrl: "https://api.superset.sh", + resourceName: "Superset MCP Server", + scopesSupported: ["profile", "email"], + }), + ).toEqual({ + resource: "https://api.superset.sh/api/agent/mcp", + authorization_servers: ["https://api.superset.sh"], + resource_name: "Superset MCP Server", + scopes_supported: ["profile", "email"], + }); + }); +}); diff --git a/apps/api/src/lib/oauth-metadata.ts b/apps/api/src/lib/oauth-metadata.ts new file mode 100644 index 00000000000..a724102b1de --- /dev/null +++ b/apps/api/src/lib/oauth-metadata.ts @@ -0,0 +1,54 @@ +export interface ProtectedResourceMetadataOptions { + authorizationServerUrl?: string; + resourceName?: string; + resourceDocumentation?: string; + scopesSupported?: string[]; +} + +export function getRequestOrigin(req: Request): string { + const requestUrl = new URL(req.url); + const host = req.headers.get("x-forwarded-host") ?? requestUrl.host; + const proto = + req.headers.get("x-forwarded-proto") ?? + requestUrl.protocol.replace(":", ""); + + return `${proto}://${host}`; +} + +export function normalizeResourcePath(pathname: string): string { + if (!pathname || pathname === "/") { + return ""; + } + + return pathname.startsWith("/") ? pathname : `/${pathname}`; +} + +export function getOAuthProtectedResourceMetadataUrl(req: Request): string { + const requestUrl = new URL(req.url); + return `${getRequestOrigin(req)}/.well-known/oauth-protected-resource${normalizeResourcePath( + requestUrl.pathname, + )}`; +} + +export function buildProtectedResourceMetadata( + req: Request, + resourcePath: string, + options: ProtectedResourceMetadataOptions, +): Record { + const origin = getRequestOrigin(req); + const normalizedResourcePath = normalizeResourcePath(resourcePath); + + return { + resource: `${origin}${normalizedResourcePath}`, + ...(options.authorizationServerUrl + ? { authorization_servers: [options.authorizationServerUrl] } + : {}), + ...(options.scopesSupported?.length + ? { scopes_supported: options.scopesSupported } + : {}), + ...(options.resourceName ? { resource_name: options.resourceName } : {}), + ...(options.resourceDocumentation + ? { resource_documentation: options.resourceDocumentation } + : {}), + }; +} From 6697ce9ddcae066ee088d29da050779f16ee1693 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 16:57:47 -0700 Subject: [PATCH 2/4] Handle case-insensitive bearer auth and forwarded header lists --- .../app/api/agent/[transport]/auth-flow.test.ts | 14 ++++++++++++++ .../api/src/app/api/agent/[transport]/auth-flow.ts | 6 ++---- apps/api/src/lib/oauth-metadata.test.ts | 11 +++++++++++ apps/api/src/lib/oauth-metadata.ts | 13 +++++++++++-- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts b/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts index fb774e6790e..32870d9fab6 100644 --- a/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts +++ b/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts @@ -78,6 +78,20 @@ describe("MCP auth flow", () => { expect(deps.sessionSpy).toHaveBeenCalledTimes(0); }); + it("accepts case-insensitive bearer auth schemes", async () => { + const deps = createDeps(); + + const authInfo = await verifyToken( + createRequest({ authorization: "bearer sk_live_invalid" }), + deps, + ); + + expect(authInfo).toBeUndefined(); + expect(deps.apiKeySpy).toHaveBeenCalledTimes(1); + expect(deps.oauthSpy).toHaveBeenCalledTimes(0); + expect(deps.sessionSpy).toHaveBeenCalledTimes(0); + }); + it("accepts valid API keys", async () => { const deps = createDeps({ authApi: { diff --git a/apps/api/src/app/api/agent/[transport]/auth-flow.ts b/apps/api/src/app/api/agent/[transport]/auth-flow.ts index 707e4dc3cd9..b6c8853328f 100644 --- a/apps/api/src/app/api/agent/[transport]/auth-flow.ts +++ b/apps/api/src/app/api/agent/[transport]/auth-flow.ts @@ -45,10 +45,8 @@ export interface McpRequestDeps { function getBearerToken(req: Request): string | undefined { const authorization = req.headers.get("authorization"); - - return authorization?.startsWith("Bearer ") - ? authorization.slice(7) - : undefined; + const match = authorization?.match(/^Bearer\s+(.+)$/i); + return match?.[1]; } export function isApiKeyBearerToken(token: string): boolean { diff --git a/apps/api/src/lib/oauth-metadata.test.ts b/apps/api/src/lib/oauth-metadata.test.ts index 4599d040bcd..7a93dbbdb66 100644 --- a/apps/api/src/lib/oauth-metadata.test.ts +++ b/apps/api/src/lib/oauth-metadata.test.ts @@ -18,6 +18,17 @@ describe("oauth metadata helpers", () => { expect(getRequestOrigin(request)).toBe("https://api.superset.sh"); }); + it("uses the first forwarded host and proto values when proxies append lists", () => { + const request = new Request("http://internal/api/agent/mcp", { + headers: { + "x-forwarded-host": "api.superset.sh, internal.example", + "x-forwarded-proto": "https, http", + }, + }); + + expect(getRequestOrigin(request)).toBe("https://api.superset.sh"); + }); + it("builds a path-specific protected resource metadata URL", () => { const request = new Request("https://api.superset.sh/api/agent/mcp"); diff --git a/apps/api/src/lib/oauth-metadata.ts b/apps/api/src/lib/oauth-metadata.ts index a724102b1de..c8f838a4691 100644 --- a/apps/api/src/lib/oauth-metadata.ts +++ b/apps/api/src/lib/oauth-metadata.ts @@ -5,11 +5,20 @@ export interface ProtectedResourceMetadataOptions { scopesSupported?: string[]; } +function getFirstForwardedValue(value: string | null): string | undefined { + return value + ?.split(",") + .map((part) => part.trim()) + .find(Boolean); +} + export function getRequestOrigin(req: Request): string { const requestUrl = new URL(req.url); - const host = req.headers.get("x-forwarded-host") ?? requestUrl.host; + const host = + getFirstForwardedValue(req.headers.get("x-forwarded-host")) ?? + requestUrl.host; const proto = - req.headers.get("x-forwarded-proto") ?? + getFirstForwardedValue(req.headers.get("x-forwarded-proto")) ?? requestUrl.protocol.replace(":", ""); return `${proto}://${host}`; From b607ea72ceebdcb1c4d69d359cd5c3ac850c993b Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 18:54:05 -0700 Subject: [PATCH 3/4] Sanitize auth error logs and normalize API URLs --- .../api/agent/[transport]/auth-flow.test.ts | 27 ++++++++++--- .../app/api/agent/[transport]/auth-flow.ts | 38 +++++++++++++++---- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts b/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts index 32870d9fab6..a9dcf5983c2 100644 --- a/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts +++ b/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts @@ -125,13 +125,15 @@ describe("MCP auth flow", () => { }); it("accepts OAuth access tokens before session lookup", async () => { + const verifyAccessToken = mock(async () => ({ + sub: "user-2", + organizationId: "org-2", + scope: "profile email", + azp: "client-1", + })) as McpRequestDeps["verifyAccessToken"]; const deps = createDeps({ - verifyAccessToken: mock(async () => ({ - sub: "user-2", - organizationId: "org-2", - scope: "profile email", - azp: "client-1", - })) as McpRequestDeps["verifyAccessToken"], + apiUrl: "https://api.superset.sh/", + verifyAccessToken, }); const authInfo = await verifyToken( @@ -151,6 +153,19 @@ describe("MCP auth flow", () => { }, }); expect(deps.sessionSpy).toHaveBeenCalledTimes(0); + expect( + ( + verifyAccessToken as typeof verifyAccessToken & { + mock: { calls: unknown[][] }; + } + ).mock.calls[0]?.[1], + ).toEqual({ + jwksUrl: "https://api.superset.sh/api/auth/jwks", + verifyOptions: { + issuer: "https://api.superset.sh", + audience: ["https://api.superset.sh", "https://api.superset.sh/"], + }, + }); }); it("accepts opaque bearer session tokens without attempting OAuth verification", async () => { diff --git a/apps/api/src/app/api/agent/[transport]/auth-flow.ts b/apps/api/src/app/api/agent/[transport]/auth-flow.ts index b6c8853328f..d09e4d38a94 100644 --- a/apps/api/src/app/api/agent/[transport]/auth-flow.ts +++ b/apps/api/src/app/api/agent/[transport]/auth-flow.ts @@ -53,6 +53,26 @@ export function isApiKeyBearerToken(token: string): boolean { return token.startsWith("sk_live_"); } +function normalizeApiUrl(apiUrl: string): string { + return apiUrl.replace(/\/+$/, ""); +} + +function getSafeAuthErrorName(error: unknown): string { + if (error && typeof error === "object") { + const errorName = "name" in error ? error.name : undefined; + if (typeof errorName === "string" && errorName.length > 0) { + return errorName; + } + + const errorCode = "code" in error ? error.code : undefined; + if (typeof errorCode === "string" && errorCode.length > 0) { + return errorCode; + } + } + + return "AuthVerificationError"; +} + function looksLikeJwt(token: string): boolean { const parts = token.split("."); return parts.length === 3 && parts.every(Boolean); @@ -173,6 +193,7 @@ export async function verifyToken( deps: McpRequestDeps, ): Promise { const bearerToken = getBearerToken(req); + const apiUrl = normalizeApiUrl(deps.apiUrl); let oauthVerificationError: unknown; if (bearerToken) { @@ -186,7 +207,9 @@ export async function verifyToken( return buildApiKeyAuthInfo(result.key); } } catch (error) { - console.error("[mcp/auth] API key verification failed:", error); + console.error("[mcp/auth] API key verification failed", { + errorName: getSafeAuthErrorName(error), + }); } return undefined; @@ -195,10 +218,10 @@ export async function verifyToken( if (looksLikeJwt(bearerToken)) { try { const payload = (await deps.verifyAccessToken(bearerToken, { - jwksUrl: `${deps.apiUrl}/api/auth/jwks`, + jwksUrl: `${apiUrl}/api/auth/jwks`, verifyOptions: { - issuer: deps.apiUrl, - audience: [deps.apiUrl, `${deps.apiUrl}/`], + issuer: apiUrl, + audience: [apiUrl, `${apiUrl}/`], }, })) as Record; @@ -218,10 +241,9 @@ export async function verifyToken( } if (oauthVerificationError) { - console.error( - "[mcp/auth] Access token verification failed:", - oauthVerificationError, - ); + console.error("[mcp/auth] Access token verification failed", { + errorName: getSafeAuthErrorName(oauthVerificationError), + }); } return undefined; From c7282eb95bb83680170f390f0a5033cf7dc3e9d8 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 19:09:26 -0700 Subject: [PATCH 4/4] Fail closed on invalid verified OAuth tokens --- .../api/agent/[transport]/auth-flow.test.ts | 26 +++++++++++++++++++ .../app/api/agent/[transport]/auth-flow.ts | 5 +--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts b/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts index a9dcf5983c2..7a589e0397e 100644 --- a/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts +++ b/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts @@ -228,6 +228,32 @@ describe("MCP auth flow", () => { expect(deps.oauthSpy).toHaveBeenCalledTimes(1); }); + it("does not fall back to session auth after a verified JWT is missing required claims", async () => { + const sessionSpy = mock(async () => ({ + user: { id: "user-3" }, + session: { activeOrganizationId: "org-3" }, + })); + const verifyAccessToken = mock(async () => ({ + sub: "user-2", + })) as McpRequestDeps["verifyAccessToken"]; + const deps = createDeps({ + authApi: { + getSession: sessionSpy, + verifyApiKey: mock(async () => ({ valid: false, key: null })), + }, + verifyAccessToken, + }); + + const authInfo = await verifyToken( + createRequest({ authorization: "Bearer verified.jwt.token" }), + deps, + ); + + expect(authInfo).toBeUndefined(); + expect(verifyAccessToken).toHaveBeenCalledTimes(1); + expect(sessionSpy).toHaveBeenCalledTimes(0); + }); + it("returns a path-specific unauthorized challenge", () => { const response = unauthorizedResponse(createRequest()); diff --git a/apps/api/src/app/api/agent/[transport]/auth-flow.ts b/apps/api/src/app/api/agent/[transport]/auth-flow.ts index d09e4d38a94..45213a4dd2f 100644 --- a/apps/api/src/app/api/agent/[transport]/auth-flow.ts +++ b/apps/api/src/app/api/agent/[transport]/auth-flow.ts @@ -225,10 +225,7 @@ export async function verifyToken( }, })) as Record; - const authInfo = buildOAuthAuthInfo(bearerToken, payload); - if (authInfo) { - return authInfo; - } + return buildOAuthAuthInfo(bearerToken, payload); } catch (error) { oauthVerificationError = error; }