feat(settings): wire up the password-free passkey opt-in - #21223
Conversation
3c6caea to
ecfec14
Compare
ecfec14 to
42b7eb9
Compare
9ef908f to
c1863cf
Compare
c1863cf to
38f69e6
Compare
38f69e6 to
6aaa464
Compare
bcolsson
left a comment
There was a problem hiding this comment.
Would it be possible to keep the old string IDs for the duplicate strings to reduce the churn of having to retranslate these?
6aaa464 to
00f6841
Compare
Sorry about that! Reverted now. |
0c37e19 to
9aa449b
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces cross-surface auth-flow changes involving sensitive key material handling and server-side wrap replacement semantics that warrant final human review for security and correctness.
Review details
- Files reviewed: 32/32 changed files
- Comments generated: 0 new
- Review effort level: Lite
| inline-passwordless-sync-setup-enabling = Enabling… | ||
| inline-passwordless-sync-setup-not-now-button = Not now | ||
| # Success message shown in the Settings alert bar after the passkey was stored. | ||
| inline-passwordless-sync-setup-success-alert = This passkey is set up for password-free sign-in. |
There was a problem hiding this comment.
The success and error message strings are under discussion and will likely change before this is merged
nshirley
left a comment
There was a problem hiding this comment.
Just a few more questions! 🙂
| * by the passkey ceremony; `kB` is filled in by the password step that | ||
| * follows. | ||
| */ | ||
| export type PasskeyWrapData = { |
There was a problem hiding this comment.
Kind of thinking out loud here. I don't know exactly what the mechanism might look like (took a shot at it below), but I see this state is having to be carefully managed across several places and I wonder if there's a way to centralize the management of it; hoisting the guard to clear it to the app index?
// something like this in app/index to prevent it from leaking out
const WRAP_MATERIAL_ROUTES = ['/signin_passkey_fallback', '/inline_passwordless_sync_setup'];
useEffect(() => {
if (!WRAP_MATERIAL_ROUTES.some((r) => location.pathname.startsWith(r))) {
sensitiveDataClient.clearPasskeyWrapData();
}
}, [location.pathname, sensitiveDataClient]);I also could be over-thinking this, but curious what you think?
There was a problem hiding this comment.
I like this. Updated.
00c5046 to
3b4f402
Compare
3b4f402 to
853054a
Compare
Because: - A desktop browser sign-in that needs encryption keys (Sync, for example) still requires a password after a PRF passkey until a wrap is stored; this is the one place the user can store one. - The proof minted at sign-in lives ten minutes and the password step can outlast it. This commit: - Requests the `passkey` scope on eligible sign-ins and holds the PRF output, proof and `kB` in SensitiveDataClient. - Routes sign-ins where the account has a password and the passkey has no wrap to /inline_passwordless_sync_setup in place of the browser's post-sign-in landing page. - Adds the opt-in container: stores the wrap, lands in Settings with a banner, and zeroes the held material on every exit. - Steps up with the passkey for a fresh proof when the sign-in one has expired, then submits the same envelope again. - Replaces a stored wrap that predates `keysChangedAt` on POST /passkey/wraps instead of refusing it. Closes #FXA-13151
853054a to
b0899e0
Compare
Because
passkeyPasswordlessSyncEnabledflag; nothing renders when it is off.This pull request
passkeyscope on desktop Sync sign-ins and holds the PRF output, proof andkBinSensitiveDataClient./inline_passwordless_sync_setupin place of the browser's post-sign-in landing page.POST /passkey/wrapsreplaces a stored wrap that predateskeysChangedAtinstead of refusing it, deleting only the row it read; the opt-in treats a remaining conflict as already stored.Issue that this pull request solves
Closes: FXA-13151
Checklist
Put an
xin the boxes that applyHow to review (Optional)
lib/passkeys/signin-flow.ts(eligibility and stash),lib/passkeys/wrap/creation.ts(store and step-up retry),pages/InlinePasswordlessSyncSetup/container.tsx(exit paths),passkey.service.tsstorePasskeyWrap(stale replacement).SensitiveDataClientand is zeroed on every exit, including unmount and a ceremony or navigation that finishes after the page is gone. The retry re-POSTs the already-sealed envelope, so it needs nokB. A wrap conflict on this surface can only be a live wrap from the same key epoch, hence success. The stale replacement is a read, then a delete pinned to that row'screatedAt, then an insert; zero rows deleted, or a non-finitekeysChangedAt, is a conflict.Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
syncHidePromoAfterLoginflag also withdraws this offer. Whether it should is an open product question.