Repository navigation
fix(auth): close external browser tab after desktop passkey/OAuth handoff - #1015
Conversation
…doff External system-browser tabs sat spinning forever after a successful passkey ceremony or OAuth sign-in: the passkey pages stayed on the "Returning to the desktop app…" spinner, and the OAuth callbacks 302'd straight to a pagespace:// URL the browser couldn't resolve. Both flows now render a terminal "you can close this window" state — passkey pages transition to a `complete` React state shortly after firing the deep link, and OAuth callbacks return an HTML bridge that uses meta-refresh to dispatch the deep link without inline scripts (CSP-clean). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 19 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntroduces a new "handoff bridge" mechanism that returns HTML with meta refresh redirection instead of direct HTTP redirects for OAuth callbacks. Includes new HTML generation utility, response wrapper with security headers, updated callbacks for Apple and Google OAuth, and completion UI for passkey flows with cleanup handling for unmount scenarios. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser as Browser
participant Handler as OAuth Handler
participant MobileApp as Mobile App
User->>Handler: Click "Sign in with Google/Apple"
Handler->>Browser: 200 OK + HTML (meta refresh)
Browser->>Browser: Execute meta refresh
Browser->>MobileApp: Navigate to pagespace://auth-exchange?code=...&provider=...
MobileApp->>MobileApp: Process authentication
Handler->>Browser: Render HandoffCompleteCard
Browser->>Browser: Show "You're signed in" + Close button
User->>Browser: Click Close (or auto-close after timeout)
Browser->>Browser: window.close()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Extracts the duplicated buildHandoffBridgeResponse wrapper and HANDOFF_BRIDGE_CSP constant from the apple and google OAuth callback routes into _shared/handoffBridgeResponse.ts. Drops the dead img-src data: directive (no inline <img> is rendered; the SVG is inline, and default-src 'none' already denies images). Adds route-level assertions that each callback emits the hardened security header set (CSP, Cache-Control, X-Frame-Options, X-Content-Type-Options, Referrer-Policy) so future edits to the bridge wrapper cannot silently weaken the response. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolves conflict in apps/web/src/app/auth/passkey-register-external/page.tsx by combining both branches: - Keeps master's PasskeyRegisterExternalView extraction with the ALREADY_REGISTERED / CANCELLED error-code rendering paths from #1013 / #1014. - Keeps this branch's `complete` state machine, post-deep-link settle timer, best-effort window.close(), and HandoffCompleteCard terminal-state rendering. PasskeyRegisterExternalView is extended with a `complete` kind that renders <HandoffCompleteCard variant="passkey-added" />, so the page component can rely on the View for all rendering while still owning the timer-driven state transitions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Merged latest |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/auth/passkey-external/page.tsx (1)
35-57:⚠️ Potential issue | 🟠 MajorHandle rejected ceremony promises to avoid unhandled rejection/stuck UI.
Line 35 starts an async chain without a rejection path. If the ceremony throws/rejects, users can be left without an error state.
Proposed fix
- void runPasskeyExternalCeremony({ - deviceId: params.deviceId, - deviceName: params.deviceName, - }).then((result) => { - if (unmounted) return; - if (result.ok) { - setStatus({ kind: 'redirecting' }); - window.location.href = result.deepLink; - settleTimer = setTimeout(() => { - if (unmounted) return; - setStatus({ kind: 'complete' }); - try { - window.close(); - } catch { - // Best-effort: many browsers refuse window.close() for tabs - // not opened via window.open(). The terminal UI is the fallback. - } - }, HANDOFF_SETTLE_MS); - } else { - setStatus({ kind: 'error', message: result.error }); - } - }); + void runPasskeyExternalCeremony({ + deviceId: params.deviceId, + deviceName: params.deviceName, + }) + .then((result) => { + if (unmounted) return; + if (result.ok) { + setStatus({ kind: 'redirecting' }); + window.location.href = result.deepLink; + settleTimer = setTimeout(() => { + if (unmounted) return; + setStatus({ kind: 'complete' }); + try { + window.close(); + } catch { + // Best-effort: many browsers refuse window.close() for tabs + // not opened via window.open(). The terminal UI is the fallback. + } + }, HANDOFF_SETTLE_MS); + } else { + setStatus({ kind: 'error', message: result.error }); + } + }) + .catch((err: unknown) => { + if (unmounted) return; + setStatus({ + kind: 'error', + message: err instanceof Error ? err.message : 'Unexpected error', + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/auth/passkey-external/page.tsx` around lines 35 - 57, The async call to runPasskeyExternalCeremony is missing a rejection handler, which can leave the UI stuck; update the invocation (the code calling runPasskeyExternalCeremony with params.deviceId/deviceName) to handle promise rejections — either by awaiting it inside an async IIFE with try/catch or by appending .catch(...) — and in the catch setStatus({ kind: 'error', message: /* friendly error or err.message */ }), clear any pending settleTimer, and respect the unmounted check before updating state; ensure the existing success path (setting redirect/complete and window.close) remains unchanged and still uses HANDOFF_SETTLE_MS.
🧹 Nitpick comments (3)
apps/web/src/app/api/auth/_shared/handoffBridgeResponse.ts (1)
1-2: Use kebab-case filename for this shared module.Please rename
handoffBridgeResponse.tstohandoff-bridge-response.tsand update import paths accordingly.As per coding guidelines:
Use kebab-case for filenames, except React hooks (camelCase with use prefix), Zustand stores (camelCase with use prefix), and React components (PascalCase).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/auth/_shared/handoffBridgeResponse.ts` around lines 1 - 2, Rename the module file from handoffBridgeResponse.ts to handoff-bridge-response.ts and update all imports that reference handoffBridgeResponse to the new kebab-case filename; specifically update any files importing buildHandoffBridgeHtml or NextResponse from this module to import from './handoff-bridge-response' (or the correct relative path), and ensure the exported symbols (e.g., buildHandoffBridgeHtml) remain unchanged so existing callers continue to work.apps/web/src/app/api/auth/_shared/__tests__/buildHandoffBridgeHtml.test.ts (1)
1-2: Rename test file to kebab-case for repo consistency.
buildHandoffBridgeHtml.test.tsshould follow kebab-case naming (e.g.,build-handoff-bridge-html.test.ts) to match repository standards.As per coding guidelines:
Use kebab-case for filenames, except React hooks (camelCase with use prefix), Zustand stores (camelCase with use prefix), and React components (PascalCase).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/auth/_shared/__tests__/buildHandoffBridgeHtml.test.ts` around lines 1 - 2, Rename the test file from buildHandoffBridgeHtml.test.ts to kebab-case build-handoff-bridge-html.test.ts and update any imports or references that point to the old filename (e.g., test runner entries or other files importing this test). Ensure the test still imports buildHandoffBridgeHtml from ../buildHandoffBridgeHtml and keeps the existing describe/it/expect usage intact; run the test suite to verify the renamed file is discovered by Vitest.apps/web/src/app/api/auth/_shared/buildHandoffBridgeHtml.ts (1)
1-5: Rename this utility file to kebab-case.
buildHandoffBridgeHtml.tsshould be renamed tobuild-handoff-bridge-html.tsto align with repo conventions.As per coding guidelines:
Use kebab-case for filenames, except React hooks (camelCase with use prefix), Zustand stores (camelCase with use prefix), and React components (PascalCase).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/auth/_shared/buildHandoffBridgeHtml.ts` around lines 1 - 5, The file name uses camelCase; rename the module from buildHandoffBridgeHtml.ts to kebab-case build-handoff-bridge-html.ts and update all imports/exports accordingly (search for BuildHandoffBridgeHtmlParams, any exported functions or default export in that file and update their import paths). Ensure any CI/tsconfig references still resolve after the rename and run a quick typecheck to catch broken imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/src/app/auth/passkey-external/page.tsx`:
- Around line 35-57: The async call to runPasskeyExternalCeremony is missing a
rejection handler, which can leave the UI stuck; update the invocation (the code
calling runPasskeyExternalCeremony with params.deviceId/deviceName) to handle
promise rejections — either by awaiting it inside an async IIFE with try/catch
or by appending .catch(...) — and in the catch setStatus({ kind: 'error',
message: /* friendly error or err.message */ }), clear any pending settleTimer,
and respect the unmounted check before updating state; ensure the existing
success path (setting redirect/complete and window.close) remains unchanged and
still uses HANDOFF_SETTLE_MS.
---
Nitpick comments:
In `@apps/web/src/app/api/auth/_shared/__tests__/buildHandoffBridgeHtml.test.ts`:
- Around line 1-2: Rename the test file from buildHandoffBridgeHtml.test.ts to
kebab-case build-handoff-bridge-html.test.ts and update any imports or
references that point to the old filename (e.g., test runner entries or other
files importing this test). Ensure the test still imports buildHandoffBridgeHtml
from ../buildHandoffBridgeHtml and keeps the existing describe/it/expect usage
intact; run the test suite to verify the renamed file is discovered by Vitest.
In `@apps/web/src/app/api/auth/_shared/buildHandoffBridgeHtml.ts`:
- Around line 1-5: The file name uses camelCase; rename the module from
buildHandoffBridgeHtml.ts to kebab-case build-handoff-bridge-html.ts and update
all imports/exports accordingly (search for BuildHandoffBridgeHtmlParams, any
exported functions or default export in that file and update their import
paths). Ensure any CI/tsconfig references still resolve after the rename and run
a quick typecheck to catch broken imports.
In `@apps/web/src/app/api/auth/_shared/handoffBridgeResponse.ts`:
- Around line 1-2: Rename the module file from handoffBridgeResponse.ts to
handoff-bridge-response.ts and update all imports that reference
handoffBridgeResponse to the new kebab-case filename; specifically update any
files importing buildHandoffBridgeHtml or NextResponse from this module to
import from './handoff-bridge-response' (or the correct relative path), and
ensure the exported symbols (e.g., buildHandoffBridgeHtml) remain unchanged so
existing callers continue to work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48b4149b-edc9-44d8-82d4-306aa4f8b427
📒 Files selected for processing (11)
apps/web/src/app/api/auth/_shared/__tests__/buildHandoffBridgeHtml.test.tsapps/web/src/app/api/auth/_shared/buildHandoffBridgeHtml.tsapps/web/src/app/api/auth/_shared/handoffBridgeResponse.tsapps/web/src/app/api/auth/apple/callback/__tests__/route.test.tsapps/web/src/app/api/auth/apple/callback/route.tsapps/web/src/app/api/auth/google/callback/__tests__/route.test.tsapps/web/src/app/api/auth/google/callback/route.tsapps/web/src/app/auth/passkey-external/page.tsxapps/web/src/app/auth/passkey-register-external/page.tsxapps/web/src/components/auth/HandoffCompleteCard.tsxapps/web/src/components/auth/PasskeyRegisterExternalView.tsx
The previous invocation used void runPasskeyExternalCeremony(...).then(...) with no rejection handler, so any thrown error inside the ceremony (network failure, NotAllowedError, etc.) left the UI stuck on the running spinner. Add a .catch that respects the unmounted check, clears any pending settle timer, and surfaces the error message via setStatus, mirroring the register-external page. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
/auth/passkey-external,/auth/passkey-register-external) now transition to a terminal "complete" state ~600ms after firing thepagespace://deep link, replacing the eternal "Returning to the desktop app…" spinner. Adds sharedHandoffCompleteCardand best-effortwindow.close().pagespace://auth-exchange?code=…(which left the browser tab pending forever). They now return a 200 HTML bridge response built bybuildHandoffBridgeHtml, which uses<meta http-equiv="refresh">to dispatch the deep link (CSP-clean — no inline scripts), shows the same success card, and includes a<noscript>fallback anchor.<style>for the bridge HTML while keepingdefault-src 'none'. iOS branch left untouched (Capacitor handles deep links differently).Test plan
pnpm --filter web typecheckpnpm --filter web exec vitest runforgoogle/callback,apple/callback,_shared/buildHandoffBridgeHtml, andrunPasskey*Ceremony— 149 tests passing🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Security Improvements