mirror of
https://github.com/rowboatlabs/rowboat.git
synced 2026-06-12 19:55:19 +02:00
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.
This commit is contained in:
parent
01d28bc8ba
commit
4fd613dc5f
2 changed files with 63 additions and 43 deletions
|
|
@ -106,31 +106,36 @@ function tryBindPort(
|
|||
/**
|
||||
* Create a local HTTP server to handle OAuth callback.
|
||||
*
|
||||
* When `fixedPort` is true, only the given port is tried — used for providers
|
||||
* with a pre-registered redirect URI (e.g. Google BYOK) where the port must
|
||||
* match exactly what the user registered at the OAuth provider console.
|
||||
* 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.
|
||||
*
|
||||
* When `fixedPort` is false (default), tries `port` through `port + PORT_RANGE_SIZE - 1`
|
||||
* and binds on the first available one, handling both EADDRINUSE and EACCES
|
||||
* (the latter is common on Windows when Hyper-V/WSL2 reserve the port).
|
||||
* 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>,
|
||||
fixedPort = false,
|
||||
opts: { fallback?: boolean } = {},
|
||||
): Promise<AuthServerResult> {
|
||||
const limit = fixedPort ? port : port + PORT_RANGE_SIZE - 1;
|
||||
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 ((code === 'EADDRINUSE' || code === 'EACCES') && p < limit) {
|
||||
if (fallback && (code === 'EADDRINUSE' || code === 'EACCES') && p < limit) {
|
||||
console.warn(`[OAuth] Port ${p} unavailable (${code}), trying ${p + 1}…`);
|
||||
continue;
|
||||
}
|
||||
if (fixedPort) {
|
||||
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));
|
||||
|
|
|
|||
|
|
@ -220,7 +220,8 @@ export async function resolveStartPort(
|
|||
|
||||
const registeredPort = await clientRepo.getRegisteredPort(provider);
|
||||
try {
|
||||
const probe = await createAuthServer(registeredPort, () => { /* probe */ }, true);
|
||||
// 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;
|
||||
|
|
@ -386,45 +387,59 @@ export async function connectProvider(provider: string, credentials?: { clientId
|
|||
}
|
||||
}
|
||||
},
|
||||
isStaticClient, // fixedPort=true for static providers (Google BYOK)
|
||||
// Static providers (Google BYOK) keep fixed-port behaviour to match the
|
||||
// pre-registered redirect URI at the provider's console. DCR providers
|
||||
// can fall back since we register the actual bound port below.
|
||||
{ fallback: !isStaticClient },
|
||||
);
|
||||
|
||||
// --- OAuth config + PKCE (uses actual bound port for redirect URI) ---
|
||||
const redirectUri = buildRedirectUri(boundPort);
|
||||
const config = await getProviderConfiguration(provider, redirectUri, credentials);
|
||||
// Server is bound. Any throw between here and `activeFlow = ...` would
|
||||
// leak the port — `cancelActiveFlow` only closes it once activeFlow is set.
|
||||
try {
|
||||
const redirectUri = buildRedirectUri(boundPort);
|
||||
const config = await getProviderConfiguration(provider, redirectUri, credentials);
|
||||
|
||||
const { verifier: codeVerifier, challenge: codeChallenge } = await oauthClient.generatePKCE();
|
||||
state = oauthClient.generateState();
|
||||
const { verifier: codeVerifier, challenge: codeChallenge } = await oauthClient.generatePKCE();
|
||||
state = oauthClient.generateState();
|
||||
|
||||
const scopes = providerConfig.scopes || [];
|
||||
activeFlows.set(state, { codeVerifier, provider, config });
|
||||
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,
|
||||
});
|
||||
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');
|
||||
// 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);
|
||||
}
|
||||
}, 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 };
|
||||
throw setupError;
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('OAuth connection failed:', error);
|
||||
return {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue