mirror of
https://github.com/browseros-ai/BrowserOS.git
synced 2026-05-13 23:53:25 +00:00
fix: lazy OAuth callback server with cancel+retry (Codex CLI pattern) (#515)
The OAuth callback server on port 1455 was bound eagerly at startup, crashing the server if another BrowserOS instance was already running. Rewrite as a lazy class (OAuthCallbackServer) that: - Only binds port 1455 when the user initiates a ChatGPT Pro login - Sends GET /cancel to any existing server on the port first, then retries up to 5 times (follows Codex CLI's cancel+retry pattern) - Exposes /cancel endpoint so other instances/tools can cancel us - Releases the port after the OAuth callback arrives - Device-code providers (GitHub Copilot, Qwen) never touch port 1455 This allows running eval, dev instances, and multiple BrowserOS instances without port conflicts. OAuth login works on whichever instance initiates it — the others continue without OAuth.
This commit is contained in:
@@ -50,11 +50,19 @@ export function createOAuthRoutes(deps: OAuthRouteDeps) {
|
||||
provider: providerId,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
})
|
||||
|
||||
const message =
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: 'Failed to start authentication. Please try again.'
|
||||
return c.json({ error: message }, 500)
|
||||
: 'Failed to start authentication.'
|
||||
|
||||
// Port conflict — clear actionable message
|
||||
const status =
|
||||
error instanceof Error && error.message.includes('callback port')
|
||||
? 503
|
||||
: 500
|
||||
|
||||
return c.json({ error: message }, status)
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -80,7 +80,7 @@ export async function createHttpServer(config: HttpServerConfig) {
|
||||
|
||||
const { onShutdown } = config
|
||||
|
||||
// Initialize OAuth token manager + callback server (port released on process exit)
|
||||
// Initialize OAuth token manager (callback server binds lazily on first PKCE login)
|
||||
const tokenManager = browserosId
|
||||
? initializeOAuth(getDb(), browserosId)
|
||||
: null
|
||||
@@ -110,6 +110,7 @@ export async function createHttpServer(config: HttpServerConfig) {
|
||||
'/shutdown',
|
||||
createShutdownRoute({
|
||||
onShutdown: () => {
|
||||
tokenManager?.stopCallbackServer()
|
||||
klavisProxy?.close().catch((err) =>
|
||||
logger.warn('Failed to close Klavis proxy transport', {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
|
||||
@@ -3,68 +3,164 @@
|
||||
* Copyright 2025 BrowserOS
|
||||
* SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
*
|
||||
* Temporary HTTP server on port 1455 for OAuth callbacks.
|
||||
* OpenAI's OAuth requires redirect_uri to use this specific port
|
||||
* (matching the Codex CLI client ID registration).
|
||||
* Lazy OAuth callback server on port 1455.
|
||||
*
|
||||
* Port 1455 is required by OpenAI's Codex CLI OAuth client registration
|
||||
* (redirect_uri must be http://localhost:1455/auth/callback).
|
||||
*
|
||||
* Unlike the old implementation that bound the port at startup, this class:
|
||||
* - Only binds when the user initiates a PKCE login flow
|
||||
* - Sends GET /cancel to any existing server on the port first (Codex CLI pattern)
|
||||
* - Exposes /cancel so other instances can cancel us
|
||||
* - Releases the port after the callback arrives and no flows are pending
|
||||
*/
|
||||
|
||||
import { OAUTH_CALLBACK_PORT } from '@browseros/shared/constants/ports'
|
||||
import { logger } from '../../logger'
|
||||
import type { OAuthTokenManager } from './token-manager'
|
||||
|
||||
export function startOAuthCallbackServer(tokenManager: OAuthTokenManager): {
|
||||
stop: () => void
|
||||
} {
|
||||
const server = Bun.serve({
|
||||
port: OAUTH_CALLBACK_PORT,
|
||||
hostname: '127.0.0.1',
|
||||
fetch: async (req) => {
|
||||
const url = new URL(req.url)
|
||||
if (url.pathname !== '/auth/callback') {
|
||||
return new Response('Not found', { status: 404 })
|
||||
}
|
||||
const MAX_BIND_ATTEMPTS = 5
|
||||
const RETRY_DELAY_MS = 300
|
||||
|
||||
const code = url.searchParams.get('code')
|
||||
const state = url.searchParams.get('state')
|
||||
const error = url.searchParams.get('error')
|
||||
export class OAuthCallbackServer {
|
||||
private server: ReturnType<typeof Bun.serve> | null = null
|
||||
private tokenManager: OAuthTokenManager | null = null
|
||||
|
||||
if (error) {
|
||||
const description = url.searchParams.get('error_description') || error
|
||||
logger.warn('OAuth callback received error', { error, description })
|
||||
return htmlResponse(errorPage(description))
|
||||
}
|
||||
|
||||
if (!code || !state) {
|
||||
return htmlResponse(errorPage('Missing authorization code or state'))
|
||||
}
|
||||
|
||||
try {
|
||||
await tokenManager.handleCallback(code, state)
|
||||
|
||||
// Always show success page — chrome-extension:// redirects are blocked by Chromium.
|
||||
// The extension polls /oauth/:provider/status and detects auth automatically.
|
||||
return htmlResponse(successPage())
|
||||
} catch (err) {
|
||||
logger.error('OAuth callback failed', {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
})
|
||||
return htmlResponse(
|
||||
errorPage(
|
||||
err instanceof Error ? err.message : 'Authentication failed',
|
||||
),
|
||||
)
|
||||
}
|
||||
},
|
||||
})
|
||||
|
||||
logger.info('OAuth callback server started', { port: OAUTH_CALLBACK_PORT })
|
||||
|
||||
return {
|
||||
stop: () => {
|
||||
server.stop()
|
||||
logger.info('OAuth callback server stopped')
|
||||
},
|
||||
setTokenManager(manager: OAuthTokenManager): void {
|
||||
this.tokenManager = manager
|
||||
}
|
||||
|
||||
isRunning(): boolean {
|
||||
return this.server !== null
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure the callback server is running on port 1455.
|
||||
* If the port is already held by another process, sends GET /cancel
|
||||
* to ask it to release, then retries.
|
||||
*/
|
||||
async ensureRunning(): Promise<void> {
|
||||
if (this.server) return
|
||||
|
||||
if (!this.tokenManager) {
|
||||
throw new Error('OAuth callback server not initialized')
|
||||
}
|
||||
|
||||
let cancelSent = false
|
||||
|
||||
for (let attempt = 1; attempt <= MAX_BIND_ATTEMPTS; attempt++) {
|
||||
try {
|
||||
this.bind()
|
||||
return
|
||||
} catch {
|
||||
if (!cancelSent) {
|
||||
cancelSent = true
|
||||
await this.sendCancel()
|
||||
}
|
||||
|
||||
if (attempt < MAX_BIND_ATTEMPTS) {
|
||||
await sleep(RETRY_DELAY_MS)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`OAuth callback port ${OAUTH_CALLBACK_PORT} is in use by another process. ` +
|
||||
'Close other BrowserOS instances or CLI tools and try again.',
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop the server and release port 1455.
|
||||
*/
|
||||
stop(): void {
|
||||
if (this.server) {
|
||||
this.server.stop()
|
||||
this.server = null
|
||||
logger.info('OAuth callback server stopped', {
|
||||
port: OAUTH_CALLBACK_PORT,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
private bind(): void {
|
||||
const tokenManager = this.tokenManager!
|
||||
|
||||
this.server = Bun.serve({
|
||||
port: OAUTH_CALLBACK_PORT,
|
||||
hostname: '127.0.0.1',
|
||||
fetch: async (req) => {
|
||||
const url = new URL(req.url)
|
||||
|
||||
// /cancel — let other instances ask us to release the port
|
||||
if (url.pathname === '/cancel') {
|
||||
logger.info('OAuth callback server received cancel request')
|
||||
// Schedule stop after responding
|
||||
queueMicrotask(() => this.stop())
|
||||
return new Response('Login cancelled', { status: 200 })
|
||||
}
|
||||
|
||||
if (url.pathname !== '/auth/callback') {
|
||||
return new Response('Not found', { status: 404 })
|
||||
}
|
||||
|
||||
const code = url.searchParams.get('code')
|
||||
const state = url.searchParams.get('state')
|
||||
const error = url.searchParams.get('error')
|
||||
|
||||
if (error) {
|
||||
const description = url.searchParams.get('error_description') || error
|
||||
logger.warn('OAuth callback received error', {
|
||||
error,
|
||||
description,
|
||||
})
|
||||
return htmlResponse(errorPage(description))
|
||||
}
|
||||
|
||||
if (!code || !state) {
|
||||
return htmlResponse(errorPage('Missing authorization code or state'))
|
||||
}
|
||||
|
||||
try {
|
||||
await tokenManager.handleCallback(code, state)
|
||||
return htmlResponse(successPage())
|
||||
} catch (err) {
|
||||
logger.error('OAuth callback failed', {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
})
|
||||
return htmlResponse(
|
||||
errorPage(
|
||||
err instanceof Error ? err.message : 'Authentication failed',
|
||||
),
|
||||
)
|
||||
}
|
||||
},
|
||||
})
|
||||
|
||||
logger.info('OAuth callback server started', {
|
||||
port: OAUTH_CALLBACK_PORT,
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Send GET /cancel to any existing server on port 1455.
|
||||
* This politely asks the other process to release the port.
|
||||
* Follows the Codex CLI pattern (codex-rs/login/src/server.rs).
|
||||
*/
|
||||
private async sendCancel(): Promise<void> {
|
||||
try {
|
||||
await fetch(`http://127.0.0.1:${OAUTH_CALLBACK_PORT}/cancel`, {
|
||||
signal: AbortSignal.timeout(2000),
|
||||
})
|
||||
logger.info('Sent cancel to existing OAuth callback server')
|
||||
} catch {
|
||||
// Server might not support /cancel or might not be running
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function sleep(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms))
|
||||
}
|
||||
|
||||
function htmlResponse(html: string): Response {
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
|
||||
import type { Database } from 'bun:sqlite'
|
||||
import { startOAuthCallbackServer } from './callback-server'
|
||||
import { OAuthCallbackServer } from './callback-server'
|
||||
import { OAuthTokenManager } from './token-manager'
|
||||
import { OAuthTokenStore } from './token-store'
|
||||
|
||||
@@ -16,8 +16,9 @@ export function initializeOAuth(
|
||||
browserosId: string,
|
||||
): OAuthTokenManager {
|
||||
const store = new OAuthTokenStore(db)
|
||||
tokenManager = new OAuthTokenManager(store, browserosId)
|
||||
startOAuthCallbackServer(tokenManager)
|
||||
const callbackServer = new OAuthCallbackServer()
|
||||
tokenManager = new OAuthTokenManager(store, browserosId, callbackServer)
|
||||
callbackServer.setTokenManager(tokenManager)
|
||||
return tokenManager
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
import { OAUTH_CALLBACK_PORT } from '@browseros/shared/constants/ports'
|
||||
import { TIMEOUTS } from '@browseros/shared/constants/timeouts'
|
||||
import { logger } from '../../logger'
|
||||
import type { OAuthCallbackServer } from './callback-server'
|
||||
import { getOAuthProvider, type OAuthProviderConfig } from './providers'
|
||||
import type { OAuthTokenStore, StoredOAuthTokens } from './token-store'
|
||||
|
||||
@@ -58,6 +59,7 @@ export class OAuthTokenManager {
|
||||
constructor(
|
||||
private readonly store: OAuthTokenStore,
|
||||
private readonly browserosId: string,
|
||||
private readonly callbackServer: OAuthCallbackServer,
|
||||
) {}
|
||||
|
||||
// --- PKCE flow (ChatGPT Plus/Pro) ---
|
||||
@@ -66,6 +68,9 @@ export class OAuthTokenManager {
|
||||
providerId: string,
|
||||
redirectBackUrl?: string,
|
||||
): Promise<string> {
|
||||
// Lazily start callback server — only needed for PKCE flow
|
||||
await this.callbackServer.ensureRunning()
|
||||
|
||||
const provider = getOAuthProvider(providerId)
|
||||
if (!provider) throw new Error(`Unknown OAuth provider: ${providerId}`)
|
||||
|
||||
@@ -154,6 +159,7 @@ export class OAuthTokenManager {
|
||||
|
||||
this.store.upsertTokens(this.browserosId, flow.provider, tokens)
|
||||
this.pendingFlows.delete(state)
|
||||
this.stopCallbackIfIdle()
|
||||
|
||||
logger.info('OAuth authentication successful', {
|
||||
provider: flow.provider,
|
||||
@@ -444,6 +450,17 @@ export class OAuthTokenManager {
|
||||
this.store.deleteTokens(this.browserosId, provider)
|
||||
}
|
||||
|
||||
stopCallbackServer(): void {
|
||||
this.callbackServer.stop()
|
||||
}
|
||||
|
||||
private stopCallbackIfIdle(): void {
|
||||
const hasPkceFlows = [...this.pendingFlows.values()].some(() => true)
|
||||
if (!hasPkceFlows) {
|
||||
this.callbackServer.stop()
|
||||
}
|
||||
}
|
||||
|
||||
private cleanExpiredFlows(): void {
|
||||
const now = Date.now()
|
||||
for (const [state, flow] of this.pendingFlows) {
|
||||
|
||||
Reference in New Issue
Block a user