Login per site onboarding

app/scripts/background.js

@@ -316,7 +316,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,
@@ -356,10 +355,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
@@ -406,13 +402,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)
   }
 
   //

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)

app/scripts/controllers/onboarding.js

@@ -64,7 +64,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

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

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')
@@ -1285,28 +1285,32 @@ 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
     }
 
     // setup multiplexing
     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'))
   }
 
@@ -1317,15 +1321,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)
   }
 
   /**
@@ -1380,14 +1383,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
-   */
-  setupProviderConnection (outStream, senderUrl, extensionId) {
-    const origin = senderUrl.hostname
-    const engine = this.setupProviderEngine(senderUrl, extensionId)
+   * @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, 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 })
@@ -1415,10 +1427,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
@@ -1435,6 +1450,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)
@@ -1474,45 +1494,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

package.json

@@ -191,8 +191,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",

test/unit/app/controllers/metamask-controller-test.js

@@ -32,8 +32,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
@@ -805,7 +812,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()
@@ -820,11 +830,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
     })
@@ -838,13 +848,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)
     })
   })