fix: middleware to use RestrictedEthMethods instead of RestrictedMethods

MetaMask · Jun 6, 2024 · 621f65d · 621f65d
1 parent 437d527
commit 621f65d
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 56 deletions.
diff --git a/app/scripts/lib/createEvmMethodsToNonEvmAccountReqFilterMiddleware.test.ts b/app/scripts/lib/createEvmMethodsToNonEvmAccountReqFilterMiddleware.test.ts
@@ -1,6 +1,9 @@
 import { jsonrpc2 } from '@metamask/utils';
-import createEvmMethodsToNonEvmAccountReqFilterMiddleware from './createEvmMethodsToNonEvmAccountReqFilterMiddleware';
 import { BtcAccountType, EthAccountType } from '@metamask/keyring-api';
+import createEvmMethodsToNonEvmAccountReqFilterMiddleware, {
+  EvmMethodsToNonEvmAccountFilterMessenger,
+} from './createEvmMethodsToNonEvmAccountReqFilterMiddleware';
+import { Json } from 'json-rpc-engine';
 
 describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
   const getMockRequest = (method: string, params?: any) => ({
@@ -17,35 +20,29 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
     {
       method: 'eth_accounts',
       calledNext: false,
-      calledEnd: true,
     },
     {
       method: 'eth_sendRawTransaction',
       calledNext: false,
-      calledEnd: true,
     },
     {
       method: 'eth_sendTransaction',
       calledNext: false,
-      calledEnd: true,
     },
     { method: 'eth_sign', calledNext: false, calledEnd: true },
     { method: 'eth_signTypedData', calledNext: false, calledEnd: true },
     {
       method: 'eth_signTypedData_v1',
       calledNext: false,
-      calledEnd: true,
     },
     {
       method: 'eth_signTypedData_v3',
       calledNext: false,
-      calledEnd: true,
     },
     {
       method: 'eth_signTypedData_v4',
 
       calledNext: false,
-      calledEnd: true,
     },
 
     // evm requests not associated with an account
@@ -58,55 +55,59 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
     {
       method: 'wallet_requestSnaps',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'snap_getClientStatus',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_addEthereumChain',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_getPermissions',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_requestPermissions',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_revokePermissions',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_switchEthereumChain',
       calledNext: true,
-      calledEnd: false,
+    },
+
+    // wallet_requestPermissions request
+    {
+      method: 'wallet_requestPermissions',
+      params: [{ eth_accounts: {} }],
+      calledNext: false,
+    },
+
+    {
+      method: 'wallet_requestPermissions',
+      params: [{ snap_getClientStatus: {} }],
+      calledNext: true,
     },
   ])(
     `method $method with non-EVM account is passed to next called $calledNext times`,
     ({
       method,
       params,
       calledNext,
-      calledEnd,
     }: {
       method: string;
-      params: any;
+      params?: Json;
       calledNext: number;
-      calledEnd: number;
     }) => {
       const filterFn = createEvmMethodsToNonEvmAccountReqFilterMiddleware({
         messenger: {
           call: jest.fn().mockReturnValue({ type: BtcAccountType.P2wpkh }),
-        },
+        } as unknown as EvmMethodsToNonEvmAccountFilterMessenger,
       });
       const nextMock = jest.fn();
       const endMock = jest.fn();
@@ -119,7 +120,7 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
       );
 
       expect(nextMock).toHaveBeenCalledTimes(calledNext ? 1 : 0);
-      expect(endMock).toHaveBeenCalledTimes(calledEnd ? 1 : 0);
+      expect(endMock).toHaveBeenCalledTimes(calledNext ? 0 : 1);
     },
   );
 
@@ -129,34 +130,28 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
     {
       method: 'eth_accounts',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'eth_sendRawTransaction',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'eth_sendTransaction',
       calledNext: true,
-      calledEnd: false,
     },
     { method: 'eth_sign', calledNext: true, calledEnd: false },
     { method: 'eth_signTypedData', calledNext: true, calledEnd: false },
     {
       method: 'eth_signTypedData_v1',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'eth_signTypedData_v3',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'eth_signTypedData_v4',
       calledNext: true,
-      calledEnd: false,
     },
 
     // evm requests not associated with an account
@@ -169,55 +164,59 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
     {
       method: 'wallet_requestSnaps',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'snap_getClientStatus',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_addEthereumChain',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_getPermissions',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_requestPermissions',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_revokePermissions',
       calledNext: true,
-      calledEnd: false,
     },
     {
       method: 'wallet_switchEthereumChain',
       calledNext: true,
-      calledEnd: false,
+    },
+
+    // wallet_requestPermissions request
+    {
+      method: 'wallet_requestPermissions',
+      params: [{ eth_accounts: {} }],
+      calledNext: true,
+    },
+
+    {
+      method: 'wallet_requestPermissions',
+      params: [{ snap_getClientStatus: {} }],
+      calledNext: true,
     },
   ])(
     `method $method with EVM account is passed to next called $calledNext times`,
     ({
       method,
       params,
       calledNext,
-      calledEnd,
     }: {
       method: string;
-      params: any;
+      params?: Json;
       calledNext: number;
-      calledEnd: number;
     }) => {
       const filterFn = createEvmMethodsToNonEvmAccountReqFilterMiddleware({
         messenger: {
           call: jest.fn().mockReturnValue({ type: EthAccountType.Eoa }),
-        },
+        } as unknown as EvmMethodsToNonEvmAccountFilterMessenger,
       });
       const nextMock = jest.fn();
       const endMock = jest.fn();
@@ -230,7 +229,7 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
       );
 
       expect(nextMock).toHaveBeenCalledTimes(calledNext ? 1 : 0);
-      expect(endMock).toHaveBeenCalledTimes(calledEnd ? 1 : 0);
+      expect(endMock).toHaveBeenCalledTimes(calledNext ? 0 : 1);
     },
   );
 });
diff --git a/app/scripts/lib/createEvmMethodsToNonEvmAccountReqFilterMiddleware.ts b/app/scripts/lib/createEvmMethodsToNonEvmAccountReqFilterMiddleware.ts
@@ -1,18 +1,33 @@
 import { isEvmAccountType } from '@metamask/keyring-api';
+import { RestrictedControllerMessenger } from '@metamask/base-controller';
+import { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller';
 import { JsonRpcMiddleware } from 'json-rpc-engine';
 import log from 'loglevel';
-import { RestrictedMethods } from '../../../shared/constants/permissions';
+import { RestrictedEthMethods } from '../../../shared/constants/permissions';
 import { UnrestrictedEthSigningMethods } from '../controllers/permissions';
 
+type AllowedActions = AccountsControllerGetSelectedAccountAction;
+
+export type EvmMethodsToNonEvmAccountFilterMessenger =
+  RestrictedControllerMessenger<
+    'EvmMethodsToNonEvmAccountFilterMessenger',
+    AllowedActions,
+    never,
+    AllowedActions['type'],
+    never
+  >;
+
 /**
  * Returns a middleware that filters out requests whose requests are restricted to EVM accounts.
  *
+ * @param opt - The middleware options.
+ * @param opt.messenger - The messenger object.
  * @returns The middleware function.
  */
 export default function createEvmMethodsToNonEvmAccountReqFilterMiddleware({
   messenger,
 }: {
-  messenger: any;
+  messenger: EvmMethodsToNonEvmAccountFilterMessenger;
 }): JsonRpcMiddleware<unknown, void> {
   return function filterEvmRequestToNonEvmAccountsMiddleware(
     req,
@@ -30,7 +45,7 @@ export default function createEvmMethodsToNonEvmAccountReqFilterMiddleware({
     }
 
     const ethMethodsRequiringEthAccount = [
-      ...Object.values(RestrictedMethods),
+      ...Object.values(RestrictedEthMethods),
       ...UnrestrictedEthSigningMethods,
     ].includes(req.method);
 
@@ -43,24 +58,26 @@ export default function createEvmMethodsToNonEvmAccountReqFilterMiddleware({
     const isWalletRequestPermission =
       req.method === 'wallet_requestPermissions';
 
-    const permissionMethodRequest = (
-      (req?.params as Array<{ [key: string]: {} }>) || []
-    ).map((request) => Object.keys(request)[0]);
+    if (req?.params && Array.isArray(req.params)) {
+      const permissionMethodRequest = req.params.map(
+        (request) => Object.keys(request)[0],
+      );
 
-    const isEvmPermissionRequest = [...Object.values(RestrictedMethods)].some(
-      (method) => permissionMethodRequest.includes(method),
-    );
+      const isEvmPermissionRequest = [
+        ...Object.values(RestrictedEthMethods),
+      ].some((method) => permissionMethodRequest.includes(method));
 
-    if (isWalletRequestPermission && isEvmPermissionRequest) {
-      log.debug(
-        "Non evm account can't request this method",
-        permissionMethodRequest,
-      );
-      return end(
-        new Error(
-          `Non evm account can't request this method: ${permissionMethodRequest.toString()}`,
-        ),
-      );
+      if (isWalletRequestPermission && isEvmPermissionRequest) {
+        log.debug(
+          "Non evm account can't request this method",
+          permissionMethodRequest,
+        );
+        return end(
+          new Error(
+            `Non evm account can't request this method: ${permissionMethodRequest.toString()}`,
+          ),
+        );
+      }
     }
 
     return next();