Skip to content

task(passkey): Return the wrap envelope from passkey sign-in - #21130

Draft
nshirley wants to merge 1 commit into
mainfrom
FXA-13143
Draft

task(passkey): Return the wrap envelope from passkey sign-in#21130
nshirley wants to merge 1 commit into
mainfrom
FXA-13143

Conversation

@nshirley

@nshirley nshirley commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Because

  • Passwordless Sync needs the wrap envelope to unseal kB, and the only caller that needs it is the sign-in that is about to use it.
  • /passkey/authentication/finish is reachable only by completing an assertion against a challenge from /start, so possession of the authenticator is already proven — a stronger gate than a session token on a separate read.

This pull request

  • Returns passkeyWrap from POST /passkey/authentication/finish, but only on a keys-required sign-in, so no other caller is handed key material.
  • Withholds a wrap that predates the account's keysChangedAt, since it seals a kB the account no longer uses.
  • Treats a missing or stale wrap as absent rather than an error, so the client falls back to the password step instead of the sign-in failing.
  • Fires the login metrics event and security event here when a wrap is returned, rather than deferring them to a password step that no longer happens.
  • Reports a stale wrap to Sentry and the log — it means a key rotation left a row behind and the user has silently lost passwordless sign-in.

Issue that this pull request solves

Closes: FXA-13143

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 (Optional)

  • Key files/areas to focus on: PasskeyService.getPasskeyWrap and authenticationFinish in packages/fxa-auth-server/lib/routes/passkeys.ts
  • Suggested review order: service → route handler → response schema → tests
  • Risky or complex parts: the staleness comparison. It is inverted (!(wrap.createdAt >= keysChangedAt)) so a non-finite keysChangedAt withholds rather than serves — it is the only control enforcing the invariant, and > would read false for NaN and quietly disable it.

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

  • Scope change from the ticket. FXA-13143 originally specified GET /passkey/keys/{credentialId}. That endpoint was dropped — nothing needs it, since settings re-enrolment writes a new wrap rather than reading the old one. Reasoning is in a comment on the ticket.
  • No new errno. Nothing returns an HTTP error for a missing or stale wrap, so PASSKEY_WRAP_STALE was not allocated and 236 stays free.
  • Independent of task(passkey): Implement POST /passkey/wraps #21116 (FXA-13142). Both touch passkeys.ts in different regions, so whichever lands second may need a trivial rebase, but neither blocks the other.
  • Open question for the passwordless owner: Account.createPassword (fxa-shared/db/models/auth/account.ts:505) sets keysChangedAt: now unconditionally, but the v2 branch of POST /password/create supplies wrapKb from the client specifically to preserve kB. If that path really does preserve it, a user who enrols a wrap and later adds a password will have that wrap judged stale — permanently, since create-only means re-enrolment 409s. The Sentry alert added here will tell us whether it happens in practice.
  • No fxa-auth-client change is needed: passkeyWrap rides the existing /passkey/authentication/finish response.
  • Integration tests for this path are not included; the existing passkey_wraps.in.spec.ts covers wrap storage only.

Because:
- Passwordless Sync needs the envelope to unseal kB, and the only caller
  that needs it is the sign-in that is about to use it.
- /authentication/finish is reachable only by completing an assertion
  against a challenge from /start, so possession of the authenticator is
  already proven — a stronger gate than a session token on a read.

This commit:
- Returns passkeyWrap from /passkey/authentication/finish, for a
  keys-required sign-in only, behind passwordlessSyncEnabled.
- Withholds a wrap that predates the account's keysChangedAt, and reports
  that to Sentry: it means a rotation left a row behind and the user has
  silently lost passwordless sign-in.
- Reads the wrap after minting the session and swallows any failure, so
  nothing about it can fail a sign-in the assertion already verified.
- Refuses a disabled account, which every password sign-in path already
  does and which now also withholds key material.
- Moves envelope encode/decode into the repository, keyed off the widths,
  replacing hand-written field lists on both routes.

Closes #FXA-13143
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.

1 participant