Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions packages/client/src/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,12 @@
// The resolved scope is used consistently for both DCR and the authorization request.
const resolvedScope = scope || resourceMetadata?.scopes_supported?.join(' ') || provider.clientMetadata.scope;

// For the token exchange, only forward scope that was explicitly requested via
// WWW-Authenticate (the `scope` parameter). Do not inject PRM scopes_supported or
// clientMetadata.scope — either can exceed what the server granted, triggering
// invalid_scope errors per RFC 6749 §4.1.3.
const tokenExchangeScope = scope;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this trying to achieve?

The scope we send to /token should be the scope we sent to /authorize, so we should always use the same scope calculation logic. We recently made a change in the refresh flow to trigger an auth flow if the scope exceeds the scope on the existing token which might be relevant here.

Scopes also accumulate over 403's so it would be necessary to increase the scope across multiple challenges as well.


// Handle client registration if needed
let clientInformation = await Promise.resolve(provider.clientInformation());
if (!clientInformation) {
Expand Down Expand Up @@ -562,6 +568,7 @@
metadata,
resource,
authorizationCode,
scope: tokenExchangeScope,
fetchFn
});

Expand Down Expand Up @@ -1198,14 +1205,21 @@
export function prepareAuthorizationCodeRequest(
authorizationCode: string,
codeVerifier: string,
redirectUri: string | URL
redirectUri: string | URL,
scope?: string
): URLSearchParams {

Check warning on line 1210 in packages/client/src/client/auth.ts

View check run for this annotation

Claude / Claude Code Review

exchangeAuthorization not updated to accept/forward scope

Nit: `exchangeAuthorization()` was not updated to accept or forward a `scope` parameter, even though this PR added scope support to `prepareAuthorizationCodeRequest()` which it calls (line 1324). This is a consistency gap — callers using `exchangeAuthorization()` directly cannot pass scope through to the token request. Not a regression since this function never had scope support before, but worth adding for API completeness.
Comment on lines +1208 to 1210
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit: exchangeAuthorization() was not updated to accept or forward a scope parameter, even though this PR added scope support to prepareAuthorizationCodeRequest() which it calls (line 1324). This is a consistency gap — callers using exchangeAuthorization() directly cannot pass scope through to the token request. Not a regression since this function never had scope support before, but worth adding for API completeness.

Extended reasoning...

Bug Analysis

exchangeAuthorization() is a public exported function (line 1302) that exchanges an authorization code for tokens. It calls prepareAuthorizationCodeRequest(authorizationCode, codeVerifier, redirectUri) at line 1324 — notably without a scope argument.

This PR added an optional scope parameter to prepareAuthorizationCodeRequest() (line 1208) and correctly threads scope through the primary flow: auth() computes tokenExchangeScope → passes it to fetchToken() → which passes it to prepareAuthorizationCodeRequest(). However, the mid-level exchangeAuthorization() function was not updated — it neither accepts scope in its options object (lines 1304-1322) nor passes it to prepareAuthorizationCodeRequest().

Concrete example

A caller using exchangeAuthorization() directly for an Azure AD integration:

const tokens = await exchangeAuthorization(serverUrl, {
  clientInformation,
  authorizationCode: code,
  codeVerifier: verifier,
  redirectUri: redirectUrl,
  // No way to pass scope here — the options type doesn't include it
});

The resulting token request will never include a scope parameter, even though prepareAuthorizationCodeRequest now supports it. For providers like Azure AD that require scope in the token exchange, this means users of exchangeAuthorization() cannot benefit from this PR's fix.

Impact assessment

This is not a regressionexchangeAuthorization works exactly as it did before this PR. The primary auth flow (auth()fetchToken()) is correctly updated and is the recommended path. Additionally, exchangeAuthorization is not called internally by the SDK's auth() function; it exists as a standalone public API for consumers who manage their own auth flow.

Suggested fix

Add an optional scope?: string to exchangeAuthorization()'s options and forward it:

const tokenRequestParams = prepareAuthorizationCodeRequest(
  authorizationCode, codeVerifier, redirectUri, scope
);

This would make the public API surface consistent — all three levels (low-level prepareAuthorizationCodeRequest, mid-level exchangeAuthorization, and high-level fetchToken/auth()) would support scope forwarding.

return new URLSearchParams({
const params = new URLSearchParams({
grant_type: 'authorization_code',
code: authorizationCode,
code_verifier: codeVerifier,
redirect_uri: String(redirectUri)
});

if (scope) {
params.set('scope', scope);
}

return params;
}

/**
Expand Down Expand Up @@ -1401,21 +1415,25 @@
metadata,
resource,
authorizationCode,
scope,
fetchFn
}: {
metadata?: AuthorizationServerMetadata;
resource?: URL;
/** Authorization code for the default `authorization_code` grant flow */
authorizationCode?: string;
scope?: string;
fetchFn?: FetchLike;
} = {}
): Promise<OAuthTokens> {
const scope = provider.clientMetadata.scope;

// Use provider's prepareTokenRequest if available, otherwise fall back to authorization_code
let tokenRequestParams: URLSearchParams | undefined;
if (provider.prepareTokenRequest) {
tokenRequestParams = await provider.prepareTokenRequest(scope);
// For non-interactive flows (client_credentials, jwt-bearer), include the client's
// configured scope as a default — the server has never narrowed it via an authorization
// grant, so clientMetadata.scope is the expected starting point.
const tokenRequestScope = scope ?? provider.clientMetadata.scope;
tokenRequestParams = await provider.prepareTokenRequest(tokenRequestScope);
}

// Default to authorization_code grant if no custom prepareTokenRequest
Expand All @@ -1427,7 +1445,10 @@
throw new Error('redirectUrl is required for authorization_code flow');
}
const codeVerifier = await provider.codeVerifier();
tokenRequestParams = prepareAuthorizationCodeRequest(authorizationCode, codeVerifier, provider.redirectUrl);
// Only include scope in the token request when explicitly provided — do not inject
// clientMetadata.scope because the provider may have narrowed the granted scope during
// authorization and re-sending a broader scope would cause an invalid_scope error.
tokenRequestParams = prepareAuthorizationCodeRequest(authorizationCode, codeVerifier, provider.redirectUrl, scope);
}

const clientInformation = await provider.clientInformation();
Expand Down
95 changes: 95 additions & 0 deletions packages/client/test/client/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import {
discoverOAuthServerInfo,
exchangeAuthorization,
extractWWWAuthenticateParams,
fetchToken,
isHttpsUrl,
prepareAuthorizationCodeRequest,
refreshAuthorization,
registerClient,
selectClientAuthMethod,
Expand Down Expand Up @@ -1519,6 +1521,7 @@ describe('OAuth Authorization', () => {
expect(options.headers.get('Authorization')).toBe('Basic ' + btoa('client123:secret123'));
expect(body.get('redirect_uri')).toBe('http://localhost:3000/callback');
expect(body.get('resource')).toBe('https://api.example.com/mcp-server');
expect(body.get('scope')).toBeNull();
});

it('allows for string "expires_in" values', async () => {
Expand Down Expand Up @@ -1845,6 +1848,98 @@ describe('OAuth Authorization', () => {
});
});

describe('prepareAuthorizationCodeRequest', () => {
it('includes scope when provided', () => {
const params = prepareAuthorizationCodeRequest('code123', 'verifier123', 'http://localhost:3000/callback', 'read write');

expect(params.get('grant_type')).toBe('authorization_code');
expect(params.get('code')).toBe('code123');
expect(params.get('code_verifier')).toBe('verifier123');
expect(params.get('redirect_uri')).toBe('http://localhost:3000/callback');
expect(params.get('scope')).toBe('read write');
});

it('omits scope when not provided', () => {
const params = prepareAuthorizationCodeRequest('code123', 'verifier123', 'http://localhost:3000/callback');

expect(params.get('scope')).toBeNull();
});
});

describe('fetchToken', () => {
const validTokens: OAuthTokens = {
access_token: 'access123',
token_type: 'Bearer',
expires_in: 3600
};

function createFetchTokenProvider(overrides: Partial<OAuthClientProvider> = {}): OAuthClientProvider {
return {
get redirectUrl() {
return 'http://localhost:3000/callback';
},
get clientMetadata() {
return {
redirect_uris: ['http://localhost:3000/callback'],
client_name: 'Test Client',
scope: 'client:default'
};
},
clientInformation: vi.fn().mockResolvedValue({
client_id: 'client123',
client_secret: 'secret123'
}),
tokens: vi.fn(),
saveTokens: vi.fn(),
redirectToAuthorization: vi.fn(),
saveCodeVerifier: vi.fn(),
codeVerifier: vi.fn().mockResolvedValue('verifier123'),
...overrides
};
}

it('includes explicitly passed scope in authorization code token requests', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
status: 200,
json: async () => validTokens
});

const provider = createFetchTokenProvider();

await fetchToken(provider, 'https://auth.example.com', {
authorizationCode: 'code123',
scope: 'read write'
});

const body = mockFetch.mock.calls[0]![1].body as URLSearchParams;
expect(body.get('grant_type')).toBe('authorization_code');
expect(body.get('code')).toBe('code123');
expect(body.get('code_verifier')).toBe('verifier123');
expect(body.get('redirect_uri')).toBe('http://localhost:3000/callback');
expect(body.get('scope')).toBe('read write');
});

it('does not inject scope from clientMetadata when no explicit scope is passed', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
status: 200,
json: async () => validTokens
});

const provider = createFetchTokenProvider();

await fetchToken(provider, 'https://auth.example.com', {
authorizationCode: 'code123'
});

const body = mockFetch.mock.calls[0]![1].body as URLSearchParams;
// Do NOT inject clientMetadata.scope into the token request — providers that narrowed
// scope during authorization would reject a broader scope with invalid_scope.
expect(body.get('scope')).toBeNull();
});
});

describe('registerClient', () => {
const validClientMetadata = {
redirect_uris: ['http://localhost:3000/callback'],
Expand Down
Loading