From b76875ac69f52f5b864c42a0c2842b269615ee51 Mon Sep 17 00:00:00 2001 From: Nicholas Ellul Date: Mon, 31 Jul 2023 08:48:48 -0400 Subject: [PATCH] Update @metamask/phishing-controller to v4.0.0 (#18840) * Update phishing controller to v4.0.0 * Move phishing e2e test utilities into its own helper.js * Update phishing detection e2e test * Update MetaMask Controller test mocks * Update mv3 phishing tests * Fix test for 500 error on warning page * Allow for directories in test folder * Update migration number * Linting fixes * Remove fail on console error * Separate mocks from helpers * Have migration delete PhishingController state entirely * Remove phishing detection directory * Only delete the listState in migration * Bump migration version --- .../metamask-controller.actions.test.js | 44 ++++- app/scripts/metamask-controller.test.js | 43 ++++- app/scripts/migrations/090.test.js | 109 +++++++++++ app/scripts/migrations/090.ts | 37 ++++ app/scripts/migrations/index.js | 2 + package.json | 2 +- test/e2e/helpers.js | 81 --------- test/e2e/mock-e2e.js | 32 +--- .../mv3/phishing-warning-sw-restart.spec.js | 17 +- test/e2e/run-all.js | 18 +- test/e2e/tests/phishing-controller/helpers.js | 25 +++ test/e2e/tests/phishing-controller/mocks.js | 172 ++++++++++++++++++ .../phishing-detection.spec.js | 163 +++++++++++------ yarn.lock | 14 +- 14 files changed, 565 insertions(+), 194 deletions(-) create mode 100644 app/scripts/migrations/090.test.js create mode 100644 app/scripts/migrations/090.ts create mode 100644 test/e2e/tests/phishing-controller/helpers.js create mode 100644 test/e2e/tests/phishing-controller/mocks.js rename test/e2e/tests/{ => phishing-controller}/phishing-detection.spec.js (69%) diff --git a/app/scripts/metamask-controller.actions.test.js b/app/scripts/metamask-controller.actions.test.js index c1ea740f8..5e7c771a1 100644 --- a/app/scripts/metamask-controller.actions.test.js +++ b/app/scripts/metamask-controller.actions.test.js @@ -1,7 +1,14 @@ import { strict as assert } from 'assert'; import sinon from 'sinon'; import proxyquire from 'proxyquire'; - +import { + ListNames, + METAMASK_STALELIST_URL, + METAMASK_HOTLIST_DIFF_URL, + PHISHING_CONFIG_BASE_URL, + METAMASK_STALELIST_FILE, + METAMASK_HOTLIST_DIFF_FILE, +} from '@metamask/phishing-controller'; import { ApprovalRequestNotFoundError } from '@metamask/approval-controller'; import { PermissionsRequestNotFoundError } from '@metamask/permission-controller'; import nock from 'nock'; @@ -59,21 +66,28 @@ describe('MetaMaskController', function () { }); beforeEach(function () { - nock('https://static.metafi.codefi.network') + nock(PHISHING_CONFIG_BASE_URL) .persist() - .get('/api/v1/lists/stalelist.json') + .get(METAMASK_STALELIST_FILE) .reply( 200, JSON.stringify({ version: 2, tolerance: 2, - fuzzylist: [], - allowlist: [], - blocklist: ['127.0.0.1'], - lastUpdated: 0, + lastUpdated: 1, + eth_phishing_detect_config: { + fuzzylist: [], + allowlist: [], + blocklist: ['127.0.0.1'], + name: ListNames.MetaMask, + }, + phishfort_hotlist: { + blocklist: [], + name: ListNames.Phishfort, + }, }), ) - .get('/api/v1/lists/hotlist.json') + .get(METAMASK_HOTLIST_DIFF_FILE) .reply( 200, JSON.stringify([ @@ -110,6 +124,20 @@ describe('MetaMaskController', function () { await ganacheServer.quit(); }); + describe('Phishing Detection Mock', function () { + it('should be updated to use v1 of the API', function () { + // Update the fixture above if this test fails + assert.equal( + METAMASK_STALELIST_URL, + 'https://phishing-detection.metafi.codefi.network/v1/stalelist', + ); + assert.equal( + METAMASK_HOTLIST_DIFF_URL, + 'https://phishing-detection.metafi.codefi.network/v1/diffsSince', + ); + }); + }); + describe('#addNewAccount', function () { it('two parallel calls with same accountCount give same result', async function () { await metamaskController.createNewVaultAndKeychain('test@123'); diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index a8d360e10..5bed8c899 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -7,6 +7,14 @@ import EthQuery from 'eth-query'; import proxyquire from 'proxyquire'; import browser from 'webextension-polyfill'; import { wordlist as englishWordlist } from '@metamask/scure-bip39/dist/wordlists/english'; +import { + ListNames, + METAMASK_STALELIST_URL, + METAMASK_HOTLIST_DIFF_URL, + PHISHING_CONFIG_BASE_URL, + METAMASK_STALELIST_FILE, + METAMASK_HOTLIST_DIFF_FILE, +} from '@metamask/phishing-controller'; import { TransactionStatus } from '../../shared/constants/transaction'; import createTxMeta from '../../test/lib/createTxMeta'; import { NETWORK_TYPES } from '../../shared/constants/network'; @@ -185,21 +193,28 @@ describe('MetaMaskController', function () { .persist() .get(/.*/u) .reply(200, '{"JPY":12415.9}'); - nock('https://static.metafi.codefi.network') + nock(PHISHING_CONFIG_BASE_URL) .persist() - .get('/api/v1/lists/stalelist.json') + .get(METAMASK_STALELIST_FILE) .reply( 200, JSON.stringify({ version: 2, tolerance: 2, - fuzzylist: [], - allowlist: [], - blocklist: ['127.0.0.1'], - lastUpdated: 0, + lastUpdated: 1, + eth_phishing_detect_config: { + fuzzylist: [], + allowlist: [], + blocklist: ['127.0.0.1'], + name: ListNames.MetaMask, + }, + phishfort_hotlist: { + blocklist: [], + name: ListNames.Phishfort, + }, }), ) - .get('/api/v1/lists/hotlist.json') + .get(METAMASK_HOTLIST_DIFF_FILE) .reply( 200, JSON.stringify([ @@ -223,6 +238,20 @@ describe('MetaMaskController', function () { await ganacheServer.quit(); }); + describe('Phishing Detection Mock', function () { + it('should be updated to use v1 of the API', function () { + // Update the fixture above if this test fails + assert.equal( + METAMASK_STALELIST_URL, + 'https://phishing-detection.metafi.codefi.network/v1/stalelist', + ); + assert.equal( + METAMASK_HOTLIST_DIFF_URL, + 'https://phishing-detection.metafi.codefi.network/v1/diffsSince', + ); + }); + }); + describe('MetaMaskController Behaviour', function () { let metamaskController; diff --git a/app/scripts/migrations/090.test.js b/app/scripts/migrations/090.test.js new file mode 100644 index 000000000..6a28c60f2 --- /dev/null +++ b/app/scripts/migrations/090.test.js @@ -0,0 +1,109 @@ +import { migrate, version } from './090'; + +const PREVIOUS_VERSION = version - 1; + +describe('migration #90', () => { + it('updates the version metadata', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: {}, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.meta).toStrictEqual({ + version, + }); + }); + + it('does not change the state if the phishing controller state does not exist', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { test: '123' }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does not change the state if the phishing controller state is invalid', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { PhishingController: 'this is not valid' }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does not change the state if the listState property does not exist', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { + PhishingController: { test: 123 }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('deletes the "listState" property', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { PhishingController: { listState: {} } }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data.PhishingController.listState).toBeUndefined(); + }); + + it('deletes the listState if present', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { PhishingController: { listState: { test: 123 } } }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + PhishingController: {}, + }); + }); + + it('does not delete the allowlist if present', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { + PhishingController: { + whitelist: ['foobar.com'], + listState: { test: 123 }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + PhishingController: { whitelist: ['foobar.com'] }, + }); + }); +}); diff --git a/app/scripts/migrations/090.ts b/app/scripts/migrations/090.ts new file mode 100644 index 000000000..e45ec05e4 --- /dev/null +++ b/app/scripts/migrations/090.ts @@ -0,0 +1,37 @@ +import { cloneDeep } from 'lodash'; +import { hasProperty, isObject } from '@metamask/utils'; + +export const version = 90; + +/** + * Explain the purpose of the migration here. + * + * @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist. + * @param originalVersionedData.meta - State metadata. + * @param originalVersionedData.meta.version - The current state version. + * @param originalVersionedData.data - The persisted MetaMask state, keyed by controller. + * @returns Updated versioned MetaMask extension state. + */ +export async function migrate(originalVersionedData: { + meta: { version: number }; + data: Record; +}) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + versionedData.data = transformState(versionedData.data); + return versionedData; +} + +function transformState(state: Record) { + if ( + !hasProperty(state, 'PhishingController') || + !isObject(state.PhishingController) || + !hasProperty(state.PhishingController, 'listState') + ) { + return state; + } + + delete state.PhishingController.listState; + + return state; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 429ef7959..adbe48252 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -93,6 +93,7 @@ import * as m086 from './086'; import * as m087 from './087'; import * as m088 from './088'; import * as m089 from './089'; +import * as m090 from './090'; const migrations = [ m002, @@ -183,6 +184,7 @@ const migrations = [ m087, m088, m089, + m090, ]; export default migrations; diff --git a/package.json b/package.json index ca3fada5f..0261e26af 100644 --- a/package.json +++ b/package.json @@ -256,7 +256,7 @@ "@metamask/notification-controller": "^3.0.0", "@metamask/obs-store": "^8.1.0", "@metamask/permission-controller": "^4.0.0", - "@metamask/phishing-controller": "^3.0.0", + "@metamask/phishing-controller": "^4.0.0", "@metamask/post-message-stream": "^6.0.0", "@metamask/ppom-validator": "^0.1.2", "@metamask/providers": "^11.1.0", diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index 536c13bbb..cbdacf71f 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -506,85 +506,6 @@ const openDapp = async (driver, contract = null, dappURL = DAPP_URL) => { ? await driver.openNewPage(`${dappURL}/?contract=${contract}`) : await driver.openNewPage(dappURL); }; -const STALELIST_URL = - 'https://static.metafi.codefi.network/api/v1/lists/stalelist.json'; - -const emptyHtmlPage = ` - - - - title - - - Empty page - -`; - -/** - * Setup fetch mocks for the phishing detection feature. - * - * The mock configuration will show that "127.0.0.1" is blocked. The dynamic lookup on the warning - * page can be customized, so that we can test both the MetaMask and PhishFort block cases. - * - * @param {import('mockttp').Mockttp} mockServer - The mock server. - * @param {object} metamaskPhishingConfigResponse - The response for the dynamic phishing - * configuration lookup performed by the warning page. - */ -async function setupPhishingDetectionMocks( - mockServer, - metamaskPhishingConfigResponse, -) { - await mockServer.forGet(STALELIST_URL).thenCallback(() => { - return { - statusCode: 200, - json: { - version: 2, - tolerance: 2, - fuzzylist: [], - allowlist: [], - blocklist: ['127.0.0.1'], - lastUpdated: 0, - }, - }; - }); - - await mockServer - .forGet('https://github.com/MetaMask/eth-phishing-detect/issues/new') - .thenCallback(() => { - return { - statusCode: 200, - body: emptyHtmlPage, - }; - }); - await mockServer - .forGet('https://github.com/phishfort/phishfort-lists/issues/new') - .thenCallback(() => { - return { - statusCode: 200, - body: emptyHtmlPage, - }; - }); - - await mockServer - .forGet( - 'https://raw.githubusercontent.com/MetaMask/eth-phishing-detect/master/src/config.json', - ) - .thenCallback(() => metamaskPhishingConfigResponse); -} - -function mockPhishingDetection(mockServer) { - setupPhishingDetectionMocks(mockServer, { - statusCode: 200, - json: { - version: 2, - tolerance: 2, - fuzzylist: [], - whitelist: [], - blacklist: ['127.0.0.1'], - lastUpdated: 0, - }, - }); -} const PRIVATE_KEY = '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC'; @@ -840,8 +761,6 @@ module.exports = { importWrongSRPOnboardingFlow, testSRPDropdownIterations, openDapp, - mockPhishingDetection, - setupPhishingDetectionMocks, defaultGanacheOptions, sendTransaction, findAnotherAccountFromAccountList, diff --git a/test/e2e/mock-e2e.js b/test/e2e/mock-e2e.js index a3e48208d..997c040c2 100644 --- a/test/e2e/mock-e2e.js +++ b/test/e2e/mock-e2e.js @@ -4,21 +4,9 @@ const blacklistedHosts = [ 'mainnet.infura.io', 'sepolia.infura.io', ]; - -const HOTLIST_URL = - 'https://static.metafi.codefi.network/api/v1/lists/hotlist.json'; -const STALELIST_URL = - 'https://static.metafi.codefi.network/api/v1/lists/stalelist.json'; - -const emptyHotlist = []; -const emptyStalelist = { - version: 2, - tolerance: 2, - fuzzylist: [], - allowlist: [], - blocklist: [], - lastUpdated: 0, -}; +const { + mockEmptyStalelistAndHotlist, +} = require('./tests/phishing-controller/mocks'); /** * Setup E2E network mocks. @@ -385,19 +373,7 @@ async function setupMocking(server, testSpecificMock, { chainId }) { }; }); - await server.forGet(STALELIST_URL).thenCallback(() => { - return { - statusCode: 200, - json: emptyStalelist, - }; - }); - - await server.forGet(HOTLIST_URL).thenCallback(() => { - return { - statusCode: 200, - json: emptyHotlist, - }; - }); + await mockEmptyStalelistAndHotlist(server); await server .forPost('https://customnetwork.com/api/customRPC') diff --git a/test/e2e/mv3/phishing-warning-sw-restart.spec.js b/test/e2e/mv3/phishing-warning-sw-restart.spec.js index 2e10f35b2..8de3ad7e0 100644 --- a/test/e2e/mv3/phishing-warning-sw-restart.spec.js +++ b/test/e2e/mv3/phishing-warning-sw-restart.spec.js @@ -1,7 +1,7 @@ const { strict: assert } = require('assert'); +const FixtureBuilder = require('../fixture-builder'); const { withFixtures, - mockPhishingDetection, openDapp, defaultGanacheOptions, assertAccountBalanceForDOM, @@ -11,7 +11,11 @@ const { unlockWallet, terminateServiceWorker, } = require('../helpers'); -const FixtureBuilder = require('../fixture-builder'); + +const { + setupPhishingDetectionMocks, + BlockProvider, +} = require('../tests/phishing-controller/helpers'); describe('Phishing warning page', function () { const driverOptions = { openDevToolsForTabs: true }; @@ -21,12 +25,17 @@ describe('Phishing warning page', function () { await withFixtures( { - dapp: true, fixtures: new FixtureBuilder().build(), ganacheOptions: defaultGanacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, driverOptions, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + }, + dapp: true, }, async ({ driver, ganacheServer }) => { await driver.navigate(); diff --git a/test/e2e/run-all.js b/test/e2e/run-all.js index d97d6c8a1..28c43da28 100644 --- a/test/e2e/run-all.js +++ b/test/e2e/run-all.js @@ -6,10 +6,20 @@ const { runInShell } = require('../../development/lib/run-command'); const { exitWithError } = require('../../development/lib/exit-with-error'); const getTestPathsForTestDir = async (testDir) => { - const testFilenames = await fs.readdir(testDir); - const testPaths = testFilenames.map((filename) => - path.join(testDir, filename), - ); + const testFilenames = await fs.readdir(testDir, { withFileTypes: true }); + const testPaths = []; + + for (const itemInDirectory of testFilenames) { + const fullPath = path.join(testDir, itemInDirectory.name); + + if (itemInDirectory.isDirectory()) { + const subDirPaths = await getTestPathsForTestDir(fullPath); + testPaths.push(...subDirPaths); + } else if (fullPath.endsWith('.spec.js')) { + testPaths.push(fullPath); + } + } + return testPaths; }; diff --git a/test/e2e/tests/phishing-controller/helpers.js b/test/e2e/tests/phishing-controller/helpers.js new file mode 100644 index 000000000..a00d5ddb3 --- /dev/null +++ b/test/e2e/tests/phishing-controller/helpers.js @@ -0,0 +1,25 @@ +const { + METAMASK_STALELIST_URL, + METAMASK_HOTLIST_DIFF_URL, + ListNames, +} = require('@metamask/phishing-controller'); + +/** + * The block provider names. + * + * @enum {BlockProvider} + * @readonly + * @property {string} MetaMask - The name of the MetaMask block provider. + * @property {string} PhishFort - The name of the PhishFort block provider. + */ +const BlockProvider = { + MetaMask: 'metamask', + PhishFort: 'phishfort', +}; + +module.exports = { + METAMASK_HOTLIST_DIFF_URL, + METAMASK_STALELIST_URL, + BlockProvider, + ListNames, +}; diff --git a/test/e2e/tests/phishing-controller/mocks.js b/test/e2e/tests/phishing-controller/mocks.js new file mode 100644 index 000000000..f15e4b848 --- /dev/null +++ b/test/e2e/tests/phishing-controller/mocks.js @@ -0,0 +1,172 @@ +const { + METAMASK_STALELIST_URL, + METAMASK_HOTLIST_DIFF_URL, + ListNames, + BlockProvider, +} = require('./helpers'); + +// last updated must not be 0 +const lastUpdated = 1; +const defaultHotlist = { data: [] }; +const defaultStalelist = { + version: 2, + tolerance: 2, + lastUpdated, + eth_phishing_detect_config: { + fuzzylist: [], + allowlist: [], + blocklist: [], + name: ListNames.MetaMask, + }, + phishfort_hotlist: { + blocklist: [], + name: ListNames.Phishfort, + }, +}; + +const emptyHtmlPage = (blockProvider) => ` + + + + title + + + Empty page by ${blockProvider} + +`; + +/** + * Setup fetch mocks for the phishing detection feature. + * + * The mock configuration will show that "127.0.0.1" is blocked. The dynamic lookup on the warning + * page can be customized, so that we can test both the MetaMask and PhishFort block cases. + * + * @param {import('mockttp').Mockttp} mockServer - The mock server. + * @param {object} mockPhishingConfigResponseConfig - The response for the dynamic phishing + * @param {number} mockPhishingConfigResponseConfig.statusCode - The status code for the response. + * @param {string[]} mockPhishingConfigResponseConfig.blocklist - The blocklist for the response. + * @param {BlockProvider} mockPhishingConfigResponseConfig.blockProvider - The name of the provider who blocked the page. + * configuration lookup performed by the warning page. + */ +async function setupPhishingDetectionMocks( + mockServer, + { + statusCode = 200, + blocklist = ['127.0.0.1'], + blockProvider = BlockProvider.MetaMask, + }, +) { + const blockProviderConfig = resolveProviderConfigName(blockProvider); + + const response = + statusCode >= 400 + ? { statusCode } + : { + statusCode, + json: { + data: { + ...defaultStalelist, + [blockProviderConfig]: { + ...defaultStalelist[blockProviderConfig], + blocklist, + }, + }, + }, + }; + + await mockServer.forGet(METAMASK_STALELIST_URL).thenCallback(() => { + return response; + }); + + await mockServer + .forGet(`${METAMASK_HOTLIST_DIFF_URL}/${lastUpdated}`) + .thenCallback(() => { + return { + statusCode: 200, + json: defaultHotlist, + }; + }); + + await mockServer + .forGet('https://github.com/MetaMask/eth-phishing-detect/issues/new') + .thenCallback(() => { + return { + statusCode: 200, + body: emptyHtmlPage(blockProvider), + }; + }); + + await mockServer + .forGet('https://github.com/phishfort/phishfort-lists/issues/new') + .thenCallback(() => { + return { + statusCode: 200, + body: emptyHtmlPage(blockProvider), + }; + }); +} + +/** + * Mocks the request made from the phishing warning page to check eth-phishing-detect + * + * @param {*} mockServer + * @param {*} metamaskPhishingConfigResponse + */ +async function mockConfigLookupOnWarningPage( + mockServer, + metamaskPhishingConfigResponse, +) { + await mockServer + .forGet( + 'https://raw.githubusercontent.com/MetaMask/eth-phishing-detect/master/src/config.json', + ) + .thenCallback(() => metamaskPhishingConfigResponse); +} + +/** + * Setup fallback mocks for default behaviour of the phishing detection feature. + * + * This sets up default mocks for a mockttp server when included in test/e2e/mock-e2e.js + * + * @param {import('mockttp').Mockttp} mockServer - The mock server. + */ + +async function mockEmptyStalelistAndHotlist(mockServer) { + await mockServer.forGet(METAMASK_STALELIST_URL).thenCallback(() => { + return { + statusCode: 200, + json: { ...defaultStalelist }, + }; + }); + + await mockServer + .forGet(`${METAMASK_HOTLIST_DIFF_URL}/${lastUpdated}`) + .thenCallback(() => { + return { + statusCode: 200, + json: defaultHotlist, + }; + }); +} + +/** + * + * @param {BlockProvider} providerName - The name of the provider who issued the block. + * @returns {string} The name of the phishing config in the response. + */ +function resolveProviderConfigName(providerName) { + switch (providerName.toLowerCase()) { + case BlockProvider.MetaMask: + return 'eth_phishing_detect_config'; + case BlockProvider.PhishFort: + return 'phishfort_hotlist'; + default: + throw new Error('Provider name must either be metamask or phishfort'); + } +} + +module.exports = { + setupPhishingDetectionMocks, + mockEmptyStalelistAndHotlist, + mockConfigLookupOnWarningPage, +}; diff --git a/test/e2e/tests/phishing-detection.spec.js b/test/e2e/tests/phishing-controller/phishing-detection.spec.js similarity index 69% rename from test/e2e/tests/phishing-detection.spec.js rename to test/e2e/tests/phishing-controller/phishing-detection.spec.js index 735d2bfb2..2e29a2411 100644 --- a/test/e2e/tests/phishing-detection.spec.js +++ b/test/e2e/tests/phishing-controller/phishing-detection.spec.js @@ -1,12 +1,17 @@ const { strict: assert } = require('assert'); + +const { convertToHexValue, withFixtures, openDapp } = require('../../helpers'); +const FixtureBuilder = require('../../fixture-builder'); +const { + METAMASK_HOTLIST_DIFF_URL, + METAMASK_STALELIST_URL, + BlockProvider, +} = require('./helpers'); + const { - convertToHexValue, - withFixtures, - openDapp, setupPhishingDetectionMocks, - mockPhishingDetection, -} = require('../helpers'); -const FixtureBuilder = require('../fixture-builder'); + mockConfigLookupOnWarningPage, +} = require('./mocks'); describe('Phishing Detection', function () { const ganacheOptions = { @@ -19,13 +24,32 @@ describe('Phishing Detection', function () { ], }; + describe('Phishing Detection Mock', function () { + it('should be updated to use v1 of the API', function () { + // Update the fixture in phishing-controller/mocks.js if this test fails + assert.equal( + METAMASK_STALELIST_URL, + 'https://phishing-detection.metafi.codefi.network/v1/stalelist', + ); + assert.equal( + METAMASK_HOTLIST_DIFF_URL, + 'https://phishing-detection.metafi.codefi.network/v1/diffsSince', + ); + }); + }); + it('should display the MetaMask Phishing Detection page and take the user to the blocked page if they continue', async function () { await withFixtures( { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + }, dapp: true, failOnConsoleError: false, }, @@ -44,12 +68,20 @@ describe('Phishing Detection', function () { }); it('should display the MetaMask Phishing Detection page in an iframe and take the user to the blocked page if they continue', async function () { + const DAPP_WITH_IFRAMED_PAGE_ON_BLOCKLIST = 'http://localhost:8080/'; + const IFRAMED_HOSTNAME = '127.0.0.1'; + await withFixtures( { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: [IFRAMED_HOSTNAME], + }); + }, dapp: true, dappPaths: ['mock-page-with-iframe'], dappOptions: { @@ -61,7 +93,7 @@ describe('Phishing Detection', function () { await driver.navigate(); await driver.fill('#password', 'correct horse battery staple'); await driver.press('#password', driver.Key.ENTER); - await driver.openNewPage('http://localhost:8080/'); + await driver.openNewPage(DAPP_WITH_IFRAMED_PAGE_ON_BLOCKLIST); const iframe = await driver.findElement('iframe'); @@ -85,7 +117,12 @@ describe('Phishing Detection', function () { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + }, dapp: true, dappPaths: ['mock-page-with-disallowed-iframe'], dappOptions: { @@ -125,7 +162,11 @@ describe('Phishing Detection', function () { ganacheOptions, title: this.test.title, testSpecificMock: (mockServer) => { - setupPhishingDetectionMocks(mockServer, { statusCode: 500 }); + setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + mockConfigLookupOnWarningPage(mockServer, { statusCode: 500 }); }, dapp: true, failOnConsoleError: false, @@ -139,7 +180,9 @@ describe('Phishing Detection', function () { await driver.clickElement({ text: 'report a detection problem.' }); // wait for page to load before checking URL. - await driver.findElement({ text: 'Empty page' }); + await driver.findElement({ + text: `Empty page by ${BlockProvider.MetaMask}`, + }); assert.equal( await driver.getCurrentUrl(), `https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20127.0.0.1&body=http%3A%2F%2F127.0.0.1%3A8080%2F`, @@ -149,50 +192,18 @@ describe('Phishing Detection', function () { }); it('should navigate the user to eth-phishing-detect to dispute a block from MetaMask', async function () { + // Must be site on actual eth-phishing-detect blocklist + const phishingSite = new URL('https://test.metamask-phishing.io'); + await withFixtures( { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, - dapp: true, - failOnConsoleError: false, - }, - async ({ driver }) => { - await driver.navigate(); - await driver.fill('#password', 'correct horse battery staple'); - await driver.press('#password', driver.Key.ENTER); - await openDapp(driver); - - await driver.clickElement({ text: 'report a detection problem.' }); - - // wait for page to load before checking URL. - await driver.findElement({ text: 'Empty page' }); - assert.equal( - await driver.getCurrentUrl(), - `https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20127.0.0.1&body=http%3A%2F%2F127.0.0.1%3A8080%2F`, - ); - }, - ); - }); - - it('should navigate the user to PhishFort to dispute a block from MetaMask', async function () { - await withFixtures( - { - fixtures: new FixtureBuilder().build(), - ganacheOptions, - title: this.test.title, - testSpecificMock: (mockServer) => { - setupPhishingDetectionMocks(mockServer, { - statusCode: 200, - json: { - version: 2, - tolerance: 2, - fuzzylist: [], - whitelist: [], - blacklist: [], - lastUpdated: 0, - }, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: [phishingSite.hostname], }); }, dapp: true, @@ -202,12 +213,51 @@ describe('Phishing Detection', function () { await driver.navigate(); await driver.fill('#password', 'correct horse battery staple'); await driver.press('#password', driver.Key.ENTER); - await openDapp(driver); + await driver.openNewPage(phishingSite.href); await driver.clickElement({ text: 'report a detection problem.' }); // wait for page to load before checking URL. - await driver.findElement({ text: 'Empty page' }); + await driver.findElement({ + text: `Empty page by ${BlockProvider.MetaMask}`, + }); + assert.equal( + await driver.getCurrentUrl(), + `https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20${encodeURIComponent( + phishingSite.hostname, + )}&body=${encodeURIComponent(phishingSite.href)}`, + ); + }, + ); + }); + + it('should navigate the user to PhishFort to dispute a Phishfort Block', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions, + title: this.test.title, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.PhishFort, + blocklist: ['127.0.0.1'], + }); + }, + dapp: true, + failOnConsoleError: false, + }, + async ({ driver }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + await driver.openNewPage('http://127.0.0.1:8080'); + + await driver.clickElement({ text: 'report a detection problem.' }); + + // wait for page to load before checking URL. + await driver.findElement({ + text: `Empty page by ${BlockProvider.PhishFort}`, + }); assert.equal( await driver.getCurrentUrl(), `https://github.com/phishfort/phishfort-lists/issues/new?title=[Legitimate%20Site%20Blocked]%20127.0.0.1&body=http%3A%2F%2F127.0.0.1%3A8080%2F`, @@ -222,7 +272,12 @@ describe('Phishing Detection', function () { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + }, dapp: true, dappPaths: ['mock-page-with-disallowed-iframe'], dappOptions: { diff --git a/yarn.lock b/yarn.lock index 5bd48d075..2d7aa26eb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3909,7 +3909,7 @@ __metadata: languageName: node linkType: hard -"@metamask/controller-utils@npm:^3.0.0, @metamask/controller-utils@npm:^3.4.0": +"@metamask/controller-utils@npm:^3.0.0, @metamask/controller-utils@npm:^3.1.0, @metamask/controller-utils@npm:^3.4.0": version: 3.4.0 resolution: "@metamask/controller-utils@npm:3.4.0" dependencies: @@ -4494,16 +4494,16 @@ __metadata: languageName: node linkType: hard -"@metamask/phishing-controller@npm:^3.0.0": - version: 3.0.0 - resolution: "@metamask/phishing-controller@npm:3.0.0" +"@metamask/phishing-controller@npm:^4.0.0": + version: 4.0.0 + resolution: "@metamask/phishing-controller@npm:4.0.0" dependencies: "@metamask/base-controller": ^2.0.0 - "@metamask/controller-utils": ^3.0.0 + "@metamask/controller-utils": ^3.1.0 "@types/punycode": ^2.1.0 eth-phishing-detect: ^1.2.0 punycode: ^2.1.1 - checksum: b0b9a86cba1928f0fd22a2aed196d75dc19a5e56547efe1b533d7ae06eaaf9432a6ee5004a8fd477f52310b50c2f3635a1e70ac83e3670f4cc6a1f488a674d73 + checksum: 15de581f7bec21d75531167275c68d7bbeae7fdaad02268749ba0a71c4d3ccb53718d963d6583e90c337407f65b7fcc9a89eb76c6f731802c2668a8425d5df89 languageName: node linkType: hard @@ -24273,7 +24273,7 @@ __metadata: "@metamask/notification-controller": ^3.0.0 "@metamask/obs-store": ^8.1.0 "@metamask/permission-controller": ^4.0.0 - "@metamask/phishing-controller": ^3.0.0 + "@metamask/phishing-controller": ^4.0.0 "@metamask/phishing-warning": ^2.1.0 "@metamask/post-message-stream": ^6.0.0 "@metamask/ppom-validator": ^0.1.2