From 9eee7dc155085aca92ace9d81522ecdd72c5bbce Mon Sep 17 00:00:00 2001 From: Howard Braham Date: Tue, 18 Jul 2023 13:44:08 -0700 Subject: [PATCH 1/6] 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 --- .../handlers/add-ethereum-chain.js | 21 +---- app/scripts/lib/util.ts | 76 +++++++++++---- package.json | 1 - ui/pages/keychains/reveal-seed.js | 2 +- .../networks-form/networks-form.js | 93 ++++++++----------- .../security-tab/security-tab.component.js | 47 +++++----- .../security-tab/security-tab.test.js | 65 +++++++++++++ yarn.lock | 8 -- 8 files changed, 194 insertions(+), 119 deletions(-) diff --git a/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js b/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js index 7c0c5af57b5b..9d80596a3cb5 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js @@ -1,5 +1,4 @@ import { ethErrors, errorCodes } from 'eth-rpc-errors'; -import validUrl from 'valid-url'; import { omit } from 'lodash'; import { ApprovalType } from '@metamask/controller-utils'; import { @@ -11,6 +10,7 @@ import { isSafeChainId, } from '../../../../../shared/modules/network.utils'; import { MetaMetricsNetworkEventSource } from '../../../../../shared/constants/metametrics'; +import { isLocalhostOrHttps } from '../../util'; const addEthereumChain = { methodNames: [MESSAGE_TYPE.ADD_ETHEREUM_CHAIN], @@ -83,27 +83,14 @@ async function addEthereumChainHandler( ); } - const isLocalhost = (strUrl) => { - try { - const url = new URL(strUrl); - return url.hostname === 'localhost' || url.hostname === '127.0.0.1'; - } catch (error) { - return false; - } - }; - const firstValidRPCUrl = Array.isArray(rpcUrls) - ? rpcUrls.find( - (rpcUrl) => isLocalhost(rpcUrl) || validUrl.isHttpsUri(rpcUrl), - ) + ? rpcUrls.find((rpcUrl) => isLocalhostOrHttps(rpcUrl)) : null; const firstValidBlockExplorerUrl = blockExplorerUrls !== null && Array.isArray(blockExplorerUrls) - ? blockExplorerUrls.find( - (blockExplorerUrl) => - isLocalhost(blockExplorerUrl) || - validUrl.isHttpsUri(blockExplorerUrl), + ? blockExplorerUrls.find((blockExplorerUrl) => + isLocalhostOrHttps(blockExplorerUrl), ) : null; diff --git a/app/scripts/lib/util.ts b/app/scripts/lib/util.ts index dc21a9608ae2..e6cf9edcce00 100644 --- a/app/scripts/lib/util.ts +++ b/app/scripts/lib/util.ts @@ -1,24 +1,23 @@ +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, - PLATFORM_OPERA, + ENVIRONMENT_TYPE_FULLSCREEN, + 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 +142,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 +234,53 @@ export function previousValueComparator( } export function addUrlProtocolPrefix(urlString: string) { - if (!urlString.match(/(^http:\/\/)|(^https:\/\/)/u)) { - return `https://${urlString}`; + let trimmed = urlString.trim(); + + if (trimmed.length > 0 && !trimmed.match(/(^http:\/\/)|(^https:\/\/)/u)) { + trimmed = `https://${trimmed}`; + } + + if (isValidUrl(trimmed)) { + return trimmed; + } + + return null; +} + +export function isLocalhostOrHttps(urlString: string) { + try { + const url = new URL(urlString); + return ( + url.hostname === 'localhost' || + url.hostname === '127.0.0.1' || + url.protocol === 'https:' + ); + } catch (error) { + return false; + } +} + +export function isValidUrl(urlString: string): boolean { + try { + const url = new URL(urlString); + + return url.hostname.length > 0 && url.pathname.length > 0; + } catch (error) { + return false; + } +} + +export function isWebUrl(urlString: string): boolean { + try { + const url = new URL(urlString); + return ( + url.hostname.length > 0 && + (url.protocol === 'https:' || url.protocol === 'http:') && + url.pathname.length > 0 + ); + } catch (error) { + return false; } - return urlString; } interface FormattedTransactionMeta { diff --git a/package.json b/package.json index 23f872073eda..42a6ba5066b7 100644 --- a/package.json +++ b/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" }, diff --git a/ui/pages/keychains/reveal-seed.js b/ui/pages/keychains/reveal-seed.js index 3dec64888f90..c2f0125fb098 100644 --- a/ui/pages/keychains/reveal-seed.js +++ b/ui/pages/keychains/reveal-seed.js @@ -129,7 +129,7 @@ const RevealSeedPage = () => { const renderPasswordPromptContent = () => { return ( -
handleSubmit(event)}> + { 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 !== '') { - let errorKey; - let errorMessage; - - if (isValidWhenAppended(url)) { - errorKey = 'urlErrorMsg'; - errorMessage = t('urlErrorMsg'); - } else { - errorKey = 'invalidBlockExplorerURL'; - errorMessage = t('invalidBlockExplorerURL'); + if (url.length > 0 && !isWebUrl(url)) { + if (isWebUrl(`https://${url}`)) { + return { + key: 'urlErrorMsg', + msg: t('urlErrorMsg'), + }; } return { - key: errorKey, - msg: errorMessage, + key: 'invalidBlockExplorerURL', + 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 !== '') { - let errorKey; - let errorMessage; - if (isValidWhenAppended(url)) { - errorKey = 'urlErrorMsg'; - errorMessage = t('urlErrorMsg'); - } else { - errorKey = 'invalidRPC'; - errorMessage = t('invalidRPC'); + if (url.length > 0 && !isWebUrl(url)) { + if (isWebUrl(`https://${url}`)) { + return { + key: 'urlErrorMsg', + msg: t('urlErrorMsg'), + }; } - return { - key: errorKey, - msg: errorMessage, + key: 'invalidRPC', + msg: t('invalidRPC'), }; } else if (matchingRPCUrl && matchingRPCUrl !== selectedNetworkRpcUrl) { return { diff --git a/ui/pages/settings/security-tab/security-tab.component.js b/ui/pages/settings/security-tab/security-tab.component.js index 00f75117d582..f3b27ea8cf85 100644 --- a/ui/pages/settings/security-tab/security-tab.component.js +++ b/ui/pages/settings/security-tab/security-tab.component.js @@ -367,9 +367,7 @@ export default class SecurityTab extends PureComponent { renderIpfsGatewayControl() { const { t } = this.context; - const { ipfsGatewayError } = this.state; - const { useAddressBarEnsResolution, setUseAddressBarEnsResolution } = - this.props; + let ipfsError = ''; const handleIpfsGatewaySave = (gateway) => { const url = gateway ? new URL(addUrlProtocolPrefix(gateway)) : ''; @@ -379,31 +377,34 @@ export default class SecurityTab extends PureComponent { }; const handleIpfsGatewayChange = (url) => { - this.setState(() => { - let ipfsError = ''; - + if (url.length > 0) { try { - const urlObj = new URL(addUrlProtocolPrefix(url)); - if (!urlObj.host) { - throw new Error(); + const validUrl = addUrlProtocolPrefix(url); + + 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 = - error.message === 'Forbidden gateway' - ? t('forbiddenIpfsGateway') - : t('invalidIpfsGateway'); + ipfsError = t('invalidIpfsGateway'); } + } else { + ipfsError = t('invalidIpfsGateway'); + } - handleIpfsGatewaySave(url); - return { - ipfsGateway: url, - ipfsGatewayError: ipfsError, - }; + this.setState({ + ipfsGateway: url, + ipfsGatewayError: ipfsError, }); }; @@ -446,7 +447,7 @@ export default class SecurityTab extends PureComponent { 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 +509,10 @@ export default class SecurityTab extends PureComponent { data-testid="ipfs-gateway-resolution-container" > setUseAddressBarEnsResolution(!value)} + value={this.props.useAddressBarEnsResolution} + onToggle={(value) => + this.props.setUseAddressBarEnsResolution(!value) + } offLabel={t('off')} onLabel={t('on')} /> diff --git a/ui/pages/settings/security-tab/security-tab.test.js b/ui/pages/settings/security-tab/security-tab.test.js index eb03f74fb513..201b68fd234b 100644 --- a/ui/pages/settings/security-tab/security-tab.test.js +++ b/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'; @@ -103,4 +107,65 @@ describe('Security Tab', () => { screen.queryByTestId(`srp_stage_introduction`), ).not.toBeInTheDocument(); }); + + it('sets IPFS gateway', async () => { + const user = userEvent.setup(); + renderWithProvider(, 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('clicks "Add Custom Network"', async () => { + const user = userEvent.setup(); + renderWithProvider(, 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(); + }); }); diff --git a/yarn.lock b/yarn.lock index 23b862842b97..a767c800f23d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -24539,7 +24539,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" @@ -34340,13 +34339,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" From 2a25c2aa56995f5400531f1a80a851846064e919 Mon Sep 17 00:00:00 2001 From: Howard Braham Date: Wed, 2 Aug 2023 12:09:34 -0700 Subject: [PATCH 2/6] changes after #20172 was merged --- .../__snapshots__/security-tab.test.js.snap | 2 +- .../security-tab/security-tab.component.js | 36 ++++++++----------- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap b/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap index f9e256234bb6..bd0e809fa7dd 100644 --- a/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap +++ b/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap @@ -476,7 +476,7 @@ exports[`Security Tab should match snapshot 1`] = `
0, }; settingsRefCounter = 0; @@ -369,13 +367,6 @@ export default class SecurityTab extends PureComponent { const { t } = this.context; let ipfsError = ''; - const handleIpfsGatewaySave = (gateway) => { - const url = gateway ? new URL(addUrlProtocolPrefix(gateway)) : ''; - const { host } = url; - - this.props.setIpfsGateway(host); - }; - const handleIpfsGatewayChange = (url) => { if (url.length > 0) { try { @@ -408,11 +399,6 @@ export default class SecurityTab extends PureComponent { }); }; - const handleIpfsToggle = (url) => { - url?.length < 1 - ? handleIpfsGatewayChange(IPFS_DEFAULT_GATEWAY_URL) - : handleIpfsGatewayChange(''); - }; return (
{ - handleIpfsToggle(value); - this.setState({ ipfsToggle: Boolean(value) }); + if (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')} />
- {!this.state.ipfsToggle && ( + {this.state.ipfsToggle && (
{t('addIPFSGateway')}
handleIpfsGatewayChange(e.target.value)} error={this.state.ipfsGatewayError} From 0950d3683869e33962a9b77cac40e2a9d18a7457 Mon Sep 17 00:00:00 2001 From: Howard Braham Date: Wed, 2 Aug 2023 21:13:37 -0700 Subject: [PATCH 3/6] improved URL validation (specifically spaces) --- app/scripts/lib/util.ts | 51 +++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/app/scripts/lib/util.ts b/app/scripts/lib/util.ts index e6cf9edcce00..b7c4b23872ae 100644 --- a/app/scripts/lib/util.ts +++ b/app/scripts/lib/util.ts @@ -240,47 +240,48 @@ export function addUrlProtocolPrefix(urlString: string) { trimmed = `https://${trimmed}`; } - if (isValidUrl(trimmed)) { + if (getValidUrl(trimmed) !== null) { return trimmed; } return null; } -export function isLocalhostOrHttps(urlString: string) { +export function getValidUrl(urlString: string): URL | null { try { const url = new URL(urlString); - return ( - url.hostname === 'localhost' || - url.hostname === '127.0.0.1' || - url.protocol === 'https:' - ); + + 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 false; + return null; } } -export function isValidUrl(urlString: string): boolean { - try { - const url = new URL(urlString); +export function isLocalhostOrHttps(urlString: string) { + const url = getValidUrl(urlString); - return url.hostname.length > 0 && url.pathname.length > 0; - } catch (error) { - return false; - } + return ( + url !== null && + (url.hostname === 'localhost' || + url.hostname === '127.0.0.1' || + url.protocol === 'https:') + ); } export function isWebUrl(urlString: string): boolean { - try { - const url = new URL(urlString); - return ( - url.hostname.length > 0 && - (url.protocol === 'https:' || url.protocol === 'http:') && - url.pathname.length > 0 - ); - } catch (error) { - return false; - } + const url = getValidUrl(urlString); + + return ( + url !== null && (url.protocol === 'https:' || url.protocol === 'http:') + ); } interface FormattedTransactionMeta { From aaddb3680232d83448d488471852c30004994c61 Mon Sep 17 00:00:00 2001 From: Howard Braham Date: Thu, 3 Aug 2023 00:08:23 -0700 Subject: [PATCH 4/6] better Jest coverage --- app/scripts/lib/util.test.js | 60 ++++++++++++++++--- jest.config.js | 5 +- test/data/mock-state.json | 2 + .../security-tab/security-tab.component.js | 2 +- .../security-tab/security-tab.test.js | 47 ++++++++++++++- 5 files changed, 101 insertions(+), 15 deletions(-) diff --git a/app/scripts/lib/util.test.js b/app/scripts/lib/util.test.js index c93e9f8e05f7..b1f4899a9598 100644 --- a/app/scripts/lib/util.test.js +++ b/app/scripts/lib/util.test.js @@ -1,24 +1,28 @@ -import { isPrefixedFormattedHexString } from '../../../shared/modules/network.utils'; import { - ENVIRONMENT_TYPE_POPUP, - ENVIRONMENT_TYPE_NOTIFICATION, - ENVIRONMENT_TYPE_FULLSCREEN, ENVIRONMENT_TYPE_BACKGROUND, - PLATFORM_FIREFOX, - PLATFORM_OPERA, + ENVIRONMENT_TYPE_FULLSCREEN, + 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, + isLocalhostOrHttps, + isWebUrl, } from './util'; describe('app utils', () => { @@ -73,6 +77,46 @@ 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 isLocalhostOrHttps', () => { + expect(isLocalhostOrHttps('http://example.com')).toStrictEqual(false); + expect(isLocalhostOrHttps('https://example.com')).toStrictEqual(true); + expect(isLocalhostOrHttps('http://localhost')).toStrictEqual(true); + expect(isLocalhostOrHttps('http://127.0.0.1')).toStrictEqual(true); + }); + + 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); diff --git a/jest.config.js b/jest.config.js index 9f4a2a858bc8..0e245e4f60b2 100644 --- a/jest.config.js +++ b/jest.config.js @@ -5,7 +5,7 @@ module.exports = { '/app/scripts/controllers/sign.ts', '/app/scripts/controllers/decrypt-message.ts', '/app/scripts/flask/**/*.js', - '/app/scripts/lib/**/*.js', + '/app/scripts/lib/**/*.(js|ts)', '/app/scripts/lib/createRPCMethodTrackingMiddleware.js', '/app/scripts/migrations/*.js', '/app/scripts/migrations/*.ts', @@ -42,8 +42,7 @@ module.exports = { '/app/scripts/controllers/sign.test.ts', '/app/scripts/controllers/decrypt-message.test.ts', '/app/scripts/flask/**/*.test.js', - '/app/scripts/lib/**/*.test.js', - '/app/scripts/lib/**/*.test.ts', + '/app/scripts/lib/**/*.test.(js|ts)', '/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js', '/app/scripts/migrations/*.test.(js|ts)', '/app/scripts/platforms/*.test.js', diff --git a/test/data/mock-state.json b/test/data/mock-state.json index 69db1fc0559a..ae58d5dba03d 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -340,6 +340,8 @@ "unapprovedTypedMessagesCount": 0, "useTokenDetection": true, "useCurrencyRateCheck": true, + "useNftDetection": true, + "openSeaEnabled": true, "advancedGasFee": { "maxBaseFee": "75", "priorityFee": "2" diff --git a/ui/pages/settings/security-tab/security-tab.component.js b/ui/pages/settings/security-tab/security-tab.component.js index 9fabe4c2da53..390edfd8eb43 100644 --- a/ui/pages/settings/security-tab/security-tab.component.js +++ b/ui/pages/settings/security-tab/security-tab.component.js @@ -413,7 +413,7 @@ export default class SecurityTab extends PureComponent { {t('ipfsGatewayDescription')}
-
+
{ diff --git a/ui/pages/settings/security-tab/security-tab.test.js b/ui/pages/settings/security-tab/security-tab.test.js index 201b68fd234b..56a89dbde28f 100644 --- a/ui/pages/settings/security-tab/security-tab.test.js +++ b/ui/pages/settings/security-tab/security-tab.test.js @@ -25,8 +25,10 @@ describe('Security Tab', () => { const mockStore = configureMockStore([thunk])(mockState); - function toggleCheckbox(testId, initialState) { - renderWithProvider(, mockStore); + function toggleCheckbox(testId, initialState, skipRender = false) { + if (!skipRender) { + renderWithProvider(, mockStore); + } const container = screen.getByTestId(testId); const checkbox = queryByRole(container, 'checkbox'); @@ -50,12 +52,35 @@ describe('Security Tab', () => { expect(container).toMatchSnapshot(); }); + it('toggles opensea api enabled off', async () => { + expect(await toggleCheckbox('enableOpenSeaAPI', true)).toBe(true); + }); + + it('toggles opensea api enabled on', async () => { + mockState.metamask.openSeaEnabled = false; + + const localMockStore = configureMockStore([thunk])(mockState); + renderWithProvider(, localMockStore); + + expect(await toggleCheckbox('enableOpenSeaAPI', false, true)).toBe(true); + }); + it('toggles Display NFT media enabled', async () => { expect(await toggleCheckbox('displayNftMedia', false)).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(, localMockStore); + + expect(await toggleCheckbox('useNftDetection', false, true)).toBe(true); }); it('toggles phishing detection', async () => { @@ -151,6 +176,22 @@ describe('Security Tab', () => { expect(screen.queryByText(tEn('forbiddenIpfsGateway'))).toBeInTheDocument(); }); + it('toggles IPFS gateway', async () => { + mockState.metamask.ipfsGateway = ''; + + const localMockStore = configureMockStore([thunk])(mockState); + renderWithProvider(, 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(, mockStore); From 703b3da05924ea09b3bb3a08ad9331c25935edcb Mon Sep 17 00:00:00 2001 From: Howard Braham Date: Thu, 3 Aug 2023 14:49:26 -0700 Subject: [PATCH 5/6] response to legobeat review --- .../handlers/add-ethereum-chain.js | 19 +++++++++++++++---- app/scripts/lib/util.test.js | 8 -------- app/scripts/lib/util.ts | 14 ++------------ 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js b/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js index 9d80596a3cb5..f7c2d586c5d4 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js @@ -1,16 +1,16 @@ -import { ethErrors, errorCodes } from 'eth-rpc-errors'; -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 { isLocalhostOrHttps } from '../../util'; +import { getValidUrl } from '../../util'; const addEthereumChain = { methodNames: [MESSAGE_TYPE.ADD_ETHEREUM_CHAIN], @@ -83,6 +83,17 @@ async function addEthereumChainHandler( ); } + function isLocalhostOrHttps(urlString) { + const url = getValidUrl(urlString); + + return ( + url !== null && + (url.hostname === 'localhost' || + url.hostname === '127.0.0.1' || + url.protocol === 'https:') + ); + } + const firstValidRPCUrl = Array.isArray(rpcUrls) ? rpcUrls.find((rpcUrl) => isLocalhostOrHttps(rpcUrl)) : null; diff --git a/app/scripts/lib/util.test.js b/app/scripts/lib/util.test.js index b1f4899a9598..3b70d9be7a73 100644 --- a/app/scripts/lib/util.test.js +++ b/app/scripts/lib/util.test.js @@ -21,7 +21,6 @@ import { getEnvironmentType, getPlatform, getValidUrl, - isLocalhostOrHttps, isWebUrl, } from './util'; @@ -91,13 +90,6 @@ describe('app utils', () => { expect(addUrlProtocolPrefix('exa mple.com')).toStrictEqual(null); }); - it('should test isLocalhostOrHttps', () => { - expect(isLocalhostOrHttps('http://example.com')).toStrictEqual(false); - expect(isLocalhostOrHttps('https://example.com')).toStrictEqual(true); - expect(isLocalhostOrHttps('http://localhost')).toStrictEqual(true); - expect(isLocalhostOrHttps('http://127.0.0.1')).toStrictEqual(true); - }); - it('should test isWebUrl', () => { expect(isWebUrl('http://example.com')).toStrictEqual(true); expect(isWebUrl('https://example.com')).toStrictEqual(true); diff --git a/app/scripts/lib/util.ts b/app/scripts/lib/util.ts index b7c4b23872ae..f6b93e307a9c 100644 --- a/app/scripts/lib/util.ts +++ b/app/scripts/lib/util.ts @@ -1,3 +1,4 @@ +import url from 'url'; import { AccessList } from '@ethereumjs/tx'; import BN from 'bn.js'; import { memoize } from 'lodash'; @@ -236,7 +237,7 @@ export function previousValueComparator( export function addUrlProtocolPrefix(urlString: string) { let trimmed = urlString.trim(); - if (trimmed.length > 0 && !trimmed.match(/(^http:\/\/)|(^https:\/\/)/u)) { + if (trimmed.length && !url.parse(trimmed).protocol) { trimmed = `https://${trimmed}`; } @@ -265,17 +266,6 @@ export function getValidUrl(urlString: string): URL | null { } } -export function isLocalhostOrHttps(urlString: string) { - const url = getValidUrl(urlString); - - return ( - url !== null && - (url.hostname === 'localhost' || - url.hostname === '127.0.0.1' || - url.protocol === 'https:') - ); -} - export function isWebUrl(urlString: string): boolean { const url = getValidUrl(urlString); From a82362e46f21405479f2321b315c40468743dbe5 Mon Sep 17 00:00:00 2001 From: Howard Braham Date: Thu, 3 Aug 2023 15:08:07 -0700 Subject: [PATCH 6/6] fixing lint and Jest --- app/scripts/lib/util.ts | 4 +- .../__snapshots__/security-tab.test.js.snap | 87 ++++++++++--------- .../security-tab/security-tab.component.js | 62 +++++++------ .../security-tab/security-tab.test.js | 4 - 4 files changed, 82 insertions(+), 75 deletions(-) diff --git a/app/scripts/lib/util.ts b/app/scripts/lib/util.ts index f6b93e307a9c..a6d159bf478d 100644 --- a/app/scripts/lib/util.ts +++ b/app/scripts/lib/util.ts @@ -1,4 +1,4 @@ -import url from 'url'; +import urlLib from 'url'; import { AccessList } from '@ethereumjs/tx'; import BN from 'bn.js'; import { memoize } from 'lodash'; @@ -237,7 +237,7 @@ export function previousValueComparator( export function addUrlProtocolPrefix(urlString: string) { let trimmed = urlString.trim(); - if (trimmed.length && !url.parse(trimmed).protocol) { + if (trimmed.length && !urlLib.parse(trimmed).protocol) { trimmed = `https://${trimmed}`; } diff --git a/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap b/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap index bd0e809fa7dd..cfd9c041083e 100644 --- a/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap +++ b/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap @@ -448,6 +448,7 @@ exports[`Security Tab should match snapshot 1`] = `
-