diff --git a/src/main/auth/firebaseBridge/copyLinkBanner.ts b/src/main/auth/firebaseBridge/copyLinkBanner.ts index 89423954..6a500d74 100644 --- a/src/main/auth/firebaseBridge/copyLinkBanner.ts +++ b/src/main/auth/firebaseBridge/copyLinkBanner.ts @@ -1,12 +1,12 @@ /** * Sign-in "copy login link" card. * - * When `handleFirebasePopup` opens the loopback login URL in the user's - * DEFAULT browser, that browser may not be where they're signed into - * Google/GitHub. We inject a small floating card into the embedded Cloud - * view (the surface the user is looking at) offering "Copy link" / "Open - * again" so they can finish sign-in in a browser of their choice — the - * same affordance Notion / Claude / Zoom provide. + * When `handleFirebasePopup` opens the Cloud login URL in the user's DEFAULT + * browser, that browser may not be where they're signed into Google/GitHub. We + * inject a small floating card into the embedded Cloud view (the surface the + * user is looking at) offering "Copy link" / "Open again" so they can finish + * sign-in in a browser of their choice — the same affordance Notion / Claude / + * Zoom provide. * * Injected with `insertCSS` + `executeJavaScript`, like * `injectMacPasskeyWarning`. The URL is string-baked via `JSON.stringify` @@ -81,7 +81,7 @@ export function buildCopyLinkBannerScript(url: string, labels: CopyLinkBannerLab copy: JSON.stringify(labels.copy), copied: JSON.stringify(labels.copied), openAgain: JSON.stringify(labels.openAgain), - dismiss: JSON.stringify(labels.dismiss), + dismiss: JSON.stringify(labels.dismiss) } return `(function(){try{ var URL=${u}, ID=${id}; diff --git a/src/main/auth/firebaseBridge/index.ts b/src/main/auth/firebaseBridge/index.ts index 7dc460b9..c45846ce 100644 --- a/src/main/auth/firebaseBridge/index.ts +++ b/src/main/auth/firebaseBridge/index.ts @@ -1,6 +1,7 @@ +import { randomBytes } from 'node:crypto' + import { shell, type BrowserWindow, type WebContents } from 'electron' -import { detectFirebaseEnv } from './config' import { buildCopyLinkBannerScript, buildRemoveCopyLinkBannerScript, @@ -9,7 +10,7 @@ import { } from './copyLinkBanner' import { buildIndexedDbInjectScript } from './inject' import { extractProviderId, type SupportedProvider } from './intercept' -import { startBridgeServer } from './server' +import { startCloudLoginCallbackServer, type BridgeHandle } from './server' import * as i18n from '../../lib/i18n' import * as mainTelemetry from '../../lib/telemetry' @@ -81,15 +82,16 @@ export interface HandleFirebasePopupOpts { * * Flow: * 1. Detect prod/dev project + IdP from the intercepted URL. - * 2. Spin up a loopback HTTP server with a bridge page that runs - * `signInWithPopup` in the user's system browser (passkeys + - * saved-passwords + existing IdP sessions all work there). - * 3. Await the bridge's `/callback` carrying `auth.currentUser.toJSON()`. - * 4. Inject the serialized user into the embedded view's + * 2. Spin up a loopback HTTP server that receives the completed Cloud login. + * 3. Open the real Cloud login page in the user's system browser so + * comfy.org PostHog cookies, passkeys, saved passwords, and existing + * IdP sessions all work there. + * 4. Await the bridge's `/callback` carrying `auth.currentUser.toJSON()`. + * 5. Inject the serialized user into the embedded view's * `firebaseLocalStorageDb` IndexedDB and reload — Firebase's SDK * rehydrates from persistence on init, fires `onAuthStateChanged`, * and the existing `/auth/session` post handles the rest. - * 5. Focus the Desktop window so the user is yanked back into the + * 6. Focus the Desktop window so the user is yanked back into the * app without needing to alt-tab from their browser. * * Errors are reported via the optional `onError` callback (the caller @@ -107,7 +109,7 @@ export interface HandleFirebasePopupOpts { * an open browser tab, we close the stale bridge (freeing the port) * before spinning up the new one. */ -let activeBridge: Awaited> | null = null +let activeBridge: BridgeHandle | null = null /** * Teardown for the in-flight "copy login link" card: removes the injected @@ -171,6 +173,33 @@ function showCopyLinkBanner(comfyContents: WebContents, loginUrl: string): void */ const POST_SIGNIN_HOLD_MS = 3000 +function getCloudLoginOrigin(comfyContents: WebContents): string { + try { + const currentUrl = new URL(comfyContents.getURL()) + if (currentUrl.protocol === 'https:' || currentUrl.protocol === 'http:') { + return currentUrl.origin + } + } catch { + // Fall through to production Cloud. + } + return 'https://cloud.comfy.org' +} + +function buildCloudDesktopLoginUrl( + comfyContents: WebContents, + callbackUrl: string, + state: string +): string { + const loginUrl = new URL('/cloud/login', getCloudLoginOrigin(comfyContents)) + loginUrl.searchParams.set('desktop_login_callback', callbackUrl) + loginUrl.searchParams.set('desktop_login_state', state) + return loginUrl.href +} + +function createDesktopLoginState(): string { + return randomBytes(24).toString('base64url') +} + export async function handleFirebasePopup( url: string, comfyContents: WebContents, @@ -185,8 +214,6 @@ export async function handleFirebasePopup( // `provider` splits Google vs GitHub conversion + failure rates. The // success leg is emitted by bindSignedInUser's app:user_logged_in. mainTelemetry.capture('comfy.desktop.auth.sign_in_started', { provider: providerId }) - const env = detectFirebaseEnv(url) - // Kill any stale bridge from a prior sign-in attempt the user // didn't complete. Without this, the second Sign-in click hits an // EADDRINUSE on the fixed loopback port and the user sees an @@ -204,19 +231,13 @@ export async function handleFirebasePopup( // new attempt doesn't stack a second card or leak a stale listener. runBannerCleanup() - let handle: Awaited> | null = null + let handle: BridgeHandle | null = null try { - handle = await startBridgeServer({ env, providerId }) + const state = createDesktopLoginState() + handle = await startCloudLoginCallbackServer({ state }) activeBridge = handle - // Append a per-attempt nonce so browsers don't focus an existing - // stale tab from a previous (perhaps wrong-provider) sign-in - // attempt. macOS Chrome / Safari treat shell.openExternal of an - // identical URL as "focus the open tab" rather than "open fresh" - // — without the nonce the user would still see yesterday's GitHub - // bridge page when they intended to start a new Google flow. - // Capture the full nonce'd URL once so the auto-opened tab, the - // "Copy link" button, and "Open again" all hand out the same link. - const loginUrl = `${handle.url}?n=${Date.now().toString(36)}` + const callbackUrl = new URL('callback', handle.url).href + const loginUrl = buildCloudDesktopLoginUrl(comfyContents, callbackUrl, state) void shell.openExternal(loginUrl) // Surface a Notion/Claude-style "didn't open? copy the link" card in // the Cloud view so users can finish sign-in in a non-default browser. diff --git a/src/main/auth/firebaseBridge/server.test.ts b/src/main/auth/firebaseBridge/server.test.ts index 55fa7efd..988f807d 100644 --- a/src/main/auth/firebaseBridge/server.test.ts +++ b/src/main/auth/firebaseBridge/server.test.ts @@ -1,6 +1,50 @@ +import { request } from 'node:http' + import { describe, expect, it } from 'vitest' -import { BRIDGE_PORT, startBridgeServer } from './server' +import { BRIDGE_PORT, startBridgeServer, startCloudLoginCallbackServer } from './server' + +function requestRaw( + url: URL, + opts: { + method: string + origin?: string + body?: string + headers?: Record + } +): Promise<{ + status: number + headers: Record + body: string +}> { + return new Promise((resolve, reject) => { + const req = request( + url, + { + method: opts.method, + headers: { + ...(opts.origin ? { Origin: opts.origin } : {}), + ...(opts.body ? { 'Content-Type': 'application/json' } : {}), + ...opts.headers + } + }, + (res) => { + const chunks: Buffer[] = [] + res.on('data', (chunk: Buffer) => chunks.push(chunk)) + res.on('end', () => + resolve({ + status: res.statusCode ?? 0, + headers: res.headers, + body: Buffer.concat(chunks).toString('utf8') + }) + ) + } + ) + req.on('error', reject) + if (opts.body) req.write(opts.body) + req.end() + }) +} describe('startBridgeServer', () => { it('serves a 204 for /favicon.ico', async () => { @@ -32,7 +76,7 @@ describe('startBridgeServer', () => { try { const res = await fetch( `${handle.url}?error=access_denied&error_description=user+cancelled`, - { redirect: 'manual' }, + { redirect: 'manual' } ) expect(res.status).toBe(200) const body = await res.text() @@ -85,4 +129,68 @@ describe('startBridgeServer', () => { // OAuth client's redirect-URI allowlist is the constant itself. expect(BRIDGE_PORT).toBe(9876) }) + + it('accepts a Cloud login callback with matching state', async () => { + const handle = await startCloudLoginCallbackServer({ state: 'state-123', port: 0 }) + try { + const res = await requestRaw(new URL('callback', handle.url), { + method: 'POST', + origin: 'https://cloud.comfy.org', + body: JSON.stringify({ + state: 'state-123', + apiKey: 'api-key', + user: { uid: 'user-123' } + }) + }) + expect(res.status).toBe(204) + expect(res.headers['access-control-allow-origin']).toBe('https://cloud.comfy.org') + await expect(handle.signInPromise).resolves.toEqual({ + apiKey: 'api-key', + user: { uid: 'user-123' } + }) + } finally { + handle.close() + } + }) + + it('rejects a Cloud login callback with mismatched state', async () => { + const handle = await startCloudLoginCallbackServer({ state: 'state-123', port: 0 }) + const rejected = expect(handle.signInPromise).rejects.toThrow(/state mismatch/) + try { + const res = await fetch(new URL('callback', handle.url), { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Origin: 'https://cloud.comfy.org' + }, + body: JSON.stringify({ + state: 'wrong', + apiKey: 'api-key', + user: { uid: 'user-123' } + }) + }) + expect(res.status).toBe(403) + await rejected + } finally { + handle.close() + } + }) + + it('preflights Cloud login callbacks from allowed origins', async () => { + const handle = await startCloudLoginCallbackServer({ state: 'state-123', port: 0 }) + try { + const res = await requestRaw(new URL('callback', handle.url), { + method: 'OPTIONS', + origin: 'https://cloud.comfy.org', + headers: { + 'Access-Control-Request-Method': 'POST' + } + }) + expect(res.status).toBe(204) + expect(res.headers['access-control-allow-origin']).toBe('https://cloud.comfy.org') + expect(res.headers['access-control-allow-methods']).toContain('POST') + } finally { + handle.close() + } + }) }) diff --git a/src/main/auth/firebaseBridge/server.ts b/src/main/auth/firebaseBridge/server.ts index 2c95906b..3c6457ed 100644 --- a/src/main/auth/firebaseBridge/server.ts +++ b/src/main/auth/firebaseBridge/server.ts @@ -4,11 +4,7 @@ import type { AddressInfo } from 'node:net' import { renderDoneHtml, renderErrorHtml, renderPopupBridgeHtml } from './bridgeHtml' import { getFirebaseConfig, type FirebaseEnv } from './config' import type { SupportedProvider } from './intercept' -import { - buildPersistedUser, - createOauthAuthUri, - signInWithIdpExchange, -} from './oauth' +import { buildPersistedUser, createOauthAuthUri, signInWithIdpExchange } from './oauth' const MAX_BODY_BYTES = 64 * 1024 @@ -68,6 +64,40 @@ export interface StartBridgeOpts { port?: number } +export interface StartCloudLoginCallbackServerOpts { + state: string + /** Default 5 minutes — long enough for password managers, 2FA, account-picker UI. */ + timeoutMs?: number + /** Override the loopback port. Defaults to `BRIDGE_PORT` (9876). Tests pass `0` to get a kernel-assigned port. */ + port?: number +} + +function isAllowedCloudOrigin(origin: string | undefined): boolean { + if (!origin) return true + try { + const url = new URL(origin) + if (url.protocol !== 'https:' && url.protocol !== 'http:') return false + if (url.hostname === 'cloud.comfy.org') return true + if (url.hostname === 'localhost' || url.hostname === '127.0.0.1') return true + return false + } catch { + return false + } +} + +function setCorsHeaders(req: IncomingMessage, res: ServerResponse): boolean { + const origin = req.headers.origin + if (!isAllowedCloudOrigin(origin)) return false + if (origin) { + res.setHeader('Access-Control-Allow-Origin', origin) + res.setHeader('Vary', 'Origin') + } + res.setHeader('Access-Control-Allow-Methods', 'POST, OPTIONS') + res.setHeader('Access-Control-Allow-Headers', 'Content-Type') + res.setHeader('Access-Control-Max-Age', '600') + return true +} + /** * Start the loopback bridge, binding `127.0.0.1:` and advertising * `http://localhost:`. Routes: `GET /` initiates (302 to IdP or serves @@ -218,7 +248,7 @@ export function startBridgeServer(opts: StartBridgeOpts): Promise firebaseConfig.apiKey, providerId, requestUri, - sessionId, + sessionId ) const persistedUser = buildPersistedUser(firebaseConfig, idpResponse, providerId) res.statusCode = 200 @@ -262,3 +292,154 @@ export function startBridgeServer(opts: StartBridgeOpts): Promise }) }) } + +/** + * Start a localhost receiver for the real Cloud login page. Unlike + * `startBridgeServer`, this does not host Firebase UI on localhost. Desktop + * opens cloud.comfy.org in the system browser so PostHog can identify the + * browser's first-party comfy.org cookie, then Cloud POSTs the Firebase user + * back to this callback. + */ +export function startCloudLoginCallbackServer( + opts: StartCloudLoginCallbackServerOpts +): Promise { + const { state, timeoutMs = 5 * 60_000, port = BRIDGE_PORT } = opts + + return new Promise((resolveHandle, rejectHandle) => { + let resolved = false + let signInResolve!: (r: SignInResult) => void + let signInReject!: (err: Error) => void + const signInPromise = new Promise((res, rej) => { + signInResolve = res + signInReject = rej + }) + + let server: Server | null = null + + const close = (): void => { + if (server) { + try { + server.closeAllConnections?.() + server.close() + } catch { + // best-effort shutdown + } + server = null + } + clearTimeout(timeoutHandle) + } + + const finishWithError = (err: Error): void => { + if (!resolved) { + resolved = true + signInReject(err) + } + } + + const finishWithSuccess = (result: SignInResult): void => { + if (!resolved) { + resolved = true + signInResolve(result) + } + } + + const timeoutHandle = setTimeout(() => { + finishWithError(new Error('Cloud login bridge timed out waiting for sign-in')) + close() + }, timeoutMs) + + server = createServer((req, res) => { + res.setHeader('Connection', 'close') + void handleRequest(req, res).catch((err: unknown) => { + const msg = err instanceof Error ? err.message : String(err) + if (!res.headersSent) { + res.statusCode = 500 + setCorsHeaders(req, res) + res.setHeader('Content-Type', 'text/plain; charset=utf-8') + res.end(msg) + } + finishWithError(err instanceof Error ? err : new Error(msg)) + }) + }) + + async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise { + const remoteHost = req.socket.remoteAddress ?? '' + const isLoopback = + remoteHost === '127.0.0.1' || remoteHost === '::1' || remoteHost === '::ffff:127.0.0.1' + if (!isLoopback) { + res.statusCode = 403 + res.end() + return + } + + const url = req.url ?? '/' + const queryStart = url.indexOf('?') + const path = queryStart >= 0 ? url.slice(0, queryStart) : url + + if (req.method === 'GET' && path === '/favicon.ico') { + res.statusCode = 204 + res.end() + return + } + + if (path !== '/callback') { + res.statusCode = 404 + res.end() + return + } + + if (!setCorsHeaders(req, res)) { + res.statusCode = 403 + res.end() + return + } + + if (req.method === 'OPTIONS') { + res.statusCode = 204 + res.end() + return + } + + if (req.method !== 'POST') { + res.statusCode = 405 + res.end() + return + } + + const body = (await readJsonBody(req)) as { + state?: unknown + user?: unknown + apiKey?: unknown + } + if (body.state !== state) { + res.statusCode = 403 + res.setHeader('Content-Type', 'text/plain; charset=utf-8') + res.end('Invalid state') + finishWithError(new Error('Cloud login callback state mismatch')) + return + } + if (!body.user || typeof body.user !== 'object' || typeof body.apiKey !== 'string') { + res.statusCode = 400 + res.setHeader('Content-Type', 'text/plain; charset=utf-8') + res.end('Missing user payload') + return + } + + res.statusCode = 204 + res.end() + finishWithSuccess({ user: body.user as Record, apiKey: body.apiKey }) + } + + server.on('error', (err: Error) => { + finishWithError(err) + rejectHandle(err) + close() + }) + + server.listen(port, '127.0.0.1', () => { + const addr = server!.address() as AddressInfo + const url = `http://localhost:${addr.port}/` + resolveHandle({ url, signInPromise, close }) + }) + }) +}