8000 [auth] adjust default validation for resource parameter in client flow, and server example by pcarleton · Pull Request #664 · modelcontextprotocol/typescript-sdk · GitHub
[go: up one dir, main page]

Skip to content

[auth] adjust default validation for resource parameter in client flow, and server example #664

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.

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
merged 2 commits into from
Jun 20, 2025
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
63 changes: 62 additions & 1 deletion src/client/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1041,9 +1041,70 @@ 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"
);
});

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) => {
const urlString = url.toString();

if (urlString.includes("/.well-known/oauth-protected-resource")) {
return Promise.resolve({
ok: true,
status: 200,
json: async () => ({
// Resource is a prefix of the requested server URL
resource: "https://api.example.com/",
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 });
});

// Mock provider methods
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
});
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
(mockProvider.redirectToAuthorization as jest.Mock).mockResolvedValue(undefined);

// Call auth with a URL that has the resource as prefix
const result = await auth(mockProvider, {
serverUrl: "https://api.example.com/mcp-server/endpoint",
});

expect(result).toBe("REDIRECT");

// Verify the authorization URL includes the resource parameter from PRM
expect(mockProvider.redirectToAuthorization).toHaveBeenCalledWith(
expect.objectContaining({
searchParams: expect.any(URLSearchParams),
})
);

const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
const authUrl: URL = redirectCall[0];
// Should use the PRM's resource value, not the full requested URL
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/");
});
});
});
19 changes: 11 additions & 8 deletions src/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -197,14 +197,17 @@ export async function auth(
return "REDIRECT";
}

async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
export async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
let 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 })) {
// If the resource mentioned in metadata is valid, prefer it since it is what the server is telling us to request.
resource = new URL(resourceMetadata.resource);
} else {
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${resource} (or origin)`);
}
}

return resource;
Expand Down
3 changes: 2 additions & 1 deletion src/examples/server/simpleStreamableHttp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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}`);
}
}
Expand Down
35 changes: 33 additions & 2 deletions src/shared/auth-utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { resourceUrlFromServerUrl } from './auth-utils.js';
import { resourceUrlFromServerUrl, checkResourceAllowed } from './auth-utils.js';

describe('auth-utils', () => {
describe('resourceUrlFromServerUrl', () => {
Expand Down Expand Up @@ -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);
});
});
});
44 changes: 42 additions & 2 deletions src/shared/auth-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
0