feat: add PKCE conformance tests (#119)
* 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 * 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. * chore: skip pkce-no-s256-support scenario for now * chore: fix formatting * 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
This commit is contained in:
parent
2d52de5924
commit
4a2ba8ae7b
3 changed files with 303 additions and 1 deletions
|
|
@ -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[];
|
||||
|
|
@ -27,6 +41,8 @@ export interface AuthServerOptions {
|
|||
clientIdMetadataDocumentSupported?: boolean;
|
||||
/** Set to true to NOT advertise registration_endpoint (for pre-registration tests) */
|
||||
disableDynamicRegistration?: boolean;
|
||||
/** PKCE code_challenge_methods_supported. Set to null to omit from metadata. Default: ['S256'] */
|
||||
codeChallengeMethodsSupported?: string[] | null;
|
||||
tokenVerifier?: MockTokenVerifier;
|
||||
onTokenRequest?: (requestData: {
|
||||
scope?: string;
|
||||
|
|
@ -68,6 +84,7 @@ export function createAuthServer(
|
|||
tokenEndpointAuthSigningAlgValuesSupported,
|
||||
clientIdMetadataDocumentSupported,
|
||||
disableDynamicRegistration = false,
|
||||
codeChallengeMethodsSupported = ['S256'],
|
||||
tokenVerifier,
|
||||
onTokenRequest,
|
||||
onAuthorizationRequest,
|
||||
|
|
@ -76,6 +93,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`,
|
||||
|
|
@ -122,7 +141,10 @@ export function createAuthServer(
|
|||
}),
|
||||
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:
|
||||
|
|
@ -165,6 +187,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(' ') : [];
|
||||
|
|
@ -206,6 +263,61 @@ 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)
|
||||
// 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()}`;
|
||||
let scopes: string[] = lastAuthorizationScopes;
|
||||
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
|
||||
|
|
@ -99,4 +100,16 @@ 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',
|
||||
'pkce-verifier-matches-challenge'
|
||||
]
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -76,5 +76,9 @@ export const SpecReferences: { [key: string]: SpecReference } = {
|
|||
MCP_PREREGISTRATION: {
|
||||
id: 'MCP-Preregistration',
|
||||
url: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#preregistration'
|
||||
},
|
||||
MCP_PKCE: {
|
||||
id: 'MCP-PKCE-requirement',
|
||||
url: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#authorization-code-protection'
|
||||
}
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in a new issue