From 055a07be1a23a698e8109dc1f5d5909635a1122a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Wed, 20 May 2026 17:04:38 +0800 Subject: [PATCH] fix: Prevent ChatGPT OAuth authorization from failing before callback OpenAI rejects Codex OAuth authorize URLs when the redirect URI uses the desktop API server's dynamic port. The desktop ChatGPT Official flow now starts a temporary Codex callback listener and generates the authorize URL with that callback port, while keeping token exchange and storage in the existing desktop OAuth service. Constraint: OpenAI Codex OAuth client accepts the Codex-compatible localhost callback shape, not arbitrary desktop API ports. Rejected: Keep routing the callback through the desktop server port | that fails before the browser can return an authorization code. Confidence: high Scope-risk: narrow Directive: Do not put non-Codex query parameters or dynamic desktop ports into the OpenAI authorize URL without live authorization-page verification. Tested: bun test src/server/__tests__/haha-openai-oauth-service.test.ts src/server/__tests__/haha-openai-oauth-api.test.ts Tested: bun test src/services/openaiAuth/client.test.ts src/services/openaiAuth/fetch.test.ts src/services/openaiAuth/storage.test.ts Tested: bun run check:server Tested: bun run check:coverage Tested: bun run verify Not-tested: Completing a real account OAuth approval in the browser after the fix. --- .../__tests__/haha-openai-oauth-api.test.ts | 8 + .../haha-openai-oauth-service.test.ts | 158 +++++++++++++++++- src/server/api/haha-openai-oauth.ts | 2 +- src/server/services/hahaOpenAIOAuthService.ts | 143 +++++++++++++++- 4 files changed, 295 insertions(+), 16 deletions(-) diff --git a/src/server/__tests__/haha-openai-oauth-api.test.ts b/src/server/__tests__/haha-openai-oauth-api.test.ts index e7b19950..4df4a528 100644 --- a/src/server/__tests__/haha-openai-oauth-api.test.ts +++ b/src/server/__tests__/haha-openai-oauth-api.test.ts @@ -24,6 +24,8 @@ async function setup() { } async function teardown() { + hahaOpenAIOAuthService.dispose() + hahaOpenAIOAuthService.resetCallbackPortForTests() if (originalConfigDir === undefined) { delete process.env.CLAUDE_CONFIG_DIR } else { @@ -68,6 +70,9 @@ describe('POST /api/haha-openai-oauth/start', () => { afterEach(teardown) test('returns authorize URL with PKCE challenge', async () => { + const callbackPort = await getFreePort() + hahaOpenAIOAuthService.setCallbackPortForTests(callbackPort) + const { req, url, segments } = buildReq( 'POST', '/api/haha-openai-oauth/start', @@ -81,6 +86,9 @@ describe('POST /api/haha-openai-oauth/start', () => { 'codex_cli_simplified_flow=true', ) expect(data.authorizeUrl).toContain( + encodeURIComponent(`http://localhost:${callbackPort}/auth/callback`), + ) + expect(data.authorizeUrl).not.toContain( encodeURIComponent('http://localhost:54321/auth/callback'), ) expect(data.authorizeUrl).not.toContain('originator=') diff --git a/src/server/__tests__/haha-openai-oauth-service.test.ts b/src/server/__tests__/haha-openai-oauth-service.test.ts index db5de249..aac45c50 100644 --- a/src/server/__tests__/haha-openai-oauth-service.test.ts +++ b/src/server/__tests__/haha-openai-oauth-service.test.ts @@ -6,6 +6,7 @@ import { describe, test, expect, beforeEach, afterEach, spyOn } from 'bun:test' import * as fs from 'fs/promises' import * as path from 'path' import * as os from 'os' +import { createConnection, createServer } from 'net' import { HahaOpenAIOAuthService, getHahaOpenAIOAuthFilePath, @@ -15,6 +16,58 @@ import { let tmpDir: string let originalConfigDir: string | undefined let service: HahaOpenAIOAuthService +let callbackPort: number + +async function getFreePort(): Promise { + return await new Promise((resolve, reject) => { + const server = createServer() + server.on('error', reject) + server.listen(0, '127.0.0.1', () => { + const address = server.address() + if (!address || typeof address === 'string') { + server.close(() => reject(new Error('Failed to allocate test port'))) + return + } + const port = address.port + server.close(() => resolve(port)) + }) + }) +} + +async function getLocalCallback( + callbackPath: string, +): Promise<{ status: number; body: string }> { + return await new Promise((resolve, reject) => { + const socket = createConnection( + { host: 'localhost', port: callbackPort }, + () => { + socket.write( + `GET ${callbackPath} HTTP/1.1\r\nHost: localhost:${callbackPort}\r\nConnection: close\r\n\r\n`, + ) + }, + ) + let raw = '' + socket.setEncoding('utf8') + socket.on('data', (chunk) => { + raw += chunk + }) + socket.on('end', () => { + const status = Number.parseInt( + raw.match(/^HTTP\/1\.[01] (\d{3})/)?.[1] ?? '0', + 10, + ) + const body = raw.split('\r\n\r\n').slice(1).join('\r\n\r\n') + resolve({ status, body }) + }) + socket.on('error', reject) + }) +} + +function mockJwt(payload: Record): string { + const encode = (value: Record) => + Buffer.from(JSON.stringify(value)).toString('base64url') + return `${encode({ alg: 'none' })}.${encode(payload)}.signature` +} async function setup() { tmpDir = await fs.mkdtemp( @@ -22,10 +75,12 @@ async function setup() { ) originalConfigDir = process.env.CLAUDE_CONFIG_DIR process.env.CLAUDE_CONFIG_DIR = tmpDir - service = new HahaOpenAIOAuthService() + callbackPort = await getFreePort() + service = new HahaOpenAIOAuthService({ callbackPort }) } async function teardown() { + service.dispose() if (originalConfigDir === undefined) { delete process.env.CLAUDE_CONFIG_DIR } else { @@ -111,8 +166,8 @@ describe('HahaOpenAIOAuthService — session management', () => { beforeEach(setup) afterEach(teardown) - test('startSession creates session with PKCE + state', () => { - const session = service.startSession({ serverPort: 54321 }) + test('startSession creates session with PKCE + fixed Codex callback port', async () => { + const session = await service.startSession({ serverPort: 54321 }) expect(session.state).toMatch(/^[a-f0-9]{64}$/) expect(session.codeVerifier).toMatch(/^[a-f0-9]{128}$/) expect(session.authorizeUrl).toContain('code_challenge_method=S256') @@ -123,13 +178,16 @@ describe('HahaOpenAIOAuthService — session management', () => { 'codex_cli_simplified_flow=true', ) expect(session.authorizeUrl).toContain( + encodeURIComponent(`http://localhost:${callbackPort}/auth/callback`), + ) + expect(session.authorizeUrl).not.toContain( encodeURIComponent('http://localhost:54321/auth/callback'), ) expect(session.authorizeUrl).not.toContain('originator=') }) - test('getSession returns stored session by state', () => { - const session = service.startSession({ serverPort: 54321 }) + test('getSession returns stored session by state', async () => { + const session = await service.startSession({ serverPort: 54321 }) const found = service.getSession(session.state) expect(found?.codeVerifier).toBe(session.codeVerifier) }) @@ -138,11 +196,97 @@ describe('HahaOpenAIOAuthService — session management', () => { expect(service.getSession('unknown-state')).toBeNull() }) - test('consumeSession removes session after fetch', () => { - const session = service.startSession({ serverPort: 54321 }) + test('consumeSession removes session after fetch', async () => { + const session = await service.startSession({ serverPort: 54321 }) expect(service.consumeSession(session.state)).not.toBeNull() expect(service.getSession(session.state)).toBeNull() }) + + test('callback listener exchanges the authorization code and saves tokens', async () => { + const originalFetch = globalThis.fetch + const session = await service.startSession({ serverPort: 54321 }) + let tokenRequestBody = '' + + globalThis.fetch = (async (_url, init) => { + tokenRequestBody = String(init?.body ?? '') + return new Response( + JSON.stringify({ + access_token: 'openai-access-token', + refresh_token: 'openai-refresh-token', + expires_in: 3600, + id_token: mockJwt({ + email: 'test@example.com', + 'https://api.openai.com/auth': { + chatgpt_account_id: 'acct_123', + }, + }), + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ) + }) as typeof fetch + + try { + const res = await getLocalCallback( + `/auth/callback?code=auth-code&state=${session.state}`, + ) + + expect(res.status).toBe(200) + expect(res.body).toContain('OpenAI Login Successful') + expect(tokenRequestBody).toContain('code=auth-code') + expect(tokenRequestBody).toContain( + `redirect_uri=${encodeURIComponent(`http://localhost:${callbackPort}/auth/callback`)}`, + ) + expect(tokenRequestBody).toContain( + `code_verifier=${session.codeVerifier}`, + ) + + const tokens = await service.loadTokens() + expect(tokens?.accessToken).toBe('openai-access-token') + expect(tokens?.refreshToken).toBe('openai-refresh-token') + expect(tokens?.email).toBe('test@example.com') + expect(tokens?.accountId).toBe('acct_123') + } finally { + globalThis.fetch = originalFetch + } + }) + + test('callback listener renders an error page when token exchange fails', async () => { + const originalFetch = globalThis.fetch + const session = await service.startSession({ serverPort: 54321 }) + + globalThis.fetch = (async () => { + return new Response('bad request', { status: 400 }) + }) as typeof fetch + + try { + const res = await getLocalCallback( + `/auth/callback?code=bad-code&state=${session.state}`, + ) + + expect(res.status).toBe(200) + expect(res.body).toContain('OpenAI Login Failed') + expect(res.body).toContain('OpenAI token exchange failed: 400') + expect(await service.loadTokens()).toBeNull() + } finally { + globalThis.fetch = originalFetch + } + }) + + test('callback listener clears the session after an invalid callback', async () => { + const consoleSpy = spyOn(console, 'error').mockImplementation(() => {}) + const session = await service.startSession({ serverPort: 54321 }) + + try { + const res = await getLocalCallback(`/auth/callback?state=${session.state}`) + + expect(res.status).toBe(400) + expect(res.body).toContain('Authorization code not found') + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(service.getSession(session.state)).toBeNull() + } finally { + consoleSpy.mockRestore() + } + }) }) describe('HahaOpenAIOAuthService — ensureFreshAccessToken', () => { diff --git a/src/server/api/haha-openai-oauth.ts b/src/server/api/haha-openai-oauth.ts index 21f01c2b..d2360ffd 100644 --- a/src/server/api/haha-openai-oauth.ts +++ b/src/server/api/haha-openai-oauth.ts @@ -42,7 +42,7 @@ export async function handleHahaOpenAIOAuthApi( if (!parsed.success) { throw ApiError.badRequest('serverPort (positive integer) required') } - const session = hahaOpenAIOAuthService.startSession({ + const session = await hahaOpenAIOAuthService.startSession({ serverPort: parsed.data.serverPort, }) return Response.json({ diff --git a/src/server/services/hahaOpenAIOAuthService.ts b/src/server/services/hahaOpenAIOAuthService.ts index 4f71b252..2b82d9e8 100644 --- a/src/server/services/hahaOpenAIOAuthService.ts +++ b/src/server/services/hahaOpenAIOAuthService.ts @@ -12,6 +12,7 @@ import * as fs from 'fs/promises' import * as os from 'os' import * as path from 'path' +import { AuthCodeListener } from '../../services/oauth/auth-code-listener.js' import { buildOpenAIAuthorizeUrl, exchangeOpenAICodeForTokens, @@ -22,6 +23,7 @@ import { normalizeOpenAITokens, withRefreshedAccessToken, OPENAI_CODEX_REDIRECT_PATH, + OPENAI_CODEX_OAUTH_PORT, } from '../../services/openaiAuth/client.js' import type { OpenAIOAuthTokenResponse } from '../../services/openaiAuth/types.js' @@ -39,8 +41,10 @@ export type OpenAIOAuthSession = { state: string codeVerifier: string authorizeUrl: string - serverPort: number + redirectUri: string createdAt: number + authCodeListener?: AuthCodeListener + expiresTimer?: ReturnType } type OpenAIRefreshFn = ( @@ -49,6 +53,30 @@ type OpenAIRefreshFn = ( const SESSION_TTL_MS = 5 * 60 * 1000 +const HTML_SUCCESS = ` +OpenAI Login Success + +

OpenAI Login Successful

You can close this window and return to Claude Code Haha.

+ +` + +function renderErrorHtml(message: string): string { + return ` +OpenAI Login Failed + +

OpenAI Login Failed

${escapeHtml(message)}
+` +} + +function escapeHtml(s: string): string { + return s + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, ''') +} + export function getHahaOpenAIOAuthFilePath(): string { const configDir = process.env.CLAUDE_CONFIG_DIR || path.join(os.homedir(), '.claude') @@ -58,11 +86,26 @@ export function getHahaOpenAIOAuthFilePath(): string { export class HahaOpenAIOAuthService { private sessions = new Map() private refreshFn: OpenAIRefreshFn = refreshOpenAITokens + private callbackPort: number + + constructor(options: { callbackPort?: number } = {}) { + this.callbackPort = options.callbackPort ?? OPENAI_CODEX_OAUTH_PORT + } setRefreshFn(fn: OpenAIRefreshFn): void { this.refreshFn = fn } + setCallbackPortForTests(port: number): void { + this.dispose() + this.callbackPort = port + } + + resetCallbackPortForTests(): void { + this.dispose() + this.callbackPort = OPENAI_CODEX_OAUTH_PORT + } + getOAuthFilePath(): string { return getHahaOpenAIOAuthFilePath() } @@ -101,13 +144,25 @@ export class HahaOpenAIOAuthService { } } - startSession({ serverPort }: { serverPort: number }): OpenAIOAuthSession { + async startSession(_input: { serverPort: number }): Promise { this.pruneExpiredSessions() + this.dispose() const codeVerifier = generateOpenAICodeVerifier() const state = generateOpenAIState() + const authCodeListener = new AuthCodeListener(OPENAI_CODEX_REDIRECT_PATH) - const redirectUri = `http://localhost:${serverPort}${OPENAI_CODEX_REDIRECT_PATH}` + try { + await authCodeListener.start(this.callbackPort) + } catch (err) { + authCodeListener.close() + const message = err instanceof Error ? err.message : String(err) + throw new Error( + `OpenAI OAuth callback port ${this.callbackPort} is unavailable: ${message}`, + ) + } + + const redirectUri = `http://localhost:${this.callbackPort}${OPENAI_CODEX_REDIRECT_PATH}` const authorizeUrl = buildOpenAIAuthorizeUrl({ redirectUri, codeVerifier, @@ -118,10 +173,20 @@ export class HahaOpenAIOAuthService { state, codeVerifier, authorizeUrl, - serverPort, + redirectUri, createdAt: Date.now(), + authCodeListener, } + session.expiresTimer = setTimeout(() => { + if (this.sessions.get(state) === session) { + this.closeSession(session) + this.sessions.delete(state) + } + }, SESSION_TTL_MS) + session.expiresTimer.unref?.() + this.sessions.set(state, session) + this.waitForDesktopCallback(session) return session } @@ -129,6 +194,7 @@ export class HahaOpenAIOAuthService { const s = this.sessions.get(state) if (!s) return null if (Date.now() - s.createdAt > SESSION_TTL_MS) { + this.closeSession(s) this.sessions.delete(state) return null } @@ -137,17 +203,79 @@ export class HahaOpenAIOAuthService { consumeSession(state: string): OpenAIOAuthSession | null { const s = this.getSession(state) - if (s) this.sessions.delete(state) + if (s) { + this.clearSessionTimer(s) + this.sessions.delete(state) + } return s } private pruneExpiredSessions(): void { const now = Date.now() for (const [state, s] of this.sessions.entries()) { - if (now - s.createdAt > SESSION_TTL_MS) this.sessions.delete(state) + if (now - s.createdAt > SESSION_TTL_MS) { + this.closeSession(s) + this.sessions.delete(state) + } } } + private waitForDesktopCallback(session: OpenAIOAuthSession): void { + const listener = session.authCodeListener + if (!listener) return + + void listener + .waitForAuthorization(session.state, async () => {}) + .then(async (authorizationCode) => { + try { + await this.completeSession(authorizationCode, session.state) + listener.handleSuccessRedirect([], (res) => { + res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }) + res.end(HTML_SUCCESS) + }) + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + listener.handleSuccessRedirect([], (res) => { + res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }) + res.end(renderErrorHtml(message)) + }) + } finally { + this.closeSession(session) + this.sessions.delete(session.state) + } + }) + .catch((err) => { + if (this.sessions.get(session.state) === session) { + this.closeSession(session) + this.sessions.delete(session.state) + } + console.error( + '[HahaOpenAIOAuthService] OAuth callback listener failed:', + err instanceof Error ? err.message : err, + ) + }) + } + + private clearSessionTimer(session: OpenAIOAuthSession): void { + if (session.expiresTimer) { + clearTimeout(session.expiresTimer) + session.expiresTimer = undefined + } + } + + private closeSession(session: OpenAIOAuthSession): void { + this.clearSessionTimer(session) + session.authCodeListener?.close() + session.authCodeListener = undefined + } + + dispose(): void { + for (const session of this.sessions.values()) { + this.closeSession(session) + } + this.sessions.clear() + } + async completeSession( authorizationCode: string, state: string, @@ -157,10 +285,9 @@ export class HahaOpenAIOAuthService { throw new Error('OpenAI OAuth session not found or expired') } - const redirectUri = `http://localhost:${session.serverPort}${OPENAI_CODEX_REDIRECT_PATH}` const response = await exchangeOpenAICodeForTokens({ code: authorizationCode, - redirectUri, + redirectUri: session.redirectUri, codeVerifier: session.codeVerifier, })