fix: fix ConnectPage when a non-EVM account is selected (#28436)

## **Description**

The currently selected non-EVM account was part of the requested account
on during the account connection flow. The `ConnectPage` was allowing to
"confirm" the request even if the UI was showing 0 account.

To fix this, we no longer uses the currently selected account if this
account is not EVM-compatible.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28436?quickstart=1)

## **Related issues**

Fixes: MetaMask/accounts-planning#670

## **Manual testing steps**

1. `yarn start:flask`
2. Enable Bitcoin support
3. Create a Bitcoin account
4. Select this account
5. Go to: https://metamask.github.io/test-dapp/
6. Try to connect your accounts
    - None accounts should be selected by default
    - You should not be able to "confirm" the dialog

## **Screenshots/Recordings**

### **Before**

![Screenshot 2024-11-13 at 14 57
11](https://github.com/user-attachments/assets/0b1b3de2-e968-408a-9aea-c9ea4006a1b1)

If you try to connect, you will get:

![Screenshot 2024-11-13 at 14 57
44](https://github.com/user-attachments/assets/a0c8e507-7530-4de7-a73f-5414ba1a2656)

### **After**

![Screenshot 2024-11-13 at 15 01
39](https://github.com/user-attachments/assets/170f4ec3-01a9-422e-a32c-66ad81d5568e)


## **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/develop/.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/develop/.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.

Author: ccharly

test/jest/mocks.ts

@@ -4,6 +4,7 @@ import {
   BtcMethod,
   BtcAccountType,
   InternalAccount,
+  isEvmAccountType,
 } from '@metamask/keyring-api';
 import { KeyringTypes } from '@metamask/keyring-controller';
 import { v4 as uuidv4 } from 'uuid';
@@ -14,6 +15,9 @@ import {
   initialState,
 } from '../../ui/ducks/send';
 import { MetaMaskReduxState } from '../../ui/store/store';
+import mockState from '../data/mock-state.json';
+
+export type MockState = typeof mockState;
 
 export const MOCK_DEFAULT_ADDRESS =
   '0xd5e099c71b797516c10ed0f0d895f429c2781111';
@@ -245,3 +249,63 @@ export const getSelectedInternalAccountFromMockState = (
     state.metamask.internalAccounts.selectedAccount
   ];
 };
+
+export function overrideAccountsFromMockState<
+  MockMetaMaskState extends MockState['metamask'],
+>(
+  state: { metamask: MockMetaMaskState },
+  accounts: InternalAccount[],
+  selectedAccountId?: string,
+): { metamask: MockMetaMaskState } {
+  // First, re-create the accounts mapping and the currently selected account.
+  const [{ id: newFirstAccountId }] = accounts;
+  const newSelectedAccount = selectedAccountId ?? newFirstAccountId ?? '';
+  const newInternalAccounts = accounts.reduce(
+    (
+      acc: MetaMaskReduxState['metamask']['internalAccounts']['accounts'],
+      account,
+    ) => {
+      acc[account.id] = account;
+      return acc;
+    },
+    {},
+  );
+
+  // Re-create the keyring mapping too, since some selectors are using their internal
+  // account list.
+  const newKeyrings: MetaMaskReduxState['metamask']['keyrings'] = [];
+  for (const keyring of state.metamask.keyrings) {
+    const newAccountsForKeyring = [];
+    for (const account of accounts) {
+      if (account.metadata.keyring.type === keyring.type) {
+        newAccountsForKeyring.push(account.address);
+      }
+    }
+    newKeyrings.push({
+      type: keyring.type,
+      accounts: newAccountsForKeyring,
+    });
+  }
+
+  // Compute balances for EVM addresses:
+  // FIXME: Looks like there's no `balances` type in `MetaMaskReduxState`.
+  const newBalances: Record<string, string> = {};
+  for (const account of accounts) {
+    if (isEvmAccountType(account.type)) {
+      newBalances[account.address] = '0x0';
+    }
+  }
+
+  return {
+    ...state,
+    metamask: {
+      ...state.metamask,
+      internalAccounts: {
+        accounts: newInternalAccounts,
+        selectedAccount: newSelectedAccount,
+      },
+      keyrings: newKeyrings,
+      balances: newBalances,
+    },
+  };
+}

ui/pages/permissions-connect/connect-page/connect-page.test.tsx

@@ -7,34 +7,42 @@ import {
   EndowmentTypes,
   RestrictedMethods,
 } from '../../../../shared/constants/permissions';
-import { ConnectPage, ConnectPageRequest } from './connect-page';
+import { overrideAccountsFromMockState } from '../../../../test/jest/mocks';
+import {
+  MOCK_ACCOUNT_BIP122_P2WPKH,
+  MOCK_ACCOUNT_EOA,
+} from '../../../../test/data/mock-accounts';
+import { ConnectPage, ConnectPageProps } from './connect-page';
+
+const mockTestDappUrl = 'https://test.dapp';
 
 const render = (
-  props: {
-    request: ConnectPageRequest;
-    permissionsRequestId: string;
-    rejectPermissionsRequest: (id: string) => void;
-    approveConnection: (request: ConnectPageRequest) => void;
-    activeTabOrigin: string;
-  } = {
-    request: {
-      id: '1',
-      origin: 'https://test.dapp',
-    },
-    permissionsRequestId: '1',
-    rejectPermissionsRequest: jest.fn(),
-    approveConnection: jest.fn(),
-    activeTabOrigin: 'https://test.dapp',
-  },
-  state = {},
+  options: {
+    props?: ConnectPageProps;
+    state?: object;
+  } = {},
 ) => {
+  const {
+    props = {
+      request: {
+        id: '1',
+        origin: mockTestDappUrl,
+      },
+      permissionsRequestId: '1',
+      rejectPermissionsRequest: jest.fn(),
+      approveConnection: jest.fn(),
+      activeTabOrigin: mockTestDappUrl,
+    },
+    state,
+  } = options;
+
   const store = configureStore({
     ...mockState,
     metamask: {
       ...mockState.metamask,
       ...state,
       permissionHistory: {
-        'https://test.dapp': {
+        mockTestDappUrl: {
           eth_accounts: {
             accounts: {
               '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc': 1709225290848,
@@ -44,7 +52,7 @@ const render = (
       },
     },
     activeTab: {
-      origin: 'https://test.dapp',
+      origin: mockTestDappUrl,
     },
   });
   return renderWithProvider(<ConnectPage {...props} />, store);
@@ -82,33 +90,56 @@ describe('ConnectPage', () => {
 
   it('should render with defaults from the requested permissions', () => {
     const { container } = render({
-      request: {
-        id: '1',
-        origin: 'https://test.dapp',
-        permissions: {
-          [RestrictedMethods.eth_accounts]: {
-            caveats: [
-              {
-                type: CaveatTypes.restrictReturnedAccounts,
-                value: ['0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'],
-              },
-            ],
-          },
-          [EndowmentTypes.permittedChains]: {
-            caveats: [
-              {
-                type: CaveatTypes.restrictNetworkSwitching,
-                value: ['0x1'],
-              },
-            ],
+      props: {
+        request: {
+          id: '1',
+          origin: mockTestDappUrl,
+          permissions: {
+            [RestrictedMethods.eth_accounts]: {
+              caveats: [
+                {
+                  type: CaveatTypes.restrictReturnedAccounts,
+                  value: ['0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'],
+                },
+              ],
+            },
+            [EndowmentTypes.permittedChains]: {
+              caveats: [
+                {
+                  type: CaveatTypes.restrictNetworkSwitching,
+                  value: ['0x1'],
+                },
+              ],
+            },
           },
         },
+        permissionsRequestId: '1',
+        rejectPermissionsRequest: jest.fn(),
+        approveConnection: jest.fn(),
+        activeTabOrigin: mockTestDappUrl,
       },
-      permissionsRequestId: '1',
-      rejectPermissionsRequest: jest.fn(),
-      approveConnection: jest.fn(),
-      activeTabOrigin: 'https://test.dapp',
     });
     expect(container).toMatchSnapshot();
   });
+
+  it('should render a disabled confirm if current account is a non-EVM account', () => {
+    // NOTE: We select the non-EVM account by default here!
+    const mockSelectedAccountId = MOCK_ACCOUNT_BIP122_P2WPKH.id;
+    const mockAccounts = [MOCK_ACCOUNT_EOA, MOCK_ACCOUNT_BIP122_P2WPKH];
+    const mockAccountsState = overrideAccountsFromMockState(
+      mockState,
+      mockAccounts,
+      mockSelectedAccountId,
+    );
+
+    const { getByText } = render({
+      state: mockAccountsState.metamask,
+    });
+    const confirmButton = getByText('Connect');
+    const cancelButton = getByText('Cancel');
+    // The currently selected account is a Bitcoin account, the "connecting account list" would be
+    // empty by default and thus, we cannot confirm without explictly select an EVM account.
+    expect(confirmButton).toBeDisabled();
+    expect(cancelButton).toBeDefined();
+  });
 });

ui/pages/permissions-connect/connect-page/connect-page.tsx

@@ -50,7 +50,7 @@ export type ConnectPageRequest = {
   >;
 };
 
-type ConnectPageProps = {
+export type ConnectPageProps = {
   request: ConnectPageRequest;
   permissionsRequestId: string;
   rejectPermissionsRequest: (id: string) => void;
@@ -124,10 +124,11 @@ export const ConnectPage: React.FC<ConnectPageProps> = ({
   }, [accounts, internalAccounts]);
 
   const currentAccount = useSelector(getSelectedInternalAccount);
+  const currentAccountAddress = isEvmAccountType(currentAccount.type)
+    ? [currentAccount.address]
+    : []; // We do not support non-EVM accounts connections
   const defaultAccountsAddresses =
-    requestedAccounts.length > 0
-      ? requestedAccounts
-      : [currentAccount?.address];
+    requestedAccounts.length > 0 ? requestedAccounts : currentAccountAddress;
   const [selectedAccountAddresses, setSelectedAccountAddresses] = useState(
     defaultAccountsAddresses,
   );

ui/pages/permissions-connect/permissions-connect.component.js

@@ -41,10 +41,12 @@ function getDefaultSelectedAccounts(currentAddress, permissionsRequest) {
     return new Set(
       requestedAccounts
         .map((address) => address.toLowerCase())
+        // We only consider EVM accounts here (used for `eth_requestAccounts` or `eth_accounts`)
         .filter(isEthAddress),
     );
   }
 
+  // We only consider EVM accounts here (used for `eth_requestAccounts` or `eth_accounts`)
   return new Set(isEthAddress(currentAddress) ? [currentAddress] : []);
 }