From 67b73a5352ec4dd1a19e13526c96c3228be2b20d Mon Sep 17 00:00:00 2001 From: Owen Craston Date: Wed, 23 Oct 2024 15:17:15 -0700 Subject: [PATCH 1/4] feat: Remove Account Snap Warning (Flask) (#11451) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** 1. What is the reason for the change? - This change adds an extra warning to users when trying to remove an account snap (https://metamask.github.io/snap-simple-keyring/latest/ for example) AND the user has snap accounts in their wallet - Snap accounts are not managed/owned by metamask and therefore cannot be recovered in metamask. User's must manage and backup these accounts in the snaps UI. 2. What is the improvement/solution? - Adds an extra layer of friction when trying to remove an account snap and the user has snap accounts in their wallet - This is a `FLASK ONLY` feature ## **Related issues** Fixes: https://github.com/MetaMask/accounts-planning/issues/155 ## **Manual testing steps** 1. check this branch 2. open `.js.env` 3. ensure that `export METAMASK_BUILD_TYPE=flask` is set to flask (not main) 4. build the app on this branch 5. create/import a wallet 6. inside the mobile browser, open this URL: https://metamask.github.io/snap-simple-keyring/latest/ 7. connect this snap 8. click create account, you can do this as many times as you want. 9. navigate back to the main wallet view 10. NOTICE there should be new accounts in your wallet with the name "Snap Account x" 11. Go to settings 12. navigate to snaps 13. locate the Snap simple keyring settings 14. scroll to the bottom of the page and click the red "Remove" button 15. Notice: new warning should pop up indicating that the user should backup these accounts before removing the snap 16. click continue 17. you should be prompted to type the name of the snap in the text input. 18. YOU SHOULD NOT BE ABLE TO REMOVE THE SNAP WITHOUT TYPING The CORRECT SNAP NAME 19. Once you type the correct snap name, you should be able to remove the snap 20. After clicking remove the snap should be removed. #### Testing a non account snap 1. check this branch 2. open `.js.env` 3. ensure that `export METAMASK_BUILD_TYPE=flask` is set to flask (not main) 4. build the app on this branch 5. create/import a wallet 6. navigate to settings/snaps 7. click on the pre installed Message signing snap 8. click on the remove button at the bottom 9. `THERE SHOULD NOT BE A WARNING` #### Testing removing a snap account without snap accounts in the wallet 1. check this branch 2. open `.js.env` 3. ensure that `export METAMASK_BUILD_TYPE=flask` is set to flask (not main) 4. build the app on this branch 5. create/import a wallet 6. inside the mobile browser, open this URL: https://metamask.github.io/snap-simple-keyring/latest/ 7. connect this snap. Do not create any accounts this time. 8. Go to settings 9. navigate to snaps 10. locate the Snap simple keyring settings 11. scroll to the bottom of the page and click the red "Remove" button 12. There should be no warning pop up and the snap should be removed ## **Screenshots/Recordings** ### Extension Version Screenshot 2024-10-01 at 5 38 58 PM Screenshot 2024-10-01 at 5 39 14 PM Screenshot 2024-10-01 at 5 39 20 PM ### **After** with snap accounts: https://github.com/user-attachments/assets/24aa2993-98fa-4d72-bc1f-b7f9c1aeb6ad Screenshot 2024-10-10 at 3 26 46 PM Without snap accounts: https://github.com/user-attachments/assets/7cf42c03-dc6e-40fb-b490-fb0eb4d8ba83 ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] 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. --- .../Views/AccountActions/AccountActions.tsx | 62 ++- .../KeyringSnapRemovalWarning.constants.ts | 9 + .../KeyringSnapRemovalWarning.styles.ts | 43 ++ .../KeyringSnapRemovalWarning.tsx | 226 ++++++++++ .../Snaps/KeyringSnapRemovalWarning/index.ts | 3 + .../test/KeyringSnapRemovalWarning.test.tsx | 337 ++++++++++++++ .../Views/Snaps/SnapSettings/SnapSettings.tsx | 168 +++++-- .../SnapSettings/test/SnapSettings.test.tsx | 419 ++++++++++++++---- .../KeyringAccountListItem.constants.ts | 4 + .../KeyringAccountListItem.styles.ts | 36 ++ .../KeyringAccountListItem.tsx | 74 ++++ .../KeyringAccountListItem/index.ts | 3 + .../test/KeyringAccountListItem.test.tsx | 42 ++ app/core/Engine.ts | 59 +-- .../SnapKeyring/utils/getAccountsBySnapId.ts | 17 + app/util/address/index.test.ts | 12 + app/util/address/index.ts | 11 + app/util/test/accountsControllerTestUtils.ts | 31 ++ .../Modals/AccountActionsModal.selectors.js | 5 +- locales/languages/en.json | 20 +- 20 files changed, 1421 insertions(+), 160 deletions(-) create mode 100644 app/components/Views/Snaps/KeyringSnapRemovalWarning/KeyringSnapRemovalWarning.constants.ts create mode 100644 app/components/Views/Snaps/KeyringSnapRemovalWarning/KeyringSnapRemovalWarning.styles.ts create mode 100644 app/components/Views/Snaps/KeyringSnapRemovalWarning/KeyringSnapRemovalWarning.tsx create mode 100644 app/components/Views/Snaps/KeyringSnapRemovalWarning/index.ts create mode 100644 app/components/Views/Snaps/KeyringSnapRemovalWarning/test/KeyringSnapRemovalWarning.test.tsx create mode 100644 app/components/Views/Snaps/components/KeyringAccountListItem/KeyringAccountListItem.constants.ts create mode 100644 app/components/Views/Snaps/components/KeyringAccountListItem/KeyringAccountListItem.styles.ts create mode 100644 app/components/Views/Snaps/components/KeyringAccountListItem/KeyringAccountListItem.tsx create mode 100644 app/components/Views/Snaps/components/KeyringAccountListItem/index.ts create mode 100644 app/components/Views/Snaps/components/KeyringAccountListItem/test/KeyringAccountListItem.test.tsx create mode 100644 app/core/SnapKeyring/utils/getAccountsBySnapId.ts diff --git a/app/components/Views/AccountActions/AccountActions.tsx b/app/components/Views/AccountActions/AccountActions.tsx index 22feb6a46b9..99547d6cfda 100644 --- a/app/components/Views/AccountActions/AccountActions.tsx +++ b/app/components/Views/AccountActions/AccountActions.tsx @@ -34,7 +34,12 @@ import { protectWalletModalVisible } from '../../../actions/user'; import Routes from '../../../constants/navigation/Routes'; import { AccountActionsModalSelectorsIDs } from '../../../../e2e/selectors/Modals/AccountActionsModal.selectors'; import { useMetrics } from '../../../components/hooks/useMetrics'; -import { isHardwareAccount } from '../../../util/address'; +import { + isHardwareAccount, + ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) + isSnapAccount, + ///: END:ONLY_INCLUDE_IF +} from '../../../util/address'; import { removeAccountsFromPermissions } from '../../../core/Permissions'; import ExtendedKeyringTypes, { HardwareDeviceTypes, @@ -189,6 +194,49 @@ const AccountActions = () => { } }, [controllers.KeyringController]); + ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) + + /** + * Remove the snap account from the keyring + */ + const removeSnapAccount = useCallback(async () => { + if (selectedAddress) { + await controllers.KeyringController.removeAccount(selectedAddress as Hex); + await removeAccountsFromPermissions([selectedAddress]); + trackEvent(MetaMetricsEvents.ACCOUNT_REMOVED, { + accountType: keyring?.type, + selectedAddress, + }); + } + }, [ + controllers.KeyringController, + keyring?.type, + selectedAddress, + trackEvent, + ]); + + const showRemoveSnapAccountAlert = useCallback(() => { + Alert.alert( + strings('accounts.remove_snap_account'), + strings('accounts.remove_snap_account_alert_description'), + [ + { + text: strings('accounts.remove_account_alert_cancel_btn'), + style: 'cancel', + }, + { + text: strings('accounts.remove_account_alert_remove_btn'), + onPress: async () => { + sheetRef.current?.onCloseBottomSheet(async () => { + await removeSnapAccount(); + }); + }, + }, + ], + ); + }, [removeSnapAccount]); + ///: END:ONLY_INCLUDE_IF + /** * Forget the device if there are no more accounts in the keyring * @param keyringType - The keyring type @@ -306,6 +354,18 @@ const AccountActions = () => { testID={AccountActionsModalSelectorsIDs.REMOVE_HARDWARE_ACCOUNT} /> )} + { + ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) + selectedAddress && isSnapAccount(selectedAddress) && ( + + ) + ///: END:ONLY_INCLUDE_IF + } { + const { theme } = params; + const { colors } = theme; + + return StyleSheet.create({ + bottomSheet: { + flex: 1, + }, + container: { + paddingHorizontal: 16, + }, + description: { + paddingVertical: 8, + }, + buttonContainer: { + paddingTop: 16, + }, + input: { + borderWidth: 1, + borderColor: colors.border.default, + borderRadius: 4, + padding: 10, + marginVertical: 10, + }, + errorText: { + color: colors.error.default, + }, + placeholderText: { + color: colors.text.muted, + }, + scrollView: { + flexGrow: 1, + maxHeight: 300, + }, + }); +}; + +export default styleSheet; +///: END:ONLY_INCLUDE_IF diff --git a/app/components/Views/Snaps/KeyringSnapRemovalWarning/KeyringSnapRemovalWarning.tsx b/app/components/Views/Snaps/KeyringSnapRemovalWarning/KeyringSnapRemovalWarning.tsx new file mode 100644 index 00000000000..b2ec560b5dd --- /dev/null +++ b/app/components/Views/Snaps/KeyringSnapRemovalWarning/KeyringSnapRemovalWarning.tsx @@ -0,0 +1,226 @@ +///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) +import React, { + useEffect, + useRef, + useState, + useCallback, + useMemo, +} from 'react'; +import { View, TextInput, ScrollView } from 'react-native'; +import { NativeViewGestureHandler } from 'react-native-gesture-handler'; +import { Snap } from '@metamask/snaps-utils'; +import BottomSheet, { + BottomSheetRef, +} from '../../../../component-library/components/BottomSheets/BottomSheet'; +import Text, { + TextVariant, +} from '../../../../component-library/components/Texts/Text'; +import { InternalAccount } from '@metamask/keyring-api'; +import BannerAlert from '../../../../component-library/components/Banners/Banner/variants/BannerAlert'; +import { BannerAlertSeverity } from '../../../../component-library/components/Banners/Banner'; +import BottomSheetHeader from '../../../../component-library/components/BottomSheets/BottomSheetHeader'; +import { useStyles } from '../../../hooks/useStyles'; +import stylesheet from './KeyringSnapRemovalWarning.styles'; +import { strings } from '../../../../../locales/i18n'; +import { KeyringAccountListItem } from '../components/KeyringAccountListItem'; +import { getAccountLink } from '@metamask/etherscan-link'; +import { useSelector } from 'react-redux'; +import { selectProviderConfig } from '../../../../selectors/networkController'; +import BottomSheetFooter, { + ButtonsAlignment, +} from '../../../../component-library/components/BottomSheets/BottomSheetFooter'; +import { + ButtonProps, + ButtonSize, + ButtonVariants, +} from '../../../../component-library/components/Buttons/Button/Button.types'; +import { + KEYRING_SNAP_REMOVAL_WARNING, + KEYRING_SNAP_REMOVAL_WARNING_CANCEL, + KEYRING_SNAP_REMOVAL_WARNING_CONTINUE, + KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT, +} from './KeyringSnapRemovalWarning.constants'; +import Logger from '../../../../util/Logger'; + +interface KeyringSnapRemovalWarningProps { + snap: Snap; + keyringAccounts: InternalAccount[]; + onCancel: () => void; + onClose: () => void; + onSubmit: () => void; +} + +export default function KeyringSnapRemovalWarning({ + snap, + keyringAccounts, + onCancel, + onClose, + onSubmit, +}: KeyringSnapRemovalWarningProps) { + const [showConfirmation, setShowConfirmation] = useState(false); + const [confirmedRemoval, setConfirmedRemoval] = useState(false); + const [confirmationInput, setConfirmationInput] = useState(''); + const [error, setError] = useState(false); + const { chainId } = useSelector(selectProviderConfig); + const { styles } = useStyles(stylesheet, {}); + const bottomSheetRef = useRef(null); + + useEffect(() => { + setShowConfirmation(keyringAccounts.length === 0); + }, [keyringAccounts]); + + const validateConfirmationInput = useCallback( + (input: string): boolean => input === snap.manifest.proposedName, + [snap.manifest.proposedName], + ); + + const handleConfirmationInputChange = useCallback( + (text: string) => { + setConfirmationInput(text); + setConfirmedRemoval(validateConfirmationInput(text)); + }, + [validateConfirmationInput], + ); + + const handleContinuePress = useCallback(() => { + if (!showConfirmation) { + setShowConfirmation(true); + } else if (confirmedRemoval) { + try { + onSubmit(); + } catch (e) { + Logger.error( + e as Error, + 'KeyringSnapRemovalWarning: error while removing snap', + ); + setError(true); + } + } + }, [showConfirmation, confirmedRemoval, onSubmit]); + + const cancelButtonProps: ButtonProps = useMemo( + () => ({ + variant: ButtonVariants.Secondary, + label: strings( + 'app_settings.snaps.snap_settings.remove_account_snap_warning.cancel_button', + ), + size: ButtonSize.Lg, + onPress: onCancel, + testID: KEYRING_SNAP_REMOVAL_WARNING_CANCEL, + }), + [onCancel], + ); + + const continueButtonProps: ButtonProps = useMemo( + () => ({ + variant: ButtonVariants.Primary, + label: showConfirmation + ? strings( + 'app_settings.snaps.snap_settings.remove_account_snap_warning.remove_snap_button', + ) + : strings( + 'app_settings.snaps.snap_settings.remove_account_snap_warning.continue_button', + ), + size: ButtonSize.Lg, + onPress: handleContinuePress, + isDisabled: showConfirmation && !confirmedRemoval, + isDanger: showConfirmation, + testID: KEYRING_SNAP_REMOVAL_WARNING_CONTINUE, + }), + [showConfirmation, confirmedRemoval, handleContinuePress], + ); + + const buttonPropsArray = useMemo( + () => [cancelButtonProps, continueButtonProps], + [cancelButtonProps, continueButtonProps], + ); + + const accountListItems = useMemo( + () => + keyringAccounts.map((account, index) => ( + + )), + [keyringAccounts, chainId], + ); + + return ( + + + + + {strings( + 'app_settings.snaps.snap_settings.remove_account_snap_warning.title', + )} + + + + {showConfirmation ? ( + <> + + {`${strings( + 'app_settings.snaps.snap_settings.remove_account_snap_warning.remove_account_snap_alert_description_1', + )} `} + + {snap.manifest.proposedName} + + {` ${strings( + 'app_settings.snaps.snap_settings.remove_account_snap_warning.remove_account_snap_alert_description_2', + )}`} + + + {error && ( + + {strings( + 'app_settings.snaps.snap_settings.remove_account_snap_warning.remove_snap_error', + { + snapName: snap.manifest.proposedName, + }, + )} + + )} + + ) : ( + <> + + {strings( + 'app_settings.snaps.snap_settings.remove_account_snap_warning.description', + )} + + + + {accountListItems} + + + + )} + + + + ); +} +///: END:ONLY_INCLUDE_IF diff --git a/app/components/Views/Snaps/KeyringSnapRemovalWarning/index.ts b/app/components/Views/Snaps/KeyringSnapRemovalWarning/index.ts new file mode 100644 index 00000000000..94d58b3d412 --- /dev/null +++ b/app/components/Views/Snaps/KeyringSnapRemovalWarning/index.ts @@ -0,0 +1,3 @@ +///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) +export { default as KeyringSnapRemovalWarning } from './KeyringSnapRemovalWarning'; +///: END:ONLY_INCLUDE_IF diff --git a/app/components/Views/Snaps/KeyringSnapRemovalWarning/test/KeyringSnapRemovalWarning.test.tsx b/app/components/Views/Snaps/KeyringSnapRemovalWarning/test/KeyringSnapRemovalWarning.test.tsx new file mode 100644 index 00000000000..6359b5a7b3d --- /dev/null +++ b/app/components/Views/Snaps/KeyringSnapRemovalWarning/test/KeyringSnapRemovalWarning.test.tsx @@ -0,0 +1,337 @@ +import React from 'react'; +import { fireEvent, waitFor } from '@testing-library/react-native'; +import KeyringSnapRemovalWarning from '../KeyringSnapRemovalWarning'; +import { + KEYRING_SNAP_REMOVAL_WARNING, + KEYRING_SNAP_REMOVAL_WARNING_CANCEL, + KEYRING_SNAP_REMOVAL_WARNING_CONTINUE, + KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT, +} from '../KeyringSnapRemovalWarning.constants'; +import { useSelector } from 'react-redux'; +import renderWithProvider from '../../../../../util/test/renderWithProvider'; +import { Snap, SnapStatus } from '@metamask/snaps-utils'; +import { SnapId } from '@metamask/snaps-sdk'; +import { SemVerVersion } from '@metamask/utils'; +import { createMockSnapInternalAccount } from '../../../../../util/test/accountsControllerTestUtils'; +import { KEYRING_ACCOUNT_LIST_ITEM } from '../../components/KeyringAccountListItem/KeyringAccountListItem.constants'; + +jest.mock('react-redux', () => ({ + ...jest.requireActual('react-redux'), + useSelector: jest.fn(), +})); + +jest.mock('react-native-safe-area-context', () => { + // using disting digits for mock rects to make sure they are not mixed up + const inset = { top: 1, right: 2, bottom: 3, left: 4 }; + const frame = { width: 5, height: 6, x: 7, y: 8 }; + return { + SafeAreaProvider: jest.fn().mockImplementation(({ children }) => children), + SafeAreaConsumer: jest + .fn() + .mockImplementation(({ children }) => children(inset)), + useSafeAreaInsets: jest.fn().mockImplementation(() => inset), + useSafeAreaFrame: jest.fn().mockImplementation(() => frame), + }; +}); + +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + useNavigation: () => ({ + navigate: jest.fn(), + }), + }; +}); + +describe('KeyringSnapRemovalWarning', () => { + const mockSnapName = 'MetaMask Simple Snap Keyring'; + const mockSnap: Snap = { + blocked: false, + enabled: true, + id: 'npm:@metamask/snap-simple-keyring-snap' as SnapId, + initialPermissions: { + 'endowment:keyring': { + allowedOrigins: ['https://metamask.github.io', 'metamask.github.io'], + }, + 'endowment:rpc': { + dapps: true, + }, + snap_manageAccounts: {}, + snap_manageState: {}, + }, + manifest: { + version: '1.1.6' as SemVerVersion, + description: 'An example of a key management snap for a simple keyring.', + proposedName: mockSnapName, + repository: { + type: 'git', + url: 'git+https://github.com/MetaMask/snap-simple-keyring.git', + }, + source: { + shasum: 'P2BbaJn6jb7+ecBF6mJJnheQ4j8dtEZ8O4FLqLv8e8M=', + location: { + npm: { + filePath: 'dist/bundle.js', + iconPath: 'images/icon.svg', + packageName: '@metamask/snap-simple-keyring-snap', + registry: 'https://registry.npmjs.org/', + }, + }, + }, + initialPermissions: { + 'endowment:keyring': { + allowedOrigins: ['https://metamask.github.io', 'metamask.github.io'], + }, + 'endowment:rpc': { + dapps: true, + }, + snap_manageAccounts: {}, + snap_manageState: {}, + }, + manifestVersion: '0.1', + }, + status: 'stopped' as SnapStatus, + sourceCode: '', + version: '1.1.6' as SemVerVersion, + versionHistory: [ + { + version: '1.1.6', + date: 1727403640652, + origin: 'https://metamask.github.io', + }, + ], + auxiliaryFiles: [], + localizationFiles: [], + }; + + const MOCK_ADDRESS_1 = '0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272'; + const MOCK_ADDRESS_2 = '0xA7E9922b0e7DB390c3B108127739eFebe4d6293E'; + + const mockKeyringAccount1 = createMockSnapInternalAccount( + MOCK_ADDRESS_1, + 'Snap Account 1', + ); + const mockKeyringAccount2 = createMockSnapInternalAccount( + MOCK_ADDRESS_2, + 'Snap Account 2', + ); + + const mockKeyringAccounts = [mockKeyringAccount1, mockKeyringAccount2]; + const onCancelMock = jest.fn(); + const onCloseMock = jest.fn(); + const onSubmitMock = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + (useSelector as jest.Mock).mockReturnValue({ chainId: '1' }); + }); + + it('renders correctly with initial props', () => { + const { getByTestId, queryByText } = renderWithProvider( + , + ); + + const continueButton = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_CONTINUE); + expect(continueButton).toBeTruthy(); + expect(continueButton.props.children[1].props.children).toBe('Continue'); + + const cancelButton = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_CANCEL); + expect(cancelButton).toBeTruthy(); + expect(cancelButton.props.children[1].props.children).toBe('Cancel'); + + const warningBannerTitle = queryByText( + 'Be sure you can access any accounts created by this Snap on your own before removing it', + ); + expect(warningBannerTitle).toBeTruthy(); + }); + + it('renders the correct number of keyring account list items', () => { + const { getAllByTestId } = renderWithProvider( + , + ); + + const accountListItems = getAllByTestId(KEYRING_ACCOUNT_LIST_ITEM); + expect(accountListItems).toHaveLength(mockKeyringAccounts.length); + }); + it('shows confirmation input when keyringAccounts is empty', () => { + const { getByTestId } = renderWithProvider( + , + ); + const continueButton = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_CONTINUE); + expect(continueButton).toBeTruthy(); + expect(continueButton.props.disabled).toBe(true); + expect(continueButton.props.children[1].props.children).toBe('Remove Snap'); + + const textInput = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT); + expect(textInput).toBeTruthy(); + }); + + it('enables continue button when correct snap name is entered', async () => { + const { getByTestId } = renderWithProvider( + , + ); + + const continueButton = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_CONTINUE); + expect(continueButton.props.disabled).toBe(true); + + const textInput = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT); + expect(textInput).toBeTruthy(); + fireEvent.changeText(textInput, mockSnapName); + + await waitFor(() => { + expect(continueButton.props.disabled).toBe(false); + }); + }); + + it('does not enable continue button when incorrect snap name is entered', async () => { + const { getByTestId } = renderWithProvider( + , + ); + + const continueButton = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_CONTINUE); + expect(continueButton.props.disabled).toBe(true); + + const textInput = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT); + expect(textInput).toBeTruthy(); + fireEvent.changeText(textInput, 'Wrong snap name'); + + await waitFor(() => { + expect(continueButton.props.disabled).toBe(true); + }); + }); + + it('calls onSubmit when confirmed and continue is pressed', async () => { + const { getByTestId } = renderWithProvider( + , + ); + + const textInput = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT); + expect(textInput).toBeTruthy(); + fireEvent.changeText(textInput, mockSnapName); + + const continueButton = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_CONTINUE); + fireEvent.press(continueButton); + expect(onSubmitMock).toHaveBeenCalled(); + }); + + it('displays error when onSubmit throws', async () => { + onSubmitMock.mockImplementation(() => { + throw new Error('Error'); + }); + + const { getByTestId, getByText } = renderWithProvider( + , + ); + + const textInput = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT); + fireEvent.changeText(textInput, mockSnapName); + + const continueButton = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_CONTINUE); + fireEvent.press(continueButton); + + await waitFor(() => { + expect( + getByText( + `Failed to remove ${mockSnapName}`, + ), + ).toBeTruthy(); + }); + }); + + it('calls onCancel when cancel button is pressed', () => { + const { getByTestId } = renderWithProvider( + , + ); + + const cancelButton = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_CANCEL); + fireEvent.press(cancelButton); + + expect(onCancelMock).toHaveBeenCalled(); + }); + + it('calls onClose when BottomSheet is closed', () => { + const { getByTestId } = renderWithProvider( + , + ); + + const bottomSheet = getByTestId(KEYRING_SNAP_REMOVAL_WARNING); + fireEvent(bottomSheet, 'onClose'); + + expect(onCloseMock).toHaveBeenCalled(); + }); + it('allows removal of snaps with empty names and keeps the continue button enabled', () => { + const { getByTestId } = renderWithProvider( + , + ); + const textInput = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT); + fireEvent.changeText(textInput, ''); + const continueButton = getByTestId(KEYRING_SNAP_REMOVAL_WARNING_CONTINUE); + expect(continueButton.props.disabled).toBe(false); + expect(textInput.props.value).toBe(''); + expect(continueButton.props.children[1].props.children).toBe('Remove Snap'); + }); +}); diff --git a/app/components/Views/Snaps/SnapSettings/SnapSettings.tsx b/app/components/Views/Snaps/SnapSettings/SnapSettings.tsx index 19f739400eb..d56c1d0b200 100644 --- a/app/components/Views/Snaps/SnapSettings/SnapSettings.tsx +++ b/app/components/Views/Snaps/SnapSettings/SnapSettings.tsx @@ -1,5 +1,5 @@ -///: BEGIN:ONLY_INCLUDE_IF(external-snaps) -import React, { useCallback, useEffect } from 'react'; +///: BEGIN:ONLY_INCLUDE_IF(external-snaps,keyring-snaps) +import React, { useCallback, useEffect, useState } from 'react'; import { View, ScrollView, SafeAreaView } from 'react-native'; import Engine from '../../../../core/Engine'; @@ -28,7 +28,11 @@ import { useStyles } from '../../../hooks/useStyles'; import { useSelector } from 'react-redux'; import SNAP_SETTINGS_REMOVE_BUTTON from './SnapSettings.constants'; import { selectPermissionControllerState } from '../../../../selectors/snaps/permissionController'; - +import KeyringSnapRemovalWarning from '../KeyringSnapRemovalWarning/KeyringSnapRemovalWarning'; +import { getAccountsBySnapId } from '../../../../core/SnapKeyring/utils/getAccountsBySnapId'; +import { selectInternalAccounts } from '../../../../selectors/accountsController'; +import { InternalAccount } from '@metamask/keyring-api'; +import Logger from '../../../../util/Logger'; interface SnapSettingsProps { snap: Snap; } @@ -42,9 +46,16 @@ const SnapSettings = () => { const navigation = useNavigation(); const { snap } = useParams(); - const permissionsState = useSelector(selectPermissionControllerState); + const [ + isShowingSnapKeyringRemoveWarning, + setIsShowingSnapKeyringRemoveWarning, + ] = useState(false); + + const [keyringAccounts, setKeyringAccounts] = useState([]); + const internalAccounts = useSelector(selectInternalAccounts); + // TODO: Replace "any" with type // eslint-disable-next-line @typescript-eslint/no-explicit-any function getPermissionSubjects(state: any) { @@ -70,53 +81,118 @@ const SnapSettings = () => { ); }, [colors, navigation, snap.manifest.proposedName]); + const isKeyringSnap = Boolean(permissionsFromController?.snap_manageAccounts); + + useEffect(() => { + if (isKeyringSnap) { + (async () => { + const addresses = await getAccountsBySnapId(snap.id); + const snapIdentities = Object.values(internalAccounts).filter( + (internalAccount) => + addresses.includes(internalAccount.address.toLowerCase()), + ); + setKeyringAccounts(snapIdentities); + })(); + } + }, [snap?.id, internalAccounts, isKeyringSnap]); + + const handleKeyringSnapRemovalWarningClose = useCallback(() => { + setIsShowingSnapKeyringRemoveWarning(false); + }, []); + + const removeSnap = useCallback(async () => { - // TODO: Replace "any" with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const { SnapController } = Engine.context as any; + const { SnapController } = Engine.context; await SnapController.removeSnap(snap.id); + + if (isKeyringSnap && keyringAccounts.length > 0) { + try { + for (const keyringAccount of keyringAccounts) { + await Engine.removeAccount(keyringAccount.address); + } + } catch(error) { + Logger.error(error as Error, 'SnapSettings: failed to remove snap accounts when calling Engine.removeAccount'); + } + } navigation.goBack(); - }, [navigation, snap.id]); + }, [isKeyringSnap, keyringAccounts, navigation, snap.id]); + + const handleRemoveSnap = useCallback(() => { + if (isKeyringSnap && keyringAccounts.length > 0) { + setIsShowingSnapKeyringRemoveWarning(true); + } else { + removeSnap(); + } + }, [isKeyringSnap, keyringAccounts.length, removeSnap]); + + + const handleRemoveSnapKeyring = useCallback(() => { + try { + setIsShowingSnapKeyringRemoveWarning(true); + removeSnap(); + setIsShowingSnapKeyringRemoveWarning(false); + } catch { + setIsShowingSnapKeyringRemoveWarning(false); + } finally { + setIsShowingSnapKeyringRemoveWarning(false); + } + }, [removeSnap]); + + const shouldRenderRemoveSnapAccountWarning = + isShowingSnapKeyringRemoveWarning && + isKeyringSnap && + keyringAccounts.length > 0; return ( - - - - - - - - - - - - {strings( - 'app_settings.snaps.snap_settings.remove_snap_section_title', - )} - - - {strings( - 'app_settings.snaps.snap_settings.remove_snap_section_description', - )} - -