Repository navigation
fix(auth): clear error when adding a passkey on a device that already has one - #1014
Conversation
Extend runPasskeyRegisterExternalCeremony tests to assert a new
`code` discriminator on the { ok: false } branch — 'ALREADY_REGISTERED'
for InvalidStateError and 'CANCELLED' for NotAllowedError — and add a
page-level test for a PasskeyRegisterExternalView that renders distinct
headers per code ("This device is already set up" / "Registration
cancelled" / "Registration failed"). Both fail against the current code.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…gister-external The external passkey register ceremony shipped in #1012 bubbled WebAuthn's InvalidStateError as a raw message, so users trying to add a second passkey on an authenticator they already have one registered with saw gibberish like "The operation either timed out or was not allowed" with no actionable context. The server correctly populates excludeCredentials (that is spec-recommended behaviour); the bug was purely the client-side error mapping. - runPasskeyRegisterExternalCeremony now returns a structured { ok: false, error, code } shape, emitting code='ALREADY_REGISTERED' with a friendly message for InvalidStateError and code='CANCELLED' for NotAllowedError. Generic errors still surface err.message with no code, matching prior behaviour. - passkey-register-external/page.tsx now exports a pure PasskeyRegisterExternalView that branches on the error code to render distinct headers ("This device is already set up" / "Registration cancelled" / "Registration failed") with softer icons for the first two cases. The page shell still owns the ceremony effect and just passes the status down. 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 29 minutes and 46 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 (5)
✨ 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 |
![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: 987bec6405
ℹ️ 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".
….js build Next.js 15 page.tsx files only allow default + specific metadata exports. Exporting PasskeyRegisterExternalView and its Status type from the page broke production build with 'not a valid Page export field'. Extract the pure view component and its Status type into apps/web/src/components/auth/PasskeyRegisterExternalView.tsx. The page now only exports the default and imports the view from its new home. Tests updated to import from the component module. 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>
Summary
A desktop user on
desktop-v1.0.22(shipped via #1012) reported:They have a Touch ID passkey registered on their Mac and tried to add another from Settings → Security. The external register page showed an unhelpful error like "The operation either timed out or was not allowed" with no actionable context, leading them to conclude the feature was broken.
Root cause is WebAuthn behaving correctly: the server in
packages/lib/src/auth/passkey-service.tspopulatesexcludeCredentialswith the user's existing credential IDs (the spec-recommended pattern), Touch ID sees its own credential in the exclude list, and refuses withInvalidStateError. The existing web flow (PasskeyManager.tsxline 195) maps this to a friendly toast — but the NEW external register ceremony shipped in #1012 forgot to copy that branch and bubbled the raw message.This PR fixes the messaging, not the WebAuthn rule. To actually register a second passkey, users still need a different authenticator (another Mac, an iPhone, a hardware security key, or iCloud Keychain sync). The message now spells that out.
What changed
runPasskeyRegisterExternalCeremonynow returns{ ok: false, error, code? }wherecodeis'ALREADY_REGISTERED'forInvalidStateErrorand'CANCELLED'forNotAllowedError. Generic errors surfaceerr.messagewith no code, matching prior behaviour.passkey-register-external/page.tsxextracts a purePasskeyRegisterExternalViewthat branches oncodeto render distinct headers with softer icons:ALREADY_REGISTERED→ "This device is already set up" +CheckCircle2(muted)CANCELLED→ "Registration cancelled" +Info(muted)ShieldAlert(destructive)NOT touched
packages/lib/src/auth/passkey-service.ts— serverexcludeCredentialsis correct and stays.apps/web/src/app/api/auth/passkey/register/options/route.ts,apps/web/src/app/api/auth/passkey/register/route.ts— untouched; the parallel rate-limit hot fix (pu/passkey-register-ratelimit-fix) owns these.runPasskeyExternalCeremony.ts(sign-in, not register) —.getdoes not throwInvalidStateError.PasskeyManager.tsx— its web-pathInvalidStateErrortoast already works.Parallel hot fix
This PR is one of two independent hot fixes flowing from #1012. The rate-limit fix is being handled in a separate worktree / PR.
Commits
4dfe5dbd— test(auth): RED passkey register external existing-credential UX987bec64— fix(auth): map InvalidStateError to clear already-registered UX on register-externalTest plan
Automated (all green locally):
pnpm --filter web typecheckpnpm --filter web lint(no new warnings)pnpm --filter web exec vitest run src/components/auth/__tests__/runPasskeyRegisterExternalCeremony.test.ts— 7/7 passing (addsInvalidStateError,CANCELLED-code, and generic-error cases)pnpm --filter web exec vitest run src/app/auth/__tests__/passkey-register-external-page.test.tsx— 5/5 passing (asserts each status variant renders its own header)Manual smoke test (to run against
desktop-v1.0.22+ once shipped):codediscrimination works.🤖 Generated with Claude Code