From b14be979657447b533b3365265923b96c63e2897 Mon Sep 17 00:00:00 2001 From: cyph3rasi Date: Tue, 14 Jul 2026 23:23:56 -0700 Subject: [PATCH] Make Stuffbox linking resilient to cross-origin popup isolation and stale callbacks Hop-State: A_06FP8WHY25K99094H3X60A8 Hop-Proposal: R_06FP8WGCVDDG61H0VZ1XQJR Hop-Task: T_06FP8V5D2HSJG08NRN6VS3G Hop-Attempt: AT_06FP8V5D2KATGJNV2M2SGRR --- src/app/api/storage/configuration/route.ts | 1 + .../api/storage/stuffbox/callback/route.ts | 16 +- src/app/api/storage/stuffbox/connect/route.ts | 6 +- src/components/StorageConfigurationPrompt.tsx | 166 ++++++++++-------- src/lib/stuffbox/browser-upload.test.ts | 25 +++ src/lib/stuffbox/browser-upload.ts | 16 ++ src/lib/stuffbox/connection-monitor.test.ts | 79 +++++++++ src/lib/stuffbox/connection-monitor.ts | 101 +++++++++++ src/lib/stuffbox/popup-response.test.ts | 3 +- src/lib/stuffbox/popup-response.ts | 4 +- 10 files changed, 332 insertions(+), 85 deletions(-) create mode 100644 src/lib/stuffbox/browser-upload.test.ts create mode 100644 src/lib/stuffbox/connection-monitor.test.ts create mode 100644 src/lib/stuffbox/connection-monitor.ts diff --git a/src/app/api/storage/configuration/route.ts b/src/app/api/storage/configuration/route.ts index 48403e4..3678d32 100644 --- a/src/app/api/storage/configuration/route.ts +++ b/src/app/api/storage/configuration/route.ts @@ -28,6 +28,7 @@ export async function GET() { provider: stuffbox ? 'stuffbox' : user.storageProvider ? 's3' : null, stuffboxAvailable: Boolean(configuredStuffboxUrl()), stuffboxBaseUrl: stuffbox?.baseUrl ?? null, + stuffboxUpdatedAt: stuffbox?.updatedAt.toISOString() ?? null, s3Provider: user.storageProvider ?? null, }); } catch (error) { diff --git a/src/app/api/storage/stuffbox/callback/route.ts b/src/app/api/storage/stuffbox/callback/route.ts index e09f928..198a8c6 100644 --- a/src/app/api/storage/stuffbox/callback/route.ts +++ b/src/app/api/storage/stuffbox/callback/route.ts @@ -5,8 +5,8 @@ import { consumeStuffboxConnectionState } from '@/lib/stuffbox/connection-state' import { saveStuffboxTokens } from '@/lib/stuffbox/tokens'; import { renderStuffboxPopupResponse } from '@/lib/stuffbox/popup-response'; -function popupResponse(origin: string, success: boolean, message: string): NextResponse { - return new NextResponse(renderStuffboxPopupResponse(origin, success, message), { +function popupResponse(origin: string, success: boolean, message: string, attemptId?: string): NextResponse { + return new NextResponse(renderStuffboxPopupResponse(origin, success, message, attemptId), { status: success ? 200 : 400, headers: { 'Content-Type': 'text/html; charset=utf-8', 'Cache-Control': 'no-store' }, }); @@ -14,16 +14,18 @@ function popupResponse(origin: string, success: boolean, message: string): NextR export async function GET(request: NextRequest) { const origin = request.nextUrl.origin; + let attemptId: string | undefined; try { const user = await requireAuth(); const pending = await consumeStuffboxConnectionState(); + attemptId = pending?.state; const code = request.nextUrl.searchParams.get('code'); const state = request.nextUrl.searchParams.get('state'); const denied = request.nextUrl.searchParams.get('error'); - if (denied) return popupResponse(origin, false, 'Stuffbox access was not approved.'); + if (denied) return popupResponse(origin, false, 'Stuffbox access was not approved.', attemptId); if (!pending || pending.userId !== user.id || !code || state !== pending.state) { - return popupResponse(origin, false, 'The Stuffbox connection request is invalid or expired.'); + return popupResponse(origin, false, 'The Stuffbox connection request is invalid or expired.', attemptId); } const tokens = await exchangeAuthorizationCode(pending.baseUrl, { @@ -32,12 +34,12 @@ export async function GET(request: NextRequest) { redirectUri: pending.redirectUri, }); await saveStuffboxTokens(user.id, pending.baseUrl, tokens); - return popupResponse(new URL(pending.redirectUri).origin, true, 'Stuffbox connected.'); + return popupResponse(new URL(pending.redirectUri).origin, true, 'Stuffbox connected.', attemptId); } catch (error) { if (error instanceof StuffboxApiError) { - return popupResponse(origin, false, error.message); + return popupResponse(origin, false, error.message, attemptId); } console.error('Stuffbox callback error:', error); - return popupResponse(origin, false, 'Stuffbox could not be connected.'); + return popupResponse(origin, false, 'Stuffbox could not be connected.', attemptId); } } diff --git a/src/app/api/storage/stuffbox/connect/route.ts b/src/app/api/storage/stuffbox/connect/route.ts index 5669b1c..fd1007d 100644 --- a/src/app/api/storage/stuffbox/connect/route.ts +++ b/src/app/api/storage/stuffbox/connect/route.ts @@ -49,7 +49,11 @@ export async function POST(request: NextRequest) { expiresAt: Math.min(Date.parse(connection.expiresAt) || Date.now() + 10 * 60_000, Date.now() + 10 * 60_000), }); - return NextResponse.json({ authorizationUrl: connection.authorizationUrl }); + return NextResponse.json({ + authorizationUrl: connection.authorizationUrl, + connectionStartedAt: new Date().toISOString(), + connectionAttempt: pkce.state, + }); } catch (error) { if (error instanceof Error && error.message === 'Authentication required') { return NextResponse.json({ error: 'Authentication required' }, { status: 401 }); diff --git a/src/components/StorageConfigurationPrompt.tsx b/src/components/StorageConfigurationPrompt.tsx index cf6eeb3..62ced84 100644 --- a/src/components/StorageConfigurationPrompt.tsx +++ b/src/components/StorageConfigurationPrompt.tsx @@ -1,8 +1,13 @@ 'use client'; -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { Box, ChevronDown, ExternalLink } from 'lucide-react'; -import { getStorageProvider } from '@/lib/stuffbox/browser-upload'; +import { hasNewStuffboxConnection } from '@/lib/stuffbox/browser-upload'; +import { + monitorStuffboxConnection, + StuffboxConnectionCancelledError, + type StuffboxConnectionResult, +} from '@/lib/stuffbox/connection-monitor'; interface StorageConfigurationPromptProps { open: boolean; @@ -25,6 +30,34 @@ export function StorageConfigurationPrompt({ open, onConfigured, onCancel, varia const [isLoading, setIsLoading] = useState(false); const [stuffboxAvailable, setStuffboxAvailable] = useState(false); const [showS3, setShowS3] = useState(false); + const [isConnectingStuffbox, setIsConnectingStuffbox] = useState(false); + const connectionAbortRef = useRef(null); + const connectionPopupRef = useRef(null); + + const closeConnectionPopup = () => { + try { connectionPopupRef.current?.close(); } catch { /* COOP may sever the popup handle. */ } + connectionPopupRef.current = null; + }; + + const cancelStuffboxConnection = () => { + connectionAbortRef.current?.abort(); + connectionAbortRef.current = null; + closeConnectionPopup(); + setIsConnectingStuffbox(false); + }; + + useEffect(() => () => { + connectionAbortRef.current?.abort(); + try { connectionPopupRef.current?.close(); } catch { /* Best-effort cleanup. */ } + }, []); + + useEffect(() => { + if (open) return; + connectionAbortRef.current?.abort(); + connectionAbortRef.current = null; + try { connectionPopupRef.current?.close(); } catch { /* Best-effort cleanup. */ } + connectionPopupRef.current = null; + }, [open]); useEffect(() => { if (!open) return; @@ -47,92 +80,72 @@ export function StorageConfigurationPrompt({ open, onConfigured, onCancel, varia const connectStuffbox = async () => { setError(''); - setIsSubmitting(true); + setIsConnectingStuffbox(true); const popup = window.open('', 'synapsis-stuffbox', 'popup,width=620,height=760'); if (!popup) { setError('Your browser blocked the Stuffbox window. Allow popups and try again.'); - setIsSubmitting(false); + setIsConnectingStuffbox(false); return; } + connectionPopupRef.current = popup; popup.document.body.textContent = 'Connecting to Stuffbox…'; + const controller = new AbortController(); + connectionAbortRef.current = controller; try { - const response = await fetch('/api/storage/stuffbox/connect', { method: 'POST' }); + const response = await fetch('/api/storage/stuffbox/connect', { + method: 'POST', + signal: controller.signal, + }); const data = await response.json().catch(() => ({})); - if (!response.ok || !data.authorizationUrl) throw new Error(data.error || 'Unable to connect Stuffbox'); - const result = new Promise((resolve, reject) => { - let settled = false; - let verifyingClosedPopup = false; - let channel: BroadcastChannel | null = null; - const timeout = window.setTimeout(() => { - finish({ type: 'synapsis:stuffbox', success: false, message: 'Stuffbox connection timed out.' }); - }, 10 * 60_000); - const popupClosed = window.setInterval(() => { - if (popup.closed) void verifyConnectionAfterPopupClosed(); - }, 500); + if (!response.ok || !data.authorizationUrl || !data.connectionStartedAt || !data.connectionAttempt) { + throw new Error(data.error || 'Unable to connect Stuffbox'); + } + const result = monitorStuffboxConnection({ + signal: controller.signal, + checkConnected: async () => hasNewStuffboxConnection(data.connectionStartedAt), + subscribe: (finish) => { + let channel: BroadcastChannel | null = null; + const finishCurrentAttempt = (result: StuffboxConnectionResult) => { + if (result.attemptId === data.connectionAttempt) finish(result); + }; - async function verifyConnectionAfterPopupClosed() { - if (settled || verifyingClosedPopup) return; - verifyingClosedPopup = true; - window.clearInterval(popupClosed); - - for (let attempt = 0; attempt < 4 && !settled; attempt += 1) { - await new Promise(resolveDelay => window.setTimeout(resolveDelay, 250)); - if (settled) return; - try { - if (await getStorageProvider() === 'stuffbox') { - finish({ type: 'synapsis:stuffbox', success: true, message: 'Stuffbox connected.' }); - return; - } - } catch { /* Retry while the callback finishes saving the connection. */ } + function receive(event: MessageEvent) { + if (event.origin !== window.location.origin || event.data?.type !== 'synapsis:stuffbox') return; + finishCurrentAttempt(event.data as StuffboxConnectionResult); } - finish({ type: 'synapsis:stuffbox', success: false, message: 'The Stuffbox window was closed before connecting.' }); - } + function receiveStorage(event: StorageEvent) { + if (event.key !== 'synapsis:stuffbox:result' || !event.newValue) return; + try { finishCurrentAttempt(JSON.parse(event.newValue)); } catch { /* Ignore unrelated storage values. */ } + } - function cleanup() { - window.clearTimeout(timeout); - window.clearInterval(popupClosed); - window.removeEventListener('message', receive); - window.removeEventListener('storage', receiveStorage); - channel?.close(); - } - - function finish(data: { type?: string; success?: boolean; message?: string }) { - if (settled || data.type !== 'synapsis:stuffbox') return; - settled = true; - cleanup(); - window.focus(); - if (data.success) resolve(); - else reject(new Error(data.message || 'Stuffbox could not be connected.')); - } - - function receive(event: MessageEvent) { - if (event.origin !== window.location.origin || event.data?.type !== 'synapsis:stuffbox') return; - finish(event.data); - } - - function receiveStorage(event: StorageEvent) { - if (event.key !== 'synapsis:stuffbox:result' || !event.newValue) return; - try { finish(JSON.parse(event.newValue)); } catch { /* Ignore unrelated storage values. */ } - } - - window.addEventListener('message', receive); - window.addEventListener('storage', receiveStorage); - if ('BroadcastChannel' in window) { - channel = new BroadcastChannel('synapsis:stuffbox'); - channel.onmessage = (event) => finish(event.data); - } + window.addEventListener('message', receive); + window.addEventListener('storage', receiveStorage); + if ('BroadcastChannel' in window) { + channel = new BroadcastChannel('synapsis:stuffbox'); + channel.onmessage = (event) => finishCurrentAttempt(event.data); + } + return () => { + window.removeEventListener('message', receive); + window.removeEventListener('storage', receiveStorage); + channel?.close(); + }; + }, }); popup.location.href = data.authorizationUrl; await result; - if (!popup.closed) popup.close(); + window.focus(); + closeConnectionPopup(); await onConfigured(); } catch (connectError) { - if (!popup.closed) popup.close(); - setError(connectError instanceof Error ? connectError.message : 'Unable to connect Stuffbox'); + closeConnectionPopup(); + if (!controller.signal.aborted && !(connectError instanceof StuffboxConnectionCancelledError)) { + setError(connectError instanceof Error ? connectError.message : 'Unable to connect Stuffbox'); + } } finally { - setIsSubmitting(false); + connectionAbortRef.current = null; + setIsConnectingStuffbox(false); } }; @@ -177,12 +190,17 @@ export function StorageConfigurationPrompt({ open, onConfigured, onCancel, varia - + {isConnectingStuffbox && ( +
+ Approve access in the Stuffbox window. This page will continue automatically. +
+ )} - @@ -214,7 +232,7 @@ export function StorageConfigurationPrompt({ open, onConfigured, onCancel, varia {error &&
{error}
} {variant === 'modal' && ( -
+
)} ); @@ -222,7 +240,7 @@ export function StorageConfigurationPrompt({ open, onConfigured, onCancel, varia if (variant === 'inline') return content; return ( -
+
{ cancelStuffboxConnection(); onCancel(); }}>
event.stopPropagation()}> {content}
diff --git a/src/lib/stuffbox/browser-upload.test.ts b/src/lib/stuffbox/browser-upload.test.ts new file mode 100644 index 0000000..d18072a --- /dev/null +++ b/src/lib/stuffbox/browser-upload.test.ts @@ -0,0 +1,25 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { hasNewStuffboxConnection } from './browser-upload'; + +afterEach(() => vi.unstubAllGlobals()); + +describe('hasNewStuffboxConnection', () => { + it('accepts a connection saved after the current attempt started', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(JSON.stringify({ + provider: 'stuffbox', + stuffboxUpdatedAt: '2026-07-15T06:00:01.000Z', + }), { status: 200 }))); + + await expect(hasNewStuffboxConnection('2026-07-15T06:00:00.000Z')).resolves.toBe(true); + }); + + it('does not mistake an older connection for completion of a new attempt', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(JSON.stringify({ + provider: 'stuffbox', + stuffboxUpdatedAt: '2026-07-15T05:59:59.000Z', + }), { status: 200 }))); + + await expect(hasNewStuffboxConnection('2026-07-15T06:00:00.000Z')).resolves.toBe(false); + }); +}); + diff --git a/src/lib/stuffbox/browser-upload.ts b/src/lib/stuffbox/browser-upload.ts index 3296b34..7f0d860 100644 --- a/src/lib/stuffbox/browser-upload.ts +++ b/src/lib/stuffbox/browser-upload.ts @@ -26,6 +26,7 @@ interface JsonResponse extends Record { error?: string; code?: string; provider?: string | null; + stuffboxUpdatedAt?: string | null; } export type StorageProvider = 'stuffbox' | 's3' | null; @@ -45,6 +46,21 @@ export async function getStorageProvider(): Promise { : null; } +export async function hasNewStuffboxConnection(startedAt: string): Promise { + const response = await fetch('/api/storage/configuration', { cache: 'no-store' }); + const configuration = await json(response); + if (!response.ok) { + throw new MediaUploadError(configuration.error || 'Unable to verify the Stuffbox connection', undefined, response.status); + } + + const updatedAt = typeof configuration.stuffboxUpdatedAt === 'string' + ? Date.parse(configuration.stuffboxUpdatedAt) + : Number.NaN; + return configuration.provider === 'stuffbox' + && Number.isFinite(updatedAt) + && updatedAt >= Date.parse(startedAt); +} + function directPut( uploadUrl: string, file: File, diff --git a/src/lib/stuffbox/connection-monitor.test.ts b/src/lib/stuffbox/connection-monitor.test.ts new file mode 100644 index 0000000..1c83b0b --- /dev/null +++ b/src/lib/stuffbox/connection-monitor.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from 'vitest'; +import { + monitorStuffboxConnection, + StuffboxConnectionCancelledError, + type StuffboxConnectionResult, +} from './connection-monitor'; + +function subscription() { + let receive: ((result: StuffboxConnectionResult) => void) | undefined; + return { + subscribe: (next: (result: StuffboxConnectionResult) => void) => { + receive = next; + return () => { receive = undefined; }; + }, + send: (result: StuffboxConnectionResult) => receive?.(result), + }; +} + +describe('monitorStuffboxConnection', () => { + it('uses persisted server state as the source of truth', async () => { + const messages = subscription(); + let checks = 0; + const result = monitorStuffboxConnection({ + subscribe: messages.subscribe, + checkConnected: async () => ++checks >= 3, + pollIntervalMs: 1, + timeoutMs: 100, + }); + + await expect(result).resolves.toBeUndefined(); + expect(checks).toBeGreaterThanOrEqual(3); + }); + + it('verifies success messages against server state before resolving', async () => { + const messages = subscription(); + let connected = false; + const result = monitorStuffboxConnection({ + subscribe: messages.subscribe, + checkConnected: async () => connected, + pollIntervalMs: 20, + timeoutMs: 100, + }); + + messages.send({ type: 'synapsis:stuffbox', success: true }); + connected = true; + messages.send({ type: 'synapsis:stuffbox', success: true }); + + await expect(result).resolves.toBeUndefined(); + }); + + it('surfaces an explicit denial from Stuffbox', async () => { + const messages = subscription(); + const result = monitorStuffboxConnection({ + subscribe: messages.subscribe, + checkConnected: async () => false, + pollIntervalMs: 20, + timeoutMs: 100, + }); + + messages.send({ type: 'synapsis:stuffbox', success: false, message: 'Access denied.' }); + await expect(result).rejects.toThrow('Access denied.'); + }); + + it('can be cancelled without waiting for the timeout', async () => { + const messages = subscription(); + const controller = new AbortController(); + const result = monitorStuffboxConnection({ + subscribe: messages.subscribe, + checkConnected: async () => false, + signal: controller.signal, + pollIntervalMs: 20, + timeoutMs: 100, + }); + + controller.abort(); + await expect(result).rejects.toBeInstanceOf(StuffboxConnectionCancelledError); + }); +}); + diff --git a/src/lib/stuffbox/connection-monitor.ts b/src/lib/stuffbox/connection-monitor.ts new file mode 100644 index 0000000..f666f89 --- /dev/null +++ b/src/lib/stuffbox/connection-monitor.ts @@ -0,0 +1,101 @@ +export interface StuffboxConnectionResult { + type?: string; + success?: boolean; + message?: string; + attemptId?: string; +} + +interface MonitorOptions { + checkConnected: () => Promise; + subscribe: (receive: (result: StuffboxConnectionResult) => void) => () => void; + signal?: AbortSignal; + pollIntervalMs?: number; + timeoutMs?: number; +} + +export class StuffboxConnectionCancelledError extends Error { + constructor() { + super('Stuffbox connection cancelled'); + this.name = 'StuffboxConnectionCancelledError'; + } +} + +/** + * Wait for the server to confirm a Stuffbox connection. + * + * Popup window state is intentionally absent here. Cross-Origin-Opener-Policy + * can make a live cross-origin popup appear closed to its opener. Messages are + * useful accelerators, but the node's persisted connection is the authority. + */ +export function monitorStuffboxConnection({ + checkConnected, + subscribe, + signal, + pollIntervalMs = 750, + timeoutMs = 10 * 60_000, +}: MonitorOptions): Promise { + return new Promise((resolve, reject) => { + let settled = false; + let checking = false; + let cleanupSubscription = () => {}; + const timers: { + interval?: ReturnType; + timeout?: ReturnType; + } = {}; + + const abort = () => finish(new StuffboxConnectionCancelledError()); + signal?.addEventListener('abort', abort, { once: true }); + + function cleanup() { + if (timers.interval !== undefined) globalThis.clearInterval(timers.interval); + if (timers.timeout !== undefined) globalThis.clearTimeout(timers.timeout); + signal?.removeEventListener('abort', abort); + cleanupSubscription(); + } + + function finish(error?: Error) { + if (settled) return; + settled = true; + cleanup(); + if (error) reject(error); + else resolve(); + } + + async function checkNow() { + if (settled || checking) return; + checking = true; + try { + if (await checkConnected()) finish(); + } catch { + // Transient status failures are retried until timeout or cancellation. + } finally { + checking = false; + } + } + + if (signal?.aborted) { + abort(); + return; + } + + cleanupSubscription = subscribe((result) => { + if (result.type !== 'synapsis:stuffbox') return; + if (result.success) { + void checkNow(); + } else { + finish(new Error(result.message || 'Stuffbox access was not approved.')); + } + }); + if (settled) { + cleanupSubscription(); + return; + } + + timers.interval = globalThis.setInterval(() => void checkNow(), pollIntervalMs); + timers.timeout = globalThis.setTimeout(() => { + finish(new Error('Stuffbox connection timed out. You can safely try again.')); + }, timeoutMs); + + void checkNow(); + }); +} diff --git a/src/lib/stuffbox/popup-response.test.ts b/src/lib/stuffbox/popup-response.test.ts index 1ad32a7..b7556a4 100644 --- a/src/lib/stuffbox/popup-response.test.ts +++ b/src/lib/stuffbox/popup-response.test.ts @@ -3,10 +3,11 @@ import { renderStuffboxPopupResponse } from './popup-response'; describe('renderStuffboxPopupResponse', () => { it('notifies the main tab without navigating the popup into Synapsis', () => { - const html = renderStuffboxPopupResponse('https://synapsis.example', true, 'Stuffbox connected.'); + const html = renderStuffboxPopupResponse('https://synapsis.example', true, 'Stuffbox connected.', 'attempt-123'); expect(html).toContain("new BroadcastChannel('synapsis:stuffbox')"); expect(html).toContain("localStorage.setItem('synapsis:stuffbox:result'"); + expect(html).toContain('attempt-123'); expect(html).toContain('window.setTimeout(() => window.close(), 250)'); expect(html).not.toContain('window.location.replace'); }); diff --git a/src/lib/stuffbox/popup-response.ts b/src/lib/stuffbox/popup-response.ts index 495c37d..4d19197 100644 --- a/src/lib/stuffbox/popup-response.ts +++ b/src/lib/stuffbox/popup-response.ts @@ -1,6 +1,6 @@ -export function renderStuffboxPopupResponse(origin: string, success: boolean, message: string): string { +export function renderStuffboxPopupResponse(origin: string, success: boolean, message: string, attemptId?: string): string { const safeJson = (value: unknown) => JSON.stringify(value).replace(/Stuffbox