mirror of
https://github.com/rowboatlabs/rowboat.git
synced 2026-07-21 21:31:12 +02:00
fix: clean up OAuth server when flow is abandoned or restarted
Cancel any existing OAuth flow before starting a new one to prevent "Port 8080 is already in use" errors. Adds module-level tracking of the active flow and a cancelActiveFlow() helper for proper cleanup. Also reduces timeout from 5 minutes to 2 minutes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
4a47006aa9
commit
7a59b28651
1 changed files with 62 additions and 15 deletions
|
|
@ -1,4 +1,5 @@
|
||||||
import { shell } from 'electron';
|
import { shell } from 'electron';
|
||||||
|
import type { Server } from 'http';
|
||||||
import { createAuthServer } from './auth-server.js';
|
import { createAuthServer } from './auth-server.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';
|
||||||
|
|
@ -14,12 +15,48 @@ import { emitOAuthEvent } from './ipc.js';
|
||||||
const REDIRECT_URI = 'http://localhost:8080/oauth/callback';
|
const REDIRECT_URI = 'http://localhost:8080/oauth/callback';
|
||||||
|
|
||||||
// Store active OAuth flows (state -> { codeVerifier, provider, config })
|
// Store active OAuth flows (state -> { codeVerifier, provider, config })
|
||||||
const activeFlows = new Map<string, {
|
const activeFlows = new Map<string, {
|
||||||
codeVerifier: string;
|
codeVerifier: string;
|
||||||
provider: string;
|
provider: string;
|
||||||
config: Configuration;
|
config: Configuration;
|
||||||
}>();
|
}>();
|
||||||
|
|
||||||
|
// Module-level state for tracking the active OAuth flow
|
||||||
|
interface ActiveOAuthFlow {
|
||||||
|
provider: string;
|
||||||
|
state: string;
|
||||||
|
server: Server;
|
||||||
|
cleanupTimeout: NodeJS.Timeout;
|
||||||
|
}
|
||||||
|
|
||||||
|
let activeFlow: ActiveOAuthFlow | null = null;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Cancel any active OAuth flow, cleaning up resources
|
||||||
|
*/
|
||||||
|
function cancelActiveFlow(reason: string = 'cancelled'): void {
|
||||||
|
if (!activeFlow) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log(`[OAuth] Cancelling active flow for ${activeFlow.provider}: ${reason}`);
|
||||||
|
|
||||||
|
clearTimeout(activeFlow.cleanupTimeout);
|
||||||
|
activeFlow.server.close();
|
||||||
|
activeFlows.delete(activeFlow.state);
|
||||||
|
|
||||||
|
// Only emit event for user-visible cancellations
|
||||||
|
if (reason !== 'new_flow_started') {
|
||||||
|
emitOAuthEvent({
|
||||||
|
provider: activeFlow.provider,
|
||||||
|
success: false,
|
||||||
|
error: `OAuth flow ${reason}`
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
activeFlow = null;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get OAuth repository from DI container
|
* Get OAuth repository from DI container
|
||||||
*/
|
*/
|
||||||
|
|
@ -98,6 +135,10 @@ async function getProviderConfiguration(provider: string): Promise<Configuration
|
||||||
export async function connectProvider(provider: string): Promise<{ success: boolean; error?: string }> {
|
export async function connectProvider(provider: string): Promise<{ success: boolean; error?: string }> {
|
||||||
try {
|
try {
|
||||||
console.log(`[OAuth] Starting connection flow for ${provider}...`);
|
console.log(`[OAuth] Starting connection flow for ${provider}...`);
|
||||||
|
|
||||||
|
// Cancel any existing flow before starting a new one
|
||||||
|
cancelActiveFlow('new_flow_started');
|
||||||
|
|
||||||
const oauthRepo = getOAuthRepo();
|
const oauthRepo = getOAuthRepo();
|
||||||
const providerConfig = getProviderConfig(provider);
|
const providerConfig = getProviderConfig(provider);
|
||||||
|
|
||||||
|
|
@ -122,9 +163,6 @@ export async function connectProvider(provider: string): Promise<{ success: bool
|
||||||
state,
|
state,
|
||||||
});
|
});
|
||||||
|
|
||||||
// Declare timeout variable (will be set after server is created)
|
|
||||||
let cleanupTimeout: NodeJS.Timeout;
|
|
||||||
|
|
||||||
// Create callback server
|
// Create callback server
|
||||||
const { server } = await createAuthServer(8080, async (code, receivedState) => {
|
const { server } = await createAuthServer(8080, async (code, receivedState) => {
|
||||||
// Validate state
|
// Validate state
|
||||||
|
|
@ -140,7 +178,7 @@ export async function connectProvider(provider: string): Promise<{ success: bool
|
||||||
try {
|
try {
|
||||||
// Build callback URL for token exchange
|
// Build callback URL for token exchange
|
||||||
const callbackUrl = new URL(`${REDIRECT_URI}?code=${code}&state=${receivedState}`);
|
const callbackUrl = new URL(`${REDIRECT_URI}?code=${code}&state=${receivedState}`);
|
||||||
|
|
||||||
// Exchange code for tokens
|
// Exchange code for tokens
|
||||||
console.log(`[OAuth] Exchanging authorization code for tokens (${provider})...`);
|
console.log(`[OAuth] Exchanging authorization code for tokens (${provider})...`);
|
||||||
const tokens = await oauthClient.exchangeCodeForTokens(
|
const tokens = await oauthClient.exchangeCodeForTokens(
|
||||||
|
|
@ -172,21 +210,30 @@ export async function connectProvider(provider: string): Promise<{ success: bool
|
||||||
} finally {
|
} finally {
|
||||||
// Clean up
|
// Clean up
|
||||||
activeFlows.delete(state);
|
activeFlows.delete(state);
|
||||||
server.close();
|
if (activeFlow && activeFlow.state === state) {
|
||||||
clearTimeout(cleanupTimeout);
|
clearTimeout(activeFlow.cleanupTimeout);
|
||||||
|
activeFlow.server.close();
|
||||||
|
activeFlow = null;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// Set timeout to clean up abandoned flows (5 minutes)
|
// Set timeout to clean up abandoned flows (2 minutes)
|
||||||
// This prevents memory leaks if user never completes the OAuth flow
|
// This prevents memory leaks if user never completes the OAuth flow
|
||||||
cleanupTimeout = setTimeout(() => {
|
const cleanupTimeout = setTimeout(() => {
|
||||||
if (activeFlows.has(state)) {
|
if (activeFlow?.state === state) {
|
||||||
console.log(`[OAuth] Cleaning up abandoned OAuth flow for ${provider} (timeout)`);
|
console.log(`[OAuth] Cleaning up abandoned OAuth flow for ${provider} (timeout)`);
|
||||||
activeFlows.delete(state);
|
cancelActiveFlow('timed_out');
|
||||||
server.close();
|
|
||||||
emitOAuthEvent({ provider, success: false, error: 'OAuth flow timed out' });
|
|
||||||
}
|
}
|
||||||
}, 5 * 60 * 1000); // 5 minutes
|
}, 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)
|
// Open in system browser (shares cookies/sessions with user's regular browser)
|
||||||
shell.openExternal(authUrl.toString());
|
shell.openExternal(authUrl.toString());
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue