diff --git a/examples/clients/typescript/auth-test-echo-scope.ts b/examples/clients/typescript/auth-test-echo-scope.ts new file mode 100644 index 0000000..7a197c6 --- /dev/null +++ b/examples/clients/typescript/auth-test-echo-scope.ts @@ -0,0 +1,62 @@ +/** + * Negative client for SEP-2350: on a step-up 403, re-authorizes with ONLY the + * scope from the challenge (does not accumulate previously-granted scopes). + * Expected to trigger sep-2350-scope-union-on-reauth = WARNING. + */ +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { + auth, + extractWWWAuthenticateParams +} from '@modelcontextprotocol/sdk/client/auth.js'; +import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'; +import { withOAuthRetry } from './helpers/withOAuthRetry'; +import { ConformanceOAuthProvider } from './helpers/ConformanceOAuthProvider'; +import { runAsCli } from './helpers/cliRunner'; + +// handle401 variant that ECHOES the challenge scope verbatim (no SEP-2350 union). +const handle401EchoScope = async ( + response: Response, + provider: ConformanceOAuthProvider, + next: FetchLike, + serverUrl: string | URL +): Promise => { + 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 + }); + } +}; + +export async function runClient(serverUrl: string): Promise { + const client = new Client( + { name: 'auth-test-echo-scope', version: '1.0.0' }, + { capabilities: {} } + ); + const oauthFetch = withOAuthRetry( + 'auth-test-echo-scope', + new URL(serverUrl), + handle401EchoScope + )(fetch); + const transport = new StreamableHTTPClientTransport(new URL(serverUrl), { + fetch: oauthFetch + }); + await client.connect(transport); + await client.listTools(); + await client.callTool({ name: 'test-tool', arguments: {} }); + await transport.close(); +} + +runAsCli(runClient, import.meta.url, 'auth-test-echo-scope '); diff --git a/examples/clients/typescript/helpers/withOAuthRetry.ts b/examples/clients/typescript/helpers/withOAuthRetry.ts index 3d5c898..92767e9 100644 --- a/examples/clients/typescript/helpers/withOAuthRetry.ts +++ b/examples/clients/typescript/helpers/withOAuthRetry.ts @@ -7,13 +7,33 @@ import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'; import type { Middleware } from '@modelcontextprotocol/sdk/client/middleware.js'; import { ConformanceOAuthProvider } from './ConformanceOAuthProvider'; +/** + * SEP-2350: a well-behaved client computes the union of previously-granted + * scopes and the newly challenged scopes when re-authorizing, so it doesn't + * lose permissions needed for other operations. + */ +const unionScopes = ( + prior: string | undefined, + challenged: string | undefined +): string | undefined => { + const set = new Set( + [...(prior?.split(' ') ?? []), ...(challenged?.split(' ') ?? [])].filter( + Boolean + ) + ); + return set.size > 0 ? [...set].join(' ') : undefined; +}; + export const handle401 = async ( response: Response, provider: ConformanceOAuthProvider, next: FetchLike, serverUrl: string | URL ): Promise => { - const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response); + const { resourceMetadataUrl, scope: challengedScope } = + extractWWWAuthenticateParams(response); + const prior = (await provider.tokens())?.scope; + const scope = unionScopes(prior, challengedScope); let result = await auth(provider, { serverUrl, resourceMetadataUrl, diff --git a/src/scenarios/client/auth/index.test.ts b/src/scenarios/client/auth/index.test.ts index cd09793..929c528 100644 --- a/src/scenarios/client/auth/index.test.ts +++ b/src/scenarios/client/auth/index.test.ts @@ -17,6 +17,7 @@ import { runClient as noPkceClient } from '../../../../examples/clients/typescri import { runClient as reuseCredsClient } from '../../../../examples/clients/typescript/auth-test-reuse-credentials'; import { runClient as noAppTypeClient } from '../../../../examples/clients/typescript/auth-test-no-application-type'; import { runClient as noIssValidationClient } from '../../../../examples/clients/typescript/auth-test'; +import { runClient as echoScopeClient } from '../../../../examples/clients/typescript/auth-test-echo-scope'; import { getHandler } from '../../../../examples/clients/typescript/everything-client'; import { setLogLevel } from '../../../../examples/clients/typescript/helpers/logger'; @@ -126,7 +127,17 @@ describe('Negative tests', () => { test('client only responds to 401, not 403', async () => { const runner = new InlineClientRunner(ignore403Client); await runClientAgainstScenario(runner, 'auth/scope-step-up', { - expectedFailureSlugs: ['scope-step-up-escalation'] + expectedFailureSlugs: [ + 'scope-step-up-escalation', + 'sep-2350-scope-union-on-reauth' + ] + }); + }); + + test('client echoes challenge scope without accumulating prior grant (SEP-2350)', async () => { + const runner = new InlineClientRunner(echoScopeClient); + await runClientAgainstScenario(runner, 'auth/scope-step-up', { + expectedFailureSlugs: ['sep-2350-scope-union-on-reauth'] }); }); diff --git a/src/scenarios/client/auth/scope-handling.ts b/src/scenarios/client/auth/scope-handling.ts index e1d0ff1..c24d51d 100644 --- a/src/scenarios/client/auth/scope-handling.ts +++ b/src/scenarios/client/auth/scope-handling.ts @@ -295,7 +295,11 @@ export class ScopeStepUpAuthScenario implements Scenario { this.checks = []; const initialScope = 'mcp:basic'; - const escalatedScopes = ['mcp:basic', 'mcp:write']; + // tools/call gates on mcp:write only (not the union) so the scenario can + // complete even for clients that don't accumulate; the SEP-2350 check then + // observes whether the previously-granted mcp:basic was retained. + const stepUpScope = 'mcp:write'; + const escalatedScopes = [initialScope, stepUpScope]; const tokenVerifier = new MockTokenVerifier(this.checks, escalatedScopes); let authRequestCount = 0; @@ -323,21 +327,40 @@ export class ScopeStepUpAuthScenario implements Scenario { } }); } else if (authRequestCount === 2) { - // Second auth request - should escalate to mcp:basic + mcp:write - const hasAllScopes = escalatedScopes.every((s) => - requestedScopes.includes(s) - ); + // Second auth request after a 403 challenge that listed only the + // *missing* scope (mcp:write). Two distinct assertions: + // - escalation: client included the challenged scope at all + // - SEP-2350 union: client *also* kept the previously-granted scope + // that was NOT in the challenge (i.e., computed prior ∪ challenge) + const includesChallenged = requestedScopes.includes(stepUpScope); this.checks.push({ id: 'scope-step-up-escalation', name: 'Client scope escalation for step-up auth', - description: hasAllScopes - ? 'Client correctly escalated scopes for step-up authentication' + description: includesChallenged + ? 'Client correctly requested the challenged scope for step-up authentication' : 'Client SHOULD request additional scopes when receiving 403 with new scope requirements', - status: hasAllScopes ? 'SUCCESS' : 'WARNING', + status: includesChallenged ? 'SUCCESS' : 'WARNING', timestamp: data.timestamp, specReferences: [SpecReferences.MCP_SCOPE_SELECTION_STRATEGY], details: { - expectedScopes: escalatedScopes.join(' '), + challengedScope: stepUpScope, + requestedScope: data.scope || 'none' + } + }); + + const retainedPrior = requestedScopes.includes(initialScope); + this.checks.push({ + id: 'sep-2350-scope-union-on-reauth', + name: 'Client accumulates previously-granted scopes on re-authorization', + description: retainedPrior + ? 'Client included previously-granted scopes alongside the newly challenged scope when re-authorizing' + : 'Client SHOULD compute the union of previously requested scopes and newly challenged scopes when initiating re-authorization (SEP-2350); previously-granted scope was dropped', + status: retainedPrior ? 'SUCCESS' : 'WARNING', + timestamp: data.timestamp, + specReferences: [SpecReferences.MCP_SCOPE_CHALLENGE_HANDLING], + details: { + previouslyGranted: initialScope, + challengedScope: stepUpScope, requestedScope: data.scope || 'none' } }); @@ -388,19 +411,21 @@ export class ScopeStepUpAuthScenario implements Scenario { // Determine required scopes based on method const isToolCall = method === 'tools/call'; - const requiredScopes = isToolCall ? escalatedScopes : [initialScope]; + const requiredScopes = isToolCall ? [stepUpScope] : [initialScope]; const hasRequiredScopes = requiredScopes.every((s) => tokenScopes.includes(s) ); if (!hasRequiredScopes) { - // Has token but insufficient scopes - return 403 + // Has token but insufficient scopes - return 403. Challenge with only + // the step-up scope so SEP-2350 union accumulation is observable: a + // client that just echoes the challenge would drop mcp:basic. return res .status(403) .set( 'WWW-Authenticate', - `Bearer scope="${requiredScopes.join(' ')}", resource_metadata="${resourceMetadataUrl()}", error="insufficient_scope"` + `Bearer scope="${stepUpScope}", resource_metadata="${resourceMetadataUrl()}", error="insufficient_scope"` ) .json({ error: 'insufficient_scope', @@ -418,7 +443,12 @@ export class ScopeStepUpAuthScenario implements Scenario { { prmPath: '/.well-known/oauth-protected-resource/mcp', requiredScopes: escalatedScopes, - scopesSupported: [initialScope], + // Deliberately disjoint from initialScope/stepUpScope so the SEP-2350 + // union check can't be satisfied by a client that unions + // scopes_supported ∪ challenge instead of prior-grant ∪ challenge. + // The spec allows this: clients MUST NOT assume any set relationship + // between challenged scopes and scopes_supported. + scopesSupported: ['mcp:profile'], includeScopeInWwwAuth: true, authMiddleware: stepUpMiddleware, tokenVerifier @@ -465,6 +495,15 @@ export class ScopeStepUpAuthScenario implements Scenario { timestamp: new Date().toISOString(), specReferences: [SpecReferences.MCP_SCOPE_SELECTION_STRATEGY] }); + this.checks.push({ + id: 'sep-2350-scope-union-on-reauth', + name: 'Client accumulates previously-granted scopes on re-authorization', + description: + 'Client did not make a second authorization request - scope union check could not be performed', + status: 'FAILURE', + timestamp: new Date().toISOString(), + specReferences: [SpecReferences.MCP_SCOPE_CHALLENGE_HANDLING] + }); } return this.checks; diff --git a/src/seps/sep-2350.yaml b/src/seps/sep-2350.yaml new file mode 100644 index 0000000..486c768 --- /dev/null +++ b/src/seps/sep-2350.yaml @@ -0,0 +1,11 @@ +sep: 2350 +spec_url: https://modelcontextprotocol.io/specification/draft/basic/authorization#runtime-insufficient-scope-errors +requirements: + - check: sep-2350-scope-union-on-reauth + text: 'When re-authorizing, clients SHOULD include these scopes alongside any previously granted scopes to avoid losing permissions needed for other operations. / Clients SHOULD compute the union of previously requested scopes and newly challenged scopes when initiating re-authorization.' + url: https://modelcontextprotocol.io/specification/draft/basic/authorization#protected-resource-metadata-discovery-requirements + - check: sep-2350-server-single-challenge + text: 'Regardless of the approach chosen, servers SHOULD include all scopes required for the current operation in a single challenge.' + + - text: 'When responding with insufficient scope errors, servers SHOULD include the scopes needed to satisfy the current operation in the scope parameter, consistent with RFC 6750 Section 3.1.' + excluded: 'reword of pre-existing requirement (request->operation, +RFC6750 cite); no normative delta; harness already emits scope= in WWW-Authenticate'