Skip to content

Commit 1b329d2

Browse files
tommasinitommasini
authored
chore: Abstract turn off remember me function into auth hook (MetaMask#23917)
<!-- 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** This PR centralizes authentication-related operations by introducing a new `useAuthentication` hook in the `app/core/Authentication/` directory. The hook encapsulates the logic for turning off the "Remember Me" feature and locking the app, which was previously scattered directly in the `TurnOffRememberMeModal` component. **Reason for the change:** - Authentication logic was directly embedded in UI components, making it difficult to reuse and test - Redux dispatch calls and Authentication service calls were mixed with component logic - No centralized place for authentication-related operations **Improvement/Solution:** - Created `useAuthentication` hook that combines Redux action dispatching with Authentication service calls - Moved `turnOffRememberMeAndLockApp` logic from `TurnOffRememberMeModal` to the new hook - Updated `TurnOffRememberMeModal` to use the centralized hook instead of direct dispatch and service calls - Added comprehensive unit test coverage (12 tests) following project testing guidelines - Hook is exported from `app/core/Authentication/index.ts` for easy consumption This change improves code organization, testability, and reusability of authentication features across the application. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Turn off Remember Me and lock app Scenario: user turns off Remember Me feature Given the user has "Remember Me" enabled and is logged into the app And the user navigates to the Turn Off Remember Me modal When user enters their password correctly And user taps the "Turn Off Remember Me" button Then the "Remember Me" feature is disabled And the app is locked And the user is redirected to the login screen ``` ## **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 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 - [ ] 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] > Modernizes auth flows and centralizes logic while tightening tests. > > - API change: `Authentication.updateAuthPreference` now accepts `{ authType, password }`; all callers updated (`TurnOffRememberMeModal`, `LoginOptionsSettings`, `RememberMeOptionSection`, related tests) > - New `useAuthentication` hook (exported via `core/Authentication/index.ts`) with unit tests; provides `lockApp` > - `TurnOffRememberMeModal`: simplifies disable flow, restores previous auth with entered password, clears stored state, updates Redux, and removes direct `lockApp` calls; improves loading handling with `act` > - Authentication internals: `componentAuthenticationType` prioritizes `REMEMBER_ME`, uses `PASSCODE_DISABLED` to infer BIOMETRIC/PASSCODE, and `storePassword` now dispatches `passwordSet`; removed lock-time side effects; `lockApp` gains `allowRememberMe` control and ordering tests > - Expanded tests across settings sections to cover password-required flows, callbacks, mutual exclusivity, and error handling > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d511fa8. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 4c40232 commit 1b329d2

11 files changed

Lines changed: 649 additions & 246 deletions

File tree

‎app/components/UI/TurnOffRememberMeModal/TurnOffRememberMeModal.test.tsx‎

Lines changed: 95 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,42 @@ jest.mock('../../../component-library/components/Texts/Text', () => {
2929
});
3030

3131
import React from 'react';
32-
import { fireEvent, waitFor } from '@testing-library/react-native';
32+
import { fireEvent, waitFor, act } from '@testing-library/react-native';
3333
import renderWithProvider from '../../../util/test/renderWithProvider';
3434
import TurnOffRememberMeModal from './TurnOffRememberMeModal';
3535
import AUTHENTICATION_TYPE from '../../../constants/userProperties';
3636
import { PREVIOUS_AUTH_TYPE_BEFORE_REMEMBER_ME } from '../../../constants/storage';
3737

38-
// Mock Authentication
39-
jest.mock('../../../core', () => ({
38+
// Mock ReduxService
39+
import ReduxService from '../../../core/redux/ReduxService';
40+
import type { ReduxStore } from '../../../core/redux/types';
41+
42+
// Mock Authentication source file (exports: export const Authentication = ...)
43+
jest.mock('../../../core/Authentication/Authentication', () => ({
4044
Authentication: {
4145
updateAuthPreference: jest.fn(),
42-
lockApp: jest.fn(),
4346
},
4447
}));
4548

49+
// Mock Authentication index (exports default Authentication)
50+
jest.mock('../../../core/Authentication', () => {
51+
const authSource = jest.requireMock(
52+
'../../../core/Authentication/Authentication',
53+
);
54+
return {
55+
__esModule: true,
56+
default: authSource.Authentication,
57+
};
58+
});
59+
60+
// Mock Authentication from core index (used by component directly)
61+
jest.mock('../../../core', () => {
62+
const authModule = jest.requireMock('../../../core/Authentication');
63+
return {
64+
Authentication: authModule.default,
65+
};
66+
});
67+
4668
// Mock StorageWrapper
4769
jest.mock('../../../store/storage-wrapper', () => ({
4870
__esModule: true,
@@ -204,34 +226,65 @@ jest.mock('../WarningExistingUserModal', () => {
204226

205227
describe('TurnOffRememberMeModal', () => {
206228
let mockDoesPasswordMatch: jest.Mock;
207-
let mockUpdateAuthPreference: jest.Mock;
208-
let mockLockApp: jest.Mock;
209229
let mockGetItem: jest.Mock;
210230
let mockRemoveItem: jest.Mock;
231+
let mockUpdateAuthPreference: jest.Mock;
232+
233+
const createMockReduxStore = (): ReduxStore =>
234+
({
235+
dispatch: jest.fn(),
236+
getState: jest.fn(() => ({
237+
user: {
238+
existingUser: false,
239+
},
240+
security: {
241+
allowLoginWithRememberMe: true,
242+
},
243+
settings: {
244+
lockTime: -1,
245+
},
246+
engine: {
247+
backgroundState: {
248+
SeedlessOnboardingController: {
249+
vault: null, // Not in seedless flow
250+
},
251+
},
252+
},
253+
})),
254+
subscribe: jest.fn(),
255+
replaceReducer: jest.fn(),
256+
[Symbol.observable]: jest.fn(),
257+
}) as unknown as ReduxStore;
211258

212259
beforeEach(() => {
213260
jest.clearAllMocks();
214261

262+
// Mock ReduxService store
263+
const mockStore = createMockReduxStore();
264+
jest.spyOn(ReduxService, 'store', 'get').mockReturnValue(mockStore);
265+
215266
// Get the mocked functions from the modules
216267
const passwordModule = jest.requireMock('../../../util/password');
217268
mockDoesPasswordMatch = passwordModule.doesPasswordMatch as jest.Mock;
218269

219-
const coreModule = jest.requireMock('../../../core');
220-
mockUpdateAuthPreference = coreModule.Authentication
221-
.updateAuthPreference as jest.Mock;
222-
mockLockApp = coreModule.Authentication.lockApp as jest.Mock;
223-
224270
const storageModule = jest.requireMock('../../../store/storage-wrapper');
225271
mockGetItem = storageModule.default.getItem as jest.Mock;
226272
mockRemoveItem = storageModule.default.removeItem as jest.Mock;
227273

228-
// Clear and reset mockDismissModal
274+
// Get Authentication mocks from the mocked module
275+
const authModule = jest.requireMock(
276+
'../../../core/Authentication/Authentication',
277+
);
278+
mockUpdateAuthPreference = authModule.Authentication
279+
.updateAuthPreference as jest.Mock;
280+
281+
// Clear and reset mocks
229282
mockDismissModal.mockClear();
283+
mockUpdateAuthPreference.mockClear();
230284

231285
// Set default mock implementations
232286
mockDoesPasswordMatch.mockResolvedValue({ valid: false });
233287
mockUpdateAuthPreference.mockResolvedValue(undefined);
234-
mockLockApp.mockResolvedValue(undefined);
235288
mockGetItem.mockResolvedValue(null);
236289
mockRemoveItem.mockResolvedValue(undefined);
237290
});
@@ -308,20 +361,21 @@ describe('TurnOffRememberMeModal', () => {
308361

309362
const button = getByTestId('warning-modal-cancel-button');
310363

311-
fireEvent.press(button);
364+
await act(async () => {
365+
fireEvent.press(button);
366+
});
312367

313368
await waitFor(() => {
314369
expect(mockGetItem).toHaveBeenCalledWith(
315370
PREVIOUS_AUTH_TYPE_BEFORE_REMEMBER_ME,
316371
);
317-
expect(mockUpdateAuthPreference).toHaveBeenCalledWith(
318-
AUTHENTICATION_TYPE.BIOMETRIC,
319-
'ValidPassword123!',
320-
);
372+
expect(mockUpdateAuthPreference).toHaveBeenCalledWith({
373+
authType: AUTHENTICATION_TYPE.BIOMETRIC,
374+
password: 'ValidPassword123!',
375+
});
321376
expect(mockRemoveItem).toHaveBeenCalledWith(
322377
PREVIOUS_AUTH_TYPE_BEFORE_REMEMBER_ME,
323378
);
324-
expect(mockLockApp).toHaveBeenCalled();
325379
expect(mockDismissModal).toHaveBeenCalled();
326380
});
327381
});
@@ -343,13 +397,15 @@ describe('TurnOffRememberMeModal', () => {
343397

344398
const button = getByTestId('warning-modal-cancel-button');
345399

346-
fireEvent.press(button);
400+
await act(async () => {
401+
fireEvent.press(button);
402+
});
347403

348404
await waitFor(() => {
349-
expect(mockUpdateAuthPreference).toHaveBeenCalledWith(
350-
AUTHENTICATION_TYPE.PASSWORD,
351-
'ValidPassword123!',
352-
);
405+
expect(mockUpdateAuthPreference).toHaveBeenCalledWith({
406+
authType: AUTHENTICATION_TYPE.PASSWORD,
407+
password: 'ValidPassword123!',
408+
});
353409
});
354410
});
355411

@@ -374,7 +430,9 @@ describe('TurnOffRememberMeModal', () => {
374430
});
375431

376432
const button = getByTestId('warning-modal-cancel-button');
377-
fireEvent.press(button);
433+
await act(async () => {
434+
fireEvent.press(button);
435+
});
378436

379437
await waitFor(() => {
380438
expect(mockUpdateAuthPreference).toHaveBeenCalled();
@@ -387,9 +445,6 @@ describe('TurnOffRememberMeModal', () => {
387445

388446
if (resolveUpdateAuthPreference) {
389447
resolveUpdateAuthPreference();
390-
await waitFor(() => {
391-
expect(mockLockApp).toHaveBeenCalled();
392-
});
393448
}
394449
});
395450

@@ -414,7 +469,9 @@ describe('TurnOffRememberMeModal', () => {
414469
});
415470

416471
const button = getByTestId('warning-modal-cancel-button');
417-
fireEvent.press(button);
472+
await act(async () => {
473+
fireEvent.press(button);
474+
});
418475

419476
await waitFor(() => {
420477
expect(mockUpdateAuthPreference).toHaveBeenCalled();
@@ -427,9 +484,6 @@ describe('TurnOffRememberMeModal', () => {
427484

428485
if (resolveUpdateAuthPreference) {
429486
resolveUpdateAuthPreference();
430-
await waitFor(() => {
431-
expect(mockLockApp).toHaveBeenCalled();
432-
});
433487
}
434488
});
435489

@@ -450,11 +504,12 @@ describe('TurnOffRememberMeModal', () => {
450504
});
451505

452506
const button = getByTestId('warning-modal-cancel-button');
453-
fireEvent.press(button);
507+
await act(async () => {
508+
fireEvent.press(button);
509+
});
454510

455511
await waitFor(() => {
456512
expect(mockUpdateAuthPreference).toHaveBeenCalled();
457-
expect(mockLockApp).toHaveBeenCalled();
458513
expect(mockDismissModal).toHaveBeenCalled();
459514
});
460515
});
@@ -480,7 +535,9 @@ describe('TurnOffRememberMeModal', () => {
480535

481536
const button = getByTestId('warning-modal-cancel-button');
482537

483-
fireEvent.press(button);
538+
await act(async () => {
539+
fireEvent.press(button);
540+
});
484541

485542
await waitFor(() => {
486543
expect(mockUpdateAuthPreference).toHaveBeenCalled();
@@ -512,11 +569,12 @@ describe('TurnOffRememberMeModal', () => {
512569
});
513570

514571
const button = getByTestId('warning-modal-cancel-button');
515-
fireEvent.press(button);
572+
await act(async () => {
573+
fireEvent.press(button);
574+
});
516575

517576
await waitFor(() => {
518577
expect(mockUpdateAuthPreference).toHaveBeenCalled();
519-
expect(mockLockApp).toHaveBeenCalled();
520578
expect(mockDismissModal).toHaveBeenCalled();
521579
});
522580
});

‎app/components/UI/TurnOffRememberMeModal/TurnOffRememberMeModal.tsx‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,16 +92,15 @@ const TurnOffRememberMeModal = () => {
9292
: AUTHENTICATION_TYPE.PASSWORD;
9393

9494
// Use the password entered in the modal to restore auth method
95-
await Authentication.updateAuthPreference(
96-
authTypeToRestore,
97-
passwordText,
98-
);
95+
await Authentication.updateAuthPreference({
96+
authType: authTypeToRestore,
97+
password: passwordText,
98+
});
9999
// Clear the stored previous auth type after successful restoration
100100
await StorageWrapper.removeItem(PREVIOUS_AUTH_TYPE_BEFORE_REMEMBER_ME);
101101
// Only set Redux state after operation completes successfully
102102
dispatch(setAllowLoginWithRememberMe(false));
103-
Authentication.lockApp();
104-
// Dismiss modal after successful operation
103+
105104
dismissModal();
106105
} catch (error) {
107106
// If update fails, still disable remember me and lock app
@@ -111,7 +110,7 @@ const TurnOffRememberMeModal = () => {
111110
error as Error,
112111
'Failed to restore auth preference when disabling remember me',
113112
);
114-
Authentication.lockApp();
113+
115114
// Dismiss modal even on error
116115
dismissModal();
117116
} finally {

‎app/components/Views/Settings/SecuritySettings/Sections/LoginOptionsSettings.test.tsx‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,9 @@ describe('LoginOptionsSettings', () => {
202202
fireEvent(toggle, 'onValueChange', true);
203203

204204
await waitFor(() => {
205-
expect(mockUpdateAuthPreference).toHaveBeenCalledWith(
206-
AUTHENTICATION_TYPE.BIOMETRIC,
207-
);
205+
expect(mockUpdateAuthPreference).toHaveBeenCalledWith({
206+
authType: AUTHENTICATION_TYPE.BIOMETRIC,
207+
});
208208
});
209209
});
210210

@@ -234,9 +234,9 @@ describe('LoginOptionsSettings', () => {
234234
fireEvent(toggle, 'onValueChange', false);
235235

236236
await waitFor(() => {
237-
expect(mockUpdateAuthPreference).toHaveBeenCalledWith(
238-
AUTHENTICATION_TYPE.PASSWORD,
239-
);
237+
expect(mockUpdateAuthPreference).toHaveBeenCalledWith({
238+
authType: AUTHENTICATION_TYPE.PASSWORD,
239+
});
240240
});
241241
});
242242

@@ -304,10 +304,10 @@ describe('LoginOptionsSettings', () => {
304304
await passwordCallback('test-password');
305305

306306
await waitFor(() => {
307-
expect(mockUpdateAuthPreference).toHaveBeenCalledWith(
308-
AUTHENTICATION_TYPE.BIOMETRIC,
309-
'test-password',
310-
);
307+
expect(mockUpdateAuthPreference).toHaveBeenCalledWith({
308+
authType: AUTHENTICATION_TYPE.BIOMETRIC,
309+
password: 'test-password',
310+
});
311311
});
312312
}
313313
});
@@ -510,10 +510,10 @@ describe('LoginOptionsSettings', () => {
510510
await passwordCallback('test-password');
511511

512512
await waitFor(() => {
513-
expect(mockUpdateAuthPreference).toHaveBeenCalledWith(
514-
AUTHENTICATION_TYPE.BIOMETRIC,
515-
'test-password',
516-
);
513+
expect(mockUpdateAuthPreference).toHaveBeenCalledWith({
514+
authType: AUTHENTICATION_TYPE.BIOMETRIC,
515+
password: 'test-password',
516+
});
517517
});
518518
}
519519
});
@@ -607,10 +607,10 @@ describe('LoginOptionsSettings', () => {
607607
await passwordCallback('test-password');
608608

609609
await waitFor(() => {
610-
expect(mockUpdateAuthPreference).toHaveBeenCalledWith(
611-
AUTHENTICATION_TYPE.PASSCODE,
612-
'test-password',
613-
);
610+
expect(mockUpdateAuthPreference).toHaveBeenCalledWith({
611+
authType: AUTHENTICATION_TYPE.PASSCODE,
612+
password: 'test-password',
613+
});
614614
});
615615
}
616616
});
@@ -659,10 +659,10 @@ describe('LoginOptionsSettings', () => {
659659
await passwordCallback('test-password');
660660

661661
await waitFor(() => {
662-
expect(mockUpdateAuthPreference).toHaveBeenCalledWith(
663-
AUTHENTICATION_TYPE.PASSCODE,
664-
'test-password',
665-
);
662+
expect(mockUpdateAuthPreference).toHaveBeenCalledWith({
663+
authType: AUTHENTICATION_TYPE.PASSCODE,
664+
password: 'test-password',
665+
});
666666
});
667667
}
668668
});

0 commit comments

Comments
 (0)