From ebf84708b5a431f60db25fd6f8b8f5bf00cebc21 Mon Sep 17 00:00:00 2001 From: Luca Chang Date: Wed, 1 Oct 2025 15:38:09 -0700 Subject: [PATCH] chore: default to S256 code challenge if not specified --- src/client/auth.test.ts | 68 ++++++++++++++++++++++------------------- src/client/auth.ts | 31 ++++++++----------- 2 files changed, 48 insertions(+), 51 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index f28163d14..aa084ce8b 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -796,32 +796,6 @@ describe("OAuth Authorization", () => { expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); }); - it("throws error when OIDC provider does not support S256 PKCE", async () => { - // OAuth discovery fails - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // OpenID Connect discovery succeeds but without S256 support - const invalidOpenIdMetadata = { - ...validOpenIdMetadata, - code_challenge_methods_supported: ["plain"], // Missing S256 - }; - - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => invalidOpenIdMetadata, - }); - - await expect( - discoverAuthorizationServerMetadata( - "https://auth.example.com" - ) - ).rejects.toThrow("does not support S256 code challenge method required by MCP specification"); - }); - it("continues on 4xx errors", async () => { mockFetch.mockResolvedValueOnce({ ok: false, @@ -936,6 +910,17 @@ describe("OAuth Authorization", () => { code_challenge_methods_supported: ["S256"], }; + const validOpenIdMetadata = { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/auth", + token_endpoint: "https://auth.example.com/token", + jwks_uri: "https://auth.example.com/jwks", + subject_types_supported: ["public"], + id_token_signing_alg_values_supported: ["RS256"], + response_types_supported: ["code"], + code_challenge_methods_supported: ["S256"], + }; + const validClientInfo = { client_id: "client123", client_secret: "secret123", @@ -1033,11 +1018,11 @@ describe("OAuth Authorization", () => { expect(authorizationUrl.searchParams.get("prompt")).toBe("consent"); }); - it("uses metadata authorization_endpoint when provided", async () => { + it.each([validMetadata, validOpenIdMetadata])("uses metadata authorization_endpoint when provided", async (baseMetadata) => { const { authorizationUrl } = await startAuthorization( "https://auth.example.com", { - metadata: validMetadata, + metadata: baseMetadata, clientInformation: validClientInfo, redirectUrl: "http://localhost:3000/callback", } @@ -1048,9 +1033,9 @@ describe("OAuth Authorization", () => { ); }); - it("validates response type support", async () => { + it.each([validMetadata, validOpenIdMetadata])("validates response type support", async (baseMetadata) => { const metadata = { - ...validMetadata, + ...baseMetadata, response_types_supported: ["token"], // Does not support 'code' }; @@ -1063,9 +1048,28 @@ describe("OAuth Authorization", () => { ).rejects.toThrow(/does not support response type/); }); - it("validates PKCE support", async () => { + // https://github.com/modelcontextprotocol/typescript-sdk/issues/832 + it.each([validMetadata, validOpenIdMetadata])("assumes supported code challenge methods includes S256 if absent", async (baseMetadata) => { + const metadata = { + ...baseMetadata, + response_types_supported: ["code"], + code_challenge_methods_supported: undefined + }; + + const { authorizationUrl } = await startAuthorization("https://auth.example.com", { + metadata, + clientInformation: validClientInfo, + redirectUrl: "http://localhost:3000/callback", + }) + + expect(authorizationUrl.toString()).toMatch( + /^https:\/\/auth\.example\.com\/auth\?.+&code_challenge_method=S256/ + ); + }); + + it.each([validMetadata, validOpenIdMetadata])("validates supported code challenge methods includes S256 if present", async (baseMetadata) => { const metadata = { - ...validMetadata, + ...baseMetadata, response_types_supported: ["code"], code_challenge_methods_supported: ["plain"], // Does not support 'S256' }; diff --git a/src/client/auth.ts b/src/client/auth.ts index fcc320f17..e142e5f07 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -139,6 +139,9 @@ export class UnauthorizedError extends Error { type ClientAuthMethod = 'client_secret_basic' | 'client_secret_post' | 'none'; +const AUTHORIZATION_CODE_RESPONSE_TYPE = "code"; +const AUTHORIZATION_CODE_CHALLENGE_METHOD = "S256"; + /** * Determines the best client authentication method to use based on server support and client configuration. * @@ -779,16 +782,7 @@ export async function discoverAuthorizationServerMetadata( if (type === 'oauth') { return OAuthMetadataSchema.parse(await response.json()); } else { - const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); - - // MCP spec requires OIDC providers to support S256 PKCE - if (!metadata.code_challenge_methods_supported?.includes('S256')) { - throw new Error( - `Incompatible OIDC provider at ${endpointUrl}: does not support S256 code challenge method required by MCP specification` - ); - } - - return metadata; + return OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); } } @@ -816,25 +810,24 @@ export async function startAuthorization( resource?: URL; }, ): Promise<{ authorizationUrl: URL; codeVerifier: string }> { - const responseType = "code"; - const codeChallengeMethod = "S256"; + let authorizationUrl: URL; if (metadata) { authorizationUrl = new URL(metadata.authorization_endpoint); - if (!metadata.response_types_supported.includes(responseType)) { + if (!metadata.response_types_supported.includes(AUTHORIZATION_CODE_RESPONSE_TYPE)) { throw new Error( - `Incompatible auth server: does not support response type ${responseType}`, + `Incompatible auth server: does not support response type ${AUTHORIZATION_CODE_RESPONSE_TYPE}`, ); } if ( - !metadata.code_challenge_methods_supported || - !metadata.code_challenge_methods_supported.includes(codeChallengeMethod) + metadata.code_challenge_methods_supported && + !metadata.code_challenge_methods_supported.includes(AUTHORIZATION_CODE_CHALLENGE_METHOD) ) { throw new Error( - `Incompatible auth server: does not support code challenge method ${codeChallengeMethod}`, + `Incompatible auth server: does not support code challenge method ${AUTHORIZATION_CODE_CHALLENGE_METHOD}`, ); } } else { @@ -846,12 +839,12 @@ export async function startAuthorization( const codeVerifier = challenge.code_verifier; const codeChallenge = challenge.code_challenge; - authorizationUrl.searchParams.set("response_type", responseType); + authorizationUrl.searchParams.set("response_type", AUTHORIZATION_CODE_RESPONSE_TYPE); authorizationUrl.searchParams.set("client_id", clientInformation.client_id); authorizationUrl.searchParams.set("code_challenge", codeChallenge); authorizationUrl.searchParams.set( "code_challenge_method", - codeChallengeMethod, + AUTHORIZATION_CODE_CHALLENGE_METHOD, ); authorizationUrl.searchParams.set("redirect_uri", String(redirectUrl));