diff --git a/clients/tui/__tests__/App.test.tsx b/clients/tui/__tests__/App.test.tsx index b8efdb7c..dcf3be47 100644 --- a/clients/tui/__tests__/App.test.tsx +++ b/clients/tui/__tests__/App.test.tsx @@ -983,9 +983,12 @@ describe("App (OAuth flows)", () => { const r = await mount(oneHttp()); await press(r, ["c"]); await waitUntil(() => h.cb.opts !== null); - await h.cb.opts!.onCallback({ code: "abc" }); + await h.cb.opts!.onCallback({ code: "abc", iss: "https://as.example" }); await tick(); - expect(h.clientSpies.completeOAuthFlow).toHaveBeenCalledWith("abc"); + expect(h.clientSpies.completeOAuthFlow).toHaveBeenCalledWith( + "abc", + "https://as.example", + ); }); it("reports an OAuth callback error during a 401 connect", async () => { diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 289fc371..92ad40b2 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -2326,6 +2326,7 @@ function App() { connectStartRef.current = Date.now(); await client.resumeAfterOAuth(params.code, { remoteSessionId: resumeSnapshot?.remoteSessionId, + iss: params.iss, }); } catch (err) { connectStartRef.current = undefined; diff --git a/clients/web/src/test/core/auth/ema/emaFlow.test.ts b/clients/web/src/test/core/auth/ema/emaFlow.test.ts index 25ff3b78..2b71f484 100644 --- a/clients/web/src/test/core/auth/ema/emaFlow.test.ts +++ b/clients/web/src/test/core/auth/ema/emaFlow.test.ts @@ -381,6 +381,7 @@ describe("emaFlow", () => { const tokens = await completeEmaIdpAuthorizationAndMint( config, "auth-code", + IDP_ISSUER, ); expect(tokens.access_token).toBe("resource-access-token"); @@ -415,7 +416,7 @@ describe("emaFlow", () => { await startEmaIdpAuthorization(config); await expect( - completeEmaIdpAuthorizationAndMint(config, "auth-code"), + completeEmaIdpAuthorizationAndMint(config, "auth-code", IDP_ISSUER), ).rejects.toThrow(/EMA legs 2–3 \(resource token mint\)/); }); @@ -480,7 +481,7 @@ describe("emaFlow", () => { }); await expect( - completeEmaIdpAuthorizationAndMint(config, "auth-code"), + completeEmaIdpAuthorizationAndMint(config, "auth-code", IDP_ISSUER), ).rejects.toThrow( "EMA leg 1 (IdP authorization code exchange): leg1-non-error", ); diff --git a/clients/web/src/test/core/auth/ema/idpOidc.test.ts b/clients/web/src/test/core/auth/ema/idpOidc.test.ts index 1c6d73f3..1f9fec62 100644 --- a/clients/web/src/test/core/auth/ema/idpOidc.test.ts +++ b/clients/web/src/test/core/auth/ema/idpOidc.test.ts @@ -390,6 +390,7 @@ describe("completeIdpOidcAuthorization", () => { const result = await completeIdpOidcAuthorization({ idp, authorizationCode: "auth-code-xyz", + iss: IDP_ISSUER, redirectUrl: "http://127.0.0.1:3000/oauth/callback", storage, fetchFn, @@ -465,6 +466,7 @@ describe("completeIdpOidcAuthorization", () => { const result = await completeIdpOidcAuthorization({ idp, authorizationCode: "code", + iss: IDP_ISSUER, redirectUrl: "http://127.0.0.1:3000/oauth/callback", storage, fetchFn, @@ -498,10 +500,67 @@ describe("completeIdpOidcAuthorization", () => { completeIdpOidcAuthorization({ idp, authorizationCode: "code", + iss: IDP_ISSUER, redirectUrl: "http://127.0.0.1:3000/oauth/callback", storage, fetchFn, }), ).rejects.toThrow("IdP token response did not include an ID Token"); }); + + // Regression: the IdP metadata advertises + // `authorization_response_iss_parameter_supported`, so the SDK enforces + // RFC 9207 §2.4 and rejects the exchange unless the callback `iss` is + // forwarded. Dropping `iss` anywhere between the callback and + // `exchangeAuthorization` breaks EMA leg 1 against every real IdP. + it("forwards the callback iss to the token exchange (RFC 9207)", async () => { + storage = buildStorage(); + const idToken = jwtWithExp(Math.floor(Date.now() / 1000) + 3600); + const fetchFn = vi.fn( + async () => + new Response( + JSON.stringify({ + id_token: idToken, + access_token: "at", + token_type: "Bearer", + }), + ), + ); + + await expect( + completeIdpOidcAuthorization({ + idp, + authorizationCode: "code", + redirectUrl: "http://127.0.0.1:3000/oauth/callback", + storage, + fetchFn, + }), + ).rejects.toThrow(/Issuer mismatch/); + + const result = await completeIdpOidcAuthorization({ + idp, + authorizationCode: "code", + iss: IDP_ISSUER, + redirectUrl: "http://127.0.0.1:3000/oauth/callback", + storage, + fetchFn, + }); + expect(result.idToken).toBe(idToken); + }); + + it("rejects a callback iss from a different issuer (mix-up defense)", async () => { + storage = buildStorage(); + const fetchFn = vi.fn(); + await expect( + completeIdpOidcAuthorization({ + idp, + authorizationCode: "code", + iss: "https://evil.example.com", + redirectUrl: "http://127.0.0.1:3000/oauth/callback", + storage, + fetchFn, + }), + ).rejects.toThrow(/Issuer mismatch/); + expect(fetchFn).not.toHaveBeenCalled(); + }); }); diff --git a/clients/web/src/test/core/auth/runner-interactive-oauth.test.ts b/clients/web/src/test/core/auth/runner-interactive-oauth.test.ts index 1dbf740f..d572e0bb 100644 --- a/clients/web/src/test/core/auth/runner-interactive-oauth.test.ts +++ b/clients/web/src/test/core/auth/runner-interactive-oauth.test.ts @@ -56,15 +56,21 @@ function createMockCallbackServer(handlers: { } as unknown as OAuthCallbackServer; } -/** Drive the loopback callback the way a browser redirect would. */ +const CALLBACK_ISS = "https://as.example"; + +/** + * Drive the loopback callback the way a browser redirect would, including the + * RFC 9207 `iss` that real authorization servers send. + */ async function simulateCallback( handlers: MockCallbackHandlers, code = "auth-code-123", + iss: string | undefined = CALLBACK_ISS, ): Promise { if (!handlers.onCallback) { throw new Error("onCallback not registered"); } - await handlers.onCallback({ code }); + await handlers.onCallback({ code, iss }); } describe("runRunnerInteractiveOAuth", () => { @@ -145,7 +151,10 @@ describe("runRunnerInteractiveOAuth", () => { }); expect(result).toEqual({ kind: "success" }); - expect(client.completeOAuthFlow).toHaveBeenCalledWith("auth-code-123"); + expect(client.completeOAuthFlow).toHaveBeenCalledWith( + "auth-code-123", + CALLBACK_ISS, + ); expect(redirectUrlProvider.redirectUrl).toBe( "http://127.0.0.1:6276/oauth/callback", ); @@ -179,7 +188,10 @@ describe("runRunnerInteractiveOAuth", () => { authorizationUrl, ); expect(client.authenticate).not.toHaveBeenCalled(); - expect(client.completeOAuthFlow).toHaveBeenCalledWith("step-up-code"); + expect(client.completeOAuthFlow).toHaveBeenCalledWith( + "step-up-code", + CALLBACK_ISS, + ); }); it("returns insufficient_scope when post-step-up check fails", async () => { diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index 064c9301..b26538dd 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -1525,11 +1525,12 @@ describe("OAuthManager", () => { ); expect(outcome.kind).toBe("interactive"); - await manager.completeOAuthFlow("auth-code"); + await manager.completeOAuthFlow("auth-code", "https://idp.example.com"); expect(mintSpy).toHaveBeenCalledWith( expect.objectContaining({ scope: "mcp tools:read weather:read" }), "auth-code", + "https://idp.example.com", ); expect(storageOf(params).saveScope).toHaveBeenCalledWith( SERVER_URL, diff --git a/clients/web/src/test/integration/mcp/ema-mock-servers.ts b/clients/web/src/test/integration/mcp/ema-mock-servers.ts index 45ea16d0..d502cf9c 100644 --- a/clients/web/src/test/integration/mcp/ema-mock-servers.ts +++ b/clients/web/src/test/integration/mcp/ema-mock-servers.ts @@ -21,7 +21,15 @@ export const EMA_MOCK_IDP_CLIENT_SECRET = "ema-mock-idp-secret"; export const EMA_MOCK_RESOURCE_CLIENT_ID = "ema-mock-resource-client"; export const EMA_MOCK_RESOURCE_CLIENT_SECRET = "ema-mock-resource-secret"; -/** Fields required by SDK `OAuthMetadataSchema` for discovery in tests. */ +/** + * Fields required by SDK `OAuthMetadataSchema` for discovery in tests. + * + * `authorization_response_iss_parameter_supported` mirrors real-world IdPs + * (e.g. Okta/xaa.dev): once advertised, the SDK enforces RFC 9207 §2.4 and + * rejects a callback whose `iss` is missing. Keep it on so the mock stays at + * least as strict as production servers — with it absent the SDK silently takes + * its lenient path and a dropped `iss` goes unnoticed. + */ export function minimalOAuthAsMetadata( baseUrl: string, authorizationEndpoint = `${baseUrl}/authorize`, @@ -32,6 +40,7 @@ export function minimalOAuthAsMetadata( token_endpoint: `${baseUrl}/token`, response_types_supported: ["code"], token_endpoint_auth_methods_supported: ["client_secret_post"], + authorization_response_iss_parameter_supported: true, }; } diff --git a/core/auth/ema/emaFlow.ts b/core/auth/ema/emaFlow.ts index b727fbf4..eb3f40f0 100644 --- a/core/auth/ema/emaFlow.ts +++ b/core/auth/ema/emaFlow.ts @@ -125,11 +125,13 @@ export async function startEmaIdpAuthorization( export async function completeEmaIdpAuthorizationAndMint( config: EmaFlowConfig, authorizationCode: string, + iss?: string, ): Promise { try { await completeIdpOidcAuthorization({ idp: config.idp, authorizationCode, + iss, redirectUrl: config.redirectUrl, storage: config.storage, fetchFn: config.fetchFn, diff --git a/core/auth/ema/idpOidc.ts b/core/auth/ema/idpOidc.ts index 78fc6978..60f3bdbb 100644 --- a/core/auth/ema/idpOidc.ts +++ b/core/auth/ema/idpOidc.ts @@ -97,6 +97,12 @@ export async function startIdpOidcAuthorization(params: { export async function completeIdpOidcAuthorization(params: { idp: EnterpriseManagedAuthIdpConfig; authorizationCode: string; + /** + * RFC 9207 `iss` from the IdP callback. Forwarded to the SDK, which rejects + * the exchange when the IdP metadata advertises + * `authorization_response_iss_parameter_supported` but no `iss` arrives. + */ + iss?: string; redirectUrl: string; storage: OAuthStorage; fetchFn?: typeof fetch; @@ -127,6 +133,7 @@ export async function completeIdpOidcAuthorization(params: { metadata, clientInformation, authorizationCode: params.authorizationCode, + iss: params.iss, codeVerifier, redirectUri: params.redirectUrl, fetchFn: params.fetchFn, diff --git a/core/auth/mcpAuth.ts b/core/auth/mcpAuth.ts index 45b7b1e6..4638605e 100644 --- a/core/auth/mcpAuth.ts +++ b/core/auth/mcpAuth.ts @@ -44,7 +44,7 @@ export type McpAuthResult = "AUTHORIZED" | "REDIRECT"; export interface McpAuthOptions { serverUrl: string | URL; authorizationCode?: string; - /** RFC 9207 callback `iss` — forwarded on v2 upgrade; ignored by v1 SDK `auth()`. */ + /** RFC 9207 callback `iss`, validated by the SDK against the metadata issuer. */ iss?: string; scope?: string; resourceMetadataUrl?: URL; @@ -74,6 +74,7 @@ export async function mcpAuth( return sdkAuth(provider, { serverUrl: options.serverUrl, authorizationCode: options.authorizationCode, + iss: options.iss, scope: options.scope, resourceMetadataUrl: options.resourceMetadataUrl, fetchFn: options.fetchFn, diff --git a/core/auth/node/oauth-callback-server.ts b/core/auth/node/oauth-callback-server.ts index 2b5149ec..37a0cce6 100644 --- a/core/auth/node/oauth-callback-server.ts +++ b/core/auth/node/oauth-callback-server.ts @@ -30,6 +30,8 @@ function escapeHtml(s: string): string { export type OAuthCallbackHandler = (params: { code: string; state?: string; + /** RFC 9207 `iss`, when the authorization server sends it. */ + iss?: string; }) => Promise; export type OAuthErrorHandler = (params: { @@ -181,7 +183,7 @@ export class OAuthCallbackServer { this.handled = true; const cb = this.onCallback; if (cb) { - cb({ code: params.code, state }) + cb({ code: params.code, state, iss: params.iss }) .then(() => { send(200, SUCCESS_HTML); void this.stop(); diff --git a/core/auth/node/runner-interactive-oauth.ts b/core/auth/node/runner-interactive-oauth.ts index f68a3280..edbb394d 100644 --- a/core/auth/node/runner-interactive-oauth.ts +++ b/core/auth/node/runner-interactive-oauth.ts @@ -9,7 +9,7 @@ import type { RunnerOAuthCallbackConfig } from "./runner-oauth-callback.js"; export interface RunnerInteractiveOAuthClient { authenticate(): Promise; beginInteractiveAuthorization(authorizationUrl: URL): Promise; - completeOAuthFlow(authorizationCode: string): Promise; + completeOAuthFlow(authorizationCode: string, iss?: string): Promise; checkAuthChallengeSatisfied(challenge: AuthChallenge): Promise; } @@ -67,7 +67,7 @@ export async function runRunnerInteractiveOAuth( path: options.callbackListen.pathname, onCallback: async (params) => { try { - await options.client.completeOAuthFlow(params.code); + await options.client.completeOAuthFlow(params.code, params.iss); flowResolve(); } catch (err) { flowReject(err instanceof Error ? err : new Error(String(err))); diff --git a/core/auth/types.ts b/core/auth/types.ts index e40a7416..82b0bfb4 100644 --- a/core/auth/types.ts +++ b/core/auth/types.ts @@ -105,6 +105,12 @@ export type CallbackParams = successful: true; // The authorization code is generated by the authorization server. code: string; + // RFC 9207 `iss`. Present only when the authorization server includes it. + // Forwarded to the SDK, which validates it against the issuer in the + // validated metadata (RFC 9207 §2.4) to detect mix-up attacks. Servers + // advertising `authorization_response_iss_parameter_supported: true` MUST + // send it, and the SDK rejects the callback when it is missing. + iss?: string; } | { successful: false; diff --git a/core/auth/utils.ts b/core/auth/utils.ts index c15bb974..95ebccca 100644 --- a/core/auth/utils.ts +++ b/core/auth/utils.ts @@ -89,7 +89,10 @@ export const parseOAuthCallbackParams = (location: string): CallbackParams => { const code = params.get("code"); if (code) { - return { successful: true, code }; + const iss = params.get("iss"); + return iss === null + ? { successful: true, code } + : { successful: true, code, iss }; } const error = params.get("error"); diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 6d34e504..c2452dc3 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -3360,9 +3360,9 @@ export class InspectorClient extends InspectorClientEventTarget { */ async resumeAfterOAuth( authorizationCode: string, - options?: { remoteSessionId?: string }, + options?: { remoteSessionId?: string; iss?: string }, ): Promise { - await this.completeOAuthFlow(authorizationCode); + await this.completeOAuthFlow(authorizationCode, options?.iss); const remoteSessionId = options?.remoteSessionId; const transport = this.baseTransport; diff --git a/core/mcp/oauthManager.ts b/core/mcp/oauthManager.ts index 64b0b461..4297f70c 100644 --- a/core/mcp/oauthManager.ts +++ b/core/mcp/oauthManager.ts @@ -293,6 +293,7 @@ export class OAuthManager { const tokens = await completeEmaIdpAuthorizationAndMint( config, authorizationCode, + iss, ); const requestedScope = this.pendingAuthorizationScope; const scopeToPersist = resolvePersistedScopeAfterGrant(