Skip to content
Merged
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
49 changes: 41 additions & 8 deletions packages/client/src/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,38 @@ export async function auth(
}
}

/**
* Selects scopes per the MCP spec and augment for refresh token support.
*/
export function determineScope(options: {
requestedScope?: string;
resourceMetadata?: OAuthProtectedResourceMetadata;
authServerMetadata?: AuthorizationServerMetadata;
clientMetadata: OAuthClientMetadata;
}): string | undefined {
const { requestedScope, resourceMetadata, authServerMetadata, clientMetadata } = options;

// Scope selection priority (MCP spec):
// 1. WWW-Authenticate header scope
// 2. PRM scopes_supported
// 3. clientMetadata.scope (SDK fallback)
// 4. Omit scope parameter
let effectiveScope = requestedScope || resourceMetadata?.scopes_supported?.join(' ') || clientMetadata.scope;

// SEP-2207: Append offline_access when the AS advertises it
// and the client supports the refresh_token grant.
if (
effectiveScope &&
authServerMetadata?.scopes_supported?.includes('offline_access') &&
!effectiveScope.split(' ').includes('offline_access') &&
clientMetadata.grant_types?.includes('refresh_token')
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.

note: for a CIMD client, they might not have filled out clientMetadata locally, not sure there's a great way to solve that unless we allow expressing "I want a refresh token" explicitly at the provider layer and let that flow into client metadata.

) {
effectiveScope = `${effectiveScope} offline_access`;
}

return effectiveScope;
}

async function authInternal(
provider: OAuthClientProvider,
{
Expand Down Expand Up @@ -659,12 +691,13 @@ async function authInternal(
await provider.saveResourceUrl?.(String(resource));
}

// Apply scope selection strategy (SEP-835):
// 1. WWW-Authenticate scope (passed via `scope` param)
// 2. PRM scopes_supported
// 3. Client metadata scope (user-configured fallback)
// The resolved scope is used consistently for both DCR and the authorization request.
const resolvedScope = scope || resourceMetadata?.scopes_supported?.join(' ') || provider.clientMetadata.scope;
// Scope selection used consistently for DCR and the authorization request.
const resolvedScope = determineScope({
requestedScope: scope,
resourceMetadata,
authServerMetadata: metadata,
clientMetadata: provider.clientMetadata
});

// Handle client registration if needed
let clientInformation = await Promise.resolve(provider.clientInformation());
Comment on lines +694 to 703
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: fetchToken() (line 650) receives the raw scope parameter instead of resolvedScope, so non-interactive flows using prepareTokenRequest() miss the PRM scopes_supported fallback and the offline_access augmentation from determineScope(). In practice the impact is minimal — authorization_code flows don't send scope in the token request at all, and client_credentials + refresh tokens is not a valid combination per RFC 6749 §4.4.3 — but passing resolvedScope here would be more consistent with the stated goal of centralizing scope selection.

Extended reasoning...

What the bug is

In authInternal(), determineScope() computes resolvedScope (line 590-596) with a three-level fallback (WWW-Authenticate scope → PRM scopes_supportedclientMetadata.scope) plus SEP-2207 offline_access augmentation. This resolvedScope is correctly passed to registerClient() (line 631) and startAuthorization() (line 688). However, at line 650, fetchToken() receives the raw scope parameter — the original value from auth() options — not resolvedScope.

How fetchToken handles scope differently

Inside fetchToken() (line 1503), scope resolution follows a completely different chain: const effectiveScope = scope ?? provider.clientMetadata.scope. This bypasses both the PRM scopes_supported fallback (priority 2 in the MCP spec) and the offline_access augmentation added by this PR. The effectiveScope is then passed to provider.prepareTokenRequest(effectiveScope) for non-interactive flows.

Step-by-step proof for non-interactive flow

Consider a client_credentials provider where: (1) no explicit scope is passed to auth(), (2) PRM metadata has scopes_supported: ["mcp:read", "mcp:write"], (3) AS metadata has scopes_supported including offline_access, and (4) clientMetadata.grant_types includes refresh_token but clientMetadata.scope is undefined.

  • determineScope() returns "mcp:read mcp:write offline_access" (PRM fallback + augmentation)
  • DCR registration uses this resolved scope ✓
  • fetchToken() receives scope = undefined, computes effectiveScope = undefined ?? undefined = undefined
  • prepareTokenRequest(undefined) gets no scope at all, vs the intended "mcp:read mcp:write offline_access"

Why the practical impact is minimal

For authorization_code flows (the primary target of this PR), prepareAuthorizationCodeRequest() only sends grant_type, code, code_verifier, and redirect_uri — scope is not included in the token exchange request because it was already baked into the authorization URL via startAuthorization(resolvedScope). So this bug has zero impact on auth code flows.

For client_credentials flows, RFC 6749 §4.4.3 states the AS "MUST NOT issue a refresh token," making offline_access semantically meaningless. The PRM fallback gap is more relevant here, but providers implementing prepareTokenRequest() have full control over what scope they send.

Suggested fix

Pass resolvedScope instead of scope at line 650 for consistency with the PR's goal of centralizing scope selection via determineScope(). This is a one-line change: scoperesolvedScope in the fetchToken() call.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@felixweinberger happy to update this if you want. Was trying to be surgical, but the review is probably accurate. lmk

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will go ahead and fix this one

Expand Down Expand Up @@ -718,7 +751,7 @@ async function authInternal(
metadata,
resource,
authorizationCode,
scope,
scope: resolvedScope,
fetchFn
});

Expand Down Expand Up @@ -1360,7 +1393,7 @@ export async function startAuthorization(
authorizationUrl.searchParams.set('scope', scope);
}

if (scope?.includes('offline_access')) {
if (scope?.split(' ').includes('offline_access')) {
// if the request includes the OIDC-only "offline_access" scope,
// we need to set the prompt to "consent" to ensure the user is prompted to grant offline access
// https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
Expand Down
224 changes: 224 additions & 0 deletions packages/client/test/client/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { OAuthClientProvider } from '../../src/client/auth.js';
import {
auth,
buildDiscoveryUrls,
determineScope,
discoverAuthorizationServerMetadata,
discoverOAuthMetadata,
discoverOAuthProtectedResourceMetadata,
Expand Down Expand Up @@ -3734,4 +3735,227 @@ describe('OAuth Authorization', () => {
});
});
});

describe('determineScope', () => {
const baseClientMetadata = {
redirect_uris: ['http://localhost:3000/callback'],
client_name: 'Test Client'
};

describe('MCP Scope Selection Strategy', () => {
it('returns explicit requestedScope as-is (priority 1)', () => {
const result = determineScope({
requestedScope: 'files:read',
resourceMetadata: {
resource: 'https://api.example.com/',
scopes_supported: ['mcp:read', 'mcp:write']
},
clientMetadata: {
...baseClientMetadata,
scope: 'fallback:scope'
}
});

expect(result).toBe('files:read');
});

it('uses PRM scopes_supported when no explicit scope (priority 2)', () => {
const result = determineScope({
resourceMetadata: {
resource: 'https://api.example.com/',
scopes_supported: ['mcp:read', 'mcp:write', 'mcp:admin']
},
clientMetadata: {
...baseClientMetadata,
scope: 'fallback:scope'
}
});

expect(result).toBe('mcp:read mcp:write mcp:admin');
});

it('falls back to clientMetadata.scope when no PRM scopes (priority 3)', () => {
const result = determineScope({
resourceMetadata: {
resource: 'https://api.example.com/'
},
clientMetadata: {
...baseClientMetadata,
scope: 'client:default'
}
});

expect(result).toBe('client:default');
});

it('returns undefined when no scope source available (priority 4)', () => {
const result = determineScope({
clientMetadata: baseClientMetadata
});

expect(result).toBeUndefined();
});

it('returns undefined when PRM has no scopes_supported and clientMetadata has no scope', () => {
const result = determineScope({
resourceMetadata: {
resource: 'https://api.example.com/'
},
clientMetadata: baseClientMetadata
});

expect(result).toBeUndefined();
});
});

describe('SEP-2207: offline_access scope augmentation', () => {
const asMetadataWithOfflineAccess = {
issuer: 'https://auth.example.com',
authorization_endpoint: 'https://auth.example.com/authorize',
token_endpoint: 'https://auth.example.com/token',
response_types_supported: ['code'] as string[],
scopes_supported: ['openid', 'profile', 'offline_access']
};

const asMetadataWithoutOfflineAccess = {
issuer: 'https://auth.example.com',
authorization_endpoint: 'https://auth.example.com/authorize',
token_endpoint: 'https://auth.example.com/token',
response_types_supported: ['code'] as string[],
scopes_supported: ['openid', 'profile']
};

const clientMetadataWithRefreshToken = {
...baseClientMetadata,
grant_types: ['authorization_code', 'refresh_token']
};

it('augments explicit scope with offline_access', () => {
const result = determineScope({
requestedScope: 'mcp:read mcp:write',
resourceMetadata: {
resource: 'https://api.example.com/',
scopes_supported: ['mcp:read', 'mcp:write']
},
authServerMetadata: asMetadataWithOfflineAccess,
clientMetadata: clientMetadataWithRefreshToken
});

expect(result).toBe('mcp:read mcp:write offline_access');
});

it('adds offline_access when AS supports it and client grant_types includes refresh_token', () => {
const result = determineScope({
resourceMetadata: {
resource: 'https://api.example.com/',
scopes_supported: ['mcp:read', 'mcp:write']
},
authServerMetadata: asMetadataWithOfflineAccess,
clientMetadata: clientMetadataWithRefreshToken
});

expect(result).toBe('mcp:read mcp:write offline_access');
});

it('adds offline_access when using clientMetadata.scope fallback', () => {
const result = determineScope({
authServerMetadata: asMetadataWithOfflineAccess,
clientMetadata: {
...clientMetadataWithRefreshToken,
scope: 'mcp:tools'
}
});

expect(result).toBe('mcp:tools offline_access');
});

it('does NOT augment when no other scopes are present', () => {
const result = determineScope({
authServerMetadata: asMetadataWithOfflineAccess,
clientMetadata: clientMetadataWithRefreshToken
});

expect(result).toBeUndefined();
});

it('does NOT augment when AS metadata lacks offline_access', () => {
const result = determineScope({
resourceMetadata: {
resource: 'https://api.example.com/',
scopes_supported: ['mcp:read', 'mcp:write']
},
authServerMetadata: asMetadataWithoutOfflineAccess,
clientMetadata: clientMetadataWithRefreshToken
});

expect(result).toBe('mcp:read mcp:write');
});

it('does NOT augment when AS metadata is undefined', () => {
const result = determineScope({
resourceMetadata: {
resource: 'https://api.example.com/',
scopes_supported: ['mcp:read', 'mcp:write']
},
clientMetadata: clientMetadataWithRefreshToken
});

expect(result).toBe('mcp:read mcp:write');
});

it('does NOT augment when offline_access already in clientMetadata.scope', () => {
const result = determineScope({
authServerMetadata: asMetadataWithOfflineAccess,
clientMetadata: {
...clientMetadataWithRefreshToken,
scope: 'mcp:tools offline_access'
}
});

expect(result).toBe('mcp:tools offline_access');
});

it('does NOT augment when non-compliant PRM already includes offline_access', () => {
const result = determineScope({
resourceMetadata: {
resource: 'https://api.example.com/',
scopes_supported: ['mcp:read', 'offline_access', 'mcp:write']
},
authServerMetadata: asMetadataWithOfflineAccess,
clientMetadata: clientMetadataWithRefreshToken
});

expect(result).toBe('mcp:read offline_access mcp:write');
});

it('does NOT augment when grant_types omits refresh_token', () => {
const result = determineScope({
resourceMetadata: {
resource: 'https://api.example.com/',
scopes_supported: ['mcp:read', 'mcp:write']
},
authServerMetadata: asMetadataWithOfflineAccess,
clientMetadata: {
...baseClientMetadata,
grant_types: ['authorization_code']
}
});

expect(result).toBe('mcp:read mcp:write');
});

it('does NOT augment when grant_types is undefined (respects OAuth defaults)', () => {
const result = determineScope({
resourceMetadata: {
resource: 'https://api.example.com/',
scopes_supported: ['mcp:read', 'mcp:write']
},
authServerMetadata: asMetadataWithOfflineAccess,
clientMetadata: baseClientMetadata
});

expect(result).toBe('mcp:read mcp:write');
});
});
});
});
4 changes: 3 additions & 1 deletion test/conformance/src/everythingClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,9 @@ registerScenarios(
'auth/scope-retry-limit',
'auth/token-endpoint-auth-basic',
'auth/token-endpoint-auth-post',
'auth/token-endpoint-auth-none'
'auth/token-endpoint-auth-none',
'auth/offline-access-scope',
'auth/offline-access-not-supported'
],
runAuthClient
);
Expand Down
Loading