mirror of
https://github.com/rowboatlabs/rowboat.git
synced 2026-05-02 20:03:21 +02:00
fix(oauth): full callback URL, Google clientId, refresh, and review follow-ups
- Pass full OAuth callback URL through auth-server for openid-client validation - Composio + Google flows: duplicate-callback guard; preserve timeout cleanup - Persist and expose Google clientId via oauth:getState; hydrate UI from useConnectors - getAccessToken returns refreshed credentials; clearer errors and missing-state handling - IPC schema: per-provider userId + clientId - Docs: google-setup redirect URI and troubleshooting Made-with: Cursor
This commit is contained in:
parent
598aeb59cc
commit
e1c6758a3f
8 changed files with 101 additions and 20 deletions
|
|
@ -22,10 +22,11 @@ export interface AuthServerResult {
|
|||
/**
|
||||
* Create a local HTTP server to handle OAuth callback
|
||||
* 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: (params: Record<string, string>) => void | Promise<void>
|
||||
onCallback: (callbackUrl: URL) => void | Promise<void>
|
||||
): Promise<AuthServerResult> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const server = createServer((req, res) => {
|
||||
|
|
@ -38,8 +39,6 @@ export function createAuthServer(
|
|||
const url = new URL(req.url, `http://localhost:${port}`);
|
||||
|
||||
if (url.pathname === OAUTH_CALLBACK_PATH) {
|
||||
const code = url.searchParams.get('code');
|
||||
const state = url.searchParams.get('state');
|
||||
const error = url.searchParams.get('error');
|
||||
|
||||
if (error) {
|
||||
|
|
@ -65,9 +64,8 @@ export function createAuthServer(
|
|||
return;
|
||||
}
|
||||
|
||||
// Handle callback - either traditional OAuth with code/state or Composio-style notification
|
||||
// Composio callbacks may not have code/state, just a notification that the flow completed
|
||||
onCallback(Object.fromEntries(url.searchParams.entries()));
|
||||
// Handle callback - pass full URL so params like iss (OpenID Connect) are preserved for token exchange
|
||||
onCallback(url);
|
||||
|
||||
res.writeHead(200, { 'Content-Type': 'text/html' });
|
||||
res.end(`
|
||||
|
|
|
|||
|
|
@ -151,7 +151,7 @@ export async function initiateConnection(toolkitSlug: string): Promise<{
|
|||
// Set up callback server
|
||||
const timeoutRef: { current: NodeJS.Timeout | null } = { current: null };
|
||||
let callbackHandled = false;
|
||||
const { server } = await createAuthServer(8081, async () => {
|
||||
const { server } = await createAuthServer(8081, async (_callbackUrl) => {
|
||||
// Guard against duplicate callbacks (browser may send multiple requests)
|
||||
if (callbackHandled) return;
|
||||
callbackHandled = true;
|
||||
|
|
|
|||
|
|
@ -14,6 +14,43 @@ import { emitOAuthEvent } from './ipc.js';
|
|||
|
||||
const REDIRECT_URI = 'http://localhost:8080/oauth/callback';
|
||||
|
||||
/** Top-level openid-client messages that often wrap a more specific cause. */
|
||||
const OPAQUE_OAUTH_TOP_MESSAGES = new Set(['invalid response encountered']);
|
||||
|
||||
function firstCauseMessage(error: unknown): string | undefined {
|
||||
if (error == null || typeof error !== 'object' || !('cause' in error)) {
|
||||
return undefined;
|
||||
}
|
||||
const cause = (error as { cause?: unknown }).cause;
|
||||
if (cause instanceof Error && cause.message.trim()) {
|
||||
return cause.message;
|
||||
}
|
||||
if (typeof cause === 'string' && cause.trim()) {
|
||||
return cause;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* User-facing message for token-exchange failures. Prefer the first cause message when
|
||||
* the top-level message is opaque (common for openid-client) or when code is OAUTH_INVALID_RESPONSE.
|
||||
* The catch block below still logs the full cause chain for any error; this helper stays conservative.
|
||||
*/
|
||||
function getOAuthErrorMessage(error: unknown): string {
|
||||
const msg = error instanceof Error ? error.message : 'Unknown error';
|
||||
const code = error != null && typeof error === 'object' && 'code' in error
|
||||
? (error as { code?: string }).code
|
||||
: undefined;
|
||||
const causeMsg = firstCauseMessage(error);
|
||||
if (code === 'OAUTH_INVALID_RESPONSE' && causeMsg) {
|
||||
return causeMsg;
|
||||
}
|
||||
if (causeMsg && OPAQUE_OAUTH_TOP_MESSAGES.has(msg.trim().toLowerCase())) {
|
||||
return causeMsg;
|
||||
}
|
||||
return msg;
|
||||
}
|
||||
|
||||
// Store active OAuth flows (state -> { codeVerifier, provider, config })
|
||||
const activeFlows = new Map<string, {
|
||||
codeVerifier: string;
|
||||
|
|
@ -167,6 +204,11 @@ export async function connectProvider(provider: string, clientId?: string): Prom
|
|||
// Get or create OAuth configuration
|
||||
const config = await getProviderConfiguration(provider, clientId);
|
||||
|
||||
// Persist Google client ID so it survives restarts and failed token exchanges
|
||||
if (provider === 'google' && clientId) {
|
||||
await oauthRepo.upsert(provider, { clientId });
|
||||
}
|
||||
|
||||
// Generate PKCE codes
|
||||
const { verifier: codeVerifier, challenge: codeChallenge } = await oauthClient.generatePKCE();
|
||||
const state = oauthClient.generateState();
|
||||
|
|
@ -187,12 +229,17 @@ export async function connectProvider(provider: string, clientId?: string): Prom
|
|||
|
||||
// Create callback server
|
||||
let callbackHandled = false;
|
||||
const { server } = await createAuthServer(8080, async (params: Record<string, string>) => {
|
||||
const { server } = await createAuthServer(8080, async (callbackUrl) => {
|
||||
// Guard against duplicate callbacks (browser may send multiple requests)
|
||||
if (callbackHandled) return;
|
||||
callbackHandled = true;
|
||||
// Validate state
|
||||
if (params.state !== state) {
|
||||
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');
|
||||
}
|
||||
|
||||
|
|
@ -202,10 +249,7 @@ export async function connectProvider(provider: string, clientId?: string): Prom
|
|||
}
|
||||
|
||||
try {
|
||||
// Build callback URL for token exchange
|
||||
const callbackUrl = new URL(`${REDIRECT_URI}?${new URLSearchParams(params).toString()}`);
|
||||
|
||||
// Exchange code for tokens
|
||||
// Use full callback URL (includes iss, scope, etc.) so openid-client validation succeeds
|
||||
console.log(`[OAuth] Exchanging authorization code for tokens (${provider})...`);
|
||||
const tokens = await oauthClient.exchangeCodeForTokens(
|
||||
flow.config,
|
||||
|
|
@ -234,7 +278,15 @@ export async function connectProvider(provider: string, clientId?: string): Prom
|
|||
emitOAuthEvent({ provider, success: true });
|
||||
} catch (error) {
|
||||
console.error('OAuth token exchange failed:', error);
|
||||
const errorMessage = error instanceof Error ? error.message : 'Unknown error';
|
||||
// Log cause chain for debugging (e.g. OAUTH_INVALID_RESPONSE -> OperationProcessingError)
|
||||
let cause: unknown = error;
|
||||
while (cause != null && typeof cause === 'object' && 'cause' in cause) {
|
||||
cause = (cause as { cause?: unknown }).cause;
|
||||
if (cause != null) {
|
||||
console.error('[OAuth] Caused by:', cause);
|
||||
}
|
||||
}
|
||||
const errorMessage = getOAuthErrorMessage(error);
|
||||
emitOAuthEvent({ provider, success: false, error: errorMessage });
|
||||
throw error;
|
||||
} finally {
|
||||
|
|
@ -302,8 +354,8 @@ export async function disconnectProvider(provider: string): Promise<{ success: b
|
|||
export async function getAccessToken(provider: string): Promise<string | null> {
|
||||
try {
|
||||
const oauthRepo = getOAuthRepo();
|
||||
|
||||
const { tokens } = await oauthRepo.read(provider);
|
||||
|
||||
let { tokens } = await oauthRepo.read(provider);
|
||||
if (!tokens) {
|
||||
return null;
|
||||
}
|
||||
|
|
@ -319,11 +371,12 @@ export async function getAccessToken(provider: string): Promise<string | null> {
|
|||
try {
|
||||
// Get configuration for refresh
|
||||
const config = await getProviderConfiguration(provider);
|
||||
|
||||
|
||||
// Refresh token, preserving existing scopes
|
||||
const existingScopes = tokens.scopes;
|
||||
const refreshedTokens = await oauthClient.refreshTokens(config, tokens.refresh_token, existingScopes);
|
||||
await oauthRepo.upsert(provider, { tokens });
|
||||
await oauthRepo.upsert(provider, { tokens: refreshedTokens });
|
||||
tokens = refreshedTokens;
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : 'Token refresh failed';
|
||||
await oauthRepo.upsert(provider, { error: message });
|
||||
|
|
|
|||
|
|
@ -517,6 +517,10 @@ export function OnboardingModal({ open, onComplete }: OnboardingModalProps) {
|
|||
isConnecting: false,
|
||||
}
|
||||
}
|
||||
// Hydrate in-memory Google client ID from persisted config so Connect can skip re-entry
|
||||
if (config.google?.clientId) {
|
||||
setGoogleClientId(config.google.clientId)
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('Failed to check connection status for providers:', error)
|
||||
for (const provider of providers) {
|
||||
|
|
|
|||
|
|
@ -426,6 +426,9 @@ export function useConnectors(active: boolean) {
|
|||
try {
|
||||
const result = await window.ipc.invoke('oauth:getState', null)
|
||||
const config = result.config || {}
|
||||
if (config.google?.clientId) {
|
||||
setGoogleClientId(config.google.clientId)
|
||||
}
|
||||
const statusMap: Record<string, ProviderStatus> = {}
|
||||
|
||||
for (const provider of providers) {
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ const OAuthConfigSchema = z.object({
|
|||
const ClientFacingConfigSchema = z.record(z.string(), z.object({
|
||||
connected: z.boolean(),
|
||||
error: z.string().nullable().optional(),
|
||||
clientId: z.string().nullable().optional(),
|
||||
}));
|
||||
|
||||
const LegacyOauthConfigSchema = z.record(z.string(), OAuthTokens);
|
||||
|
|
@ -111,8 +112,9 @@ export class FSOAuthRepo implements IOAuthRepo {
|
|||
clientFacingConfig[provider] = {
|
||||
connected: !!providerConfig.tokens,
|
||||
error: providerConfig.error,
|
||||
clientId: providerConfig.clientId ?? null,
|
||||
};
|
||||
}
|
||||
return clientFacingConfig;
|
||||
return ClientFacingConfigSchema.parse(clientFacingConfig);
|
||||
}
|
||||
}
|
||||
|
|
@ -252,6 +252,7 @@ const ipcSchemas = {
|
|||
connected: z.boolean(),
|
||||
error: z.string().nullable().optional(),
|
||||
userId: z.string().optional(),
|
||||
clientId: z.string().nullable().optional(),
|
||||
})),
|
||||
}),
|
||||
},
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue