From 05698647f8237614d80cfe01ffdadafceef39950 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 16 Aug 2026 21:13:51 -0400 Subject: [PATCH] fix(auth): never apply endpoint overrides on the enterprise-managed leg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overrides are per-server configuration for the *resource* server's authorization server, but the fetch wrapper that applies them is shared with the enterprise-managed (EMA) flow, whose IdP OIDC discovery runs through the same fetch and satisfies the metadata predicate. An authorization override could therefore redirect the IdP login to the resource AS, and a token override could send the IdP code or refresh token there. OAuthManager.getEndpointOverrides() now returns nothing when the server is enterprise-managed — the same boundary redirectToExternalAuthorization draws for the custom authorization parameters (#2018) — with a regression test, the note surfaced on both form fields, and the rule documented. Signed-off-by: cliffhall --- .../ServerSettingsForm.test.tsx | 28 +++++++++++++++++++ .../ServerSettingsForm/ServerSettingsForm.tsx | 11 ++++++-- .../src/test/core/mcp/oauthManager.test.ts | 23 +++++++++++++++ core/auth/endpointOverrides.ts | 8 +++++- core/mcp/oauthManager.ts | 10 +++++++ docs/mcp-server-configuration.md | 2 ++ 6 files changed, 79 insertions(+), 3 deletions(-) diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index ac26d2eb..09ded171 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -999,6 +999,34 @@ describe("ServerSettingsForm", () => { ); }); + it("says the endpoint overrides are unused under enterprise-managed auth", () => { + const { rerender } = renderWithMantine( + , + ); + expect( + screen.getAllByText( + /Not applied while Enterprise-managed authorization is on/, + ), + ).toHaveLength(2); + + rerender( + , + ); + expect( + screen.queryByText( + /Not applied while Enterprise-managed authorization is on/, + ), + ).not.toBeInTheDocument(); + }); + it("flags an endpoint override that is not an absolute http(s) URL", () => { renderWithMantine( p.key.trim()) .filter((key) => isReservedAuthorizationParam(key)); @@ -868,7 +875,7 @@ export function ServerSettingsForm({ { tokenUrl: "https://staging.test/token", }); }); + + // The EMA leg authorizes against the enterprise IdP — a different + // authorization server, whose OIDC discovery runs through the same fetch the + // override wrapper is installed on. Applying a resource-AS override there + // would redirect the IdP login (or the IdP code exchange) to the resource + // AS. Regression test for the review finding on PR #2037. + it("suppresses the overrides under enterprise-managed authorization", () => { + const manager = new OAuthManager(createMockParams()); + manager.setOAuthConfig({ + authorizationUrl: "https://staging.test/authorize", + tokenUrl: "https://staging.test/token", + }); + expect(manager.getEndpointOverrides()).toBeDefined(); + + manager.setOAuthConfig({ enterpriseManaged: true }); + expect(manager.getEndpointOverrides()).toBeUndefined(); + + manager.setOAuthConfig({ enterpriseManaged: false }); + expect(manager.getEndpointOverrides()).toEqual({ + authorizationUrl: "https://staging.test/authorize", + tokenUrl: "https://staging.test/token", + }); + }); }); describe("getServerUrl propagation", () => { diff --git a/core/auth/endpointOverrides.ts b/core/auth/endpointOverrides.ts index 00528cad..7ae07155 100644 --- a/core/auth/endpointOverrides.ts +++ b/core/auth/endpointOverrides.ts @@ -22,7 +22,7 @@ * sees both is the `fetchFn` the SDK uses for discovery, so the override is * applied to the metadata document in flight. * - * Two consequences worth knowing: + * Three consequences worth knowing: * * - **A metadata document is required.** When discovery returns nothing, the SDK * falls back to `/authorize` and `/token` on the authorization server's origin @@ -30,6 +30,12 @@ * overriding "whatever urls the authorization server returns" — and a server * publishing no metadata is already on a path where the endpoints are not * being advertised in the first place. + * - **They do not apply to the enterprise-managed (EMA) leg.** That flow + * authorizes against the enterprise IdP — a different authorization server — + * and its OIDC discovery runs through this same fetch, so `OAuthManager` + * suppresses the overrides when the server is enterprise-managed. This mirrors + * `redirectToExternalAuthorization` skipping the custom authorization + * parameters (#2018). * - **The Network tab shows the patched document.** The wrapper is applied to the * base fetch, inside the request tracker, so what the tab renders is the * metadata as the flow consumed it. That is the useful reading: it explains why diff --git a/core/mcp/oauthManager.ts b/core/mcp/oauthManager.ts index b17b3d19..59846e2a 100644 --- a/core/mcp/oauthManager.ts +++ b/core/mcp/oauthManager.ts @@ -114,8 +114,18 @@ export class OAuthManager { * `setOAuthConfig` between requests takes effect without rebuilding the * client's fetch. `InspectorClient` reads this from inside its fetch wrapper. * (#1906) + * + * Returns nothing under enterprise-managed authorization, for the same reason + * `redirectToExternalAuthorization` skips the custom authorization parameters + * (#2018): the EMA leg authorizes against the enterprise IdP, a *different* + * authorization server, and its OIDC discovery runs through this same fetch + * (`emaFlow` → `idpOidc`). Rewriting that document would point the IdP login — + * or the IdP code/refresh-token exchange — at the resource server's + * authorization server, which is both wrong and a way to leak an IdP + * credential across an authorization-server boundary. */ getEndpointOverrides(): OAuthEndpointOverrides | undefined { + if (this.isEnterpriseManaged()) return undefined; const { authorizationUrl, tokenUrl } = this.oauthConfig; if (!authorizationUrl && !tokenUrl) return undefined; return { diff --git a/docs/mcp-server-configuration.md b/docs/mcp-server-configuration.md index fa3e9409..c317adfb 100644 --- a/docs/mcp-server-configuration.md +++ b/docs/mcp-server-configuration.md @@ -181,6 +181,8 @@ These have no analog in the broader `mcp.json` ecosystem. Each is **omitted on w `oauth.authorizationUrl` and `oauth.tokenUrl` override the `authorization_endpoint` and `token_endpoint` that authorization-server metadata discovery resolved. The Inspector deliberately has no such fields by default — it resolves both from the AS's metadata document, exactly as a real MCP host does — but a server under development often advertises its *production* authorization server while you want to hit staging. Set either (they are independent) to an absolute `http(s)` URL and it replaces what the metadata returned, for both the authorization request and the token request; leave blank to use discovery. A malformed value is flagged inline in the form and dropped with a warning at connect time rather than failing the connection. Edit them in Server Settings → Authorization ("Authorization URL override" / "Token URL override"). +They are **not applied under enterprise-managed authorization** (`oauth.enterpriseManaged`), for the same reason `oauth.authorizationParams` isn't: that flow authorizes against the enterprise IdP — a different authorization server, whose OIDC discovery would otherwise be rewritten too, pointing the IdP login (or the IdP code exchange) at the resource server's authorization server. + > Because the override is applied to the *discovered metadata document*, a server whose authorization server publishes no metadata at all is unaffected — there the SDK falls back to `/authorize` and `/token` on the AS origin, and there is nothing to override. A catalog carrying these fields: