From 99c709ff8f43a37a41df1a17a72d337ff01a4d76 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Fri, 28 Jul 2023 11:21:43 -0500 Subject: [PATCH] Allow user to turn off ENS DNS resolution (#20102) * Allow user to turn off IPFS gateway resolution * Add end to end test for toggle on and off * Fix jest tests and snapshots * Change variable name * Implement provided content * Use MetaMask eth instead * Allow searching for ENS setting * Fix jest --------- Co-authored-by: Brad Decker --- app/_locales/en/messages.json | 18 ++++ app/scripts/background.js | 3 + app/scripts/controllers/preferences.js | 10 ++ app/scripts/lib/ens-ipfs/setup.js | 22 ++++- app/scripts/metamask-controller.js | 4 + test/e2e/helpers.js | 9 +- test/e2e/tests/ipfs-ens-resolution.spec.js | 79 ++++++++++++++++ ui/helpers/constants/settings.js | 7 ++ ui/helpers/utils/settings-search.test.js | 2 +- .../privacy-settings/privacy-settings.js | 37 ++++++++ .../privacy-settings/privacy-settings.test.js | 13 +++ .../__snapshots__/security-tab.test.js.snap | 94 +++++++++++++++++++ .../security-tab/security-tab.component.js | 66 +++++++++++++ .../security-tab/security-tab.container.js | 5 + ui/store/actions.ts | 13 +++ 15 files changed, 379 insertions(+), 3 deletions(-) create mode 100644 test/e2e/tests/ipfs-ens-resolution.spec.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 45fd6204f..ea628a029 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1426,6 +1426,24 @@ "enhancedTokenDetectionAlertMessage": { "message": "Enhanced token detection is currently available on $1. $2" }, + "ensDomainsSettingDescriptionIntro": { + "message": "MetaMask lets you see ENS domains like \"https://metamask.eth\" right in your browser's address bar. Here's how it works:" + }, + "ensDomainsSettingDescriptionOutro": { + "message": "Regular browsers don't usually handle ENS or IPFS addresses, but MetaMask helps with that. Using this feature might share your IP address with IPFS third-party services." + }, + "ensDomainsSettingDescriptionPoint1": { + "message": "MetaMask checks with Ethereum's ENS contract to find the code connected to the ENS name." + }, + "ensDomainsSettingDescriptionPoint2": { + "message": "If the code is linked to IPFS, it gets the content from the IPFS network." + }, + "ensDomainsSettingDescriptionPoint3": { + "message": "Then, you can see the content, usually a website or something similar." + }, + "ensDomainsSettingTitle": { + "message": "Show ENS domains in address bar" + }, "ensIllegalCharacter": { "message": "Illegal character for ENS." }, diff --git a/app/scripts/background.js b/app/scripts/background.js index e3416b796..d949aefc8 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -466,6 +466,9 @@ export function setupController( getIpfsGateway: controller.preferencesController.getIpfsGateway.bind( controller.preferencesController, ), + getUseAddressBarEnsResolution: () => + controller.preferencesController.store.getState() + .useAddressBarEnsResolution, provider: controller.provider, }); diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index e21f3239e..5b591cb44 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -67,6 +67,7 @@ export default class PreferencesController { }, // ENS decentralized website resolution ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, + useAddressBarEnsResolution: true, infuraBlocked: null, ledgerTransportType: window.navigator.hid ? LedgerTransportTypes.webhid @@ -480,6 +481,15 @@ export default class PreferencesController { return domain; } + /** + * A setter for the `useAddressBarEnsResolution` property + * + * @param {boolean} useAddressBarEnsResolution - Whether or not user prefers IPFS resolution for domains + */ + async setUseAddressBarEnsResolution(useAddressBarEnsResolution) { + this.store.updateState({ useAddressBarEnsResolution }); + } + /** * A setter for the `ledgerTransportType` property. * diff --git a/app/scripts/lib/ens-ipfs/setup.js b/app/scripts/lib/ens-ipfs/setup.js index 942b888cc..77052b05e 100644 --- a/app/scripts/lib/ens-ipfs/setup.js +++ b/app/scripts/lib/ens-ipfs/setup.js @@ -13,6 +13,7 @@ export default function setupEnsIpfsResolver({ provider, getCurrentChainId, getIpfsGateway, + getUseAddressBarEnsResolution, }) { // install listener const urlPatterns = supportedTopLevelDomains.map((tld) => `*://*.${tld}/*`); @@ -33,7 +34,12 @@ export default function setupEnsIpfsResolver({ const { tabId, url } = details; // ignore requests that are not associated with tabs // only attempt ENS resolution on mainnet - if (tabId === -1 || getCurrentChainId() !== '0x1') { + if ( + (tabId === -1 || getCurrentChainId() !== '0x1') && + // E2E tests use a chain other than 0x1, so for testing, + // allow the reuqest to pass through + !process.env.IN_TEST + ) { return; } // parse ens name @@ -50,9 +56,23 @@ export default function setupEnsIpfsResolver({ async function attemptResolve({ tabId, name, pathname, search, fragment }) { const ipfsGateway = getIpfsGateway(); + const useAddressBarEnsResolution = getUseAddressBarEnsResolution(); + + if (!useAddressBarEnsResolution || ipfsGateway === '') { + return; + } browser.tabs.update(tabId, { url: `loading.html` }); + let url = `https://app.ens.domains/name/${name}`; + + // If we're testing ENS domain resolution support, + // we assume the ENS domains URL + if (process.env.IN_TEST) { + browser.tabs.update(tabId, { url }); + return; + } + try { const { type, hash } = await resolveEnsToIpfsContentId({ provider, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 29f1a1a4e..7ff6b5d26 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -2233,6 +2233,10 @@ export default class MetamaskController extends EventEmitter { setIpfsGateway: preferencesController.setIpfsGateway.bind( preferencesController, ), + setUseAddressBarEnsResolution: + preferencesController.setUseAddressBarEnsResolution.bind( + preferencesController, + ), setParticipateInMetaMetrics: metaMetricsController.setParticipateInMetaMetrics.bind( metaMetricsController, diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index b1e17b350..916074937 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -190,7 +190,14 @@ async function withFixtures(options, testSuite) { if (phishingPageServer.isRunning()) { await phishingPageServer.quit(); } - await mockServer.stop(); + + // Since mockServer could be stop'd at another location, + // use a try/catch to avoid an error + try { + await mockServer.stop(); + } catch (e) { + console.log('mockServer already stopped'); + } } } } diff --git a/test/e2e/tests/ipfs-ens-resolution.spec.js b/test/e2e/tests/ipfs-ens-resolution.spec.js new file mode 100644 index 000000000..98683de08 --- /dev/null +++ b/test/e2e/tests/ipfs-ens-resolution.spec.js @@ -0,0 +1,79 @@ +const { strict: assert } = require('assert'); +const { buildWebDriver } = require('../webdriver'); +const { withFixtures } = require('../helpers'); +const FixtureBuilder = require('../fixture-builder'); + +describe('Settings', function () { + const ENS_NAME = 'metamask.eth'; + const ENS_NAME_URL = `https://${ENS_NAME}/`; + const ENS_DESTINATION_URL = `https://app.ens.domains/name/${ENS_NAME}`; + + it('Redirects to ENS domains when user inputs ENS into address bar', async function () { + // Using proxy port that doesn't resolve so that the browser can error out properly + // on the ".eth" hostname. The proxy does too much interference with 8000. + const { driver } = await buildWebDriver({ proxyUrl: '127.0.0.1:8001' }); + await driver.navigate(); + + // The setting defaults to "on" so we can simply enter an ENS address + // into the address bar and listen for address change + try { + await driver.openNewPage(ENS_NAME_URL); + } catch (e) { + // Ignore ERR_PROXY_CONNECTION_FAILED error + // since all we care about is getting to the correct URL + } + + // Ensure that the redirect to ENS Domains has happened + const currentUrl = await driver.getCurrentUrl(); + assert.equal(currentUrl, ENS_DESTINATION_URL); + + await driver.quit(); + }); + + it('Does not lookup IPFS data for ENS Domain when switched off', async function () { + let server; + + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + title: this.test.title, + testSpecificMock: (mockServer) => { + server = mockServer; + }, + }, + async ({ driver }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + + // goes to the settings screen + await driver.clickElement( + '[data-testid="account-options-menu-button"]', + ); + await driver.clickElement({ text: 'Settings', tag: 'div' }); + await driver.clickElement({ text: 'Security & privacy', tag: 'div' }); + + // turns off IPFS domain resolution + await driver.clickElement( + '[data-testid="ipfs-gateway-resolution-container"] .toggle-button', + ); + + // Now that we no longer need the MetaMask UI, and want the browser + // to handle the request error, we need to stop the server + await server.stop(); + + try { + await driver.openNewPage(ENS_NAME_URL); + } catch (e) { + // Ignore ERR_PROXY_CONNECTION_FAILED error + // since all we care about is getting to the correct URL + } + + // Ensure that the redirect to ENS Domains does not happen + // Instead, the domain will be kept which is a 404 + const currentUrl = await driver.getCurrentUrl(); + assert.equal(currentUrl, ENS_NAME_URL); + }, + ); + }); +}); diff --git a/ui/helpers/constants/settings.js b/ui/helpers/constants/settings.js index 6c38bdd17..54fb73d11 100644 --- a/ui/helpers/constants/settings.js +++ b/ui/helpers/constants/settings.js @@ -205,6 +205,13 @@ export const SETTINGS_CONSTANTS = [ route: `${SECURITY_ROUTE}#price-checker`, icon: 'fa fa-lock', }, + { + tabMessage: (t) => t('securityAndPrivacy'), + sectionMessage: (t) => t('ensDomainsSettingTitle'), + descriptionMessage: (t) => t('ensDomainsSettingDescriptionIntro'), + route: `${SECURITY_ROUTE}#ens-domains`, + icon: 'fa fa-lock', + }, { tabMessage: (t) => t('alerts'), sectionMessage: (t) => t('alertSettingsUnconnectedAccount'), diff --git a/ui/helpers/utils/settings-search.test.js b/ui/helpers/utils/settings-search.test.js index 07a7f3aa5..70f02d5ee 100644 --- a/ui/helpers/utils/settings-search.test.js +++ b/ui/helpers/utils/settings-search.test.js @@ -165,7 +165,7 @@ describe('Settings Search Utils', () => { it('should get good security & privacy section number', () => { expect( getNumberOfSettingsInSection(t, t('securityAndPrivacy')), - ).toStrictEqual(9); + ).toStrictEqual(10); }); it('should get good alerts section number', () => { diff --git a/ui/pages/onboarding-flow/privacy-settings/privacy-settings.js b/ui/pages/onboarding-flow/privacy-settings/privacy-settings.js index 67f6f04fe..a977a8a23 100644 --- a/ui/pages/onboarding-flow/privacy-settings/privacy-settings.js +++ b/ui/pages/onboarding-flow/privacy-settings/privacy-settings.js @@ -13,6 +13,7 @@ import { } from '../../../../shared/lib/ui-utils'; import { PickerNetwork, + Text, TextField, } from '../../../components/component-library'; import Box from '../../../components/ui/box/box'; @@ -22,6 +23,7 @@ import { MetaMetricsContext } from '../../../contexts/metametrics'; import { FONT_WEIGHT, TextColor, + TextVariant, TypographyVariant, } from '../../../helpers/constants/design-system'; import { ONBOARDING_PIN_EXTENSION_ROUTE } from '../../../helpers/constants/routes'; @@ -35,6 +37,7 @@ import { setUseMultiAccountBalanceChecker, setUsePhishDetect, setUseTokenDetection, + setUseAddressBarEnsResolution, showModal, toggleNetworkMenu, } from '../../../store/actions'; @@ -54,6 +57,7 @@ export default function PrivacySettings() { setMultiAccountBalanceCheckerEnabled, ] = useState(true); const [ipfsURL, setIPFSURL] = useState(''); + const [addressBarResolution, setAddressBarResolution] = useState(true); const [ipfsError, setIPFSError] = useState(null); const trackEvent = useContext(MetaMetricsContext); @@ -70,6 +74,7 @@ export default function PrivacySettings() { ); dispatch(setUseCurrencyRateCheck(turnOnCurrencyRateCheck)); dispatch(setCompletedOnboarding()); + dispatch(setUseAddressBarEnsResolution(addressBarResolution)); if (ipfsURL && !ipfsError) { const { host } = new URL(addUrlProtocolPrefix(ipfsURL)); @@ -252,6 +257,38 @@ export default function PrivacySettings() { } /> + + + {t('ensDomainsSettingDescriptionIntro')} + + + + {t('ensDomainsSettingDescriptionPoint1')} + + + {t('ensDomainsSettingDescriptionPoint2')} + + + {t('ensDomainsSettingDescriptionPoint3')} + + + + {t('ensDomainsSettingDescriptionOutro')} + + + } + /> { .fn() .mockImplementation(() => Promise.resolve()); const setUseMultiAccountBalanceCheckerStub = jest.fn(); + const setUseAddressBarEnsResolutionStub = jest.fn(); setBackgroundConnection({ setFeatureFlag: setFeatureFlagStub, @@ -37,6 +38,7 @@ describe('Privacy Settings Onboarding View', () => { setIpfsGateway: setIpfsGatewayStub, completeOnboarding: completeOnboardingStub, setUseMultiAccountBalanceChecker: setUseMultiAccountBalanceCheckerStub, + setUseAddressBarEnsResolution: setUseAddressBarEnsResolutionStub, }); it('should update preferences', () => { @@ -50,6 +52,7 @@ describe('Privacy Settings Onboarding View', () => { expect(setUseTokenDetectionStub).toHaveBeenCalledTimes(0); expect(setUseMultiAccountBalanceCheckerStub).toHaveBeenCalledTimes(0); expect(setUseCurrencyRateCheckStub).toHaveBeenCalledTimes(0); + expect(setUseAddressBarEnsResolutionStub).toHaveBeenCalledTimes(0); const toggles = container.querySelectorAll('input[type=checkbox]'); const submitButton = getByText('Done'); @@ -60,6 +63,7 @@ describe('Privacy Settings Onboarding View', () => { fireEvent.click(toggles[2]); fireEvent.click(toggles[3]); fireEvent.click(toggles[4]); + fireEvent.click(toggles[5]); fireEvent.click(submitButton); expect(setFeatureFlagStub).toHaveBeenCalledTimes(1); @@ -67,6 +71,7 @@ describe('Privacy Settings Onboarding View', () => { expect(setUseTokenDetectionStub).toHaveBeenCalledTimes(1); expect(setUseMultiAccountBalanceCheckerStub).toHaveBeenCalledTimes(1); expect(setUseCurrencyRateCheckStub).toHaveBeenCalledTimes(1); + expect(setUseAddressBarEnsResolutionStub).toHaveBeenCalledTimes(1); expect(setFeatureFlagStub.mock.calls[0][1]).toStrictEqual(false); expect(setUsePhishDetectStub.mock.calls[0][0]).toStrictEqual(false); @@ -75,6 +80,9 @@ describe('Privacy Settings Onboarding View', () => { false, ); expect(setUseCurrencyRateCheckStub.mock.calls[0][0]).toStrictEqual(false); + expect(setUseAddressBarEnsResolutionStub.mock.calls[0][0]).toStrictEqual( + false, + ); // toggle back to true fireEvent.click(toggles[0]); @@ -82,12 +90,14 @@ describe('Privacy Settings Onboarding View', () => { fireEvent.click(toggles[2]); fireEvent.click(toggles[3]); fireEvent.click(toggles[4]); + fireEvent.click(toggles[5]); fireEvent.click(submitButton); expect(setFeatureFlagStub).toHaveBeenCalledTimes(2); expect(setUsePhishDetectStub).toHaveBeenCalledTimes(2); expect(setUseTokenDetectionStub).toHaveBeenCalledTimes(2); expect(setUseMultiAccountBalanceCheckerStub).toHaveBeenCalledTimes(2); expect(setUseCurrencyRateCheckStub).toHaveBeenCalledTimes(2); + expect(setUseAddressBarEnsResolutionStub).toHaveBeenCalledTimes(2); expect(setFeatureFlagStub.mock.calls[1][1]).toStrictEqual(true); expect(setUsePhishDetectStub.mock.calls[1][0]).toStrictEqual(true); @@ -96,6 +106,9 @@ describe('Privacy Settings Onboarding View', () => { true, ); expect(setUseCurrencyRateCheckStub.mock.calls[1][0]).toStrictEqual(true); + expect(setUseAddressBarEnsResolutionStub.mock.calls[1][0]).toStrictEqual( + true, + ); }); describe('IPFS', () => { 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 7f219c7bd..a2ca0bdb4 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 @@ -405,6 +405,100 @@ exports[`Security Tab should match snapshot 1`] = ` +
+ Show ENS domains in address bar +
+ + MetaMask lets you see ENS domains like "https://metamask.eth" right in your browser's address bar. Here's how it works: + +
    +
  • + MetaMask checks with Ethereum's ENS contract to find the code connected to the ENS name. +
    • +
  • + If the code is linked to IPFS, it gets the content from the IPFS network. +
    • +
  • + Then, you can see the content, usually a website or something similar. +
    • +
+ + Regular browsers don't usually handle ENS or IPFS addresses, but MetaMask helps with that. Using this feature might share your IP address with IPFS third-party services. + +
+
+
+
+
{ const url = new URL(addUrlProtocolPrefix(gateway)); @@ -370,6 +379,63 @@ export default class SecurityTab extends PureComponent { />
+
+ {t('ensDomainsSettingTitle')} +
+ + {t('ensDomainsSettingDescriptionIntro')} + + + + {t('ensDomainsSettingDescriptionPoint1')} + + + {t('ensDomainsSettingDescriptionPoint2')} + + + {t('ensDomainsSettingDescriptionPoint3')} + + + + {t('ensDomainsSettingDescriptionOutro')} + +
+
+
+
+ setUseAddressBarEnsResolution(!value)} + offLabel={t('off')} + onLabel={t('on')} + /> +
+
); } diff --git a/ui/pages/settings/security-tab/security-tab.container.js b/ui/pages/settings/security-tab/security-tab.container.js index 1d4251b5c..d9ba96857 100644 --- a/ui/pages/settings/security-tab/security-tab.container.js +++ b/ui/pages/settings/security-tab/security-tab.container.js @@ -9,6 +9,7 @@ import { setUseMultiAccountBalanceChecker, setUsePhishDetect, setUseTokenDetection, + setUseAddressBarEnsResolution, } from '../../../store/actions'; import SecurityTab from './security-tab.component'; @@ -25,6 +26,7 @@ const mapStateToProps = (state) => { ipfsGateway, useMultiAccountBalanceChecker, useCurrencyRateCheck, + useAddressBarEnsResolution, } = metamask; return { @@ -36,6 +38,7 @@ const mapStateToProps = (state) => { ipfsGateway, useMultiAccountBalanceChecker, useCurrencyRateCheck, + useAddressBarEnsResolution, }; }; @@ -56,6 +59,8 @@ const mapDispatchToProps = (dispatch) => { setUseMultiAccountBalanceChecker: (value) => { return dispatch(setUseMultiAccountBalanceChecker(value)); }, + setUseAddressBarEnsResolution: (value) => + dispatch(setUseAddressBarEnsResolution(value)), }; }; diff --git a/ui/store/actions.ts b/ui/store/actions.ts index 176fafefa..af3b677c2 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -3095,6 +3095,19 @@ export function setIpfsGateway( }; } +export function setUseAddressBarEnsResolution( + val: string, +): ThunkAction { + return (dispatch: MetaMaskReduxDispatch) => { + log.debug(`background.setUseAddressBarEnsResolution`); + callBackgroundMethod('setUseAddressBarEnsResolution', [val], (err) => { + if (err) { + dispatch(displayWarning(err)); + } + }); + }; +} + export function updateCurrentLocale( key: string, ): ThunkAction {