From 9c05d2247af910b029729f167a6341eb9483da71 Mon Sep 17 00:00:00 2001 From: Kauan Guesser Date: Thu, 3 Sep 2026 18:49:23 -0300 Subject: [PATCH] fix(client): accept OAuth servers without response issuer support --- .changeset/calm-badgers-compare.md | 7 ++ docs/security-and-oauth.md | 6 +- packages/mcp-client/README.md | 12 +-- packages/mcp-client/src/oauth/bootstrap.ts | 7 +- packages/mcp-client/src/oauth/protocol.ts | 21 +++-- .../mcp-client/tests/oauth-bootstrap.test.ts | 22 ++++- .../mcp-client/tests/oauth-protocol.test.ts | 85 ++++++++++++++++++- 7 files changed, 131 insertions(+), 29 deletions(-) create mode 100644 .changeset/calm-badgers-compare.md diff --git a/.changeset/calm-badgers-compare.md b/.changeset/calm-badgers-compare.md new file mode 100644 index 0000000..4571586 --- /dev/null +++ b/.changeset/calm-badgers-compare.md @@ -0,0 +1,7 @@ +--- +"@nestm/mcp-client": patch +--- + +Accept OAuth authorization servers that do not advertise RFC 9207 response issuer support. The +callback still requires an exact `iss` when support is advertised and rejects every mismatched +`iss` value when one is returned. diff --git a/docs/security-and-oauth.md b/docs/security-and-oauth.md index 386281a..1706ad7 100644 --- a/docs/security-and-oauth.md +++ b/docs/security-and-oauth.md @@ -28,8 +28,8 @@ That pass-through is useful for application-owned service identities. A multi-te shared or per-user credentials should use `@nestm/mcp-client/oauth` instead. Its strict facade accepts only pre-provisioned clients, performs RFC 9728 and RFC 8414 discovery in two separately policy-checked phases, requires exact configured resource and issuer values, requires PKCE S256, -requires RFC 9207 authorization-response issuer identification, and pins the discovered endpoints -into the single-use callback transaction. It never calls the SDK full-flow orchestrator, never +validates RFC 9207 authorization-response issuer identification when advertised or returned, and +pins the discovered endpoints into the single-use callback transaction. It never calls the SDK full-flow orchestrator, never performs Dynamic Client Registration, and never rediscovers a token endpoint while redeeming an authorization code. @@ -454,7 +454,7 @@ NestM lifecycle events omit inputs and outputs, and server/gateway operation pri - [ ] Decorated Nest capabilities use `handlerAuthorization`; HTTP-exchange middleware is not treated as a stdio policy. - [ ] Gateway downstream and upstream credentials are separate. - [ ] OAuth pending state is single-use, authenticated-encrypted, session-bound, and pinned to exact issuer, resource, endpoints, client, owner, and revision. -- [ ] Interactive OAuth authorities advertise RFC 9207 and every callback carries the exact expected `iss` value. +- [ ] When an interactive OAuth authority advertises RFC 9207, every callback carries the exact expected `iss` value; any unadvertised but present `iss` is also compared exactly. - [ ] OAuth discovery and token calls use a redirect-denying, DNS-pinned, size/time-bounded fetch plus fail-closed endpoint policy. - [ ] Refresh rotation uses a durable pre-dispatch claim plus exact-revision commit; stale claims never reactivate, and terminal invalidation evicts credential-bound client leases. - [ ] Stdio definitions are trusted, allowlisted, and sandboxed. diff --git a/packages/mcp-client/README.md b/packages/mcp-client/README.md index 0611de7..97067ce 100644 --- a/packages/mcp-client/README.md +++ b/packages/mcp-client/README.md @@ -132,8 +132,9 @@ token storage remain host-application responsibilities. Multi-tenant hosts should use the dedicated `@nestm/mcp-client/oauth` surface instead of allowing the transport to start an interactive SDK flow. The strict facade only accepts clients that were provisioned out of band. It has no Dynamic Client Registration operation, requires exact resource -and issuer bindings, requires PKCE S256 plus the RFC 9207 authorization-response issuer parameter, -and re-checks every discovered or credentialed endpoint through host policy. +and issuer bindings plus PKCE S256, validates the RFC 9207 authorization-response issuer parameter +when advertised or returned, and re-checks every discovered or credentialed endpoint through host +policy. ```ts import { McpClientOAuthProtocol } from "@nestm/mcp-client/oauth"; @@ -170,9 +171,10 @@ redirectUser(started.authorizationUrl); On callback, parse the parameters, derive the state lookup digest, and atomically take the pending transaction before calling `exchangeAuthorization`. A consumed transaction stays consumed even when token exchange has an ambiguous network result. The facade validates state lifetime, callback -issuer, client identity, and the authority/endpoints pinned before redirect; callbacks without the -exact RFC 9207 `iss` value fail closed, and the facade never rediscovers an endpoint while redeeming -a code. +issuer, client identity, and the authority/endpoints pinned before redirect. When the authority +advertises RFC 9207, callbacks without the exact `iss` value fail closed. An unadvertised but +present `iss` is still compared exactly; omission is accepted only when the authority did not +advertise it. The facade never rediscovers an endpoint while redeeming a code. The subpath also provides a revisioned credential-store port and `McpClientOAuthRefreshCoordinator`. Refreshes for one opaque identity and exact revision share one diff --git a/packages/mcp-client/src/oauth/bootstrap.ts b/packages/mcp-client/src/oauth/bootstrap.ts index 71a6ca2..6e60392 100644 --- a/packages/mcp-client/src/oauth/bootstrap.ts +++ b/packages/mcp-client/src/oauth/bootstrap.ts @@ -132,6 +132,7 @@ export const McpClientOAuthStrictCompatibilityIssue = { TokenEndpointAuthenticationUnsupported: "token_endpoint_authentication_unsupported", AuthorizationCodeGrantUnsupported: "authorization_code_grant_unsupported", QueryResponseModeUnsupported: "query_response_mode_unsupported", + /** @deprecated RFC 9207 support is optional and this issue is no longer emitted. */ AuthorizationResponseIssuerUnsupported: "authorization_response_issuer_unsupported", } as const; @@ -573,9 +574,6 @@ function strictCompatibilityIssues( ) { issues.push(McpClientOAuthStrictCompatibilityIssue.QueryResponseModeUnsupported); } - if (metadata.authorization_response_iss_parameter_supported !== true) { - issues.push(McpClientOAuthStrictCompatibilityIssue.AuthorizationResponseIssuerUnsupported); - } return Object.freeze(issues); } @@ -615,7 +613,8 @@ function createStrictAuthority(input: { McpClientOAuthBootstrapErrorCode.AuthorityInvalid, ), }), - authorizationResponseIssuerParameterSupported: true, + authorizationResponseIssuerParameterSupported: + input.metadata.authorization_response_iss_parameter_supported === true, }); } diff --git a/packages/mcp-client/src/oauth/protocol.ts b/packages/mcp-client/src/oauth/protocol.ts index c42c36f..7548796 100644 --- a/packages/mcp-client/src/oauth/protocol.ts +++ b/packages/mcp-client/src/oauth/protocol.ts @@ -150,6 +150,7 @@ export interface McpClientOAuthAuthority { readonly grantTypesSupported?: readonly string[]; readonly resourceScopesSupported?: readonly string[]; readonly authorizationScopesSupported?: readonly string[]; + /** Whether RFC 9207 requires an `iss` parameter in the authorization response. */ readonly authorizationResponseIssuerParameterSupported: boolean; } @@ -640,7 +641,6 @@ function createAuthority(input: { !responseTypesSupported.includes("code") || !codeChallengeMethodsSupported.includes("S256") || tokenEndpointAuthMethodsSupported.length === 0 || - !isLiteralTrue(input.metadata.authorization_response_iss_parameter_supported) || (input.metadata.grant_types_supported !== undefined && !input.metadata.grant_types_supported.includes("authorization_code")) || (input.metadata.response_modes_supported !== undefined && @@ -671,7 +671,7 @@ function createAuthority(input: { authorizationScopesSupported: normalizeScopeMetadataList(input.metadata.scopes_supported), }), authorizationResponseIssuerParameterSupported: - input.metadata.authorization_response_iss_parameter_supported, + input.metadata.authorization_response_iss_parameter_supported === true, }); } @@ -709,7 +709,7 @@ function normalizeAuthority(authority: McpClientOAuthAuthority): McpClientOAuthA authority.authorizationResponseIssuerParameterSupported, }); if ( - !isLiteralTrue(normalized.authorizationResponseIssuerParameterSupported) || + typeof normalized.authorizationResponseIssuerParameterSupported !== "boolean" || !normalized.responseTypesSupported.includes("code") || !normalized.codeChallengeMethodsSupported.includes("S256") || normalized.tokenEndpointAuthMethodsSupported.length === 0 || @@ -1045,10 +1045,13 @@ function assertCallbackIssuer( callbackIssuer: string | undefined, authority: McpClientOAuthAuthority, ): void { - if ( - !isLiteralTrue(authority.authorizationResponseIssuerParameterSupported) || - callbackIssuer !== authority.issuer - ) { + if (callbackIssuer === undefined) { + if (authority.authorizationResponseIssuerParameterSupported) { + throw transactionInvalidError(); + } + return; + } + if (callbackIssuer !== authority.issuer) { throw transactionInvalidError(); } } @@ -1252,10 +1255,6 @@ function containsControlCharacter(value: string): boolean { return false; } -function isLiteralTrue(value: unknown): value is true { - return value === true; -} - function isClientAuthenticationMethod( value: string, ): value is McpClientOAuthClientAuthentication["method"] { diff --git a/packages/mcp-client/tests/oauth-bootstrap.test.ts b/packages/mcp-client/tests/oauth-bootstrap.test.ts index de33416..6f104e0 100644 --- a/packages/mcp-client/tests/oauth-bootstrap.test.ts +++ b/packages/mcp-client/tests/oauth-bootstrap.test.ts @@ -224,7 +224,6 @@ describe("McpClientOAuthBootstrap", () => { authorizationServerMetadata({ code_challenge_methods_supported: undefined, token_endpoint_auth_methods_supported: undefined, - authorization_response_iss_parameter_supported: false, }), ); @@ -241,13 +240,32 @@ describe("McpClientOAuthBootstrap", () => { issues: [ McpClientOAuthStrictCompatibilityIssue.PkceS256Unsupported, McpClientOAuthStrictCompatibilityIssue.TokenEndpointAuthenticationUnsupported, - McpClientOAuthStrictCompatibilityIssue.AuthorizationResponseIssuerUnsupported, ], }); expect(JSON.stringify(result)).not.toContain(AUTHORIZATION_ENDPOINT); expect(JSON.stringify(result)).not.toContain(TOKEN_ENDPOINT); }); + it.each([ + { name: "omitted", supported: undefined }, + { name: "false", supported: false }, + ] as const)( + "accepts an authorization server with $name RFC 9207 response-issuer support", + async ({ supported }) => { + const bootstrap = discoveryBootstrap( + authorizationServerMetadata({ + authorization_response_iss_parameter_supported: supported, + }), + ); + + const result = await bootstrap.discover({ serverUrl: SERVER_URL }); + + expect(result.kind).toBe("ready"); + if (result.kind !== "ready") throw new Error("Expected ready discovery."); + expect(result.candidate.authority.authorizationResponseIssuerParameterSupported).toBe(false); + }, + ); + it("returns an authority accepted directly by the strict authorization starter", async () => { const bootstrap = discoveryBootstrap(authorizationServerMetadata()); const result = await bootstrap.discover({ serverUrl: SERVER_URL }); diff --git a/packages/mcp-client/tests/oauth-protocol.test.ts b/packages/mcp-client/tests/oauth-protocol.test.ts index 8524782..8e6bd74 100644 --- a/packages/mcp-client/tests/oauth-protocol.test.ts +++ b/packages/mcp-client/tests/oauth-protocol.test.ts @@ -181,7 +181,7 @@ describe("McpClientOAuthProtocol discovery", () => { { name: "omitted", supported: undefined }, { name: "false", supported: false }, ] as const)( - "rejects $name RFC 9207 authorization-response issuer capability", + "accepts $name RFC 9207 authorization-response issuer capability", async ({ supported }) => { const requests: RecordedRequest[] = []; const protocol = discoveryProtocol(requests, { @@ -196,7 +196,9 @@ describe("McpClientOAuthProtocol discovery", () => { resource: RESOURCE_URL, issuer: ISSUER_URL, }), - ).rejects.toMatchObject({ code: McpClientOAuthProtocolErrorCode.AuthorityInvalid }); + ).resolves.toMatchObject({ + authorizationResponseIssuerParameterSupported: false, + }); expect(requests).toHaveLength(2); }, ); @@ -497,10 +499,9 @@ describe("McpClientOAuthProtocol authorization transactions", () => { it.each([ { name: "omitted", supported: undefined }, - { name: "false", supported: false }, { name: "a truthy non-boolean", supported: "true" }, ] as const)( - "rejects a manually supplied authority with RFC 9207 capability $name", + "rejects a manually supplied authority with invalid RFC 9207 capability $name", async ({ supported }) => { const authority = defaultAuthority(); Object.defineProperty(authority, "authorizationResponseIssuerParameterSupported", { @@ -520,6 +521,22 @@ describe("McpClientOAuthProtocol authorization transactions", () => { }, ); + it("accepts a manually supplied authority without RFC 9207 response-issuer support", async () => { + const endpointPolicy = vi.fn(allowEndpoint); + const protocol = new McpClientOAuthProtocol({ fetch: unexpectedFetch, endpointPolicy }); + + await expect( + protocol.startAuthorization({ + authority: defaultAuthority({ + authorizationResponseIssuerParameterSupported: false, + }), + client: noneClient(), + redirectUri: REDIRECT_URI, + }), + ).resolves.toMatchObject({ transaction: { authority: { issuer: ISSUER_URL } } }); + expect(endpointPolicy).toHaveBeenCalledOnce(); + }); + it("rejects a manual authority that omits authorization_code from advertised grants", async () => { const endpointPolicy = vi.fn(allowEndpoint); const protocol = new McpClientOAuthProtocol({ fetch: unexpectedFetch, endpointPolicy }); @@ -848,6 +865,66 @@ describe("McpClientOAuthProtocol authorization transactions", () => { expect(requests).toHaveLength(0); }); + it.each([ + { name: "an omitted issuer", callbackIssuer: undefined }, + { name: "the exact issuer", callbackIssuer: ISSUER_URL }, + ] as const)( + "accepts $name when RFC 9207 response-issuer support was not advertised", + async ({ callbackIssuer }) => { + const requests: RecordedRequest[] = []; + const protocol = new McpClientOAuthProtocol({ + fetch: recordingFetch(requests, async () => tokenResponse()), + endpointPolicy: allowEndpoint, + now: () => 100, + }); + const started = await protocol.startAuthorization({ + authority: defaultAuthority({ + authorizationResponseIssuerParameterSupported: false, + }), + client: noneClient(), + redirectUri: REDIRECT_URI, + }); + const state = requireParameter(new URL(started.authorizationUrl), "state"); + const callback = new URLSearchParams({ code: "authorization-code", state }); + if (callbackIssuer !== undefined) callback.set("iss", callbackIssuer); + + await expect( + protocol.exchangeAuthorization({ + transaction: started.transaction, + client: noneClient(), + callback, + }), + ).resolves.toMatchObject({ access_token: "access-token" }); + expect(requests).toHaveLength(1); + }, + ); + + it("rejects a mismatched issuer even when RFC 9207 response-issuer support was not advertised", async () => { + const requests: RecordedRequest[] = []; + const protocol = new McpClientOAuthProtocol({ + fetch: recordingFetch(requests, async () => tokenResponse()), + endpointPolicy: allowEndpoint, + now: () => 100, + }); + const started = await protocol.startAuthorization({ + authority: defaultAuthority({ + authorizationResponseIssuerParameterSupported: false, + }), + client: noneClient(), + redirectUri: REDIRECT_URI, + }); + const state = requireParameter(new URL(started.authorizationUrl), "state"); + + await expect( + protocol.exchangeAuthorization({ + transaction: started.transaction, + client: noneClient(), + callback: successfulCallback(state, "https://attacker.example.test"), + }), + ).rejects.toMatchObject({ code: McpClientOAuthProtocolErrorCode.TransactionInvalid }); + expect(requests).toHaveLength(0); + }); + it("rejects transaction client, authentication, and pinned endpoint swaps before network", async () => { const requests: RecordedRequest[] = []; const policyCalls: PolicyObservation[] = [];