diff --git a/src/components/ProviderManager.test.tsx b/src/components/ProviderManager.test.tsx index 22e8c6a64..d596fed64 100644 --- a/src/components/ProviderManager.test.tsx +++ b/src/components/ProviderManager.test.tsx @@ -342,6 +342,10 @@ function mockProviderManagerDependencies( authUrl?: string browserOpened?: boolean | null message?: string + submitManualCallback?: (input: string) => { + ok: boolean + error?: string + } } }, ): void { @@ -1586,6 +1590,143 @@ test('ProviderManager first-run Codex OAuth switches the current session after l await mounted.dispose() }) +test('ProviderManager Codex OAuth waiting state masks the paste field and delegates a good callback', async () => { + delete process.env.CLAUDE_CODE_SIMPLE + delete process.env.CLAUDE_CODE_USE_GITHUB + delete process.env.GITHUB_TOKEN + delete process.env.GH_TOKEN + delete process.env.SSH_CONNECTION + delete process.env.SSH_CLIENT + + const onDone = mock(() => {}) + const submitManualCallback = mock((_input: string) => ({ ok: true })) + + mockProviderManagerDependencies( + () => undefined, + async () => undefined, + { + // Stay in `waiting` (never call onAuthenticated) so the manual-paste UI + // renders. The hook returns a spy submitManualCallback. + useCodexOAuthFlow: () => ({ + state: 'waiting', + authUrl: 'https://chatgpt.com/codex', + browserOpened: true, + submitManualCallback, + }), + }, + ) + + const nonce = `${Date.now()}-${Math.random()}` + const { ProviderManager } = await import(`./ProviderManager.js?ts=${nonce}`) + const mounted = await mountProviderManager(ProviderManager, { + mode: 'first-run', + onDone, + }) + + await waitForFrameOutput( + mounted.getOutput, + frame => frame.includes('Set up provider') && frame.includes('Codex OAuth'), + ) + + await navigateToPreset(mounted.stdin, 'Codex OAuth') + mounted.stdin.write('\r') + + // Non-SSH session shows the generic "paste the callback URL" hint and the input. + await waitForFrameOutput( + mounted.getOutput, + frame => + frame.includes('Callback URL') && + frame.includes('paste the full callback URL'), + ) + + const callbackUrl = + 'http://localhost:41100/auth/callback?code=goodsecret&state=s' + mounted.stdin.write(callbackUrl) + // The pasted secret must be masked — the raw code must never reach the frame. + await waitForFrameOutput( + mounted.getOutput, + frame => !frame.includes('goodsecret') && frame.includes('Callback URL'), + ) + + mounted.stdin.write('\r') + await waitForCondition(() => submitManualCallback.mock.calls.length > 0) + expect(submitManualCallback).toHaveBeenCalledWith(callbackUrl) + // A successful submit leaves no inline error on screen. + expect( + stripAnsi(extractLastFrame(mounted.getOutput())), + ).not.toContain('State mismatch') + + await mounted.dispose() +}) + +test('ProviderManager Codex OAuth waiting state shows the SSH banner and surfaces a bad-callback error', async () => { + delete process.env.CLAUDE_CODE_SIMPLE + delete process.env.CLAUDE_CODE_USE_GITHUB + delete process.env.GITHUB_TOKEN + delete process.env.GH_TOKEN + process.env.SSH_CONNECTION = '10.0.0.1 22 10.0.0.2 22' + delete process.env.SSH_CLIENT + + const onDone = mock(() => {}) + const submitManualCallback = mock((_input: string) => ({ + ok: false, + error: 'State mismatch', + })) + + try { + mockProviderManagerDependencies( + () => undefined, + async () => undefined, + { + useCodexOAuthFlow: () => ({ + state: 'waiting', + authUrl: 'https://chatgpt.com/codex', + browserOpened: true, + submitManualCallback, + }), + }, + ) + + const nonce = `${Date.now()}-${Math.random()}` + const { ProviderManager } = await import(`./ProviderManager.js?ts=${nonce}`) + const mounted = await mountProviderManager(ProviderManager, { + mode: 'first-run', + onDone, + }) + + await waitForFrameOutput( + mounted.getOutput, + frame => + frame.includes('Set up provider') && frame.includes('Codex OAuth'), + ) + + await navigateToPreset(mounted.stdin, 'Codex OAuth') + mounted.stdin.write('\r') + + // SSH session shows the dedicated banner instead of the generic hint. + await waitForFrameOutput( + mounted.getOutput, + frame => + frame.includes('SSH session detected') && + frame.includes('Callback URL'), + ) + + mounted.stdin.write('http://localhost:41100/auth/callback?code=x&state=s') + mounted.stdin.write('\r') + + // A rejected callback renders the inline error returned by the hook. + await waitForFrameOutput( + mounted.getOutput, + frame => frame.includes('State mismatch'), + ) + expect(submitManualCallback).toHaveBeenCalledTimes(1) + + await mounted.dispose() + } finally { + delete process.env.SSH_CONNECTION + } +}) + test('ProviderManager first-run Codex OAuth surfaces credential storage warnings', async () => { delete process.env.CLAUDE_CODE_SIMPLE delete process.env.CLAUDE_CODE_USE_GITHUB diff --git a/src/components/ProviderManager.tsx b/src/components/ProviderManager.tsx index 2b527a4df..0e5b5ad8d 100644 --- a/src/components/ProviderManager.tsx +++ b/src/components/ProviderManager.tsx @@ -602,6 +602,40 @@ function XaiManualCodeInput({ ) } +function CodexManualCallbackInput({ + onSubmit, +}: { + onSubmit: (input: string) => void +}): React.ReactNode { + const [value, setValue] = React.useState('') + const [cursorOffset, setCursorOffset] = React.useState(0) + const { columns: terminalColumns } = useTerminalSize() + const inputColumns = Math.max(20, Math.min(120, terminalColumns - 12)) + return ( + + Callback URL › + { + const trimmed = submitted.trim() + if (trimmed) onSubmit(trimmed) + }} + // The pasted callback URL carries the OAuth `code` and `state` query + // params — enough to complete the in-flight exchange — so mask it the + // same way the xAI manual-code field above does, to keep it out of + // terminal scrollback, recordings, and shared sessions. + mask="*" + // The parent `CodexOAuthSetup` owns Esc via `useKeybinding('confirm:no')`. + disableEscapeDoublePress + /> + + ) +} + function CodexOAuthSetup({ onBack, onConfigured, @@ -638,6 +672,10 @@ function CodexOAuthSetup({ const status = useCodexOAuthFlow({ onAuthenticated: handleAuthenticated, }) + const [pasteError, setPasteError] = React.useState() + const isRemoteSession = Boolean( + process.env['SSH_CONNECTION'] || process.env['SSH_CLIENT'], + ) if (status.state === 'error') { return ( @@ -693,6 +731,34 @@ function CodexOAuthSetup({ ) : ( Opening your browser... )} + {status.state === 'waiting' ? ( + <> + {isRemoteSession ? ( + + SSH session detected — the browser cannot reach this host's + localhost callback. After signing in, copy the full URL your + browser was redirected to (it starts with http://localhost:) and + paste it below. + + ) : ( + + If the browser cannot reach localhost (remote / containerized + session), paste the full callback URL it was redirected to: + + )} + { + const result = status.submitManualCallback(input) + if (!result.ok) { + setPasteError(result.error) + } else { + setPasteError(undefined) + } + }} + /> + {pasteError ? {pasteError} : null} + + ) : null} Press Esc to cancel and go back. ) diff --git a/src/components/useCodexOAuthFlow.test.tsx b/src/components/useCodexOAuthFlow.test.tsx index 3bd7eaf9e..662bd871f 100644 --- a/src/components/useCodexOAuthFlow.test.tsx +++ b/src/components/useCodexOAuthFlow.test.tsx @@ -122,6 +122,7 @@ test('does not persist credentials when downstream setup rejects', async () => { return TOKENS }, cleanup, + submitManualCallback: () => ({ ok: true as const }), }), openBrowser: async () => true, saveCodexCredentials, @@ -192,6 +193,7 @@ test('persists credentials with profile linkage after downstream setup succeeds' return TOKENS }, cleanup, + submitManualCallback: () => ({ ok: true as const }), }), openBrowser: async () => true, saveCodexCredentials, @@ -271,6 +273,7 @@ test('returns a successful storage warning without entering the error state', as return TOKENS }, cleanup, + submitManualCallback: () => ({ ok: true as const }), }), openBrowser: async () => true, saveCodexCredentials, @@ -343,6 +346,7 @@ test('reports credential persistence failures without token values', async () => return TOKENS }, cleanup, + submitManualCallback: () => ({ ok: true as const }), }), openBrowser: async () => true, saveCodexCredentials, @@ -389,3 +393,94 @@ test('reports credential persistence failures without token values', async () => await Bun.sleep(0) } }) + +test('exposes submitManualCallback in the waiting state and delegates success and failure results', async () => { + // The service's manual-callback result is what the ProviderManager UI relies + // on; verify the hook surfaces it in the waiting status and passes both the + // raw input and the service's result straight back to the caller. + const submitManualCallback = mock((input: string) => + input.includes('code=good') + ? ({ ok: true as const }) + : ({ ok: false as const, error: 'State mismatch' }), + ) + const cleanup = mock(() => {}) + const deps = { + createOAuthService: () => ({ + async startOAuthFlow( + onAuthorizationUrl: (authUrl: string) => void | Promise, + ) { + await onAuthorizationUrl('https://chatgpt.com/codex') + // Stay in the waiting state so the manual-callback path is reachable. + return new Promise(() => {}) + }, + cleanup, + submitManualCallback, + }), + openBrowser: async () => false, + saveCodexCredentials: mock(() => ({ success: true })), + isBareMode: () => false, + } + + const { useCodexOAuthFlow } = await import( + `./useCodexOAuthFlow.js?waiting-manual-${Date.now()}-${Math.random()}` + ) + + let latestStatus: + | { + state: string + submitManualCallback?: ( + input: string, + ) => { ok: boolean; error?: string } + } + | undefined + + // Stable reference: a fresh inline `onAuthenticated` on every render would + // re-run the hook's effect each render, restarting the OAuth flow and + // looping setStatus → render → setStatus ("Maximum update depth exceeded"). + // The other tests in this file memoize the callback for the same reason. + const onAuthenticated = async (): Promise => {} + + function Harness(): React.ReactNode { + const status = useCodexOAuthFlow({ + onAuthenticated, + deps, + }) + latestStatus = status as typeof latestStatus + return {status.state} + } + + const streams = createTestStreams() + const root = await createRoot({ + stdout: streams.stdout as unknown as NodeJS.WriteStream, + stdin: streams.stdin as unknown as NodeJS.ReadStream, + patchConsole: false, + }) + root.render() + + try { + await waitForCondition(() => latestStatus?.state === 'waiting') + expect(latestStatus?.state).toBe('waiting') + expect(typeof latestStatus?.submitManualCallback).toBe('function') + + const okResult = latestStatus?.submitManualCallback?.( + 'http://localhost:41100/auth/callback?code=good&state=s', + ) + expect(okResult).toEqual({ ok: true }) + + const failResult = latestStatus?.submitManualCallback?.( + 'http://localhost:41100/auth/callback?code=bad&state=s', + ) + expect(failResult?.ok).toBe(false) + expect(failResult?.error).toBe('State mismatch') + + expect(submitManualCallback).toHaveBeenCalledTimes(2) + expect(submitManualCallback).toHaveBeenCalledWith( + 'http://localhost:41100/auth/callback?code=good&state=s', + ) + } finally { + root.unmount() + streams.stdin.end() + streams.stdout.end() + await Bun.sleep(0) + } +}) diff --git a/src/components/useCodexOAuthFlow.ts b/src/components/useCodexOAuthFlow.ts index bacf38735..0261a51ba 100644 --- a/src/components/useCodexOAuthFlow.ts +++ b/src/components/useCodexOAuthFlow.ts @@ -2,6 +2,7 @@ import * as React from 'react' import { CodexOAuthService, + type CodexManualCallbackResult, type CodexOAuthTokens, } from '../services/api/codexOAuth.js' import { openBrowser } from '../utils/browser.js' @@ -14,6 +15,7 @@ export type CodexOAuthFlowStatus = state: 'waiting' authUrl: string browserOpened: boolean | null + submitManualCallback: (input: string) => CodexManualCallbackResult } | { state: 'error' @@ -24,20 +26,19 @@ type PersistCodexOAuthCredentials = (options?: { profileId?: string }) => { warning?: string } +type CodexOAuthServiceLike = Pick< + CodexOAuthService, + 'startOAuthFlow' | 'cleanup' | 'submitManualCallback' +> + type CodexOAuthFlowDependencies = { - createOAuthService?: () => Pick< - CodexOAuthService, - 'startOAuthFlow' | 'cleanup' - > + createOAuthService?: () => CodexOAuthServiceLike openBrowser?: typeof openBrowser saveCodexCredentials?: typeof saveCodexCredentials isBareMode?: typeof isBareMode } -function createDefaultOAuthService(): Pick< - CodexOAuthService, - 'startOAuthFlow' | 'cleanup' -> { +function createDefaultOAuthService(): CodexOAuthServiceLike { return new CodexOAuthService() } @@ -71,6 +72,9 @@ export function useCodexOAuthFlow(options: { let cancelled = false const oauthService = createOAuthService() + const submitManualCallback = ( + input: string, + ): CodexManualCallbackResult => oauthService.submitManualCallback(input) void oauthService .startOAuthFlow(async authUrl => { @@ -79,6 +83,7 @@ export function useCodexOAuthFlow(options: { state: 'waiting', authUrl, browserOpened: null, + submitManualCallback, }) const browserOpened = await openBrowserFn(authUrl) if (cancelled) return @@ -86,6 +91,7 @@ export function useCodexOAuthFlow(options: { state: 'waiting', authUrl, browserOpened, + submitManualCallback, }) }) .then(async tokens => { diff --git a/src/services/api/codexOAuth.test.ts b/src/services/api/codexOAuth.test.ts index c5f1fcf7b..7990d3140 100644 --- a/src/services/api/codexOAuth.test.ts +++ b/src/services/api/codexOAuth.test.ts @@ -257,6 +257,121 @@ test('serves updated success copy after a successful Codex OAuth flow', async () } }) +test('manual callback paste completes the flow when the loopback is unreachable', async () => { + await acquireCodexOAuthTestIsolation() + + try { + process.env.CODEX_OAUTH_CLIENT_ID = 'test-client-id' + + globalThis.fetch = asMockFetch( + mock(async () => { + return new Response( + JSON.stringify({ + access_token: 'manual-access-token', + refresh_token: 'manual-refresh-token', + }), + { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }, + ) + }), + ) + + // Hanging listener — never resolves on its own. The manual paste path + // must be what completes the flow. + let capturedState = '' + let pending = false + const hangingListenerFactory = ((callbackPath: string) => ({ + callbackPath, + async start(): Promise { + return 41100 + }, + hasPendingResponse(): boolean { + return pending + }, + async waitForAuthorization( + state: string, + onReady: () => Promise, + ): Promise { + capturedState = state + pending = true + await onReady() + return new Promise(() => { + /* never resolves */ + }) + }, + handleSuccessRedirect(): void { + pending = false + }, + handleErrorRedirect(): void { + pending = false + }, + cancelPendingAuthorization(): void { + pending = false + }, + })) as unknown as NonNullable< + ConstructorParameters[0] + >['createAuthCodeListener'] + + const service = new CodexOAuthService({ + callbackPort: 0, + callbackHost: '127.0.0.1', + createAuthCodeListener: hangingListenerFactory, + }) + + const flowPromise = service.startOAuthFlow(async () => {}) + + // Wait until startOAuthFlow has populated expectedState via the listener. + // Bound the wait so a regression that never captures the state fails with a + // clear assertion instead of hanging the suite indefinitely. + const stateDeadline = Date.now() + 5_000 + while (!capturedState) { + if (Date.now() > stateDeadline) { + throw new Error( + 'startOAuthFlow did not capture the OAuth state within 5s', + ) + } + await Bun.sleep(0) + } + + const stateMismatch = service.submitManualCallback( + 'http://localhost:41100/auth/callback?code=foo&state=wrong', + ) + expect(stateMismatch.ok).toBe(false) + if (!stateMismatch.ok) { + expect(stateMismatch.error).toContain('State mismatch') + } + + const missingCode = service.submitManualCallback( + `http://localhost:41100/auth/callback?state=${capturedState}`, + ) + expect(missingCode.ok).toBe(false) + if (!missingCode.ok) { + expect(missingCode.error).toContain('`code`') + } + + const errorRedirect = service.submitManualCallback( + `http://localhost:41100/auth/callback?error=access_denied&state=${capturedState}`, + ) + expect(errorRedirect.ok).toBe(false) + if (!errorRedirect.ok) { + expect(errorRedirect.error).toContain('access_denied') + } + + const success = service.submitManualCallback( + `http://localhost:41100/auth/callback?code=manual-auth-code&state=${capturedState}`, + ) + expect(success.ok).toBe(true) + + const tokens = await flowPromise + expect(tokens.accessToken).toBe('manual-access-token') + expect(tokens.refreshToken).toBe('manual-refresh-token') + } finally { + restoreCodexOAuthTestIsolation() + } +}) + test('cancellation during token exchange returns a cancelled page and rejects the flow', async () => { await acquireCodexOAuthTestIsolation() diff --git a/src/services/api/codexOAuth.ts b/src/services/api/codexOAuth.ts index be2b99d24..be9dd19b1 100644 --- a/src/services/api/codexOAuth.ts +++ b/src/services/api/codexOAuth.ts @@ -203,10 +203,17 @@ type CodexOAuthListener = Pick< | 'cancelPendingAuthorization' > +export type CodexManualCallbackResult = + | { ok: true } + | { ok: false; error: string } + export class CodexOAuthService { private authCodeListener: CodexOAuthListener | null = null private port: number | null = null private tokenExchangeAbortController: AbortController | null = null + private manualResolver: ((authorizationCode: string) => void) | null = null + private manualRejecter: ((error: Error) => void) | null = null + private expectedState: string | null = null constructor(private readonly options: CodexOAuthServiceOptions = {}) {} @@ -214,6 +221,83 @@ export class CodexOAuthService { return new Error('Codex OAuth flow was cancelled.') } + /** + * Recover the flow when the loopback callback is unreachable — typically a + * remote SSH session where the user's browser redirects to a localhost URL + * that resolves to their workstation, not the openclaude host. The user + * pastes the full redirected URL (or just its query string), we validate + * the state parameter against the in-flight flow, and resolve the same + * authorization code the loopback path would have produced. + * + * Returns a structured outcome instead of throwing so the UI layer can + * surface parse / state errors inline without unmounting the flow. + */ + submitManualCallback(input: string): CodexManualCallbackResult { + const trimmed = input.trim() + if (!trimmed) { + return { ok: false, error: 'Paste the callback URL or its query string.' } + } + if (!this.manualResolver || !this.expectedState) { + return { + ok: false, + error: 'No Codex OAuth flow is waiting for a manual callback.', + } + } + + let searchParams: URLSearchParams + try { + if (trimmed.includes('://')) { + searchParams = new URL(trimmed).searchParams + } else { + searchParams = new URLSearchParams( + trimmed.startsWith('?') ? trimmed.slice(1) : trimmed, + ) + } + } catch { + return { + ok: false, + error: 'Could not parse the callback URL — paste the full address.', + } + } + + const code = asTrimmedString(searchParams.get('code') ?? undefined) + const state = asTrimmedString(searchParams.get('state') ?? undefined) + const errorParam = asTrimmedString(searchParams.get('error') ?? undefined) + if (errorParam) { + const description = asTrimmedString( + searchParams.get('error_description') ?? undefined, + ) + return { + ok: false, + error: description + ? `Authorization failed: ${errorParam} — ${description}` + : `Authorization failed: ${errorParam}`, + } + } + if (!code) { + return { ok: false, error: 'Callback URL is missing the `code` parameter.' } + } + if (!state) { + return { + ok: false, + error: 'Callback URL is missing the `state` parameter.', + } + } + if (state !== this.expectedState) { + return { + ok: false, + error: + 'State mismatch — the URL is from a different login attempt. Start over.', + } + } + + const resolver = this.manualResolver + this.manualResolver = null + this.manualRejecter = null + resolver(code) + return { ok: true } + } + async startOAuthFlow( authURLHandler: (authUrl: string) => Promise, ): Promise { @@ -242,13 +326,29 @@ export class CodexOAuthService { state, }) + this.expectedState = state + const manualPromise = new Promise((resolve, reject) => { + this.manualResolver = resolve + this.manualRejecter = reject + }) + // Manual path may never be taken; swallow its rejection (raised from + // cleanup()) so it doesn't bubble as an unhandledRejection when the + // loopback wins the race. + manualPromise.catch(() => undefined) + try { - const authorizationCode = await authCodeListener.waitForAuthorization( + const loopbackPromise = authCodeListener.waitForAuthorization( state, async () => { await authURLHandler(authUrl) }, ) + loopbackPromise.catch(() => undefined) + + const authorizationCode = await Promise.race([ + loopbackPromise, + manualPromise, + ]) const tokenExchangeAbortController = new AbortController() this.tokenExchangeAbortController = tokenExchangeAbortController @@ -344,5 +444,13 @@ export class CodexOAuthService { this.authCodeListener?.cancelPendingAuthorization(cancellationError) this.authCodeListener = null this.port = null + + // Unblock any caller awaiting Promise.race against the manual path so the + // race resolves to the listener's rejection (or to this cancellation) + // instead of hanging forever. + this.manualRejecter?.(cancellationError) + this.manualResolver = null + this.manualRejecter = null + this.expectedState = null } }