From 551a43942e412f43aee13de67ee7a796f5763831 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 17 Jun 2025 00:33:27 +0100 Subject: [PATCH] use URL for resource throughout --- src/client/auth.test.ts | 24 +++++++++---------- src/client/sse.ts | 8 +++---- src/client/streamableHttp.ts | 8 +++---- src/server/auth/handlers/token.test.ts | 8 +++---- src/server/auth/handlers/token.ts | 4 ++-- .../auth/providers/proxyProvider.test.ts | 6 ++--- src/server/auth/providers/proxyProvider.ts | 4 ++-- 7 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 9a067405..44516130 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -347,7 +347,7 @@ describe("OAuth Authorization", () => { { clientInformation: validClientInfo, redirectUrl: "http://localhost:3000/callback", - resource: "https://api.example.com/mcp-server", + resource: new URL("https://api.example.com/mcp-server"), } ); @@ -526,7 +526,7 @@ describe("OAuth Authorization", () => { authorizationCode: "code123", codeVerifier: "verifier123", redirectUri: "http://localhost:3000/callback", - resource: "https://api.example.com/mcp-server", + resource: new URL("https://api.example.com/mcp-server"), }); expect(tokens).toEqual(validTokens); @@ -650,7 +650,7 @@ describe("OAuth Authorization", () => { const tokens = await refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", - resource: "https://api.example.com/mcp-server", + resource: new URL("https://api.example.com/mcp-server"), }); expect(tokens).toEqual(validTokensWithNewRefreshToken); @@ -939,7 +939,7 @@ describe("OAuth Authorization", () => { // Call the auth function with a resource that has a fragment const result = await auth(mockProvider, { serverUrl: "https://resource.example.com", - resource: "https://api.example.com/mcp-server#fragment", + resource: new URL("https://api.example.com/mcp-server#fragment"), }); expect(result).toBe("REDIRECT"); @@ -988,7 +988,7 @@ describe("OAuth Authorization", () => { // Call auth without authorization code (should trigger redirect) const result = await auth(mockProvider, { serverUrl: "https://resource.example.com", - resource: "https://api.example.com/mcp-server", + resource: new URL("https://api.example.com/mcp-server"), }); expect(result).toBe("REDIRECT"); @@ -1050,7 +1050,7 @@ describe("OAuth Authorization", () => { const result = await auth(mockProvider, { serverUrl: "https://resource.example.com", authorizationCode: "auth-code-123", - resource: "https://api.example.com/mcp-server", + resource: new URL("https://api.example.com/mcp-server"), }); expect(result).toBe("AUTHORIZED"); @@ -1112,7 +1112,7 @@ describe("OAuth Authorization", () => { // Call auth with existing tokens (should trigger refresh) const result = await auth(mockProvider, { serverUrl: "https://resource.example.com", - resource: "https://api.example.com/mcp-server", + resource: new URL("https://api.example.com/mcp-server"), }); expect(result).toBe("AUTHORIZED"); @@ -1161,7 +1161,7 @@ describe("OAuth Authorization", () => { // Call auth with empty resource parameter const result = await auth(mockProvider, { serverUrl: "https://resource.example.com", - resource: "", + resource: undefined, }); expect(result).toBe("REDIRECT"); @@ -1204,7 +1204,7 @@ describe("OAuth Authorization", () => { // Call auth with resource containing multiple # symbols const result = await auth(mockProvider, { serverUrl: "https://resource.example.com", - resource: "https://api.example.com/mcp-server#fragment#another", + resource: new URL("https://api.example.com/mcp-server#fragment#another"), }); expect(result).toBe("REDIRECT"); @@ -1249,7 +1249,7 @@ describe("OAuth Authorization", () => { // multiple MCP servers on the same domain const result1 = await auth(mockProvider, { serverUrl: "https://api.example.com", - resource: "https://api.example.com/mcp-server-1/v1", + resource: new URL("https://api.example.com/mcp-server-1/v1"), }); expect(result1).toBe("REDIRECT"); @@ -1264,7 +1264,7 @@ describe("OAuth Authorization", () => { // Test with different path on same domain const result2 = await auth(mockProvider, { serverUrl: "https://api.example.com", - resource: "https://api.example.com/mcp-server-2/v1", + resource: new URL("https://api.example.com/mcp-server-2/v1"), }); expect(result2).toBe("REDIRECT"); @@ -1309,7 +1309,7 @@ describe("OAuth Authorization", () => { // Call auth with resource containing query parameters const result = await auth(mockProvider, { serverUrl: "https://resource.example.com", - resource: "https://api.example.com/mcp-server?param=value&another=test", + resource: new URL("https://api.example.com/mcp-server?param=value&another=test"), }); expect(result).toBe("REDIRECT"); diff --git a/src/client/sse.ts b/src/client/sse.ts index c484bde9..41f21de6 100644 --- a/src/client/sse.ts +++ b/src/client/sse.ts @@ -2,7 +2,7 @@ import { EventSource, type ErrorEvent, type EventSourceInit } from "eventsource" import { Transport } from "../shared/transport.js"; import { JSONRPCMessage, JSONRPCMessageSchema } from "../types.js"; import { auth, AuthResult, extractResourceMetadataUrl, OAuthClientProvider, UnauthorizedError } from "./auth.js"; -import { extractResourceUri } from "../shared/auth-utils.js"; +import { resourceUrlFromServerUrl } from "../shared/auth-utils.js"; export class SseError extends Error { constructor( @@ -90,7 +90,7 @@ export class SSEClientTransport implements Transport { result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, - resource: extractResourceUri(this._url) + resource: resourceUrlFromServerUrl(new URL(this._url)) }); } catch (error) { this.onerror?.(error as Error); @@ -210,7 +210,7 @@ export class SSEClientTransport implements Transport { serverUrl: this._url, authorizationCode, resourceMetadataUrl: this._resourceMetadataUrl, - resource: extractResourceUri(this._url) + resource: resourceUrlFromServerUrl(new URL(this._url)) }); if (result !== "AUTHORIZED") { throw new UnauthorizedError("Failed to authorize"); @@ -249,7 +249,7 @@ export class SSEClientTransport implements Transport { const result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, - resource: extractResourceUri(this._url) + resource: resourceUrlFromServerUrl(new URL(this._url)) }); if (result !== "AUTHORIZED") { throw new UnauthorizedError(); diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index 25c41bf3..3534fb45 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -2,7 +2,7 @@ import { Transport } from "../shared/transport.js"; import { isInitializedNotification, isJSONRPCRequest, isJSONRPCResponse, JSONRPCMessage, JSONRPCMessageSchema } from "../types.js"; import { auth, AuthResult, extractResourceMetadataUrl, OAuthClientProvider, UnauthorizedError } from "./auth.js"; import { EventSourceParserStream } from "eventsource-parser/stream"; -import { extractResourceUri } from "../shared/auth-utils.js"; +import { resourceUrlFromServerUrl } from "../shared/auth-utils.js"; // Default reconnection options for StreamableHTTP connections const DEFAULT_STREAMABLE_HTTP_RECONNECTION_OPTIONS: StreamableHTTPReconnectionOptions = { @@ -153,7 +153,7 @@ export class StreamableHTTPClientTransport implements Transport { result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, - resource: extractResourceUri(this._url) + resource: resourceUrlFromServerUrl(new URL(this._url)) }); } catch (error) { this.onerror?.(error as Error); @@ -371,7 +371,7 @@ export class StreamableHTTPClientTransport implements Transport { serverUrl: this._url, authorizationCode, resourceMetadataUrl: this._resourceMetadataUrl, - resource: extractResourceUri(this._url) + resource: resourceUrlFromServerUrl(new URL(this._url)) }); if (result !== "AUTHORIZED") { throw new UnauthorizedError("Failed to authorize"); @@ -423,7 +423,7 @@ export class StreamableHTTPClientTransport implements Transport { const result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, - resource: extractResourceUri(this._url) + resource: resourceUrlFromServerUrl(new URL(this._url)) }); if (result !== "AUTHORIZED") { throw new UnauthorizedError(); diff --git a/src/server/auth/handlers/token.test.ts b/src/server/auth/handlers/token.test.ts index 68794c36..63b47f53 100644 --- a/src/server/auth/handlers/token.test.ts +++ b/src/server/auth/handlers/token.test.ts @@ -303,7 +303,7 @@ describe('Token Handler', () => { 'valid_code', undefined, // code_verifier is undefined after PKCE validation undefined, // redirect_uri - 'https://api.example.com/resource' // resource parameter + new URL('https://api.example.com/resource') // resource parameter ); }); @@ -371,7 +371,7 @@ describe('Token Handler', () => { 'valid_code', undefined, // code_verifier is undefined after PKCE validation 'https://example.com/callback', // redirect_uri - 'https://api.example.com/resource' // resource parameter + new URL('https://api.example.com/resource') // resource parameter ); }); @@ -585,7 +585,7 @@ describe('Token Handler', () => { validClient, 'valid_refresh_token', undefined, // scopes - 'https://api.example.com/resource' // resource parameter + new URL('https://api.example.com/resource') // resource parameter ); }); @@ -648,7 +648,7 @@ describe('Token Handler', () => { validClient, 'valid_refresh_token', ['profile', 'email'], // scopes - 'https://api.example.com/resource' // resource parameter + new URL('https://api.example.com/resource') // resource parameter ); }); }); diff --git a/src/server/auth/handlers/token.ts b/src/server/auth/handlers/token.ts index 92fe9921..3ffd4cf2 100644 --- a/src/server/auth/handlers/token.ts +++ b/src/server/auth/handlers/token.ts @@ -113,7 +113,7 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand code, skipLocalPkceValidation ? code_verifier : undefined, redirect_uri, - resource + resource ? new URL(resource) : undefined ); res.status(200).json(tokens); break; @@ -131,7 +131,7 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand // The provider can decide how to validate it const scopes = scope?.split(" "); - const tokens = await provider.exchangeRefreshToken(client, refresh_token, scopes, resource); + const tokens = await provider.exchangeRefreshToken(client, refresh_token, scopes, resource ? new URL(resource) : undefined); res.status(200).json(tokens); break; } diff --git a/src/server/auth/providers/proxyProvider.test.ts b/src/server/auth/providers/proxyProvider.test.ts index 75dc1a15..b834c659 100644 --- a/src/server/auth/providers/proxyProvider.test.ts +++ b/src/server/auth/providers/proxyProvider.test.ts @@ -213,7 +213,7 @@ describe("Proxy OAuth Server Provider", () => { 'test-code', 'test-verifier', 'https://example.com/callback', - 'https://api.example.com/resource' + new URL('https://api.example.com/resource') ); expect(global.fetch).toHaveBeenCalledWith( @@ -267,7 +267,7 @@ describe("Proxy OAuth Server Provider", () => { validClient, 'test-refresh-token', ['read', 'write'], - 'https://api.example.com/resource' + new URL('https://api.example.com/resource') ); expect(global.fetch).toHaveBeenCalledWith( @@ -301,7 +301,7 @@ describe("Proxy OAuth Server Provider", () => { validClient, 'test-refresh-token', ['profile', 'email'], - 'https://api.example.com/resource' + new URL('https://api.example.com/resource') ); const fetchCall = (global.fetch as jest.Mock).mock.calls[0]; diff --git a/src/server/auth/providers/proxyProvider.ts b/src/server/auth/providers/proxyProvider.ts index 4c807444..de74862b 100644 --- a/src/server/auth/providers/proxyProvider.ts +++ b/src/server/auth/providers/proxyProvider.ts @@ -175,7 +175,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } if (resource) { - params.append("resource", resource); + params.append("resource", resource.href); } const response = await fetch(this._endpoints.tokenUrl, { @@ -217,7 +217,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } if (resource) { - params.set("resource", resource); + params.set("resource", resource.href); } const response = await fetch(this._endpoints.tokenUrl, {