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
6 changes: 6 additions & 0 deletions .changeset/oauth-prm-resource-fragment-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@modelcontextprotocol/core': patch
'@modelcontextprotocol/client': patch
---

Tighten OAuth Protected Resource Metadata `resource` validation per RFC 8707 §2: identifiers containing a fragment component are now rejected by `OAuthProtectedResourceMetadataSchema`. The error message thrown by `selectResourceURL` on origin/path mismatch now points at `OAuthClientProvider.validateResourceURL` as the supported override for non-URL RFC 8707 indicators (e.g. `urn:`, `api://`) or identifiers served from a different origin. The `validateResourceURL` JSDoc has been clarified to document this override use case and reference RFC 9728 §3.3 / §7.3 for the strict-default rationale.
21 changes: 18 additions & 3 deletions packages/client/src/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,16 @@ export interface OAuthClientProvider {
* RFC 8707 Resource Indicator. If left undefined, default
* validation behavior will be used.
*
* Implementations must verify the returned resource matches the MCP server.
* The default validation accepts URL `resource` values with no fragment that share
* origin with, and are equal to or a parent path of, the MCP server URL. This is
* stricter than RFC 8707 (which permits any absolute URI) and looser than RFC 9728
* §3.3 (which requires identity with the resource identifier used for discovery).
* Implement this hook to opt in to accepting:
* - non-URL absolute URI resource indicators per RFC 8707 §2 (e.g. `urn:...`, `api://...`)
* - identifiers served from a different origin than the MCP endpoint
*
* Implementations remain responsible for any audience-binding policy required by the
* application. See RFC 9728 §3.3 / §7.3 for the security rationale behind the strict default.
*/
validateResourceURL?(serverUrl: string | URL, resource?: string): Promise<URL | undefined>;

Expand Down Expand Up @@ -857,9 +866,15 @@ export async function selectResourceURL(
return undefined;
}

// Validate that the metadata's resource is compatible with our request
// Validate that the metadata's resource is compatible with our request.
// Per RFC 9728 §3.3 / §7.3, refusing mismatched PRM resources prevents an attacker-controlled
// endpoint from advertising metadata that impersonates another resource.
if (!checkResourceAllowed({ requestedResource: defaultResource, configuredResource: resourceMetadata.resource })) {
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin)`);
throw new Error(
`Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin). ` +
`Override OAuthClientProvider.validateResourceURL to opt in to accepting non-URL RFC 8707 resource indicators ` +
`(e.g. 'urn:' or 'api://') or identifiers served from a different origin.`
);
}
// Prefer the resource from metadata since it's what the server is telling us to request
return new URL(resourceMetadata.resource);
Expand Down
76 changes: 76 additions & 0 deletions packages/client/test/client/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,34 @@ describe('OAuth Authorization', () => {
await expect(discoverOAuthProtectedResourceMetadata('https://resource.example.com')).rejects.toThrow();
});

it('rejects metadata whose resource identifier includes a fragment (RFC 8707 §2)', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
status: 200,
json: async () => ({
resource: 'https://resource.example.com/mcp#section'
})
});

await expect(discoverOAuthProtectedResourceMetadata('https://resource.example.com')).rejects.toThrow(
"Protected Resource Metadata 'resource' MUST NOT include a fragment component (RFC 8707 §2)"
);
});

it('rejects metadata whose resource identifier ends with an empty fragment', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
status: 200,
json: async () => ({
resource: 'https://resource.example.com/mcp#'
})
});

await expect(discoverOAuthProtectedResourceMetadata('https://resource.example.com')).rejects.toThrow(
"Protected Resource Metadata 'resource' MUST NOT include a fragment component (RFC 8707 §2)"
);
});

it('returns metadata when discovery succeeds with path', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
Expand Down Expand Up @@ -2652,6 +2680,54 @@ describe('OAuth Authorization', () => {
);
});

it('throws an actionable error pointing at validateResourceURL when PRM resource origin mismatches', async () => {
// Same setup as the override test above, but with NO override on the provider —
// this exercises the default `selectResourceURL` rejection path and asserts the
// error guides integrators to the documented escape hatch.
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://different-resource.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']
})
});
}

return Promise.resolve({ ok: false, status: 404 });
});

(mockProvider.clientInformation as Mock).mockResolvedValue({
client_id: 'test-client',
client_secret: 'test-secret'
});
(mockProvider.tokens as Mock).mockResolvedValue(undefined);
(mockProvider.saveCodeVerifier as Mock).mockResolvedValue(undefined);
(mockProvider.redirectToAuthorization as Mock).mockResolvedValue(undefined);

await expect(
auth(mockProvider, {
serverUrl: 'https://api.example.com/mcp-server'
})
).rejects.toThrow(/validateResourceURL/);
});

it('uses prefix of server URL from PRM resource as resource parameter', async () => {
// Mock successful metadata discovery with resource URL that is a prefix of requested URL
mockFetch.mockImplementation(url => {
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/shared/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ export const SafeUrlSchema = z
* RFC 9728 OAuth Protected Resource Metadata
*/
export const OAuthProtectedResourceMetadataSchema = z.looseObject({
resource: z.string().url(),
resource: z
.string()
.url()
.refine(value => !value.includes('#'), {
message: "Protected Resource Metadata 'resource' MUST NOT include a fragment component (RFC 8707 §2)"
}),
authorization_servers: z.array(SafeUrlSchema).optional(),
jwks_uri: z.string().url().optional(),
scopes_supported: z.array(z.string()).optional(),
Expand Down
41 changes: 41 additions & 0 deletions packages/core/test/shared/auth.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
OAuthClientMetadataSchema,
OAuthMetadataSchema,
OAuthProtectedResourceMetadataSchema,
OpenIdProviderMetadataSchema,
OptionalSafeUrlSchema,
SafeUrlSchema
Expand Down Expand Up @@ -70,6 +71,46 @@ describe('OAuthMetadataSchema', () => {
});
});

describe('OAuthProtectedResourceMetadataSchema', () => {
it('accepts a resource identifier without a fragment', () => {
const metadata = {
resource: 'https://api.example.com/mcp'
};

expect(() => OAuthProtectedResourceMetadataSchema.parse(metadata)).not.toThrow();
});

it('rejects a resource identifier with a fragment', () => {
const metadata = {
resource: 'https://api.example.com/mcp#frag'
};

expect(() => OAuthProtectedResourceMetadataSchema.parse(metadata)).toThrow(
"Protected Resource Metadata 'resource' MUST NOT include a fragment component (RFC 8707 §2)"
);
});

it('rejects a resource identifier with an empty fragment', () => {
// RFC 3986 §3.5: presence of "#" delimits a fragment, even when empty.
const metadata = {
resource: 'https://api.example.com/mcp#'
};

expect(() => OAuthProtectedResourceMetadataSchema.parse(metadata)).toThrow(
"Protected Resource Metadata 'resource' MUST NOT include a fragment component (RFC 8707 §2)"
);
});

it('accepts percent-encoded "#" inside the path or query', () => {
// %23 is not a fragment delimiter; only a literal "#" is.
const metadata = {
resource: 'https://api.example.com/mcp%23section'
};

expect(() => OAuthProtectedResourceMetadataSchema.parse(metadata)).not.toThrow();
});
});

describe('OpenIdProviderMetadataSchema', () => {
it('validates complete OpenID Provider metadata', () => {
const metadata = {
Expand Down
Loading