fix: fall back to next port when OAuth callback server can't bind 8080 (#560)

* fix: fall back to next port when OAuth callback server can't bind 8080

On Windows with Hyper-V/WSL2/Docker, port 8080 is often reserved by the
OS (EACCES) or already in use (EADDRINUSE), making sign-in completely
impossible. The app now scans 8080–8089 and binds the first available
port. For DCR providers, a stale registration locked to a blocked port
is detected and cleared so the client re-registers on the new port.
Static-client providers (Google BYOK) keep fixed-port behaviour with a
clear error message instead of a raw Node.js exception.

* fix: keep createAuthServer fixed-port by default, opt-in fallback

Address review feedback:
- Flip createAuthServer default to fixed-port; fallback is now opt-in via
  { fallback: true }. Composio (composio-handler.ts) keeps exact-port
  semantics with no code change — only the Rowboat sign-in call site,
  which builds its redirect URI from the actual bound port, opts in.
- Wrap post-bind setup (DCR, PKCE, auth URL) in try/catch and close the
  server on any failure so the port is released for retries.

* fix: clear stale DCR registration when bound port differs from start port
This commit is contained in:
gagan 2026-05-22 00:10:41 +05:30 committed by GitHub
parent c4888e2899
commit 0a3fc3736f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 302 additions and 167 deletions

View file

@ -2,7 +2,8 @@ import { createServer, Server } from 'http';
import { URL } from 'url'; import { URL } from 'url';
const OAUTH_CALLBACK_PATH = '/oauth/callback'; const OAUTH_CALLBACK_PATH = '/oauth/callback';
const DEFAULT_PORT = 8080; export const DEFAULT_PORT = 8080;
export const PORT_RANGE_SIZE = 10;
/** Escape HTML special characters to prevent XSS */ /** Escape HTML special characters to prevent XSS */
function escapeHtml(str: string): string { function escapeHtml(str: string): string {
@ -19,13 +20,8 @@ export interface AuthServerResult {
port: number; port: number;
} }
/** function tryBindPort(
* Create a local HTTP server to handle OAuth callback port: number,
* Listens on http://localhost:8080/oauth/callback
* Passes the full callback URL (including iss, scope, etc.) so openid-client validation succeeds.
*/
export function createAuthServer(
port: number = DEFAULT_PORT,
onCallback: (callbackUrl: URL) => void | Promise<void> onCallback: (callbackUrl: URL) => void | Promise<void>
): Promise<AuthServerResult> { ): Promise<AuthServerResult> {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
@ -96,8 +92,10 @@ export function createAuthServer(
}); });
server.on('error', (err: NodeJS.ErrnoException) => { server.on('error', (err: NodeJS.ErrnoException) => {
if (err.code === 'EADDRINUSE') { server.close();
reject(new Error(`Port ${port} is already in use`)); if (err.code === 'EADDRINUSE' || err.code === 'EACCES') {
// Signal caller to try next port
reject(Object.assign(new Error(err.code), { code: err.code }));
} else { } else {
reject(err); reject(err);
} }
@ -105,3 +103,51 @@ export function createAuthServer(
}); });
} }
/**
* Create a local HTTP server to handle OAuth callback.
*
* Defaults to fixed-port behaviour: only `port` is tried, and a clear error is
* thrown if it cannot be bound. This is the right behaviour for any provider
* whose redirect URI is pre-registered (Google BYOK, Composio, etc.) those
* callers must keep using the exact port they've handed to the provider.
*
* Opt into `{ fallback: true }` only when the caller is prepared to use the
* port returned in `AuthServerResult` (i.e. the redirect URI is built from the
* actual bound port, not hard-coded). With fallback enabled, scans `port`
* through `port + PORT_RANGE_SIZE - 1` and binds the first available, handling
* both EADDRINUSE and EACCES (the latter is common on Windows when
* Hyper-V/WSL2 reserve the port).
*/
export async function createAuthServer(
port: number = DEFAULT_PORT,
onCallback: (callbackUrl: URL) => void | Promise<void>,
opts: { fallback?: boolean } = {},
): Promise<AuthServerResult> {
const fallback = opts.fallback === true;
const limit = fallback ? port + PORT_RANGE_SIZE - 1 : port;
for (let p = port; p <= limit; p++) {
try {
return await tryBindPort(p, onCallback);
} catch (err) {
const code = (err as NodeJS.ErrnoException).code;
if (fallback && (code === 'EADDRINUSE' || code === 'EACCES') && p < limit) {
console.warn(`[OAuth] Port ${p} unavailable (${code}), trying ${p + 1}`);
continue;
}
if (!fallback) {
const reason = code === 'EACCES' || code === 'EADDRINUSE'
? `Port ${port} is unavailable (${code}). This port must be free for sign-in to work — close any app using it and try again.`
: (err instanceof Error ? err.message : String(err));
throw new Error(reason);
}
throw new Error(
`No available port found in range ${port}${limit}. Free a port in that range and try again.`
);
}
}
// Unreachable — loop always returns or throws — but satisfies TypeScript
throw new Error(`No available port found in range ${port}${limit}.`);
}

View file

@ -1,6 +1,7 @@
import { shell } from 'electron'; import { shell } from 'electron';
import type { Server } from 'http'; import type { Server } from 'http';
import { createAuthServer } from './auth-server.js'; import { createAuthServer } from './auth-server.js';
import { DEFAULT_CALLBACK_PORT } from '@x/core/dist/auth/client-repo.js';
import * as oauthClient from '@x/core/dist/auth/oauth-client.js'; import * as oauthClient from '@x/core/dist/auth/oauth-client.js';
import type { Configuration } from '@x/core/dist/auth/oauth-client.js'; import type { Configuration } from '@x/core/dist/auth/oauth-client.js';
import { getProviderConfig, getAvailableProviders } from '@x/core/dist/auth/providers.js'; import { getProviderConfig, getAvailableProviders } from '@x/core/dist/auth/providers.js';
@ -17,7 +18,9 @@ import { isSignedIn } from '@x/core/dist/account/account.js';
import { getWebappUrl } from '@x/core/dist/config/remote-config.js'; import { getWebappUrl } from '@x/core/dist/config/remote-config.js';
import { claimTokensViaBackend } from '@x/core/dist/auth/google-backend-oauth.js'; import { claimTokensViaBackend } from '@x/core/dist/auth/google-backend-oauth.js';
const REDIRECT_URI = 'http://localhost:8080/oauth/callback'; function buildRedirectUri(port: number): string {
return `http://localhost:${port}/oauth/callback`;
}
/** Top-level openid-client messages that often wrap a more specific cause. */ /** Top-level openid-client messages that often wrap a more specific cause. */
const OPAQUE_OAUTH_TOP_MESSAGES = new Set(['invalid response encountered']); const OPAQUE_OAUTH_TOP_MESSAGES = new Set(['invalid response encountered']);
@ -114,9 +117,15 @@ function getClientRegistrationRepo(): IClientRegistrationRepo {
} }
/** /**
* Get or create OAuth configuration for a provider * Get or create OAuth configuration for a provider.
* `redirectUri` is required for DCR providers it is the actual callback URI
* (including port) that was just bound, so the registration and auth URL stay in sync.
*/ */
async function getProviderConfiguration(provider: string, credentialsOverride?: { clientId: string; clientSecret: string }): Promise<Configuration> { async function getProviderConfiguration(
provider: string,
redirectUri: string = buildRedirectUri(DEFAULT_CALLBACK_PORT),
credentialsOverride?: { clientId: string; clientSecret: string },
): Promise<Configuration> {
const config = await getProviderConfig(provider); const config = await getProviderConfig(provider);
const resolveClientCredentials = async (): Promise<{ clientId: string; clientSecret?: string }> => { const resolveClientCredentials = async (): Promise<{ clientId: string; clientSecret?: string }> => {
if (config.client.mode === 'static' && config.client.clientId) { if (config.client.mode === 'static' && config.client.clientId) {
@ -157,17 +166,20 @@ async function getProviderConfiguration(provider: string, credentialsOverride?:
); );
} }
// Register new client // Register new client with the actual redirect URI (port already bound)
const scopes = config.scopes || []; const scopes = config.scopes || [];
const { config: oauthConfig, registration } = await oauthClient.registerClient( const { config: oauthConfig, registration } = await oauthClient.registerClient(
config.discovery.issuer, config.discovery.issuer,
[REDIRECT_URI], [redirectUri],
scopes scopes
); );
// Save registration for future use // Parse port from redirectUri (e.g. "http://localhost:8081/...") and save
await clientRepo.saveClientRegistration(provider, registration); const boundPort = new URL(redirectUri).port
console.log(`[OAuth] ${provider}: DCR registration saved`); ? parseInt(new URL(redirectUri).port, 10)
: DEFAULT_CALLBACK_PORT;
await clientRepo.saveClientRegistration(provider, registration, boundPort);
console.log(`[OAuth] ${provider}: DCR registration saved (port ${boundPort})`);
return oauthConfig; return oauthConfig;
} }
@ -189,6 +201,37 @@ async function getProviderConfiguration(provider: string, credentialsOverride?:
} }
} }
/**
* Determine which port to start the OAuth callback server on for a DCR provider.
*
* If the provider has an existing registration, probes the port it was registered
* on. If that port is still available, returns it so the existing client_id keeps
* working. If it is blocked, clears the stale registration (forcing re-registration
* on the next available port) and returns DEFAULT_CALLBACK_PORT as the scan base.
*
* Exported for unit testing.
*/
export async function resolveStartPort(
provider: string,
clientRepo: IClientRegistrationRepo,
): Promise<number> {
const existingReg = await clientRepo.getClientRegistration(provider);
if (!existingReg) return DEFAULT_CALLBACK_PORT;
const registeredPort = await clientRepo.getRegisteredPort(provider);
try {
// Probe — fixed-port (no fallback) so we know whether the exact registered port is free
const probe = await createAuthServer(registeredPort, () => { /* probe */ });
probe.server.close();
console.log(`[OAuth] ${provider}: registered port ${registeredPort} still available`);
return registeredPort;
} catch {
console.log(`[OAuth] ${provider}: registered port ${registeredPort} blocked, clearing DCR registration`);
await clientRepo.clearClientRegistration(provider);
return DEFAULT_CALLBACK_PORT;
}
}
/** /**
* Initiate OAuth flow for a provider * Initiate OAuth flow for a provider
*/ */
@ -225,154 +268,188 @@ export async function connectProvider(provider: string, credentials?: { clientId
} }
} }
// Get or create OAuth configuration // For static-client providers (Google BYOK) the redirect URI is pre-registered
const config = await getProviderConfiguration(provider, credentials); // at the OAuth provider console on a fixed port — we must not scan.
// For DCR providers, resolveStartPort handles the re-registration trap.
const isStaticClient = providerConfig.client.mode === 'static';
const startPort = isStaticClient
? DEFAULT_CALLBACK_PORT
: await resolveStartPort(provider, getClientRegistrationRepo());
// Generate PKCE codes // --- Callback server ---
const { verifier: codeVerifier, challenge: codeChallenge } = await oauthClient.generatePKCE(); // Declare `state` before the closure so the callback can close over its binding.
const state = oauthClient.generateState(); // The variable is assigned below, before shell.openExternal, so it is always
// set by the time any browser request arrives.
// Get scopes from config let state = '';
const scopes = providerConfig.scopes || [];
// Store flow state
activeFlows.set(state, { codeVerifier, provider, config });
// Build authorization URL
const authUrl = oauthClient.buildAuthorizationUrl(config, {
redirect_uri: REDIRECT_URI,
scope: scopes.join(' '),
code_challenge: codeChallenge,
state,
});
// Create callback server
let callbackHandled = false; let callbackHandled = false;
const { server } = await createAuthServer(8080, async (callbackUrl) => {
// Guard against duplicate callbacks (browser may send multiple requests)
if (callbackHandled) return;
callbackHandled = true;
const receivedState = callbackUrl.searchParams.get('state');
if (receivedState == null || receivedState === '') {
throw new Error(
'OAuth callback missing state parameter. Complete sign-in in the browser or check the redirect URI.'
);
}
if (receivedState !== state) {
throw new Error('Invalid state parameter - possible CSRF attack');
}
const flow = activeFlows.get(state); const { server, port: boundPort } = await createAuthServer(
if (!flow || flow.provider !== provider) { startPort,
throw new Error('Invalid OAuth flow state'); async (callbackUrl) => {
} // Guard against duplicate callbacks (browser may send multiple requests)
if (callbackHandled) return;
try { callbackHandled = true;
// Use full callback URL (includes iss, scope, etc.) so openid-client validation succeeds const receivedState = callbackUrl.searchParams.get('state');
console.log(`[OAuth] Exchanging authorization code for tokens (${provider})...`); if (receivedState == null || receivedState === '') {
const tokens = await oauthClient.exchangeCodeForTokens( throw new Error(
flow.config, 'OAuth callback missing state parameter. Complete sign-in in the browser or check the redirect URI.'
callbackUrl, );
flow.codeVerifier, }
state if (receivedState !== state) {
); throw new Error('Invalid state parameter - possible CSRF attack');
// Save tokens and credentials. For Google, BYOK is the only path
// that reaches this token exchange (rowboat path returns above
// before any local server runs); stamp mode: 'byok' so a future
// refresh / reconnect can't get confused with a rowboat entry.
console.log(`[OAuth] Token exchange successful for ${provider}`);
await oauthRepo.upsert(provider, {
tokens,
...(credentials ? { clientId: credentials.clientId, clientSecret: credentials.clientSecret } : {}),
...(provider === 'google' ? { mode: 'byok' as const } : {}),
error: null,
});
// Trigger immediate sync for relevant providers
if (provider === 'google') {
triggerGmailSync();
triggerCalendarSync();
} else if (provider === 'fireflies-ai') {
triggerFirefliesSync();
} }
// For Rowboat sign-in, ensure user + Stripe customer exist before const flow = activeFlows.get(state);
// notifying the renderer. Without this, parallel API calls from if (!flow || flow.provider !== provider) {
// multiple renderer hooks race to create the user, causing duplicates. throw new Error('Invalid OAuth flow state');
let signedInUserId: string | undefined; }
if (provider === 'rowboat') {
try { try {
const billing = await getBillingInfo(); // Use full callback URL (includes iss, scope, etc.) so openid-client validation succeeds
if (billing.userId) { console.log(`[OAuth] Exchanging authorization code for tokens (${provider})...`);
signedInUserId = billing.userId; const tokens = await oauthClient.exchangeCodeForTokens(
analyticsIdentify(billing.userId, { flow.config,
...(billing.userEmail ? { email: billing.userEmail } : {}), callbackUrl,
plan: billing.subscriptionPlan, flow.codeVerifier,
status: billing.subscriptionStatus, state
}); );
analyticsCapture('user_signed_in', {
plan: billing.subscriptionPlan, // Save tokens and credentials. For Google, BYOK is the only path
status: billing.subscriptionStatus, // that reaches this token exchange (rowboat path returns above
}); // before any local server runs); stamp mode: 'byok' so a future
// refresh / reconnect can't get confused with a rowboat entry.
console.log(`[OAuth] Token exchange successful for ${provider}`);
await oauthRepo.upsert(provider, {
tokens,
...(credentials ? { clientId: credentials.clientId, clientSecret: credentials.clientSecret } : {}),
...(provider === 'google' ? { mode: 'byok' as const } : {}),
error: null,
});
// Trigger immediate sync for relevant providers
if (provider === 'google') {
triggerGmailSync();
triggerCalendarSync();
} else if (provider === 'fireflies-ai') {
triggerFirefliesSync();
}
// For Rowboat sign-in, ensure user + Stripe customer exist before
// notifying the renderer. Without this, parallel API calls from
// multiple renderer hooks race to create the user, causing duplicates.
let signedInUserId: string | undefined;
if (provider === 'rowboat') {
try {
const billing = await getBillingInfo();
if (billing.userId) {
signedInUserId = billing.userId;
analyticsIdentify(billing.userId, {
...(billing.userEmail ? { email: billing.userEmail } : {}),
plan: billing.subscriptionPlan,
status: billing.subscriptionStatus,
});
analyticsCapture('user_signed_in', {
plan: billing.subscriptionPlan,
status: billing.subscriptionStatus,
});
}
} catch (meError) {
console.error('[OAuth] Failed to initialize user via /v1/me:', meError);
} }
} catch (meError) {
console.error('[OAuth] Failed to initialize user via /v1/me:', meError);
} }
}
// Emit success event to renderer // Emit success event to renderer
emitOAuthEvent({ emitOAuthEvent({
provider, provider,
success: true, success: true,
...(signedInUserId ? { userId: signedInUserId } : {}), ...(signedInUserId ? { userId: signedInUserId } : {}),
}); });
} catch (error) { } catch (error) {
console.error('OAuth token exchange failed:', error); console.error('OAuth token exchange failed:', error);
// Log cause chain for debugging (e.g. OAUTH_INVALID_RESPONSE -> OperationProcessingError) // Log cause chain for debugging (e.g. OAUTH_INVALID_RESPONSE -> OperationProcessingError)
let cause: unknown = error; let cause: unknown = error;
while (cause != null && typeof cause === 'object' && 'cause' in cause) { while (cause != null && typeof cause === 'object' && 'cause' in cause) {
cause = (cause as { cause?: unknown }).cause; cause = (cause as { cause?: unknown }).cause;
if (cause != null) { if (cause != null) {
console.error('[OAuth] Caused by:', cause); console.error('[OAuth] Caused by:', cause);
}
}
const errorMessage = getOAuthErrorMessage(error);
emitOAuthEvent({ provider, success: false, error: errorMessage });
throw error;
} finally {
// Clean up
activeFlows.delete(state);
if (activeFlow && activeFlow.state === state) {
clearTimeout(activeFlow.cleanupTimeout);
activeFlow.server.close();
activeFlow = null;
} }
} }
const errorMessage = getOAuthErrorMessage(error); },
emitOAuthEvent({ provider, success: false, error: errorMessage }); // Static providers (Google BYOK) keep fixed-port behaviour to match the
throw error; // pre-registered redirect URI at the provider's console. DCR providers
} finally { // can fall back since we register the actual bound port below.
// Clean up { fallback: !isStaticClient },
);
// Server is bound. Any throw between here and `activeFlow = ...` would
// leak the port — `cancelActiveFlow` only closes it once activeFlow is set.
try {
// TOCTOU guard: resolveStartPort probed the registered port and found it
// free, but the port could have been grabbed between probe and real bind,
// causing fallback to a different port. The cached client_id is registered
// for the old port — clear it so getProviderConfiguration re-registers
// with the actual bound port.
if (!isStaticClient && boundPort !== startPort) {
console.log(`[OAuth] ${provider}: bound port ${boundPort} differs from start port ${startPort}, clearing stale DCR registration`);
await getClientRegistrationRepo().clearClientRegistration(provider);
}
const redirectUri = buildRedirectUri(boundPort);
const config = await getProviderConfiguration(provider, redirectUri, credentials);
const { verifier: codeVerifier, challenge: codeChallenge } = await oauthClient.generatePKCE();
state = oauthClient.generateState();
const scopes = providerConfig.scopes || [];
activeFlows.set(state, { codeVerifier, provider, config });
const authUrl = oauthClient.buildAuthorizationUrl(config, {
redirect_uri: redirectUri,
scope: scopes.join(' '),
code_challenge: codeChallenge,
state,
});
// Set timeout to clean up abandoned flows (2 minutes)
const cleanupTimeout = setTimeout(() => {
if (activeFlow?.state === state) {
console.log(`[OAuth] Cleaning up abandoned OAuth flow for ${provider} (timeout)`);
cancelActiveFlow('timed_out');
}
}, 2 * 60 * 1000);
activeFlow = {
provider,
state,
server,
cleanupTimeout,
};
// Open in system browser (shares cookies/sessions with user's regular browser)
shell.openExternal(authUrl.toString());
return { success: true };
} catch (setupError) {
// Post-bind setup failed — close the server so the port is released and
// a retry isn't blocked by our own zombie listener.
server.close();
if (state) {
activeFlows.delete(state); activeFlows.delete(state);
if (activeFlow && activeFlow.state === state) {
clearTimeout(activeFlow.cleanupTimeout);
activeFlow.server.close();
activeFlow = null;
}
} }
}); throw setupError;
}
// Set timeout to clean up abandoned flows (2 minutes)
// This prevents memory leaks if user never completes the OAuth flow
const cleanupTimeout = setTimeout(() => {
if (activeFlow?.state === state) {
console.log(`[OAuth] Cleaning up abandoned OAuth flow for ${provider} (timeout)`);
cancelActiveFlow('timed_out');
}
}, 2 * 60 * 1000); // 2 minutes
// Store complete flow state for cleanup
activeFlow = {
provider,
state,
server,
cleanupTimeout,
};
// Open in system browser (shares cookies/sessions with user's regular browser)
shell.openExternal(authUrl.toString());
// Wait for callback (server will handle it)
return { success: true };
} catch (error) { } catch (error) {
console.error('OAuth connection failed:', error); console.error('OAuth connection failed:', error);
return { return {

View file

@ -3,14 +3,21 @@ import fs from 'fs/promises';
import path from 'path'; import path from 'path';
import { ClientRegistrationResponse } from './types.js'; import { ClientRegistrationResponse } from './types.js';
export const DEFAULT_CALLBACK_PORT = 8080;
export interface IClientRegistrationRepo { export interface IClientRegistrationRepo {
getClientRegistration(provider: string): Promise<ClientRegistrationResponse | null>; getClientRegistration(provider: string): Promise<ClientRegistrationResponse | null>;
saveClientRegistration(provider: string, registration: ClientRegistrationResponse): Promise<void>; /** Returns the port that was used when DCR-registering this provider, or DEFAULT_CALLBACK_PORT if not stored. */
getRegisteredPort(provider: string): Promise<number>;
saveClientRegistration(provider: string, registration: ClientRegistrationResponse, port: number): Promise<void>;
clearClientRegistration(provider: string): Promise<void>; clearClientRegistration(provider: string): Promise<void>;
} }
// _registeredPort is our private field — stripped by Zod when we parse the RFC response fields
type StoredEntry = Record<string, unknown> & { _registeredPort?: number };
type ClientRegistrationStorage = { type ClientRegistrationStorage = {
[provider: string]: ClientRegistrationResponse; [provider: string]: StoredEntry;
}; };
export class FSClientRegistrationRepo implements IClientRegistrationRepo { export class FSClientRegistrationRepo implements IClientRegistrationRepo {
@ -45,14 +52,14 @@ export class FSClientRegistrationRepo implements IClientRegistrationRepo {
async getClientRegistration(provider: string): Promise<ClientRegistrationResponse | null> { async getClientRegistration(provider: string): Promise<ClientRegistrationResponse | null> {
const config = await this.readConfig(); const config = await this.readConfig();
const registration = config[provider]; const entry = config[provider];
if (!registration) { if (!entry) {
return null; return null;
} }
// Validate registration structure // Validate registration structure (Zod strips unknown fields like _registeredPort)
try { try {
return ClientRegistrationResponse.parse(registration); return ClientRegistrationResponse.parse(entry);
} catch { } catch {
// Invalid registration, remove it // Invalid registration, remove it
await this.clearClientRegistration(provider); await this.clearClientRegistration(provider);
@ -60,9 +67,14 @@ export class FSClientRegistrationRepo implements IClientRegistrationRepo {
} }
} }
async saveClientRegistration(provider: string, registration: ClientRegistrationResponse): Promise<void> { async getRegisteredPort(provider: string): Promise<number> {
const config = await this.readConfig(); const config = await this.readConfig();
config[provider] = registration; return config[provider]?._registeredPort ?? DEFAULT_CALLBACK_PORT;
}
async saveClientRegistration(provider: string, registration: ClientRegistrationResponse, port: number): Promise<void> {
const config = await this.readConfig();
config[provider] = { ...registration, _registeredPort: port };
await this.writeConfig(config); await this.writeConfig(config);
} }