Skip to content

feat(passkeys): implement sign-in ceremony on Index and Signin pages#20575

Merged
vpomerleau merged 1 commit into
mainfrom
FXA-13099
May 20, 2026
Merged

feat(passkeys): implement sign-in ceremony on Index and Signin pages#20575
vpomerleau merged 1 commit into
mainfrom
FXA-13099

Conversation

@vpomerleau
Copy link
Copy Markdown
Contributor

@vpomerleau vpomerleau commented May 11, 2026

Because

  • We need to wire the passkey WebAuthn ceremony to the passkey button introduced in FXA-13487 so users can sign in with a passkey from the Index and Signin pages.

This pull request

  • Adds usePasskeySignIn hook owning the WebAuthn ceremony, error categorisation, Sync merge gate, account persistence, and post-auth routing. Validates the accountProfile response before relying on email, and sanitises Sentry captures so server-error responses cannot leak PII.
  • Wires the hook into Index and Signin; plumbs authClient and finishOAuthFlowHandler through the Index container.
  • Mounts /signin/passkey/fallback as a temporary stand-in for FXA-13100; hides the passkey button on Sync entry-points behind the same TODO.
  • Surfaces a tailored "passkey not recognized" banner when the server returns PASSKEY_NOT_FOUND (errno 224); does not report to Sentry since it is an expected divergence between server and authenticator state.
  • Fixes the auth-server passkey route to return sessionToken.data instead of .id so the client can Hawk-sign subsequent requests.
  • Enables passkeyAuthenticationEnabled in local.json-dist (dev only; production flag stays off via the convict default).
  • Adds functional tests covering the passkey sign-in happy, cancel, and PASSKEY_NOT_FOUND paths.

Issue that this pull request solves

Closes: FXA-13099

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review

  • Key files: packages/fxa-settings/src/lib/passkeys/signin-flow.ts (the hook), packages/fxa-settings/src/pages/{Index,Signin}/ (wiring), packages/fxa-auth-server/lib/routes/passkeys.ts (sessionToken fix), packages/functional-tests/tests/passkeyAuth/passkey-signin.spec.ts (new tests).

Other information

  • Telemetry: Glean events for this flow are intentionally omitted here. FXA-13475 owns the telemetry implementation.
  • Sync end-to-end: the passkey button is currently hidden on Sync entry-points via !integration.isSync(). FXA-13100 will replace the temporary SetPasswordContainer mount at /signin/passkey/fallback with a container that derives Sync keys via verifyPasswordAfterPasskey; the Sync button can re-enable then.
  • Auth-server fix: the sessionToken.id → .data change in passkeys.ts is a real bug latent in main since 5a17be0bb7 — every passkey sign-in via the existing route would have failed downstream Hawk-authenticated calls. Worth filing a separate bug to track for backport visibility.

@vpomerleau vpomerleau force-pushed the FXA-13099 branch 5 times, most recently from 878ef7a to 20b8131 Compare May 15, 2026 06:27
@vpomerleau vpomerleau marked this pull request as ready for review May 15, 2026 16:20
Copilot AI review requested due to automatic review settings May 15, 2026 16:20
@vpomerleau vpomerleau requested review from a team as code owners May 15, 2026 16:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements the passkey WebAuthn sign-in ceremony, wiring a new usePasskeySignIn hook into the Index and Signin pages and fixing an auth-server bug in the existing passkey route.

Changes:

  • Adds usePasskeySignIn hook owning the WebAuthn ceremony, error categorization (incl. new AbortError mapping and PASSKEY_NOT_FOUND banner), Sync merge gate, account persistence, and post-auth navigation; PII-sanitized Sentry captures.
  • Wires hook into Index (with disableAutoSubmit to suppress suggested-email auto-submit on passkey click) and Signin; widens IndexIntegration and mocks accordingly; mounts /post_verify/passkey/set_password/* for the no-password Sync fallback; hides the button for Sync and cached signin.
  • Fixes auth-server passkey route to return sessionToken.data (Hawk key) instead of .id; enables passkeyAuthenticationEnabled in dev local.json-dist; adds polyfill assertion() mode and functional tests covering happy/cancel/PASSKEY_NOT_FOUND paths.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/fxa-settings/src/lib/passkeys/signin-flow.ts New hook implementing the full passkey sign-in ceremony
packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx Unit tests for the hook
packages/fxa-settings/src/lib/passkeys/webauthn-errors.ts Adds AbortError mapping (treated like NotAllowedError)
packages/fxa-settings/src/lib/passkeys/webauthn-errors.test.ts Test coverage for the new AbortError mapping
packages/fxa-settings/src/lib/passkeys/en.ftl New passkey-authentication-error-not-found string
packages/fxa-settings/src/pages/Signin/index.tsx Wires hook; hides button on Sync and cached signin
packages/fxa-settings/src/pages/Index/index.tsx Wires hook; cancels auto-submit before passkey click
packages/fxa-settings/src/pages/Index/container.tsx Plumbs authClient/finishOAuthFlowHandler/disableAutoSubmit
packages/fxa-settings/src/pages/Index/interfaces.ts Widens IndexIntegration with new method picks
packages/fxa-settings/src/pages/Index/mocks.tsx Mock updates for widened integration & new props
packages/fxa-settings/src/components/App/index.tsx Mounts SetPasswordContainer at /post_verify/passkey/set_password/*
packages/fxa-content-server/server/config/local.json-dist Enables passkeyAuthenticationEnabled in dev
packages/fxa-auth-server/lib/routes/passkeys.ts Returns sessionToken.data instead of .id
packages/fxa-auth-server/lib/routes/passkeys.spec.ts Updates assertions for the .data/.id shape
packages/functional-tests/lib/passkeyPolyfill.ts New assertion() mode; passthrough of canonical WebAuthn error names
packages/functional-tests/pages/signin.ts New passkeySigninButton locator; tightens signInButton matcher
packages/functional-tests/tests/passkeyAuth/passkey-signin.spec.ts New functional tests (happy/cancel/PASSKEY_NOT_FOUND)
packages/functional-tests/tests/_reference/passkey-virtual-authenticator.spec.ts Header comment refresh; suite still skipped

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dschom dschom self-requested a review May 15, 2026 17:49
Comment thread packages/fxa-settings/src/lib/passkeys/signin-flow.ts Outdated
Comment thread packages/fxa-settings/src/lib/passkeys/signin-flow.ts
Comment thread packages/fxa-settings/src/lib/passkeys/signin-flow.ts Outdated
Comment thread packages/fxa-settings/src/lib/passkeys/signin-flow.ts Outdated
@vpomerleau vpomerleau force-pushed the FXA-13099 branch 2 times, most recently from ec2879f to 8f3e9c6 Compare May 19, 2026 20:58
Copy link
Copy Markdown
Contributor

@LZoog LZoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Tested this on both 1Password, and Apple keychain, and can sign in from email-first and on the non-cached signin screen.

  • Nice to have: are we planning to add a timestamp to "last used" or just keep that as the date? (I know I could check Figma heheh)
  • Are we planning to file an a11y review? Curious what they would recommend - when the button is dimmed, it might make sense for it to tell the user somehow that we're waiting for them to perform an action...

✅ Saw these errors as expected. Do we want to remove the space above? (I get why we have the error messaging here, but it does also read slightly funny with the "Or", maybe we can direct focus there when we do a11y improvements)

Image Image Image

❔:

This is probably intentional, but why not hide the button the same way we do third party auth for Sync, is it so we don't have to think about it in phase 2 when some can be used for passkeys, since that'll be a much easier sign-in for those users? Seems reasonable.
Image

Buuut maybe at least until phase 2, we hide the button on this page for Sync users, since all it does is add friction and take them to the exact same page (to enter a password)?

Image

Copy link
Copy Markdown
Contributor

@LZoog LZoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vpomerleau I probably didn't do as thorough of a code review as I could have, but I did look through it and didn't notice anything I'd require a change on before merge, and I did a decent amount of manual testing here. 👍

* to call the profile server. Five minutes is well over any single request's
* latency budget while keeping the token's blast radius small.
*/
export const PROFILE_OAUTH_TOKEN_TTL_SECONDS = 300;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably the right call to make this a new file, we should move some stuff in the Constants file over here.

I had some questions about 300, but I see you've just moved it into a new file here, so seems fine.

Because:
We need to wire the passkey WebAuthn ceremony to the passkey button
introduced in FXA-13487.

This commit:
 * Adds a usePasskeySignIn hook driving the ceremony, error handling,
   Sync merge gate, and post-auth routing, and wires it into the Index
   and Signin pages.
 * Enables passkeyAuthenticationEnabled in local.json-dist.
 * Adds functional tests covering the passkey sign-in happy, cancel,
   and PASSKEY_NOT_FOUND paths.
 * Tweaks SigninPasskeyFallback to align with the latest design
   (avatar + email row with the real avatar fetched, single full-width
   Continue button, copy fixes).

Closes #FXA-13099
@vpomerleau
Copy link
Copy Markdown
Contributor Author

Thanks very much @LZoog for the testing and review!

  • I've reduced the spacing between the separator and banner
  • That's a good thought about adding a timestamp for the creation/last used at. Showing only date matches the design, but it may be more helpful to the user to add a timestamp. I'll connect with UX about this.
  • a11y review is in progress 👍
  • re Sync - using a passkey provides AAL2 verification, so it has the benefit of eliminating the need for additional verification. If it ends up being confusing, we could consider removing the option but this is as designed for now.

@vpomerleau vpomerleau merged commit 046b6bc into main May 20, 2026
21 checks passed
@vpomerleau vpomerleau deleted the FXA-13099 branch May 20, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants