Repository navigation
feat(desktop): passkey-registered deep link + preload bridge - #1011
Merged
Merged
Conversation
Sub-task: Desktop Deep Link + Preload Bridge (passkey-add-desktop-fix epic) Covers: - pure handlePasskeyRegistered host routing + side effects - full handleDeepLink routing regression (auth-exchange still fetches) - handleDeepLink focuses window + sends passkey:registered IPC - unknown pagespace:// host still falls through to generic deep-link - preload exposes window.electron.passkey.onRegistered with unsubscribe Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…oad bridge Sub-task: Desktop Deep Link + Preload Bridge (passkey-add-desktop-fix epic) - New pure handler `handlePasskeyRegistered(url, deps)` in `apps/desktop/src/main/passkey-deep-link.ts` — host-checks the URL, focuses the main window, and broadcasts `passkey:registered` on the renderer IPC channel. No HTTP call, no token exchange, no session mutation — the credential is already persisted by the external passkey register endpoint before the deep link fires. - `handleDeepLink` dispatcher in `deep-links.ts` now routes `pagespace://passkey-registered` through the new handler, before the generic `deep-link` fallback. `pagespace://auth-exchange` behaviour is untouched (covered by regression test). - Preload exposes `window.electron.passkey.onRegistered(cb)` returning an unsubscribe function. Typed on both the preload-side `ElectronAPI` and the renderer ambient type at `apps/web/src/types/electron.d.ts` so downstream `PasskeyManager.tsx` can call it type-safely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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 40 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 (7)
✨ 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 |
Use expect.stringContaining/objectContaining instead of an awkward `as unknown as [string, RequestInit]` cast. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d0f115b9c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e channel YAGNI — the only broadcast is `passkey:registered` with no payload. A variadic signature hints at future flexibility we do not need and complicates the test doubles. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ne exists
Codex PR review flagged that when pagespace://passkey-registered arrives
while mainWindow is null — the macOS case where the user closes all
windows but the app stays in the dock — the handler silently consumed
the event: focusWindow was a no-op and sendToRenderer hit a dead
webContents, so the renderer never got passkey:registered and the
passkey list never refreshed.
Fix:
- If mainWindow is null when the deep link arrives, call createWindow()
to spawn one.
- If the window exists but is still loading, defer the
webContents.send('passkey:registered') onto its next did-finish-load
via webContents.once, so the renderer subscription is guaranteed to
be installed before we broadcast.
- Otherwise, deliver immediately as before.
Adds a regression test covering the null-window cold-start path.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…path `window` collides with the DOM global (undefined in the Electron main process, but still a footgun). `target` also lets the deferred did-finish-load callback reuse the captured binding instead of a second optional-chained main-window lookup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2witstudios
added a commit
that referenced
this pull request
Apr 14, 2026
Tests for the integration layer that wires the foundation PRs (#1008, #1009, #1010, #1011) into a working end-to-end flow for adding a passkey from the Settings → Security panel on desktop. Fails until the routes, ceremony helper, external page, and PasskeyManager desktop branch land in the next commit. - handoff mint route: session auth + CSRF + rate limit, mints handoffToken via createPasskeyRegisterHandoff and audits auth.token.created - register/options handoffToken branch: peek (non-destructive), skips session auth + CSRF, 401 HANDOFF_INVALID + audit on miss, rate limit bound to resolved userId, session path regression - register verify handoffToken branch: consume (one-time-use), second call rejected, rate limit bound to resolved userId, session path regression - runPasskeyRegisterExternalCeremony: options → startRegistration → verify → pagespace://passkey-registered, NotAllowedError → cancelled result - PasskeyManager desktop branch: update-prompt when bridge missing, POST handoff → openExternal with /auth/passkey-register-external URL, no in-window startRegistration, onRegistered → mutate('/api/auth/passkey') + toast, unsubscribe on unmount, web regression Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2witstudios
added a commit
that referenced
this pull request
Apr 14, 2026
Wires the merged foundation PRs (#1008, #1009, #1010, #1011) into a working end-to-end flow so Electron users can add a passkey from Settings → Security. Electron's Chromium cannot drive Touch ID / Windows Hello without entitlements we don't ship, so the ceremony is delegated to the system browser via the same `shell.openExternal` + `pagespace://` deep-link pattern PR #1001 used for sign-in. New: - POST /api/auth/passkey/register/handoff — authenticated mint endpoint for one-time handoff tokens (session auth + CSRF + per-user rate limit). - apps/web/src/components/auth/runPasskeyRegisterExternalCeremony.ts — options → startRegistration → verify → pagespace://passkey-registered. - apps/web/src/app/auth/passkey-register-external/page.tsx — system-browser landing page that runs the ceremony and deep-links back to the desktop app. Modified in place (mirroring authenticate/route.ts desktopExchange branch): - register/options/route.ts — handoffToken branch uses peekPasskeyRegisterHandoff (non-destructive; verify step still needs the token), skips session + CSRF, 401 HANDOFF_INVALID on miss, rate limit bound to resolved userId. - register/route.ts — handoffToken branch uses consumePasskeyRegisterHandoff (atomic one-time-use), 401 on miss, same rate limit binding. - PasskeyManager.tsx — desktop branch in handleRegister: update-prompt guard when bridge missing, POST handoff → openExternal with /auth/passkey-register- external URL, no in-window startRegistration. Deep-link subscription via window.electron.passkey.onRegistered refreshes the passkey list and toasts on return. Web path unchanged. Completes the Passkey Add Desktop Fix epic (tasks/passkey-add-desktop-fix.md). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
10 tasks
2witstudios
added a commit
that referenced
this pull request
Apr 14, 2026
* test(auth): RED — passkey add-from-settings desktop integration Tests for the integration layer that wires the foundation PRs (#1008, #1009, #1010, #1011) into a working end-to-end flow for adding a passkey from the Settings → Security panel on desktop. Fails until the routes, ceremony helper, external page, and PasskeyManager desktop branch land in the next commit. - handoff mint route: session auth + CSRF + rate limit, mints handoffToken via createPasskeyRegisterHandoff and audits auth.token.created - register/options handoffToken branch: peek (non-destructive), skips session auth + CSRF, 401 HANDOFF_INVALID + audit on miss, rate limit bound to resolved userId, session path regression - register verify handoffToken branch: consume (one-time-use), second call rejected, rate limit bound to resolved userId, session path regression - runPasskeyRegisterExternalCeremony: options → startRegistration → verify → pagespace://passkey-registered, NotAllowedError → cancelled result - PasskeyManager desktop branch: update-prompt when bridge missing, POST handoff → openExternal with /auth/passkey-register-external URL, no in-window startRegistration, onRegistered → mutate('/api/auth/passkey') + toast, unsubscribe on unmount, web regression Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(auth): GREEN — passkey add-from-settings desktop integration Wires the merged foundation PRs (#1008, #1009, #1010, #1011) into a working end-to-end flow so Electron users can add a passkey from Settings → Security. Electron's Chromium cannot drive Touch ID / Windows Hello without entitlements we don't ship, so the ceremony is delegated to the system browser via the same `shell.openExternal` + `pagespace://` deep-link pattern PR #1001 used for sign-in. New: - POST /api/auth/passkey/register/handoff — authenticated mint endpoint for one-time handoff tokens (session auth + CSRF + per-user rate limit). - apps/web/src/components/auth/runPasskeyRegisterExternalCeremony.ts — options → startRegistration → verify → pagespace://passkey-registered. - apps/web/src/app/auth/passkey-register-external/page.tsx — system-browser landing page that runs the ceremony and deep-links back to the desktop app. Modified in place (mirroring authenticate/route.ts desktopExchange branch): - register/options/route.ts — handoffToken branch uses peekPasskeyRegisterHandoff (non-destructive; verify step still needs the token), skips session + CSRF, 401 HANDOFF_INVALID on miss, rate limit bound to resolved userId. - register/route.ts — handoffToken branch uses consumePasskeyRegisterHandoff (atomic one-time-use), 401 on miss, same rate limit binding. - PasskeyManager.tsx — desktop branch in handleRegister: update-prompt guard when bridge missing, POST handoff → openExternal with /auth/passkey-register- external URL, no in-window startRegistration. Deep-link subscription via window.electron.passkey.onRegistered refreshes the passkey list and toasts on return. Web path unchanged. Completes the Passkey Add Desktop Fix epic (tasks/passkey-add-desktop-fix.md). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(auth): address PR #1012 review feedback — fragment handoff, 400 on parse Addresses Codex and CodeRabbit review feedback on PR #1012: **Security — move handoff token out of the query string** (CodeRabbit Major on passkey-register-external/page.tsx) - buildPasskeyRegisterExternalUrl now places handoffToken in the URL fragment (`#handoffToken=...`), never the query. Fragments never hit the network, server access logs, CDN logs, or the Referer header, so the capability token no longer leaks anywhere a log scraper or a same-origin fetch could see it. - parsePasskeyRegisterExternalParams takes an optional `hash` argument and reads the token from there; deviceId/deviceName remain in the query. - External page clears the fragment via history.replaceState after reading, so the token is not left sitting in the browser address bar. - Round-trip and missing-field tests updated to cover the new transport. **Robustness — request body parsing** (Codex P2 + CodeRabbit Minor) - register/route.ts wraps `await req.json()` in try/catch and returns a controlled 400 on malformed bodies instead of falling to the outer catch and emitting internal-error noise to unauthenticated callers. - register/options/route.ts readOptionalJson normalizes non-plain-object parses (null, arrays, primitives) to `{}` so `body.handoffToken` access never throws. Regression tests for `'null'` and malformed bodies confirm the session-auth path still runs. **Robustness — unhandled promise on external page** (Codex P2 on passkey-register-external/page.tsx) - Effect now attaches a .catch that surfaces the error to the status state instead of leaving the spinner stuck if runPasskeyRegisterExternalCeremony rejects (e.g. browser offline, fetch throw). **Cleanup — ceremony helper typing** (CodeRabbit Nit on runPasskeyRegisterExternalCeremony.ts) - Derived `RegisterOptionsJSON` from `Parameters<typeof startRegistration>` and typed the parsed options directly so startRegistration no longer needs a double `as unknown as` cast. **Test hygiene** (CodeRabbit Nit on PasskeyManager.desktop.test.tsx) - Replaced a raw `setTimeout(10)` with a `waitFor` on an observable assertion. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(auth): tighten Bearer check + surface parse errors in options route Addresses a second round of review findings on PR #1012: **CSRF bypass must require the Bearer scheme, not any Authorization header** The three routes touched by this PR (`register/handoff`, `register/options`, `register`) previously skipped CSRF whenever `authorization` was present, which would let a non-Bearer scheme (`Basic`, `Digest`, random garbage) bypass CSRF even though `authenticateSessionRequest` would reject it upstream. Now each route uses `getBearerToken(req)` (already the canonical scheme check in `@/lib/auth`, previously private) and only skips CSRF when the header actually carries a `Bearer ` prefix. `getBearerToken` is now exported as a named symbol. Regression tests added to all three route suites: an `Authorization: Basic …` header with a bad `x-csrf-token` must still return 403. **`options/route.ts` must surface malformed JSON as 400, not fall through** `readOptionalJson` previously caught `JSON.parse` errors and returned `{}`, which meant a syntactically invalid body would silently engage the session path instead of returning a controlled 400 (mismatched with `register/route.ts` behavior). The helper no longer catches `SyntaxError`; the caller wraps the call in a try/catch and returns a 400 `{ error: 'Invalid request body' }`. Empty body still resolves to `{}` (so callers without a body still reach the session path), and non-object JSON (`null`, arrays, primitives) still normalizes to `{}`. The existing "treats malformed JSON as empty body and falls through" test is inverted to "returns 400 (not 500, not session fallthrough) when body is malformed JSON" and now also asserts `authenticateSessionRequest` is never called. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sub-task Desktop Deep Link + Preload Bridge of the
passkey-add-desktop-fixepic.Wires up Electron so the system browser can complete a Settings → Security passkey registration and return control to the desktop app via
pagespace://passkey-registered. The credential is already persisted server-side (by the external register endpoint built in a parallel worktree) — all this task does is focus/create the main window and broadcast a typed IPC event the renderer can subscribe to.apps/desktop/src/main/passkey-deep-link.ts— pure handlerhandlePasskeyRegistered(url, deps). Host-checks, logs, then asks the wiring to focus the window and broadcastpasskey:registered. No fetch, no token exchange, no session mutation.apps/desktop/src/main/deep-links.ts— dispatcher gains the new case afterauth-exchange/auth-error, before the genericdeep-linkfallback. Wiring forfocusWindow/sendToRenderer:mainWindowexists and is loaded → focus +webContents.sendimmediately.mainWindowexists butisLoading()→ defer the send onto the nextdid-finish-loadviawebContents.once(...).mainWindowisnull(macOS: all windows closed while app stays in the dock) → callcreateWindow(), then defer the send ontodid-finish-loadof the new window. This is the Codex review fix.apps/desktop/src/preload/index.ts— new top-levelpasskey: { onRegistered(cb): () => void }bridge usingipcRenderer.on+removeListenerfor clean unsubscribe. Typed on the localElectronAPIinterface.apps/web/src/types/electron.d.ts— renderer ambient type gains the matchingpasskey.onRegisteredentry so downstreamPasskeyManager.tsxcan call it type-safely. This is where the downstream PasskeyManager task should import the type from.Explicitly not touched:
auth:open-externalIPC handler,ALLOWED_AUTH_HOSTNAMES,ALLOWED_PASSKEY_PATHS— those belong to the parallel "Electron Open-External Allowlist" task.pagespace://auth-exchangebehaviour is fully preserved (regression test).Commits
ba789d1e—test(desktop): RED passkey-registered deep link + preload bridge7d0f115b—feat(desktop): handle pagespace://passkey-registered deep link + preload bridgeab120b31—test(desktop): clean up regression assertion in deep-links.test.ts5c3616e9—refactor(desktop): narrow PasskeyDeepLinkDeps.sendToRenderer to single channel9468a34f—fix(desktop): passkey-registered must create window + defer IPC if none existsRequirement checklist
pagespace://passkey-registeredfocuses the mainBrowserWindow(restore if minimized, show, focus)pagespace://passkey-registeredbroadcastspasskey:registeredto the renderer viawebContents.sendpagespace://passkey-registeredwith no existing main window creates one and defers the IPC untildid-finish-load— no silent drops on macOS cold-open from dockpagespace://passkey-registeredmakes no HTTP call, token exchange, or session mutation (asserted byfetchSpy.not.toHaveBeenCalled())pagespace://auth-exchange?code=...still POSTs to/api/auth/desktop/exchangeand never emitspasskey:registered(asserted)pagespace://foo-barstill falls through to the genericdeep-linkIPC channel (asserted)window.electron.passkey.onRegistered(cb)subscribes viaipcRenderer.on('passkey:registered', …)ipcRenderer.removeListenerand stops delivering callbacks (asserted)ElectronAPIatapps/web/src/types/electron.d.tstyped, noanypnpm --filter desktop typecheckcleanpnpm --filter desktop testclean (120 tests passing, including 4 new deep-links dispatcher tests, 3 passkey-deep-link unit tests, 3 preload bridge tests)pnpm --filter web typecheckclean (afterpackages/lib+packages/dbbuilds)pnpm lintclean monorepo-wideRegression proof
apps/desktop/src/main/__tests__/deep-links.test.ts→ "given pagespace://auth-exchange (regression) › should still POST to /api/auth/desktop/exchange and never broadcast passkey:registered". Stubsfetch, callshandleDeepLink('pagespace://auth-exchange?code=code123&provider=google'), and asserts:fetchcalled exactly once against/api/auth/desktop/exchangewithmethod: 'POST'webContents.sendnever called with'passkey:registered'Ambient type location for downstream PasskeyManager task
The downstream
PasskeyManager.tsxrenderer task should import viawindow.electron.passkey.onRegistered— typed inapps/web/src/types/electron.d.tson theElectronAPIinterface. The preload-side mirror lives inapps/desktop/src/preload/index.tson its ownElectronAPIexport.Test plan
pnpm --filter desktop test— 120 passed (was 119, +1 null-window regression)pnpm --filter desktop typecheck— cleanpnpm --filter web typecheck— cleanpnpm lint— cleanpagespace://passkey-registeredtriggers the renderer subscription