Skip to content

Commit ec8c228

Browse files
vbudhramvbudhram
authored
Merge pull request #20466 from mozilla/fxa-13494
fix(settings): respect Firefox-signed-in user on cached signin page
2 parents 04a9e51 + cfd89ba commit ec8c228

3 files changed

Lines changed: 306 additions & 0 deletions

File tree

‎packages/functional-tests/tests/oauth/syncSignIn.spec.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
import { expect, test } from '../../lib/fixtures/standard';
6+
import {
7+
smartWindowDesktopOAuthQueryParams,
8+
syncDesktopOAuthQueryParams,
9+
} from '../../lib/query-params';
610

711
test.describe('severity-1 #smoke', () => {
812
test.describe('signin with OAuth after Sync', () => {
@@ -96,6 +100,85 @@ test.describe('severity-1 #smoke', () => {
96100
// OAuth sign-in should succeed even though the sync session was not verified
97101
expect(await relier.isLoggedIn()).toBe(true);
98102
});
103+
104+
// After SyncOAuth signin and a localStorage wipe, OAuth-native flows
105+
// should still suggest the browser user instead of the email-first form.
106+
test('SmartWindow respects browser-signed-in user after SyncOAuth, localStorage cleared', async ({
107+
target,
108+
syncOAuthBrowserPages: {
109+
page,
110+
signin,
111+
signinTokenCode,
112+
connectAnotherDevice,
113+
},
114+
testAccountTracker,
115+
}) => {
116+
const syncCredentials = await testAccountTracker.signUpSync();
117+
118+
// Real SyncOAuth signin so Firefox stores the user via fxaccounts:login.
119+
await signin.goto('/authorization', syncDesktopOAuthQueryParams);
120+
await signin.fillOutEmailFirstForm(syncCredentials.email);
121+
await signin.fillOutPasswordForm(syncCredentials.password);
122+
await page.waitForURL(/signin_token_code/);
123+
const code = await target.emailClient.getVerifyLoginCode(
124+
syncCredentials.email
125+
);
126+
await signinTokenCode.fillOutCodeForm(code);
127+
await expect(connectAnotherDevice.fxaConnected).toBeVisible();
128+
129+
await page.context().clearCookies();
130+
await page.evaluate(() => {
131+
localStorage.clear();
132+
sessionStorage.clear();
133+
});
134+
135+
await signin.goto('/authorization', smartWindowDesktopOAuthQueryParams);
136+
await expect(signin.passwordFormHeading).toBeVisible();
137+
await expect(page.getByText(syncCredentials.email)).toBeVisible();
138+
await expect(signin.emailFirstHeading).not.toBeVisible();
139+
});
140+
141+
// Same scenario, taken end-to-end through token-code to /settings.
142+
test('SmartWindow cached signin completes token verification', async ({
143+
target,
144+
syncOAuthBrowserPages: {
145+
page,
146+
signin,
147+
signinTokenCode,
148+
connectAnotherDevice,
149+
},
150+
testAccountTracker,
151+
}) => {
152+
const syncCredentials = await testAccountTracker.signUpSync();
153+
154+
await signin.goto('/authorization', syncDesktopOAuthQueryParams);
155+
await signin.fillOutEmailFirstForm(syncCredentials.email);
156+
await signin.fillOutPasswordForm(syncCredentials.password);
157+
await page.waitForURL(/signin_token_code/);
158+
let code = await target.emailClient.getVerifyLoginCode(
159+
syncCredentials.email
160+
);
161+
await signinTokenCode.fillOutCodeForm(code);
162+
await expect(connectAnotherDevice.fxaConnected).toBeVisible();
163+
164+
await page.context().clearCookies();
165+
await page.evaluate(() => {
166+
localStorage.clear();
167+
sessionStorage.clear();
168+
});
169+
170+
await signin.goto('/authorization', smartWindowDesktopOAuthQueryParams);
171+
await expect(signin.passwordFormHeading).toBeVisible();
172+
await expect(page.getByText(syncCredentials.email)).toBeVisible();
173+
await expect(signin.emailFirstHeading).not.toBeVisible();
174+
175+
await signin.fillOutPasswordForm(syncCredentials.password);
176+
await page.waitForURL(/signin_token_code/);
177+
code = await target.emailClient.getVerifyLoginCode(syncCredentials.email);
178+
await signinTokenCode.fillOutCodeForm(code);
179+
180+
await page.waitForURL(/settings/);
181+
});
99182
});
100183

101184
test.describe('signin to Sync after OAuth', () => {

‎packages/fxa-settings/src/pages/Index/container.test.tsx‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { IndexQueryParams } from '../../models/pages/index';
2424
import { GenericData, ModelValidationErrors } from '../../lib/model-data';
2525
import { mockUseFxAStatus } from '../../lib/hooks/useFxAStatus/mocks';
2626
import { firefox } from '../../lib/channels/firefox';
27+
import * as storageUtils from '../../lib/storage-utils';
2728

2829
let mockLocationState = {};
2930
let mockNavigate = jest.fn();
@@ -58,9 +59,19 @@ jest.mock('../../lib/channels/firefox', () => ({
5859
...jest.requireActual('../../lib/channels/firefox'),
5960
firefox: {
6061
fxaCanLinkAccount: jest.fn(),
62+
requestSignedInUser: jest.fn().mockResolvedValue(undefined),
6163
},
6264
}));
6365

66+
jest.mock('../../lib/storage-utils', () => {
67+
const actual = jest.requireActual('../../lib/storage-utils');
68+
return {
69+
...actual,
70+
persistAccount: jest.fn(),
71+
setCurrentAccount: jest.fn(),
72+
};
73+
});
74+
6475
jest.mock('../../lib/email-domain-validator', () => ({
6576
checkEmailDomain: jest.fn(),
6677
}));
@@ -606,6 +617,172 @@ describe('IndexContainer', () => {
606617
});
607618
});
608619

620+
describe('WebChannel browser-user fallback', () => {
621+
let isProbablyFirefoxSpy: jest.SpyInstance;
622+
623+
beforeEach(() => {
624+
isProbablyFirefoxSpy = jest
625+
.spyOn(ModelsModule, 'isProbablyFirefox')
626+
.mockReturnValue(true);
627+
(firefox.requestSignedInUser as jest.Mock).mockReset();
628+
(storageUtils.persistAccount as jest.Mock).mockReset();
629+
(storageUtils.setCurrentAccount as jest.Mock).mockReset();
630+
});
631+
632+
afterEach(() => {
633+
isProbablyFirefoxSpy.mockRestore();
634+
});
635+
636+
it('persists browser user (without sessionToken) when verified is false', async () => {
637+
jest.spyOn(cache, 'currentAccount').mockReturnValue(undefined);
638+
jest.spyOn(cache, 'lastStoredAccount').mockReturnValue(undefined);
639+
(firefox.requestSignedInUser as jest.Mock).mockResolvedValue({
640+
uid: 'browseruid123',
641+
email: '[email protected]',
642+
sessionToken: 'untrusted-token',
643+
verified: false,
644+
});
645+
646+
renderWithLocalizationProvider(
647+
<LocationProvider>
648+
<IndexContainer
649+
{...{
650+
integration,
651+
serviceName: MozServices.Default,
652+
useFxAStatusResult: mockUseFxAStatusResult,
653+
}}
654+
/>
655+
</LocationProvider>
656+
);
657+
658+
await waitFor(() => {
659+
expect(firefox.requestSignedInUser).toHaveBeenCalled();
660+
});
661+
await waitFor(() => {
662+
expect(storageUtils.persistAccount).toHaveBeenCalledWith(
663+
expect.objectContaining({
664+
uid: 'browseruid123',
665+
email: '[email protected]',
666+
})
667+
);
668+
});
669+
const persisted = (storageUtils.persistAccount as jest.Mock).mock
670+
.calls[0][0];
671+
expect(persisted.sessionToken).toBeUndefined();
672+
expect(storageUtils.setCurrentAccount).toHaveBeenCalledWith(
673+
'browseruid123'
674+
);
675+
});
676+
677+
it('persists browser sessionToken when verified is true', async () => {
678+
jest.spyOn(cache, 'currentAccount').mockReturnValue(undefined);
679+
jest.spyOn(cache, 'lastStoredAccount').mockReturnValue(undefined);
680+
(firefox.requestSignedInUser as jest.Mock).mockResolvedValue({
681+
uid: 'browseruid123',
682+
email: '[email protected]',
683+
sessionToken: 'trusted-token',
684+
verified: true,
685+
});
686+
687+
renderWithLocalizationProvider(
688+
<LocationProvider>
689+
<IndexContainer
690+
{...{
691+
integration,
692+
serviceName: MozServices.Default,
693+
useFxAStatusResult: mockUseFxAStatusResult,
694+
}}
695+
/>
696+
</LocationProvider>
697+
);
698+
699+
await waitFor(() => {
700+
expect(storageUtils.persistAccount).toHaveBeenCalledWith(
701+
expect.objectContaining({
702+
uid: 'browseruid123',
703+
email: '[email protected]',
704+
sessionToken: 'trusted-token',
705+
})
706+
);
707+
});
708+
});
709+
710+
it('does not query the browser when localStorage already has an account', async () => {
711+
jest.spyOn(cache, 'currentAccount').mockReturnValue({
712+
uid: 'cacheduid',
713+
email: '[email protected]',
714+
lastLogin: Date.now(),
715+
});
716+
jest.spyOn(cache, 'lastStoredAccount').mockReturnValue(undefined);
717+
718+
renderWithLocalizationProvider(
719+
<LocationProvider>
720+
<IndexContainer
721+
{...{
722+
integration,
723+
serviceName: MozServices.Default,
724+
useFxAStatusResult: mockUseFxAStatusResult,
725+
}}
726+
/>
727+
</LocationProvider>
728+
);
729+
730+
// Cached account triggers auto-submit; navigation proves we skipped the WebChannel call.
731+
await waitFor(() => {
732+
expect(mockNavigate).toHaveBeenCalled();
733+
});
734+
expect(firefox.requestSignedInUser).not.toHaveBeenCalled();
735+
expect(storageUtils.persistAccount).not.toHaveBeenCalled();
736+
});
737+
738+
it('does not query the browser when not running in Firefox', async () => {
739+
isProbablyFirefoxSpy.mockReturnValue(false);
740+
jest.spyOn(cache, 'currentAccount').mockReturnValue(undefined);
741+
jest.spyOn(cache, 'lastStoredAccount').mockReturnValue(undefined);
742+
743+
renderWithLocalizationProvider(
744+
<LocationProvider>
745+
<IndexContainer
746+
{...{
747+
integration,
748+
serviceName: MozServices.Default,
749+
useFxAStatusResult: mockUseFxAStatusResult,
750+
}}
751+
/>
752+
</LocationProvider>
753+
);
754+
755+
await waitFor(() => {
756+
expect(IndexModule.default).toHaveBeenCalled();
757+
});
758+
expect(firefox.requestSignedInUser).not.toHaveBeenCalled();
759+
});
760+
761+
it('does not persist when the browser returns no signed-in user', async () => {
762+
jest.spyOn(cache, 'currentAccount').mockReturnValue(undefined);
763+
jest.spyOn(cache, 'lastStoredAccount').mockReturnValue(undefined);
764+
(firefox.requestSignedInUser as jest.Mock).mockResolvedValue(undefined);
765+
766+
renderWithLocalizationProvider(
767+
<LocationProvider>
768+
<IndexContainer
769+
{...{
770+
integration,
771+
serviceName: MozServices.Default,
772+
useFxAStatusResult: mockUseFxAStatusResult,
773+
}}
774+
/>
775+
</LocationProvider>
776+
);
777+
778+
await waitFor(() => {
779+
expect(firefox.requestSignedInUser).toHaveBeenCalled();
780+
});
781+
expect(storageUtils.persistAccount).not.toHaveBeenCalled();
782+
expect(storageUtils.setCurrentAccount).not.toHaveBeenCalled();
783+
});
784+
});
785+
609786
describe('processEmailSubmission', () => {
610787
describe('success', () => {
611788
it('with a new valid email', async () => {

‎packages/fxa-settings/src/pages/Index/container.tsx‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@ import { AuthError } from '../../lib/oauth';
2222

2323
import {
2424
isOAuthNativeIntegration,
25+
isProbablyFirefox,
2526
useAuthClient,
2627
useConfig,
2728
useFtlMsgResolver,
2829
} from '../../models';
2930
import { isOAuthWebIntegration } from '../../models/integrations/oauth-web-integration';
3031
import { isUnsupportedContext } from '../../models/integrations/utils';
3132
import { IndexQueryParams } from '../../models/pages/index';
33+
import { persistAccount, setCurrentAccount } from '../../lib/storage-utils';
3234

3335
import Index from '.';
3436
import { getLocalizedEmailValidationErrorMessage } from './errorMessageMapper';
@@ -100,6 +102,44 @@ const IndexContainer = ({
100102
const { prefillEmail, deleteAccountSuccess, hasBounced } =
101103
location.state || {};
102104

105+
// sessionToken is dropped unless verified===true; the browser's flag can be
106+
// stale (no follow-up fxaLogin after SyncOAuth token-code), so we re-prompt.
107+
const [browserUserChecked, setBrowserUserChecked] = useState(false);
108+
useEffect(() => {
109+
if (browserUserChecked) {
110+
return;
111+
}
112+
if (currentAccount() || lastStoredAccount() || !isProbablyFirefox()) {
113+
setBrowserUserChecked(true);
114+
return;
115+
}
116+
let cancelled = false;
117+
(async () => {
118+
const userFromBrowser = await firefox.requestSignedInUser(
119+
integration.data?.context || '',
120+
false,
121+
integration.data?.service || ''
122+
);
123+
if (cancelled) {
124+
return;
125+
}
126+
if (userFromBrowser?.uid && userFromBrowser.email) {
127+
const { uid, email, sessionToken, verified } = userFromBrowser;
128+
persistAccount({
129+
uid,
130+
email,
131+
lastLogin: Date.now(),
132+
...(verified === true ? { sessionToken } : {}),
133+
});
134+
setCurrentAccount(uid);
135+
}
136+
setBrowserUserChecked(true);
137+
})();
138+
return () => {
139+
cancelled = true;
140+
};
141+
}, [browserUserChecked, integration]);
142+
103143
// Only used for suggestedEmail; handleSuccessNavigation reads fresh from
104144
// localStorage to avoid stale closures when checking session state.
105145
const cachedAccount = currentAccount() || lastStoredAccount();
@@ -338,6 +378,11 @@ const IndexContainer = ({
338378
return;
339379
}
340380

381+
// Wait for the WebChannel check; otherwise the email-first form flashes briefly.
382+
if (!browserUserChecked) {
383+
return;
384+
}
385+
341386
if (isUnsupportedContext(integration.data.context)) {
342387
hardNavigate('/update_firefox', {}, true);
343388
} else if (shouldTrySuggestedEmail && !attemptedEmailAutoSubmit.current) {
@@ -363,6 +408,7 @@ const IndexContainer = ({
363408
}, [
364409
ftlMsgResolver,
365410
attemptedEmailAutoSubmit,
411+
browserUserChecked,
366412
navigateWithQuery,
367413
processEmailSubmission,
368414
suggestedEmail,

0 commit comments

Comments
 (0)