Repository navigation
feat(auth): passkey register handoff Redis module - #1010
Merged
Merged
Conversation
Failing tests for the passkey register handoff Redis module covering create/peek/consume semantics, one-time-use atomicity, malformed input, and Redis-unavailable fail modes. Next commit implements the module. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Redis-backed one-time handoff tokens that let an authenticated desktop user hand their identity to the system browser for the passkey add ceremony. Mirrors exchange-codes: 32-byte base64url tokens, SHA-256 hashed at rest, 300s TTL, atomic Lua GET+DEL on consume. Adds peek() so the external options endpoint can validate without invalidating before the verify step. Fail-closed on create, log-and-null on reads. Foundation for the passkey-add-desktop-fix epic; no routes or callers wired up in this commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 44 minutes and 44 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 (3)
✨ 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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: 98174e209b
ℹ️ 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".
redis.get / redis.eval can reject after the client is acquired (connection drops, command timeouts). Previously these would bubble up and break the documented 'never throws' contract on peek/consume, surfacing as 500s in the external passkey options/verify flows during transient outages. Wrap both calls in try/catch and return null with a logged error, matching the fail-closed posture used when the client itself is unavailable. 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
Epic:
tasks/passkey-add-desktop-fix.md— sub-task Server Register Handoff Token Module (task 1 of 4).Adds
packages/lib/src/auth/passkey-register-handoff.ts: a Redis-backed, short-lived, one-time token that lets the authenticated Electron renderer hand the user's identity to the system browser so the external passkey-add ceremony can authorize without a PageSpace session cookie. Foundation only — no routes, no callers, no UI wire-up. Those are the other three epic sub-tasks running in parallel worktrees.Mirrors
auth/exchange-codesexactly:auth:passkey-register-handoff:<hash>)GET+DELon consume (no race on double-use)nullon peek/consume when Redis is unavailablepeek(non-destructive) is the new piece vs. exchange-codes: the external options endpoint needs to validate the token without deleting it, since the verify endpoint must still find it.TDD commits
4a90d57a—test(auth): RED passkey-register-handoff module98174e20—feat(auth): passkey-register-handoff Redis moduleEpic requirement checklist
create: generates 32-byte base64url token, SHA-256 hashed key, stores{ userId, createdAt }with 300s TTLpeek: non-destructive GET, returns data ornull, never throws on bad inputconsume: atomic Lua GET+DEL, one-time use, returns data ornull{ userId }{ userId }and deletes the keynullnullwithout throwingnullnullpackages/lib/src/auth/index.tsbarrelanytypes; no new Redis key prefix, TTL, or hashing scheme inventedTest plan
pnpm --filter @pagespace/lib test passkey-register-handoff— 19/19 passingpnpm --filter @pagespace/lib typecheck— clean🤖 Generated with Claude Code