adjust default validation for resource parameter in client flow, and server example

This commit is contained in:
Paul Carleton 2025-06-19 17:52:24 +01:00
commit 87da0e0c3a
No known key found for this signature in database
5 changed files with 85 additions and 13 deletions

View file

@ -1041,7 +1041,7 @@ describe("OAuth Authorization", () => {
// Verify custom validation method was called
expect(mockValidateResourceURL).toHaveBeenCalledWith(
"https://api.example.com/mcp-server",
new URL("https://api.example.com/mcp-server"),
"https://different-resource.example.com/mcp-server"
);
});

View file

@ -2,7 +2,7 @@ import pkceChallenge from "pkce-challenge";
import { LATEST_PROTOCOL_VERSION } from "../types.js";
import type { OAuthClientMetadata, OAuthClientInformation, OAuthTokens, OAuthMetadata, OAuthClientInformationFull, OAuthProtectedResourceMetadata } from "../shared/auth.js";
import { OAuthClientInformationFullSchema, OAuthMetadataSchema, OAuthProtectedResourceMetadataSchema, OAuthTokensSchema } from "../shared/auth.js";
import { resourceUrlFromServerUrl } from "../shared/auth-utils.js";
import { checkResourceAllowed, resourceUrlFromServerUrl } from "../shared/auth-utils.js";
/**
* Implements an end-to-end OAuth client to be used with one MCP server.
@ -198,13 +198,13 @@ export async function auth(
}
async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
const resource = resourceUrlFromServerUrl(serverUrl);
if (provider.validateResourceURL) {
return await provider.validateResourceURL(serverUrl, resourceMetadata?.resource);
}
const resource = resourceUrlFromServerUrl(typeof serverUrl === "string" ? new URL(serverUrl) : serverUrl);
if (resourceMetadata && resourceMetadata.resource !== resource.href) {
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${resource}`);
return await provider.validateResourceURL(resource, resourceMetadata?.resource);
} else if (resourceMetadata) {
if (!checkResourceAllowed({ requestedResource: resource, configuredResource: resourceMetadata.resource })) {
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${resource} (or origin)`);
}
}
return resource;

View file

@ -9,6 +9,7 @@ import { CallToolResult, GetPromptResult, isInitializeRequest, PrimitiveSchemaDe
import { InMemoryEventStore } from '../shared/inMemoryEventStore.js';
import { setupAuthServer } from './demoInMemoryOAuthProvider.js';
import { OAuthMetadata } from 'src/shared/auth.js';
import { checkResourceAllowed } from 'src/shared/auth-utils.js';
// Check for OAuth flag
const useOAuth = process.argv.includes('--oauth');
@ -463,7 +464,7 @@ if (useOAuth) {
if (!data.aud) {
throw new Error(`Resource Indicator (RFC8707) missing`);
}
if (data.aud !== mcpServerUrl.href) {
if (!checkResourceAllowed({ requestedResource: data.aud, configuredResource: mcpServerUrl })) {
throw new Error(`Expected resource indicator ${mcpServerUrl}, got: ${data.aud}`);
}
}

View file

@ -1,4 +1,4 @@
import { resourceUrlFromServerUrl } from './auth-utils.js';
import { resourceUrlFromServerUrl, checkResourceAllowed } from './auth-utils.js';
describe('auth-utils', () => {
describe('resourceUrlFromServerUrl', () => {
@ -27,4 +27,35 @@ describe('auth-utils', () => {
expect(resourceUrlFromServerUrl(new URL('https://example.com/path/')).href).toBe('https://example.com/path/');
});
});
});
describe('resourceMatches', () => {
it('should match identical URLs', () => {
expect(checkResourceAllowed({ requestedResource: 'https://example.com/path', configuredResource: 'https://example.com/path' })).toBe(true);
expect(checkResourceAllowed({ requestedResource: 'https://example.com/', configuredResource: 'https://example.com/' })).toBe(true);
});
it('should not match URLs with different paths', () => {
expect(checkResourceAllowed({ requestedResource: 'https://example.com/path1', configuredResource: 'https://example.com/path2' })).toBe(false);
expect(checkResourceAllowed({ requestedResource: 'https://example.com/', configuredResource: 'https://example.com/path' })).toBe(false);
});
it('should not match URLs with different domains', () => {
expect(checkResourceAllowed({ requestedResource: 'https://example.com/path', configuredResource: 'https://example.org/path' })).toBe(false);
});
it('should not match URLs with different ports', () => {
expect(checkResourceAllowed({ requestedResource: 'https://example.com:8080/path', configuredResource: 'https://example.com/path' })).toBe(false);
});
it('should not match URLs where one path is a sub-path of another', () => {
expect(checkResourceAllowed({ requestedResource: 'https://example.com/mcpxxxx', configuredResource: 'https://example.com/mcp' })).toBe(false);
expect(checkResourceAllowed({ requestedResource: 'https://example.com/folder', configuredResource: 'https://example.com/folder/subfolder' })).toBe(false);
expect(checkResourceAllowed({ requestedResource: 'https://example.com/api/v1', configuredResource: 'https://example.com/api' })).toBe(true);
});
it('should handle trailing slashes vs no trailing slashes', () => {
expect(checkResourceAllowed({ requestedResource: 'https://example.com/mcp/', configuredResource: 'https://example.com/mcp' })).toBe(true);
expect(checkResourceAllowed({ requestedResource: 'https://example.com/folder', configuredResource: 'https://example.com/folder/' })).toBe(false);
});
});
});

View file

@ -7,8 +7,48 @@
* RFC 8707 section 2 states that resource URIs "MUST NOT include a fragment component".
* Keeps everything else unchanged (scheme, domain, port, path, query).
*/
export function resourceUrlFromServerUrl(url: URL): URL {
const resourceURL = new URL(url.href);
export function resourceUrlFromServerUrl(url: URL | string ): URL {
const resourceURL = typeof url === "string" ? new URL(url) : new URL(url.href);
resourceURL.hash = ''; // Remove fragment
return resourceURL;
}
/**
* Checks if a requested resource URL matches a configured resource URL.
* A requested resource matches if it has the same scheme, domain, port,
* and its path starts with the configured resource's path.
*
* @param requestedResource The resource URL being requested
* @param configuredResource The resource URL that has been configured
* @returns true if the requested resource matches the configured resource, false otherwise
*/
export function checkResourceAllowed(
{ requestedResource, configuredResource }: {
requestedResource: URL | string;
configuredResource: URL | string
}
): boolean {
const requested = typeof requestedResource === "string" ? new URL(requestedResource) : new URL(requestedResource.href);
const configured = typeof configuredResource === "string" ? new URL(configuredResource) : new URL(configuredResource.href);
// Compare the origin (scheme, domain, and port)
if (requested.origin !== configured.origin) {
return false;
}
// Handle cases like requested=/foo and configured=/foo/
if (requested.pathname.length < configured.pathname.length) {
return false
}
// Check if the requested path starts with the configured path
// Ensure both paths end with / for proper comparison
// This ensures that if we have paths like "/api" and "/api/users",
// we properly detect that "/api/users" is a subpath of "/api"
// By adding a trailing slash if missing, we avoid false positives
// where paths like "/api123" would incorrectly match "/api"
const requestedPath = requested.pathname.endsWith('/') ? requested.pathname : requested.pathname + '/';
const configuredPath = configured.pathname.endsWith('/') ? configured.pathname : configured.pathname + '/';
return requestedPath.startsWith(configuredPath);
}