fix(settings): fixed two IPFS gateway issues (#19700)

* fix(settings): fixed two IPFS gateway issues

- adds back in two bugfixes that were originally in #19283
  - fixes #16871
  - fixes #18140

- achieves 100% code coverage for /ui/pages/settings/security-tab
- removes the npm package `valid-url`, which has not been updated in 10 years

* changes after #20172 was merged

* improved URL validation (specifically spaces)

* better Jest coverage

* response to legobeat review

* fixing lint and Jest

app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js

@@ -1,16 +1,16 @@
-import { ethErrors, errorCodes } from 'eth-rpc-errors';
-import validUrl from 'valid-url';
-import { omit } from 'lodash';
 import { ApprovalType } from '@metamask/controller-utils';
+import { errorCodes, ethErrors } from 'eth-rpc-errors';
+import { omit } from 'lodash';
 import {
   MESSAGE_TYPE,
   UNKNOWN_TICKER_SYMBOL,
 } from '../../../../../shared/constants/app';
+import { MetaMetricsNetworkEventSource } from '../../../../../shared/constants/metametrics';
 import {
   isPrefixedFormattedHexString,
   isSafeChainId,
 } from '../../../../../shared/modules/network.utils';
-import { MetaMetricsNetworkEventSource } from '../../../../../shared/constants/metametrics';
+import { getValidUrl } from '../../util';
 
 const addEthereumChain = {
   methodNames: [MESSAGE_TYPE.ADD_ETHEREUM_CHAIN],
@@ -83,27 +83,25 @@ async function addEthereumChainHandler(
     );
   }
 
-  const isLocalhost = (strUrl) => {
+  function isLocalhostOrHttps(urlString) {
-    try {
+    const url = getValidUrl(urlString);
-      const url = new URL(strUrl);
+
-      return url.hostname === 'localhost' || url.hostname === '127.0.0.1';
+    return (
-    } catch (error) {
+      url !== null &&
-      return false;
+      (url.hostname === 'localhost' ||
-    }
+        url.hostname === '127.0.0.1' ||
-  };
+        url.protocol === 'https:')
+    );
+  }
 
   const firstValidRPCUrl = Array.isArray(rpcUrls)
-    ? rpcUrls.find(
+    ? rpcUrls.find((rpcUrl) => isLocalhostOrHttps(rpcUrl))
-        (rpcUrl) => isLocalhost(rpcUrl) || validUrl.isHttpsUri(rpcUrl),
-      )
     : null;
 
   const firstValidBlockExplorerUrl =
     blockExplorerUrls !== null && Array.isArray(blockExplorerUrls)
-      ? blockExplorerUrls.find(
+      ? blockExplorerUrls.find((blockExplorerUrl) =>
-          (blockExplorerUrl) =>
+          isLocalhostOrHttps(blockExplorerUrl),
-            isLocalhost(blockExplorerUrl) ||
-            validUrl.isHttpsUri(blockExplorerUrl),
         )
       : null;
 

app/scripts/lib/util.test.js

@@ -1,24 +1,27 @@
-import { isPrefixedFormattedHexString } from '../../../shared/modules/network.utils';
 import {
-  ENVIRONMENT_TYPE_POPUP,
-  ENVIRONMENT_TYPE_NOTIFICATION,
-  ENVIRONMENT_TYPE_FULLSCREEN,
   ENVIRONMENT_TYPE_BACKGROUND,
-  PLATFORM_FIREFOX,
+  ENVIRONMENT_TYPE_FULLSCREEN,
-  PLATFORM_OPERA,
+  ENVIRONMENT_TYPE_NOTIFICATION,
+  ENVIRONMENT_TYPE_POPUP,
   PLATFORM_CHROME,
   PLATFORM_EDGE,
+  PLATFORM_FIREFOX,
+  PLATFORM_OPERA,
 } from '../../../shared/constants/app';
 import {
+  TransactionEnvelopeType,
   TransactionStatus,
   TransactionType,
-  TransactionEnvelopeType,
 } from '../../../shared/constants/transaction';
+import { isPrefixedFormattedHexString } from '../../../shared/modules/network.utils';
 import {
+  addUrlProtocolPrefix,
   deferredPromise,
+  formatTxMetaForRpcResult,
   getEnvironmentType,
   getPlatform,
-  formatTxMetaForRpcResult,
+  getValidUrl,
+  isWebUrl,
 } from './util';
 
 describe('app utils', () => {
@@ -73,6 +76,39 @@ describe('app utils', () => {
     });
   });
 
+  describe('URL utils', () => {
+    it('should test addUrlProtocolPrefix', () => {
+      expect(addUrlProtocolPrefix('http://example.com')).toStrictEqual(
+        'http://example.com',
+      );
+      expect(addUrlProtocolPrefix('https://example.com')).toStrictEqual(
+        'https://example.com',
+      );
+      expect(addUrlProtocolPrefix('example.com')).toStrictEqual(
+        'https://example.com',
+      );
+      expect(addUrlProtocolPrefix('exa mple.com')).toStrictEqual(null);
+    });
+
+    it('should test isWebUrl', () => {
+      expect(isWebUrl('http://example.com')).toStrictEqual(true);
+      expect(isWebUrl('https://example.com')).toStrictEqual(true);
+      expect(isWebUrl('https://exa mple.com')).toStrictEqual(false);
+      expect(isWebUrl('')).toStrictEqual(false);
+    });
+
+    it('should test getValidUrl', () => {
+      expect(getValidUrl('http://example.com').toString()).toStrictEqual(
+        'http://example.com/',
+      );
+      expect(getValidUrl('https://example.com').toString()).toStrictEqual(
+        'https://example.com/',
+      );
+      expect(getValidUrl('https://exa%20mple.com')).toStrictEqual(null);
+      expect(getValidUrl('')).toStrictEqual(null);
+    });
+  });
+
   describe('isPrefixedFormattedHexString', () => {
     it('should return true for valid hex strings', () => {
       expect(isPrefixedFormattedHexString('0x1')).toStrictEqual(true);

app/scripts/lib/util.ts

@@ -1,24 +1,24 @@
+import urlLib from 'url';
+import { AccessList } from '@ethereumjs/tx';
 import BN from 'bn.js';
 import { memoize } from 'lodash';
-import { AccessList } from '@ethereumjs/tx';
-import { CHAIN_IDS, TEST_CHAINS } from '../../../shared/constants/network';
-
 import {
-  ENVIRONMENT_TYPE_POPUP,
-  ENVIRONMENT_TYPE_NOTIFICATION,
-  ENVIRONMENT_TYPE_FULLSCREEN,
   ENVIRONMENT_TYPE_BACKGROUND,
-  PLATFORM_FIREFOX,
+  ENVIRONMENT_TYPE_FULLSCREEN,
-  PLATFORM_OPERA,
+  ENVIRONMENT_TYPE_NOTIFICATION,
+  ENVIRONMENT_TYPE_POPUP,
+  PLATFORM_BRAVE,
   PLATFORM_CHROME,
   PLATFORM_EDGE,
-  PLATFORM_BRAVE,
+  PLATFORM_FIREFOX,
+  PLATFORM_OPERA,
 } from '../../../shared/constants/app';
-import { stripHexPrefix } from '../../../shared/modules/hexstring-utils';
+import { CHAIN_IDS, TEST_CHAINS } from '../../../shared/constants/network';
 import {
   TransactionEnvelopeType,
   TransactionMeta,
 } from '../../../shared/constants/transaction';
+import { stripHexPrefix } from '../../../shared/modules/hexstring-utils';
 
 /**
  * @see {@link getEnvironmentType}
@@ -143,13 +143,13 @@ function checkAlarmExists(alarmList: { name: string }[], alarmName: string) {
 }
 
 export {
-  getPlatform,
-  getEnvironmentType,
-  hexToBn,
   BnMultiplyByFraction,
   addHexPrefix,
-  getChainType,
   checkAlarmExists,
+  getChainType,
+  getEnvironmentType,
+  getPlatform,
+  hexToBn,
 };
 
 // Taken from https://stackoverflow.com/a/1349426/3696652
@@ -235,10 +235,43 @@ export function previousValueComparator<A>(
 }
 
 export function addUrlProtocolPrefix(urlString: string) {
-  if (!urlString.match(/(^http:\/\/)|(^https:\/\/)/u)) {
+  let trimmed = urlString.trim();
-    return `https://${urlString}`;
+
+  if (trimmed.length && !urlLib.parse(trimmed).protocol) {
+    trimmed = `https://${trimmed}`;
   }
-  return urlString;
+
+  if (getValidUrl(trimmed) !== null) {
+    return trimmed;
+  }
+
+  return null;
+}
+
+export function getValidUrl(urlString: string): URL | null {
+  try {
+    const url = new URL(urlString);
+
+    if (url.hostname.length === 0 || url.pathname.length === 0) {
+      return null;
+    }
+
+    if (url.hostname !== decodeURIComponent(url.hostname)) {
+      return null; // will happen if there's a %, a space, or other invalid character in the hostname
+    }
+
+    return url;
+  } catch (error) {
+    return null;
+  }
+}
+
+export function isWebUrl(urlString: string): boolean {
+  const url = getValidUrl(urlString);
+
+  return (
+    url !== null && (url.protocol === 'https:' || url.protocol === 'http:')
+  );
 }
 
 interface FormattedTransactionMeta {

jest.config.js

@@ -8,7 +8,7 @@ module.exports = {
     '<rootDir>/app/scripts/controllers/transactions/EtherscanRemoteTransactionSource.ts',
     '<rootDir>/app/scripts/controllers/transactions/IncomingTransactionHelper.ts',
     '<rootDir>/app/scripts/flask/**/*.js',
-    '<rootDir>/app/scripts/lib/**/*.js',
+    '<rootDir>/app/scripts/lib/**/*.(js|ts)',
     '<rootDir>/app/scripts/lib/createRPCMethodTrackingMiddleware.js',
     '<rootDir>/app/scripts/migrations/*.js',
     '<rootDir>/app/scripts/migrations/*.ts',
@@ -48,8 +48,7 @@ module.exports = {
     '<rootDir>/app/scripts/controllers/sign.test.ts',
     '<rootDir>/app/scripts/controllers/decrypt-message.test.ts',
     '<rootDir>/app/scripts/flask/**/*.test.js',
-    '<rootDir>/app/scripts/lib/**/*.test.js',
+    '<rootDir>/app/scripts/lib/**/*.test.(js|ts)',
-    '<rootDir>/app/scripts/lib/**/*.test.ts',
     '<rootDir>/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js',
     '<rootDir>/app/scripts/migrations/*.test.(js|ts)',
     '<rootDir>/app/scripts/platforms/*.test.js',

package.json

@@ -362,7 +362,6 @@
     "single-call-balance-checker-abi": "^1.0.0",
     "unicode-confusables": "^0.1.1",
     "uuid": "^8.3.2",
-    "valid-url": "^1.0.9",
     "web3-stream-provider": "^4.0.0",
     "zxcvbn": "^4.4.2"
   },

test/data/mock-state.json

@@ -340,6 +340,8 @@
     "unapprovedTypedMessagesCount": 0,
     "useTokenDetection": true,
     "useCurrencyRateCheck": true,
+    "useNftDetection": true,
+    "openSeaEnabled": true,
     "advancedGasFee": {
       "maxBaseFee": "75",
       "priorityFee": "2"

ui/pages/keychains/reveal-seed.js

@@ -129,7 +129,7 @@ const RevealSeedPage = () => {
 
   const renderPasswordPromptContent = () => {
     return (
-      <form onSubmit={(event) => handleSubmit(event)}>
+      <form onSubmit={handleSubmit}>
         <Label htmlFor="password-box">{t('enterPasswordContinue')}</Label>
         <TextField
           inputProps={{

ui/pages/settings/networks-tab/networks-form/networks-form.js

@@ -1,3 +1,7 @@
+import classnames from 'classnames';
+import { isEqual } from 'lodash';
+import log from 'loglevel';
+import PropTypes from 'prop-types';
 import React, {
   useCallback,
   useContext,
@@ -6,12 +10,18 @@ import React, {
   useState,
 } from 'react';
 import { useDispatch } from 'react-redux';
-import PropTypes from 'prop-types';
+import { isWebUrl } from '../../../../../app/scripts/lib/util';
-import validUrl from 'valid-url';
+import {
-import log from 'loglevel';
+  MetaMetricsEventCategory,
-import classnames from 'classnames';
+  MetaMetricsEventName,
-import { isEqual } from 'lodash';
+  MetaMetricsNetworkEventSource,
-import { useI18nContext } from '../../../../hooks/useI18nContext';
+} from '../../../../../shared/constants/metametrics';
+import {
+  FEATURED_RPCS,
+  infuraProjectId,
+} from '../../../../../shared/constants/network';
+import fetchWithCache from '../../../../../shared/lib/fetch-with-cache';
+import { decimalToHex } from '../../../../../shared/modules/conversion.utils';
 import {
   isPrefixedFormattedHexString,
   isSafeChainId,
@@ -20,27 +30,17 @@ import { jsonRpcRequest } from '../../../../../shared/modules/rpc.utils';
 import ActionableMessage from '../../../../components/ui/actionable-message';
 import Button from '../../../../components/ui/button';
 import FormField from '../../../../components/ui/form-field';
-import {
-  setSelectedNetworkConfigurationId,
-  upsertNetworkConfiguration,
-  editAndSetNetworkConfiguration,
-  showModal,
-  setNewNetworkAdded,
-} from '../../../../store/actions';
-import fetchWithCache from '../../../../../shared/lib/fetch-with-cache';
-import { usePrevious } from '../../../../hooks/usePrevious';
-import {
-  MetaMetricsEventCategory,
-  MetaMetricsEventName,
-  MetaMetricsNetworkEventSource,
-} from '../../../../../shared/constants/metametrics';
-import {
-  infuraProjectId,
-  FEATURED_RPCS,
-} from '../../../../../shared/constants/network';
-import { decimalToHex } from '../../../../../shared/modules/conversion.utils';
 import { MetaMetricsContext } from '../../../../contexts/metametrics';
 import { getNetworkLabelKey } from '../../../../helpers/utils/i18n-helper';
+import { useI18nContext } from '../../../../hooks/useI18nContext';
+import { usePrevious } from '../../../../hooks/usePrevious';
+import {
+  editAndSetNetworkConfiguration,
+  setNewNetworkAdded,
+  setSelectedNetworkConfigurationId,
+  showModal,
+  upsertNetworkConfiguration,
+} from '../../../../store/actions';
 
 /**
  * Attempts to convert the given chainId to a decimal string, for display
@@ -74,11 +74,6 @@ const prefixChainId = (chainId) => {
   return prefixedChainId;
 };
 
-const isValidWhenAppended = (url) => {
-  const appendedRpc = `http://${url}`;
-  return validUrl.isWebUri(appendedRpc) && !url.match(/^https?:\/\/$/u);
-};
-
 const NetworksForm = ({
   addNewNetwork,
   restrictHeight,
@@ -208,23 +203,20 @@ const NetworksForm = ({
 
   const validateBlockExplorerURL = useCallback(
     (url) => {
-      if (!validUrl.isWebUri(url) && url !== '') {
+      if (url.length > 0 && !isWebUrl(url)) {
-        let errorKey;
+        if (isWebUrl(`https://${url}`)) {
-        let errorMessage;
+          return {
-
+            key: 'urlErrorMsg',
-        if (isValidWhenAppended(url)) {
+            msg: t('urlErrorMsg'),
-          errorKey = 'urlErrorMsg';
+          };
-          errorMessage = t('urlErrorMsg');
-        } else {
-          errorKey = 'invalidBlockExplorerURL';
-          errorMessage = t('invalidBlockExplorerURL');
         }
 
         return {
-          key: errorKey,
+          key: 'invalidBlockExplorerURL',
-          msg: errorMessage,
+          msg: t('invalidBlockExplorerURL'),
         };
       }
+
       return null;
     },
     [t],
@@ -407,7 +399,6 @@ const NetworksForm = ({
 
   const validateRPCUrl = useCallback(
     (url) => {
-      const isValidUrl = validUrl.isWebUri(url);
       const [
         {
           rpcUrl: matchingRPCUrl = null,
@@ -417,20 +408,16 @@ const NetworksForm = ({
       ] = networksToRender.filter((e) => e.rpcUrl === url);
       const { rpcUrl: selectedNetworkRpcUrl } = selectedNetwork;
 
-      if (!isValidUrl && url !== '') {
+      if (url.length > 0 && !isWebUrl(url)) {
-        let errorKey;
+        if (isWebUrl(`https://${url}`)) {
-        let errorMessage;
+          return {
-        if (isValidWhenAppended(url)) {
+            key: 'urlErrorMsg',
-          errorKey = 'urlErrorMsg';
+            msg: t('urlErrorMsg'),
-          errorMessage = t('urlErrorMsg');
+          };
-        } else {
-          errorKey = 'invalidRPC';
-          errorMessage = t('invalidRPC');
         }
-
         return {
-          key: errorKey,
+          key: 'invalidRPC',
-          msg: errorMessage,
+          msg: t('invalidRPC'),
         };
       } else if (matchingRPCUrl && matchingRPCUrl !== selectedNetworkRpcUrl) {
         return {

ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap

@@ -448,6 +448,7 @@ exports[`Security Tab should match snapshot 1`] = `
         </div>
         <div
           class="settings-page__content-item-col"
+          data-testid="ipfsToggle"
         >
           <label
             class="toggle-button toggle-button--on"
@@ -476,7 +477,7 @@ exports[`Security Tab should match snapshot 1`] = `
               <input
                 style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
                 type="checkbox"
-                value="dweb.link"
+                value="true"
               />
             </div>
             <div
@@ -786,54 +787,58 @@ exports[`Security Tab should match snapshot 1`] = `
           </div>
         </div>
         <div
-          class="settings-page__content-item-col"
+          class="settings-page__content-item"
-          data-testid="displayNftMedia"
         >
-          <label
+          <div
-            class="toggle-button toggle-button--off"
+            class="settings-page__content-item-col"
-            tabindex="0"
+            data-testid="enableOpenSeaAPI"
           >
-            <div
+            <label
-              style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
+              class="toggle-button toggle-button--on"
+              tabindex="0"
             >
               <div
-                style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(242, 244, 246);"
+                style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
               >
                 <div
-                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"
+                  style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(242, 244, 246);"
-                />
+                >
+                  <div
+                    style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 1; width: 26px; height: 20px; left: 4px;"
+                  />
+                  <div
+                    style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgba(255, 255, 255, 0.6); bottom: 0px; margin-top: auto; margin-bottom: auto; padding-right: 5px; line-height: 0; width: 26px; height: 20px; opacity: 0;"
+                  />
+                </div>
                 <div
-                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgba(255, 255, 255, 0.6); bottom: 0px; margin-top: auto; margin-bottom: auto; padding-right: 5px; line-height: 0; width: 26px; height: 20px; opacity: 1;"
+                  style="position: absolute; height: 100%; top: 0px; left: 0px; display: flex; flex: 1; align-self: stretch; align-items: center; justify-content: flex-start;"
+                >
+                  <div
+                    style="width: 18px; height: 18px; display: flex; align-self: center; box-shadow: none; border-radius: 50%; box-sizing: border-box; position: relative; background-color: rgb(3, 125, 214); left: 18px;"
+                  />
+                </div>
+                <input
+                  style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
+                  type="checkbox"
+                  value="true"
                 />
               </div>
               <div
-                style="position: absolute; height: 100%; top: 0px; left: 0px; display: flex; flex: 1; align-self: stretch; align-items: center; justify-content: flex-start;"
+                class="toggle-button__status"
               >
-                <div
+                <span
-                  style="width: 18px; height: 18px; display: flex; align-self: center; box-shadow: none; border-radius: 50%; box-sizing: border-box; position: relative; background-color: rgb(106, 115, 125); left: 3px;"
+                  class="toggle-button__label-off"
-                />
+                >
+                  Off
+                </span>
+                <span
+                  class="toggle-button__label-on"
+                >
+                  On
+                </span>
               </div>
-              <input
+            </label>
-                style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
+          </div>
-                type="checkbox"
-                value="false"
-              />
-            </div>
-            <div
-              class="toggle-button__status"
-            >
-              <span
-                class="toggle-button__label-off"
-              >
-                Off
-              </span>
-              <span
-                class="toggle-button__label-on"
-              >
-                On
-              </span>
-            </div>
-          </label>
         </div>
       </div>
       <div
@@ -878,7 +883,7 @@ exports[`Security Tab should match snapshot 1`] = `
           data-testid="useNftDetection"
         >
           <label
-            class="toggle-button toggle-button--off"
+            class="toggle-button toggle-button--on"
             tabindex="0"
           >
             <div
@@ -888,23 +893,23 @@ exports[`Security Tab should match snapshot 1`] = `
                 style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(242, 244, 246);"
               >
                 <div
-                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"
+                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 1; width: 26px; height: 20px; left: 4px;"
                 />
                 <div
-                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgba(255, 255, 255, 0.6); bottom: 0px; margin-top: auto; margin-bottom: auto; padding-right: 5px; line-height: 0; width: 26px; height: 20px; opacity: 1;"
+                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgba(255, 255, 255, 0.6); bottom: 0px; margin-top: auto; margin-bottom: auto; padding-right: 5px; line-height: 0; width: 26px; height: 20px; opacity: 0;"
                 />
               </div>
               <div
                 style="position: absolute; height: 100%; top: 0px; left: 0px; display: flex; flex: 1; align-self: stretch; align-items: center; justify-content: flex-start;"
               >
                 <div
-                  style="width: 18px; height: 18px; display: flex; align-self: center; box-shadow: none; border-radius: 50%; box-sizing: border-box; position: relative; background-color: rgb(106, 115, 125); left: 3px;"
+                  style="width: 18px; height: 18px; display: flex; align-self: center; box-shadow: none; border-radius: 50%; box-sizing: border-box; position: relative; background-color: rgb(3, 125, 214); left: 18px;"
                 />
               </div>
               <input
                 style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
                 type="checkbox"
-                value="false"
+                value="true"
               />
             </div>
             <div

ui/pages/settings/security-tab/security-tab.component.js

@@ -23,8 +23,8 @@ import {
 import SRPQuiz from '../../../components/app/srp-quiz-modal/SRPQuiz';
 import {
   BUTTON_SIZES,
-  Button,
   Box,
+  Button,
   Text,
 } from '../../../components/component-library';
 import TextField from '../../../components/ui/text-field';
@@ -76,10 +76,10 @@ export default class SecurityTab extends PureComponent {
   };
 
   state = {
-    ipfsGateway: this.props.ipfsGateway,
+    ipfsGateway: this.props.ipfsGateway || IPFS_DEFAULT_GATEWAY_URL,
     ipfsGatewayError: '',
     srpQuizModalVisible: false,
-    ipfsToggle: false,
+    ipfsToggle: this.props.ipfsGateway.length > 0,
   };
 
   settingsRefCounter = 0;
@@ -367,51 +367,40 @@ export default class SecurityTab extends PureComponent {
 
   renderIpfsGatewayControl() {
     const { t } = this.context;
-    const { ipfsGatewayError } = this.state;
+    let ipfsError = '';
-    const { useAddressBarEnsResolution, setUseAddressBarEnsResolution } =
-      this.props;
-
-    const handleIpfsGatewaySave = (gateway) => {
-      const url = gateway ? new URL(addUrlProtocolPrefix(gateway)) : '';
-      const { host } = url;
-
-      this.props.setIpfsGateway(host);
-    };
 
     const handleIpfsGatewayChange = (url) => {
-      this.setState(() => {
+      if (url.length > 0) {
-        let ipfsError = '';
-
         try {
-          const urlObj = new URL(addUrlProtocolPrefix(url));
+          const validUrl = addUrlProtocolPrefix(url);
-          if (!urlObj.host) {
+
-            throw new Error();
+          if (!validUrl) {
+            ipfsError = t('invalidIpfsGateway');
           }
 
+          const urlObj = new URL(validUrl);
+
           // don't allow the use of this gateway
           if (urlObj.host === 'gateway.ipfs.io') {
-            throw new Error('Forbidden gateway');
+            ipfsError = t('forbiddenIpfsGateway');
+          }
+
+          if (ipfsError.length === 0) {
+            this.props.setIpfsGateway(urlObj.host);
           }
         } catch (error) {
-          ipfsError =
+          ipfsError = t('invalidIpfsGateway');
-            error.message === 'Forbidden gateway'
-              ? t('forbiddenIpfsGateway')
-              : t('invalidIpfsGateway');
         }
+      } else {
+        ipfsError = t('invalidIpfsGateway');
+      }
 
-        handleIpfsGatewaySave(url);
+      this.setState({
-        return {
+        ipfsGateway: url,
-          ipfsGateway: url,
+        ipfsGatewayError: ipfsError,
-          ipfsGatewayError: ipfsError,
-        };
       });
     };
 
-    const handleIpfsToggle = (url) => {
-      url?.length < 1
-        ? handleIpfsGatewayChange(IPFS_DEFAULT_GATEWAY_URL)
-        : handleIpfsGatewayChange('');
-    };
     return (
       <Box
         ref={this.settingsRefs[6]}
@@ -426,27 +415,36 @@ export default class SecurityTab extends PureComponent {
             {t('ipfsGatewayDescription')}
           </div>
         </div>
-        <div className="settings-page__content-item-col">
+        <div
+          className="settings-page__content-item-col"
+          data-testid="ipfsToggle"
+        >
           <ToggleButton
-            value={this.state.ipfsGateway}
+            value={this.state.ipfsToggle}
             onToggle={(value) => {
-              handleIpfsToggle(value);
+              if (value) {
-              this.setState({ ipfsToggle: Boolean(value) });
+                // turning from true to false
+                this.props.setIpfsGateway('');
+              } else {
+                // turning from false to true
+                handleIpfsGatewayChange(this.state.ipfsGateway);
+              }
+
+              this.setState({ ipfsToggle: !value });
             }}
             offLabel={t('off')}
             onLabel={t('on')}
           />
         </div>
-        {!this.state.ipfsToggle && (
+        {this.state.ipfsToggle && (
           <div className="settings-page__content-item">
             <span>{t('addIPFSGateway')}</span>
             <div className="settings-page__content-item-col">
               <TextField
                 type="text"
-                disabled={!this.state.ipfsGateway}
                 value={this.state.ipfsGateway}
                 onChange={(e) => handleIpfsGatewayChange(e.target.value)}
-                error={ipfsGatewayError}
+                error={this.state.ipfsGatewayError}
                 fullWidth
                 margin="dense"
               />
@@ -508,8 +506,10 @@ export default class SecurityTab extends PureComponent {
             data-testid="ipfs-gateway-resolution-container"
           >
             <ToggleButton
-              value={useAddressBarEnsResolution}
+              value={this.props.useAddressBarEnsResolution}
-              onToggle={(value) => setUseAddressBarEnsResolution(!value)}
+              onToggle={(value) =>
+                this.props.setUseAddressBarEnsResolution(!value)
+              }
               offLabel={t('off')}
               onLabel={t('on')}
             />
@@ -696,31 +696,32 @@ export default class SecurityTab extends PureComponent {
             {t('displayNftMediaDescription')}
           </div>
         </div>
-
+        <div className="settings-page__content-item">
-        <div
+          <div
-          className="settings-page__content-item-col"
+            className="settings-page__content-item-col"
-          data-testid="displayNftMedia"
+            data-testid="enableOpenSeaAPI"
-        >
+          >
-          <ToggleButton
+            <ToggleButton
-            value={openSeaEnabled}
+              value={openSeaEnabled}
-            onToggle={(value) => {
+              onToggle={(value) => {
-              this.context.trackEvent({
+                this.context.trackEvent({
-                category: MetaMetricsEventCategory.Settings,
+                  category: MetaMetricsEventCategory.Settings,
-                event: 'Enabled/Disable OpenSea',
+                  event: 'Enabled/Disable OpenSea',
-                properties: {
+                  properties: {
-                  action: 'Enabled/Disable OpenSea',
+                    action: 'Enabled/Disable OpenSea',
-                  legacy_event: true,
+                    legacy_event: true,
-                },
+                  },
-              });
+                });
-              // value is positive when being toggled off
+                // value is positive when being toggled off
-              if (value && useNftDetection) {
+                if (value && useNftDetection) {
-                setUseNftDetection(false);
+                  setUseNftDetection(false);
-              }
+                }
-              setOpenSeaEnabled(!value);
+                setOpenSeaEnabled(!value);
-            }}
+              }}
-            offLabel={t('off')}
+              offLabel={t('off')}
-            onLabel={t('on')}
+              onLabel={t('on')}
-          />
+            />
+          </div>
         </div>
       </Box>
     );

ui/pages/settings/security-tab/security-tab.test.js

@@ -1,8 +1,12 @@
 import { fireEvent, queryByRole, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import React from 'react';
 import configureMockStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
+import { getEnvironmentType } from '../../../../app/scripts/lib/util';
+import { ENVIRONMENT_TYPE_POPUP } from '../../../../shared/constants/app';
 import mockState from '../../../../test/data/mock-state.json';
+import { tEn } from '../../../../test/lib/i18n-helpers';
 import { renderWithProvider } from '../../../../test/lib/render-helpers';
 import SecurityTab from './security-tab.container';
 
@@ -21,8 +25,10 @@ describe('Security Tab', () => {
 
   const mockStore = configureMockStore([thunk])(mockState);
 
-  function toggleCheckbox(testId, initialState) {
+  function toggleCheckbox(testId, initialState, skipRender = false) {
-    renderWithProvider(<SecurityTab />, mockStore);
+    if (!skipRender) {
+      renderWithProvider(<SecurityTab />, mockStore);
+    }
 
     const container = screen.getByTestId(testId);
     const checkbox = queryByRole(container, 'checkbox');
@@ -46,12 +52,31 @@ describe('Security Tab', () => {
     expect(container).toMatchSnapshot();
   });
 
-  it('toggles Display NFT media enabled', async () => {
+  it('toggles opensea api enabled off', async () => {
-    expect(await toggleCheckbox('displayNftMedia', false)).toBe(true);
+    expect(await toggleCheckbox('enableOpenSeaAPI', true)).toBe(true);
+  });
+
+  it('toggles opensea api enabled on', async () => {
+    mockState.metamask.openSeaEnabled = false;
+
+    const localMockStore = configureMockStore([thunk])(mockState);
+    renderWithProvider(<SecurityTab />, localMockStore);
+
+    expect(await toggleCheckbox('enableOpenSeaAPI', false, true)).toBe(true);
   });
 
   it('toggles nft detection', async () => {
-    expect(await toggleCheckbox('useNftDetection', false)).toBe(true);
+    expect(await toggleCheckbox('useNftDetection', true)).toBe(true);
+  });
+
+  it('toggles nft detection from another initial state', async () => {
+    mockState.metamask.openSeaEnabled = false;
+    mockState.metamask.useNftDetection = false;
+
+    const localMockStore = configureMockStore([thunk])(mockState);
+    renderWithProvider(<SecurityTab />, localMockStore);
+
+    expect(await toggleCheckbox('useNftDetection', false, true)).toBe(true);
   });
 
   it('toggles phishing detection', async () => {
@@ -103,4 +128,81 @@ describe('Security Tab', () => {
       screen.queryByTestId(`srp_stage_introduction`),
     ).not.toBeInTheDocument();
   });
+
+  it('sets IPFS gateway', async () => {
+    const user = userEvent.setup();
+    renderWithProvider(<SecurityTab />, mockStore);
+
+    const ipfsField = screen.getByDisplayValue(mockState.metamask.ipfsGateway);
+
+    await user.click(ipfsField);
+
+    await userEvent.clear(ipfsField);
+
+    expect(ipfsField).toHaveValue('');
+    expect(screen.queryByText(tEn('invalidIpfsGateway'))).toBeInTheDocument();
+    expect(
+      screen.queryByText(tEn('forbiddenIpfsGateway')),
+    ).not.toBeInTheDocument();
+
+    await userEvent.type(ipfsField, 'https://');
+
+    expect(ipfsField).toHaveValue('https://');
+    expect(screen.queryByText(tEn('invalidIpfsGateway'))).toBeInTheDocument();
+    expect(
+      screen.queryByText(tEn('forbiddenIpfsGateway')),
+    ).not.toBeInTheDocument();
+
+    await userEvent.type(ipfsField, '//');
+
+    expect(ipfsField).toHaveValue('https:////');
+    expect(screen.queryByText(tEn('invalidIpfsGateway'))).toBeInTheDocument();
+    expect(
+      screen.queryByText(tEn('forbiddenIpfsGateway')),
+    ).not.toBeInTheDocument();
+
+    await userEvent.clear(ipfsField);
+
+    await userEvent.type(ipfsField, 'gateway.ipfs.io');
+
+    expect(ipfsField).toHaveValue('gateway.ipfs.io');
+    expect(
+      screen.queryByText(tEn('invalidIpfsGateway')),
+    ).not.toBeInTheDocument();
+    expect(screen.queryByText(tEn('forbiddenIpfsGateway'))).toBeInTheDocument();
+  });
+
+  it('toggles IPFS gateway', async () => {
+    mockState.metamask.ipfsGateway = '';
+
+    const localMockStore = configureMockStore([thunk])(mockState);
+    renderWithProvider(<SecurityTab />, localMockStore);
+
+    expect(await toggleCheckbox('ipfsToggle', false, true)).toBe(true);
+    expect(await toggleCheckbox('ipfsToggle', true, true)).toBe(true);
+  });
+
+  it('toggles ENS domains in address bar', async () => {
+    expect(
+      await toggleCheckbox('ipfs-gateway-resolution-container', false),
+    ).toBe(true);
+  });
+
+  it('clicks "Add Custom Network"', async () => {
+    const user = userEvent.setup();
+    renderWithProvider(<SecurityTab />, mockStore);
+
+    // Test the default path where `getEnvironmentType() === undefined`
+    await user.click(screen.getByText(tEn('addCustomNetwork')));
+
+    // Now force it down the path where `getEnvironmentType() === ENVIRONMENT_TYPE_POPUP`
+    jest
+      .mocked(getEnvironmentType)
+      .mockImplementationOnce(() => ENVIRONMENT_TYPE_POPUP);
+
+    global.platform = { openExtensionInBrowser: jest.fn() };
+
+    await user.click(screen.getByText(tEn('addCustomNetwork')));
+    expect(global.platform.openExtensionInBrowser).toHaveBeenCalled();
+  });
 });

yarn.lock

@@ -24494,7 +24494,6 @@ __metadata:
     typescript: "npm:~4.4.0"
     unicode-confusables: "npm:^0.1.1"
     uuid: "npm:^8.3.2"
-    valid-url: "npm:^1.0.9"
     vinyl: "npm:^2.2.1"
     vinyl-buffer: "npm:^1.0.1"
     vinyl-source-stream: "npm:^2.0.0"
@@ -34288,13 +34287,6 @@ __metadata:
   languageName: node
   linkType: hard
 
-"valid-url@npm:^1.0.9":
-  version: 1.0.9
-  resolution: "valid-url@npm:1.0.9"
-  checksum: 343dfaf85eb3691dc8eb93f7bc007be1ee6091e6c6d1a68bf633cb85e4bf2930e34ca9214fb2c3330de5b652510b257a8ee1ff0a0a37df0925e9dabf93ee512d
-  languageName: node
-  linkType: hard
-
 "validate-npm-package-license@npm:^3.0.1":
   version: 3.0.4
   resolution: "validate-npm-package-license@npm:3.0.4"