Skip to content

Commit 86412e0

Browse files
lwin-kyawlwin-kyaw
authored
fix: social login authentication state before rehydrate cp-13.11.0 (#38170)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Fixed social login authentication-state validation in rehydration flow. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/38170?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: fixed social login authentication validation in rehydrate. ## **Related issues** Fixes: #38172 ## **Manual testing steps** 1. Click on create new wallet and login with existing social login account 2. After completing social login, close the browser at the unlock page 3. Open the browser and wallet again. 4. You should be redirect back to the welcome page ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Refactors social login onboarding to async-validate auth before redirects, adjusts first-time flow checks, and simplifies Account Exist/Not Found to navigate without mutating flow state; updates tests accordingly. > > - **Onboarding Welcome (`ui/pages/onboarding-flow/welcome/welcome.js`)** > - Replace `getIsSocialLoginUserAuthenticated` with `getIsSocialLoginFlow` and async auth check via `getIsSeedlessOnboardingUserAuthenticated` before redirecting. > - Guard redirects with `isMounted` flag; handle social create vs import navigation accordingly. > - Defer setting `firstTimeFlowType` for social flows until after OAuth result (set to `socialCreate` for new users, `socialImport` for existing). > - **Account pages** > - `account-exist.tsx` and `account-not-found.tsx`: > - Remove `setFirstTimeFlowType` dispatches; rely on navigation only. > - In `useEffect`, invert/clarify conditions to show page only for the matching `FirstTimeFlowType` (`socialImport` for exist, `socialCreate` for not-found); otherwise redirect to `ONBOARDING_WELCOME_ROUTE`. > - Add "use different login method" handler to reset onboarding state and navigate to welcome. > - **Tests** > - Update tests to remove expectations around `setFirstTimeFlowType` and reflect new routing conditions and reset behavior in both account pages. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 97b3b51. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent e7f54a4 commit 86412e0

File tree

5 files changed

+52
-48
lines changed

5 files changed

+52
-48
lines changed

‎ui/pages/onboarding-flow/account-exist/account-exist.test.tsx‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,6 @@ describe('Account Exist Seedless Onboarding View', () => {
4646
});
4747

4848
it('should navigate to the unlock page when the button is clicked', async () => {
49-
const setFirstTimeFlowTypeSpy = jest
50-
.spyOn(Actions, 'setFirstTimeFlowType')
51-
.mockReturnValue(jest.fn().mockResolvedValueOnce(null));
52-
5349
const { getByText } = renderWithProvider(<AccountExist />, customMockStore);
5450
const loginButton = getByText('Log in');
5551
fireEvent.click(loginButton);
@@ -58,18 +54,15 @@ describe('Account Exist Seedless Onboarding View', () => {
5854
expect(mockUseNavigate).toHaveBeenCalledWith(ONBOARDING_UNLOCK_ROUTE, {
5955
replace: true,
6056
});
61-
expect(setFirstTimeFlowTypeSpy).toHaveBeenCalledWith(
62-
FirstTimeFlowType.socialImport,
63-
);
6457
});
6558
});
6659

67-
it('should navigate to the welcome page when the firstTimeFlowType is not socialCreate', () => {
60+
it('should navigate to the welcome page when the firstTimeFlowType is not socialImport', () => {
6861
const store = configureMockStore([thunk])({
6962
...mockState,
7063
metamask: {
7164
...mockState.metamask,
72-
firstTimeFlowType: FirstTimeFlowType.socialImport,
65+
firstTimeFlowType: FirstTimeFlowType.socialCreate,
7366
},
7467
});
7568

‎ui/pages/onboarding-flow/account-exist/account-exist.tsx‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import { FirstTimeFlowType } from '../../../../shared/constants/onboarding';
2929
import {
3030
forceUpdateMetamaskState,
3131
resetOnboarding,
32-
setFirstTimeFlowType,
3332
} from '../../../store/actions';
3433
import { MetaMetricsContext } from '../../../contexts/metametrics';
3534
import {
@@ -80,15 +79,11 @@ export default function AccountExist() {
8079
tags: { source: 'account_status_redirect' },
8180
parentContext: onboardingParentContext?.current,
8281
});
83-
await dispatch(setFirstTimeFlowType(FirstTimeFlowType.socialImport));
8482
navigate(ONBOARDING_UNLOCK_ROUTE, { replace: true });
8583
};
8684

8785
useEffect(() => {
88-
if (firstTimeFlowType !== FirstTimeFlowType.socialCreate) {
89-
navigate(ONBOARDING_WELCOME_ROUTE, { replace: true });
90-
}
91-
if (firstTimeFlowType === FirstTimeFlowType.socialCreate) {
86+
if (firstTimeFlowType === FirstTimeFlowType.socialImport) {
9287
// Track page view event for account already exists page
9388
trackEvent({
9489
category: MetaMetricsEventCategory.Onboarding,
@@ -99,9 +94,11 @@ export default function AccountExist() {
9994
op: TraceOperation.OnboardingUserJourney,
10095
parentContext: onboardingParentContext?.current,
10196
});
97+
} else {
98+
navigate(ONBOARDING_WELCOME_ROUTE, { replace: true });
10299
}
103100
return () => {
104-
if (firstTimeFlowType === FirstTimeFlowType.socialCreate) {
101+
if (firstTimeFlowType === FirstTimeFlowType.socialImport) {
105102
bufferedEndTrace?.({
106103
name: TraceName.OnboardingNewSocialAccountExists,
107104
});

‎ui/pages/onboarding-flow/account-not-found/account-not-found.test.tsx‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,6 @@ describe('Account Not Found Seedless Onboarding View', () => {
5353
});
5454

5555
it('should navigate to the create-password route when the button is clicked', async () => {
56-
const setFirstTimeFlowTypeSpy = jest
57-
.spyOn(Actions, 'setFirstTimeFlowType')
58-
.mockReturnValue(jest.fn().mockResolvedValueOnce(null));
59-
6056
const { getByText } = renderWithProvider(
6157
<AccountNotFound />,
6258
customMockStore,
@@ -66,9 +62,6 @@ describe('Account Not Found Seedless Onboarding View', () => {
6662
fireEvent.click(loginButton);
6763

6864
await waitFor(() => {
69-
expect(setFirstTimeFlowTypeSpy).toHaveBeenCalledWith(
70-
FirstTimeFlowType.socialCreate,
71-
);
7265
expect(mockUseNavigate).toHaveBeenCalledWith(
7366
ONBOARDING_CREATE_PASSWORD_ROUTE,
7467
{
@@ -78,12 +71,12 @@ describe('Account Not Found Seedless Onboarding View', () => {
7871
});
7972
});
8073

81-
it('should navigate to the welcome page when the firstTimeFlowType is not socialImport', () => {
74+
it('should navigate to the welcome page when the firstTimeFlowType is not socialCreate', () => {
8275
const store = configureMockStore([thunk])({
8376
...mockState,
8477
metamask: {
8578
...mockState.metamask,
86-
firstTimeFlowType: FirstTimeFlowType.socialCreate,
79+
firstTimeFlowType: FirstTimeFlowType.socialImport,
8780
},
8881
});
8982

‎ui/pages/onboarding-flow/account-not-found/account-not-found.tsx‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import { FirstTimeFlowType } from '../../../../shared/constants/onboarding';
2929
import {
3030
forceUpdateMetamaskState,
3131
resetOnboarding,
32-
setFirstTimeFlowType,
3332
} from '../../../store/actions';
3433
import {
3534
MetaMetricsEventAccountType,
@@ -74,16 +73,11 @@ export default function AccountNotFound() {
7473
tags: { source: 'account_status_redirect' },
7574
parentContext: onboardingParentContext?.current,
7675
});
77-
dispatch(setFirstTimeFlowType(FirstTimeFlowType.socialCreate));
7876
navigate(ONBOARDING_CREATE_PASSWORD_ROUTE, { replace: true });
7977
};
8078

8179
useEffect(() => {
82-
if (firstTimeFlowType !== FirstTimeFlowType.socialImport) {
83-
// if the onboarding flow is not social import, redirect to the welcome page
84-
navigate(ONBOARDING_WELCOME_ROUTE, { replace: true });
85-
}
86-
if (firstTimeFlowType === FirstTimeFlowType.socialImport) {
80+
if (firstTimeFlowType === FirstTimeFlowType.socialCreate) {
8781
trackEvent({
8882
category: MetaMetricsEventCategory.Onboarding,
8983
event: MetaMetricsEventName.AccountNotFoundPageViewed,
@@ -93,9 +87,11 @@ export default function AccountNotFound() {
9387
op: TraceOperation.OnboardingUserJourney,
9488
parentContext: onboardingParentContext?.current,
9589
});
90+
} else {
91+
navigate(ONBOARDING_WELCOME_ROUTE, { replace: true });
9692
}
9793
return () => {
98-
if (firstTimeFlowType === FirstTimeFlowType.socialImport) {
94+
if (firstTimeFlowType === FirstTimeFlowType.socialCreate) {
9995
bufferedEndTrace?.({
10096
name: TraceName.OnboardingExistingSocialAccountNotFound,
10197
});

‎ui/pages/onboarding-flow/welcome/welcome.js‎

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,15 @@ import {
2323
getCurrentKeyring,
2424
getFirstTimeFlowType,
2525
getIsParticipateInMetaMetricsSet,
26-
getIsSocialLoginUserAuthenticated,
26+
getIsSocialLoginFlow,
2727
} from '../../../selectors';
2828
import { FirstTimeFlowType } from '../../../../shared/constants/onboarding';
2929
import { MetaMetricsContext } from '../../../contexts/metametrics';
3030
import {
3131
setFirstTimeFlowType,
3232
startOAuthLogin,
3333
setParticipateInMetaMetrics,
34+
getIsSeedlessOnboardingUserAuthenticated,
3435
} from '../../../store/actions';
3536
import {
3637
MetaMetricsEventAccountType,
@@ -72,9 +73,7 @@ export default function OnboardingWelcome() {
7273
getIsSeedlessOnboardingFeatureEnabled();
7374
const firstTimeFlowType = useSelector(getFirstTimeFlowType);
7475
const isWalletResetInProgress = useSelector(getIsWalletResetInProgress);
75-
const isUserAuthenticatedWithSocialLogin = useSelector(
76-
getIsSocialLoginUserAuthenticated,
77-
);
76+
const isSocialLoginFLow = useSelector(getIsSocialLoginFlow);
7877
const isParticipateInMetaMetricsSet = useSelector(
7978
getIsParticipateInMetaMetricsSet,
8079
);
@@ -94,9 +93,23 @@ export default function OnboardingWelcome() {
9493
useState(shouldSkipAnimation);
9594

9695
const isFireFox = getBrowserName() === PLATFORM_FIREFOX;
96+
97+
const getIsUserAuthenticatedWithSocialLogin = useCallback(async () => {
98+
if (!isSocialLoginFLow) {
99+
return true;
100+
}
101+
102+
const isSeedlessOnboardingUserAuthenticated = await dispatch(
103+
getIsSeedlessOnboardingUserAuthenticated(),
104+
);
105+
return isSeedlessOnboardingUserAuthenticated;
106+
}, [dispatch, isSocialLoginFLow]);
107+
97108
// Don't allow users to come back to this screen after they
98109
// have already imported or created a wallet
99110
useEffect(() => {
111+
let isMounted = true;
112+
100113
if (
101114
currentKeyring &&
102115
!newAccountCreationInProgress &&
@@ -117,22 +130,33 @@ export default function OnboardingWelcome() {
117130
} else {
118131
navigate(ONBOARDING_REVIEW_SRP_ROUTE, { replace: true });
119132
}
120-
} else if (isUserAuthenticatedWithSocialLogin) {
121-
if (firstTimeFlowType === FirstTimeFlowType.socialCreate) {
122-
navigate(ONBOARDING_CREATE_PASSWORD_ROUTE, { replace: true });
123-
} else {
124-
navigate(ONBOARDING_UNLOCK_ROUTE, { replace: true });
125-
}
133+
} else if (isSocialLoginFLow) {
134+
(async () => {
135+
const isUserAuthenticatedWithSocialLogin =
136+
await getIsUserAuthenticatedWithSocialLogin();
137+
if (isMounted && isUserAuthenticatedWithSocialLogin) {
138+
if (firstTimeFlowType === FirstTimeFlowType.socialCreate) {
139+
navigate(ONBOARDING_CREATE_PASSWORD_ROUTE, { replace: true });
140+
} else {
141+
navigate(ONBOARDING_UNLOCK_ROUTE, { replace: true });
142+
}
143+
}
144+
})();
126145
}
146+
147+
return () => {
148+
isMounted = false;
149+
};
127150
}, [
128151
currentKeyring,
129152
navigate,
130153
firstTimeFlowType,
131154
newAccountCreationInProgress,
132155
isParticipateInMetaMetricsSet,
133-
isUserAuthenticatedWithSocialLogin,
156+
getIsUserAuthenticatedWithSocialLogin,
134157
isFireFox,
135158
isWalletResetInProgress,
159+
isSocialLoginFLow,
136160
]);
137161

138162
const trackEvent = useContext(MetaMetricsContext);
@@ -255,7 +279,6 @@ export default function OnboardingWelcome() {
255279
async (socialConnectionType) => {
256280
setIsLoggingIn(true);
257281
setNewAccountCreationInProgress(true);
258-
await dispatch(setFirstTimeFlowType(FirstTimeFlowType.socialCreate));
259282

260283
trackEvent({
261284
category: MetaMetricsEventCategory.Onboarding,
@@ -282,8 +305,10 @@ export default function OnboardingWelcome() {
282305
op: TraceOperation.OnboardingUserJourney,
283306
parentContext: onboardingParentContext.current,
284307
});
308+
await dispatch(setFirstTimeFlowType(FirstTimeFlowType.socialCreate));
285309
navigate(ONBOARDING_CREATE_PASSWORD_ROUTE, { replace: true });
286310
} else {
311+
await dispatch(setFirstTimeFlowType(FirstTimeFlowType.socialImport));
287312
navigate(ONBOARDING_ACCOUNT_EXIST, { replace: true });
288313
}
289314
} catch (error) {
@@ -306,8 +331,6 @@ export default function OnboardingWelcome() {
306331
const onSocialLoginImportClick = useCallback(
307332
async (socialConnectionType) => {
308333
setIsLoggingIn(true);
309-
dispatch(setFirstTimeFlowType(FirstTimeFlowType.socialImport));
310-
311334
trackEvent({
312335
category: MetaMetricsEventCategory.Onboarding,
313336
event: MetaMetricsEventName.WalletImportStarted,
@@ -329,13 +352,15 @@ export default function OnboardingWelcome() {
329352
});
330353

331354
if (isNewUser) {
355+
await dispatch(setFirstTimeFlowType(FirstTimeFlowType.socialCreate));
332356
navigate(ONBOARDING_ACCOUNT_NOT_FOUND);
333357
} else {
334358
bufferedTrace?.({
335359
name: TraceName.OnboardingExistingSocialLogin,
336360
op: TraceOperation.OnboardingUserJourney,
337361
parentContext: onboardingParentContext.current,
338362
});
363+
await dispatch(setFirstTimeFlowType(FirstTimeFlowType.socialImport));
339364
navigate(ONBOARDING_UNLOCK_ROUTE);
340365
}
341366
} catch (error) {
@@ -345,13 +370,13 @@ export default function OnboardingWelcome() {
345370
}
346371
},
347372
[
348-
dispatch,
349373
handleSocialLogin,
350374
trackEvent,
351375
navigate,
352376
onboardingParentContext,
353377
handleSocialLoginError,
354378
bufferedTrace,
379+
dispatch,
355380
],
356381
);
357382

0 commit comments

Comments
 (0)