feat(settings): pre-keys keyless login for desktop passkey Sync sign-in#20864
Closed
vpomerleau wants to merge 1 commit into
Closed
feat(settings): pre-keys keyless login for desktop passkey Sync sign-in#20864vpomerleau wants to merge 1 commit into
vpomerleau wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates FxA Settings sign-in/sign-up flows to support “pre-keys” keyless login for desktop Sync passkey sign-in, so Firefox can reflect “signed in” immediately after passkey verification while deferring the keyed fxaccounts:oauth_login until Sync keys exist. It also refactors keys_optional handling so useFxAStatus reports only the raw browser capability and the Integration model applies per-integration policy.
Changes:
- Add a desktop-only “pre-keys”
fxaccounts:loginin the passkey sign-in flow whenkeys_optionalis advertised, and threadsyncPreKeysLoginSentto avoid duplicate login messages later. - Move keys-related policy decisions onto the Integration model (
requiresPasswordForLogin,supportsKeylessLogin,allowsPreKeysSyncLogin,nonSyncKeysRequirePassword) and add unit tests. - Rename the threaded capability prop from
supportsKeysOptionalLogintobrowserSupportsKeysOptionalacross Settings (components, containers, stories, tests), plus formatting-only route/component readability updates.
Reviewed changes
Copilot reviewed 51 out of 51 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fxa-settings/src/pages/Signup/mocks.tsx | Update signup mocks to implement supportsKeylessLogin and rename prop to browserSupportsKeysOptional. |
| packages/fxa-settings/src/pages/Signup/interfaces.ts | Extend signup integration picks to include supportsKeylessLogin. |
| packages/fxa-settings/src/pages/Signup/index.tsx | Use integration policy (supportsKeylessLogin / requiresPasswordForLogin) and rename capability prop. |
| packages/fxa-settings/src/pages/Signup/index.test.tsx | Update tests for renamed capability prop and third-party-auth visibility logic. |
| packages/fxa-settings/src/pages/Signup/index.stories.tsx | Update stories for renamed capability prop. |
| packages/fxa-settings/src/pages/Signup/container.test.tsx | Update container test integration mock + renamed capability prop. |
| packages/fxa-settings/src/pages/Signin/utils.ts | Rename navigation option field and formatting adjustments. |
| packages/fxa-settings/src/pages/Signin/utils.test.ts | Update tests for renamed navigation option field and formatting. |
| packages/fxa-settings/src/pages/Signin/SigninUnblock/mocks.tsx | Add new integration policy methods in mocks (supportsKeylessLogin, allowsPreKeysSyncLogin, nonSyncKeysRequirePassword). |
| packages/fxa-settings/src/pages/Signin/SigninTotpCode/mocks.tsx | Rename parameter/prop to browserSupportsKeysOptional in mocks. |
| packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx | Use browserSupportsKeysOptional from useFxAStatusResult. |
| packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.test.tsx | Formatting updates + renamed prop wiring in tests. |
| packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/interfaces.ts | Rename container prop to browserSupportsKeysOptional. |
| packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx | Use renamed prop for requiresPasswordForLogin decision. |
| packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/interfaces.ts | Rename prop to browserSupportsKeysOptional. |
| packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.tsx | Use renamed prop consistently for password redirect logic. |
| packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.test.tsx | Update tests for renamed prop. |
| packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx | Thread renamed prop into recovery code page. |
| packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/mocks.tsx | Rename prop and update mock useFxAStatus wiring. |
| packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx | Thread renamed capability into passkey sign-in hook and set-password redirect logic. |
| packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.test.tsx | Update tests for renamed prop. |
| packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.tsx | Suppress handleFxaLogin if pre-keys keyless login already fired (syncPreKeysLoginSent). |
| packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.test.tsx | Add coverage ensuring keyless login suppression when syncPreKeysLoginSent. |
| packages/fxa-settings/src/pages/Signin/mocks.tsx | Update signin mocks with new integration policy methods + rename capability prop. |
| packages/fxa-settings/src/pages/Signin/interfaces.ts | Add policy methods to integration pick types; rename navigation option field; add syncPreKeysLoginSent to location state. |
| packages/fxa-settings/src/pages/Signin/index.tsx | Use renamed capability and integration policy for third-party-auth visibility. |
| packages/fxa-settings/src/pages/Signin/index.test.tsx | Update tests for renamed capability prop. |
| packages/fxa-settings/src/pages/Signin/index.stories.tsx | Update stories for renamed capability prop. |
| packages/fxa-settings/src/pages/Signin/components/SigninDecider/index.tsx | Move non-sync password/keys logic to integration helpers and thread renamed capability prop. |
| packages/fxa-settings/src/pages/Signin/components/SigninDecider/index.test.tsx | Update tests for renamed capability prop. |
| packages/fxa-settings/src/pages/Signin/components/SigninCached/index.tsx | Thread renamed capability prop and use it for redirect decisions. |
| packages/fxa-settings/src/pages/Signin/components/SigninCached/index.test.tsx | Update tests for renamed prop. |
| packages/fxa-settings/src/pages/Signin/components/SigninAlternativeAuthOptions/index.tsx | Thread renamed capability prop into passkey sign-in hook. |
| packages/fxa-settings/src/pages/PostVerify/ThirdPartyAuthCallback/index.tsx | Use renamed capability in requiresPasswordForLogin and navigation options. |
| packages/fxa-settings/src/pages/PostVerify/ThirdPartyAuthCallback/index.test.tsx | Update tests for renamed capability prop. |
| packages/fxa-settings/src/pages/PostVerify/SetPassword/interfaces.ts | Add syncPreKeysLoginSent to location state type. |
| packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx | Suppress handleFxaLogin if pre-keys login already sent; gate route validity on renamed capability prop. |
| packages/fxa-settings/src/pages/PostVerify/SetPassword/container.test.tsx | Add coverage ensuring keyless login suppression when syncPreKeysLoginSent + rename prop. |
| packages/fxa-settings/src/pages/Index/mocks.tsx | Update index mocks with new integration policy methods + rename capability prop. |
| packages/fxa-settings/src/pages/Index/interfaces.ts | Add policy methods to index integration picks. |
| packages/fxa-settings/src/pages/Index/index.tsx | Thread renamed capability and use integration policy for third-party-auth visibility. |
| packages/fxa-settings/src/pages/Index/index.test.tsx | Update tests for renamed capability prop. |
| packages/fxa-settings/src/pages/Index/index.stories.tsx | Update stories for renamed capability prop. |
| packages/fxa-settings/src/models/integrations/integration.ts | Add supportsKeylessLogin, allowsPreKeysSyncLogin, nonSyncKeysRequirePassword; refactor requiresPasswordForLogin. |
| packages/fxa-settings/src/models/integrations/integration.test.ts | Add unit tests for new integration policy methods. |
| packages/fxa-settings/src/lib/passkeys/signin-flow.ts | Add pre-keys keyless fxaLogin for desktop Sync passkey flow; thread syncPreKeysLoginSent. |
| packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx | Add tests for pre-keys keyless login behavior and state threading. |
| packages/fxa-settings/src/lib/hooks/useFxAStatus/mocks.tsx | Rename mocked capability field to browserSupportsKeysOptional. |
| packages/fxa-settings/src/lib/hooks/useFxAStatus/index.tsx | Make hook report raw keys_optional capability as browserSupportsKeysOptional (integration-agnostic). |
| packages/fxa-settings/src/lib/hooks/useFxAStatus/index.test.tsx | Update tests to match raw capability reporting semantics. |
| packages/fxa-settings/src/components/App/index.tsx | Formatting-only route/component readability updates; thread renamed capability into recovery containers. |
Comments suppressed due to low confidence (1)
packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx:193
- When
syncPreKeysLoginSentis true, this suppresseshandleFxaLogin, but this container is also providing the Sync engine payload (syncEngines) that is normally forwarded viafxaLogin(seesendFxaLoginin pages/Signin/utils.ts). In the pre-keys passkey flow the earlier keylessfxaLoginsent fromlib/passkeys/signin-flow.tsusesintegration.getWebChannelServices()without sync engine info (Sync defaults to{ sync: {} }), so withhandleFxaLogin: falsethe browser may never receive the offered/declined engine lists for this flow.
handleFxaLogin: !syncPreKeysLoginSent,
handleFxaOAuthLogin: true,
showSignupConfirmedSync: true,
origin: 'post-verify-set-password',
syncEngines: {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vpomerleau
commented
Jul 14, 2026
|
|
||
| const Settings = lazy(() => import('../Settings')); | ||
|
|
||
| export const App = ({ |
Contributor
Author
There was a problem hiding this comment.
Most of the changes in this file are linting - I guess we haven't touched this one in a while.
LZoog
reviewed
Jul 14, 2026
| /> | ||
| <Route | ||
| path="/pair/supp/allow/*" | ||
| element={<PairSuppAllow integration={integration} />} |
Contributor
There was a problem hiding this comment.
Welp I guess if the linter touched these lines anyway you could go ahead and change these to {...{ integration }} 😅
| ...argOverrides, | ||
| }); | ||
|
|
||
| it('sends a keyless fxaLogin before the password step when browserSupportsKeysOptional is true', async () => { |
Contributor
There was a problem hiding this comment.
Mentioned this in Slack but bringing it here as well: why introduce a new pattern to send fxaLogin and then fxaOAuthLogin later for a Sync sign-in, instead of deferring both fxaLogin and fxaOAuthLogin like we do currently for passwordless accounts trying to sign into Sync, via "Set password"?
Thinking this through:
- I know we send the events together for other passwordless Sync sign-ins
- I know we send them together during a 2FA sign in
- I know we don't send them together if the user is signing up for a new account and that it puts the user into a "Finish account set up" state.
- I think we also don't send them together when we ask for an email confirmation code, but at that point we send
verified: falseand already have the Sync key. Maybe what you have is OK if it matches this behavior?
I guess it does make me wonder though, if we like this approach, if a user needing to provide or set a password because they're trying to sign into Sync and they're passwordless, if we would want to sign those users in halfway too. Not sure. You could say, "Sign in with password is different than Set Password and this should follow the confirmation code flow," but I wonder if it'd be a little less complicated if we sent them together here.
Contributor
There was a problem hiding this comment.
@vpomerleau I thought about this a bit more:
- This will probably leave the user in an "Account disconnected" state the next time they try to finish the flow since
fxaOAuthLoginis never sent and the browser expects it to be part of one flow with the given query params - I think you can do what you're wanting by sending
fxaLogin, and then sendingfxaOAuthLoginas a keyless signin immediately also during navigation when the user is taken to this page. Then, use the new web channel message Vijay implemented a few months ago to get "new oauth login query params" from Firefox. Send both together again after the user enters a password using the new query params. Firefox will see this as two logins but that seems fine to me- edit: it looks like this was landed on the Desktop side: https://bugzilla.mozilla.org/show_bug.cgi?id=1990334 but we didn't pull this in and start using it yet (FXA-11915)
- One other oddity, is for the first set of messages that's a keyless login, I don't know what you should send as the 'service' value inside the webchannel message. I also am not sure what Firefox does with this on their side but it's something Mark and I had talked about long ago, but it feels funky to send
services: { sync: {}}with the denied/approved engine list when the user might abandon and not actually be signed into Sync. I think it's probably more proper to introduce a "no service login" in the web channel message this like, can Firefox handle you sendingservices: {}blank there or does it expect at least one key in that object? - This isn't going to work for Mobile because Mobile requires keys, but that's covered with the capability check and should be fine, just be sure you don't send the first sets of logins for that case, and be sure that there's an issue filed on the iOS and Android side to support this web channel messaging (the "grab new oauth query params" new one) and probably let Ross know about it or post a message in
#fxa-mobile-integrationsand mention we may want to support it as part of Sync decoupling because the special casing for this is going to have to ALSO be on if there's a response to that new web channel message and if keys optional is supported (we'd want this anyway with Mobile moving away from the custom tabs) - Be sure to send the canLinkAccount message and check if the browser version supports the UID version and send if so (I'm actually not sure if we need the check since either way for this one, we send the can link account message at the same time regardless of if UID version is supported or not; you might be able to just send it with UID and assume the field will be ignored by the browser if unsupported)
Because: - A keys-required (Sync) passkey sign-in withheld both WebChannel messages until after the password step, so the browser reflected nothing as signed-in until keys were ready. - The keys-optional check lived in the fxa_status hook, conflating the raw browser capability with per-integration client requirements. This commit: - Fires a keyless fxaccounts:login right after passkey verification on desktop when the browser advertises keys_optional; defers only the keyed oauth_login until keys exist (Sync enables once keys arrive). - useFxAStatus now reports only the raw browser capability (browserSupportsKeysOptional). The per-login keys policy moves onto the integration model: requiresPasswordForLogin, supportsKeylessLogin, allowsPreKeysSyncLogin (unit-tested). - Renames the threaded supportsKeysOptionalLogin prop to browserSupportsKeysOptional across settings. - No functional change for non-Sync integrations: supportsKeylessLogin reproduces the prior derived value (isFirefoxNonSync implies isOAuthNative). Closes #FXA-14129
vpomerleau
force-pushed
the
FXA-14129-prekeys-poc
branch
from
July 15, 2026 08:51
e66521e to
12411c5
Compare
vpomerleau
marked this pull request as draft
July 15, 2026 15:14
Contributor
Author
|
Closing this PR - request is on hold pending product decisions. |
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
keys_optionalcheck lived in thefxa_statushook, conflating the raw browser capability with per-integration client requirements.This pull request
fxaccounts:loginright after passkey verification on desktop when the browser advertiseskeys_optional, and defers only the keyedfxaccounts:oauth_login(which carries the Sync keys) until keys exist — the browser reflects "signed in" immediately and Sync enables once keys arrive (packages/fxa-settings/src/lib/passkeys/signin-flow.ts).useFxAStatus(now reports only the rawbrowserSupportsKeysOptional) onto the integration model —requiresPasswordForLogin,supportsKeylessLogin,allowsPreKeysSyncLogin, all unit-tested (packages/fxa-settings/src/models/integrations/integration.ts).supportsKeysOptionalLoginprop tobrowserSupportsKeysOptionalacross settings.isSignedIntoFirefox) — and generalizes that page for non-passkey flows via areason('passkey' | 'resume') that switches the copy, Glean, andverificationMethod(SigninDecider,SigninPasskeyFallback).INVALID_TOKENreauth clears the token and restarts a fresh sign-in.navigate()inPostVerify/SetPasswordby moving the redirect guards into auseEffect.oauth_loginwithout keys.Issue that this pull request solves
Closes: FXA-14129
Checklist
Put an
xin the boxes that applyHow to review (Optional)
lib/passkeys/signin-flow.ts(keyless login after passkey),models/integrations/integration.ts(the new methods),pages/Signin/components/SigninDecider/index.tsx(resume reroute),SigninPasskeyFallback(generalized enter-password page + stale-session recovery),PostVerify/SetPassword.signin-flow→SigninDeciderreroute → destination containers → prop threading.oauth_loginmust never be sent without keys); the resume reroute gate (isSignedIntoFirefox+ a stored account) and itsINVALID_TOKENstale-session recovery.Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Any other information that is important to this pull request.