Merge branch 'develop' into feat/mmassets-362_update-polygon-native-t…

…oken-POL

app/scripts/background.js

@@ -261,18 +261,35 @@ function maybeDetectPhishing(theController) {
       }
 
       theController.phishingController.maybeUpdateState();
-      const phishingTestResponse =
-        theController.phishingController.test(hostname);
-      if (!phishingTestResponse?.result) {
+      const phishingTestResponse = theController.phishingController.test(
+        details.url,
+      );
+
+      const blockedRequestResponse =
+        theController.phishingController.isBlockedRequest(details.url);
+
+      // if the request is not blocked, and the phishing test is not blocked, return and don't show the phishing screen
+      if (!phishingTestResponse?.result && !blockedRequestResponse.result) {
         return {};
       }
 
+      // Determine the block reason based on the type
+      let blockReason;
+      if (phishingTestResponse?.result && blockedRequestResponse.result) {
+        blockReason = `${phishingTestResponse.type} and ${blockedRequestResponse.type}`;
+      } else if (phishingTestResponse?.result) {
+        blockReason = phishingTestResponse.type;
+      } else {
+        blockReason = blockedRequestResponse.type;
+      }
+
       theController.metaMetricsController.trackEvent({
         // should we differentiate between background redirection and content script redirection?
         event: MetaMetricsEventName.PhishingPageDisplayed,
         category: MetaMetricsEventCategory.Phishing,
         properties: {
           url: hostname,
+          reason: blockReason,
         },
       });
       const querystring = new URLSearchParams({ hostname, href });
@@ -297,7 +314,10 @@ function maybeDetectPhishing(theController) {
       redirectTab(details.tabId, redirectHref);
       return {};
     },
-    { types: ['main_frame', 'sub_frame'], urls: ['http://*/*', 'https://*/*'] },
+    {
+      types: ['main_frame', 'sub_frame', 'xmlhttprequest'],
+      urls: ['http://*/*', 'https://*/*'],
+    },
     isManifestV2 ? ['blocking'] : [],
   );
 }

app/scripts/lib/snap-keyring/utils/isBlockedUrl.test.ts

@@ -10,17 +10,24 @@ describe('isBlockedUrl', () => {
     allowedEvents: [],
   });
   const phishingController = new PhishingController({
+    // @ts-expect-error TODO: Resolve/patch mismatch between messenger types
     messenger: phishingControllerMessenger,
     state: {
       phishingLists: [
         {
-          blocklist: ['https://metamask.test'],
+          blocklist: [
+            'metamask.test',
+            'QmYwAPJzv5CZsnAzt8auVTL6aKqgfZY5vHBYdbyz4ySxTm',
+            'ipfs://QmXbVAkGZMz6p8nJ3wXBng4JwvBqZWkFwnDMevL7Tz5w8y',
+            'QmT78zSuBmuS4z925WJg3vNLRiT4Mj6apb5iS4iykKs4n8',
+          ],
           allowlist: [],
           fuzzylist: [],
           tolerance: 0,
           version: 1,
           lastUpdated: 0,
           name: ListNames.MetaMask,
+          c2DomainBlocklist: [],
         },
       ],
     },
@@ -32,6 +39,16 @@ describe('isBlockedUrl', () => {
     ['https://metamask.io', false],
     ['https://metamask.test', true],
     ['sftp://metamask.io', true],
+    ['ipfs://QmYwAPJzv5CZsnAzt8auVTL6aKqgfZY5vHBYdbyz4ySxTm', true],
+    ['ipfs://QmXbVAkGZMz6p8nJ3wXBng4JwvBqZWkFgnDMevL7Tz5w8y', true],
+    [
+      'https://ipfs.io/ipfs/QmT78zSuBmuS4z925WJg3vNLRiT4Mj6apb5iS4iykKs4n8',
+      true,
+    ],
+    [
+      'https://ipfs.io/ipfs/QmT78zSuBmuS4zdsf925WJsadfsdfg3vNLRiT4Mj6apb5iS4iykKs4n8',
+      false,
+    ],
     ['', true],
     ['1', true],
     [undefined, true],

app/scripts/metamask-controller.actions.test.js

@@ -89,10 +89,6 @@ describe('MetaMaskController', function () {
             blocklist: ['127.0.0.1'],
             name: ListNames.MetaMask,
           },
-          phishfort_hotlist: {
-            blocklist: [],
-            name: ListNames.Phishfort,
-          },
         }),
       )
       .get(METAMASK_HOTLIST_DIFF_FILE)

app/scripts/metamask-controller.js

@@ -2682,7 +2682,7 @@ export default class MetamaskController extends EventEmitter {
               'PhishingController:maybeUpdateState',
             );
           },
-          isOnPhishingList: (origin) => {
+          isOnPhishingList: (sender) => {
             const { usePhishDetect } =
               this.preferencesController.store.getState();
 
@@ -2692,7 +2692,7 @@ export default class MetamaskController extends EventEmitter {
 
             return this.controllerMessenger.call(
               'PhishingController:testOrigin',
-              origin,
+              sender.url,
             ).result;
           },
           createInterface: this.controllerMessenger.call.bind(
@@ -4999,7 +4999,7 @@ export default class MetamaskController extends EventEmitter {
           const { hostname } = new URL(sender.url);
           this.phishingController.maybeUpdateState();
           // Check if new connection is blocked if phishing detection is on
-          const phishingTestResponse = this.phishingController.test(hostname);
+          const phishingTestResponse = this.phishingController.test(sender.url);
           if (phishingTestResponse?.result) {
             this.sendPhishingWarning(connectionStream, hostname);
             this.metaMetricsController.trackEvent({

app/scripts/metamask-controller.test.js

@@ -323,10 +323,6 @@ describe('MetaMaskController', () => {
             blocklist: ['test.metamask-phishing.io'],
             name: ListNames.MetaMask,
           },
-          phishfort_hotlist: {
-            blocklist: [],
-            name: ListNames.Phishfort,
-          },
         }),
       )
       .get(METAMASK_HOTLIST_DIFF_FILE)

app/scripts/migrations/121.1.test.ts

@@ -1,7 +1,14 @@
 import { AccountsControllerState } from '@metamask/accounts-controller';
+import { cloneDeep } from 'lodash';
 import { createMockInternalAccount } from '../../../test/jest/mocks';
 import { migrate, version } from './121.1';
 
+const sentryCaptureExceptionMock = jest.fn();
+
+global.sentry = {
+  captureException: sentryCaptureExceptionMock,
+};
+
 const oldVersion = 121;
 
 const mockInternalAccount = createMockInternalAccount();
@@ -25,7 +32,8 @@ describe('migration #121.1', () => {
       },
     };
 
-    const newStorage = await migrate(oldStorage);
+    const newStorage = await migrate(cloneDeep(oldStorage));
+
     expect(newStorage.meta).toStrictEqual({ version });
   });
 
@@ -43,14 +51,15 @@ describe('migration #121.1', () => {
       },
     };
 
-    const newStorage = await migrate(oldStorage);
-    const {
-      internalAccounts: { selectedAccount },
-    } = newStorage.data.AccountsController as AccountsControllerState;
-    expect(selectedAccount).toStrictEqual(mockInternalAccount.id);
-    expect(newStorage.data.AccountsController).toStrictEqual(
-      mockAccountsControllerState,
-    );
+    const newStorage = await migrate(cloneDeep(oldStorage));
+
+    expect(newStorage.data.AccountsController).toStrictEqual({
+      ...mockAccountsControllerState,
+      internalAccounts: {
+        ...mockAccountsControllerState.internalAccounts,
+        selectedAccount: mockInternalAccount.id,
+      },
+    });
   });
 
   it('does nothing if the selectedAccount is found in the list of accounts', async () => {
@@ -61,9 +70,169 @@ describe('migration #121.1', () => {
       },
     };
 
-    const newStorage = await migrate(oldStorage);
-    expect(newStorage.data.AccountsController).toStrictEqual(
-      mockAccountsControllerState,
-    );
+    const newStorage = await migrate(cloneDeep(oldStorage));
+
+    expect(newStorage.data).toStrictEqual(oldStorage.data);
+  });
+
+  it('does nothing if AccountsController state is missing', async () => {
+    const oldStorage = {
+      meta: { version: oldVersion },
+      data: {
+        OtherController: {},
+      },
+    };
+
+    const newStorage = await migrate(cloneDeep(oldStorage));
+
+    expect(newStorage.data).toStrictEqual(oldStorage.data);
+  });
+
+  it('does nothing if there are no accounts', async () => {
+    const oldStorage = {
+      meta: { version: oldVersion },
+      data: {
+        AccountsController: {
+          ...mockAccountsControllerState,
+          internalAccounts: {
+            ...mockAccountsControllerState.internalAccounts,
+            accounts: {},
+          },
+        },
+      },
+    };
+
+    const newStorage = await migrate(cloneDeep(oldStorage));
+
+    expect(newStorage.data).toStrictEqual(oldStorage.data);
+  });
+
+  it('does nothing if selectedAccount is unset', async () => {
+    const oldStorage = {
+      meta: { version: oldVersion },
+      data: {
+        AccountsController: {
+          ...mockAccountsControllerState,
+          internalAccounts: {
+            ...mockAccountsControllerState.internalAccounts,
+            selectedAccount: '',
+          },
+        },
+      },
+    };
+
+    const newStorage = await migrate(cloneDeep(oldStorage));
+
+    expect(newStorage.data).toStrictEqual(oldStorage.data);
   });
+
+  const invalidState = [
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController state of type 'string'`,
+      label: 'AccountsController type',
+      state: { AccountsController: 'invalid' },
+    },
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController state, missing internalAccounts`,
+      label: 'Missing internalAccounts',
+      state: { AccountsController: {} },
+    },
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts state of type 'string'`,
+      label: 'Invalid internalAccounts',
+      state: { AccountsController: { internalAccounts: 'invalid' } },
+    },
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts state, missing selectedAccount`,
+      label: 'Missing selectedAccount',
+      state: { AccountsController: { internalAccounts: { accounts: {} } } },
+    },
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.selectedAccount state of type 'object'`,
+      label: 'Invalid selectedAccount',
+      state: {
+        AccountsController: {
+          internalAccounts: { accounts: {}, selectedAccount: {} },
+        },
+      },
+    },
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts state, missing accounts`,
+      label: 'Missing accounts',
+      state: {
+        AccountsController: {
+          internalAccounts: { selectedAccount: '' },
+        },
+      },
+    },
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.accounts state of type 'string'`,
+      label: 'Missing accounts',
+      state: {
+        AccountsController: {
+          internalAccounts: { accounts: 'invalid', selectedAccount: '' },
+        },
+      },
+    },
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.accounts state, entry found of type 'string'`,
+      label: 'Account entry type',
+      state: {
+        AccountsController: {
+          internalAccounts: {
+            accounts: { [mockInternalAccount.id]: 'invalid' },
+            selectedAccount: 'unknown id',
+          },
+        },
+      },
+    },
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.accounts state, entry found that is missing an id`,
+      label: 'Account entry missing ID',
+      state: {
+        AccountsController: {
+          internalAccounts: {
+            accounts: { [mockInternalAccount.id]: {} },
+            selectedAccount: 'unknown id',
+          },
+        },
+      },
+    },
+    {
+      errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.accounts state, entry found with an id of type 'object'`,
+      label: 'Account entry missing ID',
+      state: {
+        AccountsController: {
+          internalAccounts: {
+            accounts: { [mockInternalAccount.id]: { id: {} } },
+            selectedAccount: 'unknown id',
+          },
+        },
+      },
+    },
+  ];
+
+  // @ts-expect-error 'each' function missing from type definitions, but it does exist
+  it.each(invalidState)(
+    'captures error when state is invalid due to: $label',
+    async ({
+      errorMessage,
+      state,
+    }: {
+      errorMessage: string;
+      state: Record<string, unknown>;
+    }) => {
+      const oldStorage = {
+        meta: { version: oldVersion },
+        data: state,
+      };
+
+      const newStorage = await migrate(cloneDeep(oldStorage));
+
+      expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(
+        new Error(errorMessage),
+      );
+      expect(newStorage.data).toStrictEqual(oldStorage.data);
+    },
+  );
 });