From c7cdfd3313bcc032d20d937f07302c6edfd30ef4 Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Thu, 21 May 2026 18:03:55 +0100 Subject: [PATCH 1/2] fix(auth): send DCR HTTP Basic credentials raw, not form-url-encoded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit oauth4webapi's ClientSecretBasic form-url-encodes the Basic credential per RFC 6749 §2.3.1, turning reserved chars in the client_id into percent-escapes (a DCR-issued `twd_…` becomes `twd%5F…`). Servers that don't url-decode the Basic credential — Twist among them — then fail the lookup with "client_id not found". Replace `oauth.ClientSecretBasic` with a raw Basic `ClientAuth` that base64s `client_id:client_secret` verbatim. Raw is interoperable with both decoding and non-decoding servers for the URL-safe credentials OAuth servers issue in practice; consumers whose credentials contain `:` / `%` / `+` can still select `client_secret_post`. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 2 +- src/auth/providers/dcr.test.ts | 15 ++++++++------- src/auth/providers/dcr.ts | 26 +++++++++++++++++++++++--- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 65c4802..94c3561 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,7 @@ const provider = createDcrProvider({ }) ``` -The DCR-issued `client_id` (and `client_secret`, if returned) are stashed in the handshake and threaded through the rest of the flow. The server-returned `token_endpoint_auth_method` is authoritative (RFC 7591 §3.2.1) and overrides the configured one. By default the token exchange uses `Authorization: Basic` with form-url-encoded credentials (RFC 6749 §2.3.1); pass `tokenEndpointAuthMethod: 'client_secret_post'` to send credentials in the body instead, or `'none'` for a public-client registration. When the registration response carries no `client_secret`, the exchange falls back to a public-client POST regardless of the requested method. Any extra registration metadata (e.g. `software_statement`) goes in `clientMetadata.extra`. cli-core does **not** cache the registered client — each login mints a fresh one. +The DCR-issued `client_id` (and `client_secret`, if returned) are stashed in the handshake and threaded through the rest of the flow. The server-returned `token_endpoint_auth_method` is authoritative (RFC 7591 §3.2.1) and overrides the configured one. By default the token exchange uses `Authorization: Basic` with the credentials sent **raw** (not oauth4webapi's RFC 6749 §2.3.1 form-url-encoding) — many servers don't url-decode the Basic credential, so an encoded `client_id` with reserved chars (e.g. `twd_…` → `twd%5F…`) would fail their lookup; raw works with both decoding and non-decoding servers. Pass `tokenEndpointAuthMethod: 'client_secret_post'` to send credentials in the body instead (the safe choice if a credential contains `:` / `%` / `+`), or `'none'` for a public-client registration. When the registration response carries no `client_secret`, the exchange falls back to a public-client POST regardless of the requested method. Any extra registration metadata (e.g. `software_statement`) goes in `clientMetadata.extra`. cli-core does **not** cache the registered client — each login mints a fresh one. Both `createPkceProvider` and `createDcrProvider` accept an optional `errorHints: string[]` that is prepended to every `CliError` they throw. Use it for CLI-specific remediation that should accompany every auth failure (e.g. `['Try again: tw auth login', 'Or set TWIST_API_TOKEN environment variable']`). Server-returned response bodies (for non-2xx replies) are appended after the user hints so the actionable hint stays at the top. diff --git a/src/auth/providers/dcr.test.ts b/src/auth/providers/dcr.test.ts index c81fef8..d9052b1 100644 --- a/src/auth/providers/dcr.test.ts +++ b/src/auth/providers/dcr.test.ts @@ -47,7 +47,7 @@ describe('createDcrProvider', () => { it('prepare POSTs RFC 7591 metadata, authorize uses the issued client_id, exchangeCode sends Basic auth', async () => { const { calls, fetchImpl } = makeFetchRecorder((u) => u === REGISTRATION_URL - ? registration({ client_id: 'issuedid', client_secret: 'issuedsecret' }) + ? registration({ client_id: 'twd_id', client_secret: 'sec_ret' }) : token({ access_token: 'tok-1', expires_in: 3600 }), ) const provider = createDcrProvider({ @@ -65,7 +65,7 @@ describe('createDcrProvider', () => { }) const prepared = await provider.prepare!({ redirectUri: REDIRECT_URI, flags: {} }) - expect(prepared.handshake).toEqual({ clientId: 'issuedid', clientSecret: 'issuedsecret' }) + expect(prepared.handshake).toEqual({ clientId: 'twd_id', clientSecret: 'sec_ret' }) const regBody = JSON.parse(calls[0].init.body as string) as Record expect(regBody).toMatchObject({ @@ -88,14 +88,14 @@ describe('createDcrProvider', () => { handshake: prepared.handshake, }) const url = new URL(authorize.authorizeUrl) - expect(url.searchParams.get('client_id')).toBe('issuedid') + expect(url.searchParams.get('client_id')).toBe('twd_id') expect(url.searchParams.get('redirect_uri')).toBe(REDIRECT_URI) expect(url.searchParams.get('state')).toBe('state-123') expect(url.searchParams.get('code_challenge_method')).toBe('S256') expect(url.searchParams.get('code_challenge')).toMatch(/^[A-Za-z0-9_-]+$/) expect(url.searchParams.get('scope')).toBe('user:read threads:read') expect(typeof authorize.handshake.codeVerifier).toBe('string') - expect(authorize.handshake.clientSecret).toBe('issuedsecret') + expect(authorize.handshake.clientSecret).toBe('sec_ret') const result = await provider.exchangeCode({ code: 'auth-code', @@ -107,10 +107,11 @@ describe('createDcrProvider', () => { expect(result.expiresAt).toBeGreaterThan(Date.now()) const tokenCall = calls.find((c) => c.url === TOKEN_URL)! - // oauth4webapi form-url-encodes the credentials per RFC 6749 §2.3.1 - // before base64; alphanumeric id/secret round-trip unchanged. + // Basic credential is sent raw (NOT oauth4webapi's RFC 6749 §2.3.1 + // form-url-encoding) so a `client_id` with reserved chars like `_` + // isn't mangled to `%5F` for servers that don't url-decode it. expect(headersOf(tokenCall).get('authorization')).toBe( - `Basic ${Buffer.from('issuedid:issuedsecret', 'utf8').toString('base64')}`, + `Basic ${Buffer.from('twd_id:sec_ret', 'utf8').toString('base64')}`, ) const tokenBody = bodyOf(tokenCall) expect(tokenBody.get('grant_type')).toBe('authorization_code') diff --git a/src/auth/providers/dcr.ts b/src/auth/providers/dcr.ts index ca6b014..f53e68d 100644 --- a/src/auth/providers/dcr.ts +++ b/src/auth/providers/dcr.ts @@ -98,8 +98,9 @@ const VALID_AUTH_METHODS: ReadonlySet = new Set([ * - `authorize`: standard PKCE S256 with `client_id` read from the handshake. * - `exchangeCode`: `authorizationCodeGrantRequest` authenticated per the * handshake's server-returned auth method (falling back to the configured - * one) — `ClientSecretBasic` / `ClientSecretPost` / `None` (the last also - * when the registration response carried no `client_secret`). + * one) — HTTP Basic (sent raw; see `rawClientSecretBasic`), client-secret + * POST, or public-client `None` (the last also when the registration + * response carried no `client_secret`). * - `validateToken`: caller-supplied. */ export function createDcrProvider( @@ -242,7 +243,7 @@ export function createDcrProvider( } else if (effectiveAuthMethod === 'client_secret_post') { clientAuth = oauth.ClientSecretPost(clientSecret) } else { - clientAuth = oauth.ClientSecretBasic(clientSecret) + clientAuth = rawClientSecretBasic(clientSecret) } try { @@ -284,6 +285,25 @@ export function createDcrProvider( } } +/** + * HTTP Basic client auth that sends `client_id:client_secret` **raw** — without + * oauth4webapi's RFC 6749 §2.3.1 `application/x-www-form-urlencoded` encoding of + * each component. Many OAuth servers don't url-decode the Basic credential, so + * an encoded `client_id` containing reserved chars (e.g. a DCR-issued `twd_…` + * → `twd%5F…`) fails the server-side lookup. Raw is interoperable with both + * decoding and non-decoding servers for the URL-safe credentials OAuth servers + * issue in practice; consumers whose credentials contain `:` / `%` / `+` (rare) + * should select `client_secret_post` instead. + */ +function rawClientSecretBasic(clientSecret: string): ClientAuth { + return (_as, client, _body, headers) => { + const credentials = Buffer.from(`${client.client_id}:${clientSecret}`, 'utf8').toString( + 'base64', + ) + headers.set('authorization', `Basic ${credentials}`) + } +} + /** Thread an injected `fetchImpl` into oauth4webapi via its `customFetch` symbol. */ function customFetchOptions( oauth: typeof import('oauth4webapi'), From 299f552eee63ad57f4df093815c149d2d9b46930 Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Thu, 21 May 2026 18:22:55 +0100 Subject: [PATCH 2/2] fix(auth): RFC 3986-encode DCR Basic credentials instead of sending raw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review: fully-raw Basic silently breaks credentials containing reserved chars (`:` `%` `+` `/`) — a conformant token endpoint only reconstructs those when each component is escaped first. Encode each component with encodeURIComponent (RFC 3986): it escapes the reserved chars but, unlike oauth4webapi's stricter §2.3.1 form-encoding, leaves the unreserved `-` `_` `.` `~` intact — so servers that don't url-decode the Basic credential (a DCR-issued `twd_…` client_id) still match, with no silently-unrepresentable header. Regression fixture now uses a secret with `+` / `/` to pin both halves of the contract: preserved `_`, escaped reserved chars. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 2 +- src/auth/providers/dcr.test.ts | 16 +++++++++------- src/auth/providers/dcr.ts | 33 +++++++++++++++++---------------- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 94c3561..a619562 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,7 @@ const provider = createDcrProvider({ }) ``` -The DCR-issued `client_id` (and `client_secret`, if returned) are stashed in the handshake and threaded through the rest of the flow. The server-returned `token_endpoint_auth_method` is authoritative (RFC 7591 §3.2.1) and overrides the configured one. By default the token exchange uses `Authorization: Basic` with the credentials sent **raw** (not oauth4webapi's RFC 6749 §2.3.1 form-url-encoding) — many servers don't url-decode the Basic credential, so an encoded `client_id` with reserved chars (e.g. `twd_…` → `twd%5F…`) would fail their lookup; raw works with both decoding and non-decoding servers. Pass `tokenEndpointAuthMethod: 'client_secret_post'` to send credentials in the body instead (the safe choice if a credential contains `:` / `%` / `+`), or `'none'` for a public-client registration. When the registration response carries no `client_secret`, the exchange falls back to a public-client POST regardless of the requested method. Any extra registration metadata (e.g. `software_statement`) goes in `clientMetadata.extra`. cli-core does **not** cache the registered client — each login mints a fresh one. +The DCR-issued `client_id` (and `client_secret`, if returned) are stashed in the handshake and threaded through the rest of the flow. The server-returned `token_endpoint_auth_method` is authoritative (RFC 7591 §3.2.1) and overrides the configured one. By default the token exchange uses `Authorization: Basic` with each credential component percent-encoded via `encodeURIComponent` (RFC 3986) rather than oauth4webapi's stricter RFC 6749 §2.3.1 form-url-encoding. Both escape genuinely reserved chars (`:` / `%` / `+` / `/`) so a conformant server reconstructs them, but §2.3.1 _also_ escapes the unreserved `-` `_` `.` `~` — which breaks servers that don't url-decode the Basic credential (a DCR-issued `twd_…` would arrive as `twd%5F…` and miss the lookup). Pass `tokenEndpointAuthMethod: 'client_secret_post'` to send credentials in the body instead, or `'none'` for a public-client registration. When the registration response carries no `client_secret`, the exchange falls back to a public-client POST regardless of the requested method. Any extra registration metadata (e.g. `software_statement`) goes in `clientMetadata.extra`. cli-core does **not** cache the registered client — each login mints a fresh one. Both `createPkceProvider` and `createDcrProvider` accept an optional `errorHints: string[]` that is prepended to every `CliError` they throw. Use it for CLI-specific remediation that should accompany every auth failure (e.g. `['Try again: tw auth login', 'Or set TWIST_API_TOKEN environment variable']`). Server-returned response bodies (for non-2xx replies) are appended after the user hints so the actionable hint stays at the top. diff --git a/src/auth/providers/dcr.test.ts b/src/auth/providers/dcr.test.ts index d9052b1..3a252bb 100644 --- a/src/auth/providers/dcr.test.ts +++ b/src/auth/providers/dcr.test.ts @@ -47,7 +47,7 @@ describe('createDcrProvider', () => { it('prepare POSTs RFC 7591 metadata, authorize uses the issued client_id, exchangeCode sends Basic auth', async () => { const { calls, fetchImpl } = makeFetchRecorder((u) => u === REGISTRATION_URL - ? registration({ client_id: 'twd_id', client_secret: 'sec_ret' }) + ? registration({ client_id: 'twd_id', client_secret: 'se+cr/et' }) : token({ access_token: 'tok-1', expires_in: 3600 }), ) const provider = createDcrProvider({ @@ -65,7 +65,7 @@ describe('createDcrProvider', () => { }) const prepared = await provider.prepare!({ redirectUri: REDIRECT_URI, flags: {} }) - expect(prepared.handshake).toEqual({ clientId: 'twd_id', clientSecret: 'sec_ret' }) + expect(prepared.handshake).toEqual({ clientId: 'twd_id', clientSecret: 'se+cr/et' }) const regBody = JSON.parse(calls[0].init.body as string) as Record expect(regBody).toMatchObject({ @@ -95,7 +95,7 @@ describe('createDcrProvider', () => { expect(url.searchParams.get('code_challenge')).toMatch(/^[A-Za-z0-9_-]+$/) expect(url.searchParams.get('scope')).toBe('user:read threads:read') expect(typeof authorize.handshake.codeVerifier).toBe('string') - expect(authorize.handshake.clientSecret).toBe('sec_ret') + expect(authorize.handshake.clientSecret).toBe('se+cr/et') const result = await provider.exchangeCode({ code: 'auth-code', @@ -107,11 +107,13 @@ describe('createDcrProvider', () => { expect(result.expiresAt).toBeGreaterThan(Date.now()) const tokenCall = calls.find((c) => c.url === TOKEN_URL)! - // Basic credential is sent raw (NOT oauth4webapi's RFC 6749 §2.3.1 - // form-url-encoding) so a `client_id` with reserved chars like `_` - // isn't mangled to `%5F` for servers that don't url-decode it. + // RFC 3986 per-component encoding: the unreserved `_` is preserved (so + // servers that don't url-decode the Basic credential still match — the + // bug oauth4webapi's §2.3.1 `%5F` escaping caused), while reserved chars + // (`+` → `%2B`, `/` → `%2F`) are escaped so a conformant server + // reconstructs them. expect(headersOf(tokenCall).get('authorization')).toBe( - `Basic ${Buffer.from('twd_id:sec_ret', 'utf8').toString('base64')}`, + `Basic ${Buffer.from('twd_id:se%2Bcr%2Fet', 'utf8').toString('base64')}`, ) const tokenBody = bodyOf(tokenCall) expect(tokenBody.get('grant_type')).toBe('authorization_code') diff --git a/src/auth/providers/dcr.ts b/src/auth/providers/dcr.ts index f53e68d..43976b7 100644 --- a/src/auth/providers/dcr.ts +++ b/src/auth/providers/dcr.ts @@ -98,9 +98,9 @@ const VALID_AUTH_METHODS: ReadonlySet = new Set([ * - `authorize`: standard PKCE S256 with `client_id` read from the handshake. * - `exchangeCode`: `authorizationCodeGrantRequest` authenticated per the * handshake's server-returned auth method (falling back to the configured - * one) — HTTP Basic (sent raw; see `rawClientSecretBasic`), client-secret - * POST, or public-client `None` (the last also when the registration - * response carried no `client_secret`). + * one) — HTTP Basic (RFC 3986-encoded, see `clientSecretBasicRfc3986`), + * client-secret POST, or public-client `None` (the last also when the + * registration response carried no `client_secret`). * - `validateToken`: caller-supplied. */ export function createDcrProvider( @@ -243,7 +243,7 @@ export function createDcrProvider( } else if (effectiveAuthMethod === 'client_secret_post') { clientAuth = oauth.ClientSecretPost(clientSecret) } else { - clientAuth = rawClientSecretBasic(clientSecret) + clientAuth = clientSecretBasicRfc3986(clientSecret) } try { @@ -286,20 +286,21 @@ export function createDcrProvider( } /** - * HTTP Basic client auth that sends `client_id:client_secret` **raw** — without - * oauth4webapi's RFC 6749 §2.3.1 `application/x-www-form-urlencoded` encoding of - * each component. Many OAuth servers don't url-decode the Basic credential, so - * an encoded `client_id` containing reserved chars (e.g. a DCR-issued `twd_…` - * → `twd%5F…`) fails the server-side lookup. Raw is interoperable with both - * decoding and non-decoding servers for the URL-safe credentials OAuth servers - * issue in practice; consumers whose credentials contain `:` / `%` / `+` (rare) - * should select `client_secret_post` instead. + * HTTP Basic client auth that percent-encodes each credential component with + * `encodeURIComponent` (RFC 3986) rather than oauth4webapi's stricter RFC 6749 + * §2.3.1 `application/x-www-form-urlencoded` form. Both escape the genuinely + * reserved chars (`:` `%` `+` `/` …) so a conformant server reconstructs them — + * but §2.3.1 *also* escapes the unreserved `-` `_` `.` `~`, which breaks servers + * that don't url-decode the Basic credential (a DCR-issued `twd_…` would arrive + * as `twd%5F…` and miss the lookup). Leaving those unreserved chars intact keeps + * such servers working while still transmitting reserved chars safely. */ -function rawClientSecretBasic(clientSecret: string): ClientAuth { +function clientSecretBasicRfc3986(clientSecret: string): ClientAuth { return (_as, client, _body, headers) => { - const credentials = Buffer.from(`${client.client_id}:${clientSecret}`, 'utf8').toString( - 'base64', - ) + const credentials = Buffer.from( + `${encodeURIComponent(client.client_id)}:${encodeURIComponent(clientSecret)}`, + 'utf8', + ).toString('base64') headers.set('authorization', `Basic ${credentials}`) } }