Skip to content

Commit 049351e

Browse files
xiaoyijunxiaoyijun
committed
fix: preserve canonical URL format in OAuth resource parameter per MCP auth spec
1 parent 31acdcb commit 049351e

File tree

7 files changed

+287
-41
lines changed

7 files changed

+287
-41
lines changed

‎src/client/auth.test.ts

Lines changed: 182 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -924,7 +924,7 @@ describe("OAuth Authorization", () => {
924924
metadata: undefined,
925925
clientInformation: validClientInfo,
926926
redirectUrl: "http://localhost:3000/callback",
927-
resource: new URL("https://api.example.com/mcp-server"),
927+
resource: "https://api.example.com/mcp-server",
928928
}
929929
);
930930

@@ -1088,7 +1088,7 @@ describe("OAuth Authorization", () => {
10881088
authorizationCode: "code123",
10891089
codeVerifier: "verifier123",
10901090
redirectUri: "http://localhost:3000/callback",
1091-
resource: new URL("https://api.example.com/mcp-server"),
1091+
resource: "https://api.example.com/mcp-server",
10921092
});
10931093

10941094
expect(tokens).toEqual(validTokens);
@@ -1210,7 +1210,7 @@ describe("OAuth Authorization", () => {
12101210
authorizationCode: "code123",
12111211
codeVerifier: "verifier123",
12121212
redirectUri: "http://localhost:3000/callback",
1213-
resource: new URL("https://api.example.com/mcp-server"),
1213+
resource: "https://api.example.com/mcp-server",
12141214
fetchFn: customFetch,
12151215
});
12161216

@@ -1274,7 +1274,7 @@ describe("OAuth Authorization", () => {
12741274
const tokens = await refreshAuthorization("https://auth.example.com", {
12751275
clientInformation: validClientInfo,
12761276
refreshToken: "refresh123",
1277-
resource: new URL("https://api.example.com/mcp-server"),
1277+
resource: "https://api.example.com/mcp-server",
12781278
});
12791279

12801280
expect(tokens).toEqual(validTokensWithNewRefreshToken);
@@ -1497,6 +1497,183 @@ describe("OAuth Authorization", () => {
14971497
codeVerifier: jest.fn(),
14981498
};
14991499

1500+
describe("resource URL handling (trailing slash preservation)", () => {
1501+
beforeEach(() => {
1502+
jest.clearAllMocks();
1503+
});
1504+
1505+
it("preserves server URLs without trailing slash in resource parameter", async () => {
1506+
// Mock successful metadata discovery
1507+
mockFetch.mockImplementation((url) => {
1508+
const urlString = url.toString();
1509+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1510+
return Promise.resolve({
1511+
ok: true,
1512+
status: 200,
1513+
json: async () => ({
1514+
resource: "https://api.example.com/mcp-server", // No trailing slash
1515+
authorization_servers: ["https://auth.example.com"],
1516+
}),
1517+
});
1518+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
1519+
return Promise.resolve({
1520+
ok: true,
1521+
status: 200,
1522+
json: async () => ({
1523+
issuer: "https://auth.example.com",
1524+
authorization_endpoint: "https://auth.example.com/authorize",
1525+
token_endpoint: "https://auth.example.com/token",
1526+
response_types_supported: ["code"],
1527+
code_challenge_methods_supported: ["S256"],
1528+
}),
1529+
});
1530+
}
1531+
return Promise.resolve({ ok: false, status: 404 });
1532+
});
1533+
1534+
// Mock provider methods for authorization flow
1535+
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
1536+
client_id: "test-client",
1537+
client_secret: "test-secret",
1538+
});
1539+
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
1540+
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
1541+
(mockProvider.redirectToAuthorization as jest.Mock).mockResolvedValue(undefined);
1542+
1543+
// Call auth with URL that has no trailing slash
1544+
const result = await auth(mockProvider, {
1545+
serverUrl: "https://api.example.com/mcp-server", // No trailing slash
1546+
});
1547+
1548+
expect(result).toBe("REDIRECT");
1549+
1550+
// Verify the authorization URL includes the resource parameter WITHOUT trailing slash
1551+
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
1552+
const authUrl: URL = redirectCall[0];
1553+
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/mcp-server"); // No trailing slash
1554+
});
1555+
1556+
it("preserves server URLs with trailing slash in resource parameter", async () => {
1557+
// Mock successful metadata discovery
1558+
mockFetch.mockImplementation((url) => {
1559+
const urlString = url.toString();
1560+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1561+
return Promise.resolve({
1562+
ok: true,
1563+
status: 200,
1564+
json: async () => ({
1565+
resource: "https://api.example.com/mcp-server/", // With trailing slash
1566+
authorization_servers: ["https://auth.example.com"],
1567+
}),
1568+
});
1569+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
1570+
return Promise.resolve({
1571+
ok: true,
1572+
status: 200,
1573+
json: async () => ({
1574+
issuer: "https://auth.example.com",
1575+
authorization_endpoint: "https://auth.example.com/authorize",
1576+
token_endpoint: "https://auth.example.com/token",
1577+
response_types_supported: ["code"],
1578+
code_challenge_methods_supported: ["S256"],
1579+
}),
1580+
});
1581+
}
1582+
return Promise.resolve({ ok: false, status: 404 });
1583+
});
1584+
1585+
// Mock provider methods
1586+
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
1587+
client_id: "test-client",
1588+
client_secret: "test-secret",
1589+
});
1590+
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
1591+
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
1592+
(mockProvider.redirectToAuthorization as jest.Mock).mockResolvedValue(undefined);
1593+
1594+
// Call auth with URL that has trailing slash
1595+
const result = await auth(mockProvider, {
1596+
serverUrl: "https://api.example.com/mcp-server/", // With trailing slash
1597+
});
1598+
1599+
expect(result).toBe("REDIRECT");
1600+
1601+
// Verify the authorization URL includes the resource parameter WITH trailing slash
1602+
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
1603+
const authUrl: URL = redirectCall[0];
1604+
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/mcp-server/"); // With trailing slash
1605+
});
1606+
1607+
it("handles token exchange with preserved resource URL format", async () => {
1608+
// Mock successful metadata discovery and token exchange
1609+
mockFetch.mockImplementation((url) => {
1610+
const urlString = url.toString();
1611+
1612+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1613+
return Promise.resolve({
1614+
ok: true,
1615+
status: 200,
1616+
json: async () => ({
1617+
resource: "https://api.example.com/mcp-server", // No trailing slash
1618+
authorization_servers: ["https://auth.example.com"],
1619+
}),
1620+
});
1621+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
1622+
return Promise.resolve({
1623+
ok: true,
1624+
status: 200,
1625+
json: async () => ({
1626+
issuer: "https://auth.example.com",
1627+
authorization_endpoint: "https://auth.example.com/authorize",
1628+
token_endpoint: "https://auth.example.com/token",
1629+
response_types_supported: ["code"],
1630+
code_challenge_methods_supported: ["S256"],
1631+
}),
1632+
});
1633+
} else if (urlString.includes("/token")) {
1634+
return Promise.resolve({
1635+
ok: true,
1636+
status: 200,
1637+
json: async () => ({
1638+
access_token: "access123",
1639+
token_type: "Bearer",
1640+
expires_in: 3600,
1641+
refresh_token: "refresh123",
1642+
}),
1643+
});
1644+
}
1645+
1646+
return Promise.resolve({ ok: false, status: 404 });
1647+
});
1648+
1649+
// Mock provider methods for token exchange
1650+
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
1651+
client_id: "test-client",
1652+
client_secret: "test-secret",
1653+
});
1654+
(mockProvider.codeVerifier as jest.Mock).mockResolvedValue("test-verifier");
1655+
(mockProvider.saveTokens as jest.Mock).mockResolvedValue(undefined);
1656+
1657+
// Call auth with authorization code and URL without trailing slash
1658+
const result = await auth(mockProvider, {
1659+
serverUrl: "https://api.example.com/mcp-server", // No trailing slash
1660+
authorizationCode: "auth-code-123",
1661+
});
1662+
1663+
expect(result).toBe("AUTHORIZED");
1664+
1665+
// Find the token exchange call and verify resource parameter format
1666+
const tokenCall = mockFetch.mock.calls.find(call =>
1667+
call[0].toString().includes("/token")
1668+
);
1669+
expect(tokenCall).toBeDefined();
1670+
1671+
const body = tokenCall![1].body as URLSearchParams;
1672+
expect(body.get("resource")).toBe("https://api.example.com/mcp-server"); // No trailing slash added
1673+
expect(body.get("code")).toBe("auth-code-123");
1674+
});
1675+
});
1676+
15001677
beforeEach(() => {
15011678
jest.clearAllMocks();
15021679
});
@@ -1829,7 +2006,7 @@ describe("OAuth Authorization", () => {
18292006

18302007
// Verify custom validation method was called
18312008
expect(mockValidateResourceURL).toHaveBeenCalledWith(
1832-
new URL("https://api.example.com/mcp-server"),
2009+
"https://api.example.com/mcp-server",
18332010
"https://different-resource.example.com/mcp-server"
18342011
);
18352012
});

‎src/client/auth.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export interface OAuthClientProvider {
119119
*
120120
* Implementations must verify the returned resource matches the MCP server.
121121
*/
122-
validateResourceURL?(serverUrl: string | URL, resource?: string): Promise<URL | undefined>;
122+
validateResourceURL?(serverUrl: string, resource?: string): Promise<string | undefined>;
123123

124124
/**
125125
* If implemented, provides a way for the client to invalidate (e.g. delete) the specified
@@ -281,7 +281,7 @@ export async function parseErrorResponse(input: Response | string): Promise<OAut
281281
export async function auth(
282282
provider: OAuthClientProvider,
283283
options: {
284-
serverUrl: string | URL;
284+
serverUrl: string;
285285
authorizationCode?: string;
286286
scope?: string;
287287
resourceMetadataUrl?: URL;
@@ -312,7 +312,7 @@ async function authInternal(
312312
resourceMetadataUrl,
313313
fetchFn,
314314
}: {
315-
serverUrl: string | URL;
315+
serverUrl: string;
316316
authorizationCode?: string;
317317
scope?: string;
318318
resourceMetadataUrl?: URL;
@@ -339,7 +339,7 @@ async function authInternal(
339339
authorizationServerUrl = serverUrl;
340340
}
341341

342-
const resource: URL | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata);
342+
const resource: string | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata);
343343

344344
const metadata = await discoverAuthorizationServerMetadata(authorizationServerUrl, {
345345
fetchFn,
@@ -427,7 +427,7 @@ async function authInternal(
427427
return "REDIRECT"
428428
}
429429

430-
export async function selectResourceURL(serverUrl: string | URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
430+
export async function selectResourceURL(serverUrl: string, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<string | undefined> {
431431
const defaultResource = resourceUrlFromServerUrl(serverUrl);
432432

433433
// If provider has custom validation, delegate to it
@@ -445,7 +445,7 @@ export async function selectResourceURL(serverUrl: string | URL, provider: OAuth
445445
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin)`);
446446
}
447447
// Prefer the resource from metadata since it's what the server is telling us to request
448-
return new URL(resourceMetadata.resource);
448+
return resourceMetadata.resource;
449449
}
450450

451451
/**
@@ -807,7 +807,7 @@ export async function startAuthorization(
807807
redirectUrl: string | URL;
808808
scope?: string;
809809
state?: string;
810-
resource?: URL;
810+
resource?: string;
811811
},
812812
): Promise<{ authorizationUrl: URL; codeVerifier: string }> {
813813
const responseType = "code";
@@ -865,7 +865,7 @@ export async function startAuthorization(
865865
}
866866

867867
if (resource) {
868-
authorizationUrl.searchParams.set("resource", resource.href);
868+
authorizationUrl.searchParams.set("resource", resource);
869869
}
870870

871871
return { authorizationUrl, codeVerifier };
@@ -900,7 +900,7 @@ export async function exchangeAuthorization(
900900
authorizationCode: string;
901901
codeVerifier: string;
902902
redirectUri: string | URL;
903-
resource?: URL;
903+
resource?: string;
904904
addClientAuthentication?: OAuthClientProvider["addClientAuthentication"];
905905
fetchFn?: FetchLike;
906906
},
@@ -943,7 +943,7 @@ export async function exchangeAuthorization(
943943
}
944944

945945
if (resource) {
946-
params.set("resource", resource.href);
946+
params.set("resource", resource);
947947
}
948948

949949
const response = await (fetchFn ?? fetch)(tokenUrl, {
@@ -984,7 +984,7 @@ export async function refreshAuthorization(
984984
metadata?: AuthorizationServerMetadata;
985985
clientInformation: OAuthClientInformation;
986986
refreshToken: string;
987-
resource?: URL;
987+
resource?: string;
988988
addClientAuthentication?: OAuthClientProvider["addClientAuthentication"];
989989
fetchFn?: FetchLike;
990990
}
@@ -1027,7 +1027,7 @@ export async function refreshAuthorization(
10271027
}
10281028

10291029
if (resource) {
1030-
params.set("resource", resource.href);
1030+
params.set("resource", resource);
10311031
}
10321032

10331033
const response = await (fetchFn ?? fetch)(tokenUrl, {

‎src/client/sse.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export class SSEClientTransport implements Transport {
9393

9494
let result: AuthResult;
9595
try {
96-
result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
96+
result = await auth(this._authProvider, { serverUrl: this._url.href, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
9797
} catch (error) {
9898
this.onerror?.(error as Error);
9999
throw error;
@@ -218,7 +218,7 @@ export class SSEClientTransport implements Transport {
218218
throw new UnauthorizedError("No auth provider");
219219
}
220220

221-
const result = await auth(this._authProvider, { serverUrl: this._url, authorizationCode, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
221+
const result = await auth(this._authProvider, { serverUrl: this._url.href, authorizationCode, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
222222
if (result !== "AUTHORIZED") {
223223
throw new UnauthorizedError("Failed to authorize");
224224
}
@@ -252,7 +252,7 @@ export class SSEClientTransport implements Transport {
252252

253253
this._resourceMetadataUrl = extractResourceMetadataUrl(response);
254254

255-
const result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
255+
const result = await auth(this._authProvider, { serverUrl: this._url.href, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
256256
if (result !== "AUTHORIZED") {
257257
throw new UnauthorizedError();
258258
}

‎src/client/streamableHttp.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export class StreamableHTTPClientTransport implements Transport {
156156

157157
let result: AuthResult;
158158
try {
159-
result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
159+
result = await auth(this._authProvider, { serverUrl: this._url.href, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
160160
} catch (error) {
161161
this.onerror?.(error as Error);
162162
throw error;
@@ -392,7 +392,7 @@ export class StreamableHTTPClientTransport implements Transport {
392392
throw new UnauthorizedError("No auth provider");
393393
}
394394

395-
const result = await auth(this._authProvider, { serverUrl: this._url, authorizationCode, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
395+
const result = await auth(this._authProvider, { serverUrl: this._url.href, authorizationCode, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
396396
if (result !== "AUTHORIZED") {
397397
throw new UnauthorizedError("Failed to authorize");
398398
}
@@ -440,7 +440,7 @@ export class StreamableHTTPClientTransport implements Transport {
440440

441441
this._resourceMetadataUrl = extractResourceMetadataUrl(response);
442442

443-
const result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
443+
const result = await auth(this._authProvider, { serverUrl: this._url.href, resourceMetadataUrl: this._resourceMetadataUrl, fetchFn: this._fetch });
444444
if (result !== "AUTHORIZED") {
445445
throw new UnauthorizedError();
446446
}

‎src/examples/server/demoInMemoryOAuthProvider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ export const setupAuthServer = ({authServerUrl, mcpServerUrl, strictResource}: {
153153

154154
const validateResource = strictResource ? (resource?: URL) => {
155155
if (!resource) return false;
156-
const expectedResource = resourceUrlFromServerUrl(mcpServerUrl);
156+
const expectedResource = resourceUrlFromServerUrl(mcpServerUrl.href);
157157
return resource.toString() === expectedResource.toString();
158158
} : undefined;
159159

0 commit comments

Comments
 (0)