From 7ac1f69ebb2a87926ca13e9651c4f18501095507 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 7 Feb 2025 19:46:20 +0100 Subject: [PATCH 1/7] fix: bring back exception if invalid address passed in eth_accounts --- .../controllers/permissions/specifications.js | 69 + app/scripts/metamask-controller.js | 14 + app/scripts/metamask-controller.test.js | 1183 +++++++++-------- 3 files changed, 721 insertions(+), 545 deletions(-) diff --git a/app/scripts/controllers/permissions/specifications.js b/app/scripts/controllers/permissions/specifications.js index f4a1e3320172..3e99ea678fcc 100644 --- a/app/scripts/controllers/permissions/specifications.js +++ b/app/scripts/controllers/permissions/specifications.js @@ -192,3 +192,72 @@ export const unrestrictedMethods = Object.freeze([ 'metamaskinstitutional_openAddHardwareWallet', ///: END:ONLY_INCLUDE_IF ]); + +/** + * Validates the accounts associated with a caveat. In essence, ensures that + * the accounts value is an array of non-empty strings, and that each string + * corresponds to a PreferencesController identity. + * + * @param {string[]} accounts - The accounts associated with the caveat. + * @param {() => Record} getInternalAccounts - + * Gets all AccountsController InternalAccounts. + * TODO: Remove this function once permission refactor/factory differ work is merged into main + */ +export function validateCaveatAccounts(accounts, getInternalAccounts) { + if (!Array.isArray(accounts) || accounts.length === 0) { + throw new Error( + `${PermissionNames.eth_accounts} error: Expected non-empty array of Ethereum addresses.`, + ); + } + + const internalAccounts = getInternalAccounts(); + accounts.forEach((address) => { + if (!address || typeof address !== 'string') { + throw new Error( + `${PermissionNames.eth_accounts} error: Expected an array of Ethereum addresses. Received: "${address}".`, + ); + } + + if ( + !internalAccounts.some( + (internalAccount) => + internalAccount.address.toLowerCase() === address.toLowerCase(), + ) + ) { + throw new Error( + `${PermissionNames.eth_accounts} error: Received unrecognized address: "${address}".`, + ); + } + }); +} + +/** + * Validates the networks associated with a caveat. Ensures that + * the networks value is an array of valid chain IDs. + * + * @param {string[]} chainIdsForCaveat - The list of chain IDs to validate. + * @param {function(string): string} findNetworkClientIdByChainId - Function to find network client ID by chain ID. + * @throws {Error} If the chainIdsForCaveat is not a non-empty array of valid chain IDs. + * TODO: Remove this function once permission refactor/factory differ work is merged into main + */ +export function validateCaveatNetworks( + chainIdsForCaveat, + findNetworkClientIdByChainId, +) { + if (!Array.isArray(chainIdsForCaveat) || chainIdsForCaveat.length === 0) { + throw new Error( + `${PermissionNames.permittedChains} error: Expected non-empty array of chainIds.`, + ); + } + + chainIdsForCaveat.forEach((chainId) => { + try { + findNetworkClientIdByChainId(chainId); + } catch (e) { + console.error(e); + throw new Error( + `${PermissionNames.permittedChains} error: Received unrecognized chainId: "${chainId}". Please try adding the network first via wallet_addEthereumChain.`, + ); + } + }); +} diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a9d0a579a062..48f781bfbff9 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -316,6 +316,8 @@ import { NOTIFICATION_NAMES, unrestrictedMethods, PermissionNames, + validateCaveatAccounts, + validateCaveatNetworks, } from './controllers/permissions'; import { MetaMetricsDataDeletionController } from './controllers/metametrics-data-deletion/metametrics-data-deletion'; import { DataDeletionService } from './services/data-deletion-service'; @@ -5292,6 +5294,18 @@ export default class MetamaskController extends EventEmitter { (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, )?.value ?? []; + validateCaveatAccounts( + requestedAccounts, + this.accountsController.listAccounts.bind(this.accountsController), + ); + console.log('i passed here'); + validateCaveatNetworks( + requestedChains, + this.networkController.findNetworkClientIdByChainId.bind( + this.networkController, + ), + ); + const newCaveatValue = { requiredScopes: {}, optionalScopes: { diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 2f6cba27dd5a..09fa048a7106 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -62,6 +62,7 @@ import MetaMaskController, { ONE_KEY_VIA_TREZOR_MINOR_VERSION, } from './metamask-controller'; import { PermissionNames } from './controllers/permissions'; +import * as PermissionSpecifications from './controllers/permissions/specifications'; const { Ganache } = require('../../test/e2e/seeder/ganache'); @@ -115,6 +116,13 @@ const createLoggerMiddlewareMock = () => (req, res, next) => { } next(); }; + +jest.mock('./controllers/permissions/specifications', () => ({ + ...jest.requireActual('./controllers/permissions/specifications'), + validateCaveatAccounts: jest.fn(), + validateCaveatNetworks: jest.fn(), +})); + jest.mock('./lib/createLoggerMiddleware', () => createLoggerMiddlewareMock); const rpcMethodMiddlewareMock = { @@ -960,289 +968,472 @@ describe('MetaMaskController', () => { }); describe('#requestCaip25Approval', () => { - it('requests approval with well formed id and origin', async () => { - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: {}, - }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', - }, - }); + describe('valid requests', () => { + it('requests approval with well formed id and origin', async () => { + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ + permissions: {}, + }); + jest + .spyOn(metamaskController.permissionController, 'grantPermissions') + .mockReturnValue({ + [Caip25EndowmentPermissionName]: { + foo: 'bar', + }, + }); - await metamaskController.requestCaip25Approval('test.com', {}); + await metamaskController.requestCaip25Approval('test.com', {}); - expect( - metamaskController.approvalController.addAndShowApprovalRequest, - ).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.stringMatching(/.{21}/u), - origin: 'test.com', - requestData: expect.objectContaining({ - metadata: { - id: expect.stringMatching(/.{21}/u), - origin: 'test.com', - }, + expect( + metamaskController.approvalController.addAndShowApprovalRequest, + ).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.stringMatching(/.{21}/u), + origin: 'test.com', + requestData: expect.objectContaining({ + metadata: { + id: expect.stringMatching(/.{21}/u), + origin: 'test.com', + }, + }), + type: 'wallet_requestPermissions', }), - type: 'wallet_requestPermissions', - }), - ); + ); - const [params] = - metamaskController.approvalController.addAndShowApprovalRequest.mock - .calls[0]; - expect(params.id).toStrictEqual(params.requestData.metadata.id); - }); + const [params] = + metamaskController.approvalController.addAndShowApprovalRequest.mock + .calls[0]; + expect(params.id).toStrictEqual(params.requestData.metadata.id); + }); - it('requests approval from the ApprovalController for eth_accounts and permittedChains when only eth_accounts is specified in params and origin is not snapId', async () => { - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: {}, - }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', + it('requests approval from the ApprovalController for eth_accounts and permittedChains when only eth_accounts is specified in params and origin is not snapId', async () => { + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ + permissions: {}, + }); + jest + .spyOn(metamaskController.permissionController, 'grantPermissions') + .mockReturnValue({ + [Caip25EndowmentPermissionName]: { + foo: 'bar', + }, + }); + + await metamaskController.requestCaip25Approval('test.com', { + [PermissionNames.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['foo'], + }, + ], }, }); - await metamaskController.requestCaip25Approval('test.com', { - [PermissionNames.eth_accounts]: { - caveats: [ - { - type: CaveatTypes.restrictReturnedAccounts, - value: ['foo'], + expect( + metamaskController.approvalController.addAndShowApprovalRequest, + ).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.stringMatching(/.{21}/u), + origin: 'test.com', + requestData: { + metadata: { + id: expect.stringMatching(/.{21}/u), + origin: 'test.com', + }, + permissions: { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['wallet:eip155:foo'], + }, + }, + isMultichainOrigin: false, + }, + }, + ], + }, + }, }, - ], - }, + type: 'wallet_requestPermissions', + }), + ); }); - expect( - metamaskController.approvalController.addAndShowApprovalRequest, - ).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.stringMatching(/.{21}/u), - origin: 'test.com', - requestData: { - metadata: { - id: expect.stringMatching(/.{21}/u), - origin: 'test.com', + it('requests approval from the ApprovalController for eth_accounts and permittedChains when only permittedChains is specified in params and origin is not snapId', async () => { + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ + permissions: {}, + }); + jest + .spyOn(metamaskController.permissionController, 'grantPermissions') + .mockReturnValue({ + [Caip25EndowmentPermissionName]: { + foo: 'bar', }, - permissions: { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:foo'], + }); + + await metamaskController.requestCaip25Approval('test.com', { + [PermissionNames.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x64'], + }, + ], + }, + }); + + expect( + metamaskController.approvalController.addAndShowApprovalRequest, + ).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.stringMatching(/.{21}/u), + origin: 'test.com', + requestData: { + metadata: { + id: expect.stringMatching(/.{21}/u), + origin: 'test.com', + }, + permissions: { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [], + }, + 'eip155:100': { + accounts: [], + }, }, + isMultichainOrigin: false, }, - isMultichainOrigin: false, }, - }, - ], + ], + }, }, }, - }, - type: 'wallet_requestPermissions', - }), - ); - }); + type: 'wallet_requestPermissions', + }), + ); + }); - it('requests approval from the ApprovalController for eth_accounts and permittedChains when only permittedChains is specified in params and origin is not snapId', async () => { - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: {}, - }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', + it('requests approval from the ApprovalController for eth_accounts and permittedChains when both are specified in params and origin is not snapId', async () => { + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ + permissions: {}, + }); + jest + .spyOn(metamaskController.permissionController, 'grantPermissions') + .mockReturnValue({ + [Caip25EndowmentPermissionName]: { + foo: 'bar', + }, + }); + + await metamaskController.requestCaip25Approval('test.com', { + [PermissionNames.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['foo'], + }, + ], + }, + [PermissionNames.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x64'], + }, + ], }, }); - await metamaskController.requestCaip25Approval('test.com', { - [PermissionNames.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: ['0x64'], + expect( + metamaskController.approvalController.addAndShowApprovalRequest, + ).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.stringMatching(/.{21}/u), + origin: 'test.com', + requestData: { + metadata: { + id: expect.stringMatching(/.{21}/u), + origin: 'test.com', + }, + permissions: { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['wallet:eip155:foo'], + }, + 'eip155:100': { + accounts: ['eip155:100:foo'], + }, + }, + isMultichainOrigin: false, + }, + }, + ], + }, + }, }, - ], - }, + type: 'wallet_requestPermissions', + }), + ); }); - expect( - metamaskController.approvalController.addAndShowApprovalRequest, - ).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.stringMatching(/.{21}/u), - origin: 'test.com', - requestData: { - metadata: { - id: expect.stringMatching(/.{21}/u), - origin: 'test.com', + it('requests approval from the ApprovalController for only eth_accounts when only eth_accounts is specified in params and origin is snapId', async () => { + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ + permissions: {}, + }); + jest + .spyOn(metamaskController.permissionController, 'grantPermissions') + .mockReturnValue({ + [Caip25EndowmentPermissionName]: { + foo: 'bar', }, - permissions: { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: [], - }, - 'eip155:100': { - accounts: [], + }); + + await metamaskController.requestCaip25Approval('npm:snap', { + [PermissionNames.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['foo'], + }, + ], + }, + }); + + expect( + metamaskController.approvalController.addAndShowApprovalRequest, + ).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.stringMatching(/.{21}/u), + origin: 'npm:snap', + requestData: { + metadata: { + id: expect.stringMatching(/.{21}/u), + origin: 'npm:snap', + }, + permissions: { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['wallet:eip155:foo'], + }, }, + isMultichainOrigin: false, }, - isMultichainOrigin: false, }, - }, - ], + ], + }, }, }, - }, - type: 'wallet_requestPermissions', - }), - ); - }); + type: 'wallet_requestPermissions', + }), + ); + }); - it('requests approval from the ApprovalController for eth_accounts and permittedChains when both are specified in params and origin is not snapId', async () => { - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: {}, - }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', + it('requests approval from the ApprovalController for only eth_accounts when only permittedChains is specified in params and origin is snapId', async () => { + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ + permissions: {}, + }); + jest + .spyOn(metamaskController.permissionController, 'grantPermissions') + .mockReturnValue({ + [Caip25EndowmentPermissionName]: { + foo: 'bar', + }, + }); + + await metamaskController.requestCaip25Approval('npm:snap', { + [PermissionNames.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x64'], + }, + ], }, }); - await metamaskController.requestCaip25Approval('test.com', { - [PermissionNames.eth_accounts]: { - caveats: [ - { - type: CaveatTypes.restrictReturnedAccounts, - value: ['foo'], - }, - ], - }, - [PermissionNames.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: ['0x64'], + expect( + metamaskController.approvalController.addAndShowApprovalRequest, + ).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.stringMatching(/.{21}/u), + origin: 'npm:snap', + requestData: { + metadata: { + id: expect.stringMatching(/.{21}/u), + origin: 'npm:snap', + }, + permissions: { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [], + }, + }, + isMultichainOrigin: false, + }, + }, + ], + }, + }, }, - ], - }, + type: 'wallet_requestPermissions', + }), + ); }); - expect( - metamaskController.approvalController.addAndShowApprovalRequest, - ).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.stringMatching(/.{21}/u), - origin: 'test.com', - requestData: { - metadata: { - id: expect.stringMatching(/.{21}/u), - origin: 'test.com', + it('requests approval from the ApprovalController for only eth_accounts when both eth_accounts and permittedChains are specified in params and origin is snapId', async () => { + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ + permissions: {}, + }); + jest + .spyOn(metamaskController.permissionController, 'grantPermissions') + .mockReturnValue({ + [Caip25EndowmentPermissionName]: { + foo: 'bar', }, - permissions: { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:foo'], - }, - 'eip155:100': { - accounts: ['eip155:100:foo'], - }, - }, - isMultichainOrigin: false, - }, - }, - ], + }); + + await metamaskController.requestCaip25Approval('npm:snap', { + [PermissionNames.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['foo'], }, - }, + ], }, - type: 'wallet_requestPermissions', - }), - ); - }); - - it('requests approval from the ApprovalController for only eth_accounts when only eth_accounts is specified in params and origin is snapId', async () => { - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: {}, - }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', + [PermissionNames.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x64'], + }, + ], }, }); - await metamaskController.requestCaip25Approval('npm:snap', { - [PermissionNames.eth_accounts]: { - caveats: [ - { - type: CaveatTypes.restrictReturnedAccounts, - value: ['foo'], + expect( + metamaskController.approvalController.addAndShowApprovalRequest, + ).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.stringMatching(/.{21}/u), + origin: 'npm:snap', + requestData: { + metadata: { + id: expect.stringMatching(/.{21}/u), + origin: 'npm:snap', + }, + permissions: { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['wallet:eip155:foo'], + }, + }, + isMultichainOrigin: false, + }, + }, + ], + }, + }, }, - ], - }, + type: 'wallet_requestPermissions', + }), + ); }); - expect( - metamaskController.approvalController.addAndShowApprovalRequest, - ).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.stringMatching(/.{21}/u), - origin: 'npm:snap', - requestData: { - metadata: { - id: expect.stringMatching(/.{21}/u), - origin: 'npm:snap', - }, + it('throws an error if the eth_accounts and permittedChains approval is rejected', async () => { + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockRejectedValue(new Error('approval rejected')); + + await expect(() => + metamaskController.requestCaip25Approval('test.com', { + eth_accounts: {}, + }), + ).rejects.toThrow(new Error('approval rejected')); + }); + + it('requests CAIP-25 approval with accounts and chainIds specified from `eth_accounts` and `endowment:permittedChains` permissions caveats, and isMultichainOrigin: false if origin is not snapId', async () => { + const origin = 'test.com'; + + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ permissions: { [Caip25EndowmentPermissionName]: { caveats: [ @@ -1250,62 +1441,83 @@ describe('MetaMaskController', () => { type: Caip25CaveatType, value: { requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:foo'], - }, - }, + optionalScopes: {}, isMultichainOrigin: false, }, }, ], }, }, - }, - type: 'wallet_requestPermissions', - }), - ); - }); + }); - it('requests approval from the ApprovalController for only eth_accounts when only permittedChains is specified in params and origin is snapId', async () => { - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: {}, - }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', + await metamaskController.requestCaip25Approval('test.com', { + [RestrictedEthMethods.eth_accounts]: { + caveats: [ + { + type: 'restrictReturnedAccounts', + value: ['0xdeadbeef'], + }, + ], + }, + [EndowmentTypes.permittedChains]: { + caveats: [ + { + type: 'restrictNetworkSwitching', + value: ['0x1', '0x5'], + }, + ], }, }); - await metamaskController.requestCaip25Approval('npm:snap', { - [PermissionNames.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: ['0x64'], + expect( + metamaskController.approvalController.addAndShowApprovalRequest, + ).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.stringMatching(/.{21}/u), + origin, + requestData: { + metadata: { + id: expect.stringMatching(/.{21}/u), + origin, + }, + permissions: { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['wallet:eip155:0xdeadbeef'], + }, + 'eip155:1': { + accounts: ['eip155:1:0xdeadbeef'], + }, + 'eip155:5': { + accounts: ['eip155:5:0xdeadbeef'], + }, + }, + isMultichainOrigin: false, + }, + }, + ], + }, + }, }, - ], - }, + type: 'wallet_requestPermissions', + }), + ); }); - expect( - metamaskController.approvalController.addAndShowApprovalRequest, - ).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.stringMatching(/.{21}/u), - origin: 'npm:snap', - requestData: { - metadata: { - id: expect.stringMatching(/.{21}/u), - origin: 'npm:snap', - }, + it('requests CAIP-25 approval with approved accounts for the `wallet:eip155` scope (and no approved chainIds) with isMultichainOrigin: false if origin is snapId', async () => { + const origin = 'npm:snap'; + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ permissions: { [Caip25EndowmentPermissionName]: { caveats: [ @@ -1313,315 +1525,196 @@ describe('MetaMaskController', () => { type: Caip25CaveatType, value: { requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: [], - }, - }, + optionalScopes: {}, isMultichainOrigin: false, }, }, ], }, }, - }, - type: 'wallet_requestPermissions', - }), - ); - }); + }); - it('requests approval from the ApprovalController for only eth_accounts when both eth_accounts and permittedChains are specified in params and origin is snapId', async () => { - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: {}, - }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', + jest + .spyOn(metamaskController.permissionController, 'grantPermissions') + .mockReturnValue({ + [Caip25EndowmentPermissionName]: { + foo: 'bar', + }, + }); + + await metamaskController.requestCaip25Approval(origin, { + [RestrictedEthMethods.eth_accounts]: { + caveats: [ + { + type: 'restrictReturnedAccounts', + value: ['0xdeadbeef'], + }, + ], + }, + [EndowmentTypes.permittedChains]: { + caveats: [ + { + type: 'restrictNetworkSwitching', + value: ['0x1', '0x5'], + }, + ], }, }); - await metamaskController.requestCaip25Approval('npm:snap', { - [PermissionNames.eth_accounts]: { - caveats: [ - { - type: CaveatTypes.restrictReturnedAccounts, - value: ['foo'], - }, - ], - }, - [PermissionNames.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: ['0x64'], - }, - ], - }, - }); - - expect( - metamaskController.approvalController.addAndShowApprovalRequest, - ).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.stringMatching(/.{21}/u), - origin: 'npm:snap', - requestData: { - metadata: { - id: expect.stringMatching(/.{21}/u), - origin: 'npm:snap', - }, - permissions: { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:foo'], + expect( + metamaskController.approvalController.addAndShowApprovalRequest, + ).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.stringMatching(/.{21}/u), + origin, + requestData: expect.objectContaining({ + metadata: { + id: expect.stringMatching(/.{21}/u), + origin, + }, + permissions: { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['wallet:eip155:0xdeadbeef'], + }, }, + isMultichainOrigin: false, }, - isMultichainOrigin: false, + }, + ], + }, + }, + }), + type: 'wallet_requestPermissions', + }), + ); + }); + + it('should return sessions scopes returned from calling ApprovalController.addAndShowApprovalRequest', async () => { + const expectedPermissions = { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['wallet:eip155:0xdeadbeef'], }, }, - ], + isMultichainOrigin: false, + }, }, - }, + ], }, - type: 'wallet_requestPermissions', - }), - ); - }); + }; - it('throws an error if the eth_accounts and permittedChains approval is rejected', async () => { - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockRejectedValue(new Error('approval rejected')); + jest + .spyOn( + metamaskController.approvalController, + 'addAndShowApprovalRequest', + ) + .mockResolvedValue({ + permissions: expectedPermissions, + }); - await expect(() => - metamaskController.requestCaip25Approval('test.com', { - eth_accounts: {}, - }), - ).rejects.toThrow(new Error('approval rejected')); + const result = await metamaskController.requestCaip25Approval( + 'test.com', + {}, + ); + + expect(result).toStrictEqual(expectedPermissions); + }); }); - it('requests CAIP-25 approval with accounts and chainIds specified from `eth_accounts` and `endowment:permittedChains` permissions caveats, and isMultichainOrigin: false if origin is not snapId', async () => { - const origin = 'test.com'; + describe('invalid requests', () => { + const INVALID_CHAIN_HEX = '0x11'; + const INVALID_ADDRESS = 'invalid.eth'; - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: {}, - isMultichainOrigin: false, - }, - }, - ], + describe('unrecognized addresses requested', () => { + beforeEach(() => { + PermissionSpecifications.validateCaveatAccounts.mockImplementation( + () => { + throw new Error( + `${PermissionNames.eth_accounts} error: Received unrecognized address: "${INVALID_ADDRESS}".`, + ); }, - }, + ); }); - await metamaskController.requestCaip25Approval('test.com', { - [RestrictedEthMethods.eth_accounts]: { - caveats: [ - { - type: 'restrictReturnedAccounts', - value: ['0xdeadbeef'], - }, - ], - }, - [EndowmentTypes.permittedChains]: { - caveats: [ - { - type: 'restrictNetworkSwitching', - value: ['0x1', '0x5'], - }, - ], - }, - }); + afterEach(() => { + PermissionSpecifications.validateCaveatAccounts.mockClear(); + }); - expect( - metamaskController.approvalController.addAndShowApprovalRequest, - ).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.stringMatching(/.{21}/u), - origin, - requestData: { - metadata: { - id: expect.stringMatching(/.{21}/u), - origin, - }, - permissions: { - [Caip25EndowmentPermissionName]: { + it('should throw exception when requesting invalid account', async () => { + await expect( + metamaskController.requestCaip25Approval('test.com', { + [PermissionNames.eth_accounts]: { caveats: [ { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:0xdeadbeef'], - }, - 'eip155:1': { - accounts: ['eip155:1:0xdeadbeef'], - }, - 'eip155:5': { - accounts: ['eip155:5:0xdeadbeef'], - }, - }, - isMultichainOrigin: false, - }, + type: CaveatTypes.restrictReturnedAccounts, + value: [INVALID_ADDRESS], }, ], }, - }, - }, - type: 'wallet_requestPermissions', - }), - ); - }); + }), + ).rejects.toThrow( + `${PermissionNames.eth_accounts} error: Received unrecognized address: "${INVALID_ADDRESS}".`, + ); + }); + }); - it('requests CAIP-25 approval with approved accounts for the `wallet:eip155` scope (and no approved chainIds) with isMultichainOrigin: false if origin is snapId', async () => { - const origin = 'npm:snap'; - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: {}, - isMultichainOrigin: false, - }, - }, - ], + describe('unrecognized chain id requested', () => { + beforeEach(() => { + PermissionSpecifications.validateCaveatAccounts.mockImplementation( + () => ({}), + ); + PermissionSpecifications.validateCaveatNetworks.mockImplementation( + () => { + throw new Error( + `${PermissionNames.permittedChains} error: Received unrecognized chainId: "${INVALID_CHAIN_HEX}". Please try adding the network first via wallet_addEthereumChain.`, + ); }, - }, + ); }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', - }, + afterEach(() => { + PermissionSpecifications.validateCaveatAccounts.mockClear(); + PermissionSpecifications.validateCaveatNetworks.mockClear(); }); - await metamaskController.requestCaip25Approval(origin, { - [RestrictedEthMethods.eth_accounts]: { - caveats: [ - { - type: 'restrictReturnedAccounts', - value: ['0xdeadbeef'], - }, - ], - }, - [EndowmentTypes.permittedChains]: { - caveats: [ - { - type: 'restrictNetworkSwitching', - value: ['0x1', '0x5'], - }, - ], - }, - }); - - expect( - metamaskController.approvalController.addAndShowApprovalRequest, - ).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.stringMatching(/.{21}/u), - origin, - requestData: expect.objectContaining({ - metadata: { - id: expect.stringMatching(/.{21}/u), - origin, - }, - permissions: { - [Caip25EndowmentPermissionName]: { + it('should throw exception when requesting invalid chain id', async () => { + await expect( + metamaskController.requestCaip25Approval('test.com', { + [PermissionNames.eth_accounts]: { caveats: [ { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:0xdeadbeef'], - }, - }, - isMultichainOrigin: false, - }, + type: CaveatTypes.restrictReturnedAccounts, + value: ['0xdead'], }, ], }, - }, - }), - type: 'wallet_requestPermissions', - }), - ); - }); - - it('should return sessions scopes returned from calling ApprovalController.addAndShowApprovalRequest', async () => { - const expectedPermissions = { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:0xdeadbeef'], + [PermissionNames.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: [INVALID_CHAIN_HEX], }, - }, - isMultichainOrigin: false, + ], }, - }, - ], - }, - }; - - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - permissions: expectedPermissions, + }), + ).rejects.toThrow( + `${PermissionNames.permittedChains} error: Received unrecognized chainId: "${INVALID_CHAIN_HEX}". Please try adding the network first via wallet_addEthereumChain.`, + ); }); - - const result = await metamaskController.requestCaip25Approval( - 'test.com', - {}, - ); - - expect(result).toStrictEqual(expectedPermissions); + }); }); }); From c9df79b148c085ba949bcf7974ba8f7216595c3a Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 7 Feb 2025 20:12:08 +0100 Subject: [PATCH 2/7] code review changes --- app/scripts/controllers/permissions/specifications.js | 4 ++-- app/scripts/metamask-controller.js | 2 +- app/scripts/metamask-controller.test.js | 9 --------- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/app/scripts/controllers/permissions/specifications.js b/app/scripts/controllers/permissions/specifications.js index 3e99ea678fcc..b631829ecc2d 100644 --- a/app/scripts/controllers/permissions/specifications.js +++ b/app/scripts/controllers/permissions/specifications.js @@ -201,7 +201,7 @@ export const unrestrictedMethods = Object.freeze([ * @param {string[]} accounts - The accounts associated with the caveat. * @param {() => Record} getInternalAccounts - * Gets all AccountsController InternalAccounts. - * TODO: Remove this function once permission refactor/factory differ work is merged into main + * TODO: Remove this function once the CAIP-25 permission refactor/factory differ work is merged into main */ export function validateCaveatAccounts(accounts, getInternalAccounts) { if (!Array.isArray(accounts) || accounts.length === 0) { @@ -238,7 +238,7 @@ export function validateCaveatAccounts(accounts, getInternalAccounts) { * @param {string[]} chainIdsForCaveat - The list of chain IDs to validate. * @param {function(string): string} findNetworkClientIdByChainId - Function to find network client ID by chain ID. * @throws {Error} If the chainIdsForCaveat is not a non-empty array of valid chain IDs. - * TODO: Remove this function once permission refactor/factory differ work is merged into main + * TODO: Remove this function once the CAIP-25 permission refactor/factory differ work is merged into main */ export function validateCaveatNetworks( chainIdsForCaveat, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 48f781bfbff9..fcc2dbcf94ed 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5298,7 +5298,7 @@ export default class MetamaskController extends EventEmitter { requestedAccounts, this.accountsController.listAccounts.bind(this.accountsController), ); - console.log('i passed here'); + validateCaveatNetworks( requestedChains, this.networkController.findNetworkClientIdByChainId.bind( diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 09fa048a7106..6b76c3d8c249 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -1649,10 +1649,6 @@ describe('MetaMaskController', () => { ); }); - afterEach(() => { - PermissionSpecifications.validateCaveatAccounts.mockClear(); - }); - it('should throw exception when requesting invalid account', async () => { await expect( metamaskController.requestCaip25Approval('test.com', { @@ -1685,11 +1681,6 @@ describe('MetaMaskController', () => { ); }); - afterEach(() => { - PermissionSpecifications.validateCaveatAccounts.mockClear(); - PermissionSpecifications.validateCaveatNetworks.mockClear(); - }); - it('should throw exception when requesting invalid chain id', async () => { await expect( metamaskController.requestCaip25Approval('test.com', { From ed828d51ed2edaecf2f2db812f415e81fb5eaca1 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 7 Feb 2025 20:13:38 +0100 Subject: [PATCH 3/7] minor fix --- app/scripts/controllers/permissions/specifications.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/controllers/permissions/specifications.js b/app/scripts/controllers/permissions/specifications.js index b631829ecc2d..24db69e64130 100644 --- a/app/scripts/controllers/permissions/specifications.js +++ b/app/scripts/controllers/permissions/specifications.js @@ -201,7 +201,7 @@ export const unrestrictedMethods = Object.freeze([ * @param {string[]} accounts - The accounts associated with the caveat. * @param {() => Record} getInternalAccounts - * Gets all AccountsController InternalAccounts. - * TODO: Remove this function once the CAIP-25 permission refactor/factory differ work is merged into main + * TODO: Remove this function once the CAIP-25 permission refactor/factory differ work is merged into main */ export function validateCaveatAccounts(accounts, getInternalAccounts) { if (!Array.isArray(accounts) || accounts.length === 0) { @@ -238,7 +238,7 @@ export function validateCaveatAccounts(accounts, getInternalAccounts) { * @param {string[]} chainIdsForCaveat - The list of chain IDs to validate. * @param {function(string): string} findNetworkClientIdByChainId - Function to find network client ID by chain ID. * @throws {Error} If the chainIdsForCaveat is not a non-empty array of valid chain IDs. - * TODO: Remove this function once the CAIP-25 permission refactor/factory differ work is merged into main + * TODO: Remove this function once the CAIP-25 permission refactor/factory differ work is merged into main */ export function validateCaveatNetworks( chainIdsForCaveat, From 4c1e0dc02555d0522b560aa5d49fdf97719452d6 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 7 Feb 2025 22:25:18 +0100 Subject: [PATCH 4/7] address failing e2e tests --- app/scripts/metamask-controller.js | 48 ++++++++++++++++-------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index fcc2dbcf94ed..b61148b51f9b 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5272,6 +5272,32 @@ export default class MetamaskController extends EventEmitter { PermissionNames.permittedChains, ]); + const requestedAccounts = + permissions[PermissionNames.eth_accounts]?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, + )?.value ?? []; + + const requestedChains = + permissions[PermissionNames.permittedChains]?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, + )?.value ?? []; + + if (permissions[RestrictedMethods.eth_accounts]) { + validateCaveatAccounts( + requestedAccounts, + this.accountsController.listAccounts.bind(this.accountsController), + ); + } + + if (permissions[PermissionNames.permittedChains]) { + validateCaveatNetworks( + requestedChains, + this.networkController.findNetworkClientIdByChainId.bind( + this.networkController, + ), + ); + } + if (!permissions[RestrictedMethods.eth_accounts]) { permissions[RestrictedMethods.eth_accounts] = {}; } @@ -5284,28 +5310,6 @@ export default class MetamaskController extends EventEmitter { delete permissions[PermissionNames.permittedChains]; } - const requestedChains = - permissions[PermissionNames.permittedChains]?.caveats?.find( - (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, - )?.value ?? []; - - const requestedAccounts = - permissions[PermissionNames.eth_accounts]?.caveats?.find( - (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, - )?.value ?? []; - - validateCaveatAccounts( - requestedAccounts, - this.accountsController.listAccounts.bind(this.accountsController), - ); - - validateCaveatNetworks( - requestedChains, - this.networkController.findNetworkClientIdByChainId.bind( - this.networkController, - ), - ); - const newCaveatValue = { requiredScopes: {}, optionalScopes: { From 84df286e4e45d15a80ea146301ec47ed3280a5db Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 7 Feb 2025 22:57:05 +0100 Subject: [PATCH 5/7] address broken wallet_requestPermissions.spec.ts --- app/scripts/metamask-controller.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index b61148b51f9b..ab9c716dc2db 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5282,14 +5282,14 @@ export default class MetamaskController extends EventEmitter { (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, )?.value ?? []; - if (permissions[RestrictedMethods.eth_accounts]) { + if (permissions[PermissionNames.permittedChains]?.caveats) { validateCaveatAccounts( requestedAccounts, this.accountsController.listAccounts.bind(this.accountsController), ); } - if (permissions[PermissionNames.permittedChains]) { + if (permissions[PermissionNames.permittedChains]?.caveat) { validateCaveatNetworks( requestedChains, this.networkController.findNetworkClientIdByChainId.bind( From 6a7ef362dc80d5d2ec10e5750e70affc87773567 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Sat, 8 Feb 2025 00:54:20 +0100 Subject: [PATCH 6/7] test: fix --- app/scripts/metamask-controller.js | 6 +++--- app/scripts/metamask-controller.test.js | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index ab9c716dc2db..81596b175889 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5273,7 +5273,7 @@ export default class MetamaskController extends EventEmitter { ]); const requestedAccounts = - permissions[PermissionNames.eth_accounts]?.caveats?.find( + permissions[RestrictedMethods.eth_accounts]?.caveats?.find( (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, )?.value ?? []; @@ -5282,14 +5282,14 @@ export default class MetamaskController extends EventEmitter { (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, )?.value ?? []; - if (permissions[PermissionNames.permittedChains]?.caveats) { + if (permissions[RestrictedMethods.eth_accounts]?.caveats) { validateCaveatAccounts( requestedAccounts, this.accountsController.listAccounts.bind(this.accountsController), ); } - if (permissions[PermissionNames.permittedChains]?.caveat) { + if (permissions[PermissionNames.permittedChains]?.caveats) { validateCaveatNetworks( requestedChains, this.networkController.findNetworkClientIdByChainId.bind( diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 6b76c3d8c249..19cfac58599d 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -55,6 +55,7 @@ import { CaveatTypes, EndowmentTypes, RestrictedEthMethods, + RestrictedMethods, } from '../../shared/constants/permissions'; import { deferredPromise } from './lib/util'; import { METAMASK_COOKIE_HANDLER } from './constants/stream'; @@ -1643,7 +1644,7 @@ describe('MetaMaskController', () => { PermissionSpecifications.validateCaveatAccounts.mockImplementation( () => { throw new Error( - `${PermissionNames.eth_accounts} error: Received unrecognized address: "${INVALID_ADDRESS}".`, + `${RestrictedMethods.eth_accounts} error: Received unrecognized address: "${INVALID_ADDRESS}".`, ); }, ); @@ -1652,7 +1653,7 @@ describe('MetaMaskController', () => { it('should throw exception when requesting invalid account', async () => { await expect( metamaskController.requestCaip25Approval('test.com', { - [PermissionNames.eth_accounts]: { + [RestrictedMethods.eth_accounts]: { caveats: [ { type: CaveatTypes.restrictReturnedAccounts, @@ -1662,7 +1663,7 @@ describe('MetaMaskController', () => { }, }), ).rejects.toThrow( - `${PermissionNames.eth_accounts} error: Received unrecognized address: "${INVALID_ADDRESS}".`, + `${RestrictedMethods.eth_accounts} error: Received unrecognized address: "${INVALID_ADDRESS}".`, ); }); }); @@ -1684,7 +1685,7 @@ describe('MetaMaskController', () => { it('should throw exception when requesting invalid chain id', async () => { await expect( metamaskController.requestCaip25Approval('test.com', { - [PermissionNames.eth_accounts]: { + [RestrictedMethods.eth_accounts]: { caveats: [ { type: CaveatTypes.restrictReturnedAccounts, From 7a9a4a310577d7fa318fdca0ace2e4689a419682 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Sat, 8 Feb 2025 11:21:45 +0100 Subject: [PATCH 7/7] address nit --- app/scripts/metamask-controller.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 19cfac58599d..81af9a1a301d 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -1671,7 +1671,7 @@ describe('MetaMaskController', () => { describe('unrecognized chain id requested', () => { beforeEach(() => { PermissionSpecifications.validateCaveatAccounts.mockImplementation( - () => ({}), + () => undefined, ); PermissionSpecifications.validateCaveatNetworks.mockImplementation( () => {