Skip to content

Commit 442e13b

Browse files
ihrprihrpr
authored
Merge pull request #756 from modelcontextprotocol/patch-1
Fix oauth well-known paths to retain path and query
2 parents 11e84f0 + 5eacdf1 commit 442e13b

File tree

2 files changed

+220
-41
lines changed

2 files changed

+220
-41
lines changed

‎src/client/auth.test.ts‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,174 @@ describe("OAuth Authorization", () => {
178178
await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com"))
179179
.rejects.toThrow();
180180
});
181+
182+
it("returns metadata when discovery succeeds with path", async () => {
183+
mockFetch.mockResolvedValueOnce({
184+
ok: true,
185+
status: 200,
186+
json: async () => validMetadata,
187+
});
188+
189+
const metadata = await discoverOAuthProtectedResourceMetadata("https://resource.example.com/path/name");
190+
expect(metadata).toEqual(validMetadata);
191+
const calls = mockFetch.mock.calls;
192+
expect(calls.length).toBe(1);
193+
const [url] = calls[0];
194+
expect(url.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource/path/name");
195+
});
196+
197+
it("preserves query parameters in path-aware discovery", async () => {
198+
mockFetch.mockResolvedValueOnce({
199+
ok: true,
200+
status: 200,
201+
json: async () => validMetadata,
202+
});
203+
204+
const metadata = await discoverOAuthProtectedResourceMetadata("https://resource.example.com/path?param=value");
205+
expect(metadata).toEqual(validMetadata);
206+
const calls = mockFetch.mock.calls;
207+
expect(calls.length).toBe(1);
208+
const [url] = calls[0];
209+
expect(url.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource/path?param=value");
210+
});
211+
212+
it("falls back to root discovery when path-aware discovery returns 404", async () => {
213+
// First call (path-aware) returns 404
214+
mockFetch.mockResolvedValueOnce({
215+
ok: false,
216+
status: 404,
217+
});
218+
219+
// Second call (root fallback) succeeds
220+
mockFetch.mockResolvedValueOnce({
221+
ok: true,
222+
status: 200,
223+
json: async () => validMetadata,
224+
});
225+
226+
const metadata = await discoverOAuthProtectedResourceMetadata("https://resource.example.com/path/name");
227+
expect(metadata).toEqual(validMetadata);
228+
229+
const calls = mockFetch.mock.calls;
230+
expect(calls.length).toBe(2);
231+
232+
// First call should be path-aware
233+
const [firstUrl, firstOptions] = calls[0];
234+
expect(firstUrl.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource/path/name");
235+
expect(firstOptions.headers).toEqual({
236+
"MCP-Protocol-Version": LATEST_PROTOCOL_VERSION
237+
});
238+
239+
// Second call should be root fallback
240+
const [secondUrl, secondOptions] = calls[1];
241+
expect(secondUrl.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource");
242+
expect(secondOptions.headers).toEqual({
243+
"MCP-Protocol-Version": LATEST_PROTOCOL_VERSION
244+
});
245+
});
246+
247+
it("throws error when both path-aware and root discovery return 404", async () => {
248+
// First call (path-aware) returns 404
249+
mockFetch.mockResolvedValueOnce({
250+
ok: false,
251+
status: 404,
252+
});
253+
254+
// Second call (root fallback) also returns 404
255+
mockFetch.mockResolvedValueOnce({
256+
ok: false,
257+
status: 404,
258+
});
259+
260+
await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com/path/name"))
261+
.rejects.toThrow("Resource server does not implement OAuth 2.0 Protected Resource Metadata.");
262+
263+
const calls = mockFetch.mock.calls;
264+
expect(calls.length).toBe(2);
265+
});
266+
267+
it("does not fallback when the original URL is already at root path", async () => {
268+
// First call (path-aware for root) returns 404
269+
mockFetch.mockResolvedValueOnce({
270+
ok: false,
271+
status: 404,
272+
});
273+
274+
await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com/"))
275+
.rejects.toThrow("Resource server does not implement OAuth 2.0 Protected Resource Metadata.");
276+
277+
const calls = mockFetch.mock.calls;
278+
expect(calls.length).toBe(1); // Should not attempt fallback
279+
280+
const [url] = calls[0];
281+
expect(url.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource");
282+
});
283+
284+
it("does not fallback when the original URL has no path", async () => {
285+
// First call (path-aware for no path) returns 404
286+
mockFetch.mockResolvedValueOnce({
287+
ok: false,
288+
status: 404,
289+
});
290+
291+
await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com"))
292+
.rejects.toThrow("Resource server does not implement OAuth 2.0 Protected Resource Metadata.");
293+
294+
const calls = mockFetch.mock.calls;
295+
expect(calls.length).toBe(1); // Should not attempt fallback
296+
297+
const [url] = calls[0];
298+
expect(url.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource");
299+
});
300+
301+
it("falls back when path-aware discovery encounters CORS error", async () => {
302+
// First call (path-aware) fails with TypeError (CORS)
303+
mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError("CORS error")));
304+
305+
// Retry path-aware without headers (simulating CORS retry)
306+
mockFetch.mockResolvedValueOnce({
307+
ok: false,
308+
status: 404,
309+
});
310+
311+
// Second call (root fallback) succeeds
312+
mockFetch.mockResolvedValueOnce({
313+
ok: true,
314+
status: 200,
315+
json: async () => validMetadata,
316+
});
317+
318+
const metadata = await discoverOAuthProtectedResourceMetadata("https://resource.example.com/deep/path");
319+
expect(metadata).toEqual(validMetadata);
320+
321+
const calls = mockFetch.mock.calls;
322+
expect(calls.length).toBe(3);
323+
324+
// Final call should be root fallback
325+
const [lastUrl, lastOptions] = calls[2];
326+
expect(lastUrl.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource");
327+
expect(lastOptions.headers).toEqual({
328+
"MCP-Protocol-Version": LATEST_PROTOCOL_VERSION
329+
});
330+
});
331+
332+
it("does not fallback when resourceMetadataUrl is provided", async () => {
333+
// Call with explicit URL returns 404
334+
mockFetch.mockResolvedValueOnce({
335+
ok: false,
336+
status: 404,
337+
});
338+
339+
await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com/path", {
340+
resourceMetadataUrl: "https://custom.example.com/metadata"
341+
})).rejects.toThrow("Resource server does not implement OAuth 2.0 Protected Resource Metadata.");
342+
343+
const calls = mockFetch.mock.calls;
344+
expect(calls.length).toBe(1); // Should not attempt fallback when explicit URL is provided
345+
346+
const [url] = calls[0];
347+
expect(url.toString()).toBe("https://custom.example.com/metadata");
348+
});
181349
});
182350

183351
describe("discoverOAuthMetadata", () => {

‎src/client/auth.ts‎

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -235,12 +235,13 @@ export async function auth(
235235
serverUrl: string | URL;
236236
authorizationCode?: string;
237237
scope?: string;
238-
resourceMetadataUrl?: URL }): Promise<AuthResult> {
238+
resourceMetadataUrl?: URL
239+
}): Promise<AuthResult> {
239240

240241
let resourceMetadata: OAuthProtectedResourceMetadata | undefined;
241242
let authorizationServerUrl = serverUrl;
242243
try {
243-
resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, {resourceMetadataUrl});
244+
resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl });
244245
if (resourceMetadata.authorization_servers && resourceMetadata.authorization_servers.length > 0) {
245246
authorizationServerUrl = resourceMetadata.authorization_servers[0];
246247
}
@@ -327,7 +328,7 @@ export async function auth(
327328
return "REDIRECT";
328329
}
329330

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

333334
// If provider has custom validation, delegate to it
@@ -386,31 +387,16 @@ export async function discoverOAuthProtectedResourceMetadata(
386387
serverUrl: string | URL,
387388
opts?: { protocolVersion?: string, resourceMetadataUrl?: string | URL },
388389
): Promise<OAuthProtectedResourceMetadata> {
390+
const response = await discoverMetadataWithFallback(
391+
serverUrl,
392+
'oauth-protected-resource',
393+
{
394+
protocolVersion: opts?.protocolVersion,
395+
metadataUrl: opts?.resourceMetadataUrl,
396+
},
397+
);
389398

390-
let url: URL
391-
if (opts?.resourceMetadataUrl) {
392-
url = new URL(opts?.resourceMetadataUrl);
393-
} else {
394-
url = new URL("/.well-known/oauth-protected-resource", serverUrl);
395-
}
396-
397-
let response: Response;
398-
try {
399-
response = await fetch(url, {
400-
headers: {
401-
"MCP-Protocol-Version": opts?.protocolVersion ?? LATEST_PROTOCOL_VERSION
402-
}
403-
});
404-
} catch (error) {
405-
// CORS errors come back as TypeError
406-
if (error instanceof TypeError) {
407-
response = await fetch(url);
408-
} else {
409-
throw error;
410-
}
411-
}
412-
413-
if (response.status === 404) {
399+
if (!response || response.status === 404) {
414400
throw new Error(`Resource server does not implement OAuth 2.0 Protected Resource Metadata.`);
415401
}
416402

@@ -448,8 +434,8 @@ async function fetchWithCorsRetry(
448434
/**
449435
* Constructs the well-known path for OAuth metadata discovery
450436
*/
451-
function buildWellKnownPath(pathname: string): string {
452-
let wellKnownPath = `/.well-known/oauth-authorization-server${pathname}`;
437+
function buildWellKnownPath(wellKnownPrefix: string, pathname: string): string {
438+
let wellKnownPath = `/.well-known/${wellKnownPrefix}${pathname}`;
453439
if (pathname.endsWith('/')) {
454440
// Strip trailing slash from pathname to avoid double slashes
455441
wellKnownPath = wellKnownPath.slice(0, -1);
@@ -477,6 +463,38 @@ function shouldAttemptFallback(response: Response | undefined, pathname: string)
477463
return !response || response.status === 404 && pathname !== '/';
478464
}
479465

466+
/**
467+
* Generic function for discovering OAuth metadata with fallback support
468+
*/
469+
async function discoverMetadataWithFallback(
470+
serverUrl: string | URL,
471+
wellKnownType: 'oauth-authorization-server' | 'oauth-protected-resource',
472+
opts?: { protocolVersion?: string; metadataUrl?: string | URL },
473+
): Promise<Response | undefined> {
474+
const issuer = new URL(serverUrl);
475+
const protocolVersion = opts?.protocolVersion ?? LATEST_PROTOCOL_VERSION;
476+
477+
let url: URL;
478+
if (opts?.metadataUrl) {
479+
url = new URL(opts.metadataUrl);
480+
} else {
481+
// Try path-aware discovery first
482+
const wellKnownPath = buildWellKnownPath(wellKnownType, issuer.pathname);
483+
url = new URL(wellKnownPath, issuer);
484+
url.search = issuer.search;
485+
}
486+
487+
let response = await tryMetadataDiscovery(url, protocolVersion);
488+
489+
// If path-aware discovery fails with 404 and we're not already at root, try fallback to root discovery
490+
if (!opts?.metadataUrl && shouldAttemptFallback(response, issuer.pathname)) {
491+
const rootUrl = new URL(`/.well-known/${wellKnownType}`, issuer);
492+
response = await tryMetadataDiscovery(rootUrl, protocolVersion);
493+
}
494+
495+
return response;
496+
}
497+
480498
/**
481499
* Looks up RFC 8414 OAuth 2.0 Authorization Server Metadata.
482500
*
@@ -487,19 +505,12 @@ export async function discoverOAuthMetadata(
487505
authorizationServerUrl: string | URL,
488506
opts?: { protocolVersion?: string },
489507
): Promise<OAuthMetadata | undefined> {
490-
const issuer = new URL(authorizationServerUrl);
491-
const protocolVersion = opts?.protocolVersion ?? LATEST_PROTOCOL_VERSION;
492-
493-
// Try path-aware discovery first (RFC 8414 compliant)
494-
const wellKnownPath = buildWellKnownPath(issuer.pathname);
495-
const pathAwareUrl = new URL(wellKnownPath, issuer);
496-
let response = await tryMetadataDiscovery(pathAwareUrl, protocolVersion);
508+
const response = await discoverMetadataWithFallback(
509+
authorizationServerUrl,
510+
'oauth-authorization-server',
511+
opts,
512+
);
497513

498-
// If path-aware discovery fails with 404, try fallback to root discovery
499-
if (shouldAttemptFallback(response, issuer.pathname)) {
500-
const rootUrl = new URL("/.well-known/oauth-authorization-server", issuer);
501-
response = await tryMetadataDiscovery(rootUrl, protocolVersion);
502-
}
503514
if (!response || response.status === 404) {
504515
return undefined;
505516
}

0 commit comments

Comments
 (0)