diff --git a/examples/clients/typescript/auth-test-no-retry-limit.ts b/examples/clients/typescript/auth-test-no-retry-limit.ts new file mode 100644 index 0000000..78a2c7b --- /dev/null +++ b/examples/clients/typescript/auth-test-no-retry-limit.ts @@ -0,0 +1,113 @@ +#!/usr/bin/env node + +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { + auth, + extractWWWAuthenticateParams, + UnauthorizedError +} 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 { ConformanceOAuthProvider } from './helpers/ConformanceOAuthProvider'; +import { runAsCli } from './helpers/cliRunner'; +import { logger } from './helpers/logger'; + +/** + * Broken client that retries auth infinitely without any retry limit. + * BUG: Does not implement retry limits, causing infinite auth loops. + */ + +const withOAuthRetryNoLimit = ( + clientName: string, + baseUrl?: string | URL +): Middleware => { + const provider = new ConformanceOAuthProvider( + 'http://localhost:3000/callback', + { + client_name: clientName, + redirect_uris: ['http://localhost:3000/callback'] + } + ); + + return (next: FetchLike) => { + return async ( + input: string | URL, + init?: RequestInit + ): Promise => { + const makeRequest = async (): Promise => { + const headers = new Headers(init?.headers); + const tokens = await provider.tokens(); + if (tokens) { + headers.set('Authorization', `Bearer ${tokens.access_token}`); + } + return await next(input, { ...init, headers }); + }; + + let response = await makeRequest(); + + // BUG: No retry limit - keeps retrying on every 401/403 + while (response.status === 401 || response.status === 403) { + const serverUrl = + baseUrl || + (typeof input === 'string' ? new URL(input).origin : input.origin); + + const { resourceMetadataUrl, scope } = + extractWWWAuthenticateParams(response); + let result = await auth(provider, { + serverUrl, + resourceMetadataUrl, + scope, + fetchFn: next + }); + + if (result === 'REDIRECT') { + const authorizationCode = await provider.getAuthCode(); + result = await auth(provider, { + serverUrl, + resourceMetadataUrl, + scope, + authorizationCode, + fetchFn: next + }); + if (result !== 'AUTHORIZED') { + throw new UnauthorizedError( + `Authentication failed with result: ${result}` + ); + } + } + + response = await makeRequest(); + } + + return response; + }; + }; +}; + +export async function runClient(serverUrl: string): Promise { + const client = new Client( + { name: 'test-auth-client-no-retry-limit', version: '1.0.0' }, + { capabilities: {} } + ); + + const oauthFetch = withOAuthRetryNoLimit( + 'test-auth-client-no-retry-limit', + 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-retry-limit '); diff --git a/lefthook-local.example.yml b/lefthook-local.example.yml index 97a3fdf..3401c11 100644 --- a/lefthook-local.example.yml +++ b/lefthook-local.example.yml @@ -2,9 +2,18 @@ # To enable this: # cp lefthook-local.example.yml lefthook-local.yml -# Uncomment to add pre-commit hook for automatic linting on staged files: pre-commit: commands: lint: run: npm run lint:fix {staged_files} stage_fixed: true + +post-checkout: + jobs: + - name: 'Install Dependencies' + run: npm install + +post-merge: + jobs: + - name: 'Install Dependencies' + run: npm install diff --git a/lefthook.yml b/lefthook.yml index 445ba02..416265e 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -27,13 +27,3 @@ pre-push: - name: 'Test' run: npm run test - -post-checkout: - jobs: - - name: 'Install Dependencies' - run: npm install - -post-merge: - jobs: - - name: 'Install Dependencies' - run: npm install diff --git a/package-lock.json b/package-lock.json index a419577..5be12a5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "0.1.7", "license": "MIT", "dependencies": { - "@modelcontextprotocol/sdk": "^1.22.0", + "@modelcontextprotocol/sdk": "^1.23.0-beta.0", "commander": "^14.0.2", "express": "^5.1.0", "zod": "^3.25.76" @@ -839,9 +839,9 @@ } }, "node_modules/@modelcontextprotocol/sdk": { - "version": "1.22.0", - "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.22.0.tgz", - "integrity": "sha512-VUpl106XVTCpDmTBil2ehgJZjhyLY2QZikzF8NvTXtLRF1CvO5iEE2UNZdVIUer35vFOwMKYeUGbjJtvPWan3g==", + "version": "1.23.0-beta.0", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.23.0-beta.0.tgz", + "integrity": "sha512-NvoyrhhNcNiyf0nI8J1O+wheNiyOzK3kMTkMuwGb/TGHpSHXCcubcg0IxC/p9Aym+K4QZFxq9Wn67clOAegFKQ==", "license": "MIT", "dependencies": { "ajv": "^8.17.1", @@ -855,18 +855,22 @@ "express-rate-limit": "^7.5.0", "pkce-challenge": "^5.0.0", "raw-body": "^3.0.0", - "zod": "^3.23.8", - "zod-to-json-schema": "^3.24.1" + "zod": "^3.25 || ^4.0", + "zod-to-json-schema": "^3.25.0" }, "engines": { "node": ">=18" }, "peerDependencies": { - "@cfworker/json-schema": "^4.1.1" + "@cfworker/json-schema": "^4.1.1", + "zod": "^3.25 || ^4.0" }, "peerDependenciesMeta": { "@cfworker/json-schema": { "optional": true + }, + "zod": { + "optional": false } } }, @@ -5299,12 +5303,12 @@ } }, "node_modules/zod-to-json-schema": { - "version": "3.24.6", - "resolved": "https://registry.npmjs.org/zod-to-json-schema/-/zod-to-json-schema-3.24.6.tgz", - "integrity": "sha512-h/z3PKvcTcTetyjl1fkj79MHNEjm+HpD6NXheWjzOekY7kV+lwDYnHw+ivHkijnCSMz1yJaWBD9vu/Fcmk+vEg==", + "version": "3.25.0", + "resolved": "https://registry.npmjs.org/zod-to-json-schema/-/zod-to-json-schema-3.25.0.tgz", + "integrity": "sha512-HvWtU2UG41LALjajJrML6uQejQhNJx+JBO9IflpSja4R03iNWfKXrj6W2h7ljuLyc1nKS+9yDyL/9tD1U/yBnQ==", "license": "ISC", "peerDependencies": { - "zod": "^3.24.1" + "zod": "^3.25 || ^4" } } } diff --git a/package.json b/package.json index 61453c5..a0eb191 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "vitest": "^4.0.5" }, "dependencies": { - "@modelcontextprotocol/sdk": "^1.22.0", + "@modelcontextprotocol/sdk": "^1.23.0-beta.0", "commander": "^14.0.2", "express": "^5.1.0", "zod": "^3.25.76" diff --git a/src/index.ts b/src/index.ts index 21145d5..a65e79f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,6 +14,7 @@ import { listScenarios, listClientScenarios, listActiveClientScenarios, + listPendingClientScenarios, listAuthScenarios, listMetadataScenarios } from './scenarios'; @@ -53,7 +54,9 @@ program const suites: Record string[]> = { auth: listAuthScenarios, - metadata: listMetadataScenarios + metadata: listMetadataScenarios, + 'sep-835': () => + listAuthScenarios().filter((name) => name.startsWith('auth/scope-')) }; const suiteName = options.suite.toLowerCase(); @@ -123,8 +126,9 @@ program totalWarnings += warnings; const status = failed === 0 ? '✓' : '✗'; + const warningStr = warnings > 0 ? `, ${warnings} warnings` : ''; console.log( - `${status} ${result.scenario}: ${passed} passed, ${failed} failed` + `${status} ${result.scenario}: ${passed} passed, ${failed} failed${warningStr}` ); if (verbose && failed > 0) { @@ -149,7 +153,7 @@ program console.error('Either --scenario or --suite is required'); console.error('\nAvailable client scenarios:'); listScenarios().forEach((s) => console.error(` - ${s}`)); - console.error('\nAvailable suites: auth, metadata'); + console.error('\nAvailable suites: auth, metadata, sep-835'); process.exit(1); } @@ -193,7 +197,12 @@ program .requiredOption('--url ', 'URL of the server to test') .option( '--scenario ', - 'Scenario to test (defaults to all scenarios if not specified)' + 'Scenario to test (defaults to active suite if not specified)' + ) + .option( + '--suite ', + 'Suite to run: "active" (default, excludes pending), "all", or "pending"', + 'active' ) .action(async (options) => { try { @@ -213,10 +222,24 @@ program ); process.exit(failed > 0 ? 1 : 0); } else { - // Run all active scenarios - const scenarios = listActiveClientScenarios(); + // Run scenarios based on suite + const suite = options.suite?.toLowerCase() || 'active'; + let scenarios: string[]; + + if (suite === 'all') { + scenarios = listClientScenarios(); + } else if (suite === 'active') { + scenarios = listActiveClientScenarios(); + } else if (suite === 'pending') { + scenarios = listPendingClientScenarios(); + } else { + console.error(`Unknown suite: ${suite}`); + console.error('Available suites: active, all, pending'); + process.exit(1); + } + console.log( - `Running ${scenarios.length} scenarios against ${validated.url}\n` + `Running ${suite} suite (${scenarios.length} scenarios) against ${validated.url}\n` ); const allResults: { scenario: string; checks: ConformanceCheck[] }[] = diff --git a/src/scenarios/client/auth/index.test.ts b/src/scenarios/client/auth/index.test.ts index b04ebfc..285280f 100644 --- a/src/scenarios/client/auth/index.test.ts +++ b/src/scenarios/client/auth/index.test.ts @@ -9,24 +9,18 @@ import { runClient as noCimdClient } from '../../../../examples/clients/typescri import { runClient as ignoreScopeClient } from '../../../../examples/clients/typescript/auth-test-ignore-scope'; 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 { setLogLevel } from '../../../../examples/clients/typescript/helpers/logger'; beforeAll(() => { setLogLevel('error'); }); -const skipScenarios = new Set([ - // Waiting on typescript-sdk support in bearerAuth middleware to include - // scope in WWW-Authenticate header - // https://github.com/modelcontextprotocol/typescript-sdk/pull/1133 - 'auth/scope-from-www-authenticate', - // Waiting on typescript-sdk support for using scopes_supported from PRM - // to request scopes. - // https://github.com/modelcontextprotocol/typescript-sdk/pull/1133 - 'auth/scope-from-scopes-supported', - // Waiting on typescript-sdk support for CIMD - // https://github.com/modelcontextprotocol/typescript-sdk/pull/1127 - 'auth/basic-cimd' +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' ]); describe('Client Auth Scenarios', () => { @@ -38,7 +32,9 @@ describe('Client Auth Scenarios', () => { return; } const runner = new InlineClientRunner(goodClient); - await runClientAgainstScenario(runner, scenario.name); + await runClientAgainstScenario(runner, scenario.name, { + allowClientError: allowClientErrorScenarios.has(scenario.name) + }); }); } }); @@ -46,23 +42,23 @@ describe('Client Auth Scenarios', () => { describe('Negative tests', () => { test('bad client requests root PRM location', async () => { const runner = new InlineClientRunner(badPrmClient); - await runClientAgainstScenario(runner, 'auth/metadata-default', [ - 'prm-priority-order' - ]); + await runClientAgainstScenario(runner, 'auth/metadata-default', { + expectedFailureSlugs: ['prm-priority-order'] + }); }); test('client ignores scope from WWW-Authenticate header', async () => { const runner = new InlineClientRunner(ignoreScopeClient); - await runClientAgainstScenario(runner, 'auth/scope-from-www-authenticate', [ - 'scope-from-www-authenticate' - ]); + await runClientAgainstScenario(runner, 'auth/scope-from-www-authenticate', { + expectedFailureSlugs: ['scope-from-www-authenticate'] + }); }); test('client only requests subset of scopes_supported', async () => { const runner = new InlineClientRunner(partialScopesClient); - await runClientAgainstScenario(runner, 'auth/scope-from-scopes-supported', [ - 'scope-from-scopes-supported' - ]); + await runClientAgainstScenario(runner, 'auth/scope-from-scopes-supported', { + expectedFailureSlugs: ['scope-from-scopes-supported'] + }); }); test('client requests scope even if scopes_supported is empty', async () => { @@ -70,21 +66,31 @@ describe('Negative tests', () => { await runClientAgainstScenario( runner, 'auth/scope-omitted-when-undefined', - ['scope-omitted-when-undefined'] + { + expectedFailureSlugs: ['scope-omitted-when-undefined'] + } ); }); test('client only responds to 401, not 403', async () => { const runner = new InlineClientRunner(ignore403Client); - await runClientAgainstScenario(runner, 'auth/scope-step-up', [ - 'scope-step-up-escalation' - ]); + await runClientAgainstScenario(runner, 'auth/scope-step-up', { + expectedFailureSlugs: ['scope-step-up-escalation'] + }); }); test('client uses DCR instead of CIMD when server supports it', async () => { const runner = new InlineClientRunner(noCimdClient); - await runClientAgainstScenario(runner, 'auth/basic-cimd', [ - 'cimd-client-id-used' - ]); + await runClientAgainstScenario(runner, 'auth/basic-cimd', { + expectedFailureSlugs: ['cimd-client-id-used'] + }); + }); + + test('client retries auth infinitely without limit', async () => { + const runner = new InlineClientRunner(noRetryLimitClient); + await runClientAgainstScenario(runner, 'auth/scope-retry-limit', { + expectedFailureSlugs: ['scope-retry-limit'], + allowClientError: true + }); }); }); diff --git a/src/scenarios/client/auth/index.ts b/src/scenarios/client/auth/index.ts index 3e37108..eed1213 100644 --- a/src/scenarios/client/auth/index.ts +++ b/src/scenarios/client/auth/index.ts @@ -9,7 +9,8 @@ import { ScopeFromWwwAuthenticateScenario, ScopeFromScopesSupportedScenario, ScopeOmittedWhenUndefinedScenario, - ScopeStepUpAuthScenario + ScopeStepUpAuthScenario, + ScopeRetryLimitScenario } from './scope-handling'; export const authScenariosList: Scenario[] = [ @@ -20,5 +21,6 @@ export const authScenariosList: Scenario[] = [ new ScopeFromWwwAuthenticateScenario(), new ScopeFromScopesSupportedScenario(), new ScopeOmittedWhenUndefinedScenario(), - new ScopeStepUpAuthScenario() + new ScopeStepUpAuthScenario(), + new ScopeRetryLimitScenario() ]; diff --git a/src/scenarios/client/auth/scope-handling.ts b/src/scenarios/client/auth/scope-handling.ts index 94038f1..137371a 100644 --- a/src/scenarios/client/auth/scope-handling.ts +++ b/src/scenarios/client/auth/scope-handling.ts @@ -351,7 +351,7 @@ export class ScopeStepUpAuthScenario implements Scenario { .status(403) .set( 'WWW-Authenticate', - `Bearer scope="${requiredScopes.join(' ')}", resource_metadata="${resourceMetadataUrl()}"` + `Bearer scope="${requiredScopes.join(' ')}", resource_metadata="${resourceMetadataUrl()}", error="insufficient_scope"` ) .json({ error: 'insufficient_scope', @@ -421,3 +421,173 @@ export class ScopeStepUpAuthScenario implements Scenario { return this.checks; } } + +/** + * Scenario 5: Client implements retry limits for scope escalation + * + * Tests that clients SHOULD implement retry limits to avoid infinite + * authorization loops when receiving repeated 403 insufficient_scope errors. + * The server always returns 403 with the same scope requirement, and clients + * should stop retrying after a reasonable number of attempts (3 or fewer). + */ +export class ScopeRetryLimitScenario implements Scenario { + name = 'auth/scope-retry-limit'; + description = + 'Tests that client implements retry limits to prevent infinite authorization loops on repeated 403 responses'; + private authServer = new ServerLifecycle(); + private server = new ServerLifecycle(); + private checks: ConformanceCheck[] = []; + + async start(): Promise { + this.checks = []; + + const requiredScope = 'mcp:admin'; + const tokenVerifier = new MockTokenVerifier(this.checks, []); + let authRequestCount = 0; + + const authApp = createAuthServer(this.checks, this.authServer.getUrl, { + tokenVerifier, + onAuthorizationRequest: (data) => { + authRequestCount++; + this.checks.push({ + id: 'scope-retry-auth-attempt', + name: `Authorization attempt ${authRequestCount}`, + description: `Client made authorization request attempt ${authRequestCount}`, + status: 'INFO', + timestamp: data.timestamp, + specReferences: [SpecReferences.MCP_SCOPE_CHALLENGE_HANDLING], + details: { + attemptNumber: authRequestCount, + requestedScope: data.scope || 'none' + } + }); + } + }); + await this.authServer.start(authApp); + + const resourceMetadataUrl = () => + `${this.server.getUrl()}/.well-known/oauth-protected-resource/mcp`; + + const maxAttempts = 3; + let mcpRequestWithTokenCount = 0; + + const alwaysDeny403Middleware = async ( + req: Request, + res: Response, + next: NextFunction + ) => { + let body = req.body; + if (typeof body === 'string') { + body = JSON.parse(body); + } + const method = body?.method; + + if (method === 'initialize' || method?.startsWith('notifications/')) { + return next(); + } + + const authHeader = req.headers.authorization; + if (!authHeader || !authHeader.startsWith('Bearer ')) { + return res + .status(401) + .set( + 'WWW-Authenticate', + `Bearer scope="${requiredScope}", resource_metadata="${resourceMetadataUrl()}"` + ) + .json({ + error: 'invalid_token', + error_description: 'Missing Authorization header' + }); + } + + mcpRequestWithTokenCount++; + + if (mcpRequestWithTokenCount > maxAttempts) { + return res.status(410).json({ + error: 'test_complete', + error_description: + 'Test is over - client exceeded maximum retry attempts' + }); + } + + return res + .status(403) + .set( + 'WWW-Authenticate', + `Bearer error="insufficient_scope", scope="${requiredScope}", resource_metadata="${resourceMetadataUrl()}", error_description="Scope upgrade will never succeed"` + ) + .json({ + error: 'insufficient_scope', + error_description: 'Scope upgrade will never succeed' + }); + }; + + const baseApp = createServer( + this.checks, + this.server.getUrl, + this.authServer.getUrl, + { + prmPath: '/.well-known/oauth-protected-resource/mcp', + requiredScopes: [requiredScope], + scopesSupported: [requiredScope], + includeScopeInWwwAuth: true, + authMiddleware: alwaysDeny403Middleware, + tokenVerifier + } + ); + + await this.server.start(baseApp); + + return { serverUrl: `${this.server.getUrl()}/mcp` }; + } + + async stop() { + await this.authServer.stop(); + await this.server.stop(); + } + + getChecks(): ConformanceCheck[] { + const authAttempts = this.checks.filter( + (c) => c.id === 'scope-retry-auth-attempt' + ).length; + + if (authAttempts === 0) { + this.checks.push({ + id: 'scope-retry-limit', + name: 'Client retry limit for scope escalation', + description: 'Client did not make any authorization requests', + status: 'FAILURE', + timestamp: new Date().toISOString(), + specReferences: [SpecReferences.MCP_SCOPE_CHALLENGE_HANDLING] + }); + } else if (authAttempts <= 3) { + this.checks.push({ + id: 'scope-retry-limit', + name: 'Client retry limit for scope escalation', + description: `Client correctly limited retry attempts to ${authAttempts} (3 or fewer)`, + status: 'SUCCESS', + timestamp: new Date().toISOString(), + specReferences: [SpecReferences.MCP_SCOPE_CHALLENGE_HANDLING], + details: { + authorizationAttempts: authAttempts, + maxAllowed: 3 + } + }); + } else { + this.checks.push({ + id: 'scope-retry-limit', + name: 'Client retry limit for scope escalation', + description: `Client made ${authAttempts} authorization attempts (more than 3). Clients SHOULD implement retry limits to avoid infinite loops.`, + status: 'FAILURE', + timestamp: new Date().toISOString(), + specReferences: [SpecReferences.MCP_SCOPE_CHALLENGE_HANDLING], + details: { + authorizationAttempts: authAttempts, + maxAllowed: 3 + } + }); + } + + return this.checks; + } +} diff --git a/src/scenarios/client/auth/test_helpers/testClient.ts b/src/scenarios/client/auth/test_helpers/testClient.ts index df0f70c..45f4e6d 100644 --- a/src/scenarios/client/auth/test_helpers/testClient.ts +++ b/src/scenarios/client/auth/test_helpers/testClient.ts @@ -83,16 +83,19 @@ export class InlineClientRunner implements ClientRunner { } } +export interface RunClientOptions { + expectedFailureSlugs?: string[]; + allowClientError?: boolean; +} + export async function runClientAgainstScenario( - clientRunner: ClientRunner | string, + clientRunner: ClientRunner, scenarioName: string, - expectedFailureSlugs: string[] = [] + options: RunClientOptions = {} ): Promise { - // Handle backward compatibility: if string is passed, treat as file path - const runner = - typeof clientRunner === 'string' - ? new SpawnedClientRunner(clientRunner) - : clientRunner; + const { expectedFailureSlugs = [], allowClientError = false } = options; + + const runner = clientRunner; const scenario = getScenario(scenarioName); if (!scenario) { @@ -108,10 +111,10 @@ export async function runClientAgainstScenario( try { await runner.run(serverUrl); } catch (err) { - if (expectedFailureSlugs.length === 0) { + if (expectedFailureSlugs.length === 0 && !allowClientError) { throw err; // Unexpected failure } - // Otherwise, expected failure - continue to checks verification + // Otherwise, expected failure or allowed error - continue to checks verification } // Get checks from the scenario diff --git a/src/scenarios/index.ts b/src/scenarios/index.ts index 2a41815..f0d1e78 100644 --- a/src/scenarios/index.ts +++ b/src/scenarios/index.ts @@ -58,7 +58,12 @@ const pendingClientScenariosList: ClientScenario[] = [ // JSON Schema 2020-12 (SEP-1613) // This test is pending until the SDK includes PR #1135 which preserves // $schema, $defs, and additionalProperties fields in tool schemas. - new JsonSchema2020_12Scenario() + new JsonSchema2020_12Scenario(), + + // On hold until elicitation schema types are fixed + // https://github.com/modelcontextprotocol/modelcontextprotocol/pull/1863 + new ToolsCallElicitationScenario(), + new ElicitationDefaultsScenario() ]; // All client scenarios @@ -90,7 +95,7 @@ const allClientScenariosList: ClientScenario[] = [ new ElicitationDefaultsScenario(), // Elicitation scenarios (SEP-1330) - pending - ...pendingClientScenariosList, + new ElicitationEnumsScenario(), // Resources scenarios new ResourcesListScenario(), @@ -159,6 +164,10 @@ export function listActiveClientScenarios(): string[] { return activeClientScenariosList.map((scenario) => scenario.name); } +export function listPendingClientScenarios(): string[] { + return pendingClientScenariosList.map((scenario) => scenario.name); +} + export function listAuthScenarios(): string[] { return authScenariosList.map((scenario) => scenario.name); }