From ab35152c1f4b3a904b747b79a5f0c36d707a7cad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=80=E7=89=87=E4=B8=80=E7=89=87?= <192845873+gdut4140@users.noreply.github.com> Date: Sun, 25 Oct 2026 11:40:18 +0800 Subject: [PATCH] fix(client): percent-encode client_secret_basic credentials like the Python SDK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 6749 §2.3.1 requires both halves of the client credentials to be form-urlencoded before they are joined with ":". applyBasicAuth encoded them raw, so credentials containing a space, "+", "%" or a ":" were sent as different credentials, and btoa threw for anything outside Latin-1. Match the Python SDK's quote(value, safe=""): encodeURIComponent plus !'()* , which leaves ~ raw. Add unit regressions and a fetch-level test that asserts the header on the token request itself. --- .../client-secret-basic-form-encoding.md | 5 + packages/client/src/client/auth.ts | 11 +- .../test/client/authBasicEncoding.test.ts | 100 ++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 .changeset/client-secret-basic-form-encoding.md create mode 100644 packages/client/test/client/authBasicEncoding.test.ts diff --git a/.changeset/client-secret-basic-form-encoding.md b/.changeset/client-secret-basic-form-encoding.md new file mode 100644 index 0000000000..77cdc04b79 --- /dev/null +++ b/.changeset/client-secret-basic-form-encoding.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +`client_secret_basic` credentials are now percent-encoded before being joined with `:` and base64-encoded into the `Authorization` header. Each half uses the same RFC 3986 encoding as the Python SDK's `quote(value, safe="")`, so `~` stays as it is, a space becomes `%20`, and `+`, `%`, or `:` are escaped instead of corrupting what a token endpoint recovers. Previously both halves were sent raw, so a secret containing `+` reached a form-decoding endpoint as a space, a `%` broke its decoding, a `:` inside the client identifier moved the split, and any character outside Latin-1 made `btoa` throw before the request was sent. Credentials made only of unreserved characters, including `~`, are byte-identical on the wire. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index caf4fa84e4..b3c580b897 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -849,6 +849,14 @@ export function applyClientAuthentication( } } +/** RFC 3986 percent-encoding matching the Python SDK's `quote(value, safe="")`, which leaves `~` alone. */ +function percentEncode(value: string): string { + return encodeURIComponent(value).replaceAll(/[!'()*]/g, c => { + const codePoint = c.codePointAt(0) ?? 0; + return `%${codePoint.toString(16).toUpperCase()}`; + }); +} + /** * Applies HTTP Basic authentication (RFC 6749 Section 2.3.1) */ @@ -857,7 +865,8 @@ export function applyBasicAuth(clientId: string, clientSecret: string | undefine throw new Error('client_secret_basic authentication requires a client_secret'); } - const credentials = btoa(`${clientId}:${clientSecret}`); + // RFC 6749 §2.3.1: both halves are encoded before being joined with ":". + const credentials = btoa(`${percentEncode(clientId)}:${percentEncode(clientSecret)}`); headers.set('Authorization', `Basic ${credentials}`); } diff --git a/packages/client/test/client/authBasicEncoding.test.ts b/packages/client/test/client/authBasicEncoding.test.ts new file mode 100644 index 0000000000..d38d81d596 --- /dev/null +++ b/packages/client/test/client/authBasicEncoding.test.ts @@ -0,0 +1,100 @@ +/** + * RFC 6749 §2.3.1: for `client_secret_basic`, the client identifier and the + * client password are each encoded before being joined with ":" and + * base64-encoded into the `Authorization` header. The encoding is the RFC 3986 + * percent-encoding the Python SDK uses (`quote(value, safe="")`), so `~` stays + * as it is; a form-decoding token endpoint recovers both halves. + */ +import { describe, expect, it } from 'vitest'; + +import { applyBasicAuth, applyClientAuthentication, exchangeAuthorization } from '../../src/client/auth'; + +/** What a conformant authorization server recovers from a Basic header. */ +function recoverCredentials(authorization: string): { username: string; password: string } { + const decoded = Buffer.from(authorization.replace(/^Basic /, ''), 'base64').toString('utf8'); + const separator = decoded.indexOf(':'); + const formDecode = (value: string): string => decodeURIComponent(value.replace(/\+/g, ' ')); + + return { + username: formDecode(decoded.slice(0, separator)), + password: formDecode(decoded.slice(separator + 1)) + }; +} + +describe('client_secret_basic credential encoding (RFC 6749 §2.3.1)', () => { + it('percent-encodes both halves before base64-encoding them', () => { + const headers = new Headers(); + applyBasicAuth('client id', 'sec:ret+with=special', headers); + + const authorization = headers.get('Authorization'); + expect(authorization).toBeTruthy(); + + const decoded = Buffer.from(authorization!.replace(/^Basic /, ''), 'base64').toString('utf8'); + + // space -> "%20", ":" -> "%3A", "+" -> "%2B", "=" -> "%3D". + expect(decoded).toBe('client%20id:sec%3Aret%2Bwith%3Dspecial'); + }); + + it('leaves "~" unencoded like the Python SDK', () => { + const headers = new Headers(); + applyBasicAuth('cid', 'Abc8Q~xYz.-_Q', headers); + + const decoded = Buffer.from(headers.get('Authorization')!.replace(/^Basic /, ''), 'base64').toString('utf8'); + + // A form encoder would send "Q%7E"; existing secrets such as Entra's + // "Q~..." must keep their bytes. + expect(decoded).toBe('cid:Abc8Q~xYz.-_Q'); + expect(recoverCredentials(headers.get('Authorization')!).password).toBe('Abc8Q~xYz.-_Q'); + }); + + it('lets a conformant server recover the original credentials', () => { + const headers = new Headers(); + applyClientAuthentication('client_secret_basic', { client_id: 'client id', client_secret: 'a+b' }, headers, new URLSearchParams()); + + expect(recoverCredentials(headers.get('Authorization')!)).toEqual({ + username: 'client id', + password: 'a+b' + }); + }); + + it('does not throw on credentials outside Latin-1', () => { + const headers = new Headers(); + + // `btoa` rejects any code point above U+00FF, which aborted the token + // request before it was sent; form-encoding leaves only ASCII behind. + expect(() => applyBasicAuth('cid', 'p€ss 中', headers)).not.toThrow(); + expect(recoverCredentials(headers.get('Authorization')!).password).toBe('p€ss 中'); + }); + + it('encodes the credentials on the token request itself', async () => { + const authorizationHeaders: string[] = []; + const fetchFn = async (url: string | URL, init?: RequestInit): Promise => { + authorizationHeaders.push(new Headers(init?.headers).get('Authorization') ?? ''); + return new Response(JSON.stringify({ access_token: 'at', token_type: 'Bearer' }), { + status: 200, + headers: { 'content-type': 'application/json' } + }); + }; + + await exchangeAuthorization('https://as.example.com', { + metadata: { + issuer: 'https://as.example.com', + authorization_endpoint: 'https://as.example.com/authorize', + token_endpoint: 'https://as.example.com/token', + response_types_supported: ['code'], + token_endpoint_auth_methods_supported: ['client_secret_basic'] + }, + clientInformation: { client_id: 'client id', client_secret: 'a+b' }, + authorizationCode: 'the-code', + codeVerifier: 'the-verifier', + redirectUri: 'https://client.example.com/callback', + fetchFn + }); + + expect(authorizationHeaders).toHaveLength(1); + expect(recoverCredentials(authorizationHeaders[0]!)).toEqual({ + username: 'client id', + password: 'a+b' + }); + }); +});