Conversation
There was a problem hiding this comment.
Pull request overview
Fixes cached sign-in behavior in Firefox when site storage is cleared but the browser still has a signed-in user via WebChannel, ensuring the correct account is suggested and routing is safe for unverified browser sessions.
Changes:
- Add a Firefox/WebChannel fallback in
IndexContainerto fetch and persist{uid, email}when local storage is empty. - Gate rendering/loading so the email-first UI doesn’t flash before the WebChannel check completes.
- Add unit + functional test coverage for the WebChannel fallback and SmartWindow flow after clearing storage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/fxa-settings/src/pages/Index/container.tsx | Adds WebChannel fallback + loading gate; conditionally persists sessionToken only when verified === true. |
| packages/fxa-settings/src/pages/Index/container.test.tsx | Adds unit tests validating WebChannel fallback persistence behavior and skip conditions. |
| packages/functional-tests/tests/oauth/syncSignIn.spec.ts | Adds functional coverage for SmartWindow OAuth-native flow after SyncOAuth + cleared storage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ...(isTrustedSession | ||
| ? { sessionToken: userFromBrowser.sessionToken } | ||
| : {}), |
There was a problem hiding this comment.
persistAccount merges with any existing accounts[uid] entry. In the verified !== true path you omit sessionToken, but that does not clear a previously stored sessionToken (or other sensitive fields like verified) for the same uid; it would be retained via the merge. To ensure we never trust a stale token, explicitly overwrite/clear these fields when the browser user is not trusted (e.g., pass sessionToken: undefined and, if applicable, reset related verification flags) rather than omitting them from the update.
| ...(isTrustedSession | |
| ? { sessionToken: userFromBrowser.sessionToken } | |
| : {}), | |
| verified: isTrustedSession, | |
| sessionToken: isTrustedSession | |
| ? userFromBrowser.sessionToken | |
| : undefined, |
Because: When localStorage is cleared but Firefox already has a
signed-in user via WebChannel, the Index page asked for the email
instead of recognising the user. Mark observed this for SmartWindow
and VPN flows when "Clear cookies and site data on close" was enabled,
or after using "Forget about this site" for accounts.firefox.com.
This commit:
- Adds a WebChannel browser-user fallback in IndexContainer: when
localStorage is empty and we are in Firefox, request the signed-in
user from the browser and persist { uid, email } so the cached
signin page suggests the right account.
- Omits sessionToken when the browser reports verified=false. The
browser's signedInUser record can be stale for the target service
(e.g. SmartWindow) since SyncOAuth never sends a follow-up
fxaLogin with verified=true after token-code completion. Routing
the user through the password form is the safe path.
- Holds the loading state until the WebChannel check completes so
the email-first form does not briefly flash before the suggested
email appears.
- Adds unit tests for the new fallback behaviour.
- Adds functional tests under "signin with OAuth after Sync" that
drive a real SyncOAuth flow, clear cookies/localStorage, then
enter SmartWindow and assert the password form is shown with the
email pre-filled (instead of the email-first form). The second
test takes the flow through token-code to /settings.
Fixes FXA-13494
vpomerleau
left a comment
vpomerleau
left a comment
There was a problem hiding this comment.
Tested and works as described, code looks good.
Copilot's comment is a nice to have but in practice unlikely to cause problems.
Because
signedInUser.verifiedflag staysfalseeven after a successful SyncOAuth signin, becauseSigninTokenCodedeliberately does not send a follow-upfxaLoginwithverified=trueafter token-code completion (pages/Signin/SigninTokenCode/index.tsx:180). The existingAppsession-init only trustsverified === true, so it fell through to empty localStorage.This pull request
pages/Index/container.tsx: when localStorage is empty and we are in Firefox, callsfirefox.requestSignedInUserand persists{ uid, email }so the cached signin page suggests the right account.sessionTokenwhen the browser reportsverified=falseso we never trust a stale token; the user is routed to the password form, which is the safe path.isLoadingstate until the WebChannel check completes so the email-first form does not briefly flash before the suggested email appears.Issue that this pull request solves
Closes: https://mozilla-hub.atlassian.net/browse/FXA-13494
Checklist
Put an
xin the boxes that applyOther information
How to test (manual):
about:preferences#privacyand use "Clear data" / "Forget about this site" foraccounts.firefox.com(or set "Delete cookies and site data when Firefox is closed" and restart).