From 621f65d24b1a79739a5b4156db05347bb577b6a3 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 6 Jun 2024 09:42:56 +0800 Subject: [PATCH] fix: middleware to use RestrictedEthMethods instead of RestrictedMethods --- ...ToNonEvmAccountReqFilterMiddleware.test.ts | 73 +++++++++---------- ...thodsToNonEvmAccountReqFilterMiddleware.ts | 55 +++++++++----- 2 files changed, 72 insertions(+), 56 deletions(-) diff --git a/app/scripts/lib/createEvmMethodsToNonEvmAccountReqFilterMiddleware.test.ts b/app/scripts/lib/createEvmMethodsToNonEvmAccountReqFilterMiddleware.test.ts index 7f93542325fc..0907e6d8b00c 100644 --- 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,37 +55,43 @@ 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`, @@ -96,17 +99,15 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => { 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,37 +164,43 @@ 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`, @@ -207,17 +208,15 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => { 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 index d9ee1e49b979..55525ce992f1 100644 --- 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 { 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();