From 10cae56cd14928aca45e47aaa259cf0fac1413ce Mon Sep 17 00:00:00 2001 From: Satya Patel Date: Tue, 14 Apr 2026 15:08:12 -0700 Subject: [PATCH 1/3] fix(mcp): accept MCP resource URL as valid OAuth audience (#3459) MCP clients on spec 2025-06-18 send resource= in authorize/token requests. better-auth 1.5.6 validates that against validAudiences and our allowlist only contained the bare API origin, so tokens were rejected with "requested resource invalid". Add the MCP endpoint to validAudiences and to the JWT verifier's audience list. --- .../api/agent/[transport]/auth-flow.test.ts | 296 ------------------ .../app/api/agent/[transport]/auth-flow.ts | 2 +- apps/api/src/lib/oauth-metadata.test.ts | 62 ---- packages/auth/src/server.ts | 6 +- 4 files changed, 6 insertions(+), 360 deletions(-) delete mode 100644 apps/api/src/app/api/agent/[transport]/auth-flow.test.ts delete mode 100644 apps/api/src/lib/oauth-metadata.test.ts 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 deleted file mode 100644 index 7a589e0397e..00000000000 --- a/apps/api/src/app/api/agent/[transport]/auth-flow.test.ts +++ /dev/null @@ -1,296 +0,0 @@ -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 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: { - 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 verifyAccessToken = mock(async () => ({ - sub: "user-2", - organizationId: "org-2", - scope: "profile email", - azp: "client-1", - })) as McpRequestDeps["verifyAccessToken"]; - const deps = createDeps({ - apiUrl: "https://api.superset.sh/", - 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); - 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 () => { - 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("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()); - - 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 index 3d94678cd79..6dcda29e11c 100644 --- a/apps/api/src/app/api/agent/[transport]/auth-flow.ts +++ b/apps/api/src/app/api/agent/[transport]/auth-flow.ts @@ -221,7 +221,7 @@ export async function verifyToken( jwksUrl: `${apiUrl}/api/auth/jwks`, verifyOptions: { issuer: apiUrl, - audience: [apiUrl, `${apiUrl}/`], + audience: [apiUrl, `${apiUrl}/`, `${apiUrl}/api/agent/mcp`], }, })) as Record; diff --git a/apps/api/src/lib/oauth-metadata.test.ts b/apps/api/src/lib/oauth-metadata.test.ts deleted file mode 100644 index 7a93dbbdb66..00000000000 --- a/apps/api/src/lib/oauth-metadata.test.ts +++ /dev/null @@ -1,62 +0,0 @@ -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("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"); - - 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/packages/auth/src/server.ts b/packages/auth/src/server.ts index 696e4cc366c..32b4c0e92e3 100644 --- a/packages/auth/src/server.ts +++ b/packages/auth/src/server.ts @@ -202,7 +202,11 @@ export const auth = betterAuth({ consentPage: `${env.NEXT_PUBLIC_WEB_URL}/oauth/consent`, allowDynamicClientRegistration: true, allowUnauthenticatedClientRegistration: true, - validAudiences: [env.NEXT_PUBLIC_API_URL, `${env.NEXT_PUBLIC_API_URL}/`], + validAudiences: [ + env.NEXT_PUBLIC_API_URL, + `${env.NEXT_PUBLIC_API_URL}/`, + `${env.NEXT_PUBLIC_API_URL}/api/agent/mcp`, + ], silenceWarnings: { oauthAuthServerConfig: true, openidConfig: true, From e19f9ccca6043bbcec3e8ac224468d6882eeb2d1 Mon Sep 17 00:00:00 2001 From: Satya Patel Date: Tue, 14 Apr 2026 12:34:35 -0700 Subject: [PATCH 2/3] fix(desktop): drive tray menu off events, fetch real org name (#3458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the 5s tray polling interval with event-driven refresh: `mouse-enter` + host-service status-changed trigger an async rebuild. Drop the hostInfoCache Map — updateTrayMenu is now async and fetches host.info inline with a 2s AbortController timeout, so there is no stale state to manage. Unwrap the superjson envelope (`result.data.json`) that the original polling code missed, which was causing every org to render as a UUID slice (previously) or "Loading…" (after the first pass of this fix). Closes #3454 --- apps/desktop/src/main/lib/tray/index.ts | 191 ++++++++++++++---------- 1 file changed, 112 insertions(+), 79 deletions(-) diff --git a/apps/desktop/src/main/lib/tray/index.ts b/apps/desktop/src/main/lib/tray/index.ts index 313d9adf995..69dcb37d557 100644 --- a/apps/desktop/src/main/lib/tray/index.ts +++ b/apps/desktop/src/main/lib/tray/index.ts @@ -7,17 +7,16 @@ import { nativeImage, Tray, } from "electron"; +import { loadToken } from "lib/trpc/routers/auth/utils/auth-functions"; +import { env } from "main/env.main"; import { focusMainWindow, requestQuit } from "main/index"; // FORK NOTE: upstream renamed host-service-manager → host-service-coordinator (#3250) import { - getHostServiceCoordinator as getHostServiceManager, - type HostServiceStatus, + getHostServiceCoordinator, type HostServiceStatusEvent, } from "main/lib/host-service-coordinator"; import { menuEmitter } from "main/lib/menu-events"; -const POLL_INTERVAL_MS = 5000; - /** Must have "Template" suffix for macOS dark/light mode support */ const TRAY_ICON_FILENAME = "iconTemplate.png"; @@ -51,7 +50,6 @@ function getTrayIconPath(): string | null { } let tray: Tray | null = null; -let pollIntervalId: ReturnType | null = null; function createTrayIcon(): Electron.NativeImage | null { const iconPath = getTrayIconPath(); @@ -86,87 +84,127 @@ function openSettings(): void { menuEmitter.emit("open-settings"); } -// FORK NOTE: upstream coordinator simplified API — removed getServiceInfo, -// HostServiceStatus now only "starting" | "running" | "stopped" -function formatStatusLabel(status: HostServiceStatus): string { - switch (status) { - case "running": - return "Running"; - case "starting": - return "Starting..."; - case "stopped": - return "Stopped"; +interface HostInfo { + organizationName: string; + version: string; +} + +async function fetchHostInfo(organizationId: string): Promise { + const connection = getHostServiceCoordinator().getConnection(organizationId); + if (!connection) return null; + + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 2000); + try { + const res = await fetch( + `http://127.0.0.1:${connection.port}/trpc/host.info`, + { + headers: { Authorization: `Bearer ${connection.secret}` }, + signal: controller.signal, + }, + ); + if (!res.ok) return null; + const data = await res.json(); + const info = data?.result?.data?.json; + if (!info?.organization?.name) return null; + return { + organizationName: info.organization.name, + version: info.version ?? "", + }; + } catch { + return null; + } finally { + clearTimeout(timeout); } } -function buildHostServiceSubmenu(): MenuItemConstructorOptions[] { - const manager = getHostServiceManager(); - const orgIds = manager.getActiveOrganizationIds(); +function buildHostServiceSubmenu( + orgIds: string[], + infos: Map, +): MenuItemConstructorOptions[] { + const coordinator = getHostServiceCoordinator(); const menuItems: MenuItemConstructorOptions[] = []; if (orgIds.length === 0) { menuItems.push({ label: "No active services", enabled: false }); - } else { - let isFirst = true; - for (const orgId of orgIds) { - if (!isFirst) { - menuItems.push({ type: "separator" }); - } - isFirst = false; - - const status = manager.getProcessStatus(orgId); - const orgName = orgId.slice(0, 8); - const statusLabel = formatStatusLabel(status); - const isRunning = status === "running"; - - menuItems.push({ - label: orgName, - enabled: false, - }); - - menuItems.push({ - label: ` ${statusLabel}`, - enabled: false, - }); - - // FORK NOTE: restart removed — coordinator.restart() now requires - // SpawnConfig (authToken + cloudApiUrl) which is not available in tray. - // Restart can be done from Settings UI instead. - - menuItems.push({ - label: " Stop", - enabled: isRunning, - click: () => { - manager.stop(orgId); - updateTrayMenu(); - }, - }); + return menuItems; + } + + let isFirst = true; + for (const orgId of orgIds) { + if (!isFirst) { + menuItems.push({ type: "separator" }); } + isFirst = false; + + const status = coordinator.getProcessStatus(orgId); + const info = infos.get(orgId); + const isRunning = status === "running"; + const label = info?.organizationName ?? "Loading…"; + const versionSuffix = info?.version ? ` (v${info.version})` : ""; + + menuItems.push({ label, enabled: false }); + menuItems.push({ + label: ` ${status}${versionSuffix}`, + enabled: false, + }); + menuItems.push({ + label: " Restart", + enabled: isRunning, + click: () => { + void (async () => { + try { + const { token } = await loadToken(); + if (!token) return; + await coordinator.restart(orgId, { + authToken: token, + cloudApiUrl: env.NEXT_PUBLIC_API_URL, + }); + } catch (error) { + console.error( + `[Tray] Failed to restart host-service for ${orgId}:`, + error, + ); + } + void updateTrayMenu(); + })(); + }, + }); + menuItems.push({ + label: " Stop", + enabled: isRunning, + click: () => { + coordinator.stop(orgId); + void updateTrayMenu(); + }, + }); } return menuItems; } -function _formatUptime(seconds: number): string { - if (seconds < 60) return `${seconds}s`; - if (seconds < 3600) return `${Math.floor(seconds / 60)}m`; - const hours = Math.floor(seconds / 3600); - const mins = Math.floor((seconds % 3600) / 60); - return mins > 0 ? `${hours}h ${mins}m` : `${hours}h`; -} - -function updateTrayMenu(): void { +async function updateTrayMenu(): Promise { if (!tray) return; - const manager = getHostServiceManager(); - const orgIds = manager.getActiveOrganizationIds(); + const coordinator = getHostServiceCoordinator(); + const orgIds = coordinator.getActiveOrganizationIds(); + + const infoEntries = await Promise.all( + orgIds.map(async (orgId) => [orgId, await fetchHostInfo(orgId)] as const), + ); + const infos = new Map(); + for (const [orgId, info] of infoEntries) { + if (info) infos.set(orgId, info); + } + + if (!tray) return; const hasActive = orgIds.length > 0; const hostServiceLabel = hasActive ? `Host Service (${orgIds.length})` : "Host Service"; - const hostServiceSubmenu = buildHostServiceSubmenu(); + const hostServiceSubmenu = buildHostServiceSubmenu(orgIds, infos); const menu = Menu.buildFromTemplate([ { @@ -191,6 +229,9 @@ function updateTrayMenu(): void { }, }, { type: "separator" }, + // FORK NOTE: fork supports two quit modes — release (keep host-services + // alive for reattach) vs stop (fully tear them down). Preserved from + // pre-#3458 fork tray. ...(hasActive ? [ { @@ -234,19 +275,16 @@ export function initTray(): void { tray = new Tray(icon); tray.setToolTip("Superset"); - updateTrayMenu(); + void updateTrayMenu(); - const manager = getHostServiceManager(); + const manager = getHostServiceCoordinator(); manager.on("status-changed", (_event: HostServiceStatusEvent) => { - updateTrayMenu(); + void updateTrayMenu(); }); - // Periodic refresh as a fallback - pollIntervalId = setInterval(() => { - updateTrayMenu(); - }, POLL_INTERVAL_MS); - // Don't keep Electron alive just for tray updates - pollIntervalId.unref(); + tray.on("mouse-enter", () => { + void updateTrayMenu(); + }); console.log("[Tray] Initialized successfully"); } catch (error) { @@ -256,11 +294,6 @@ export function initTray(): void { /** Call on app quit */ export function disposeTray(): void { - if (pollIntervalId) { - clearInterval(pollIntervalId); - pollIntervalId = null; - } - if (tray) { tray.destroy(); tray = null; From 9629c1392c7d7d7c3a4d194ca832def68d6d4c40 Mon Sep 17 00:00:00 2001 From: MocA-Love <64681295+MocA-Love@users.noreply.github.com> Date: Wed, 15 Apr 2026 22:38:25 +0900 Subject: [PATCH 3/3] fix(fork): guard tray menu updates against stale async races Address Codex review on #177: updateTrayMenu is triggered by both status-changed and mouse-enter and awaits fetchHostInfo in between, so an older invocation can finish after a newer one and overwrite the tray state with a stale orgIds snapshot. In practice that could reintroduce a stopped service's submenu entries and flip the Quit menu between single- and dual-mode variants. Introduce a monotonic trayUpdateToken bumped on entry; drop the result if trayUpdateToken has advanced by the time fetchHostInfo resolves. --- apps/desktop/src/main/lib/tray/index.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/main/lib/tray/index.ts b/apps/desktop/src/main/lib/tray/index.ts index 69dcb37d557..eea0450088d 100644 --- a/apps/desktop/src/main/lib/tray/index.ts +++ b/apps/desktop/src/main/lib/tray/index.ts @@ -50,6 +50,11 @@ function getTrayIconPath(): string | null { } let tray: Tray | null = null; +// FORK NOTE: bump on each updateTrayMenu entry so overlapping async runs +// can drop stale results. Without this, a late-returning fetch would +// rebuild the menu from a snapshot of orgIds taken before a status change, +// re-introducing stopped services and the wrong Quit-mode variant. +let trayUpdateToken = 0; function createTrayIcon(): Electron.NativeImage | null { const iconPath = getTrayIconPath(); @@ -186,6 +191,7 @@ function buildHostServiceSubmenu( async function updateTrayMenu(): Promise { if (!tray) return; + const token = ++trayUpdateToken; const coordinator = getHostServiceCoordinator(); const orgIds = coordinator.getActiveOrganizationIds(); @@ -197,7 +203,9 @@ async function updateTrayMenu(): Promise { if (info) infos.set(orgId, info); } - if (!tray) return; + // Drop results if a newer updateTrayMenu has already started — otherwise + // an older snapshot can overwrite the newer one and show stale state. + if (!tray || token !== trayUpdateToken) return; const hasActive = orgIds.length > 0; const hostServiceLabel = hasActive