From cc22d163cbbc8c4f892c812cae21f55b0e8829e4 Mon Sep 17 00:00:00 2001 From: Konstantin Konstantinov Date: Thu, 3 Sep 2026 19:59:10 +0300 Subject: [PATCH] feat(client/auth): check the resource parameter matches the PRM-published identifier Adds resource-parameter-matches-prm to the RFC 8707 checks, emitted by the token-endpoint-auth and metadata-discovery scenarios; auth/metadata-var2 serves a pathless resource, so a client that re-serializes it through URL.href fails. Negative example client included. The everything-client (SDK 1.x) is baselined on that scenario until typescript-sdk#1972 ships. --- .../typescript/auth-test-resource-slash.ts | 73 +++++++ .../client/auth/discovery-metadata.ts | 34 +++- .../client/auth/helpers/createServer.ts | 7 +- .../auth/helpers/resourceParameterChecks.ts | 178 ++++++++++++++++++ src/scenarios/client/auth/index.test.ts | 28 ++- src/scenarios/client/auth/spec-references.ts | 8 + .../client/auth/token-endpoint-auth.ts | 126 ++----------- 7 files changed, 340 insertions(+), 114 deletions(-) create mode 100644 examples/clients/typescript/auth-test-resource-slash.ts create mode 100644 src/scenarios/client/auth/helpers/resourceParameterChecks.ts diff --git a/examples/clients/typescript/auth-test-resource-slash.ts b/examples/clients/typescript/auth-test-resource-slash.ts new file mode 100644 index 00000000..191cf221 --- /dev/null +++ b/examples/clients/typescript/auth-test-resource-slash.ts @@ -0,0 +1,73 @@ +#!/usr/bin/env node + +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { withOAuthRetryWithProvider } from './helpers/withOAuthRetry'; +import { ConformanceOAuthProvider } from './helpers/ConformanceOAuthProvider'; +import { runAsCli } from './helpers/cliRunner'; +import { logger } from './helpers/logger'; + +/** + * Broken client that re-serializes the protected resource metadata's + * `resource` through a URL parser before sending it as the RFC 8707 + * `resource` parameter. + * + * BUG: `new URL('https://example.com').href` is `https://example.com/`, so a + * pathless resource identifier gains a trailing slash and no longer matches + * the value the server published. Authorization servers that compare the + * indicator exactly (Microsoft Entra ID, AADSTS9010010) reject the request. + * This is the shape of typescript-sdk#1968 and python-sdk#2578. + */ +class SlashAppendingResourceProvider extends ConformanceOAuthProvider { + async validateResourceURL( + _serverUrl: string | URL, + resource?: string + ): Promise { + if (!resource) { + return undefined; + } + const url = new URL(resource); + // BUG: always emit the normalized form with a trailing slash, whatever + // the server published. + if (!url.pathname.endsWith('/')) { + url.pathname = `${url.pathname}/`; + } + return url; + } +} + +export async function runClient(serverUrl: string): Promise { + const provider = new SlashAppendingResourceProvider( + 'http://localhost:3000/callback', + { + client_name: 'test-auth-client-resource-slash', + redirect_uris: ['http://localhost:3000/callback'], + application_type: 'native' + } + ); + + const client = new Client( + { name: 'test-auth-client-resource-slash', version: '1.0.0' }, + { capabilities: {} } + ); + + const oauthFetch = withOAuthRetryWithProvider( + provider, + 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-resource-slash '); diff --git a/src/scenarios/client/auth/discovery-metadata.ts b/src/scenarios/client/auth/discovery-metadata.ts index 3c53e08d..c5623a92 100644 --- a/src/scenarios/client/auth/discovery-metadata.ts +++ b/src/scenarios/client/auth/discovery-metadata.ts @@ -12,6 +12,7 @@ import { ScenarioUrls } from '../../../types'; import { createAuthServer } from './helpers/createAuthServer'; import { createServer } from './helpers/createServer'; import { ServerLifecycle } from './helpers/serverLifecycle'; +import { addResourceParameterChecks } from './helpers/resourceParameterChecks'; import { SpecReferences } from './spec-references'; import { Request, Response } from 'express'; @@ -77,6 +78,13 @@ function createMetadataScenario(config: MetadataScenarioConfig): Scenario { const server = new ServerLifecycle(); let checks: ConformanceCheck[] = []; + // Track resource parameters for RFC 8707 validation. metadata-var2 serves + // the PRM at the root, so its `resource` is a bare origin: the case a URL + // parser rewrites with a trailing slash. + let authorizationResource: string | undefined; + let tokenResource: string | undefined; + let prmResource: string | undefined; + const routePrefix = config.authRoutePrefix || ''; const isOpenIdConfiguration = config.oauthMetadataLocation.includes( 'openid-configuration' @@ -97,11 +105,23 @@ function createMetadataScenario(config: MetadataScenarioConfig): Scenario { async start(ctx: ScenarioContext): Promise { checks = []; + authorizationResource = undefined; + tokenResource = undefined; + prmResource = undefined; const authApp = createAuthServer(ctx, checks, authServer.getUrl, { metadataPath: config.oauthMetadataLocation, isOpenIdConfiguration, - ...(routePrefix && { routePrefix }) + ...(routePrefix && { routePrefix }), + onAuthorizationRequest: ({ resource }) => { + authorizationResource = resource; + }, + onTokenRequest: ({ body }) => { + tokenResource = body.resource; + // Same token the auth server mints by default; these scenarios + // request no scopes. + return { token: `test-token-${Date.now()}`, scopes: [] }; + } }); // If path-based OAuth metadata, trap root requests @@ -134,7 +154,10 @@ function createMetadataScenario(config: MetadataScenarioConfig): Scenario { const app = createServer(ctx, checks, server.getUrl, getAuthServerUrl, { prmPath: config.prmLocation, - includePrmInWwwAuth: config.inWwwAuth + includePrmInWwwAuth: config.inWwwAuth, + onPrmRequest: ({ resource }) => { + prmResource = resource; + } }); // Add trap for root PRM requests if configured @@ -198,6 +221,13 @@ function createMetadataScenario(config: MetadataScenarioConfig): Scenario { } } + // RFC 8707 Resource Parameter Validation Checks + addResourceParameterChecks( + checks, + { authorizationResource, tokenResource, prmResource }, + new Date().toISOString() + ); + return checks; } }; diff --git a/src/scenarios/client/auth/helpers/createServer.ts b/src/scenarios/client/auth/helpers/createServer.ts index cd32d21b..6d308d64 100644 --- a/src/scenarios/client/auth/helpers/createServer.ts +++ b/src/scenarios/client/auth/helpers/createServer.ts @@ -30,6 +30,8 @@ export interface ServerOptions { tokenVerifier?: MockTokenVerifier; /** Override the resource field in PRM response (for testing resource mismatch) */ prmResourceOverride?: string; + /** Observe the `resource` identifier the PRM route served (RFC 8707 checks) */ + onPrmRequest?: (requestData: { resource: string; timestamp: string }) => void; } export function createServer( @@ -46,7 +48,8 @@ export function createServer( includePrmInWwwAuth = true, includeScopeInWwwAuth = false, tokenVerifier, - prmResourceOverride + prmResourceOverride, + onPrmRequest } = options; // Factory: create a fresh Server per request to avoid "Already connected" errors // after the v1.26.0 security fix (GHSA-345p-7cg4-v4c7) @@ -130,6 +133,8 @@ export function createServer( ? getBaseUrl() : `${getBaseUrl()}/mcp`); + onPrmRequest?.({ resource, timestamp: new Date().toISOString() }); + const prmResponse: any = { resource, authorization_servers: [getAuthServerUrl()] diff --git a/src/scenarios/client/auth/helpers/resourceParameterChecks.ts b/src/scenarios/client/auth/helpers/resourceParameterChecks.ts new file mode 100644 index 00000000..e7223f1e --- /dev/null +++ b/src/scenarios/client/auth/helpers/resourceParameterChecks.ts @@ -0,0 +1,178 @@ +import type { ConformanceCheck } from '../../../../types'; +import { SpecReferences } from '../spec-references'; + +/** + * What a scenario's mock servers observed about the RFC 8707 `resource` + * parameter: the values the client sent to the authorization and token + * endpoints, and the identifier the protected resource metadata served. + */ +export interface ResourceParameterObservation { + /** `resource` query parameter the client sent to the authorization endpoint. */ + authorizationResource?: string; + /** `resource` form parameter the client sent to the token endpoint. */ + tokenResource?: string; + /** `resource` value the protected resource metadata document served. */ + prmResource?: string; +} + +/** + * RFC 8707 resource-parameter checks, shared by every client-auth scenario + * whose mock servers observe the authorization and token requests. The check + * IDs are stable across scenarios so one slug finds every emission. + * + * Each check is emitted once per scenario run; a check already present under + * the same id is left alone. + */ +export function addResourceParameterChecks( + checks: ConformanceCheck[], + observed: ResourceParameterObservation, + timestamp: string +): void { + const { authorizationResource, tokenResource, prmResource } = observed; + const specRefs = [ + SpecReferences.RFC_8707_RESOURCE_INDICATORS, + SpecReferences.MCP_RESOURCE_PARAMETER + ]; + + // Check 1: Resource parameter in authorization request + if (!checks.some((c) => c.id === 'resource-parameter-in-authorization')) { + const hasResource = !!authorizationResource; + checks.push({ + id: 'resource-parameter-in-authorization', + name: 'Resource parameter in authorization request', + description: hasResource + ? 'Client included resource parameter in authorization request' + : 'Client MUST include resource parameter in authorization request per RFC 8707', + status: hasResource ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: specRefs, + details: { + resource: authorizationResource || 'not provided' + } + }); + } + + // Check 2: Resource parameter in token request + if (!checks.some((c) => c.id === 'resource-parameter-in-token')) { + const hasResource = !!tokenResource; + checks.push({ + id: 'resource-parameter-in-token', + name: 'Resource parameter in token request', + description: hasResource + ? 'Client included resource parameter in token request' + : 'Client MUST include resource parameter in token request per RFC 8707', + status: hasResource ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: specRefs, + details: { + resource: tokenResource || 'not provided' + } + }); + } + + // Check 3: Resource parameter is valid canonical URI + if (!checks.some((c) => c.id === 'resource-parameter-valid-uri')) { + const resourceToValidate = authorizationResource || tokenResource; + if (resourceToValidate) { + const validation = validateCanonicalUri(resourceToValidate); + checks.push({ + id: 'resource-parameter-valid-uri', + name: 'Resource parameter is valid canonical URI', + description: validation.valid + ? 'Resource parameter is a valid canonical URI (has scheme, no fragment)' + : `Resource parameter is invalid: ${validation.error}`, + status: validation.valid ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: specRefs, + details: { + resource: resourceToValidate, + ...(validation.error && { error: validation.error }) + } + }); + } + } + + // Check 4: Resource parameter consistency between requests + if (!checks.some((c) => c.id === 'resource-parameter-consistency')) { + if (authorizationResource && tokenResource) { + const consistent = authorizationResource === tokenResource; + checks.push({ + id: 'resource-parameter-consistency', + name: 'Resource parameter consistency', + description: consistent + ? 'Resource parameter is consistent between authorization and token requests' + : 'Resource parameter MUST be consistent between authorization and token requests', + status: consistent ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: specRefs, + details: { + authorizationResource, + tokenResource + } + }); + } + } + + // Check 5: Resource parameter equals the identifier published in PRM. + // The MCP spec requires the canonical URI, which it defines as the RFC 9728 + // `resource` value; RFC 9728 ยง3.3 requires that value to be identical to the + // identifier the client used. Re-serializing it through a URL parser is the + // usual way to break this: `new URL('https://example.com').href` is + // `https://example.com/`, and authorization servers that match the + // indicator exactly reject the extra slash. + if (!checks.some((c) => c.id === 'resource-parameter-matches-prm')) { + const sent: Array<[request: string, value: string]> = []; + if (authorizationResource !== undefined) { + sent.push(['authorization', authorizationResource]); + } + if (tokenResource !== undefined) { + sent.push(['token', tokenResource]); + } + if (prmResource !== undefined && sent.length > 0) { + const mismatched = sent + .filter(([, value]) => value !== prmResource) + .map(([request]) => request); + const matches = mismatched.length === 0; + const errorMessage = `Client MUST send the resource identifier exactly as published in protected resource metadata; the ${mismatched.join(' and ')} request sent a different value (a URL parser that appends "/" to a pathless identifier is the usual cause)`; + checks.push({ + id: 'resource-parameter-matches-prm', + name: 'Resource parameter matches protected resource metadata', + description: matches + ? 'Client sent the resource identifier exactly as published in protected resource metadata' + : errorMessage, + status: matches ? 'SUCCESS' : 'FAILURE', + timestamp, + specReferences: [ + ...specRefs, + SpecReferences.MCP_CANONICAL_SERVER_URI, + SpecReferences.RFC_9728_RESOURCE_IDENTITY + ], + ...(!matches && { errorMessage }), + details: { + prmResource, + authorizationResource: authorizationResource ?? 'not provided', + tokenResource: tokenResource ?? 'not provided' + } + }); + } + } +} + +function validateCanonicalUri(uri: string): { + valid: boolean; + error?: string; +} { + try { + const parsed = new URL(uri); + // Check for fragment (RFC 8707: MUST NOT include fragment) + if (parsed.hash) { + return { + valid: false, + error: 'contains fragment (not allowed per RFC 8707)' + }; + } + return { valid: true }; + } catch { + return { valid: false, error: 'invalid URI format' }; + } +} diff --git a/src/scenarios/client/auth/index.test.ts b/src/scenarios/client/auth/index.test.ts index 57ff27ac..61062f73 100644 --- a/src/scenarios/client/auth/index.test.ts +++ b/src/scenarios/client/auth/index.test.ts @@ -35,6 +35,7 @@ import { runClient as dpopNoAsNonceClient } from '../../../../examples/clients/t import { runClient as dpopNoRsNonceClient } from '../../../../examples/clients/typescript/auth-test-dpop-no-rs-nonce'; import { runClient as dpopNoNonceClient } from '../../../../examples/clients/typescript/auth-test-dpop-no-nonce'; import { runClient as dpopClient } from '../../../../examples/clients/typescript/auth-test-dpop'; +import { runClient as resourceSlashClient } from '../../../../examples/clients/typescript/auth-test-resource-slash'; import { getHandler } from '../../../../examples/clients/typescript/everything-client'; import { setLogLevel } from '../../../../examples/clients/typescript/helpers/logger'; import { DRAFT_PROTOCOL_VERSION } from '../../../types'; @@ -64,6 +65,21 @@ const allowClientErrorScenarios = new Set([ 'auth/metadata-issuer-mismatch' ]); +/** + * Checks the everything-client is known to fail because of a bug in the SDK + * release it depends on. Each entry is asserted to still fail, so the entry + * must be removed as soon as the pinned SDK passes. + * + * - `resource-parameter-matches-prm` on the root-PRM scenario: + * @modelcontextprotocol/sdk 1.x re-serializes a pathless PRM `resource` + * through `URL.href`, adding a trailing slash (typescript-sdk#1968, fixed + * on 2.x by #2581; 1.x backport #1972). Remove once the example's SDK + * dependency includes the backport. + */ +const knownExampleClientFailures: Record = { + 'auth/metadata-var2': ['resource-parameter-matches-prm'] +}; + describe('Client Auth Scenarios', () => { // Generate individual test for each auth scenario for (const scenario of authScenariosList) { @@ -78,7 +94,8 @@ describe('Client Auth Scenarios', () => { } const runner = new InlineClientRunner(clientFn); await runClientAgainstScenario(runner, scenario.name, { - allowClientError: allowClientErrorScenarios.has(scenario.name) + allowClientError: allowClientErrorScenarios.has(scenario.name), + expectedFailureSlugs: knownExampleClientFailures[scenario.name] }); }); } @@ -120,6 +137,15 @@ describe('Negative tests', () => { }); }); + test('client appends a trailing slash to the PRM resource identifier', async () => { + // auth/metadata-var2 serves the PRM at the root, so its `resource` is a + // bare origin: exactly the value a URL parser rewrites with a "/". + const runner = new InlineClientRunner(resourceSlashClient); + await runClientAgainstScenario(runner, 'auth/metadata-var2', { + expectedFailureSlugs: ['resource-parameter-matches-prm'] + }); + }); + test('client ignores scope from WWW-Authenticate header', async () => { const runner = new InlineClientRunner(ignoreScopeClient); await runClientAgainstScenario(runner, 'auth/scope-from-www-authenticate', { diff --git a/src/scenarios/client/auth/spec-references.ts b/src/scenarios/client/auth/spec-references.ts index 972417bd..980c0305 100644 --- a/src/scenarios/client/auth/spec-references.ts +++ b/src/scenarios/client/auth/spec-references.ts @@ -81,6 +81,14 @@ export const SpecReferences: { [key: string]: SpecReference } = { id: 'MCP-Resource-Parameter-Implementation', url: 'https://modelcontextprotocol.io/specification/draft/basic/authorization#resource-parameter-implementation' }, + MCP_CANONICAL_SERVER_URI: { + id: 'MCP-Canonical-Server-URI', + url: 'https://modelcontextprotocol.io/specification/draft/basic/authorization#canonical-server-uri' + }, + RFC_9728_RESOURCE_IDENTITY: { + id: 'RFC-9728-resource-identity', + url: 'https://www.rfc-editor.org/rfc/rfc9728.html#section-3.3' + }, MCP_PREREGISTRATION: { id: 'MCP-Preregistration', url: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#preregistration' diff --git a/src/scenarios/client/auth/token-endpoint-auth.ts b/src/scenarios/client/auth/token-endpoint-auth.ts index 53643fd2..ae046e22 100644 --- a/src/scenarios/client/auth/token-endpoint-auth.ts +++ b/src/scenarios/client/auth/token-endpoint-auth.ts @@ -6,6 +6,7 @@ import { createServer } from './helpers/createServer.js'; import { ServerLifecycle } from './helpers/serverLifecycle.js'; import { SpecReferences } from './spec-references.js'; import { MockTokenVerifier } from './helpers/mockTokenVerifier.js'; +import { addResourceParameterChecks } from './helpers/resourceParameterChecks.js'; type AuthMethod = 'client_secret_basic' | 'client_secret_post' | 'none'; @@ -56,6 +57,7 @@ class TokenEndpointAuthScenario implements Scenario { // Track resource parameters for RFC 8707 validation private authorizationResource?: string; private tokenResource?: string; + private prmResource?: string; constructor(expectedAuthMethod: AuthMethod) { this.expectedAuthMethod = expectedAuthMethod; @@ -67,6 +69,7 @@ class TokenEndpointAuthScenario implements Scenario { this.checks = []; this.authorizationResource = undefined; this.tokenResource = undefined; + this.prmResource = undefined; const tokenVerifier = new MockTokenVerifier(this.checks, []); const authApp = createAuthServer(ctx, this.checks, this.authServer.getUrl, { @@ -145,7 +148,10 @@ class TokenEndpointAuthScenario implements Scenario { { prmPath: '/.well-known/oauth-protected-resource/mcp', requiredScopes: [], - tokenVerifier + tokenVerifier, + onPrmRequest: ({ resource }) => { + this.prmResource = resource; + } } ); await this.server.start(app); @@ -173,118 +179,18 @@ class TokenEndpointAuthScenario implements Scenario { } // RFC 8707 Resource Parameter Validation Checks - this.addResourceParameterChecks(timestamp); + addResourceParameterChecks( + this.checks, + { + authorizationResource: this.authorizationResource, + tokenResource: this.tokenResource, + prmResource: this.prmResource + }, + timestamp + ); return this.checks; } - - private addResourceParameterChecks(timestamp: string): void { - const specRefs = [ - SpecReferences.RFC_8707_RESOURCE_INDICATORS, - SpecReferences.MCP_RESOURCE_PARAMETER - ]; - - // Check 1: Resource parameter in authorization request - if ( - !this.checks.some((c) => c.id === 'resource-parameter-in-authorization') - ) { - const hasResource = !!this.authorizationResource; - this.checks.push({ - id: 'resource-parameter-in-authorization', - name: 'Resource parameter in authorization request', - description: hasResource - ? 'Client included resource parameter in authorization request' - : 'Client MUST include resource parameter in authorization request per RFC 8707', - status: hasResource ? 'SUCCESS' : 'FAILURE', - timestamp, - specReferences: specRefs, - details: { - resource: this.authorizationResource || 'not provided' - } - }); - } - - // Check 2: Resource parameter in token request - if (!this.checks.some((c) => c.id === 'resource-parameter-in-token')) { - const hasResource = !!this.tokenResource; - this.checks.push({ - id: 'resource-parameter-in-token', - name: 'Resource parameter in token request', - description: hasResource - ? 'Client included resource parameter in token request' - : 'Client MUST include resource parameter in token request per RFC 8707', - status: hasResource ? 'SUCCESS' : 'FAILURE', - timestamp, - specReferences: specRefs, - details: { - resource: this.tokenResource || 'not provided' - } - }); - } - - // Check 3: Resource parameter is valid canonical URI - if (!this.checks.some((c) => c.id === 'resource-parameter-valid-uri')) { - const resourceToValidate = - this.authorizationResource || this.tokenResource; - if (resourceToValidate) { - const validation = this.validateCanonicalUri(resourceToValidate); - this.checks.push({ - id: 'resource-parameter-valid-uri', - name: 'Resource parameter is valid canonical URI', - description: validation.valid - ? 'Resource parameter is a valid canonical URI (has scheme, no fragment)' - : `Resource parameter is invalid: ${validation.error}`, - status: validation.valid ? 'SUCCESS' : 'FAILURE', - timestamp, - specReferences: specRefs, - details: { - resource: resourceToValidate, - ...(validation.error && { error: validation.error }) - } - }); - } - } - - // Check 4: Resource parameter consistency between requests - if (!this.checks.some((c) => c.id === 'resource-parameter-consistency')) { - if (this.authorizationResource && this.tokenResource) { - const consistent = this.authorizationResource === this.tokenResource; - this.checks.push({ - id: 'resource-parameter-consistency', - name: 'Resource parameter consistency', - description: consistent - ? 'Resource parameter is consistent between authorization and token requests' - : 'Resource parameter MUST be consistent between authorization and token requests', - status: consistent ? 'SUCCESS' : 'FAILURE', - timestamp, - specReferences: specRefs, - details: { - authorizationResource: this.authorizationResource, - tokenResource: this.tokenResource - } - }); - } - } - } - - private validateCanonicalUri(uri: string): { - valid: boolean; - error?: string; - } { - try { - const parsed = new URL(uri); - // Check for fragment (RFC 8707: MUST NOT include fragment) - if (parsed.hash) { - return { - valid: false, - error: 'contains fragment (not allowed per RFC 8707)' - }; - } - return { valid: true }; - } catch { - return { valid: false, error: 'invalid URI format' }; - } - } } export class ClientSecretBasicAuthScenario extends TokenEndpointAuthScenario {