From 72d2977e7202ad1c0367a420e0d9487d44f2dc5d Mon Sep 17 00:00:00 2001 From: Hennadii Ivtushok Date: Wed, 27 Apr 2022 20:14:10 +0200 Subject: [PATCH] Warn about multiple MetaMask instances running (#13836) * Add text warning on startup page * Try to detect extensions with browser API * Setup messaging between different versions of extension * Cleanup * Cleanup * Simplify check for multiple instances running * Fix a doc string + use webextension-polyfill * Fix test * Mock webextension-polyfill * Mock correctly * Catch error and show warning in both extensions * Mock as promise * Address comments * Rename build ids * Run detection code only if Chrome * Add Firefox warnings * Cleanup imports * Update connection ids * Run detection code for Firefox * Add test * Add missing await * Update tests * Cleanup * Cleanup * Improve testing * Improve tests * Log errors from sendMessage * Cleanup Co-authored-by: Frederik Bolding --- app/_locales/en/messages.json | 4 + app/scripts/detect-multiple-instances.js | 55 +++++++++ app/scripts/detect-multiple-instances.test.js | 104 ++++++++++++++++++ app/scripts/metamask-controller.js | 9 ++ app/scripts/metamask-controller.test.js | 8 ++ shared/constants/app.js | 20 ++++ .../experimental-area/experimental-area.js | 2 + 7 files changed, 202 insertions(+) create mode 100644 app/scripts/detect-multiple-instances.js create mode 100644 app/scripts/detect-multiple-instances.test.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 74e079f91..120f19d1b 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1252,6 +1252,10 @@ "message": "All Flask APIs are experimental. They may be changed or removed without notice, or they might stay on Flask indefinitely without ever being migrated to stable MetaMask. Use them at your own risk.", "description": "This message warns developers about unstable Flask APIs" }, + "flaskWelcomeWarning4": { + "message": "Make sure to disable your regular MetaMask extension when using Flask.", + "description": "This message calls to pay attention about multiple versions of MetaMask running on the same site (Flask + Prod)" + }, "flaskWelcomeWarningAcceptButton": { "message": "I accept the risks", "description": "this text is shown on a button, which the user presses to confirm they understand the risks of using Flask" diff --git a/app/scripts/detect-multiple-instances.js b/app/scripts/detect-multiple-instances.js new file mode 100644 index 000000000..d4d4544d2 --- /dev/null +++ b/app/scripts/detect-multiple-instances.js @@ -0,0 +1,55 @@ +/** + * Sets up two-way communication between the + * mainline version of extension and Flask build + * in order to detect & warn if there are two different + * versions running simultaneously. + */ + +import browser from 'webextension-polyfill'; +import { + PLATFORM_CHROME, + PLATFORM_FIREFOX, + CHROME_BUILD_IDS, + FIREFOX_BUILD_IDS, +} from '../../shared/constants/app'; +import { getPlatform } from './lib/util'; + +const MESSAGE_TEXT = 'isRunning'; + +const showWarning = () => + console.warn('Warning! You have multiple instances of MetaMask running!'); + +/** + * Handles the ping message sent from other extension. + * Displays console warning if it's active. + * + * @param message - The message received from the other extension + */ +export const onMessageReceived = (message) => { + if (message === MESSAGE_TEXT) { + showWarning(); + } +}; + +/** + * Sends the ping message sent to other extensions to detect whether it's active or not. + */ +export const checkForMultipleVersionsRunning = async () => { + if (getPlatform() !== PLATFORM_CHROME && getPlatform() !== PLATFORM_FIREFOX) { + return; + } + const buildIds = + getPlatform() === PLATFORM_CHROME ? CHROME_BUILD_IDS : FIREFOX_BUILD_IDS; + + const thisBuild = browser.runtime.id; + + for (const id of buildIds) { + if (id !== thisBuild) { + try { + await browser.runtime.sendMessage(id, MESSAGE_TEXT); + } catch (error) { + // Should do nothing if receiving end was not reached (no other instances running) + } + } + } +}; diff --git a/app/scripts/detect-multiple-instances.test.js b/app/scripts/detect-multiple-instances.test.js new file mode 100644 index 000000000..0fc04192e --- /dev/null +++ b/app/scripts/detect-multiple-instances.test.js @@ -0,0 +1,104 @@ +import { strict as assert } from 'assert'; +import browser from 'webextension-polyfill'; +import sinon from 'sinon'; +import { + PLATFORM_CHROME, + PLATFORM_EDGE, + METAMASK_BETA_CHROME_ID, + METAMASK_PROD_CHROME_ID, + METAMASK_FLASK_CHROME_ID, +} from '../../shared/constants/app'; +import { + checkForMultipleVersionsRunning, + onMessageReceived, +} from './detect-multiple-instances'; +import * as util from './lib/util'; + +describe('multiple instances running detector', function () { + const PING_MESSAGE = 'isRunning'; + + let sendMessageStub = sinon.stub(); + + beforeEach(async function () { + sinon.replace(browser, 'runtime', { + sendMessage: sendMessageStub, + id: METAMASK_BETA_CHROME_ID, + }); + + sinon.stub(util, 'getPlatform').callsFake((_) => { + return PLATFORM_CHROME; + }); + }); + + afterEach(function () { + sinon.restore(); + }); + + describe('checkForMultipleVersionsRunning', function () { + it('should send ping message to multiple instances', async function () { + await checkForMultipleVersionsRunning(); + + assert(sendMessageStub.calledTwice); + assert( + sendMessageStub + .getCall(0) + .calledWithExactly(METAMASK_PROD_CHROME_ID, PING_MESSAGE), + ); + assert( + sendMessageStub + .getCall(1) + .calledWithExactly(METAMASK_FLASK_CHROME_ID, PING_MESSAGE), + ); + }); + + it('should not send ping message if platform is not Chrome or Firefox', async function () { + util.getPlatform.restore(); + sendMessageStub = sinon.stub(); + + sinon.stub(util, 'getPlatform').callsFake((_) => { + return PLATFORM_EDGE; + }); + + await checkForMultipleVersionsRunning(); + + assert(sendMessageStub.notCalled); + }); + + it('should not expose an error outside if sendMessage throws', async function () { + sinon.restore(); + + sinon.replace(browser, 'runtime', { + sendMessage: sinon.stub().throws(), + id: METAMASK_BETA_CHROME_ID, + }); + + const spy = sinon.spy(checkForMultipleVersionsRunning); + + await checkForMultipleVersionsRunning(); + + assert(!spy.threw()); + }); + }); + + describe('onMessageReceived', function () { + beforeEach(function () { + sinon.spy(console, 'warn'); + }); + + it('should print warning message to on ping message received', async function () { + onMessageReceived(PING_MESSAGE); + + assert( + console.warn.calledWithExactly( + 'Warning! You have multiple instances of MetaMask running!', + ), + ); + }); + + it('should not print warning message if wrong message received', async function () { + onMessageReceived(PING_MESSAGE.concat('wrong')); + + assert(console.warn.notCalled); + }); + }); +}); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 66cff0823..4865b4a8d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -88,6 +88,10 @@ import { hexToDecimal } from '../../ui/helpers/utils/conversions.util'; import { getTokenValueParam } from '../../ui/helpers/utils/token-util'; import { isEqualCaseInsensitive } from '../../shared/modules/string-utils'; import { parseStandardTokenTransactionData } from '../../shared/modules/transaction.utils'; +import { + onMessageReceived, + checkForMultipleVersionsRunning, +} from './detect-multiple-instances'; import ComposableObservableStore from './lib/ComposableObservableStore'; import AccountTracker from './lib/account-tracker'; import createLoggerMiddleware from './lib/createLoggerMiddleware'; @@ -1062,6 +1066,11 @@ export default class MetamaskController extends EventEmitter { // TODO:LegacyProvider: Delete this.publicConfigStore = this.createPublicConfigStore(); + + // Multiple MetaMask instances launched warning + this.extension.runtime.onMessageExternal.addListener(onMessageReceived); + // Fire a ping message to check if other extensions are running + checkForMultipleVersionsRunning(); } ///: BEGIN:ONLY_INCLUDE_IN(flask) diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index f1771c173..6baf1b113 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -6,6 +6,7 @@ import { pubToAddress, bufferToHex } from 'ethereumjs-util'; import { obj as createThoughStream } from 'through2'; import EthQuery from 'eth-query'; import proxyquire from 'proxyquire'; +import browser from 'webextension-polyfill'; import { TRANSACTION_STATUSES } from '../../shared/constants/transaction'; import createTxMeta from '../../test/lib/createTxMeta'; import { NETWORK_TYPE_RPC } from '../../shared/constants/network'; @@ -71,6 +72,9 @@ const browserPolyfillMock = { onInstalled: { addListener: () => undefined, }, + onMessageExternal: { + addListener: () => undefined, + }, getPlatformInfo: async () => 'mac', }, }; @@ -131,6 +135,10 @@ describe('MetaMaskController', function () { .get(/.*/u) .reply(200, '{"JPY":12415.9}'); + sandbox.replace(browser, 'runtime', { + sendMessage: sandbox.stub().rejects(), + }); + metamaskController = new MetaMaskController({ showUserConfirmation: noop, encryptor: { diff --git a/shared/constants/app.js b/shared/constants/app.js index 159e740a3..0d1aecc06 100644 --- a/shared/constants/app.js +++ b/shared/constants/app.js @@ -72,3 +72,23 @@ export const POLLING_TOKEN_ENVIRONMENT_TYPES = { }; export const ORIGIN_METAMASK = 'metamask'; + +export const METAMASK_BETA_CHROME_ID = 'pbbkamfgmaedccnfkmjcofcecjhfgldn'; +export const METAMASK_PROD_CHROME_ID = 'nkbihfbeogaeaoehlefnkodbefgpgknn'; +export const METAMASK_FLASK_CHROME_ID = 'ljfoeinjpaedjfecbmggjgodbgkmjkjk'; + +export const CHROME_BUILD_IDS = [ + METAMASK_BETA_CHROME_ID, + METAMASK_PROD_CHROME_ID, + METAMASK_FLASK_CHROME_ID, +]; + +const METAMASK_BETA_FIREFOX_ID = 'webextension-beta@metamask.io'; +const METAMASK_PROD_FIREFOX_ID = 'webextension@metamask.io'; +const METAMASK_FLASK_FIREFOX_ID = 'webextension-flask@metamask.io'; + +export const FIREFOX_BUILD_IDS = [ + METAMASK_BETA_FIREFOX_ID, + METAMASK_PROD_FIREFOX_ID, + METAMASK_FLASK_FIREFOX_ID, +]; diff --git a/ui/components/app/flask/experimental-area/experimental-area.js b/ui/components/app/flask/experimental-area/experimental-area.js index 9a3a8ec1e..15160c395 100644 --- a/ui/components/app/flask/experimental-area/experimental-area.js +++ b/ui/components/app/flask/experimental-area/experimental-area.js @@ -86,6 +86,8 @@ export default function ExperimentalArea({ redirectTo }) {

{t('flaskWelcomeWarning2')}


{t('flaskWelcomeWarning3')}

+
+

{t('flaskWelcomeWarning4')}