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: