From 0465ffabff3343ea1b21bc59866f10be6fb182a9 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Fri, 23 Jan 2026 09:53:44 +0000 Subject: [PATCH 1/5] feat: add PKCE conformance tests Adds conformance checks for PKCE requirements from the MCP auth spec: - pkce-code-challenge-sent: Verifies client sends code_challenge in auth request - pkce-s256-method-used: Verifies client uses S256 code challenge method - pkce-code-verifier-sent: Verifies client sends code_verifier in token request - pkce-verifier-matches-challenge: Validates BASE64URL(SHA256(verifier)) = challenge Also adds auth/pkce-no-s256-support scenario that tests clients correctly refuse when S256 is not in code_challenge_methods_supported. All PKCE checks are added to the createAuthServer helper, so they automatically apply to all existing auth scenarios. Fixes #77 --- .../clients/typescript/everything-client.ts | 4 +- .../client/auth/helpers/createAuthServer.ts | 95 ++++++++++++++++++- src/scenarios/client/auth/index.test.ts | 4 +- src/scenarios/client/auth/index.ts | 4 +- src/scenarios/client/auth/pkce-no-support.ts | 82 ++++++++++++++++ src/scenarios/client/auth/spec-references.ts | 4 + 6 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 src/scenarios/client/auth/pkce-no-support.ts diff --git a/examples/clients/typescript/everything-client.ts b/examples/clients/typescript/everything-client.ts index 4491fe6..2af3e2f 100644 --- a/examples/clients/typescript/everything-client.ts +++ b/examples/clients/typescript/everything-client.ts @@ -139,7 +139,9 @@ registerScenarios( // Token endpoint auth method scenarios 'auth/token-endpoint-auth-basic', 'auth/token-endpoint-auth-post', - 'auth/token-endpoint-auth-none' + 'auth/token-endpoint-auth-none', + // PKCE scenarios - client should error when S256 not supported + 'auth/pkce-no-s256-support' ], runAuthClient ); diff --git a/src/scenarios/client/auth/helpers/createAuthServer.ts b/src/scenarios/client/auth/helpers/createAuthServer.ts index 1071828..56e3cce 100644 --- a/src/scenarios/client/auth/helpers/createAuthServer.ts +++ b/src/scenarios/client/auth/helpers/createAuthServer.ts @@ -1,9 +1,23 @@ import express, { Request, Response } from 'express'; +import { createHash } from 'crypto'; import type { ConformanceCheck } from '../../../../types'; import { createRequestLogger } from '../../../request-logger'; import { SpecReferences } from '../spec-references'; import { MockTokenVerifier } from './mockTokenVerifier'; +/** + * Compute S256 code challenge from a code verifier. + * BASE64URL(SHA256(code_verifier)) + */ +function computeS256Challenge(codeVerifier: string): string { + const hash = createHash('sha256').update(codeVerifier).digest(); + return hash + .toString('base64') + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/, ''); +} + export interface TokenRequestResult { token: string; scopes: string[]; @@ -25,6 +39,8 @@ export interface AuthServerOptions { tokenEndpointAuthMethodsSupported?: string[]; tokenEndpointAuthSigningAlgValuesSupported?: string[]; clientIdMetadataDocumentSupported?: boolean; + /** PKCE code_challenge_methods_supported. Set to null to omit from metadata. Default: ['S256'] */ + codeChallengeMethodsSupported?: string[] | null; tokenVerifier?: MockTokenVerifier; onTokenRequest?: (requestData: { scope?: string; @@ -65,6 +81,7 @@ export function createAuthServer( tokenEndpointAuthMethodsSupported = ['none'], tokenEndpointAuthSigningAlgValuesSupported, clientIdMetadataDocumentSupported, + codeChallengeMethodsSupported = ['S256'], tokenVerifier, onTokenRequest, onAuthorizationRequest, @@ -73,6 +90,8 @@ export function createAuthServer( // Track scopes from the most recent authorization request let lastAuthorizationScopes: string[] = []; + // Track PKCE code_challenge for verification in token request + let storedCodeChallenge: string | undefined; const authRoutes = { authorization_endpoint: `${routePrefix}/authorize`, @@ -117,7 +136,10 @@ export function createAuthServer( registration_endpoint: `${getAuthBaseUrl()}${authRoutes.registration_endpoint}`, response_types_supported: ['code'], grant_types_supported: grantTypesSupported, - code_challenge_methods_supported: ['S256'], + // PKCE support - null means omit from metadata (for negative testing) + ...(codeChallengeMethodsSupported !== null && { + code_challenge_methods_supported: codeChallengeMethodsSupported + }), token_endpoint_auth_methods_supported: tokenEndpointAuthMethodsSupported, ...(tokenEndpointAuthSigningAlgValuesSupported && { token_endpoint_auth_signing_alg_values_supported: @@ -160,6 +182,41 @@ export function createAuthServer( } }); + // PKCE: Store code_challenge for later verification + const codeChallenge = req.query.code_challenge as string | undefined; + const codeChallengeMethod = req.query.code_challenge_method as + | string + | undefined; + storedCodeChallenge = codeChallenge; + + // PKCE: Check code_challenge is present + checks.push({ + id: 'pkce-code-challenge-sent', + name: 'PKCE Code Challenge', + description: codeChallenge + ? 'Client sent code_challenge in authorization request' + : 'Client MUST send code_challenge in authorization request', + status: codeChallenge ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: [SpecReferences.MCP_PKCE] + }); + + // PKCE: Check S256 method is used + checks.push({ + id: 'pkce-s256-method-used', + name: 'PKCE S256 Method', + description: + codeChallengeMethod === 'S256' + ? 'Client used S256 code challenge method' + : 'Client MUST use S256 code challenge method when technically capable', + status: codeChallengeMethod === 'S256' ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: [SpecReferences.MCP_PKCE], + details: { + method: codeChallengeMethod || 'not specified' + } + }); + // Track scopes from authorization request for token issuance const scopeParam = req.query.scope as string | undefined; lastAuthorizationScopes = scopeParam ? scopeParam.split(' ') : []; @@ -201,6 +258,42 @@ export function createAuthServer( } }); + // PKCE: Check code_verifier is present (only for authorization_code grant) + const codeVerifier = req.body.code_verifier as string | undefined; + if (grantType === 'authorization_code') { + checks.push({ + id: 'pkce-code-verifier-sent', + name: 'PKCE Code Verifier', + description: codeVerifier + ? 'Client sent code_verifier in token request' + : 'Client MUST send code_verifier in token request', + status: codeVerifier ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: [SpecReferences.MCP_PKCE] + }); + + // PKCE: Validate code_verifier matches code_challenge (S256) + if (codeVerifier && storedCodeChallenge) { + const computedChallenge = computeS256Challenge(codeVerifier); + const matches = computedChallenge === storedCodeChallenge; + checks.push({ + id: 'pkce-verifier-matches-challenge', + name: 'PKCE Verifier Validation', + description: matches + ? 'code_verifier correctly matches code_challenge (S256)' + : 'code_verifier does not match code_challenge', + status: matches ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: [SpecReferences.MCP_PKCE], + details: { + matches, + storedChallenge: storedCodeChallenge, + computedChallenge + } + }); + } + } + let token = `test-token-${Date.now()}`; let scopes: string[] = lastAuthorizationScopes; diff --git a/src/scenarios/client/auth/index.test.ts b/src/scenarios/client/auth/index.test.ts index 6dcc020..0b8a65d 100644 --- a/src/scenarios/client/auth/index.test.ts +++ b/src/scenarios/client/auth/index.test.ts @@ -22,7 +22,9 @@ const skipScenarios = new Set([ const allowClientErrorScenarios = new Set([ // Client is expected to give up (error) after limited retries, but check should pass - 'auth/scope-retry-limit' + 'auth/scope-retry-limit', + // Client should refuse/error when S256 is not in code_challenge_methods_supported + 'auth/pkce-no-s256-support' ]); describe('Client Auth Scenarios', () => { diff --git a/src/scenarios/client/auth/index.ts b/src/scenarios/client/auth/index.ts index 805781a..cfadb39 100644 --- a/src/scenarios/client/auth/index.ts +++ b/src/scenarios/client/auth/index.ts @@ -21,6 +21,7 @@ import { ClientCredentialsJwtScenario, ClientCredentialsBasicScenario } from './client-credentials'; +import { PkceNoS256SupportScenario } from './pkce-no-support'; // Auth scenarios (required for tier 1) export const authScenariosList: Scenario[] = [ @@ -35,7 +36,8 @@ export const authScenariosList: Scenario[] = [ new ScopeRetryLimitScenario(), new ClientSecretBasicAuthScenario(), new ClientSecretPostAuthScenario(), - new PublicClientAuthScenario() + new PublicClientAuthScenario(), + new PkceNoS256SupportScenario() ]; // Extension scenarios (optional for tier 1 - protocol extensions) diff --git a/src/scenarios/client/auth/pkce-no-support.ts b/src/scenarios/client/auth/pkce-no-support.ts new file mode 100644 index 0000000..db23179 --- /dev/null +++ b/src/scenarios/client/auth/pkce-no-support.ts @@ -0,0 +1,82 @@ +/** + * PKCE No S256 Support Scenario + * + * Tests that clients correctly refuse to proceed when the authorization server + * advertises PKCE support but does NOT include S256 in the supported methods. + * + * Per MCP spec: "MCP clients MUST use the S256 code challenge method when + * technically capable" - if the server doesn't support S256, client must refuse. + * + * Note: code_challenge_methods_supported being absent is acceptable (server + * just doesn't advertise). But if present and empty or missing S256, client + * must refuse. + */ + +import type { Scenario, ConformanceCheck } from '../../../types'; +import { ScenarioUrls } from '../../../types'; +import { createAuthServer } from './helpers/createAuthServer'; +import { createServer } from './helpers/createServer'; +import { ServerLifecycle } from './helpers/serverLifecycle'; +import { SpecReferences } from './spec-references'; + +export class PkceNoS256SupportScenario implements Scenario { + name = 'auth/pkce-no-s256-support'; + description = + 'Tests that client refuses to proceed when authorization server does not support S256 (code_challenge_methods_supported is empty or missing S256)'; + private authServer = new ServerLifecycle(); + private server = new ServerLifecycle(); + private checks: ConformanceCheck[] = []; + private authorizationRequested = false; + + async start(): Promise { + this.checks = []; + this.authorizationRequested = false; + + // Create auth server with PKCE methods that don't include S256 + // (e.g., empty array or only 'plain' which is insecure) + const authApp = createAuthServer(this.checks, this.authServer.getUrl, { + codeChallengeMethodsSupported: [], // Empty = no methods supported + // Override the authorization endpoint to detect if client proceeds + onAuthorizationRequest: () => { + this.authorizationRequested = true; + } + }); + + await this.authServer.start(authApp); + + const app = createServer( + this.checks, + this.server.getUrl, + this.authServer.getUrl + ); + + await this.server.start(app); + + return { serverUrl: `${this.server.getUrl()}/mcp` }; + } + + async stop() { + await this.authServer.stop(); + await this.server.stop(); + } + + getChecks(): ConformanceCheck[] { + // Add the PKCE S256 refused check based on whether authorization was requested + this.checks.push({ + id: 'pkce-s256-required', + name: 'PKCE S256 Required', + description: this.authorizationRequested + ? 'Client proceeded with authorization despite S256 not being supported - clients MUST refuse when S256 is not available' + : 'Client correctly refused to proceed when S256 was not in code_challenge_methods_supported', + status: this.authorizationRequested ? 'FAILURE' : 'SUCCESS', + timestamp: new Date().toISOString(), + specReferences: [SpecReferences.MCP_PKCE], + details: { + authorizationRequested: this.authorizationRequested, + codeChallengeMethodsSupported: [] + } + }); + + return this.checks; + } +} diff --git a/src/scenarios/client/auth/spec-references.ts b/src/scenarios/client/auth/spec-references.ts index 52a08ca..0c35419 100644 --- a/src/scenarios/client/auth/spec-references.ts +++ b/src/scenarios/client/auth/spec-references.ts @@ -72,5 +72,9 @@ export const SpecReferences: { [key: string]: SpecReference } = { SEP_1046_CLIENT_CREDENTIALS: { id: 'SEP-1046-Client-Credentials', url: 'https://github.com/modelcontextprotocol/ext-auth/blob/main/specification/draft/oauth-client-credentials.mdx' + }, + MCP_PKCE: { + id: 'MCP-PKCE-requirement', + url: 'https://modelcontextprotocol.io/specification/2025-06-18/basic/authorization#authorization-code-protection' } }; From 702a927dbde0c6e389c685feb6779a7b4226e99a Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Fri, 23 Jan 2026 09:54:36 +0000 Subject: [PATCH 2/5] feat: add bad client example that skips PKCE Adds auth-test-no-pkce.ts which implements a custom OAuth flow that deliberately skips PKCE (no code_challenge in auth request, no code_verifier in token request). Also adds negative test to verify the conformance suite correctly detects this non-compliant behavior. --- .../clients/typescript/auth-test-no-pkce.ts | 174 ++++++++++++++++++ src/scenarios/client/auth/index.test.ts | 12 ++ 2 files changed, 186 insertions(+) create mode 100644 examples/clients/typescript/auth-test-no-pkce.ts diff --git a/examples/clients/typescript/auth-test-no-pkce.ts b/examples/clients/typescript/auth-test-no-pkce.ts new file mode 100644 index 0000000..0167270 --- /dev/null +++ b/examples/clients/typescript/auth-test-no-pkce.ts @@ -0,0 +1,174 @@ +#!/usr/bin/env node + +/** + * Broken client that doesn't use PKCE. + * + * BUG: Skips PKCE entirely - doesn't send code_challenge in authorization + * request and doesn't send code_verifier in token request. + * + * Per MCP spec: "MCP clients MUST implement PKCE according to OAuth 2.1" + */ + +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { extractWWWAuthenticateParams } from '@modelcontextprotocol/sdk/client/auth.js'; +import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'; +import type { Middleware } from '@modelcontextprotocol/sdk/client/middleware.js'; +import { runAsCli } from './helpers/cliRunner'; +import { logger } from './helpers/logger'; + +interface OAuthTokens { + access_token: string; + token_type: string; + expires_in?: number; + refresh_token?: string; + scope?: string; +} + +/** + * Custom OAuth flow that deliberately skips PKCE. + * This is intentionally broken behavior for conformance testing. + */ +async function oauthFlowWithoutPkce( + serverUrl: string | URL, + resourceMetadataUrl: string | URL, + fetchFn: FetchLike +): Promise { + const baseUrl = + typeof serverUrl === 'string' ? new URL(serverUrl).origin : serverUrl.origin; + + // 1. Fetch Protected Resource Metadata + const prmResponse = await fetchFn(resourceMetadataUrl); + if (!prmResponse.ok) { + throw new Error(`Failed to fetch PRM: ${prmResponse.status}`); + } + const prm = await prmResponse.json(); + const authServerUrl = prm.authorization_servers?.[0]; + if (!authServerUrl) { + throw new Error('No authorization server in PRM'); + } + + // 2. Fetch Authorization Server Metadata + const asMetadataUrl = new URL( + '/.well-known/oauth-authorization-server', + authServerUrl + ); + const asResponse = await fetchFn(asMetadataUrl.toString()); + if (!asResponse.ok) { + throw new Error(`Failed to fetch AS metadata: ${asResponse.status}`); + } + const asMetadata = await asResponse.json(); + + // 3. Register client (DCR) + const dcrResponse = await fetchFn(asMetadata.registration_endpoint, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + client_name: 'test-auth-client-no-pkce', + redirect_uris: ['http://localhost:3000/callback'] + }) + }); + if (!dcrResponse.ok) { + throw new Error(`DCR failed: ${dcrResponse.status}`); + } + const clientInfo = await dcrResponse.json(); + + // 4. Build authorization URL WITHOUT PKCE (BUG!) + const authUrl = new URL(asMetadata.authorization_endpoint); + authUrl.searchParams.set('response_type', 'code'); + authUrl.searchParams.set('client_id', clientInfo.client_id); + authUrl.searchParams.set('redirect_uri', 'http://localhost:3000/callback'); + authUrl.searchParams.set('state', 'test-state'); + // BUG: NOT setting code_challenge or code_challenge_method + + // 5. Fetch authorization endpoint (simulates redirect) + const authResponse = await fetchFn(authUrl.toString(), { redirect: 'manual' }); + const location = authResponse.headers.get('location'); + if (!location) { + throw new Error('No redirect from authorization endpoint'); + } + const redirectUrl = new URL(location); + const authCode = redirectUrl.searchParams.get('code'); + if (!authCode) { + throw new Error('No auth code in redirect'); + } + + // 6. Exchange code for token WITHOUT code_verifier (BUG!) + const tokenResponse = await fetchFn(asMetadata.token_endpoint, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ + grant_type: 'authorization_code', + code: authCode, + redirect_uri: 'http://localhost:3000/callback', + client_id: clientInfo.client_id + // BUG: NOT sending code_verifier + }).toString() + }); + + if (!tokenResponse.ok) { + const error = await tokenResponse.text(); + throw new Error(`Token request failed: ${tokenResponse.status} - ${error}`); + } + + return tokenResponse.json(); +} + +/** + * Creates a fetch wrapper that uses OAuth without PKCE. + */ +function withOAuthNoPkce(baseUrl: string | URL): Middleware { + let tokens: OAuthTokens | undefined; + + return (next: FetchLike) => { + return async ( + input: string | URL, + init?: RequestInit + ): Promise => { + const makeRequest = async (): Promise => { + const headers = new Headers(init?.headers); + if (tokens) { + headers.set('Authorization', `Bearer ${tokens.access_token}`); + } + return next(input, { ...init, headers }); + }; + + let response = await makeRequest(); + + if (response.status === 401) { + const { resourceMetadataUrl } = extractWWWAuthenticateParams(response); + if (!resourceMetadataUrl) { + throw new Error('No resource_metadata in WWW-Authenticate'); + } + tokens = await oauthFlowWithoutPkce(baseUrl, resourceMetadataUrl, next); + response = await makeRequest(); + } + + return response; + }; + }; +} + +export async function runClient(serverUrl: string): Promise { + const client = new Client( + { name: 'test-auth-client-no-pkce', version: '1.0.0' }, + { capabilities: {} } + ); + + const oauthFetch = withOAuthNoPkce(new URL(serverUrl))(fetch); + + const transport = new StreamableHTTPClientTransport(new URL(serverUrl), { + fetch: oauthFetch + }); + + await client.connect(transport); + logger.debug('Successfully connected to MCP server'); + + await client.listTools(); + logger.debug('Successfully listed tools'); + + await transport.close(); + logger.debug('Connection closed successfully'); +} + +runAsCli(runClient, import.meta.url, 'auth-test-no-pkce '); diff --git a/src/scenarios/client/auth/index.test.ts b/src/scenarios/client/auth/index.test.ts index 0b8a65d..c1e298f 100644 --- a/src/scenarios/client/auth/index.test.ts +++ b/src/scenarios/client/auth/index.test.ts @@ -9,6 +9,7 @@ import { runClient as ignoreScopeClient } from '../../../../examples/clients/typ import { runClient as partialScopesClient } from '../../../../examples/clients/typescript/auth-test-partial-scopes'; import { runClient as ignore403Client } from '../../../../examples/clients/typescript/auth-test-ignore-403'; import { runClient as noRetryLimitClient } from '../../../../examples/clients/typescript/auth-test-no-retry-limit'; +import { runClient as noPkceClient } from '../../../../examples/clients/typescript/auth-test-no-pkce'; import { getHandler } from '../../../../examples/clients/typescript/everything-client'; import { setLogLevel } from '../../../../examples/clients/typescript/helpers/logger'; @@ -101,4 +102,15 @@ describe('Negative tests', () => { allowClientError: true }); }); + + test('client does not use PKCE', async () => { + const runner = new InlineClientRunner(noPkceClient); + await runClientAgainstScenario(runner, 'auth/metadata-default', { + expectedFailureSlugs: [ + 'pkce-code-challenge-sent', + 'pkce-s256-method-used', + 'pkce-code-verifier-sent' + ] + }); + }); }); From 43f79b416d835e90ad6e5ff9422e4f2ca97c2c2d Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Fri, 23 Jan 2026 10:05:10 +0000 Subject: [PATCH 3/5] chore: skip pkce-no-s256-support scenario for now --- examples/clients/typescript/auth-test-no-pkce.ts | 4 +--- src/scenarios/client/auth/index.test.ts | 7 +++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/examples/clients/typescript/auth-test-no-pkce.ts b/examples/clients/typescript/auth-test-no-pkce.ts index 0167270..fe3d7a3 100644 --- a/examples/clients/typescript/auth-test-no-pkce.ts +++ b/examples/clients/typescript/auth-test-no-pkce.ts @@ -30,12 +30,10 @@ interface OAuthTokens { * This is intentionally broken behavior for conformance testing. */ async function oauthFlowWithoutPkce( - serverUrl: string | URL, + _serverUrl: string | URL, resourceMetadataUrl: string | URL, fetchFn: FetchLike ): Promise { - const baseUrl = - typeof serverUrl === 'string' ? new URL(serverUrl).origin : serverUrl.origin; // 1. Fetch Protected Resource Metadata const prmResponse = await fetchFn(resourceMetadataUrl); diff --git a/src/scenarios/client/auth/index.test.ts b/src/scenarios/client/auth/index.test.ts index c1e298f..08fbd80 100644 --- a/src/scenarios/client/auth/index.test.ts +++ b/src/scenarios/client/auth/index.test.ts @@ -18,14 +18,13 @@ beforeAll(() => { }); const skipScenarios = new Set([ - // Add scenarios that should be skipped here + // Skip for now - tests that client refuses when S256 not supported + 'auth/pkce-no-s256-support' ]); const allowClientErrorScenarios = new Set([ // Client is expected to give up (error) after limited retries, but check should pass - 'auth/scope-retry-limit', - // Client should refuse/error when S256 is not in code_challenge_methods_supported - 'auth/pkce-no-s256-support' + 'auth/scope-retry-limit' ]); describe('Client Auth Scenarios', () => { From 0315fd6a467c79827191b819d300f37586b681d5 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Fri, 23 Jan 2026 10:09:21 +0000 Subject: [PATCH 4/5] chore: fix formatting --- examples/clients/typescript/auth-test-no-pkce.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/clients/typescript/auth-test-no-pkce.ts b/examples/clients/typescript/auth-test-no-pkce.ts index fe3d7a3..308351d 100644 --- a/examples/clients/typescript/auth-test-no-pkce.ts +++ b/examples/clients/typescript/auth-test-no-pkce.ts @@ -34,7 +34,6 @@ async function oauthFlowWithoutPkce( resourceMetadataUrl: string | URL, fetchFn: FetchLike ): Promise { - // 1. Fetch Protected Resource Metadata const prmResponse = await fetchFn(resourceMetadataUrl); if (!prmResponse.ok) { @@ -80,7 +79,9 @@ async function oauthFlowWithoutPkce( // BUG: NOT setting code_challenge or code_challenge_method // 5. Fetch authorization endpoint (simulates redirect) - const authResponse = await fetchFn(authUrl.toString(), { redirect: 'manual' }); + const authResponse = await fetchFn(authUrl.toString(), { + redirect: 'manual' + }); const location = authResponse.headers.get('location'); if (!location) { throw new Error('No redirect from authorization endpoint'); From fbd656d862375c43d56ad1f28affebe5d3ce0feb Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Fri, 23 Jan 2026 11:33:21 +0000 Subject: [PATCH 5/5] refactor: remove pkce-no-s256-support scenario and always validate PKCE - Remove pkce-no-s256-support scenario (was skipped anyway) - Make pkce-verifier-matches-challenge fail if either code_challenge or code_verifier is missing, rather than only running when both present - Update negative test to expect the new failure --- .../clients/typescript/everything-client.ts | 4 +- .../client/auth/helpers/createAuthServer.ts | 55 +++++++++---- src/scenarios/client/auth/index.test.ts | 6 +- src/scenarios/client/auth/index.ts | 4 +- src/scenarios/client/auth/pkce-no-support.ts | 82 ------------------- 5 files changed, 42 insertions(+), 109 deletions(-) delete mode 100644 src/scenarios/client/auth/pkce-no-support.ts diff --git a/examples/clients/typescript/everything-client.ts b/examples/clients/typescript/everything-client.ts index 2af3e2f..4491fe6 100644 --- a/examples/clients/typescript/everything-client.ts +++ b/examples/clients/typescript/everything-client.ts @@ -139,9 +139,7 @@ registerScenarios( // Token endpoint auth method scenarios 'auth/token-endpoint-auth-basic', 'auth/token-endpoint-auth-post', - 'auth/token-endpoint-auth-none', - // PKCE scenarios - client should error when S256 not supported - 'auth/pkce-no-s256-support' + 'auth/token-endpoint-auth-none' ], runAuthClient ); diff --git a/src/scenarios/client/auth/helpers/createAuthServer.ts b/src/scenarios/client/auth/helpers/createAuthServer.ts index 56e3cce..7db37dd 100644 --- a/src/scenarios/client/auth/helpers/createAuthServer.ts +++ b/src/scenarios/client/auth/helpers/createAuthServer.ts @@ -273,25 +273,44 @@ export function createAuthServer( }); // PKCE: Validate code_verifier matches code_challenge (S256) - if (codeVerifier && storedCodeChallenge) { - const computedChallenge = computeS256Challenge(codeVerifier); - const matches = computedChallenge === storedCodeChallenge; - checks.push({ - id: 'pkce-verifier-matches-challenge', - name: 'PKCE Verifier Validation', - description: matches - ? 'code_verifier correctly matches code_challenge (S256)' - : 'code_verifier does not match code_challenge', - status: matches ? 'SUCCESS' : 'FAILURE', - timestamp, - specReferences: [SpecReferences.MCP_PKCE], - details: { - matches, - storedChallenge: storedCodeChallenge, - computedChallenge - } - }); + // Fail if either is missing + const computedChallenge = + codeVerifier && storedCodeChallenge + ? computeS256Challenge(codeVerifier) + : undefined; + const matches = + computedChallenge !== undefined && + computedChallenge === storedCodeChallenge; + + let description: string; + if (!storedCodeChallenge && !codeVerifier) { + description = + 'Neither code_challenge nor code_verifier were sent - PKCE is required'; + } else if (!storedCodeChallenge) { + description = + 'code_challenge was not sent in authorization request - PKCE is required'; + } else if (!codeVerifier) { + description = + 'code_verifier was not sent in token request - PKCE is required'; + } else if (matches) { + description = 'code_verifier correctly matches code_challenge (S256)'; + } else { + description = 'code_verifier does not match code_challenge'; } + + checks.push({ + id: 'pkce-verifier-matches-challenge', + name: 'PKCE Verifier Validation', + description, + status: matches ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: [SpecReferences.MCP_PKCE], + details: { + matches, + storedChallenge: storedCodeChallenge || 'not sent', + computedChallenge: computedChallenge || 'not computed' + } + }); } let token = `test-token-${Date.now()}`; diff --git a/src/scenarios/client/auth/index.test.ts b/src/scenarios/client/auth/index.test.ts index 08fbd80..bda5d40 100644 --- a/src/scenarios/client/auth/index.test.ts +++ b/src/scenarios/client/auth/index.test.ts @@ -18,8 +18,7 @@ beforeAll(() => { }); const skipScenarios = new Set([ - // Skip for now - tests that client refuses when S256 not supported - 'auth/pkce-no-s256-support' + // Add scenarios that should be skipped here ]); const allowClientErrorScenarios = new Set([ @@ -108,7 +107,8 @@ describe('Negative tests', () => { expectedFailureSlugs: [ 'pkce-code-challenge-sent', 'pkce-s256-method-used', - 'pkce-code-verifier-sent' + 'pkce-code-verifier-sent', + 'pkce-verifier-matches-challenge' ] }); }); diff --git a/src/scenarios/client/auth/index.ts b/src/scenarios/client/auth/index.ts index cfadb39..805781a 100644 --- a/src/scenarios/client/auth/index.ts +++ b/src/scenarios/client/auth/index.ts @@ -21,7 +21,6 @@ import { ClientCredentialsJwtScenario, ClientCredentialsBasicScenario } from './client-credentials'; -import { PkceNoS256SupportScenario } from './pkce-no-support'; // Auth scenarios (required for tier 1) export const authScenariosList: Scenario[] = [ @@ -36,8 +35,7 @@ export const authScenariosList: Scenario[] = [ new ScopeRetryLimitScenario(), new ClientSecretBasicAuthScenario(), new ClientSecretPostAuthScenario(), - new PublicClientAuthScenario(), - new PkceNoS256SupportScenario() + new PublicClientAuthScenario() ]; // Extension scenarios (optional for tier 1 - protocol extensions) diff --git a/src/scenarios/client/auth/pkce-no-support.ts b/src/scenarios/client/auth/pkce-no-support.ts deleted file mode 100644 index db23179..0000000 --- a/src/scenarios/client/auth/pkce-no-support.ts +++ /dev/null @@ -1,82 +0,0 @@ -/** - * PKCE No S256 Support Scenario - * - * Tests that clients correctly refuse to proceed when the authorization server - * advertises PKCE support but does NOT include S256 in the supported methods. - * - * Per MCP spec: "MCP clients MUST use the S256 code challenge method when - * technically capable" - if the server doesn't support S256, client must refuse. - * - * Note: code_challenge_methods_supported being absent is acceptable (server - * just doesn't advertise). But if present and empty or missing S256, client - * must refuse. - */ - -import type { Scenario, ConformanceCheck } from '../../../types'; -import { ScenarioUrls } from '../../../types'; -import { createAuthServer } from './helpers/createAuthServer'; -import { createServer } from './helpers/createServer'; -import { ServerLifecycle } from './helpers/serverLifecycle'; -import { SpecReferences } from './spec-references'; - -export class PkceNoS256SupportScenario implements Scenario { - name = 'auth/pkce-no-s256-support'; - description = - 'Tests that client refuses to proceed when authorization server does not support S256 (code_challenge_methods_supported is empty or missing S256)'; - private authServer = new ServerLifecycle(); - private server = new ServerLifecycle(); - private checks: ConformanceCheck[] = []; - private authorizationRequested = false; - - async start(): Promise { - this.checks = []; - this.authorizationRequested = false; - - // Create auth server with PKCE methods that don't include S256 - // (e.g., empty array or only 'plain' which is insecure) - const authApp = createAuthServer(this.checks, this.authServer.getUrl, { - codeChallengeMethodsSupported: [], // Empty = no methods supported - // Override the authorization endpoint to detect if client proceeds - onAuthorizationRequest: () => { - this.authorizationRequested = true; - } - }); - - await this.authServer.start(authApp); - - const app = createServer( - this.checks, - this.server.getUrl, - this.authServer.getUrl - ); - - await this.server.start(app); - - return { serverUrl: `${this.server.getUrl()}/mcp` }; - } - - async stop() { - await this.authServer.stop(); - await this.server.stop(); - } - - getChecks(): ConformanceCheck[] { - // Add the PKCE S256 refused check based on whether authorization was requested - this.checks.push({ - id: 'pkce-s256-required', - name: 'PKCE S256 Required', - description: this.authorizationRequested - ? 'Client proceeded with authorization despite S256 not being supported - clients MUST refuse when S256 is not available' - : 'Client correctly refused to proceed when S256 was not in code_challenge_methods_supported', - status: this.authorizationRequested ? 'FAILURE' : 'SUCCESS', - timestamp: new Date().toISOString(), - specReferences: [SpecReferences.MCP_PKCE], - details: { - authorizationRequested: this.authorizationRequested, - codeChallengeMethodsSupported: [] - } - }); - - return this.checks; - } -}