Skip to content

Important fix for resource validation in MCP #259089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Tyler James Leonhardt (TylerLeonhardt) merged 1 commit into from
Jul 31, 2025
+7 −141
Merged

Important fix for resource validation in MCP #259089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 0 additions & 32 deletions src/vs/base/common/oauth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -819,35 +819,3 @@ export function getClaimsFromJWT(token: string): IAuthorizationJWTClaims {
throw new Error('Failed to parse JWT token');
}
}

/**
* Extracts the resource server base URL from an OAuth protected resource metadata discovery endpoint URL.
*
* @param discoveryUrl The full URL to the OAuth protected resource metadata discovery endpoint
* @returns The base URL of the resource server
*
* @example
* ```typescript
* getResourceServerBaseUrlFromDiscoveryUrl('https://mcp.example.com/.well-known/oauth-protected-resource')
* // Returns: 'https://mcp.example.com/'
*
* getResourceServerBaseUrlFromDiscoveryUrl('https://mcp.example.com/.well-known/oauth-protected-resource/mcp')
* // Returns: 'https://mcp.example.com/mcp'
* ```
*/
export function getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl: string): string {
const url = new URL(discoveryUrl);

// Remove the well-known discovery path only if it appears at the beginning
if (!url.pathname.startsWith(AUTH_PROTECTED_RESOURCE_METADATA_DISCOVERY_PATH)) {
throw new Error(`Invalid discovery URL: expected path to start with ${AUTH_PROTECTED_RESOURCE_METADATA_DISCOVERY_PATH}`);
}

const pathWithoutDiscovery = url.pathname.substring(AUTH_PROTECTED_RESOURCE_METADATA_DISCOVERY_PATH.length);

// Construct the base URL
const baseUrl = new URL(url.origin);
baseUrl.pathname = pathWithoutDiscovery || '/';

return baseUrl.toString();
}
101 changes: 0 additions & 101 deletions src/vs/base/test/common/oauth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import * as sinon from 'sinon';
import {
getClaimsFromJWT,
getDefaultMetadataForUrl,
getResourceServerBaseUrlFromDiscoveryUrl,
isAuthorizationAuthorizeResponse,
isAuthorizationDeviceResponse,
isAuthorizationErrorResponse,
Expand Down Expand Up @@ -625,106 +624,6 @@ suite('OAuth', () => {
});
});

suite('getResourceServerBaseUrlFromDiscoveryUrl', () => {
test('should extract base URL from discovery URL at root', () => {
const discoveryUrl = 'https://mcp.example.com/.well-known/oauth-protected-resource';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://mcp.example.com/');
});

test('should extract base URL from discovery URL with subpath', () => {
const discoveryUrl = 'https://mcp.example.com/.well-known/oauth-protected-resource/mcp';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://mcp.example.com/mcp');
});

test('should extract base URL from discovery URL with nested subpath', () => {
const discoveryUrl = 'https://api.example.com/.well-known/oauth-protected-resource/v1/services/mcp';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://api.example.com/v1/services/mcp');
});

test('should handle discovery URL with port number', () => {
const discoveryUrl = 'https://localhost:8443/.well-known/oauth-protected-resource/api';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://localhost:8443/api');
});

test('should handle discovery URL with query parameters', () => {
const discoveryUrl = 'https://example.com/.well-known/oauth-protected-resource/api?version=1';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://example.com/api');
});

test('should handle discovery URL with fragment', () => {
const discoveryUrl = 'https://example.com/.well-known/oauth-protected-resource/api#section';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://example.com/api');
});

test('should handle discovery URL ending with trailing slash', () => {
const discoveryUrl = 'https://example.com/.well-known/oauth-protected-resource/api/';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://example.com/api/');
});

test('should handle HTTP URLs', () => {
const discoveryUrl = 'http://localhost:3000/.well-known/oauth-protected-resource/dev';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'http://localhost:3000/dev');
});

test('should throw error for URL without discovery path', () => {
const discoveryUrl = 'https://example.com/some/other/path';
assert.throws(
() => getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl),
/Invalid discovery URL: expected path to start with \/\.well-known\/oauth-protected-resource/
);
});

test('should throw error for URL with partial discovery path', () => {
const discoveryUrl = 'https://example.com/.well-known/oauth';
assert.throws(
() => getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl),
/Invalid discovery URL: expected path to start with \/\.well-known\/oauth-protected-resource/
);
});

test('should throw error for URL with discovery path not at beginning', () => {
const discoveryUrl = 'https://example.com/api/.well-known/oauth-protected-resource';
assert.throws(
() => getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl),
/Invalid discovery URL: expected path to start with \/\.well-known\/oauth-protected-resource/
);
});

test('should throw error for invalid URL format', () => {
const discoveryUrl = 'not-a-valid-url';
assert.throws(
() => getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl),
TypeError
);
});

test('should handle empty path after discovery path', () => {
const discoveryUrl = 'https://example.com/.well-known/oauth-protected-resource';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://example.com/');
});

test('should preserve URL encoding in subpath', () => {
const discoveryUrl = 'https://example.com/.well-known/oauth-protected-resource/api%20v1';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://example.com/api%20v1');
});

test('should normalize hostname case consistently', () => {
const discoveryUrl = 'https://MCP.EXAMPLE.COM/.well-known/oauth-protected-resource';
const result = getResourceServerBaseUrlFromDiscoveryUrl(discoveryUrl);
assert.strictEqual(result, 'https://mcp.example.com/');
});
});

suite('Client ID Fallback Scenarios', () => {
let sandbox: sinon.SinonSandbox;
let fetchStub: sinon.SinonStub;
Expand Down
15 changes: 7 additions & 8 deletions src/vs/workbench/api/common/extHostMcp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { extensionPrefixedIdentifier, McpCollectionDefinition, McpConnectionStat
import { ExtHostMcpShape, MainContext, MainThreadMcpShape } from './extHost.protocol.js';
import { IExtHostRpcService } from './extHostRpcService.js';
import * as Convert from './extHostTypeConverters.js';
import { AUTH_SERVER_METADATA_DISCOVERY_PATH, OPENID_CONNECT_DISCOVERY_PATH, getDefaultMetadataForUrl, getResourceServerBaseUrlFromDiscoveryUrl, IAuthorizationProtectedResourceMetadata, IAuthorizationServerMetadata, isAuthorizationProtectedResourceMetadata, isAuthorizationServerMetadata, parseWWWAuthenticateHeader } from '../../../base/common/oauth.js';
import { AUTH_SERVER_METADATA_DISCOVERY_PATH, OPENID_CONNECT_DISCOVERY_PATH, getDefaultMetadataForUrl, IAuthorizationProtectedResourceMetadata, IAuthorizationServerMetadata, isAuthorizationProtectedResourceMetadata, isAuthorizationServerMetadata, parseWWWAuthenticateHeader } from '../../../base/common/oauth.js';
import { URI } from '../../../base/common/uri.js';
import { MCP } from '../../contrib/mcp/common/modelContextProtocol.js';
import { CancellationError } from '../../../base/common/errors.js';
Expand Down Expand Up @@ -314,7 +314,7 @@ class McpHTTPHandle extends Disposable {
}
}

private async _populateAuthMetadata(originalResponse: Response): Promise<void> {
private async _populateAuthMetadata(url: string, originalResponse: Response): Promise<void> {
// If there is a resource_metadata challenge, use that to get the oauth server. This is done in 2 steps.
// First, extract the resource_metada challenge from the WWW-Authenticate header (if available)
let resourceMetadataChallenge: string | undefined;
Expand All @@ -331,6 +331,10 @@ class McpHTTPHandle extends Disposable {
let resource: IAuthorizationProtectedResourceMetadata | undefined;
if (resourceMetadataChallenge) {
const resourceMetadata = await this._getResourceMetadata(resourceMetadataChallenge);
// Use URL constructor for normalization - it handles hostname case and trailing slashes
if (new URL(resourceMetadata.resource).toString() !== new URL(url).toString()) {
throw new Error(`Protected Resource Metadata resource "${resourceMetadata.resource}" does not match MCP server resolved resource "${url}". The MCP server must follow OAuth spec https://datatracker.ietf.org/doc/html/rfc9728#PRConfigurationValidation`);
}
// TODO:@TylerLeonhardt support multiple authorization servers
// Consider using one that has an auth provider first, over the dynamic flow
serverMetadataUrl = resourceMetadata.authorization_servers?.[0];
Expand Down Expand Up @@ -395,11 +399,6 @@ class McpHTTPHandle extends Disposable {
}
const body = await resourceMetadataResponse.json();
if (isAuthorizationProtectedResourceMetadata(body)) {
const resolvedResource = getResourceServerBaseUrlFromDiscoveryUrl(resourceMetadata);
// Use URL constructor for normalization - it handles hostname case and trailing slashes
if (new URL(body.resource).toString() !== new URL(resolvedResource).toString()) {
throw new Error(`Protected Resource Metadata resource "${body.resource}" does not match MCP server resolved resource "${resolvedResource}". The MCP server must follow OAuth spec https://datatracker.ietf.org/doc/html/rfc9728#PRConfigurationValidation`);
}
return body;
} else {
throw new Error(`Invalid resource metadata: ${JSON.stringify(body)}`);
Expand Down Expand Up @@ -701,7 +700,7 @@ class McpHTTPHandle extends Disposable {
let res = await doFetch();
if (res.status === 401) {
if (!this._authMetadata) {
await this._populateAuthMetadata(res);
await this._populateAuthMetadata(url, res);
await this._addAuthHeader(headers);
if (headers['Authorization']) {
// Update the headers in the init object
Expand Down
Loading