Skip to content

Commit e2f3f4a

Browse files
ieowieow
authored
fix: face id run twice during rehydration (MetaMask#18036)
<!-- 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? --> Face Id (biometric) run twice during rehydration. This PR reduce FaceId (biometric) run once during rehydration ## **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: ## **Related issues** Fixes: MetaMask#18029 ## **Manual testing steps** 1. Login with seedless flow ( social login) with existing account 2. Login with password and enable biometrics 3. Face ID should only prompt once ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/f60448b0-e0ef-4c85-b664-ab0dccea4a69 ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] 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.
1 parent a3405ef commit e2f3f4a

2 files changed

Lines changed: 42 additions & 37 deletions

File tree

‎app/core/Authentication/Authentication.test.ts‎

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,6 +1112,8 @@ describe('Authentication', () => {
11121112
checkIsPasswordOutdated: jest.fn(),
11131113
} as unknown as SeedlessOnboardingController<EncryptionKey>;
11141114
Engine.context.KeyringController = {
1115+
setLocked: jest.fn(),
1116+
isUnlocked: jest.fn().mockResolvedValue(true),
11151117
addNewKeyring: jest.fn(),
11161118
createNewVaultAndRestore: jest.fn(),
11171119
withKeyring: jest
@@ -1144,8 +1146,9 @@ describe('Authentication', () => {
11441146
type: SecretType.Mnemonic,
11451147
},
11461148
]);
1147-
const newWalletAndRestoreSpy = jest
1148-
.spyOn(Authentication, 'newWalletAndRestore')
1149+
const newWalletVaultAndRestoreSpy = jest
1150+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1151+
.spyOn(Authentication as any, 'newWalletVaultAndRestore')
11491152
.mockResolvedValueOnce(undefined);
11501153

11511154
await Authentication.userEntryAuth(mockPassword, mockAuthData);
@@ -1157,13 +1160,12 @@ describe('Authentication', () => {
11571160
mockSeedPhrase1,
11581161
expect.any(Object),
11591162
);
1160-
expect(newWalletAndRestoreSpy).toHaveBeenCalledWith(
1163+
expect(newWalletVaultAndRestoreSpy).toHaveBeenCalledWith(
11611164
mockPassword,
1162-
mockAuthData,
11631165
uint8ArrayToMnemonic(mockSeedPhrase1, []),
11641166
false,
11651167
);
1166-
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(2); // logIn, passwordSet
1168+
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(3); // logIn, passwordSet, setExistingUser
11671169
expect(OAuthService.resetOauthState).toHaveBeenCalled();
11681170
});
11691171

@@ -1197,8 +1199,9 @@ describe('Authentication', () => {
11971199
getState: jest.fn(() => mockStateLocal),
11981200
} as unknown as ReduxStore);
11991201

1200-
const newWalletAndRestoreSpy = jest
1201-
.spyOn(Authentication, 'newWalletAndRestore')
1202+
const newWalletVaultAndRestoreSpy = jest
1203+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1204+
.spyOn(Authentication as any, 'newWalletVaultAndRestore')
12021205
.mockResolvedValueOnce(undefined);
12031206
(
12041207
Engine.context.KeyringController.addNewKeyring as jest.Mock
@@ -1215,9 +1218,8 @@ describe('Authentication', () => {
12151218
mockSeedPhrase1,
12161219
expect.any(Object),
12171220
);
1218-
expect(newWalletAndRestoreSpy).toHaveBeenCalledWith(
1221+
expect(newWalletVaultAndRestoreSpy).toHaveBeenCalledWith(
12191222
mockPassword,
1220-
mockAuthData,
12211223
uint8ArrayToMnemonic(mockSeedPhrase1, []),
12221224
false,
12231225
);
@@ -1234,7 +1236,7 @@ describe('Authentication', () => {
12341236
keyringId: 'new-keyring-id',
12351237
type: 'mnemonic',
12361238
});
1237-
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(2); // logIn, passwordSet
1239+
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(3); // logIn, passwordSet, setExistingUser
12381240
expect(OAuthService.resetOauthState).toHaveBeenCalled();
12391241
});
12401242

@@ -1252,8 +1254,9 @@ describe('Authentication', () => {
12521254
type: SecretType.PrivateKey,
12531255
},
12541256
]);
1255-
const newWalletAndRestoreSpy = jest
1256-
.spyOn(Authentication, 'newWalletAndRestore')
1257+
const newWalletVaultAndRestoreSpy = jest
1258+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1259+
.spyOn(Authentication as any, 'newWalletVaultAndRestore')
12571260
.mockResolvedValueOnce(undefined);
12581261
const importAccountFromPrivateKeySpy = jest
12591262
.spyOn(Authentication, 'importAccountFromPrivateKey')
@@ -1264,9 +1267,8 @@ describe('Authentication', () => {
12641267
expect(
12651268
Engine.context.SeedlessOnboardingController.fetchAllSecretData,
12661269
).toHaveBeenCalledWith(mockPassword);
1267-
expect(newWalletAndRestoreSpy).toHaveBeenCalledWith(
1270+
expect(newWalletVaultAndRestoreSpy).toHaveBeenCalledWith(
12681271
mockPassword,
1269-
mockAuthData,
12701272
uint8ArrayToMnemonic(mockSeedPhrase1, []),
12711273
false,
12721274
);
@@ -1277,7 +1279,7 @@ describe('Authentication', () => {
12771279
shouldSelectAccount: false,
12781280
},
12791281
);
1280-
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(2); // logIn and passwordSet
1282+
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(3); // logIn and passwordSet, setExistingUser
12811283
expect(OAuthService.resetOauthState).toHaveBeenCalled();
12821284
});
12831285

@@ -1296,14 +1298,15 @@ describe('Authentication', () => {
12961298
},
12971299
]);
12981300
const newWalletAndRestoreSpy = jest
1299-
.spyOn(Authentication, 'newWalletAndRestore')
1301+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1302+
.spyOn(Authentication as any, 'newWalletVaultAndRestore')
13001303
.mockResolvedValueOnce(undefined);
13011304

13021305
await Authentication.userEntryAuth(mockPassword, mockAuthData);
13031306

13041307
expect(newWalletAndRestoreSpy).toHaveBeenCalled();
13051308
expect(Logger.error).toHaveBeenCalledWith(expect.any(Error), 'unknown');
1306-
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(2); // logIn and passwordSet
1309+
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(3); // logIn and passwordSet, setExistingUser
13071310
expect(OAuthService.resetOauthState).toHaveBeenCalled();
13081311
});
13091312

@@ -1321,8 +1324,10 @@ describe('Authentication', () => {
13211324
type: SecretType.PrivateKey,
13221325
},
13231326
]);
1324-
const newWalletAndRestoreSpy = jest
1325-
.spyOn(Authentication, 'newWalletAndRestore')
1327+
1328+
const newWalletVaultAndRestoreSpy = jest
1329+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1330+
.spyOn(Authentication as any, 'newWalletVaultAndRestore')
13261331
.mockResolvedValueOnce(undefined);
13271332
const importError = new Error('Import failed');
13281333
const importAccountFromPrivateKeySpy = jest
@@ -1331,10 +1336,10 @@ describe('Authentication', () => {
13311336

13321337
await Authentication.userEntryAuth(mockPassword, mockAuthData);
13331338

1334-
expect(newWalletAndRestoreSpy).toHaveBeenCalled();
1339+
expect(newWalletVaultAndRestoreSpy).toHaveBeenCalled();
13351340
expect(importAccountFromPrivateKeySpy).toHaveBeenCalled();
13361341
expect(Logger.error).toHaveBeenCalledWith(importError);
1337-
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(2); // logIn and passwordSet
1342+
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(3); // logIn and passwordSet, setExistingUser
13381343
expect(OAuthService.resetOauthState).toHaveBeenCalled();
13391344
});
13401345

@@ -1373,8 +1378,9 @@ describe('Authentication', () => {
13731378
type: SecretType.Mnemonic,
13741379
},
13751380
]);
1376-
const newWalletAndRestoreSpy = jest
1377-
.spyOn(Authentication, 'newWalletAndRestore')
1381+
const newWalletVaultAndRestoreSpy = jest
1382+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1383+
.spyOn(Authentication as any, 'newWalletVaultAndRestore')
13781384
.mockResolvedValueOnce(undefined);
13791385
const error = new Error('Keyring add failed');
13801386
(
@@ -1403,12 +1409,12 @@ describe('Authentication', () => {
14031409
expect(
14041410
Engine.context.KeyringController.addNewKeyring,
14051411
).toHaveBeenCalledTimes(1);
1406-
expect(newWalletAndRestoreSpy).toHaveBeenCalled();
1412+
expect(newWalletVaultAndRestoreSpy).toHaveBeenCalled();
14071413
expect(
14081414
Engine.context.SeedlessOnboardingController.updateBackupMetadataState,
14091415
).not.toHaveBeenCalled();
14101416
expect(Logger.error).toHaveBeenCalledWith(error);
1411-
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(2); // logIn, passwordSet
1417+
expect(ReduxService.store.dispatch).toHaveBeenCalledTimes(3); // logIn, passwordSet, setExistingUser
14121418
expect(OAuthService.resetOauthState).toHaveBeenCalled();
14131419
});
14141420

@@ -1819,9 +1825,7 @@ describe('Authentication', () => {
18191825
} as unknown as ReduxStore);
18201826

18211827
await expect(
1822-
Authentication.syncPasswordAndUnlockWallet(mockGlobalPassword, {
1823-
currentAuthType: AUTHENTICATION_TYPE.PASSWORD,
1824-
}),
1828+
Authentication.syncPasswordAndUnlockWallet(mockGlobalPassword),
18251829
).rejects.toThrow('change password failed');
18261830

18271831
expect(Authentication.lockApp).toHaveBeenCalledWith({ locked: true });

‎app/core/Authentication/Authentication.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -512,10 +512,10 @@ class AuthenticationService {
512512

513513
if (authData.oauth2Login) {
514514
// if seedless flow - rehydrate
515-
await this.rehydrateSeedPhrase(password, authData);
515+
await this.rehydrateSeedPhrase(password);
516516
} else if (await this.checkIsSeedlessPasswordOutdated(false)) {
517517
// if seedless flow completed && seedless password is outdated, sync the password and unlock the wallet
518-
await this.syncPasswordAndUnlockWallet(password, authData);
518+
await this.syncPasswordAndUnlockWallet(password);
519519
} else {
520520
// else srp flow
521521
await this.loginVaultCreation(password);
@@ -902,10 +902,7 @@ class AuthenticationService {
902902
}
903903
};
904904

905-
rehydrateSeedPhrase = async (
906-
password: string,
907-
authData: AuthData,
908-
): Promise<void> => {
905+
rehydrateSeedPhrase = async (password: string): Promise<void> => {
909906
try {
910907
const { SeedlessOnboardingController } = Engine.context;
911908
let allSRPs: Awaited<
@@ -949,7 +946,8 @@ class AuthenticationService {
949946
}
950947

951948
const seedPhrase = uint8ArrayToMnemonic(firstSeedPhrase.data, wordlist);
952-
await this.newWalletAndRestore(password, authData, seedPhrase, false);
949+
950+
await this.newWalletVaultAndRestore(password, seedPhrase, false);
953951
// add in more srps
954952
const keyringMetadataList: KeyringMetadata[] = [];
955953
if (restOfSeedPhrases.length > 0) {
@@ -980,10 +978,14 @@ class AuthenticationService {
980978
this.addMultichainAccounts(keyringMetadataList);
981979

982980
this.dispatchOauthReset();
981+
982+
ReduxService.store.dispatch(setExistingUser(true));
983+
await StorageWrapper.removeItem(SEED_PHRASE_HINTS);
983984
} else {
984985
throw new Error('No account data found');
985986
}
986987
} catch (error) {
988+
this.lockApp({ reset: false });
987989
Logger.error(error as Error);
988990
throw error;
989991
}
@@ -997,7 +999,6 @@ class AuthenticationService {
997999
*/
9981000
syncPasswordAndUnlockWallet = async (
9991001
globalPassword: string,
1000-
authData: AuthData,
10011002
): Promise<void> => {
10021003
const { SeedlessOnboardingController, KeyringController } = Engine.context;
10031004

@@ -1036,7 +1037,7 @@ class AuthenticationService {
10361037

10371038
// rehydrate with social accounts if max keychain length exceeded
10381039
await SeedlessOnboardingController.refreshAuthTokens();
1039-
await this.rehydrateSeedPhrase(globalPassword, authData);
1040+
await this.rehydrateSeedPhrase(globalPassword);
10401041
// skip the rest of the flow ( change password and sync keyring encryption key)
10411042
return;
10421043
} else if (

0 commit comments

Comments
 (0)