Login per site onboarding (#7602)

* Remove unused onboarding stream * Pass `sender` through to `setupProviderEngine` The Port `sender` has been passed down a few more layers. This allows us to get more information from the sender deeper in the stack, but also simplifies things a bit as well. For example, now the "fake" URL object with the `metamask` hostname is no longer needed. * Create onboarding middleware This middleware intercepts `wallet_registerOnboarding` RPC messages. It will register the sender as an oboarding initiator if possible, and otherwise ignores the message.
2024-12-23 01:39:44 +01:00 · 2019-12-20 12:02:31 -03:30 · 2019-12-20 12:02:31 -03:30 · 69d418a5a3
commit 69d418a5a3
parent 8701ac38a1
8 changed files with 104 additions and 129 deletions
--- a/app/scripts/background.js
+++ b/app/scripts/background.js
@ -318,7 +318,6 @@ function setupController (initState, initLangCode) {
  //
  extension.runtime.onConnect.addListener(connectRemote)
  extension.runtime.onConnectExternal.addListener(connectExternal)
-  extension.runtime.onMessage.addListener(controller.onMessage.bind(controller))

  const metamaskInternalProcessHash = {
    [ENVIRONMENT_TYPE_POPUP]: true,
@ -358,10 +357,7 @@ function setupController (initState, initLangCode) {
      const portStream = new PortStream(remotePort)
      // communication with popup
      controller.isClientOpen = true
-      // construct fake URL for identifying internal messages
-      const metamaskUrl = new URL(window.location)
-      metamaskUrl.hostname = 'metamask'
-      controller.setupTrustedCommunication(portStream, metamaskUrl)
+      controller.setupTrustedCommunication(portStream, remotePort.sender)

      if (processName === ENVIRONMENT_TYPE_POPUP) {
        popupIsOpen = true
@ -408,13 +404,8 @@ function setupController (initState, initLangCode) {

  // communication with page or other extension
  function connectExternal (remotePort) {
-    const senderUrl = new URL(remotePort.sender.url)
-    let extensionId
-    if (remotePort.sender.id !== extension.runtime.id) {
-      extensionId = remotePort.sender.id
-    }
    const portStream = new PortStream(remotePort)
-    controller.setupUntrustedCommunication(portStream, senderUrl, extensionId)
+    controller.setupUntrustedCommunication(portStream, remotePort.sender)
  }

  //
--- a/app/scripts/contentscript.js
+++ b/app/scripts/contentscript.js
@ -1,9 +1,7 @@
 const fs = require('fs')
 const path = require('path')
 const pump = require('pump')
-const log = require('loglevel')
 const querystring = require('querystring')
-const { Writable } = require('readable-stream')
 const LocalMessageDuplexStream = require('post-message-stream')
 const ObjectMultiplex = require('obj-multiplex')
 const extension = require('extensionizer')
@ -86,44 +84,6 @@ async function setupStreams () {
    (err) => logStreamDisconnectWarning('MetaMask Background Multiplex', err)
  )

-  const onboardingStream = pageMux.createStream('onboarding')
-  const addCurrentTab = new Writable({
-    objectMode: true,
-    write: (chunk, _, callback) => {
-      if (!chunk) {
-        return callback(new Error('Malformed onboarding message'))
-      }
-
-      const handleSendMessageResponse = (error, success) => {
-        if (!error && !success) {
-          error = extension.runtime.lastError
-        }
-        if (error) {
-          log.error(`Failed to send ${chunk.type} message`, error)
-          return callback(error)
-        }
-        callback(null)
-      }
-
-      try {
-        if (chunk.type === 'registerOnboarding') {
-          extension.runtime.sendMessage({ type: 'metamask:registerOnboarding', location: window.location.href }, handleSendMessageResponse)
-        } else {
-          throw new Error(`Unrecognized onboarding message type: '${chunk.type}'`)
-        }
-      } catch (error) {
-        log.error(error)
-        return callback(error)
-      }
-    },
-  })
-
-  pump(
-    onboardingStream,
-    addCurrentTab,
-    error => console.error('MetaMask onboarding channel traffic failed', error),
-  )
-
  // forward communication across inpage-background for these channels only
  forwardTrafficBetweenMuxers('provider', pageMux, extensionMux)
  forwardTrafficBetweenMuxers('publicConfig', pageMux, extensionMux)
--- a/app/scripts/controllers/onboarding.js
+++ b/app/scripts/controllers/onboarding.js
@ -60,7 +60,7 @@ class OnboardingController {
   * @param {string} location - The location of the site registering
   * @param {string} tabId - The id of the tab registering
   */
-  async registerOnboarding (location, tabId) {
+  registerOnboarding = async (location, tabId) => {
    if (this.completedOnboarding) {
      log.debug('Ignoring registerOnboarding; user already onboarded')
      return
--- a/app/scripts/lib/createOnboardingMiddleware.js
+++ b/app/scripts/lib/createOnboardingMiddleware.js
@ -0,0 +1,29 @@
+import log from 'loglevel'
+import extension from 'extensionizer'
+
+/**
+ * Returns a middleware that intercepts `wallet_registerOnboarding` messages
+ * @param {{ location: string, tabId: number, registerOnboarding: Function }} opts - The middleware options
+ * @returns {(req: any, res: any, next: Function, end: Function) => void}
+ */
+function createOnboardingMiddleware ({ location, tabId, registerOnboarding }) {
+  return async function originMiddleware (req, res, next, end) {
+    try {
+      if (req.method !== 'wallet_registerOnboarding') {
+        next()
+        return
+      }
+      if (tabId && tabId !== extension.tabs.TAB_ID_NONE) {
+        await registerOnboarding(location, tabId)
+      } else {
+        log.debug(`'wallet_registerOnboarding' message from ${location} ignored due to missing tabId`)
+      }
+      res.result = true
+      end()
+    } catch (error) {
+      end(error)
+    }
+  }
+}
+
+export default createOnboardingMiddleware
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@ -4,7 +4,6 @@
 * @license   MIT
 */

-const assert = require('assert').strict
 const EventEmitter = require('events')
 const pump = require('pump')
 const Dnode = require('dnode')
@ -20,6 +19,7 @@ const createFilterMiddleware = require('eth-json-rpc-filters')
 const createSubscriptionManager = require('eth-json-rpc-filters/subscriptionManager')
 const createLoggerMiddleware = require('./lib/createLoggerMiddleware')
 const createOriginMiddleware = require('./lib/createOriginMiddleware')
+import createOnboardingMiddleware from './lib/createOnboardingMiddleware'
 const providerAsMiddleware = require('eth-json-rpc-middleware/providerAsMiddleware')
 const { setupMultiplex } = require('./lib/stream-utils.js')
 const KeyringController = require('eth-keyring-controller')
@ -1284,20 +1284,24 @@ module.exports = class MetamaskController extends EventEmitter {
  // SETUP
  //=============================================================================

+  /**
+   * A runtime.MessageSender object, as provided by the browser:
+   * @see https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/runtime/MessageSender
+   * @typedef {Object} MessageSender
+   */
+
  /**
   * Used to create a multiplexed stream for connecting to an untrusted context
   * like a Dapp or other extension.
   * @param {*} connectionStream - The Duplex stream to connect to.
-   * @param {URL} senderUrl - The URL of the resource requesting the stream,
-   * which may trigger a blacklist reload.
-   * @param {string} extensionId - The extension id of the sender, if the sender
-   * is an extension
+   * @param {MessageSender} sender - The sender of the messages on this stream
   */
-  setupUntrustedCommunication (connectionStream, senderUrl, extensionId) {
+  setupUntrustedCommunication (connectionStream, sender) {
+    const hostname = (new URL(sender.url)).hostname
    // Check if new connection is blacklisted
-    if (this.phishingController.test(senderUrl.hostname)) {
-      log.debug('MetaMask - sending phishing warning for', senderUrl.hostname)
-      this.sendPhishingWarning(connectionStream, senderUrl.hostname)
+    if (this.phishingController.test(hostname)) {
+      log.debug('MetaMask - sending phishing warning for', hostname)
+      this.sendPhishingWarning(connectionStream, hostname)
      return
    }

@ -1305,7 +1309,7 @@ module.exports = class MetamaskController extends EventEmitter {
    const mux = setupMultiplex(connectionStream)

    // messages between inpage and background
-    this.setupProviderConnection(mux.createStream('provider'), senderUrl, extensionId)
+    this.setupProviderConnection(mux.createStream('provider'), sender)
    this.setupPublicConfig(mux.createStream('publicConfig'))
  }

@ -1316,15 +1320,14 @@ module.exports = class MetamaskController extends EventEmitter {
   * functions, like the ability to approve transactions or sign messages.
   *
   * @param {*} connectionStream - The duplex stream to connect to.
-   * @param {URL} senderUrl - The URL requesting the connection,
-   * used in logging and error reporting.
+   * @param {MessageSender} sender - The sender of the messages on this stream
   */
-  setupTrustedCommunication (connectionStream, senderUrl) {
+  setupTrustedCommunication (connectionStream, sender) {
    // setup multiplexing
    const mux = setupMultiplex(connectionStream)
    // connect features
    this.setupControllerConnection(mux.createStream('controller'))
-    this.setupProviderConnection(mux.createStream('provider'), senderUrl)
+    this.setupProviderConnection(mux.createStream('provider'), sender, true)
  }

  /**
@ -1379,14 +1382,23 @@ module.exports = class MetamaskController extends EventEmitter {
  /**
   * A method for serving our ethereum provider over a given stream.
   * @param {*} outStream - The stream to provide over.
-   * @param {URL} senderUrl - The URI of the requesting resource.
-   * @param {string} extensionId - The id of the extension, if the requesting
-   * resource is an extension.
-   * @param {object} publicApi - The public API
+   * @param {MessageSender} sender - The sender of the messages on this stream
+   * @param {boolean} isInternal - True if this is a connection with an internal process
   */
-  setupProviderConnection (outStream, senderUrl, extensionId) {
-    const origin = senderUrl.hostname
-    const engine = this.setupProviderEngine(senderUrl, extensionId)
+  setupProviderConnection (outStream, sender, isInternal) {
+    const origin = isInternal
+      ? 'metamask'
+      : (new URL(sender.url)).hostname
+    let extensionId
+    if (sender.id !== extension.runtime.id) {
+      extensionId = sender.id
+    }
+    let tabId
+    if (sender.tab && sender.tab.id) {
+      tabId = sender.tab.id
+    }
+
+    const engine = this.setupProviderEngine({ origin, location: sender.url, extensionId, tabId })

    // setup connection
    const providerStream = createEngineStream({ engine })
@ -1414,10 +1426,13 @@ module.exports = class MetamaskController extends EventEmitter {

  /**
   * A method for creating a provider that is safely restricted for the requesting domain.
+   * @param {Object} options - Provider engine options
+   * @param {string} options.origin - The hostname of the sender
+   * @param {string} options.location - The full URL of the sender
+   * @param {extensionId} [options.extensionId] - The extension ID of the sender, if the sender is an external extension
+   * @param {tabId} [options.tabId] - The tab ID of the sender - if the sender is within a tab
   **/
-  setupProviderEngine (senderUrl, extensionId) {
-
-    const origin = senderUrl.hostname
+  setupProviderEngine ({ origin, location, extensionId, tabId }) {
    // setup json rpc engine stack
    const engine = new RpcEngine()
    const provider = this.provider
@ -1434,6 +1449,11 @@ module.exports = class MetamaskController extends EventEmitter {
    engine.push(createOriginMiddleware({ origin }))
    // logging
    engine.push(createLoggerMiddleware({ origin }))
+    engine.push(createOnboardingMiddleware({
+      location,
+      tabId,
+      registerOnboarding: this.onboardingController.registerOnboarding,
+    }))
    // filter and subscription polyfills
    engine.push(filterMiddleware)
    engine.push(subscriptionManager.middleware)
@ -1473,45 +1493,6 @@ module.exports = class MetamaskController extends EventEmitter {
    )
  }

-  // manage external connections
-
-  onMessage (message, sender, sendResponse) {
-    if (!message || !message.type) {
-      log.debug(`Ignoring invalid message: '${JSON.stringify(message)}'`)
-      return
-    }
-
-    let handleMessage
-
-    try {
-      if (message.type === 'metamask:registerOnboarding') {
-        assert(sender.tab, 'Missing tab from sender')
-        assert(sender.tab.id && sender.tab.id !== extension.tabs.TAB_ID_NONE, 'Missing tab ID from sender')
-        assert(message.location, 'Missing location from message')
-
-        handleMessage = this.onboardingController.registerOnboarding(message.location, sender.tab.id)
-      } else {
-        throw new Error(`Unrecognized message type: '${message.type}'`)
-      }
-    } catch (error) {
-      console.error(error)
-      sendResponse(error)
-      return true
-    }
-
-    if (handleMessage) {
-      handleMessage
-        .then(() => {
-          sendResponse(null, true)
-        })
-        .catch((error) => {
-          console.error(error)
-          sendResponse(error)
-        })
-      return true
-    }
-  }
-
  /**
   * Adds a reference to a connection by origin. Ignores the 'metamask' origin.
   * Caller must ensure that the returned id is stored such that the reference
--- a/package.json
+++ b/package.json
@ -192,8 +192,8 @@
    "@babel/preset-env": "^7.5.5",
    "@babel/preset-react": "^7.0.0",
    "@babel/register": "^7.5.5",
-    "@metamask/forwarder": "^1.0.0",
-    "@metamask/onboarding": "^0.1.2",
+    "@metamask/forwarder": "^1.1.0",
+    "@metamask/onboarding": "^0.2.0",
    "@sentry/cli": "^1.30.3",
    "@storybook/addon-actions": "^5.2.6",
    "@storybook/addon-info": "^5.1.1",
--- a/test/unit/app/controllers/metamask-controller-test.js
+++ b/test/unit/app/controllers/metamask-controller-test.js
@ -30,8 +30,15 @@ class ThreeBoxControllerMock {
  }
 }

+const ExtensionizerMock = {
+  runtime: {
+    id: 'fake-extension-id',
+  },
+}
+
 const MetaMaskController = proxyquire('../../../../app/scripts/metamask-controller', {
  './controllers/threebox': ThreeBoxControllerMock,
+  'extensionizer': ExtensionizerMock,
 })

 const currentNetworkId = 42
@ -783,7 +790,10 @@ describe('MetaMaskController', function () {
  describe('#setupUntrustedCommunication', function () {
    let streamTest

-    const phishingUrl = new URL('http://myethereumwalletntw.com')
+    const phishingMessageSender = {
+      url: 'http://myethereumwalletntw.com',
+      tab: {},
+    }

    afterEach(function () {
      streamTest.end()
@ -798,11 +808,11 @@ describe('MetaMaskController', function () {
        if (chunk.name !== 'phishing') {
          return cb()
        }
-        assert.equal(chunk.data.hostname, phishingUrl.hostname)
+        assert.equal(chunk.data.hostname, (new URL(phishingMessageSender.url)).hostname)
        resolve()
        cb()
      })
-      metamaskController.setupUntrustedCommunication(streamTest, phishingUrl)
+      metamaskController.setupUntrustedCommunication(streamTest, phishingMessageSender)

      await promise
    })
@ -816,13 +826,17 @@ describe('MetaMaskController', function () {
    })

    it('sets up controller dnode api for trusted communication', function (done) {
+      const messageSender = {
+        url: 'http://mycrypto.com',
+        tab: {},
+      }
      streamTest = createThoughStream((chunk, _, cb) => {
        assert.equal(chunk.name, 'controller')
        cb()
        done()
      })

-      metamaskController.setupTrustedCommunication(streamTest, 'mycrypto.com')
+      metamaskController.setupTrustedCommunication(streamTest, messageSender)
    })
  })

--- a/yarn.lock
+++ b/yarn.lock
@ -1992,15 +1992,15 @@
    scroll "^2.0.3"
    warning "^3.0.0"

-"@metamask/forwarder@^1.0.0":
-  version "1.0.0"
-  resolved "https://registry.yarnpkg.com/@metamask/forwarder/-/forwarder-1.0.0.tgz#3e321022a36561cc6e7b7c84df25f600925f4d95"
-  integrity sha512-ufgPndhZz0oNhRrixiR6cXH/HwtFwurWvbrU8zAZsFnf1hB4L2VB2Wey/P1wStIx+BJJQjyROvCDyPDoz4ny1A==
+"@metamask/forwarder@^1.1.0":
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/@metamask/forwarder/-/forwarder-1.1.0.tgz#13829d8244bbf19ea658c0b20d21a77b67de0bdd"
+  integrity sha512-Hggj4y0QIjDzKGTXzarhEPIQyFSB2bi2y6YLJNwaT4JmP30UB5Cj6gqoY0M4pj3QT57fzp0BUuGp7F/AUe28tw==

-"@metamask/onboarding@^0.1.2":
-  version "0.1.2"
-  resolved "https://registry.yarnpkg.com/@metamask/onboarding/-/onboarding-0.1.2.tgz#d5126cbb5e593d782645d6236c497e27bd38d3c4"
-  integrity sha512-+85Z5OxckGuYr5cCoMlpxASu9geJBMYvwkNWqa5qDDEYKZ8eNXHsADcVYFsvBhxFcf87dC7ty1kWljYVEfTIIA==
+"@metamask/onboarding@^0.2.0":
+  version "0.2.0"
+  resolved "https://registry.yarnpkg.com/@metamask/onboarding/-/onboarding-0.2.0.tgz#9594f6a9a1c779083d71434b9f5e6a973af941f7"
+  integrity sha512-QoMV1Gf1j3LxFhSb5gxudHOIywQQ/su8vPQ1ByC7ocQCVlZb1JqZ/+TYyoIzR2OTi1NPelhYHT3UMdhPozIAhA==
  dependencies:
    bowser "^2.5.4"