From 9c94c9e16781091df3da1d22e165bc0cf8430f3f Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 27 Oct 2026 14:03:16 +0100 Subject: [PATCH 1/2] [v1.x] fix(client): start a new authorization when the step-up needs a scope that the token lacks A refresh cannot add scopes, so the retry of a 403 insufficient_scope got the same 403 when the client had a refresh token. The new authorization also asks for the scopes requested and granted before. Same change as #2356 on main. Fixes #2255. --- .changeset/step-up-new-authorization.md | 6 ++ src/client/auth.ts | 39 +++++++++- src/client/streamableHttp.ts | 18 +++-- test/client/auth.test.ts | 96 ++++++++++++++++++++++++- test/client/redirects.test.ts | 9 ++- test/client/streamableHttp.test.ts | 51 ++++++++++++- 6 files changed, 210 insertions(+), 9 deletions(-) create mode 100644 .changeset/step-up-new-authorization.md diff --git a/.changeset/step-up-new-authorization.md b/.changeset/step-up-new-authorization.md new file mode 100644 index 0000000000..611986e1b7 --- /dev/null +++ b/.changeset/step-up-new-authorization.md @@ -0,0 +1,6 @@ +--- +'@modelcontextprotocol/sdk': patch +--- + +The step-up of a `403 insufficient_scope` now starts a new authorization when the token lacks a challenged scope, instead of refreshing the token. A refresh cannot add scopes (RFC 6749 §6), so the retry got the same 403 and failed with "Server returned 403 after trying +upscoping". The new authorization asks for the challenged scopes and for the scopes requested and granted before. `auth()` accepts a new `forceReauthorization` option. Fixes #2255. diff --git a/src/client/auth.ts b/src/client/auth.ts index dd3976f712..bba9195111 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -459,6 +459,12 @@ export async function auth( scope?: string; resourceMetadataUrl?: URL; fetchFn?: FetchLike; + /** + * Starts a new authorization even when a refresh token is available. A refresh + * cannot add scopes (RFC 6749 §6), so the step-up of a `403 insufficient_scope` + * sets this when the requested scope is not covered by the granted one. + */ + forceReauthorization?: boolean; } ): Promise { try { @@ -485,13 +491,15 @@ async function authInternal( authorizationCode, scope, resourceMetadataUrl, - fetchFn + fetchFn, + forceReauthorization }: { serverUrl: string | URL; authorizationCode?: string; scope?: string; resourceMetadataUrl?: URL; fetchFn?: FetchLike; + forceReauthorization?: boolean; } ): Promise { // Check if the provider has cached discovery state to skip discovery @@ -665,7 +673,7 @@ async function authInternal( } // Handle token refresh or new authorization - if (tokens?.refresh_token) { + if (tokens?.refresh_token && !forceReauthorization) { try { // Attempt to refresh the token const newTokens = await refreshAuthorization(authorizationServerUrl, { @@ -758,6 +766,33 @@ export async function selectResourceURL( return new URL(resourceMetadata.resource); } +/** + * Returns the union of space-delimited scope strings (RFC 6749 §3.3), each scope once, in first-seen order. + * + * The step-up of a `403 insufficient_scope` requests this union of the granted and the challenged scopes, + * so that the new token keeps the permissions granted before. + */ +export function computeScopeUnion(...scopes: ReadonlyArray): string | undefined { + const seen = new Set(); + for (const scope of scopes) { + for (const token of scope?.split(/\s+/) ?? []) { + if (token) seen.add(token); + } + } + return seen.size > 0 ? [...seen].join(' ') : undefined; +} + +/** + * Whether `union` contains a scope that `current` doesn't, both being space-delimited scope strings. + * + * An absent `current` is the empty set: an authorization server may omit the `scope` of a token + * when it equals the requested one (RFC 6749 §5.1), and refreshing would then risk dropping the new scope. + */ +export function isStrictScopeSuperset(union: string | undefined, current: string | undefined): boolean { + const currentSet = new Set((current ?? '').split(/\s+/).filter(Boolean)); + return (union ?? '').split(/\s+/).some(token => token !== '' && !currentSet.has(token)); +} + /** * Extract resource_metadata, scope, and error from WWW-Authenticate header. */ diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index 8512418b7d..55a88b53a5 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -10,7 +10,15 @@ import { unfollowedRedirect } from '../shared/transport.js'; import { isInitializedNotification, isJSONRPCRequest, isJSONRPCResultResponse, JSONRPCMessage, JSONRPCMessageSchema } from '../types.js'; -import { auth, AuthResult, extractWWWAuthenticateParams, OAuthClientProvider, UnauthorizedError } from './auth.js'; +import { + auth, + AuthResult, + computeScopeUnion, + extractWWWAuthenticateParams, + isStrictScopeSuperset, + OAuthClientProvider, + UnauthorizedError +} from './auth.js'; import { EventSourceParserStream } from 'eventsource-parser/stream'; // Default reconnection options for StreamableHTTP connections @@ -562,9 +570,9 @@ export class StreamableHTTPClientTransport implements Transport { throw new StreamableHTTPError(403, 'Server returned 403 after trying upscoping'); } - if (scope) { - this._scope = scope; - } + // Keep the scopes requested and granted before, so that the new token doesn't lose them. + const tokens = await this._authProvider.tokens(); + this._scope = computeScopeUnion(this._scope, tokens?.scope, scope); if (resourceMetadataUrl) { this._resourceMetadataUrl = resourceMetadataUrl; @@ -576,6 +584,8 @@ export class StreamableHTTPClientTransport implements Transport { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, scope: this._scope, + // a refresh cannot add the scopes that the token lacks (RFC 6749 §6) + forceReauthorization: isStrictScopeSuperset(this._scope, tokens?.scope), fetchFn: this._fetch }); diff --git a/test/client/auth.test.ts b/test/client/auth.test.ts index 45ec2806d0..75504e7cbe 100644 --- a/test/client/auth.test.ts +++ b/test/client/auth.test.ts @@ -15,7 +15,9 @@ import { type OAuthClientProvider, type OAuthDiscoveryState, selectClientAuthMethod, - isHttpsUrl + isHttpsUrl, + computeScopeUnion, + isStrictScopeSuperset } from '../../src/client/auth.js'; import { createPrivateKeyJwtAuth } from '../../src/client/auth-extensions.js'; import { InvalidClientMetadataError, ServerError } from '../../src/server/auth/errors.js'; @@ -118,6 +120,38 @@ describe('OAuth Authorization', () => { }); }); + describe('computeScopeUnion', () => { + it.each([ + { inputs: [undefined], expected: undefined }, + { inputs: [undefined, undefined], expected: undefined }, + { inputs: ['', ' '], expected: undefined }, + { inputs: ['read'], expected: 'read' }, + { inputs: ['read', undefined], expected: 'read' }, + { inputs: ['read write', 'write admin'], expected: 'read write admin' }, + { inputs: ['read', 'read'], expected: 'read' }, + { inputs: [' read write ', 'admin'], expected: 'read write admin' }, + { inputs: ['a b', 'c', 'b d'], expected: 'a b c d' } + ])('union of $inputs is $expected', ({ inputs, expected }) => { + expect(computeScopeUnion(...inputs)).toBe(expected); + }); + }); + + describe('isStrictScopeSuperset', () => { + it.each([ + { union: undefined, current: undefined, expected: false }, + { union: undefined, current: 'read', expected: false }, + { union: 'read', current: undefined, expected: true }, + { union: 'read', current: '', expected: true }, + { union: 'read', current: 'read', expected: false }, + { union: 'read write', current: 'read', expected: true }, + { union: 'read write', current: 'read write', expected: false }, + { union: 'read write', current: 'write read admin', expected: false }, + { union: 'read', current: 'read write', expected: false } + ])('isStrictScopeSuperset($union, $current) is $expected', ({ union, current, expected }) => { + expect(isStrictScopeSuperset(union, current)).toBe(expected); + }); + }); + describe('discoverOAuthProtectedResourceMetadata', () => { const validMetadata = { resource: 'https://resource.example.com', @@ -2773,6 +2807,66 @@ describe('OAuth Authorization', () => { expect(body.get('code')).toBe('auth-code-123'); }); + it('starts a new authorization instead of refreshing the token when forceReauthorization is set', async () => { + mockFetch.mockImplementation(url => { + const urlString = url.toString(); + + if (urlString.includes('/.well-known/oauth-protected-resource')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + resource: 'https://api.example.com/mcp-server', + authorization_servers: ['https://auth.example.com'] + }) + }); + } else if (urlString.includes('/.well-known/oauth-authorization-server')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'] + }) + }); + } else if (urlString.includes('/token')) { + // a refresh grant cannot add scopes (RFC 6749 section 6) + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ access_token: 'refreshed-access', token_type: 'Bearer', scope: 'read' }) + }); + } + + return Promise.resolve({ ok: false, status: 404 }); + }); + + (mockProvider.clientInformation as Mock).mockResolvedValue({ client_id: 'test-client' }); + (mockProvider.tokens as Mock).mockResolvedValue({ + access_token: 'old-access', + token_type: 'Bearer', + refresh_token: 'refresh123', + scope: 'read', + issuer: 'https://auth.example.com' + }); + + const result = await auth(mockProvider, { + serverUrl: 'https://api.example.com/mcp-server', + scope: 'read write', + forceReauthorization: true + }); + + expect(result).toBe('REDIRECT'); + expect(mockFetch.mock.calls.find(call => call[0].toString().includes('/token'))).toBeUndefined(); + + const authorizationUrl: URL = (mockProvider.redirectToAuthorization as Mock).mock.calls[0][0]; + expect(authorizationUrl.searchParams.get('scope')).toBe('read write'); + expect(authorizationUrl.searchParams.get('resource')).toBe('https://api.example.com/mcp-server'); + }); + it('excludes resource parameter in token refresh when Protected Resource Metadata is not present', async () => { // Mock metadata discovery - no protected resource metadata, but auth server metadata available mockFetch.mockImplementation(url => { diff --git a/test/client/redirects.test.ts b/test/client/redirects.test.ts index db4ada3158..95f120eb51 100644 --- a/test/client/redirects.test.ts +++ b/test/client/redirects.test.ts @@ -742,7 +742,14 @@ describe('redirects in the HTTP client transports', () => { const saved: unknown[] = []; const authProvider: OAuthClientProvider = { ...createProvider(saved, discoveredAt(endpoint)), - tokens: () => ({ access_token: 'access-0', token_type: 'Bearer', refresh_token: 'refresh-1', issuer: endpoint.url.origin }) + // the token has the challenged scope, so the step-up refreshes it + tokens: () => ({ + access_token: 'access-0', + token_type: 'Bearer', + refresh_token: 'refresh-1', + scope: 'tools', + issuer: endpoint.url.origin + }) }; transport = new StreamableHTTPClientTransport(new URL('/mcp', endpoint.url), { ...options, authProvider }); diff --git a/test/client/streamableHttp.test.ts b/test/client/streamableHttp.test.ts index 3c7a2ff964..a25fb5c4a6 100644 --- a/test/client/streamableHttp.test.ts +++ b/test/client/streamableHttp.test.ts @@ -661,11 +661,12 @@ describe('StreamableHTTPClientTransport', () => { // Verify fetch was called twice expect(fetchMock).toHaveBeenCalledTimes(2); - // Verify auth was called with the new scope + // Verify auth was called with the new scope, and without a token to refresh expect(authSpy).toHaveBeenCalledWith( mockAuthProvider, expect.objectContaining({ scope: 'new_scope', + forceReauthorization: true, resourceMetadataUrl: new URL('http://example.com/resource') }) ); @@ -673,6 +674,54 @@ describe('StreamableHTTPClientTransport', () => { authSpy.mockRestore(); }); + it.each([ + { granted: 'openid', challenged: 'tools:write', scope: 'openid tools:write', forceReauthorization: true }, + { granted: 'openid tools:write', challenged: 'tools:write', scope: 'openid tools:write', forceReauthorization: false } + ])( + 'upscopes from $granted to $challenged with scope "$scope" and forceReauthorization $forceReauthorization', + async ({ granted, challenged, scope, forceReauthorization }) => { + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + mockAuthProvider.tokens.mockResolvedValue({ + access_token: 'access-token', + token_type: 'Bearer', + refresh_token: 'refresh-token', + scope: granted + }); + + const fetchMock = global.fetch as Mock; + fetchMock + .mockResolvedValueOnce({ + ok: false, + status: 403, + statusText: 'Forbidden', + headers: new Headers({ 'WWW-Authenticate': `Bearer error="insufficient_scope", scope="${challenged}"` }), + text: () => Promise.resolve('Insufficient scope') + }) + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + // a refresh cannot add scopes (RFC 6749 section 6), and the scopes granted before are kept + expect(authSpy).toHaveBeenCalledWith(mockAuthProvider, expect.objectContaining({ scope, forceReauthorization })); + + authSpy.mockRestore(); + } + ); + it('prevents infinite upscoping on repeated 403', async () => { const message: JSONRPCMessage = { jsonrpc: '2.0', From 5240f3ba8f2465510a698d0fc7afe0507b08dc7d Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 27 Oct 2026 15:34:29 +0100 Subject: [PATCH 2/2] test(e2e): cover the step-up union and the refresh bypass, as on main The 403-scope-upgrade scenario now gives its refreshed token the challenged scope, so that it still covers the refresh and the loop guard. --- test/e2e/requirements.ts | 14 +++ test/e2e/scenarios/client-auth.test.ts | 114 ++++++++++++++++++++++++- 2 files changed, 126 insertions(+), 2 deletions(-) diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index 101215f40a..020d10b2c9 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -2066,6 +2066,20 @@ export const REQUIREMENTS: Record = { transports: ['streamableHttp'], note: 'This exercises the HTTP hosting/auth layer and OAuth client; the matrix transport arg is ignored, so it runs as a single streamableHttp-labelled cell to avoid duplicate runs.' }, + 'client-auth:stepup:scope-union': { + source: 'https://modelcontextprotocol.io/specification/draft/basic/authorization#step-up-authorization-flow', + behavior: + 'On 403 insufficient_scope the transport re-authorizes with the union of its previously-requested scope and the challenged scope (computeScopeUnion); the union is a plain string-set dedup with no hierarchical collapse.', + transports: ['streamableHttp'], + note: 'This exercises the HTTP hosting/auth layer and OAuth client; the matrix transport arg is ignored, so it runs as a single streamableHttp-labelled cell to avoid duplicate runs.' + }, + 'client-auth:stepup:refresh-bypass-on-superset': { + source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#step-up-authorization-flow', + behavior: + "On 403 insufficient_scope step-up: when the union scope is a strict superset of the current token's granted scope, auth() bypasses the refresh-token branch (forceReauthorization) and forces a fresh authorization request so the widened scope reaches the AS; when the token already covers the union, refresh is used.", + transports: ['streamableHttp'], + note: 'This exercises the HTTP hosting/auth layer and OAuth client; the matrix transport arg is ignored, so it runs as a single streamableHttp-labelled cell to avoid duplicate runs.' + }, 'client-auth:as-metadata-discovery:priority-order': { source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#authorization-server-metadata-discovery', behavior: diff --git a/test/e2e/scenarios/client-auth.test.ts b/test/e2e/scenarios/client-auth.test.ts index 0bf6378523..e60c02c5ec 100644 --- a/test/e2e/scenarios/client-auth.test.ts +++ b/test/e2e/scenarios/client-auth.test.ts @@ -16,9 +16,11 @@ import { z } from 'zod/v4'; import { Client } from '../../../src/client/index.js'; import { auth, + computeScopeUnion, discoverAuthorizationServerMetadata, discoverOAuthProtectedResourceMetadata, exchangeAuthorization, + isStrictScopeSuperset, OAuthClientProvider, refreshAuthorization, startAuthorization, @@ -409,11 +411,12 @@ verifies('client-auth:403-scope-upgrade', async (_args: TestArgs) => { } // Phase 2: when the upscoped token is still rejected with the same header, the transport stops instead of looping. + // The token here already covers the challenged scope, so refresh (not a fresh authorization) is used. const refreshAs = createMockAuthorizationServer({ - tokenResponses: [{ access_token: 'upscoped-access-token', token_type: 'Bearer' }] + tokenResponses: [{ access_token: 'upscoped-access-token', token_type: 'Bearer', scope: UPGRADED_SCOPE }] }); const refreshProvider = new RecordingOAuthClientProvider({ - tokens: { access_token: 'narrow-scope-token', token_type: 'Bearer', refresh_token: 'narrow-refresh-token' }, + tokens: { access_token: 'narrow-scope-token', token_type: 'Bearer', refresh_token: 'narrow-refresh-token', scope: UPGRADED_SCOPE }, clientInformation: { client_id: 'pre-registered-client' } }); @@ -446,6 +449,113 @@ verifies('client-auth:403-scope-upgrade', async (_args: TestArgs) => { } }); +verifies('client-auth:stepup:scope-union', async (_args: TestArgs) => { + // computeScopeUnion is a deliberate public export. + expect(computeScopeUnion('files:read openid', 'files:write')).toBe('files:read openid files:write'); + expect(computeScopeUnion('admin', 'read')).toBe('admin read'); // no hierarchical collapse + + // The transport requests the union of the token's scope and the newly-challenged scope on step-up. + const PREVIOUS_SCOPE = 'files:read openid'; + const CHALLENGED_SCOPE = 'files:write'; + const as = createMockAuthorizationServer(); + const provider = new RecordingOAuthClientProvider({ + tokens: { access_token: 'read-token', token_type: 'Bearer', scope: PREVIOUS_SCOPE }, + clientInformation: { client_id: 'pre-registered-client' } + }); + const combinedFetch = async (url: URL | string, init?: RequestInit): Promise => { + const urlObj = typeof url === 'string' ? new URL(url) : url; + if (urlObj.origin === ISSUER || urlObj.pathname.includes('/.well-known/')) { + return as.handleRequest(new Request(url, init)); + } + return new Response(null, { + status: 403, + headers: { 'WWW-Authenticate': `Bearer error="insufficient_scope", scope="${CHALLENGED_SCOPE}"` } + }); + }; + + const client = new Client({ name: 'c', version: '0' }); + const transport = new StreamableHTTPClientTransport(new URL(MCP_URL), { authProvider: provider, fetch: combinedFetch }); + try { + await expect(client.connect(transport)).rejects.toThrow(UnauthorizedError); + expect(provider.redirectedTo).toHaveLength(1); + expect(provider.redirectedTo[0].searchParams.get('scope')).toBe('files:read openid files:write'); + } finally { + await client.close(); + } +}); + +verifies('client-auth:stepup:refresh-bypass-on-superset', async (_args: TestArgs) => { + // Part A: the token was granted "files:read" and the challenge adds "files:write". The union strictly + // exceeds the token's grant, so auth() starts a fresh authorization request instead of refreshing. + { + const as = createMockAuthorizationServer(); + const provider = new RecordingOAuthClientProvider({ + tokens: { access_token: 't', token_type: 'Bearer', refresh_token: 'rt', scope: 'files:read' }, + clientInformation: { client_id: 'pre-registered-client' } + }); + const combinedFetch = async (url: URL | string, init?: RequestInit): Promise => { + const urlObj = typeof url === 'string' ? new URL(url) : url; + if (urlObj.origin === ISSUER || urlObj.pathname.includes('/.well-known/')) { + return as.handleRequest(new Request(url, init)); + } + return new Response(null, { + status: 403, + headers: { 'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="files:write"' } + }); + }; + const client = new Client({ name: 'c', version: '0' }); + const transport = new StreamableHTTPClientTransport(new URL(MCP_URL), { authProvider: provider, fetch: combinedFetch }); + try { + await expect(client.connect(transport)).rejects.toThrow(UnauthorizedError); + // Refresh was bypassed: no token-endpoint POST; the fresh authorize request carries the union scope. + expect(as.tokenCalls).toHaveLength(0); + expect(provider.redirectedTo).toHaveLength(1); + expect(provider.redirectedTo[0].searchParams.get('scope')).toBe('files:read files:write'); + expect(isStrictScopeSuperset('files:read files:write', 'files:read')).toBe(true); + } finally { + await client.close(); + } + } + + // Part B: the token already covers the challenged scope, so the union is not a strict superset and + // refresh is used. The server keeps answering 403, and the repeated challenge stops the loop. + { + const as = createMockAuthorizationServer({ + tokenResponses: [{ access_token: 't2', token_type: 'Bearer', scope: 'files:read files:write' }] + }); + const provider = new RecordingOAuthClientProvider({ + tokens: { access_token: 't', token_type: 'Bearer', refresh_token: 'rt', scope: 'files:read files:write' }, + clientInformation: { client_id: 'pre-registered-client' } + }); + const mcpPosts: string[] = []; + const combinedFetch = async (url: URL | string, init?: RequestInit): Promise => { + const urlObj = typeof url === 'string' ? new URL(url) : url; + if (urlObj.origin === ISSUER || urlObj.pathname.includes('/.well-known/')) { + return as.handleRequest(new Request(url, init)); + } + mcpPosts.push(urlObj.pathname); + return new Response(null, { + status: 403, + headers: { 'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="files:write"' } + }); + }; + const client = new Client({ name: 'c', version: '0' }); + const transport = new StreamableHTTPClientTransport(new URL(MCP_URL), { authProvider: provider, fetch: combinedFetch }); + try { + const connectPromise = client.connect(transport); + await expect(connectPromise).rejects.toBeInstanceOf(StreamableHTTPError); + await expect(connectPromise).rejects.toThrow(/403 after trying upscoping/); + expect(as.tokenCalls).toHaveLength(1); + expect(as.tokenCalls[0].body.get('grant_type')).toBe('refresh_token'); + expect(provider.redirectedTo).toHaveLength(0); + expect(mcpPosts).toHaveLength(2); + expect(isStrictScopeSuperset('files:read files:write', 'files:read files:write')).toBe(false); + } finally { + await client.close(); + } + } +}); + verifies('client-auth:as-metadata-discovery:priority-order', async (_args: TestArgs) => { const oauthMetadata: AuthorizationServerMetadata = { issuer: ISSUER,