From 5919633ad52a97e9a1008793c578c9ad1b519113 Mon Sep 17 00:00:00 2001 From: arjunkmrm Date: Tue, 16 Sep 2025 18:02:43 +0800 Subject: [PATCH 1/9] fix: add auth loop detection to prevent infinite OAuth retries --- src/client/streamableHttp.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index 12714ea44..b4085fd92 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -131,6 +131,7 @@ export class StreamableHTTPClientTransport implements Transport { private _sessionId?: string; private _reconnectionOptions: StreamableHTTPReconnectionOptions; private _protocolVersion?: string; + private _hasCompletedAuthFlow = false; // Circuit breaker: detect auth success followed by immediate 401 onclose?: () => void; onerror?: (error: Error) => void; @@ -437,6 +438,10 @@ export class StreamableHTTPClientTransport implements Transport { if (!response.ok) { if (response.status === 401 && this._authProvider) { + // Circuit breaker: Prevent infinite auth loops caused by server incorrectly rejecting valid credentials + if (this._hasCompletedAuthFlow) { + throw new UnauthorizedError("Authentication loop detected. The authorization server may be incorrectly rejecting valid credentials."); + } this._resourceMetadataUrl = extractResourceMetadataUrl(response); @@ -445,6 +450,8 @@ export class StreamableHTTPClientTransport implements Transport { throw new UnauthorizedError(); } + // Mark that we just completed auth flow + this._hasCompletedAuthFlow = true; // Purposely _not_ awaited, so we don't call onerror twice return this.send(message); } @@ -455,6 +462,9 @@ export class StreamableHTTPClientTransport implements Transport { ); } + // Reset auth loop flag on successful response + this._hasCompletedAuthFlow = false; + // If the response is 202 Accepted, there's no body to process if (response.status === 202) { // if the accepted notification is initialized, we start the SSE stream From ce2273c717d133ac50d984214f08a4de7b7c0a3d Mon Sep 17 00:00:00 2001 From: arjunkmrm Date: Tue, 16 Sep 2025 19:52:23 +0800 Subject: [PATCH 2/9] add test --- src/client/streamableHttp.test.ts | 65 ++++++++++++++++++++++++++++++- src/client/streamableHttp.ts | 6 +-- temp.md | 20 ++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 temp.md diff --git a/src/client/streamableHttp.test.ts b/src/client/streamableHttp.test.ts index fdd35ed3f..235afe9c1 100644 --- a/src/client/streamableHttp.test.ts +++ b/src/client/streamableHttp.test.ts @@ -1,4 +1,4 @@ -import { StartSSEOptions, StreamableHTTPClientTransport, StreamableHTTPReconnectionOptions } from "./streamableHttp.js"; +import { StartSSEOptions, StreamableHTTPClientTransport, StreamableHTTPReconnectionOptions, StreamableHTTPError } from "./streamableHttp.js"; import { OAuthClientProvider, UnauthorizedError } from "./auth.js"; import { JSONRPCMessage, JSONRPCRequest } from "../types.js"; import { InvalidClientError, InvalidGrantError, UnauthorizedClientError } from "../server/auth/errors.js"; @@ -1001,4 +1001,67 @@ describe("StreamableHTTPClientTransport", () => { expect(global.fetch).not.toHaveBeenCalled(); }); }); + + it("prevents infinite auth loop when server returns 401 after successful auth", async () => { + const message: JSONRPCMessage = { + jsonrpc: "2.0", + method: "test", + params: {}, + id: "test-id" + }; + + // Mock provider with refresh token to enable token refresh flow + mockAuthProvider.tokens.mockResolvedValue({ + access_token: "test-token", + token_type: "Bearer", + refresh_token: "refresh-token", + }); + + const unauthedResponse = { + ok: false, + status: 401, + statusText: "Unauthorized", + headers: new Headers() + }; + + (global.fetch as jest.Mock) + // First request - 401, triggers auth flow + .mockResolvedValueOnce(unauthedResponse) + // Resource discovery, path aware + .mockResolvedValueOnce(unauthedResponse) + // Resource discovery, root + .mockResolvedValueOnce(unauthedResponse) + // OAuth metadata discovery + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ + issuer: "http://localhost:1234", + authorization_endpoint: "http://localhost:1234/authorize", + token_endpoint: "http://localhost:1234/token", + response_types_supported: ["code"], + code_challenge_methods_supported: ["S256"], + }), + }) + // Token refresh succeeds + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ + access_token: "new-access-token", + token_type: "Bearer", + expires_in: 3600, + }), + }) + // Retry the original request - still 401 (broken server) + .mockResolvedValueOnce(unauthedResponse); + + await expect(transport.send(message)).rejects.toThrow("Server returned 401 after successful authentication"); + expect(mockAuthProvider.saveTokens).toHaveBeenCalledWith({ + access_token: "new-access-token", + token_type: "Bearer", + expires_in: 3600, + refresh_token: "refresh-token", // Refresh token is preserved + }); + }); }); diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index b4085fd92..61b4c1fa2 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -438,9 +438,9 @@ export class StreamableHTTPClientTransport implements Transport { if (!response.ok) { if (response.status === 401 && this._authProvider) { - // Circuit breaker: Prevent infinite auth loops caused by server incorrectly rejecting valid credentials + // Prevent infinite auth loop when server returns 401 after successful auth if (this._hasCompletedAuthFlow) { - throw new UnauthorizedError("Authentication loop detected. The authorization server may be incorrectly rejecting valid credentials."); + throw new StreamableHTTPError(401, "Server returned 401 after successful authentication"); } this._resourceMetadataUrl = extractResourceMetadataUrl(response); @@ -450,7 +450,7 @@ export class StreamableHTTPClientTransport implements Transport { throw new UnauthorizedError(); } - // Mark that we just completed auth flow + // Mark that we completed auth flow this._hasCompletedAuthFlow = true; // Purposely _not_ awaited, so we don't call onerror twice return this.send(message); diff --git a/temp.md b/temp.md new file mode 100644 index 000000000..b0005aa2a --- /dev/null +++ b/temp.md @@ -0,0 +1,20 @@ +## Motivation and Context +Fixes infinite OAuth loops when authorization servers reject credentials and throw 401 again immediately after successful authentication. The transport retries auth infinitely when the server returns 401 for already-authorized requests. + +## How Has This Been Tested? +Tested with MCP servers that return 401 after successful OAuth completion. Verified circuit breaker stops infinite loops while allowing legitimate auth retries. + +## Breaking Changes +None. Defensive fix that only affects infinite loop edge cases. + +## Types of changes +- [x] Bug fix (non-breaking change which fixes an issue) + +## Checklist +- [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) +- [x] My code follows the repository's style guidelines +- [x] New and existing tests pass locally +- [x] I have added appropriate error handling + +## Additional context +Uses per-transport boolean flag to detect AUTHORIZED → immediate 401 pattern. Throws clear error instead of infinite recursion. \ No newline at end of file From 19ccef791306fe8c1413fe446f9a854b141a0c91 Mon Sep 17 00:00:00 2001 From: arjunkmrm Date: Tue, 16 Sep 2025 19:58:44 +0800 Subject: [PATCH 3/9] update function comments for clarity --- src/client/streamableHttp.test.ts | 2 +- src/client/streamableHttp.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/streamableHttp.test.ts b/src/client/streamableHttp.test.ts index 235afe9c1..3783d38c0 100644 --- a/src/client/streamableHttp.test.ts +++ b/src/client/streamableHttp.test.ts @@ -1002,7 +1002,7 @@ describe("StreamableHTTPClientTransport", () => { }); }); - it("prevents infinite auth loop when server returns 401 after successful auth", async () => { + it("prevents infinite auth loops when server returns 401 after successful auth", async () => { const message: JSONRPCMessage = { jsonrpc: "2.0", method: "test", diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index 61b4c1fa2..beb97a1c0 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -438,7 +438,7 @@ export class StreamableHTTPClientTransport implements Transport { if (!response.ok) { if (response.status === 401 && this._authProvider) { - // Prevent infinite auth loop when server returns 401 after successful auth + // Prevent infinite auth loops when server returns 401 after successful auth if (this._hasCompletedAuthFlow) { throw new StreamableHTTPError(401, "Server returned 401 after successful authentication"); } From 98f43b36aaad6f0fb13c805f6b2eeaf6c6a771c0 Mon Sep 17 00:00:00 2001 From: arjunkmrm Date: Tue, 16 Sep 2025 19:59:03 +0800 Subject: [PATCH 4/9] rm temporary file --- temp.md | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 temp.md diff --git a/temp.md b/temp.md deleted file mode 100644 index b0005aa2a..000000000 --- a/temp.md +++ /dev/null @@ -1,20 +0,0 @@ -## Motivation and Context -Fixes infinite OAuth loops when authorization servers reject credentials and throw 401 again immediately after successful authentication. The transport retries auth infinitely when the server returns 401 for already-authorized requests. - -## How Has This Been Tested? -Tested with MCP servers that return 401 after successful OAuth completion. Verified circuit breaker stops infinite loops while allowing legitimate auth retries. - -## Breaking Changes -None. Defensive fix that only affects infinite loop edge cases. - -## Types of changes -- [x] Bug fix (non-breaking change which fixes an issue) - -## Checklist -- [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) -- [x] My code follows the repository's style guidelines -- [x] New and existing tests pass locally -- [x] I have added appropriate error handling - -## Additional context -Uses per-transport boolean flag to detect AUTHORIZED → immediate 401 pattern. Throws clear error instead of infinite recursion. \ No newline at end of file From 70107537f46a209f511ab7ce5f32c7d1c87feab8 Mon Sep 17 00:00:00 2001 From: arjunkmrm Date: Tue, 16 Sep 2025 21:56:48 +0800 Subject: [PATCH 5/9] wrap test in describe block --- src/client/streamableHttp.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client/streamableHttp.test.ts b/src/client/streamableHttp.test.ts index 3783d38c0..af5cd8f16 100644 --- a/src/client/streamableHttp.test.ts +++ b/src/client/streamableHttp.test.ts @@ -1,4 +1,4 @@ -import { StartSSEOptions, StreamableHTTPClientTransport, StreamableHTTPReconnectionOptions, StreamableHTTPError } from "./streamableHttp.js"; +import { StartSSEOptions, StreamableHTTPClientTransport, StreamableHTTPReconnectionOptions } from "./streamableHttp.js"; import { OAuthClientProvider, UnauthorizedError } from "./auth.js"; import { JSONRPCMessage, JSONRPCRequest } from "../types.js"; import { InvalidClientError, InvalidGrantError, UnauthorizedClientError } from "../server/auth/errors.js"; @@ -1002,7 +1002,8 @@ describe("StreamableHTTPClientTransport", () => { }); }); - it("prevents infinite auth loops when server returns 401 after successful auth", async () => { + describe("prevent infinite recursion when server returns 401 after successful auth", () => { + it("should throw error when server returns 401 after successful auth", async () => { const message: JSONRPCMessage = { jsonrpc: "2.0", method: "test", @@ -1064,4 +1065,5 @@ describe("StreamableHTTPClientTransport", () => { refresh_token: "refresh-token", // Refresh token is preserved }); }); + }); }); From 39e7ebb8aa7eb47b53a5f2d616d7f4bb18745171 Mon Sep 17 00:00:00 2001 From: arjunkmrm Date: Tue, 16 Sep 2025 21:57:08 +0800 Subject: [PATCH 6/9] update comment --- src/client/streamableHttp.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index beb97a1c0..dcd5d62ce 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -438,7 +438,7 @@ export class StreamableHTTPClientTransport implements Transport { if (!response.ok) { if (response.status === 401 && this._authProvider) { - // Prevent infinite auth loops when server returns 401 after successful auth + // Prevent infinite recursion when server returns 401 after successful auth if (this._hasCompletedAuthFlow) { throw new StreamableHTTPError(401, "Server returned 401 after successful authentication"); } From c342dacb5ed55a0366fd2512bb642d1ceaa2ed87 Mon Sep 17 00:00:00 2001 From: Vinicius Costa Date: Mon, 22 Sep 2025 09:31:40 -0300 Subject: [PATCH 7/9] Updates the sampling code example in the README (#958) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index cee7eb855..43b62ac60 100644 --- a/README.md +++ b/README.md @@ -437,7 +437,7 @@ mcpServer.registerTool( async function main() { const transport = new StdioServerTransport(); await mcpServer.connect(transport); - console.log("MCP server is running..."); + console.error("MCP server is running..."); } main().catch((error) => { From 9841a6cf2959d0f361ac5eee6e1a2e0f3515b943 Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt <2644648+TylerLeonhardt@users.noreply.github.com> Date: Mon, 22 Sep 2025 07:29:39 -0700 Subject: [PATCH 8/9] Use redirect Uri passed in in `demoInMemoryOAuthProvider` (#931) --- .../server/demoInMemoryOAuthProvider.test.ts | 298 ++++++++++++++++++ .../server/demoInMemoryOAuthProvider.ts | 6 +- 2 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 src/examples/server/demoInMemoryOAuthProvider.test.ts diff --git a/src/examples/server/demoInMemoryOAuthProvider.test.ts b/src/examples/server/demoInMemoryOAuthProvider.test.ts new file mode 100644 index 000000000..cb99e1ffb --- /dev/null +++ b/src/examples/server/demoInMemoryOAuthProvider.test.ts @@ -0,0 +1,298 @@ +import { Response } from 'express'; +import { DemoInMemoryAuthProvider, DemoInMemoryClientsStore } from './demoInMemoryOAuthProvider.js'; +import { AuthorizationParams } from '../../server/auth/provider.js'; +import { OAuthClientInformationFull } from '../../shared/auth.js'; +import { InvalidRequestError } from '../../server/auth/errors.js'; + +describe('DemoInMemoryAuthProvider', () => { + let provider: DemoInMemoryAuthProvider; + let mockResponse: Response & { getRedirectUrl: () => string }; + + const createMockResponse = (): Response & { getRedirectUrl: () => string } => { + let capturedRedirectUrl: string | undefined; + + const mockRedirect = jest.fn().mockImplementation((url: string | number, status?: number) => { + if (typeof url === 'string') { + capturedRedirectUrl = url; + } else if (typeof status === 'string') { + capturedRedirectUrl = status; + } + return mockResponse; + }); + + const mockResponse = { + redirect: mockRedirect, + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + send: jest.fn().mockReturnThis(), + getRedirectUrl: () => { + if (capturedRedirectUrl === undefined) { + throw new Error('No redirect URL was captured. Ensure redirect() was called first.'); + } + return capturedRedirectUrl; + }, + } as unknown as Response & { getRedirectUrl: () => string }; + + return mockResponse; + }; + + beforeEach(() => { + provider = new DemoInMemoryAuthProvider(); + mockResponse = createMockResponse(); + }); + + describe('authorize', () => { + const validClient: OAuthClientInformationFull = { + client_id: 'test-client', + client_secret: 'test-secret', + redirect_uris: [ + 'https://example.com/callback', + 'https://example.com/callback2' + ], + scope: 'test-scope' + }; + + it('should redirect to the requested redirect_uri when valid', async () => { + const params: AuthorizationParams = { + redirectUri: 'https://example.com/callback', + state: 'test-state', + codeChallenge: 'test-challenge', + scopes: ['test-scope'] + }; + + await provider.authorize(validClient, params, mockResponse); + + expect(mockResponse.redirect).toHaveBeenCalled(); + expect(mockResponse.getRedirectUrl()).toBeDefined(); + + const url = new URL(mockResponse.getRedirectUrl()); + expect(url.origin + url.pathname).toBe('https://example.com/callback'); + expect(url.searchParams.get('state')).toBe('test-state'); + expect(url.searchParams.has('code')).toBe(true); + }); + + it('should throw InvalidRequestError for unregistered redirect_uri', async () => { + const params: AuthorizationParams = { + redirectUri: 'https://evil.com/callback', + state: 'test-state', + codeChallenge: 'test-challenge', + scopes: ['test-scope'] + }; + + await expect( + provider.authorize(validClient, params, mockResponse) + ).rejects.toThrow(InvalidRequestError); + + await expect( + provider.authorize(validClient, params, mockResponse) + ).rejects.toThrow('Unregistered redirect_uri'); + + expect(mockResponse.redirect).not.toHaveBeenCalled(); + }); + + it('should generate unique authorization codes for multiple requests', async () => { + const params1: AuthorizationParams = { + redirectUri: 'https://example.com/callback', + state: 'state-1', + codeChallenge: 'challenge-1', + scopes: ['test-scope'] + }; + + const params2: AuthorizationParams = { + redirectUri: 'https://example.com/callback', + state: 'state-2', + codeChallenge: 'challenge-2', + scopes: ['test-scope'] + }; + + await provider.authorize(validClient, params1, mockResponse); + const firstRedirectUrl = mockResponse.getRedirectUrl(); + const firstCode = new URL(firstRedirectUrl).searchParams.get('code'); + + // Reset the mock for the second call + mockResponse = createMockResponse(); + await provider.authorize(validClient, params2, mockResponse); + const secondRedirectUrl = mockResponse.getRedirectUrl(); + const secondCode = new URL(secondRedirectUrl).searchParams.get('code'); + + expect(firstCode).toBeDefined(); + expect(secondCode).toBeDefined(); + expect(firstCode).not.toBe(secondCode); + }); + + it('should handle params without state', async () => { + const params: AuthorizationParams = { + redirectUri: 'https://example.com/callback', + codeChallenge: 'test-challenge', + scopes: ['test-scope'] + }; + + await provider.authorize(validClient, params, mockResponse); + + expect(mockResponse.redirect).toHaveBeenCalled(); + expect(mockResponse.getRedirectUrl()).toBeDefined(); + + const url = new URL(mockResponse.getRedirectUrl()); + expect(url.searchParams.has('state')).toBe(false); + expect(url.searchParams.has('code')).toBe(true); + }); + }); + + describe('challengeForAuthorizationCode', () => { + const validClient: OAuthClientInformationFull = { + client_id: 'test-client', + client_secret: 'test-secret', + redirect_uris: ['https://example.com/callback'], + scope: 'test-scope' + }; + + it('should return the code challenge for a valid authorization code', async () => { + const params: AuthorizationParams = { + redirectUri: 'https://example.com/callback', + state: 'test-state', + codeChallenge: 'test-challenge-value', + scopes: ['test-scope'] + }; + + await provider.authorize(validClient, params, mockResponse); + const code = new URL(mockResponse.getRedirectUrl()).searchParams.get('code')!; + + const challenge = await provider.challengeForAuthorizationCode(validClient, code); + expect(challenge).toBe('test-challenge-value'); + }); + + it('should throw error for invalid authorization code', async () => { + await expect( + provider.challengeForAuthorizationCode(validClient, 'invalid-code') + ).rejects.toThrow('Invalid authorization code'); + }); + }); + + describe('exchangeAuthorizationCode', () => { + const validClient: OAuthClientInformationFull = { + client_id: 'test-client', + client_secret: 'test-secret', + redirect_uris: ['https://example.com/callback'], + scope: 'test-scope' + }; + + it('should exchange valid authorization code for tokens', async () => { + const params: AuthorizationParams = { + redirectUri: 'https://example.com/callback', + state: 'test-state', + codeChallenge: 'test-challenge', + scopes: ['test-scope', 'other-scope'] + }; + + await provider.authorize(validClient, params, mockResponse); + const code = new URL(mockResponse.getRedirectUrl()).searchParams.get('code')!; + + const tokens = await provider.exchangeAuthorizationCode(validClient, code); + + expect(tokens).toEqual({ + access_token: expect.any(String), + token_type: 'bearer', + expires_in: 3600, + scope: 'test-scope other-scope' + }); + }); + + it('should throw error for invalid authorization code', async () => { + await expect( + provider.exchangeAuthorizationCode(validClient, 'invalid-code') + ).rejects.toThrow('Invalid authorization code'); + }); + + it('should throw error when client_id does not match', async () => { + const params: AuthorizationParams = { + redirectUri: 'https://example.com/callback', + state: 'test-state', + codeChallenge: 'test-challenge', + scopes: ['test-scope'] + }; + + await provider.authorize(validClient, params, mockResponse); + const code = new URL(mockResponse.getRedirectUrl()).searchParams.get('code')!; + + const differentClient: OAuthClientInformationFull = { + client_id: 'different-client', + client_secret: 'different-secret', + redirect_uris: ['https://example.com/callback'], + scope: 'test-scope' + }; + + await expect( + provider.exchangeAuthorizationCode(differentClient, code) + ).rejects.toThrow('Authorization code was not issued to this client'); + }); + + it('should delete authorization code after successful exchange', async () => { + const params: AuthorizationParams = { + redirectUri: 'https://example.com/callback', + state: 'test-state', + codeChallenge: 'test-challenge', + scopes: ['test-scope'] + }; + + await provider.authorize(validClient, params, mockResponse); + const code = new URL(mockResponse.getRedirectUrl()).searchParams.get('code')!; + + // First exchange should succeed + await provider.exchangeAuthorizationCode(validClient, code); + + // Second exchange should fail + await expect( + provider.exchangeAuthorizationCode(validClient, code) + ).rejects.toThrow('Invalid authorization code'); + }); + + it('should validate resource when validateResource is provided', async () => { + const validateResource = jest.fn().mockReturnValue(false); + const strictProvider = new DemoInMemoryAuthProvider(validateResource); + + const params: AuthorizationParams = { + redirectUri: 'https://example.com/callback', + state: 'test-state', + codeChallenge: 'test-challenge', + scopes: ['test-scope'], + resource: new URL('https://invalid-resource.com') + }; + + await strictProvider.authorize(validClient, params, mockResponse); + const code = new URL(mockResponse.getRedirectUrl()).searchParams.get('code')!; + + await expect( + strictProvider.exchangeAuthorizationCode(validClient, code) + ).rejects.toThrow('Invalid resource: https://invalid-resource.com/'); + + expect(validateResource).toHaveBeenCalledWith(params.resource); + }); + }); + + describe('DemoInMemoryClientsStore', () => { + let store: DemoInMemoryClientsStore; + + beforeEach(() => { + store = new DemoInMemoryClientsStore(); + }); + + it('should register and retrieve client', async () => { + const client: OAuthClientInformationFull = { + client_id: 'test-client', + client_secret: 'test-secret', + redirect_uris: ['https://example.com/callback'], + scope: 'test-scope' + }; + + await store.registerClient(client); + const retrieved = await store.getClient('test-client'); + + expect(retrieved).toEqual(client); + }); + + it('should return undefined for non-existent client', async () => { + const retrieved = await store.getClient('non-existent'); + expect(retrieved).toBeUndefined(); + }); + }); +}); \ No newline at end of file diff --git a/src/examples/server/demoInMemoryOAuthProvider.ts b/src/examples/server/demoInMemoryOAuthProvider.ts index c83748d35..995fa98b4 100644 --- a/src/examples/server/demoInMemoryOAuthProvider.ts +++ b/src/examples/server/demoInMemoryOAuthProvider.ts @@ -6,6 +6,7 @@ import express, { Request, Response } from "express"; import { AuthInfo } from '../../server/auth/types.js'; import { createOAuthMetadata, mcpAuthRouter } from '../../server/auth/router.js'; import { resourceUrlFromServerUrl } from '../../shared/auth-utils.js'; +import { InvalidRequestError } from '../../server/auth/errors.js'; export class DemoInMemoryClientsStore implements OAuthRegisteredClientsStore { @@ -57,7 +58,10 @@ export class DemoInMemoryAuthProvider implements OAuthServerProvider { params }); - const targetUrl = new URL(client.redirect_uris[0]); + if (!client.redirect_uris.includes(params.redirectUri)) { + throw new InvalidRequestError("Unregistered redirect_uri"); + } + const targetUrl = new URL(params.redirectUri); targetUrl.search = searchParams.toString(); res.redirect(targetUrl.toString()); } From 1d475bb3f75674a46d81dba881ea743a763cbc12 Mon Sep 17 00:00:00 2001 From: "Blust.AI" <159488814+blustAI@users.noreply.github.com> Date: Mon, 22 Sep 2025 07:32:29 -0700 Subject: [PATCH 9/9] fix(auth-router): correct Protected Resource Metadata for pathful RS and add explicit resourceServerUrl (RFC 9728) (#858) Co-authored-by: Eugene --- src/server/auth/router.ts | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/server/auth/router.ts b/src/server/auth/router.ts index a06bf73a1..727699e0c 100644 --- a/src/server/auth/router.ts +++ b/src/server/auth/router.ts @@ -41,6 +41,12 @@ export type AuthRouterOptions = { */ resourceName?: string; + /** + * The URL of the protected resource (RS) whose metadata we advertise. + * If not provided, falls back to `baseUrl` and then to `issuerUrl` (AS=RS). + */ + resourceServerUrl?: URL; + // Individual options per route authorizationOptions?: Omit; clientRegistrationOptions?: Omit; @@ -130,8 +136,8 @@ export function mcpAuthRouter(options: AuthRouterOptions): RequestHandler { router.use(mcpAuthMetadataRouter({ oauthMetadata, - // This router is used for AS+RS combo's, so the issuer is also the resource server - resourceServerUrl: new URL(oauthMetadata.issuer), + // Prefer explicit RS; otherwise fall back to AS baseUrl, then to issuer (back-compat) + resourceServerUrl: options.resourceServerUrl ?? options.baseUrl ?? new URL(oauthMetadata.issuer), serviceDocumentationUrl: options.serviceDocumentationUrl, scopesSupported: options.scopesSupported, resourceName: options.resourceName @@ -185,7 +191,7 @@ export type AuthMetadataOptions = { resourceName?: string; } -export function mcpAuthMetadataRouter(options: AuthMetadataOptions) { +export function mcpAuthMetadataRouter(options: AuthMetadataOptions): express.Router { checkIssuerUrl(new URL(options.oauthMetadata.issuer)); const router = express.Router(); @@ -202,9 +208,11 @@ export function mcpAuthMetadataRouter(options: AuthMetadataOptions) { resource_documentation: options.serviceDocumentationUrl?.href, }; - router.use("/.well-known/oauth-protected-resource", metadataHandler(protectedResourceMetadata)); + // Serve PRM at the path-specific URL per RFC 9728 + const rsPath = new URL(options.resourceServerUrl.href).pathname; + router.use(`/.well-known/oauth-protected-resource${rsPath === '/' ? '' : rsPath}`, metadataHandler(protectedResourceMetadata)); - // Always add this for backwards compatibility + // Always add this for OAuth Authorization Server metadata per RFC 8414 router.use("/.well-known/oauth-authorization-server", metadataHandler(options.oauthMetadata)); return router; @@ -219,8 +227,10 @@ export function mcpAuthMetadataRouter(options: AuthMetadataOptions) { * * @example * getOAuthProtectedResourceMetadataUrl(new URL('https://api.example.com/mcp')) - * // Returns: 'https://api.example.com/.well-known/oauth-protected-resource' + * // Returns: 'https://api.example.com/.well-known/oauth-protected-resource/mcp' */ export function getOAuthProtectedResourceMetadataUrl(serverUrl: URL): string { - return new URL('/.well-known/oauth-protected-resource', serverUrl).href; + const u = new URL(serverUrl.href); + const rsPath = u.pathname && u.pathname !== '/' ? u.pathname : ''; + return new URL(`/.well-known/oauth-protected-resource${rsPath}`, u).href; }