Skip to content

Commit ebf8470

Browse files
committed
chore: default to S256 code challenge if not specified
1 parent 856d9ec commit ebf8470

File tree

2 files changed

+48
-51
lines changed

2 files changed

+48
-51
lines changed

src/client/auth.test.ts

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -796,32 +796,6 @@ describe("OAuth Authorization", () => {
796796
expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server");
797797
});
798798

799-
it("throws error when OIDC provider does not support S256 PKCE", async () => {
800-
// OAuth discovery fails
801-
mockFetch.mockResolvedValueOnce({
802-
ok: false,
803-
status: 404,
804-
});
805-
806-
// OpenID Connect discovery succeeds but without S256 support
807-
const invalidOpenIdMetadata = {
808-
...validOpenIdMetadata,
809-
code_challenge_methods_supported: ["plain"], // Missing S256
810-
};
811-
812-
mockFetch.mockResolvedValueOnce({
813-
ok: true,
814-
status: 200,
815-
json: async () => invalidOpenIdMetadata,
816-
});
817-
818-
await expect(
819-
discoverAuthorizationServerMetadata(
820-
"https://auth.example.com"
821-
)
822-
).rejects.toThrow("does not support S256 code challenge method required by MCP specification");
823-
});
824-
825799
it("continues on 4xx errors", async () => {
826800
mockFetch.mockResolvedValueOnce({
827801
ok: false,
@@ -936,6 +910,17 @@ describe("OAuth Authorization", () => {
936910
code_challenge_methods_supported: ["S256"],
937911
};
938912

913+
const validOpenIdMetadata = {
914+
issuer: "https://auth.example.com",
915+
authorization_endpoint: "https://auth.example.com/auth",
916+
token_endpoint: "https://auth.example.com/token",
917+
jwks_uri: "https://auth.example.com/jwks",
918+
subject_types_supported: ["public"],
919+
id_token_signing_alg_values_supported: ["RS256"],
920+
response_types_supported: ["code"],
921+
code_challenge_methods_supported: ["S256"],
922+
};
923+
939924
const validClientInfo = {
940925
client_id: "client123",
941926
client_secret: "secret123",
@@ -1033,11 +1018,11 @@ describe("OAuth Authorization", () => {
10331018
expect(authorizationUrl.searchParams.get("prompt")).toBe("consent");
10341019
});
10351020

1036-
it("uses metadata authorization_endpoint when provided", async () => {
1021+
it.each([validMetadata, validOpenIdMetadata])("uses metadata authorization_endpoint when provided", async (baseMetadata) => {
10371022
const { authorizationUrl } = await startAuthorization(
10381023
"https://auth.example.com",
10391024
{
1040-
metadata: validMetadata,
1025+
metadata: baseMetadata,
10411026
clientInformation: validClientInfo,
10421027
redirectUrl: "http://localhost:3000/callback",
10431028
}
@@ -1048,9 +1033,9 @@ describe("OAuth Authorization", () => {
10481033
);
10491034
});
10501035

1051-
it("validates response type support", async () => {
1036+
it.each([validMetadata, validOpenIdMetadata])("validates response type support", async (baseMetadata) => {
10521037
const metadata = {
1053-
...validMetadata,
1038+
...baseMetadata,
10541039
response_types_supported: ["token"], // Does not support 'code'
10551040
};
10561041

@@ -1063,9 +1048,28 @@ describe("OAuth Authorization", () => {
10631048
).rejects.toThrow(/does not support response type/);
10641049
});
10651050

1066-
it("validates PKCE support", async () => {
1051+
// https://github.com/modelcontextprotocol/typescript-sdk/issues/832
1052+
it.each([validMetadata, validOpenIdMetadata])("assumes supported code challenge methods includes S256 if absent", async (baseMetadata) => {
1053+
const metadata = {
1054+
...baseMetadata,
1055+
response_types_supported: ["code"],
1056+
code_challenge_methods_supported: undefined
1057+
};
1058+
1059+
const { authorizationUrl } = await startAuthorization("https://auth.example.com", {
1060+
metadata,
1061+
clientInformation: validClientInfo,
1062+
redirectUrl: "http://localhost:3000/callback",
1063+
})
1064+
1065+
expect(authorizationUrl.toString()).toMatch(
1066+
/^https:\/\/auth\.example\.com\/auth\?.+&code_challenge_method=S256/
1067+
);
1068+
});
1069+
1070+
it.each([validMetadata, validOpenIdMetadata])("validates supported code challenge methods includes S256 if present", async (baseMetadata) => {
10671071
const metadata = {
1068-
...validMetadata,
1072+
...baseMetadata,
10691073
response_types_supported: ["code"],
10701074
code_challenge_methods_supported: ["plain"], // Does not support 'S256'
10711075
};

src/client/auth.ts

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,9 @@ export class UnauthorizedError extends Error {
139139

140140
type ClientAuthMethod = 'client_secret_basic' | 'client_secret_post' | 'none';
141141

142+
const AUTHORIZATION_CODE_RESPONSE_TYPE = "code";
143+
const AUTHORIZATION_CODE_CHALLENGE_METHOD = "S256";
144+
142145
/**
143146
* Determines the best client authentication method to use based on server support and client configuration.
144147
*
@@ -779,16 +782,7 @@ export async function discoverAuthorizationServerMetadata(
779782
if (type === 'oauth') {
780783
return OAuthMetadataSchema.parse(await response.json());
781784
} else {
782-
const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(await response.json());
783-
784-
// MCP spec requires OIDC providers to support S256 PKCE
785-
if (!metadata.code_challenge_methods_supported?.includes('S256')) {
786-
throw new Error(
787-
`Incompatible OIDC provider at ${endpointUrl}: does not support S256 code challenge method required by MCP specification`
788-
);
789-
}
790-
791-
return metadata;
785+
return OpenIdProviderDiscoveryMetadataSchema.parse(await response.json());
792786
}
793787
}
794788

@@ -816,25 +810,24 @@ export async function startAuthorization(
816810
resource?: URL;
817811
},
818812
): Promise<{ authorizationUrl: URL; codeVerifier: string }> {
819-
const responseType = "code";
820-
const codeChallengeMethod = "S256";
813+
821814

822815
let authorizationUrl: URL;
823816
if (metadata) {
824817
authorizationUrl = new URL(metadata.authorization_endpoint);
825818

826-
if (!metadata.response_types_supported.includes(responseType)) {
819+
if (!metadata.response_types_supported.includes(AUTHORIZATION_CODE_RESPONSE_TYPE)) {
827820
throw new Error(
828-
`Incompatible auth server: does not support response type ${responseType}`,
821+
`Incompatible auth server: does not support response type ${AUTHORIZATION_CODE_RESPONSE_TYPE}`,
829822
);
830823
}
831824

832825
if (
833-
!metadata.code_challenge_methods_supported ||
834-
!metadata.code_challenge_methods_supported.includes(codeChallengeMethod)
826+
metadata.code_challenge_methods_supported &&
827+
!metadata.code_challenge_methods_supported.includes(AUTHORIZATION_CODE_CHALLENGE_METHOD)
835828
) {
836829
throw new Error(
837-
`Incompatible auth server: does not support code challenge method ${codeChallengeMethod}`,
830+
`Incompatible auth server: does not support code challenge method ${AUTHORIZATION_CODE_CHALLENGE_METHOD}`,
838831
);
839832
}
840833
} else {
@@ -846,12 +839,12 @@ export async function startAuthorization(
846839
const codeVerifier = challenge.code_verifier;
847840
const codeChallenge = challenge.code_challenge;
848841

849-
authorizationUrl.searchParams.set("response_type", responseType);
842+
authorizationUrl.searchParams.set("response_type", AUTHORIZATION_CODE_RESPONSE_TYPE);
850843
authorizationUrl.searchParams.set("client_id", clientInformation.client_id);
851844
authorizationUrl.searchParams.set("code_challenge", codeChallenge);
852845
authorizationUrl.searchParams.set(
853846
"code_challenge_method",
854-
codeChallengeMethod,
847+
AUTHORIZATION_CODE_CHALLENGE_METHOD,
855848
);
856849
authorizationUrl.searchParams.set("redirect_uri", String(redirectUrl));
857850

0 commit comments

Comments
 (0)