Skip to content

Commit 6df8a92

Browse files
authored
fix: biometric bug with incorrect password during rehydration (MetaMask#27900)
<!-- 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** Seedless/OAuth rehydration was prompting biometrics with a wrong password entry too **Solution** Now the flow is password-first: unlockWallet runs with password only. After a successful unlock, post-unlock steps (including optional biometric upgrade). Failed unlocks no longer trigger biometric prompts. Jira: https://consensyssoftware.atlassian.net/browse/TO-600 <!-- 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? --> ## **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: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TO-600 ## **Manual testing steps** ```gherkin Feature: Seedless OAuth rehydration password before biometrics Scenario: Wrong password does not trigger biometrics before unlock Given the user is on the OAuth rehydration screen with a seedless wallet And device biometrics are available When the user enters an incorrect password and submits Then the app shows an invalid password error And the system biometric prompt is not shown before unlock fails Scenario: Correct password offers biometrics after successful unlock (rehydration) Given the user is on the OAuth rehydration screen And device biometrics are available When the user enters the correct password and submits Then unlock completes successfully And the app may prompt for device biometrics / keychain upgrade only after unlock succeeds ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** https://github.com/user-attachments/assets/e5ade971-0f7e-4316-b272-9f2aa7c58fb8 <!-- [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). - [ ] 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] > **Medium Risk** > Changes the seedless/OAuth rehydration login flow and post-unlock auth preference updates, which can affect authentication UX and keychain storage behavior. Scope is limited to `OAuthRehydration` and adds/adjusts tests to cover ordering and failure cases. > > **Overview** > Updates `OAuthRehydration` so biometric/device-auth prompts no longer occur *before* password verification: `unlockWallet` is called with `currentAuthType: PASSWORD`, and an optional post-unlock step (`upgradeKeychainAuthAfterSuccessfulUnlock`) requests device auth and persists the result via `updateAuthPreference`. > > Extends `OAuthRehydration` tests to assert call ordering (unlock precedes biometrics) and to ensure biometrics/auth-preference updates are not triggered when password unlock fails, including the outdated-password flow. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f0c3963. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 6fa43af commit 6df8a92

2 files changed

Lines changed: 119 additions & 18 deletions

File tree

app/components/Views/OAuthRehydration/index.test.tsx

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const mockReauthenticate = jest.fn();
3636
const mockRevealSRP = jest.fn();
3737
const mockRevealPrivateKey = jest.fn();
3838
const mockRequestBiometricsAccessControlForIOS = jest.fn();
39+
const mockUpdateAuthPreference = jest.fn();
3940

4041
jest.mock('../../../core/Authentication/hooks/useAuthentication', () => ({
4142
__esModule: true,
@@ -49,6 +50,7 @@ jest.mock('../../../core/Authentication/hooks/useAuthentication', () => ({
4950
revealPrivateKey: mockRevealPrivateKey,
5051
requestBiometricsAccessControlForIOS:
5152
mockRequestBiometricsAccessControlForIOS,
53+
updateAuthPreference: mockUpdateAuthPreference,
5254
}),
5355
}));
5456

@@ -191,6 +193,7 @@ describe('OAuthRehydration', () => {
191193
mockRequestBiometricsAccessControlForIOS.mockResolvedValue(
192194
AUTHENTICATION_TYPE.PASSWORD,
193195
);
196+
mockUpdateAuthPreference.mockResolvedValue(undefined);
194197
mockUseNetInfo.mockReturnValue({
195198
isConnected: true,
196199
isInternetReachable: true,
@@ -220,6 +223,9 @@ describe('OAuthRehydration', () => {
220223
await waitFor(() => {
221224
expect(mockReplace).toHaveBeenCalledWith(Routes.ONBOARDING.HOME_NAV);
222225
});
226+
expect(mockUnlockWallet.mock.invocationCallOrder[0]).toBeLessThan(
227+
mockRequestBiometricsAccessControlForIOS.mock.invocationCallOrder[0],
228+
);
223229
expect(mockRequestBiometricsAccessControlForIOS).toHaveBeenCalledWith(
224230
AUTHENTICATION_TYPE.DEVICE_AUTHENTICATION,
225231
);
@@ -233,6 +239,47 @@ describe('OAuthRehydration', () => {
233239
expect(mockTrackOnboarding).toHaveBeenCalled();
234240
});
235241
});
242+
243+
it('logs error when post-unlock biometric prompt fails', async () => {
244+
const biometricError = new Error('Biometric prompt failed');
245+
mockRequestBiometricsAccessControlForIOS.mockRejectedValueOnce(
246+
biometricError,
247+
);
248+
249+
const { getByTestId } = renderWithProvider(<OAuthRehydration />);
250+
await enterPasswordAndSubmit(getByTestId);
251+
252+
await waitFor(() => {
253+
expect(mockUnlockWallet).toHaveBeenCalled();
254+
});
255+
await waitFor(() => {
256+
expect(Logger.error).toHaveBeenCalledWith(
257+
biometricError,
258+
'OAuthRehydration: post-unlock biometric preference',
259+
);
260+
});
261+
});
262+
263+
it('logs error when updateAuthPreference fails after choosing device auth', async () => {
264+
mockRequestBiometricsAccessControlForIOS.mockResolvedValueOnce(
265+
AUTHENTICATION_TYPE.DEVICE_AUTHENTICATION,
266+
);
267+
const preferenceError = new Error('Keychain preference update failed');
268+
mockUpdateAuthPreference.mockRejectedValueOnce(preferenceError);
269+
270+
const { getByTestId } = renderWithProvider(<OAuthRehydration />);
271+
await enterPasswordAndSubmit(getByTestId);
272+
273+
await waitFor(() => {
274+
expect(mockUpdateAuthPreference).toHaveBeenCalled();
275+
});
276+
await waitFor(() => {
277+
expect(Logger.error).toHaveBeenCalledWith(
278+
preferenceError,
279+
'OAuthRehydration: post-unlock biometric preference',
280+
);
281+
});
282+
});
236283
});
237284

238285
describe('Password validation', () => {
@@ -246,6 +293,18 @@ describe('OAuthRehydration', () => {
246293
});
247294
});
248295

296+
it('does not prompt biometrics when password unlock fails', async () => {
297+
mockUnlockWallet.mockRejectedValue(new Error('Error: Decrypt failed'));
298+
const { getByTestId } = renderWithProvider(<OAuthRehydration />);
299+
await enterPasswordAndSubmit(getByTestId, 'wrongPassword');
300+
301+
await waitFor(() => {
302+
expect(getByTestId(LoginViewSelectors.PASSWORD_ERROR)).toBeTruthy();
303+
});
304+
expect(mockRequestBiometricsAccessControlForIOS).not.toHaveBeenCalled();
305+
expect(mockUpdateAuthPreference).not.toHaveBeenCalled();
306+
});
307+
249308
it('clears error when user types new password', async () => {
250309
mockUnlockWallet.mockRejectedValue(new Error('Error: Decrypt failed'));
251310
const { getByTestId } = renderWithProvider(<OAuthRehydration />);
@@ -858,10 +917,28 @@ describe('OAuthRehydration', () => {
858917
expect.objectContaining({
859918
authPreference: expect.objectContaining({
860919
oauth2Login: false,
920+
currentAuthType: AUTHENTICATION_TYPE.PASSWORD,
861921
}),
862922
}),
863923
);
864924
});
925+
expect(mockUnlockWallet.mock.invocationCallOrder[0]).toBeLessThan(
926+
mockRequestBiometricsAccessControlForIOS.mock.invocationCallOrder[0],
927+
);
928+
expect(mockRequestBiometricsAccessControlForIOS).toHaveBeenCalledWith(
929+
AUTHENTICATION_TYPE.DEVICE_AUTHENTICATION,
930+
);
931+
});
932+
933+
it('does not prompt biometrics before unlock when password is wrong (outdated password flow)', async () => {
934+
mockUnlockWallet.mockRejectedValue(new Error('Error: Decrypt failed'));
935+
const { getByTestId } = renderWithProvider(<OAuthRehydration />);
936+
await enterPasswordAndSubmit(getByTestId, 'wrongPassword');
937+
938+
await waitFor(() => {
939+
expect(getByTestId(LoginViewSelectors.PASSWORD_ERROR)).toBeTruthy();
940+
});
941+
expect(mockRequestBiometricsAccessControlForIOS).not.toHaveBeenCalled();
865942
});
866943

867944
it('navigates to DELETE_WALLET modal on forgot password press', () => {

app/components/Views/OAuthRehydration/index.tsx

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,38 @@ const OAuthRehydration: React.FC<OAuthRehydrationProps> = ({
150150

151151
const passwordLoginAttemptTraceCtxRef = useRef<TraceContext | null>(null);
152152

153-
const { unlockWallet, getAuthType, requestBiometricsAccessControlForIOS } =
154-
useAuthentication();
153+
const {
154+
unlockWallet,
155+
getAuthType,
156+
requestBiometricsAccessControlForIOS,
157+
updateAuthPreference,
158+
} = useAuthentication();
159+
160+
/**
161+
* After a successful password unlock, offer device auth / biometrics for keychain storage.
162+
*/
163+
const upgradeKeychainAuthAfterSuccessfulUnlock = useCallback(async () => {
164+
try {
165+
const upgradeAuthType = await requestBiometricsAccessControlForIOS(
166+
AUTHENTICATION_TYPE.DEVICE_AUTHENTICATION,
167+
);
168+
if (upgradeAuthType !== AUTHENTICATION_TYPE.PASSWORD) {
169+
await updateAuthPreference({
170+
authType: upgradeAuthType,
171+
password,
172+
fallbackToPassword: true,
173+
});
174+
}
175+
} catch (postUnlockAuthErr) {
176+
Logger.error(
177+
ensureError(
178+
postUnlockAuthErr,
179+
'Post-unlock auth preference update failed',
180+
),
181+
'OAuthRehydration: post-unlock biometric preference',
182+
);
183+
}
184+
}, [password, requestBiometricsAccessControlForIOS, updateAuthPreference]);
155185

156186
const track = useCallback(
157187
(
@@ -485,14 +515,9 @@ const OAuthRehydration: React.FC<OAuthRehydrationProps> = ({
485515

486516
setLoading(true);
487517

488-
// Ask user to allow biometrics access control
489-
const authType = await requestBiometricsAccessControlForIOS(
490-
AUTHENTICATION_TYPE.DEVICE_AUTHENTICATION,
491-
);
492-
493-
// Only set oauth2Login for normal rehydration, not when password is outdated
518+
// Password first: do not prompt biometrics until unlock succeeds
494519
const authData: AuthData = {
495-
currentAuthType: authType,
520+
currentAuthType: AUTHENTICATION_TYPE.PASSWORD,
496521
oauth2Login: true,
497522
};
498523

@@ -506,6 +531,8 @@ const OAuthRehydration: React.FC<OAuthRehydrationProps> = ({
506531
},
507532
);
508533

534+
await upgradeKeychainAuthAfterSuccessfulUnlock();
535+
509536
// Best-effort post-unlock UX: show biometric cancelled alert if needed.
510537
// Failure here must not be treated as a login error — unlock already succeeded.
511538
try {
@@ -542,7 +569,7 @@ const OAuthRehydration: React.FC<OAuthRehydrationProps> = ({
542569
track,
543570
promptBiometricFailedAlert,
544571
unlockWallet,
545-
requestBiometricsAccessControlForIOS,
572+
upgradeKeychainAuthAfterSuccessfulUnlock,
546573
]);
547574

548575
const newGlobalPasswordLogin = useCallback(async () => {
@@ -551,14 +578,9 @@ const OAuthRehydration: React.FC<OAuthRehydrationProps> = ({
551578

552579
setLoading(true);
553580

554-
// Ask user to allow biometrics access control
555-
const authType = await requestBiometricsAccessControlForIOS(
556-
AUTHENTICATION_TYPE.DEVICE_AUTHENTICATION,
557-
);
558-
559-
// Only set oauth2Login for normal rehydration, not when password is outdated
581+
// biometrics/passcode preference is applied only after sync succeeds
560582
const authData: AuthData = {
561-
currentAuthType: authType,
583+
currentAuthType: AUTHENTICATION_TYPE.PASSWORD,
562584
oauth2Login: false,
563585
};
564586

@@ -572,6 +594,8 @@ const OAuthRehydration: React.FC<OAuthRehydrationProps> = ({
572594
},
573595
);
574596

597+
await upgradeKeychainAuthAfterSuccessfulUnlock();
598+
575599
// Best-effort post-unlock UX: show biometric cancelled alert if needed.
576600
// Failure here must not be treated as a login error — unlock already succeeded.
577601
try {
@@ -593,7 +617,7 @@ const OAuthRehydration: React.FC<OAuthRehydrationProps> = ({
593617
handleLoginError,
594618
promptBiometricFailedAlert,
595619
unlockWallet,
596-
requestBiometricsAccessControlForIOS,
620+
upgradeKeychainAuthAfterSuccessfulUnlock,
597621
]);
598622

599623
// Cleanup for isMountedRef tracking

0 commit comments

Comments
 (0)