Conversation
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
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.
Because
kB, and the only caller that needs it is the sign-in that is about to use it./passkey/authentication/finishis 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
passkeyWrapfromPOST /passkey/authentication/finish, but only on a keys-required sign-in, so no other caller is handed key material.keysChangedAt, since it seals akBthe account no longer uses.Issue that this pull request solves
Closes: FXA-13143
Checklist
Put an
xin the boxes that applyHow to review (Optional)
PasskeyService.getPasskeyWrapandauthenticationFinishinpackages/fxa-auth-server/lib/routes/passkeys.ts!(wrap.createdAt >= keysChangedAt)) so a non-finitekeysChangedAtwithholds rather than serves — it is the only control enforcing the invariant, and>would read false forNaNand quietly disable it.Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
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.PASSKEY_WRAP_STALEwas not allocated and 236 stays free.passkeys.tsin different regions, so whichever lands second may need a trivial rebase, but neither blocks the other.Account.createPassword(fxa-shared/db/models/auth/account.ts:505) setskeysChangedAt: nowunconditionally, but the v2 branch ofPOST /password/createsupplieswrapKbfrom the client specifically to preservekB. 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.fxa-auth-clientchange is needed:passkeyWraprides the existing/passkey/authentication/finishresponse.passkey_wraps.in.spec.tscovers wrap storage only.