From b4837e8af883fc148f5626bdfd9d1e3728770c43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=80=E7=89=87=E4=B8=80=E7=89=87?= <3037678834@qq.com> Date: Fri, 16 Oct 2026 11:24:16 +0800 Subject: [PATCH] =?UTF-8?q?fix(client):=20form-urlencode=20client=5Fsecret?= =?UTF-8?q?=5Fbasic=20credentials=20(RFC=206749=20=C2=A72.3.1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 6749 §2.3.1 requires the client identifier and the client secret to be form-urlencoded (Appendix B) before they are joined with ":" and base64-encoded into the Authorization header; a conformant token endpoint splits the decoded value on the first ":" and form-decodes both halves. Sending them raw meant a secret containing "+" arrived as a space, "%" broke the 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 are byte-identical on the wire. --- .../client-secret-basic-form-encoding.md | 5 ++ packages/client/src/client/auth.ts | 8 +- .../test/client/authBasicEncoding.test.ts | 88 +++++++++++++++++++ 3 files changed, 100 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..473db1de6b --- /dev/null +++ b/.changeset/client-secret-basic-form-encoding.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +`client_secret_basic` credentials are now encoded as RFC 6749 §2.3.1 requires: the client identifier and the client secret are each form-urlencoded (Appendix B) before being joined with `:` and base64-encoded into the `Authorization` header. Previously both halves were sent raw, so a secret containing `+` reached a conformant token 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 are byte-identical on the wire. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index a95aca0a03..a6a1f47586 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -836,6 +836,11 @@ export function applyClientAuthentication( } } +/** `application/x-www-form-urlencoded` encoding of one value, per RFC 6749 Appendix B. */ +function formUrlEncode(value: string): string { + return new URLSearchParams({ v: value }).toString().slice('v='.length); +} + /** * Applies HTTP Basic authentication (RFC 6749 Section 2.3.1) */ @@ -844,7 +849,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 form-urlencoded before being joined with ":". + const credentials = btoa(`${formUrlEncode(clientId)}:${formUrlEncode(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..3e37287196 --- /dev/null +++ b/packages/client/test/client/authBasicEncoding.test.ts @@ -0,0 +1,88 @@ +/** + * RFC 6749 §2.3.1: for `client_secret_basic`, the client identifier and the + * client password are each encoded with the `application/x-www-form-urlencoded` + * algorithm (Appendix B) before being joined with ":" and base64-encoded into + * the `Authorization` header. A conformant token endpoint splits the decoded + * value on the first ":" and form-decodes both halves before comparing them. + */ +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('form-urlencodes 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'); + + // Appendix B: space -> "+", ":" -> "%3A", "+" -> "%2B", "=" -> "%3D". + expect(decoded).toBe('client+id:sec%3Aret%2Bwith%3Dspecial'); + }); + + 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' + }); + }); +});