Skip to content

Commit ea8caa9

Browse files
ochafikochafik
committed
Merge branch 'main' into ochafik/fix-path-as
2 parents de1ade2 + 442e13b commit ea8caa9

File tree

2 files changed

+220
-38
lines changed

2 files changed

+220
-38
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 & 38 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
}
@@ -329,7 +330,7 @@ export async function auth(
329330
return "REDIRECT";
330331
}
331332

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

335336
// If provider has custom validation, delegate to it
@@ -388,31 +389,16 @@ export async function discoverOAuthProtectedResourceMetadata(
388389
serverUrl: string | URL,
389390
opts?: { protocolVersion?: string, resourceMetadataUrl?: string | URL },
390391
): Promise<OAuthProtectedResourceMetadata> {
392+
const response = await discoverMetadataWithFallback(
393+
serverUrl,
394+
'oauth-protected-resource',
395+
{
396+
protocolVersion: opts?.protocolVersion,
397+
metadataUrl: opts?.resourceMetadataUrl,
398+
},
399+
);
391400

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

@@ -450,8 +436,8 @@ async function fetchWithCorsRetry(
450436
/**
451437
* Constructs the well-known path for OAuth metadata discovery
452438
*/
453-
function buildWellKnownPath(pathname: string): string {
454-
let wellKnownPath = `/.well-known/oauth-authorization-server${pathname}`;
439+
function buildWellKnownPath(wellKnownPrefix: string, pathname: string): string {
440+
let wellKnownPath = `/.well-known/${wellKnownPrefix}${pathname}`;
455441
if (pathname.endsWith('/')) {
456442
// Strip trailing slash from pathname to avoid double slashes
457443
wellKnownPath = wellKnownPath.slice(0, -1);
@@ -479,6 +465,38 @@ function shouldAttemptFallback(response: Response | undefined, pathname: string)
479465
return !response || response.status === 404 && pathname !== '/';
480466
}
481467

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

509-
// Try path-aware discovery first (RFC 8414 compliant)
510-
const wellKnownPath = buildWellKnownPath(issuer.pathname);
511-
const pathAwareUrl = new URL(wellKnownPath, authorizationServerUrl);
512-
let response = await tryMetadataDiscovery(pathAwareUrl, protocolVersion);
527+
const response = await discoverMetadataWithFallback(
528+
authorizationServerUrl,
529+
'oauth-authorization-server',
530+
{protocolVersion},
531+
);
513532

514-
// If path-aware discovery fails with 404, try fallback to root discovery
515-
if (shouldAttemptFallback(response, issuer.pathname)) {
516-
const rootUrl = new URL("/.well-known/oauth-authorization-server", issuer);
517-
response = await tryMetadataDiscovery(rootUrl, protocolVersion);
518-
}
519533
if (!response || response.status === 404) {
520534
return undefined;
521535
}

0 commit comments

Comments
 (0)