diff --git a/e2e/oauth-authorization.spec.ts b/e2e/oauth-authorization.spec.ts new file mode 100644 index 00000000..15d646a3 --- /dev/null +++ b/e2e/oauth-authorization.spec.ts @@ -0,0 +1,256 @@ +/** + * OAuth authorization-code popup flow (mcp-context-forge#6458). + * + * The real round trip -- BFF proxies GET /oauth/authorize/{id} to mcpgateway, + * which 302s to the OAuth provider; the provider redirects back to + * /oauth/callback, which the BFF also proxies; that page posts the result to + * window.opener and closes -- can't be driven through a real IdP in CI. What + * *is* testable end to end through a real browser, without any backend, is + * the client-side contract those two hops feed into: triggerOAuthAuthorization + * (client/src/api/servers.ts) opens the popup, listens for a same-window + * postMessage, and resolves/rejects the promise that drives the form's + * pending/success/error states. This stubs the popup's very first navigation + * (the oauth/authorize route) with the exact HTML shape mcpgateway's own + * _popup_notification_script produces, so the assertion is: does the whole + * chain from clicking "Connect server" to the success notification actually + * work, not just each piece in isolation (already covered by + * src/api/servers.test.ts and server/test/oauth-*.test.ts). + */ +import { test, expect } from "./fixtures/auth"; +import { APP } from "./utils/paths"; + +const GATEWAY_ID = "gw-oauth-1"; +const GATEWAY_NAME = "GitHub OAuth Test"; + +test.describe("OAuth authorization-code popup flow", () => { + test.beforeEach(async ({ page, apiMock }) => { + await apiMock.mockPermissions(); + await page.route("**/gateways?*", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ gateways: [], nextCursor: null }), + }); + }); + // Default happy-path stub for the redirect_uri default fetch (see the + // dedicated test below for the split-deployment value it actually + // returns). Submission is gated on this resolving (useMCPServerForm.ts's + // oauthRedirectUriUnresolved), so leaving it unmocked would leave + // "Connect server" permanently disabled here the way it correctly does + // for a real deployment where this fetch fails. + await page.route("**/oauth/callback-url", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ redirectUri: "https://app.example.com/oauth/callback" }), + }); + }); + }); + + test("create -> popup -> postMessage -> activate -> fetch tools", async ({ page, context }) => { + // Registered at the browser-context level (not just this page) so it also + // covers the popup window's own navigation, exactly like mcpgateway's + // popup-branch callback HTML: postMessage(payload, '*') then window.close(). + await context.route("**/oauth/authorize/**", async (route) => { + await route.fulfill({ + status: 200, + contentType: "text/html", + body: ``, + }); + }); + + await page.route("**/gateways", async (route) => { + if (route.request().method() !== "POST") return route.fallback(); + await route.fulfill({ + status: 201, + contentType: "application/json", + body: JSON.stringify({ id: GATEWAY_ID, name: GATEWAY_NAME }), + }); + }); + + // triggerOAuthAuthorization mints this before navigating the popup (see + // src/api/servers.ts) -- a same-origin, CSRF-protected POST the popup's + // own window.open() navigation can't carry itself. + await page.route("**/oauth/authorize-nonce", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ nonce: "e2e-test-nonce" }), + }); + }); + + await page.route(`**/gateways/${GATEWAY_ID}/state*`, async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ status: "success", message: "activated" }), + }); + }); + + await page.route(`**/oauth/fetch-tools/${GATEWAY_ID}`, async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ success: true, message: "Fetched 3 tools." }), + }); + }); + + await page.goto(APP.SERVERS); + await page.waitForLoadState("networkidle"); + + await page.getByRole("button", { name: /Connect/i }).click(); + + await page.getByLabel("Name").fill(GATEWAY_NAME); + await page.getByLabel("URL").fill("https://api.githubcopilot.com/mcp"); + + await page.getByRole("button", { name: "Advanced settings" }).click(); + await page.getByText("OAuth 2.0", { exact: true }).click(); + + await page.getByLabel(/Grant type/).click(); + await page.getByRole("option", { name: /Authorization code/i }).click(); + + await page.getByLabel("Issuer URL").fill("https://github.com"); + await page.getByLabel("Client ID").fill("test-client-id"); + await page.getByLabel("Client Secret").fill("test-client-secret"); // pragma: allowlist secret + await page.getByLabel("Authorization URL").fill("https://github.com/login/oauth/authorize"); + await page.getByLabel("Token URL").fill("https://github.com/login/oauth/access_token"); + + // Defaulted from the beforeEach's /oauth/callback-url stub (the + // redirect_uri fix, mcp-context-forge#6458) -- never guessed from + // window.location.origin. + await expect(page.getByLabel(/Redirect URI/i)).toHaveValue( + "https://app.example.com/oauth/callback", + ); + + await page.getByRole("button", { name: "Connect server" }).click(); + + await expect( + page.getByText(/Waiting for OAuth authorization in the popup window/i), + ).toBeVisible(); + await expect(page.getByText(/OAuth authorization successful/i)).toBeVisible(); + await expect(page.getByText(/Fetched 3 tools\./i)).toBeVisible(); + }); + + test("shows an error notification when the popup posts an error result", async ({ + page, + context, + }) => { + await context.route("**/oauth/authorize/**", async (route) => { + await route.fulfill({ + status: 200, + contentType: "text/html", + body: ``, + }); + }); + + await page.route("**/gateways", async (route) => { + if (route.request().method() !== "POST") return route.fallback(); + await route.fulfill({ + status: 201, + contentType: "application/json", + body: JSON.stringify({ id: GATEWAY_ID, name: GATEWAY_NAME }), + }); + }); + + await page.route("**/oauth/authorize-nonce", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ nonce: "e2e-test-nonce" }), + }); + }); + + await page.goto(APP.SERVERS); + await page.waitForLoadState("networkidle"); + + await page.getByRole("button", { name: /Connect/i }).click(); + await page.getByLabel("Name").fill(GATEWAY_NAME); + await page.getByLabel("URL").fill("https://api.githubcopilot.com/mcp"); + await page.getByRole("button", { name: "Advanced settings" }).click(); + await page.getByText("OAuth 2.0", { exact: true }).click(); + await page.getByLabel(/Grant type/).click(); + await page.getByRole("option", { name: /Authorization code/i }).click(); + await page.getByLabel("Issuer URL").fill("https://github.com"); + await page.getByLabel("Client ID").fill("test-client-id"); + await page.getByLabel("Client Secret").fill("test-client-secret"); // pragma: allowlist secret + await page.getByLabel("Authorization URL").fill("https://github.com/login/oauth/authorize"); + await page.getByLabel("Token URL").fill("https://github.com/login/oauth/access_token"); + + await page.getByRole("button", { name: "Connect server" }).click(); + + await expect(page.getByText(/User cancelled/i)).toBeVisible(); + // The form must stay open on error so the user can see it and retry. + await expect(page.getByRole("button", { name: "Connect server" })).toBeVisible(); + }); + + test("defaults redirect_uri to the BFF's own callback URL and submits it -- the split-deployment case", async ({ + page, + }) => { + // Stands in for server/src/routes/proxy/oauth-callback-url.ts's real + // response: a public origin distinct from this page's own origin, the + // way it would differ when mcpgateway itself isn't independently + // browser-reachable (see that route's doc comment for why the field + // can't just be left unset in that topology). + const BFF_CALLBACK_URL = "https://web.example.com/oauth/callback"; + await page.route("**/oauth/callback-url", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ redirectUri: BFF_CALLBACK_URL }), + }); + }); + + const createRequest = page.waitForRequest( + (request) => request.url().includes("/gateways") && request.method() === "POST", + ); + await page.route("**/gateways", async (route) => { + if (route.request().method() !== "POST") return route.fallback(); + await route.fulfill({ + status: 201, + contentType: "application/json", + body: JSON.stringify({ id: GATEWAY_ID, name: GATEWAY_NAME }), + }); + }); + + await page.goto(APP.SERVERS); + await page.waitForLoadState("networkidle"); + + await page.getByRole("button", { name: /Connect/i }).click(); + await page.getByLabel("Name").fill(GATEWAY_NAME); + await page.getByLabel("URL").fill("https://api.githubcopilot.com/mcp"); + await page.getByRole("button", { name: "Advanced settings" }).click(); + await page.getByText("OAuth 2.0", { exact: true }).click(); + await page.getByLabel(/Grant type/).click(); + await page.getByRole("option", { name: /Authorization code/i }).click(); + + await expect(page.getByLabel(/Redirect URI/i)).toHaveValue(BFF_CALLBACK_URL); + + await page.getByLabel("Issuer URL").fill("https://github.com"); + await page.getByLabel("Client ID").fill("test-client-id"); + await page.getByLabel("Client Secret").fill("test-client-secret"); // pragma: allowlist secret + await page.getByLabel("Authorization URL").fill("https://github.com/login/oauth/authorize"); + await page.getByLabel("Token URL").fill("https://github.com/login/oauth/access_token"); + + await page.getByRole("button", { name: "Connect server" }).click(); + + const request = await createRequest; + const body = request.postDataJSON() as { oauth_config?: { redirect_uri?: string } }; + expect(body.oauth_config?.redirect_uri).toBe(BFF_CALLBACK_URL); + }); +}); diff --git a/server/src/config.ts b/server/src/config.ts index 5439f671..ecd747f0 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -37,6 +37,21 @@ export const config = { // default), so this must stay above the upstream email-delivery timeout. passwordResetRequestTimeoutMs: Number(optional("PASSWORD_RESET_REQUEST_TIMEOUT_MS", "30000")), + // Shared by both OAuth popup proxy routes (routes/proxy/oauth-authorize.ts, + // oauth-callback.ts). GET /oauth/authorize/{id} can synchronously run DCR + // registration (an outbound call to the IdP's own registration/discovery + // endpoints) before it redirects, so this is sized for that -- more + // headroom than a plain API call needs. + oauthProxyTimeoutMs: Number(optional("OAUTH_PROXY_TIMEOUT_MS", "30000")), + + // TTL for the one-time nonce minted by POST /oauth/authorize-nonce and + // required by GET /oauth/authorize/:gatewayId (see + // lib/oauth-authorize-nonce.ts). Short-lived on purpose: the SPA consumes + // it within milliseconds of minting it, so this only needs to cover + // however long a client can plausibly sit on a minted-but-unused nonce + // (e.g. a popup blocked before it navigates), not the OAuth flow itself. + oauthAuthorizeNonceTtlSeconds: Number(optional("OAUTH_AUTHORIZE_NONCE_TTL_SECONDS", "120")), + // memory:// (default) = in-process store, no Redis needed — dev only. // See lib/memory-redis.ts. Use a real redis:// URL beyond a single // local dev process. optionalUnset so REDIS_URL="" also falls through @@ -104,6 +119,17 @@ if ( throw new Error("PASSWORD_RESET_REQUEST_TIMEOUT_MS must be a positive integer"); } +if (!Number.isSafeInteger(config.oauthProxyTimeoutMs) || config.oauthProxyTimeoutMs <= 0) { + throw new Error("OAUTH_PROXY_TIMEOUT_MS must be a positive integer"); +} + +if ( + !Number.isSafeInteger(config.oauthAuthorizeNonceTtlSeconds) || + config.oauthAuthorizeNonceTtlSeconds <= 0 +) { + throw new Error("OAUTH_AUTHORIZE_NONCE_TTL_SECONDS must be a positive integer"); +} + // COOKIE_SECURE=true (prod default) with neither PUBLIC_ORIGIN nor TRUST_PROXY // set means origin-guard.ts derives its expected origin from request.protocol, // which is wrong behind a TLS-terminating proxy (it reads "http" while the diff --git a/server/src/index.ts b/server/src/index.ts index 9c8d5ba5..69270114 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -23,6 +23,10 @@ import loginRoute from "./routes/auth/login.js"; import logoutRoute from "./routes/auth/logout.js"; import sessionRoute from "./routes/auth/session.js"; import catchAllProxyRoute from "./routes/proxy/catch-all.js"; +import oauthAuthorizeProxyRoute from "./routes/proxy/oauth-authorize.js"; +import oauthAuthorizeNonceRoute from "./routes/proxy/oauth-authorize-nonce.js"; +import oauthCallbackProxyRoute from "./routes/proxy/oauth-callback.js"; +import oauthCallbackUrlRoute from "./routes/proxy/oauth-callback-url.js"; import publicPasswordResetRoute from "./routes/proxy/public-password-reset.js"; import { startRevocationSubscriber } from "./routes/sse/revocation-subscriber.js"; import sseRoutes from "./routes/sse/routes.js"; @@ -49,6 +53,10 @@ await fastify.register(sessionRoute); await fastify.register(changePasswordRequiredRoute); await fastify.register(sseRoutes); await fastify.register(publicPasswordResetRoute); +await fastify.register(oauthAuthorizeNonceRoute); +await fastify.register(oauthAuthorizeProxyRoute); +await fastify.register(oauthCallbackProxyRoute); +await fastify.register(oauthCallbackUrlRoute); await fastify.register(catchAllProxyRoute); await fastify.register(appRoute); diff --git a/server/src/lib/memory-redis.ts b/server/src/lib/memory-redis.ts index 1ac29147..a8e5c284 100644 --- a/server/src/lib/memory-redis.ts +++ b/server/src/lib/memory-redis.ts @@ -54,6 +54,13 @@ export class MemoryRedis extends EventEmitter { return "OK"; } + async getdel(key: string): Promise { + const entry = store.get(key); + store.delete(key); + if (!entry || isExpired(entry)) return null; + return entry.value; + } + async del(key: string): Promise { return store.delete(key) ? 1 : 0; } diff --git a/server/src/lib/oauth-authorize-nonce.ts b/server/src/lib/oauth-authorize-nonce.ts new file mode 100644 index 00000000..adac4914 --- /dev/null +++ b/server/src/lib/oauth-authorize-nonce.ts @@ -0,0 +1,59 @@ +// Location: ./client/server/src/lib/oauth-authorize-nonce.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// One-time, session-bound nonce gating GET /oauth/authorize/:gatewayId (see +// routes/proxy/oauth-authorize-nonce.ts, routes/proxy/oauth-authorize.ts). +// +// isForbiddenCrossOrigin alone isn't enough on that route: window.open()'s +// top-level navigation carries no Origin header (GET navigations don't send +// one -- see origin-guard.ts), so the guard falls back to Sec-Fetch-Site, +// which reports "same-site" -- not "cross-site" -- for a request from a +// hostile *sibling* subdomain under the same registrable domain (e.g. +// evil.example.com against app.example.com). That sibling still rides the +// victim's SameSite=Lax session cookie on a top-level GET, so without this +// nonce it could otherwise trigger DCR registration and DB writes against a +// gateway of its choosing using the victim's session. +// +// The fix: mint the nonce only from a same-origin, CSRF-protected POST +// (fastify.csrfProtection -- the plugin already used for every other +// mutating browser->BFF call). A hostile sibling subdomain can't forge that +// POST: it doesn't have the CSRF token, which is handed to the SPA only in +// the JSON body of /auth/login and /auth/session, readable by same-origin +// script alone. The authorize route then requires this nonce and consumes +// it, so a captured or guessed authorize URL is usable at most once, and +// only by the session that minted it. + +import { randomUUID } from "node:crypto"; + +import { config } from "../config.js"; +import type { RedisLike } from "./session-store.js"; + +function nonceRedisKey(nonce: string): string { + return `${config.redisKeyPrefix}:oauth-authorize-nonce:${nonce}`; +} + +export async function mintOAuthAuthorizeNonce( + redis: RedisLike, + sessionId: string, +): Promise { + const nonce = randomUUID(); + await redis.setex(nonceRedisKey(nonce), config.oauthAuthorizeNonceTtlSeconds, sessionId); + return nonce; +} + +// GETDEL, not GET-then-DEL: reading and deleting must be one atomic op, or +// two concurrent requests for the same nonce can both read its session +// binding before either delete runs, letting both proceed. That deletes the +// nonce whether or not it matches -- a captured query string (browser +// history, a proxy access log, a copy-pasted URL) must not be replayable +// even by the same session that minted it. +export async function consumeOAuthAuthorizeNonce( + redis: RedisLike, + sessionId: string, + nonce: string | undefined, +): Promise { + if (!nonce) return false; + const mintedForSession = await redis.getdel(nonceRedisKey(nonce)); + return mintedForSession === sessionId; +} diff --git a/server/src/lib/oauth-upstream-forward.ts b/server/src/lib/oauth-upstream-forward.ts new file mode 100644 index 00000000..484059a5 --- /dev/null +++ b/server/src/lib/oauth-upstream-forward.ts @@ -0,0 +1,142 @@ +// Location: ./client/server/src/lib/oauth-upstream-forward.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// Shared GET-and-forward for the two OAuth popup proxy routes +// (routes/proxy/oauth-authorize.ts, oauth-callback.ts): fetch upstream with +// a timeout, forward status/Location/Content-Type/body, 502 on network +// failure. Kept in one place so a fix to one hop's forwarding behavior +// (e.g. a missing header, the empty-body edge case) can't silently drift +// out of sync with the other's -- same rationale as catch-all.ts centralizing +// rewriteUpstreamLocation/stripInboundHeaders for the /api/* proxy. +// +// Location is forwarded whenever present regardless of caller: harmless for +// oauth-callback.ts (mcpgateway's GET /oauth/callback never redirects), and +// it's the whole point for oauth-authorize.ts (the 302 to the OAuth +// provider). Never rewritten -- unlike catch-all's rewriteUpstreamLocation, +// which only rewrites Location values pointing back at config.contextforgeUrl +// -- because both hops here only ever redirect to an external OAuth +// provider's own absolute URL. +// +// htmlizeOAuthPopupErrors turns the JSON errors the BFF itself can produce on +// these two routes (401 from sessionAuth, 403 from isForbiddenCrossOrigin, +// 502 from the fetch failure below) into the same postMessage-and-close HTML +// shape mcpgateway's own callback page uses on success. Without this, those +// three failures leave the popup rendering raw JSON: nothing posts a +// message, so triggerOAuthAuthorization (src/api/servers.ts) never resolves +// or rejects until the user closes the popup by hand, at which point it +// reports "cancelled" -- which isn't what happened. Registered as this +// route's `onSend` hook, which still runs (and can still rewrite the +// payload) even though sessionAuth's preHandler is what called reply.send(). +// Upstream (mcpgateway) error responses forwarded as-is are untouched here -- +// they're whatever status/body mcpgateway itself chose to send, not one of +// these three BFF-generated cases. + +import type { FastifyReply, FastifyRequest } from "fastify"; + +const OAUTH_POPUP_ERROR_MESSAGES: Record = { + 401: { + error: "unauthenticated", + errorDescription: "Your session has expired. Please sign in again and retry.", + }, + 403: { + error: "cross_site_request_forbidden", + errorDescription: "This request could not be verified. Please retry from the original page.", + }, + 502: { + error: "upstream_unavailable", + errorDescription: "The OAuth provider could not be reached. Please try again.", + }, +}; + +function oauthPopupErrorHtml(error: string, errorDescription: string): string { + const payload = JSON.stringify({ + type: "oauth_callback", + status: "error", + error, + errorDescription, + }); + return ``; +} + +export async function htmlizeOAuthPopupErrors( + _request: FastifyRequest, + reply: FastifyReply, + payload: unknown, +): Promise { + const mapped = OAUTH_POPUP_ERROR_MESSAGES[reply.statusCode]; + if (!mapped) return payload; + + reply.header("content-type", "text/html; charset=utf-8"); + return oauthPopupErrorHtml(mapped.error, mapped.errorDescription); +} + +interface ForwardOAuthGetOptions { + /** Extra headers merged into the upstream request (e.g. the injected bearer token). */ + headers?: Record; + timeoutMs: number; + /** Included in the network-failure log line, e.g. "OAuth authorize". */ + logLabel: string; +} + +export async function forwardOAuthGet( + request: FastifyRequest, + reply: FastifyReply, + upstreamUrl: string, + { headers = {}, timeoutMs, logLabel }: ForwardOAuthGetOptions, +): Promise { + let upstreamResponse: Response; + try { + upstreamResponse = await fetch(upstreamUrl, { + method: "GET", + headers: { + accept: "text/html", + // Preserve real client IP for upstream audit logging, same as catch-all.ts. + "x-forwarded-for": request.ip, + "x-real-ip": request.ip, + ...headers, + }, + redirect: "manual", + signal: AbortSignal.timeout(timeoutMs), + }); + } catch (err) { + request.log.error( + { errorType: err instanceof Error ? err.name : typeof err }, + `upstream ${logLabel} request failed`, + ); + return reply.code(502).send({ error: "upstream_unavailable" }); + } + + const location = upstreamResponse.headers.get("location"); + if (location) reply.header("location", location); + + const contentType = upstreamResponse.headers.get("content-type"); + if (contentType) reply.header("content-type", contentType); + + let body: string; + try { + body = await upstreamResponse.text(); + } catch (err) { + // Headers already arrived (2xx/3xx/4xx status committed above), but the + // connection dropped mid-body -- e.g. the IdP redirect's response closing + // early. Without this, the thrown error would escape this function + // entirely and Fastify would emit a bare 500 with no body: a worse dead + // end for the popup than the 502 the pre-fetch failure above already + // produces, and one htmlizeOAuthPopupErrors can't help with since it only + // runs on a normal reply.send() completion. Clear the Location header set + // above so a stale redirect target doesn't ride along on a 502. + reply.removeHeader("location"); + request.log.error( + { errorType: err instanceof Error ? err.name : typeof err }, + `upstream ${logLabel} response body read failed`, + ); + return reply.code(502).send({ error: "upstream_unavailable" }); + } + + return reply.code(upstreamResponse.status).send(body || undefined); +} diff --git a/server/src/lib/origin-guard.ts b/server/src/lib/origin-guard.ts index 794e6368..00252ed8 100644 --- a/server/src/lib/origin-guard.ts +++ b/server/src/lib/origin-guard.ts @@ -22,17 +22,26 @@ export function isCrossSiteRequest(request: FastifyRequest): boolean { return request.headers["sec-fetch-site"] === "cross-site"; } -// null = no Origin header to check (caller falls back to isCrossSiteRequest). // config.publicOrigin, when set, is the source of truth (needed behind a // reverse proxy that isn't reflected in request.protocol/host — e.g. // TLS-terminated without TRUST_PROXY=true). Otherwise fall back to this // request's own scheme://host, which is only as trustworthy as // trustProxy's X-Forwarded-* handling (see config.ts). +// +// Also the source of truth for where *this* deployment's own /oauth/callback +// proxy is reachable (see routes/proxy/oauth-callback-url.ts) — the same +// value, for the same reason: it must not be guessed client-side from +// window.location.origin, which this exact derivation replaced for +// redirect_uri in the first place (mcp-context-forge#6458). +export function resolvePublicOrigin(request: FastifyRequest): string { + return config.publicOrigin ?? `${request.protocol}://${request.host}`; +} + +// null = no Origin header to check (caller falls back to isCrossSiteRequest). function originMismatch(request: FastifyRequest): boolean | null { const origin = request.headers.origin; if (typeof origin !== "string" || !origin) return null; - const expected = config.publicOrigin ?? `${request.protocol}://${request.host}`; - return origin !== expected; + return origin !== resolvePublicOrigin(request); } export function isForbiddenCrossOrigin(request: FastifyRequest): boolean { diff --git a/server/src/lib/session-store.ts b/server/src/lib/session-store.ts index dbdfb28b..c4adb028 100644 --- a/server/src/lib/session-store.ts +++ b/server/src/lib/session-store.ts @@ -18,6 +18,11 @@ export const SESSION_COOKIE_NAME = "bff_sid"; // ioredis with its own generics) and lets tests pass an in-memory fake. export interface RedisLike { get(key: string): Promise; + // Atomic read-then-delete (Redis GETDEL, >=6.2). Needed anywhere a value + // is consumed exactly once -- see oauth-authorize-nonce.ts -- since a + // separate get() + del() leaves a window between the two awaits where a + // second concurrent caller's get() can still observe the value. + getdel(key: string): Promise; setex(key: string, ttlSeconds: number, value: string): Promise; del(key: string): Promise; publish(channel: string, message: string): Promise; diff --git a/server/src/routes/proxy/oauth-authorize-nonce.ts b/server/src/routes/proxy/oauth-authorize-nonce.ts new file mode 100644 index 00000000..19b844dc --- /dev/null +++ b/server/src/routes/proxy/oauth-authorize-nonce.ts @@ -0,0 +1,29 @@ +// Location: ./client/server/src/routes/proxy/oauth-authorize-nonce.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// POST /oauth/authorize-nonce: mints the one-time nonce GET +// /oauth/authorize/:gatewayId now requires (see +// lib/oauth-authorize-nonce.ts for the threat this closes). Unlike that +// route, this one isn't a window.open() navigation -- triggerOAuthAuthorization +// (src/api/servers.ts) calls it as an ordinary same-origin fetch through the +// API client first, so it carries both the session cookie and the +// X-CSRF-Token header and fastify.csrfProtection applies exactly as it does +// on every other mutating browser->BFF call. + +import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; + +import { mintOAuthAuthorizeNonce } from "../../lib/oauth-authorize-nonce.js"; +import { setNoStore } from "../../lib/no-store.js"; + +export default async function oauthAuthorizeNonceRoute(fastify: FastifyInstance): Promise { + fastify.post( + "/oauth/authorize-nonce", + { preHandler: [fastify.sessionAuth, fastify.csrfProtection] }, + async (request: FastifyRequest, reply: FastifyReply) => { + setNoStore(reply); + const nonce = await mintOAuthAuthorizeNonce(fastify.redis, request.session!.sessionId); + return reply.send({ nonce }); + }, + ); +} diff --git a/server/src/routes/proxy/oauth-authorize.ts b/server/src/routes/proxy/oauth-authorize.ts new file mode 100644 index 00000000..9454c778 --- /dev/null +++ b/server/src/routes/proxy/oauth-authorize.ts @@ -0,0 +1,112 @@ +// Location: ./client/server/src/routes/proxy/oauth-authorize.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// Authenticated proxy for the OAuth authorization-code popup's first hop. +// +// src/api/servers.ts's triggerOAuthAuthorization opens this path with a raw +// `window.open` navigation, not through the API client, so it carries no +// Authorization header the way /api/* calls do (find-my-way would otherwise +// never have matched this route at all -- with none registered, it fell +// through to static.ts's SPA-fallback 404 handler, which unconditionally +// serves index.html; see mcp-context-forge#6458). mcpgateway's +// GET /oauth/authorize/{id} requires an authenticated user -- it may run DCR +// registration and DB writes before it 302s to the OAuth provider -- so, +// unlike /oauth/callback, this hop has to be proxied through the BFF, which +// injects the bearer token from the session the same way catch-all.ts does +// for /api/*. +// +// See oauth-callback.ts for the second leg: mcpgateway's own callback +// endpoint, which -- unlike this one -- needs no session and is proxied for +// a different reason (making the browser-facing redirect_uri work when the +// gateway itself isn't independently internet-reachable). +// +// GET is a safe method, so catch-all.ts's csrfIfUnsafe wouldn't cover this +// route even if applied -- and window.open() can't set an X-CSRF-Token +// header anyway. The session cookie is SameSite=Lax (session-store.ts), +// which still rides along on a top-level cross-site navigation, and this +// route is not side-effect-free (DCR registration, DB writes) -- so a +// hostile page could force a logged-in victim's browser into +// window.open(`${victimOrigin}/oauth/authorize/`) and +// have it execute with the victim's bearer token. isForbiddenCrossOrigin is +// the same guard login.ts and proxy-sse.ts already use for this exact +// category (cookie-authenticated, can't carry a CSRF token) -- see +// lib/origin-guard.ts. +// +// isForbiddenCrossOrigin alone still leaves a gap here that it doesn't for +// login.ts/proxy-sse.ts: a top-level GET navigation carries no Origin header +// at all (browsers only send Origin on non-GET or non-navigate requests), so +// the guard falls back to Sec-Fetch-Site -- which reports "same-site" rather +// than "cross-site" for a hostile *sibling* subdomain under the same +// registrable domain, and the SameSite=Lax session cookie rides along +// regardless. The `nonce` query param closes that: it must have been minted +// moments earlier by a same-origin, CSRF-protected POST (see +// routes/proxy/oauth-authorize-nonce.ts, lib/oauth-authorize-nonce.ts) that a +// sibling subdomain has no way to forge, and it is single-use, so a captured +// or replayed authorize URL can't be reused even by the session that minted it. +// +// redirect: "manual" (in forwardOAuthGet) so mcpgateway's 302 Location (the +// OAuth provider's own absolute URL) is forwarded to the browser as-is +// rather than followed server-side -- undici's fetch would otherwise try to +// navigate through it, leaking nothing sensitive but pointlessly making a +// request meant for the browser. + +import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; + +import { config } from "../../config.js"; +import { consumeOAuthAuthorizeNonce } from "../../lib/oauth-authorize-nonce.js"; +import { forwardOAuthGet, htmlizeOAuthPopupErrors } from "../../lib/oauth-upstream-forward.js"; +import { isForbiddenCrossOrigin } from "../../lib/origin-guard.js"; +import { setNoStore } from "../../lib/no-store.js"; +import { upstreamAuthHeader } from "../../lib/upstream-auth.js"; + +interface AuthorizeParams { + gatewayId: string; +} + +interface AuthorizeQuerystring { + nonce?: string; +} + +export default async function oauthAuthorizeProxyRoute(fastify: FastifyInstance): Promise { + fastify.get<{ Params: AuthorizeParams; Querystring: AuthorizeQuerystring }>( + "/oauth/authorize/:gatewayId", + { preHandler: fastify.sessionAuth, onSend: htmlizeOAuthPopupErrors }, + async ( + request: FastifyRequest<{ Params: AuthorizeParams; Querystring: AuthorizeQuerystring }>, + reply: FastifyReply, + ) => { + setNoStore(reply); + + if (isForbiddenCrossOrigin(request)) { + return reply.code(403).send({ error: "cross_site_request_forbidden" }); + } + + const sessionId = request.session!.sessionId; + const nonceIsValid = await consumeOAuthAuthorizeNonce( + fastify.redis, + sessionId, + request.query.nonce, + ); + if (!nonceIsValid) { + return reply.code(403).send({ error: "cross_site_request_forbidden" }); + } + + const bearerToken = request.session!.bearerToken; + const queryIndex = request.url.indexOf("?"); + const rawQuery = queryIndex === -1 ? "" : request.url.slice(queryIndex + 1); + // Strip the now-consumed nonce before forwarding -- mcpgateway's own + // /oauth/authorize/{id} has no use for it and shouldn't see it. + const forwardedParams = new URLSearchParams(rawQuery); + forwardedParams.delete("nonce"); + const query = forwardedParams.toString(); + const upstreamUrl = `${config.contextforgeUrl}/oauth/authorize/${encodeURIComponent(request.params.gatewayId)}${query ? `?${query}` : ""}`; + + return forwardOAuthGet(request, reply, upstreamUrl, { + headers: upstreamAuthHeader(bearerToken), + timeoutMs: config.oauthProxyTimeoutMs, + logLabel: "OAuth authorize", + }); + }, + ); +} diff --git a/server/src/routes/proxy/oauth-callback-url.ts b/server/src/routes/proxy/oauth-callback-url.ts new file mode 100644 index 00000000..40e1ec0f --- /dev/null +++ b/server/src/routes/proxy/oauth-callback-url.ts @@ -0,0 +1,41 @@ +// Location: ./client/server/src/routes/proxy/oauth-callback-url.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// GET /oauth/callback-url: tells the SPA where *this* deployment's own +// /oauth/callback proxy is reachable, so OAuth2Auth.tsx can default a new +// gateway's redirect_uri to it instead of leaving the field unset. +// +// Why unset isn't enough on its own: when nothing is stored, mcpgateway's +// own GET /oauth/authorize/{id} defaults redirect_uri to its *own* +// APP_DOMAIN-derived callback (see initiate_oauth_flow / +// _default_redirect_uri in mcp-context-forge's oauth_router.py). That's only +// browser-reachable when the gateway is independently exposed. In the common +// split deployment (only the web UI is public-facing, mcpgateway is not), +// the OAuth provider's redirect then lands on an address nothing answers. +// Defaulting the field to this BFF's own callback proxy instead makes the +// flow work regardless of topology: the provider redirects here, and +// oauth-callback.ts forwards the final hop to mcpgateway server-to-server +// over CONTEXTFORGE_URL, which is reachable by definition. +// +// Uses the same resolvePublicOrigin as origin-guard.ts's CSRF check — not +// window.location.origin — for the identical reason redirect_uri stopped +// being guessed client-side in the first place (mcp-context-forge#6458): +// behind a reverse proxy, the browser's own address isn't reliably this +// deployment's public one. + +import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; + +import { resolvePublicOrigin } from "../../lib/origin-guard.js"; +import { setNoStore } from "../../lib/no-store.js"; + +export default async function oauthCallbackUrlRoute(fastify: FastifyInstance): Promise { + fastify.get( + "/oauth/callback-url", + { preHandler: fastify.sessionAuth }, + async (request: FastifyRequest, reply: FastifyReply) => { + setNoStore(reply); + return reply.send({ redirectUri: `${resolvePublicOrigin(request)}/oauth/callback` }); + }, + ); +} diff --git a/server/src/routes/proxy/oauth-callback.ts b/server/src/routes/proxy/oauth-callback.ts new file mode 100644 index 00000000..0a825e33 --- /dev/null +++ b/server/src/routes/proxy/oauth-callback.ts @@ -0,0 +1,59 @@ +// Location: ./client/server/src/routes/proxy/oauth-callback.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// Unauthenticated proxy for the OAuth authorization-code popup's second hop. +// +// The OAuth provider redirects the browser here as a top-level navigation +// using whatever `redirect_uri` was registered for the flow. Two cases both +// need this route: +// +// - A gateway saved before this fix (or one an operator has explicitly +// pointed at the web UI's own origin) carries `oauth_config.redirect_uri +// = /oauth/callback`. Without this route that request fell +// through to static.ts's SPA-fallback 404 handler -- which unconditionally +// serves index.html -- landing the popup on a client route the React +// router doesn't recognize: blank page, no postMessage, stuck forever +// (observed live against mcp-context-forge#6458's fix). +// - Even once OAuth2Auth.tsx stops guessing a redirect_uri and the gateway +// defaults to its own APP_DOMAIN (see oauth-authorize.ts), that default +// is only browser-reachable if the gateway is independently exposed. In +// the common split deployment where only the web UI is public-facing, +// the redirect_uri needs to resolve to *this* origin regardless, with the +// BFF forwarding the final hop to the gateway server-to-server. +// +// mcpgateway's GET /oauth/callback requires no session -- security comes +// from the HMAC-verified `state` query param, not a cookie -- so this proxy, +// unlike oauth-authorize.ts, injects no bearer token and needs no +// `sessionAuth` preHandler. Its non-popup response path sets a short-lived +// jwt_token/CSRF cookie pair for a legacy "fetch tools" admin button; the +// React SPA always passes popup=true through to /oauth/authorize (see +// src/api/servers.ts), so mcpgateway always takes the popup branch here and +// never emits those cookies through this proxy. Set-Cookie is stripped +// regardless, matching catch-all.ts's rule that mcpgateway's own cookies +// must never reach the browser under the BFF's session-cookie boundary. + +import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; + +import { config } from "../../config.js"; +import { forwardOAuthGet, htmlizeOAuthPopupErrors } from "../../lib/oauth-upstream-forward.js"; +import { setNoStore } from "../../lib/no-store.js"; + +export default async function oauthCallbackProxyRoute(fastify: FastifyInstance): Promise { + fastify.get( + "/oauth/callback", + { onSend: htmlizeOAuthPopupErrors }, + async (request: FastifyRequest, reply: FastifyReply) => { + setNoStore(reply); + + const queryIndex = request.url.indexOf("?"); + const query = queryIndex === -1 ? "" : request.url.slice(queryIndex); + const upstreamUrl = `${config.contextforgeUrl}/oauth/callback${query}`; + + return forwardOAuthGet(request, reply, upstreamUrl, { + timeoutMs: config.oauthProxyTimeoutMs, + logLabel: "OAuth callback", + }); + }, + ); +} diff --git a/server/test/helpers/build-app.ts b/server/test/helpers/build-app.ts index 716d9572..3610caaa 100644 --- a/server/test/helpers/build-app.ts +++ b/server/test/helpers/build-app.ts @@ -5,7 +5,7 @@ // Test fixture: a Fastify instance wired the same way as src/index.ts, but // with an in-memory fake in place of plugins/redis.ts so tests don't need a // real Redis instance. Only the ioredis surface the app actually touches -// (get/setex/del/publish) is implemented. +// (get/getdel/setex/del/publish) is implemented. import Fastify, { type FastifyInstance } from "fastify"; import { type Redis } from "ioredis"; @@ -18,6 +18,10 @@ import loginRoute from "../../src/routes/auth/login.js"; import logoutRoute from "../../src/routes/auth/logout.js"; import sessionRoute from "../../src/routes/auth/session.js"; import catchAllProxyRoute from "../../src/routes/proxy/catch-all.js"; +import oauthAuthorizeProxyRoute from "../../src/routes/proxy/oauth-authorize.js"; +import oauthAuthorizeNonceRoute from "../../src/routes/proxy/oauth-authorize-nonce.js"; +import oauthCallbackProxyRoute from "../../src/routes/proxy/oauth-callback.js"; +import oauthCallbackUrlRoute from "../../src/routes/proxy/oauth-callback-url.js"; import publicPasswordResetRoute from "../../src/routes/proxy/public-password-reset.js"; export class FakeRedis { @@ -28,6 +32,12 @@ export class FakeRedis { return this.store.has(key) ? this.store.get(key)! : null; } + async getdel(key: string): Promise { + const value = this.store.has(key) ? this.store.get(key)! : null; + this.store.delete(key); + return value; + } + async setex(key: string, _ttlSeconds: number, value: string): Promise<"OK"> { this.store.set(key, value); return "OK"; @@ -64,6 +74,10 @@ export async function buildTestApp(opts: { withProxy?: boolean } = {}): Promise< await fastify.register(publicPasswordResetRoute); if (opts.withProxy) { + await fastify.register(oauthAuthorizeNonceRoute); + await fastify.register(oauthAuthorizeProxyRoute); + await fastify.register(oauthCallbackProxyRoute); + await fastify.register(oauthCallbackUrlRoute); await fastify.register(catchAllProxyRoute); } diff --git a/server/test/oauth-authorize-nonce.test.ts b/server/test/oauth-authorize-nonce.test.ts new file mode 100644 index 00000000..dd8be95f --- /dev/null +++ b/server/test/oauth-authorize-nonce.test.ts @@ -0,0 +1,144 @@ +// Location: ./client/server/test/oauth-authorize-nonce.test.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { buildTestApp, cookieHeaderFrom } from "./helpers/build-app.js"; + +async function seedSession() { + const { createSession } = await import("../src/lib/session-store.js"); + const app = await buildTestApp({ withProxy: true }); + const sessionId = await createSession(app.redis as never, { + bearerToken: "test-bearer-token", // pragma: allowlist secret + user: { email: "user@example.com", isAdmin: false }, + }); + return { app, cookie: `bff_sid=${sessionId}`, sessionId }; +} + +describe("POST /oauth/authorize-nonce", () => { + it("401s without a session cookie", async () => { + const { app } = await seedSession(); + + const response = await app.fastify.inject({ + method: "POST", + url: "/oauth/authorize-nonce", + }); + + expect(response.statusCode).toBe(401); + }); + + it("403s without a CSRF token even with a valid session", async () => { + const { app, cookie } = await seedSession(); + + const response = await app.fastify.inject({ + method: "POST", + url: "/oauth/authorize-nonce", + headers: { cookie }, + }); + + expect(response.statusCode).toBe(403); + }); + + it("mints a nonce for a same-origin, CSRF-token-carrying request", async () => { + const { app, cookie, sessionId } = await seedSession(); + + // Establish the CSRF secret cookie the same way every other mutating + // route's caller does: read /auth/session (or /auth/login) first. Doing + // that here rather than hand-rolling a token keeps this test bound to + // the real plugin contract instead of @fastify/csrf-protection internals. + const { getSession } = await import("../src/lib/session-store.js"); + const record = await getSession(app.redis as never, sessionId); + expect(record).not.toBeNull(); + + const sessionResponse = await app.fastify.inject({ + method: "GET", + url: "/auth/session", + headers: { cookie }, + }); + expect(sessionResponse.statusCode).toBe(200); + const { csrfToken } = sessionResponse.json() as { csrfToken: string }; + const setCookie = sessionResponse.headers["set-cookie"]; + const setCookieHeaders = Array.isArray(setCookie) ? setCookie : [setCookie as string]; + const fullCookie = `${cookie}; ${cookieHeaderFrom(setCookieHeaders)}`; + + // {} + Content-Type: application/json — exactly what src/api/client.ts's + // requestWithMeta actually sends for a bodyless POST (it always sets + // Content-Type: application/json; the caller passes {} rather than + // omitting the body). Asserted explicitly, not left to inject()'s + // defaults, because omitting it here would silently stop covering the + // FST_ERR_CTP_EMPTY_JSON_BODY regression below. + const response = await app.fastify.inject({ + method: "POST", + url: "/oauth/authorize-nonce", + headers: { + cookie: fullCookie, + "x-csrf-token": csrfToken, + "content-type": "application/json", + }, + payload: "{}", + }); + + expect(response.statusCode).toBe(200); + const { nonce } = response.json() as { nonce: string }; + expect(typeof nonce).toBe("string"); + expect(nonce.length).toBeGreaterThan(10); + }); + + // Regression: src/api/servers.ts's triggerOAuthAuthorization originally + // called api.post("/oauth/authorize-nonce") with no body argument. + // client.ts's requestWithMeta always sets Content-Type: application/json + // on a POST regardless of whether there's a body, so that produced a + // truly empty body under a json Content-Type -- which Fastify's default + // JSON parser rejects with FST_ERR_CTP_EMPTY_JSON_BODY before this route's + // preHandler (sessionAuth, csrfProtection) ever runs, a 400 with no useful + // error shape for the caller. The fix was client-side (pass {} explicitly, + // matching /auth/logout's existing call), not a server-side parser + // override like catch-all.ts's -- this route has exactly one caller. This + // test pins that empty-body-plus-json-Content-Type shape as the failure + // mode traceable back to the correct fix. + it("400s FST_ERR_CTP_EMPTY_JSON_BODY on a truly empty body under Content-Type: application/json", async () => { + const { app, cookie, sessionId } = await seedSession(); + const { getSession } = await import("../src/lib/session-store.js"); + expect(await getSession(app.redis as never, sessionId)).not.toBeNull(); + + const sessionResponse = await app.fastify.inject({ + method: "GET", + url: "/auth/session", + headers: { cookie }, + }); + const { csrfToken } = sessionResponse.json() as { csrfToken: string }; + const setCookie = sessionResponse.headers["set-cookie"]; + const setCookieHeaders = Array.isArray(setCookie) ? setCookie : [setCookie as string]; + const fullCookie = `${cookie}; ${cookieHeaderFrom(setCookieHeaders)}`; + + const response = await app.fastify.inject({ + method: "POST", + url: "/oauth/authorize-nonce", + headers: { + cookie: fullCookie, + "x-csrf-token": csrfToken, + "content-type": "application/json", + }, + // No payload at all -- the shape that used to reach the real server + // before triggerOAuthAuthorization was fixed to send {}. + }); + + expect(response.statusCode).toBe(400); + }); + + it("is unreachable via a cross-site Sec-Fetch-Site with no CSRF token, closing the same gap as oauth-authorize.ts", async () => { + const { app, cookie } = await seedSession(); + + const response = await app.fastify.inject({ + method: "POST", + url: "/oauth/authorize-nonce", + headers: { cookie, "sec-fetch-site": "same-site" }, + }); + + // No X-CSRF-Token header -> csrfProtection rejects regardless of + // Sec-Fetch-Site, which is the whole point: a hostile sibling subdomain + // has no way to have obtained the token in the first place. + expect(response.statusCode).toBe(403); + }); +}); diff --git a/server/test/oauth-authorize.test.ts b/server/test/oauth-authorize.test.ts new file mode 100644 index 00000000..4933e415 --- /dev/null +++ b/server/test/oauth-authorize.test.ts @@ -0,0 +1,308 @@ +// Location: ./client/server/test/oauth-authorize.test.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// CONTEXTFORGE_URL must be set before src/config.ts (and anything importing it) +// is first evaluated, so the fake upstream server is spun up and +// process.env.CONTEXTFORGE_URL set in beforeAll, with every module under test +// dynamic-imported afterwards rather than statically at the top of the file. + +import { createServer, type IncomingMessage, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +let upstream: Server; +let upstreamOrigin: string; +let lastRequest: + { path: string; authorization: string | undefined; accept: string | undefined } | undefined; + +beforeAll(async () => { + upstream = createServer((req: IncomingMessage, res) => { + lastRequest = { + path: req.url ?? "", + authorization: req.headers.authorization, + accept: req.headers.accept, + }; + + if (req.url?.startsWith("/oauth/authorize/missing-config")) { + res.writeHead(400, { "content-type": "application/json" }); + res.end(JSON.stringify({ detail: "Gateway is not configured for OAuth" })); + return; + } + + if (req.url?.startsWith("/oauth/authorize/network-error")) { + // No response at all -- destroying the socket is what makes fetch() + // reject inside forwardOAuthGet's catch, the actual trigger for its + // 502, as opposed to a well-formed non-2xx upstream response. + req.socket.destroy(); + return; + } + + // Mirrors mcpgateway's initiate_oauth_flow: redirect to the IdP's own + // absolute authorization URL. + res.writeHead(302, { location: "https://idp.example.com/authorize?client_id=abc" }); + res.end(); + }); + await new Promise((resolve) => upstream.listen(0, "127.0.0.1", () => resolve())); + const { port } = upstream.address() as AddressInfo; + upstreamOrigin = `http://127.0.0.1:${port}`; + process.env.CONTEXTFORGE_URL = upstreamOrigin; +}); + +afterAll(() => new Promise((resolve) => upstream.close(() => resolve()))); + +async function buildApp() { + const { buildTestApp } = await import("./helpers/build-app.js"); + return buildTestApp({ withProxy: true }); +} + +async function seedSession(app: Awaited>) { + const { createSession } = await import("../src/lib/session-store.js"); + const sessionId = await createSession(app.redis as never, { + bearerToken: "test-bearer-token", // pragma: allowlist secret + user: { email: "user@example.com", isAdmin: false }, + }); + return { cookie: `bff_sid=${sessionId}`, sessionId }; +} + +// Bypasses the HTTP-level POST /oauth/authorize-nonce route (covered in its +// own test file) the same way seedSession bypasses POST /auth/login -- +// exercises the authorize route's *consumption* of a nonce, not the minting +// route. +async function mintNonce(app: Awaited>, sessionId: string) { + const { mintOAuthAuthorizeNonce } = await import("../src/lib/oauth-authorize-nonce.js"); + return mintOAuthAuthorizeNonce(app.redis as never, sessionId); +} + +describe("GET /oauth/authorize/:gatewayId", () => { + it("401s without a session cookie, posting an oauth_callback error the opener can read", async () => { + const app = await buildApp(); + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/authorize/gw-1?popup=true", + }); + expect(response.statusCode).toBe(401); + expect(response.headers["content-type"]).toContain("text/html"); + expect(response.body).toContain("window.opener"); + expect(response.body).toContain('"type":"oauth_callback"'); + expect(response.body).toContain('"status":"error"'); + expect(response.body).toContain("window.close()"); + }); + + it("injects the bearer token and forwards the provider redirect untouched", async () => { + const app = await buildApp(); + const { cookie, sessionId } = await seedSession(app); + const nonce = await mintNonce(app, sessionId); + + const response = await app.fastify.inject({ + method: "GET", + url: `/oauth/authorize/gw-1?popup=true&nonce=${nonce}`, + headers: { cookie }, + }); + + expect(response.statusCode).toBe(302); + // Must reach the OAuth provider directly -- rewriting this the way + // catch-all.ts rewrites upstream /api/* redirects would send the popup + // back into the BFF instead of out to the IdP. + expect(response.headers.location).toBe("https://idp.example.com/authorize?client_id=abc"); + // nonce is BFF-internal and consumed before forwarding -- mcpgateway + // never sees it. + expect(lastRequest?.path).toBe("/oauth/authorize/gw-1?popup=true"); + expect(lastRequest?.authorization).toBe("Bearer test-bearer-token"); + }); + + it("never lets the browser override the injected Authorization header", async () => { + const app = await buildApp(); + const { cookie, sessionId } = await seedSession(app); + const nonce = await mintNonce(app, sessionId); + + await app.fastify.inject({ + method: "GET", + url: `/oauth/authorize/gw-1?nonce=${nonce}`, + headers: { cookie, authorization: "Bearer attacker-supplied-token" }, // pragma: allowlist secret + }); + + expect(lastRequest?.authorization).toBe("Bearer test-bearer-token"); + }); + + it("rejects a cross-site request before calling upstream", async () => { + const app = await buildApp(); + const { cookie } = await seedSession(app); + lastRequest = undefined; + + // Same shape as password-reset.test.ts's cross-origin case: a mismatched + // Origin header is what a hostile page forcing + // window.open(`${victimOrigin}/oauth/authorize/`) would send. This + // route runs DCR registration and DB writes with the victim's injected + // bearer token, and can't rely on a CSRF token (window.open sets no + // headers), so it needs the same isForbiddenCrossOrigin guard as + // login.ts/proxy-sse.ts. + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/authorize/gw-1?popup=true", + headers: { cookie, host: "app.example.test", origin: "https://evil.example.test" }, + }); + + expect(response.statusCode).toBe(403); + expect(response.headers["content-type"]).toContain("text/html"); + expect(response.body).toContain('"error":"cross_site_request_forbidden"'); + expect(response.body).toContain("window.close()"); + // Rejected before ever reaching upstream. + expect(lastRequest).toBeUndefined(); + }); + + it("forwards a non-redirect upstream error response as-is, not the popup HTML shape", async () => { + const app = await buildApp(); + const { cookie, sessionId } = await seedSession(app); + const nonce = await mintNonce(app, sessionId); + + const response = await app.fastify.inject({ + method: "GET", + url: `/oauth/authorize/missing-config?nonce=${nonce}`, + headers: { cookie }, + }); + + expect(response.statusCode).toBe(400); + expect(response.json()).toEqual({ detail: "Gateway is not configured for OAuth" }); + }); + + it("posts an oauth_callback error instead of raw JSON when the upstream connection fails", async () => { + const app = await buildApp(); + const { cookie, sessionId } = await seedSession(app); + const nonce = await mintNonce(app, sessionId); + + const response = await app.fastify.inject({ + method: "GET", + url: `/oauth/authorize/network-error?nonce=${nonce}`, + headers: { cookie }, + }); + + expect(response.statusCode).toBe(502); + expect(response.headers["content-type"]).toContain("text/html"); + expect(response.body).toContain('"error":"upstream_unavailable"'); + expect(response.body).toContain("window.close()"); + }); + + describe("nonce requirement", () => { + it("rejects a missing nonce before calling upstream", async () => { + const app = await buildApp(); + const { cookie } = await seedSession(app); + lastRequest = undefined; + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/authorize/gw-1?popup=true", + headers: { cookie }, + }); + + expect(response.statusCode).toBe(403); + expect(response.body).toContain('"error":"cross_site_request_forbidden"'); + expect(lastRequest).toBeUndefined(); + }); + + it("rejects a nonce that was never minted", async () => { + const app = await buildApp(); + const { cookie } = await seedSession(app); + lastRequest = undefined; + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/authorize/gw-1?popup=true&nonce=00000000-0000-0000-0000-000000000000", + headers: { cookie }, + }); + + expect(response.statusCode).toBe(403); + expect(lastRequest).toBeUndefined(); + }); + + it("rejects a nonce reused for a second request", async () => { + const app = await buildApp(); + const { cookie, sessionId } = await seedSession(app); + const nonce = await mintNonce(app, sessionId); + + const first = await app.fastify.inject({ + method: "GET", + url: `/oauth/authorize/gw-1?popup=true&nonce=${nonce}`, + headers: { cookie }, + }); + expect(first.statusCode).toBe(302); + + lastRequest = undefined; + const replay = await app.fastify.inject({ + method: "GET", + url: `/oauth/authorize/gw-1?popup=true&nonce=${nonce}`, + headers: { cookie }, + }); + + expect(replay.statusCode).toBe(403); + expect(lastRequest).toBeUndefined(); + }); + + // Regression: consumeOAuthAuthorizeNonce used to be a plain + // GET-then-DEL, leaving a window between the two awaits where a second + // concurrent request for the same nonce could still read its session + // binding before either request's DEL ran, letting both proceed. Fired + // together (not awaited sequentially, unlike the reuse test above) to + // actually exercise that interleaving. + it("lets exactly one of two concurrent requests for the same nonce through", async () => { + const app = await buildApp(); + const { cookie, sessionId } = await seedSession(app); + const nonce = await mintNonce(app, sessionId); + + const [first, second] = await Promise.all([ + app.fastify.inject({ + method: "GET", + url: `/oauth/authorize/gw-1?popup=true&nonce=${nonce}`, + headers: { cookie }, + }), + app.fastify.inject({ + method: "GET", + url: `/oauth/authorize/gw-1?popup=true&nonce=${nonce}`, + headers: { cookie }, + }), + ]); + + const statusCodes = [first.statusCode, second.statusCode].sort(); + expect(statusCodes).toEqual([302, 403]); + }); + + it("rejects a nonce minted for a different session", async () => { + const app = await buildApp(); + const { cookie } = await seedSession(app); + const { sessionId: otherSessionId } = await seedSession(app); + const nonceForOtherSession = await mintNonce(app, otherSessionId); + lastRequest = undefined; + + const response = await app.fastify.inject({ + method: "GET", + url: `/oauth/authorize/gw-1?popup=true&nonce=${nonceForOtherSession}`, + headers: { cookie }, + }); + + expect(response.statusCode).toBe(403); + expect(lastRequest).toBeUndefined(); + }); + + // Regression for the gap isForbiddenCrossOrigin leaves on its own: a + // top-level GET navigation from a hostile *sibling* subdomain sends no + // Origin header and a Sec-Fetch-Site of "same-site" (not "cross-site"), + // so the origin guard alone would let it through carrying the victim's + // SameSite=Lax session cookie. The nonce requirement is what actually + // stops it, since the sibling has no way to have minted one. + it("rejects a same-site request with no Origin header and no nonce", async () => { + const app = await buildApp(); + const { cookie } = await seedSession(app); + lastRequest = undefined; + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/authorize/gw-1?popup=true", + headers: { cookie, host: "app.example.test", "sec-fetch-site": "same-site" }, + }); + + expect(response.statusCode).toBe(403); + expect(lastRequest).toBeUndefined(); + }); + }); +}); diff --git a/server/test/oauth-callback-url.test.ts b/server/test/oauth-callback-url.test.ts new file mode 100644 index 00000000..a89642c6 --- /dev/null +++ b/server/test/oauth-callback-url.test.ts @@ -0,0 +1,82 @@ +// Location: ./client/server/test/oauth-callback-url.test.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// config.ts reads process.env.PUBLIC_ORIGIN once at import time (see +// config.ts's own module-load comment), so exercising both the "unset" and +// "set" derivations in one file needs vi.resetModules() + a fresh dynamic +// import between them -- a plain re-import would just return the +// already-evaluated, cached config. + +import { afterEach, describe, expect, it, vi } from "vitest"; + +const originalPublicOrigin = process.env.PUBLIC_ORIGIN; + +afterEach(() => { + if (originalPublicOrigin === undefined) delete process.env.PUBLIC_ORIGIN; + else process.env.PUBLIC_ORIGIN = originalPublicOrigin; +}); + +async function buildAppWithPublicOrigin(publicOrigin: string | undefined) { + vi.resetModules(); + if (publicOrigin === undefined) delete process.env.PUBLIC_ORIGIN; + else process.env.PUBLIC_ORIGIN = publicOrigin; + + const { buildTestApp } = await import("./helpers/build-app.js"); + return buildTestApp({ withProxy: true }); +} + +async function seedSession(app: Awaited>) { + const { createSession } = await import("../src/lib/session-store.js"); + const sessionId = await createSession(app.redis as never, { + bearerToken: "test-bearer-token", // pragma: allowlist secret + user: { email: "user@example.com", isAdmin: false }, + }); + return { cookie: `bff_sid=${sessionId}` }; +} + +describe("GET /oauth/callback-url", () => { + it("401s without a session cookie", async () => { + const app = await buildAppWithPublicOrigin(undefined); + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/callback-url", + }); + + expect(response.statusCode).toBe(401); + }); + + it("derives the callback URL from the request's own scheme/host when PUBLIC_ORIGIN is unset", async () => { + const app = await buildAppWithPublicOrigin(undefined); + const { cookie } = await seedSession(app); + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/callback-url", + headers: { cookie, host: "app.example.test" }, + }); + + expect(response.statusCode).toBe(200); + expect(response.json()).toEqual({ redirectUri: "http://app.example.test/oauth/callback" }); + }); + + it("prefers PUBLIC_ORIGIN over the request's own host -- the split-deployment case", async () => { + const app = await buildAppWithPublicOrigin("https://web.example.com"); + const { cookie } = await seedSession(app); + + // Even though this request arrived with a different Host (e.g. an + // internal LB hostname), PUBLIC_ORIGIN is the operator-declared source of + // truth for where the browser actually reaches this deployment -- and + // this is the same value the gateway's own APP_DOMAIN default would NOT + // resolve to in a split deployment, which is the whole point. + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/callback-url", + headers: { cookie, host: "internal-lb.local:8080" }, + }); + + expect(response.statusCode).toBe(200); + expect(response.json()).toEqual({ redirectUri: "https://web.example.com/oauth/callback" }); + }); +}); diff --git a/server/test/oauth-callback.test.ts b/server/test/oauth-callback.test.ts new file mode 100644 index 00000000..ca75adc6 --- /dev/null +++ b/server/test/oauth-callback.test.ts @@ -0,0 +1,133 @@ +// Location: ./client/server/test/oauth-callback.test.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// CONTEXTFORGE_URL must be set before src/config.ts (and anything importing it) +// is first evaluated, so the fake upstream server is spun up and +// process.env.CONTEXTFORGE_URL set in beforeAll, with every module under test +// dynamic-imported afterwards rather than statically at the top of the file. + +import { createServer, type IncomingMessage, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +let upstream: Server; +let upstreamOrigin: string; +let lastRequest: { path: string } | undefined; + +beforeAll(async () => { + upstream = createServer((req: IncomingMessage, res) => { + lastRequest = { path: req.url ?? "" }; + + if (req.url?.startsWith("/oauth/callback?network-error")) { + // No response at all -- destroying the socket is what makes fetch() + // reject inside forwardOAuthGet's catch, the actual trigger for its + // 502, as opposed to a well-formed non-2xx upstream response. + req.socket.destroy(); + return; + } + + if (req.url?.startsWith("/oauth/callback?truncated-body")) { + // Headers commit (fetch() resolves, status/Location/Content-Type are + // already readable), then the connection dies before the declared + // Content-Length is satisfied -- unlike network-error above, this + // fails inside upstreamResponse.text(), not the fetch() call itself. + res.writeHead(200, { "content-type": "text/html", "content-length": "1000" }); + res.write(""); + res.socket?.destroy(); + return; + } + + // Mirrors mcpgateway's oauth_callback popup branch: an HTML page whose + // inline script posts the result to window.opener and closes itself. + res.writeHead(200, { + "content-type": "text/html", + // mcpgateway sets its own jwt_token cookie on the non-popup branch; + // must never reach the browser through this proxy. + "set-cookie": "jwt_token=upstream-secret; HttpOnly", // pragma: allowlist secret + }); + res.end( + "", + ); + }); + await new Promise((resolve) => upstream.listen(0, "127.0.0.1", () => resolve())); + const { port } = upstream.address() as AddressInfo; + upstreamOrigin = `http://127.0.0.1:${port}`; + process.env.CONTEXTFORGE_URL = upstreamOrigin; +}); + +afterAll(() => new Promise((resolve) => upstream.close(() => resolve()))); + +async function buildApp() { + const { buildTestApp } = await import("./helpers/build-app.js"); + return buildTestApp({ withProxy: true }); +} + +describe("GET /oauth/callback", () => { + it("proxies with no session cookie required", async () => { + const app = await buildApp(); + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/callback?code=abc123&state=popup.xyz", + }); + + expect(response.statusCode).toBe(200); + expect(response.body).toContain("window.opener"); + expect(lastRequest?.path).toBe("/oauth/callback?code=abc123&state=popup.xyz"); + }); + + it("strips upstream Set-Cookie so mcpgateway's own cookie never reaches the browser", async () => { + const app = await buildApp(); + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/callback?code=abc123&state=popup.xyz", + }); + + expect(response.headers["set-cookie"]).toBeUndefined(); + }); + + it("posts an oauth_callback error instead of raw JSON when the upstream connection fails", async () => { + const app = await buildApp(); + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/callback?network-error=1&state=popup.xyz", + }); + + expect(response.statusCode).toBe(502); + expect(response.headers["content-type"]).toContain("text/html"); + expect(response.body).toContain('"error":"upstream_unavailable"'); + expect(response.body).toContain("window.close()"); + }); + + it("posts an oauth_callback error instead of a raw 500 when the body read fails after headers arrive", async () => { + const app = await buildApp(); + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/callback?truncated-body=1&state=popup.xyz", + }); + + expect(response.statusCode).toBe(502); + expect(response.headers["content-type"]).toContain("text/html"); + expect(response.body).toContain('"error":"upstream_unavailable"'); + expect(response.body).toContain("window.close()"); + }); + + it("forwards an OAuth provider error callback", async () => { + const app = await buildApp(); + + const response = await app.fastify.inject({ + method: "GET", + url: "/oauth/callback?error=access_denied&error_description=User+cancelled&state=popup.xyz", + }); + + expect(response.statusCode).toBe(200); + expect(lastRequest?.path).toBe( + "/oauth/callback?error=access_denied&error_description=User+cancelled&state=popup.xyz", + ); + }); +}); diff --git a/src/api/client.ts b/src/api/client.ts index 6e0de44e..b1bd9206 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -23,11 +23,19 @@ const LOGIN_PATH = "/app/login"; const API_PREFIX = "/api"; const SESSION_CHECK_PATH = "/auth/session"; -const BFF_OWNED_AUTH_PATHS = new Set([ +// Bare paths the BFF serves itself (not proxied to the upstream API) and +// that callers reach through this client rather than a raw window.open() +// navigation. /oauth/authorize-nonce joins the /auth/* routes here for the +// same reason: mintOAuthAuthorizeNonce (src/api/servers.ts) needs the +// session cookie + CSRF header this client already attaches to POSTs, and +// /api/* would route it to the upstream gateway, which has no such endpoint. +const BFF_OWNED_PATHS = new Set([ "/auth/login", "/auth/logout", "/auth/change-password-required", SESSION_CHECK_PATH, + "/oauth/authorize-nonce", + "/oauth/callback-url", ]); export class ApiError extends Error { @@ -69,7 +77,7 @@ function isAbsoluteUrl(path: string): boolean { /** Bare paths get /api/* (BFF proxy to the API); the BFF's own auth routes and absolute URLs pass through untouched. */ function resolveApiPath(path: string): string { if (isAbsoluteUrl(path)) return path; - if (BFF_OWNED_AUTH_PATHS.has(path)) return path; + if (BFF_OWNED_PATHS.has(path)) return path; if (path === API_PREFIX || path.startsWith(`${API_PREFIX}/`)) return path; return path.startsWith("/") ? `${API_PREFIX}${path}` : `${API_PREFIX}/${path}`; } diff --git a/src/api/servers.test.ts b/src/api/servers.test.ts index 3d99b937..aeb217c1 100644 --- a/src/api/servers.test.ts +++ b/src/api/servers.test.ts @@ -1,8 +1,23 @@ +import { waitFor } from "@testing-library/react"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { serversApi } from "./servers"; import { setCsrfToken } from "./client"; import type { GatewayTestRequest, GatewayHandshakeRequest } from "@/generated/types"; +function nonceResponse(nonce = "test-nonce"): Response { + return new Response(JSON.stringify({ nonce }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} + +function mockAuthPopup(): Window & { location: { href: string }; close: () => void } { + return { closed: false, location: { href: "" }, close: vi.fn() } as unknown as Window & { + location: { href: string }; + close: () => void; + }; +} + describe("serversApi", () => { const mockFetch = vi.fn(); @@ -146,25 +161,50 @@ describe("serversApi", () => { await expect(serversApi.triggerOAuthAuthorization("server-123")).rejects.toThrow( "Failed to open OAuth authorization window", ); + // Blocked before ever needing a nonce. + expect(mockFetch).not.toHaveBeenCalled(); }); - it("opens the popup with the correct URL and popup=true flag", () => { - const mockAuthWindow = { closed: false } as unknown as Window; + it("opens a blank popup synchronously, then navigates it to the authorize URL once the nonce arrives", async () => { + const mockAuthWindow = mockAuthPopup(); vi.spyOn(window, "open").mockReturnValue(mockAuthWindow); + mockFetch.mockResolvedValueOnce(nonceResponse("minted-nonce")); - // Don't await — just trigger the call + // Don't await — just trigger the call. window.open() must happen + // synchronously within this call (a user-gesture requirement popup + // blockers enforce), before the async nonce fetch below resolves. serversApi.triggerOAuthAuthorization("server-abc"); - expect(window.open).toHaveBeenCalledWith( - expect.stringContaining("/oauth/authorize/server-abc?popup=true"), - "oauth_authorization", - expect.any(String), + expect(window.open).toHaveBeenCalledWith("", "oauth_authorization", expect.any(String)); + expect(mockFetch).toHaveBeenCalledWith( + expect.stringContaining("/oauth/authorize-nonce"), + expect.objectContaining({ method: "POST" }), ); + + await waitFor(() => { + expect(mockAuthWindow.location.href).toContain( + "/oauth/authorize/server-abc?popup=true&nonce=minted-nonce", + ); + }); + }); + + it("closes the popup and rejects when minting the nonce fails", async () => { + const mockAuthWindow = mockAuthPopup(); + vi.spyOn(window, "open").mockReturnValue(mockAuthWindow); + mockFetch.mockRejectedValueOnce(new TypeError("Failed to fetch")); + + const promise = serversApi.triggerOAuthAuthorization("server-123"); + + await expect(promise).rejects.toThrow(); + expect(mockAuthWindow.close).toHaveBeenCalled(); + // Never navigated -- the popup must not sit on a blank page silently. + expect(mockAuthWindow.location.href).toBe(""); }); it("resolves with success data when popup sends a success postMessage", async () => { - const mockAuthWindow = { closed: false } as unknown as Window; + const mockAuthWindow = mockAuthPopup(); vi.spyOn(window, "open").mockReturnValue(mockAuthWindow); + mockFetch.mockResolvedValueOnce(nonceResponse()); const promise = serversApi.triggerOAuthAuthorization("server-123"); @@ -182,8 +222,9 @@ describe("serversApi", () => { }); it("rejects with errorDescription when popup sends an error postMessage", async () => { - const mockAuthWindow = { closed: false } as unknown as Window; + const mockAuthWindow = mockAuthPopup(); vi.spyOn(window, "open").mockReturnValue(mockAuthWindow); + mockFetch.mockResolvedValueOnce(nonceResponse()); const promise = serversApi.triggerOAuthAuthorization("server-123"); @@ -202,8 +243,9 @@ describe("serversApi", () => { }); it("falls back to the error code when errorDescription is absent", async () => { - const mockAuthWindow = { closed: false } as unknown as Window; + const mockAuthWindow = mockAuthPopup(); vi.spyOn(window, "open").mockReturnValue(mockAuthWindow); + mockFetch.mockResolvedValueOnce(nonceResponse()); const promise = serversApi.triggerOAuthAuthorization("server-123"); @@ -217,8 +259,9 @@ describe("serversApi", () => { }); it("falls back to generic message when neither errorDescription nor error is present", async () => { - const mockAuthWindow = { closed: false } as unknown as Window; + const mockAuthWindow = mockAuthPopup(); vi.spyOn(window, "open").mockReturnValue(mockAuthWindow); + mockFetch.mockResolvedValueOnce(nonceResponse()); const promise = serversApi.triggerOAuthAuthorization("server-123"); @@ -233,8 +276,9 @@ describe("serversApi", () => { it("ignores postMessages from other sources", async () => { vi.useFakeTimers(); - const mockAuthWindow = { closed: false } as unknown as Window; + const mockAuthWindow = mockAuthPopup(); vi.spyOn(window, "open").mockReturnValue(mockAuthWindow); + mockFetch.mockResolvedValueOnce(nonceResponse()); const promise = serversApi.triggerOAuthAuthorization("server-123"); @@ -256,8 +300,9 @@ describe("serversApi", () => { it("ignores non-oauth_callback messages from the popup", async () => { vi.useFakeTimers(); - const mockAuthWindow = { closed: false } as unknown as Window; + const mockAuthWindow = mockAuthPopup(); vi.spyOn(window, "open").mockReturnValue(mockAuthWindow); + mockFetch.mockResolvedValueOnce(nonceResponse()); const promise = serversApi.triggerOAuthAuthorization("server-123"); @@ -281,8 +326,9 @@ describe("serversApi", () => { it("rejects with cancellation message when user closes the popup", async () => { vi.useFakeTimers(); - const mockAuthWindow = { closed: false } as unknown as Window; + const mockAuthWindow = mockAuthPopup(); vi.spyOn(window, "open").mockReturnValue(mockAuthWindow); + mockFetch.mockResolvedValueOnce(nonceResponse()); const promise = serversApi.triggerOAuthAuthorization("server-123"); diff --git a/src/api/servers.ts b/src/api/servers.ts index 4f7b6b91..416532c1 100644 --- a/src/api/servers.ts +++ b/src/api/servers.ts @@ -173,15 +173,23 @@ export const serversApi = { /** * Trigger OAuth authorization flow for a gateway via a popup window. * - * Opens /oauth/authorize/{id}?popup=true in a centered popup. The backend - * encodes a "popup." prefix in the OAuth state so the callback page responds - * with window.opener.postMessage instead of rendering a full HTML page. + * Opens /oauth/authorize/{id}?popup=true&nonce=... in a centered popup. The + * backend encodes a "popup." prefix in the OAuth state so the callback page + * responds with window.opener.postMessage instead of rendering a full HTML + * page. + * + * The popup itself is a raw window.open() navigation, so it can't carry a + * CSRF header the way this client's other POSTs do — the nonce is the + * substitute (see server/src/lib/oauth-authorize-nonce.ts). Minting it is + * an async POST, so the popup window is opened blank *first*, synchronously + * within this click handler, and only navigated once the nonce comes back; + * doing the fetch before window.open() would lose the "opened from a user + * gesture" status popup blockers require. * * Returns a Promise that resolves on success or rejects on error / cancellation. */ triggerOAuthAuthorization: (id: string): Promise => { const validId = validateServerId(id); - const authUrl = `/oauth/authorize/${validId}?popup=true`; return new Promise((resolve, reject) => { const width = 600; @@ -190,7 +198,7 @@ export const serversApi = { const top = window.screenY + (window.outerHeight - height) / 2; const authWindow = window.open( - authUrl, + "", "oauth_authorization", `width=${width},height=${height},left=${left},top=${top},toolbar=no,location=no,status=no,menubar=no,scrollbars=yes,resizable=yes`, ); @@ -242,6 +250,24 @@ export const serversApi = { } } }, 1000); + + api + // {} — not omitted — so this isn't a Content-Type: application/json + // POST with a truly empty body, which Fastify's default JSON parser + // rejects with FST_ERR_CTP_EMPTY_JSON_BODY before the route handler + // (or even sessionAuth) ever runs. Same reasoning as /auth/logout. + .post<{ nonce: string }>("/oauth/authorize-nonce", {}) + .then(({ nonce }) => { + if (settled) return; + authWindow.location.href = `/oauth/authorize/${validId}?popup=true&nonce=${encodeURIComponent(nonce)}`; + }) + .catch((err: unknown) => { + if (settled) return; + settled = true; + cleanup(); + authWindow.close(); + reject(err instanceof Error ? err : new Error("Failed to start OAuth authorization")); + }); }); }, }; diff --git a/src/components/mcp-servers/AdvancedSettings.test.tsx b/src/components/mcp-servers/AdvancedSettings.test.tsx index 21b4bbfe..69918709 100644 --- a/src/components/mcp-servers/AdvancedSettings.test.tsx +++ b/src/components/mcp-servers/AdvancedSettings.test.tsx @@ -76,7 +76,6 @@ const makeProps = (overrides: Partial = {}): AdvancedSett onOAuthTokenUrlChange: vi.fn(), onOAuthGrantTypeChange: vi.fn(), onOAuthIssuerUrlChange: vi.fn(), - onOAuthRedirectUriChange: vi.fn(), onOAuthAuthorizationUrlChange: vi.fn(), onOAuthScopesChange: vi.fn(), onOAuthStoreTokensChange: vi.fn(), diff --git a/src/components/mcp-servers/AdvancedSettings.tsx b/src/components/mcp-servers/AdvancedSettings.tsx index 41b5dceb..fa42a406 100644 --- a/src/components/mcp-servers/AdvancedSettings.tsx +++ b/src/components/mcp-servers/AdvancedSettings.tsx @@ -51,6 +51,9 @@ interface AdvancedSettingsProps { oauthGrantType: string; oauthIssuerUrl: string; oauthRedirectUri: string; + isOAuthRedirectUriLoading?: boolean; + oauthRedirectUriError?: string; + onRetryOAuthRedirectUri?: () => void; oauthAuthorizationUrl: string; oauthScopes: string; oauthStoreTokens: boolean; @@ -62,7 +65,6 @@ interface AdvancedSettingsProps { onOAuthTokenUrlChange: (value: string) => void; onOAuthGrantTypeChange: (value: string) => void; onOAuthIssuerUrlChange: (value: string) => void; - onOAuthRedirectUriChange: (value: string) => void; onOAuthAuthorizationUrlChange: (value: string) => void; onOAuthScopesChange: (value: string) => void; onOAuthStoreTokensChange: (checked: boolean) => void; @@ -104,6 +106,9 @@ export function AdvancedSettings({ oauthGrantType, oauthIssuerUrl, oauthRedirectUri, + isOAuthRedirectUriLoading, + oauthRedirectUriError, + onRetryOAuthRedirectUri, oauthAuthorizationUrl, oauthScopes, oauthStoreTokens, @@ -115,7 +120,6 @@ export function AdvancedSettings({ onOAuthTokenUrlChange, onOAuthGrantTypeChange, onOAuthIssuerUrlChange, - onOAuthRedirectUriChange, onOAuthAuthorizationUrlChange, onOAuthScopesChange, onOAuthStoreTokensChange, @@ -169,6 +173,9 @@ export function AdvancedSettings({ grantType={oauthGrantType} issuerUrl={oauthIssuerUrl} redirectUri={oauthRedirectUri} + isRedirectUriLoading={isOAuthRedirectUriLoading} + redirectUriError={oauthRedirectUriError} + onRetryRedirectUri={onRetryOAuthRedirectUri} authorizationUrl={oauthAuthorizationUrl} scopes={oauthScopes} storeTokens={oauthStoreTokens} @@ -180,7 +187,6 @@ export function AdvancedSettings({ onTokenUrlChange={onOAuthTokenUrlChange} onGrantTypeChange={onOAuthGrantTypeChange} onIssuerUrlChange={onOAuthIssuerUrlChange} - onRedirectUriChange={onOAuthRedirectUriChange} onAuthorizationUrlChange={onOAuthAuthorizationUrlChange} onScopesChange={onOAuthScopesChange} onStoreTokensChange={onOAuthStoreTokensChange} diff --git a/src/components/mcp-servers/MCPServerForm.test.tsx b/src/components/mcp-servers/MCPServerForm.test.tsx index c9cd57ff..f3dfd89a 100644 --- a/src/components/mcp-servers/MCPServerForm.test.tsx +++ b/src/components/mcp-servers/MCPServerForm.test.tsx @@ -100,6 +100,12 @@ const server = setupServer( providers: [], }); }), + // Default happy path for the OAuth authorization_code redirect-uri default + // (see the "OAuth redirect URI default" describe block for slow/failing + // overrides of this). + http.get("/oauth/callback-url", () => { + return HttpResponse.json({ redirectUri: "https://app.example.com/oauth/callback" }); + }), ); beforeAll(() => server.listen({ onUnhandledRequest: "warn" })); @@ -929,6 +935,65 @@ describe("MCPServerForm", () => { }); }); + // Regression for the split-deployment bug the /oauth/callback-url default + // exists to fix: submitting while that fetch is still pending, or after it + // has failed, must not be possible -- otherwise oauthRedirectUri stays + // empty and the request falls back to mcpgateway's own APP_DOMAIN default. + describe("OAuth redirect URI default", () => { + async function selectOAuthAuthorizationCode() { + const user = userEvent.setup(); + renderWithRouter(); + await user.click(screen.getByRole("button", { name: /Advanced settings/i })); + await user.click(screen.getByRole("radio", { name: /OAuth 2\.0/i })); + await user.click(screen.getByRole("combobox", { name: /Grant type/i })); + await user.click(screen.getByRole("option", { name: /Authorization code/i })); + fireEvent.change(screen.getByLabelText(/^Name/i), { target: { value: "Test Server" } }); + fireEvent.change(screen.getByLabelText(/^URL/i), { + target: { value: "http://localhost:3000" }, + }); + return user; + } + + it("disables submit while the default redirect URI is still loading", async () => { + server.use( + http.get("/oauth/callback-url", async () => { + await new Promise(() => {}); // never resolves within the test + return HttpResponse.json({ redirectUri: "https://app.example.com/oauth/callback" }); + }), + ); + + await selectOAuthAuthorizationCode(); + + const submitButton = screen.getByRole("button", { name: /Connect server/i }); + expect(submitButton).toBeDisabled(); + }); + + it("shows a retryable error and keeps submit disabled when the fetch fails, then enables it once retried successfully", async () => { + let callCount = 0; + server.use( + http.get("/oauth/callback-url", () => { + callCount += 1; + if (callCount === 1) { + return HttpResponse.json({ detail: "unavailable" }, { status: 502 }); + } + return HttpResponse.json({ redirectUri: "https://app.example.com/oauth/callback" }); + }), + ); + + const user = await selectOAuthAuthorizationCode(); + + const submitButton = await screen.findByRole("button", { name: /Connect server/i }); + await waitFor(() => expect(submitButton).toBeDisabled()); + expect(screen.getByText(/Couldn't load the default redirect URI/i)).toBeInTheDocument(); + + const retryButton = screen.getByRole("button", { name: /Retry/i }); + await user.click(retryButton); + + await waitFor(() => expect(submitButton).toBeEnabled()); + expect(screen.queryByText(/Couldn't load the default redirect URI/i)).not.toBeInTheDocument(); + }); + }); + describe("CA Certificate Upload", () => { it("should render CA certificate upload section in advanced settings", async () => { const user = userEvent.setup(); diff --git a/src/components/mcp-servers/MCPServerForm.tsx b/src/components/mcp-servers/MCPServerForm.tsx index 501002c2..f9573b4f 100644 --- a/src/components/mcp-servers/MCPServerForm.tsx +++ b/src/components/mcp-servers/MCPServerForm.tsx @@ -93,7 +93,9 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ oauthIssuerUrl, setOAuthIssuerUrl, oauthRedirectUri, - setOAuthRedirectUri, + isOAuthRedirectUriLoading, + oauthRedirectUriError, + retryOAuthRedirectUri, oauthAuthorizationUrl, setOAuthAuthorizationUrl, oauthScopes, @@ -112,13 +114,6 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ setQueryParamApiKey, } = useMCPServerForm(serverId, prefill); - const handleRedirectUriChange = useCallback( - (uri: string) => { - setOAuthRedirectUri(uri); - }, - [setOAuthRedirectUri], - ); - const handleCancel = () => { setCreatedGateway(null); onToggle(); @@ -381,6 +376,16 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ oauthGrantType={oauthGrantType} oauthIssuerUrl={oauthIssuerUrl} oauthRedirectUri={oauthRedirectUri} + isOAuthRedirectUriLoading={isOAuthRedirectUriLoading} + oauthRedirectUriError={oauthRedirectUriError} + onRetryOAuthRedirectUri={() => { + // useQuery's execute() rejects on failure in addition to + // setting its own error state (which this component + // reads back as oauthRedirectUriError) -- swallow the + // rejection here so a repeat failure doesn't surface as + // an unhandled promise rejection. + retryOAuthRedirectUri().catch(() => {}); + }} oauthAuthorizationUrl={oauthAuthorizationUrl} oauthScopes={oauthScopes} oauthStoreTokens={oauthStoreTokens} @@ -392,7 +397,6 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ onOAuthTokenUrlChange={setOAuthTokenUrl} onOAuthGrantTypeChange={setOAuthGrantType} onOAuthIssuerUrlChange={setOAuthIssuerUrl} - onOAuthRedirectUriChange={handleRedirectUriChange} onOAuthAuthorizationUrlChange={setOAuthAuthorizationUrl} onOAuthScopesChange={setOAuthScopes} onOAuthStoreTokensChange={setOAuthStoreTokens} diff --git a/src/components/mcp-servers/OAuth2Auth.test.tsx b/src/components/mcp-servers/OAuth2Auth.test.tsx index 9674a518..cec55211 100644 --- a/src/components/mcp-servers/OAuth2Auth.test.tsx +++ b/src/components/mcp-servers/OAuth2Auth.test.tsx @@ -19,7 +19,6 @@ describe("OAuth2Auth", () => { password: "", // pragma: allowlist secret onGrantTypeChange: vi.fn(), onIssuerUrlChange: vi.fn(), - onRedirectUriChange: vi.fn(), onClientIdChange: vi.fn(), onClientSecretChange: vi.fn(), onTokenUrlChange: vi.fn(), @@ -145,24 +144,24 @@ describe("OAuth2Auth", () => { expect(onPasswordChange).toHaveBeenCalledWith("test-pass"); }); - it("shows a read-only derived redirect URI, lifts it into form state, and triggers the authorization URL callback", () => { + it("shows an auto-placeholder (not window.location.origin) with no stored redirect URI", () => { const onAuthorizationUrlChange = vi.fn(); - const onRedirectUriChange = vi.fn(); render( , ); const redirect = screen.getByLabelText(/Redirect URI/i); expect(redirect).toHaveAttribute("readonly"); - expect(redirect).toHaveValue(`${window.location.origin}/oauth/callback`); - expect(screen.getByRole("button", { name: "Copy to clipboard" })).toBeInTheDocument(); - expect(onRedirectUriChange).toHaveBeenCalledWith(`${window.location.origin}/oauth/callback`); + // The web UI's own origin is not where the gateway serves /oauth/callback + // in a split deployment (mcp-context-forge#6458) -- must never display or + // submit it as a guess. + expect(redirect).not.toHaveValue(`${window.location.origin}/oauth/callback`); + expect(screen.queryByRole("button", { name: "Copy to clipboard" })).not.toBeInTheDocument(); fireEvent.change(screen.getByLabelText(/Authorization URL/i), { target: { value: "https://auth.com/authorize" }, @@ -170,36 +169,18 @@ describe("OAuth2Auth", () => { expect(onAuthorizationUrlChange).toHaveBeenCalledWith("https://auth.com/authorize"); }); - it("displays a stored redirect URI verbatim without overwriting it", () => { - const onRedirectUriChange = vi.fn(); - + it("displays a stored redirect URI verbatim", () => { render( , ); expect(screen.getByLabelText(/Redirect URI/i)).toHaveValue( "https://public.example.com/oauth/callback", ); - expect(onRedirectUriChange).not.toHaveBeenCalled(); - }); - - it("does not set a redirect URI for non-authorization_code grants", () => { - const onRedirectUriChange = vi.fn(); - - render( - , - ); - - expect(onRedirectUriChange).not.toHaveBeenCalled(); }); it("only offers the password grant option when already selected (legacy)", () => { @@ -236,8 +217,14 @@ describe("OAuth2Auth", () => { }); }); - it("copies the redirect URI to clipboard when the copy button is clicked", async () => { - render(); + it("copies a stored redirect URI to clipboard when the copy button is clicked", async () => { + render( + , + ); const copyButton = screen.getByRole("button", { name: /Copy to clipboard/i }); await act(async () => { @@ -245,14 +232,20 @@ describe("OAuth2Auth", () => { }); expect(navigator.clipboard.writeText).toHaveBeenCalledWith( - `${window.location.origin}/oauth/callback`, + "https://public.example.com/oauth/callback", ); }); it("shows a check icon immediately after clicking copy and reverts after 2 s", async () => { vi.useFakeTimers(); - render(); + render( + , + ); const copyButton = screen.getByRole("button", { name: /Copy to clipboard/i }); await act(async () => { @@ -271,13 +264,14 @@ describe("OAuth2Auth", () => { }); describe("localhost warning", () => { - it("shows a localhost warning when the derived redirect URI points to localhost", () => { - // jsdom sets window.location.origin to 'http://localhost' + it("does not show the localhost warning with no stored redirect URI, even though jsdom's own origin is localhost", () => { + // jsdom sets window.location.origin to 'http://localhost' -- must not + // leak into the warning now that nothing is derived from it. render(); expect( - screen.getByText(/Redirect URIs derived from localhost will not work/i), - ).toBeInTheDocument(); + screen.queryByText(/will not work for external OAuth providers/i), + ).not.toBeInTheDocument(); }); it("does not show the localhost warning when a non-localhost stored redirect URI is used", () => { @@ -290,7 +284,7 @@ describe("OAuth2Auth", () => { ); expect( - screen.queryByText(/Redirect URIs derived from localhost will not work/i), + screen.queryByText(/will not work for external OAuth providers/i), ).not.toBeInTheDocument(); }); @@ -303,9 +297,7 @@ describe("OAuth2Auth", () => { />, ); - expect( - screen.getByText(/Redirect URIs derived from localhost will not work/i), - ).toBeInTheDocument(); + expect(screen.getByText(/will not work for external OAuth providers/i)).toBeInTheDocument(); }); }); }); diff --git a/src/components/mcp-servers/OAuth2Auth.tsx b/src/components/mcp-servers/OAuth2Auth.tsx index 914c5531..cf777419 100644 --- a/src/components/mcp-servers/OAuth2Auth.tsx +++ b/src/components/mcp-servers/OAuth2Auth.tsx @@ -2,7 +2,7 @@ import { useIntl } from "react-intl"; import { Input } from "@/components/ui/input"; import { Textarea } from "@/components/ui/textarea"; import { Checkbox } from "@/components/ui/checkbox"; -import { useEffect, useState } from "react"; +import { useState } from "react"; import { Check, Copy } from "lucide-react"; import { Button } from "@/components/ui/button"; import { @@ -17,6 +17,9 @@ interface OAuth2AuthProps { grantType: string; issuerUrl: string; redirectUri: string; + isRedirectUriLoading?: boolean; + redirectUriError?: string; + onRetryRedirectUri?: () => void; clientId: string; clientSecret: string; tokenUrl: string; @@ -28,7 +31,6 @@ interface OAuth2AuthProps { password: string; // pragma: allowlist secret onGrantTypeChange: (value: string) => void; onIssuerUrlChange: (value: string) => void; - onRedirectUriChange: (value: string) => void; onClientIdChange: (value: string) => void; onClientSecretChange: (value: string) => void; onTokenUrlChange: (value: string) => void; @@ -45,6 +47,9 @@ export function OAuth2Auth({ grantType, issuerUrl, redirectUri, + isRedirectUriLoading, + redirectUriError, + onRetryRedirectUri, clientId, clientSecret, tokenUrl, @@ -56,7 +61,6 @@ export function OAuth2Auth({ password, onGrantTypeChange, onIssuerUrlChange, - onRedirectUriChange, onClientIdChange, onClientSecretChange, onTokenUrlChange, @@ -69,23 +73,24 @@ export function OAuth2Auth({ errors, }: OAuth2AuthProps) { const intl = useIntl(); - const derivedRedirectUri = `${window.location.origin}/oauth/callback`; - const displayRedirectUri = redirectUri || derivedRedirectUri; - const isLocalRedirect = /^https?:\/\/(localhost|127\.0\.0\.1|\[::1\])(:|\/|$)/i.test( - displayRedirectUri, - ); + // Deliberately NOT derived from window.location.origin here: useMCPServerForm.ts + // fetches this deployment's own /oauth/callback proxy URL from the BFF + // (GET /oauth/callback-url, server-side-derived the same trustworthy way + // origin-guard.ts validates Origin) and defaults redirectUri to it as soon + // as the grant type is authorization_code, rather than leaving the field + // unset for mcpgateway's own APP_DOMAIN-based default to apply -- that + // default only works when mcpgateway is independently browser-reachable, + // not the common split deployment where only this web UI is (see + // mcp-context-forge#6458). This still briefly renders the placeholder + // branch below while that fetch is in flight. + const hasStoredRedirectUri = Boolean(redirectUri); + const isLocalRedirect = + hasStoredRedirectUri && + /^https?:\/\/(localhost|127\.0\.0\.1|\[::1\])(:|\/|$)/i.test(redirectUri); const [copied, setCopied] = useState(false); - // The displayed URI is what the OAuth app is registered with, so it has to be the value we - // store and send to the IdP — a display-only derivation submits no redirect_uri at all. - useEffect(() => { - if (grantType === "authorization_code" && !redirectUri) { - onRedirectUriChange(derivedRedirectUri); - } - }, [grantType, redirectUri, derivedRedirectUri, onRedirectUriChange]); - const handleCopyRedirect = () => { - void navigator.clipboard?.writeText(displayRedirectUri); + void navigator.clipboard?.writeText(redirectUri); setCopied(true); window.setTimeout(() => setCopied(false), 2000); }; @@ -161,27 +166,68 @@ export function OAuth2Auth({ > {intl.formatMessage({ id: "mcpServer.auth.oauth.redirectUriLabel" })} -
+ {hasStoredRedirectUri ? ( + <> +
+ + +
+

+ {intl.formatMessage({ id: "mcpServer.auth.oauth.redirectUriHelp" })} +

+ + ) : ( - -
-

- {intl.formatMessage({ id: "mcpServer.auth.oauth.redirectUriHelp" })} -

+ )} + {!hasStoredRedirectUri && redirectUriError && ( +
+

+ {intl.formatMessage({ id: "mcpServer.auth.oauth.redirectUriLoadError" })} +

+ {onRetryRedirectUri && ( + + )} +
+ )} + {!hasStoredRedirectUri && !redirectUriError && ( +

+ {intl.formatMessage({ id: "mcpServer.auth.oauth.redirectUriAutoHelp" })} +

+ )} {isLocalRedirect && (

{intl.formatMessage({ id: "mcpServer.auth.oauth.redirectUriLocalWarning" })} diff --git a/src/hooks/useMCPServerForm.ts b/src/hooks/useMCPServerForm.ts index 06492b19..a07d3547 100644 --- a/src/hooks/useMCPServerForm.ts +++ b/src/hooks/useMCPServerForm.ts @@ -212,6 +212,9 @@ export interface UseMCPServerFormReturn { oauthGrantType: string; oauthIssuerUrl: string; oauthRedirectUri: string; + isOAuthRedirectUriLoading: boolean; + oauthRedirectUriError: string | undefined; + retryOAuthRedirectUri: () => Promise; oauthAuthorizationUrl: string; oauthScopes: string; oauthStoreTokens: boolean; @@ -361,6 +364,45 @@ export function useMCPServerForm( const isEditMode = Boolean(gatewayId); + // Defaults oauthRedirectUri to this deployment's own /oauth/callback proxy + // (server/src/routes/proxy/oauth-callback-url.ts) rather than leaving the + // field unset. Unset relies on mcpgateway's own APP_DOMAIN-based default, + // which only works when the gateway is independently browser-reachable — + // not the case in a split deployment where only this web UI is + // public-facing. Fetched (not derived from window.location.origin) for the + // same reason redirect_uri stopped being guessed client-side in the first + // place: behind a reverse proxy the browser's own address isn't reliably + // this deployment's public one (see oauth-callback-url.ts). + const oauthRedirectUriNeeded = authType === "oauth" && oauthGrantType === "authorization_code"; + + const { + data: defaultOAuthRedirectUri, + isLoading: isOAuthRedirectUriLoading, + error: oauthRedirectUriFetchError, + refetch: retryOAuthRedirectUri, + } = useQuery<{ redirectUri: string }>("/oauth/callback-url", { + enabled: oauthRedirectUriNeeded, + }); + + useEffect(() => { + // Only fills a genuinely empty field — a value already loaded from a + // stored gateway (the effect below) or restored some other way always + // wins, and this never overwrites it. + if (defaultOAuthRedirectUri?.redirectUri && !oauthRedirectUri) { + setOAuthRedirectUri(defaultOAuthRedirectUri.redirectUri); + } + }, [defaultOAuthRedirectUri, oauthRedirectUri]); + + // True while the field this deployment's own redirect_uri belongs in is + // still empty for a reason the user hasn't chosen -- the fetch above is + // in flight or failed. Submitting in that window would send redirect_uri: + // undefined and silently fall back to mcpgateway's APP_DOMAIN default, + // reintroducing the split-deployment failure this fetch exists to fix. + const oauthRedirectUriUnresolved = + oauthRedirectUriNeeded && + !oauthRedirectUri && + (isOAuthRedirectUriLoading || Boolean(oauthRedirectUriFetchError)); + // Fetch server data when in edit mode // API response uses camelCase outer keys (via alias_generator), but oauth_config dict stays snake_case const { data: serverData, error: serverFetchError } = useQuery<{ @@ -755,6 +797,18 @@ export function useMCPServerForm( async (event: FormEvent, onSuccess?: (response?: unknown) => void) => { event.preventDefault(); + // Backstop for the isValid gate the submit button already applies -- + // an implicit form submit (Enter in a text field) isn't blocked by a + // disabled button, and this field going empty here would silently + // fall back to mcpgateway's APP_DOMAIN default (see + // oauthRedirectUriUnresolved above). + if (oauthRedirectUriUnresolved) { + setErrors({ + submit: "Still determining this deployment's OAuth redirect URI. Please try again.", + }); + return; + } + const formValid = validateForm(); if (formValid) { @@ -812,6 +866,7 @@ export function useMCPServerForm( authType, pendingOAuthGatewayId, handleOAuthFlow, + oauthRedirectUriUnresolved, ], ); @@ -831,8 +886,19 @@ export function useMCPServerForm( if (visibility === "team" && (!teamId || !teamId.trim())) { return false; } + if (oauthRedirectUriUnresolved) return false; return true; - }, [name, url, authType, oauthGrantType, oauthUsername, oauthPassword, visibility, teamId]); + }, [ + name, + url, + authType, + oauthGrantType, + oauthUsername, + oauthPassword, + visibility, + teamId, + oauthRedirectUriUnresolved, + ]); return { // State @@ -876,6 +942,9 @@ export function useMCPServerForm( clearOAuthNotification, fetchToolsNotification, clearFetchToolsNotification, + isOAuthRedirectUriLoading, + oauthRedirectUriError: oauthRedirectUriFetchError?.message, + retryOAuthRedirectUri, // Setters setName, diff --git a/src/i18n/locales/en-US/mcpServer.json b/src/i18n/locales/en-US/mcpServer.json index 3316e127..43b7ca10 100644 --- a/src/i18n/locales/en-US/mcpServer.json +++ b/src/i18n/locales/en-US/mcpServer.json @@ -194,7 +194,12 @@ "mcpServer.auth.oauth.redirectUriLabel": "Redirect URI", "mcpServer.auth.oauth.redirectUriCopy": "Copy to clipboard", "mcpServer.auth.oauth.redirectUriHelp": "Configure your OAuth app to use this redirect URI.", - "mcpServer.auth.oauth.redirectUriLocalWarning": "The server's public URL is not configured. Redirect URIs derived from localhost will not work for external OAuth providers.", + "mcpServer.auth.oauth.redirectUriAutoPlaceholder": "Determined automatically by the server", + "mcpServer.auth.oauth.redirectUriAutoHelp": "Set automatically from the server's public URL when authorization starts. Set one explicitly only if this gateway needs a different redirect URI registered with the provider.", + "mcpServer.auth.oauth.redirectUriLocalWarning": "This redirect URI points at localhost, which will not work for external OAuth providers.", + "mcpServer.auth.oauth.redirectUriLoading": "Loading redirect URI…", + "mcpServer.auth.oauth.redirectUriLoadError": "Couldn't load the default redirect URI. Save is disabled until this resolves.", + "mcpServer.auth.oauth.redirectUriRetry": "Retry", "mcpServer.auth.oauth.usernameLabel": "Username", "mcpServer.auth.oauth.usernamePlaceholder": "e.g. service-account", "mcpServer.auth.oauth.passwordLabel": "Password", diff --git a/src/i18n/locales/es-ES/mcpServer.json b/src/i18n/locales/es-ES/mcpServer.json index 1ca2a95d..031eae99 100644 --- a/src/i18n/locales/es-ES/mcpServer.json +++ b/src/i18n/locales/es-ES/mcpServer.json @@ -194,7 +194,12 @@ "mcpServer.auth.oauth.redirectUriLabel": "URI de redirección", "mcpServer.auth.oauth.redirectUriCopy": "Copiar al portapapeles", "mcpServer.auth.oauth.redirectUriHelp": "Configure su aplicación OAuth para usar esta URI de redirección.", - "mcpServer.auth.oauth.redirectUriLocalWarning": "La URL pública del servidor no está configurada. Las URI de redirección derivadas de localhost no funcionarán con proveedores OAuth externos.", + "mcpServer.auth.oauth.redirectUriAutoPlaceholder": "Determinado automáticamente por el servidor", + "mcpServer.auth.oauth.redirectUriAutoHelp": "Se configura automáticamente a partir de la URL pública del servidor cuando se inicia la autorización. Configure uno explícitamente solo si este gateway necesita una URI de redirección diferente registrada con el proveedor.", + "mcpServer.auth.oauth.redirectUriLocalWarning": "Esta URI de redirección apunta a localhost, lo cual no funcionará con proveedores OAuth externos.", + "mcpServer.auth.oauth.redirectUriLoading": "Cargando URI de redirección…", + "mcpServer.auth.oauth.redirectUriLoadError": "No se pudo cargar la URI de redirección predeterminada. Guardar está deshabilitado hasta que esto se resuelva.", + "mcpServer.auth.oauth.redirectUriRetry": "Reintentar", "mcpServer.auth.oauth.usernameLabel": "Nombre de usuario", "mcpServer.auth.oauth.usernamePlaceholder": "p. ej. service-account", "mcpServer.auth.oauth.passwordLabel": "Contraseña", diff --git a/src/i18n/locales/pt-BR/mcpServer.json b/src/i18n/locales/pt-BR/mcpServer.json index e0687e77..6a96be4c 100644 --- a/src/i18n/locales/pt-BR/mcpServer.json +++ b/src/i18n/locales/pt-BR/mcpServer.json @@ -194,7 +194,12 @@ "mcpServer.auth.oauth.redirectUriLabel": "URI de redirecionamento", "mcpServer.auth.oauth.redirectUriCopy": "Copiar para a área de transferência", "mcpServer.auth.oauth.redirectUriHelp": "Configure seu aplicativo OAuth para usar esta URI de redirecionamento.", - "mcpServer.auth.oauth.redirectUriLocalWarning": "A URL pública do servidor não está configurada. URIs de redirecionamento derivadas de localhost não funcionarão com provedores OAuth externos.", + "mcpServer.auth.oauth.redirectUriAutoPlaceholder": "Determinado automaticamente pelo servidor", + "mcpServer.auth.oauth.redirectUriAutoHelp": "Definido automaticamente a partir da URL pública do servidor quando a autorização é iniciada. Defina um valor explicitamente apenas se este gateway precisar de uma URI de redirecionamento diferente registrada no provedor.", + "mcpServer.auth.oauth.redirectUriLocalWarning": "Esta URI de redirecionamento aponta para localhost, o que não funcionará com provedores OAuth externos.", + "mcpServer.auth.oauth.redirectUriLoading": "Carregando URI de redirecionamento…", + "mcpServer.auth.oauth.redirectUriLoadError": "Não foi possível carregar a URI de redirecionamento padrão. Salvar está desabilitado até que isso seja resolvido.", + "mcpServer.auth.oauth.redirectUriRetry": "Tentar novamente", "mcpServer.auth.oauth.usernameLabel": "Nome de usuário", "mcpServer.auth.oauth.usernamePlaceholder": "ex.: service-account", "mcpServer.auth.oauth.passwordLabel": "Senha",