Code bath

4 files changed, 49 insertions, 33 deletions

diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js
index 0244f6fa0..0723c03f7 100644
--- a/app/scripts/contentscript.js
+++ b/app/scripts/contentscript.js
@@ -12,7 +12,7 @@ const TransformStream = require('stream').Transform
 const inpageContent = fs.readFileSync(path.join(__dirname, '..', '..', 'dist', 'chrome', 'inpage.js')).toString()
 const inpageSuffix = '//# sourceURL=' + extension.extension.getURL('inpage.js') + '\n'
 const inpageBundle = inpageContent + inpageSuffix
-let originApproved = false
+let isEnabled = false
 
 // Eventually this streaming injection could be replaced with:
 // https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.exportFunction
@@ -40,7 +40,7 @@ function injectScript (content) {
     scriptTag.textContent = content
     container.insertBefore(scriptTag, container.children[0])
   } catch (e) {
-    console.error('Metamask script injection failed.', e)
+    console.error('MetaMask script injection failed', e)
   }
 }
 
@@ -57,12 +57,11 @@ function setupStreams () {
   const pluginPort = extension.runtime.connect({ name: 'contentscript' })
   const pluginStream = new PortStream(pluginPort)
 
-  // Until this origin is approved, cut-off publicConfig stream writes at the content
-  // script level so malicious sites can't snoop on the currently-selected address
+  // Filter out selectedAddress until this origin is enabled
   const approvalTransform = new TransformStream({
     objectMode: true,
     transform: (data, _, done) => {
-      if (typeof data === 'object' && data.name && data.name === 'publicConfig' && !originApproved) {
+      if (typeof data === 'object' && data.name && data.name === 'publicConfig' && !isEnabled) {
         data.data.selectedAddress = undefined
       }
       done(null, { ...data })
@@ -117,7 +116,7 @@ function setupStreams () {
  * Establishes listeners for requests to fully-enable the provider from the dapp context
  * and for full-provider approvals and rejections from the background script context. Dapps
  * should not post messages directly and should instead call provider.enable(), which
- * handles posting these messages automatically.
+ * handles posting these messages internally.
  */
 function listenForProviderRequest () {
   window.addEventListener('message', ({ source, data }) => {
@@ -143,11 +142,10 @@ function listenForProviderRequest () {
     }
   })
 
-  extension.runtime.onMessage.addListener(({ action, isEnabled, isApproved, isUnlocked }) => {
-    if (!action) { return }
+  extension.runtime.onMessage.addListener(({ action = '', isApproved, isUnlocked }) => {
     switch (action) {
       case 'approve-provider-request':
-        originApproved = true
+        isEnabled = true
         injectScript(`window.dispatchEvent(new CustomEvent('ethereumprovider', { detail: {}}))`)
         break
       case 'reject-provider-request':
@@ -160,6 +158,7 @@ function listenForProviderRequest () {
         injectScript(`window.dispatchEvent(new CustomEvent('metamaskisunlocked', { detail: { isUnlocked: ${isUnlocked}}}))`)
         break
       case 'metamask-set-locked':
+        isEnabled = false
         injectScript(`window.dispatchEvent(new CustomEvent('metamasksetlocked', { detail: {}}))`)
         break
     }
diff --git a/app/scripts/controllers/provider-approval.js b/app/scripts/controllers/provider-approval.js
index 3af165438..42393de85 100644
--- a/app/scripts/controllers/provider-approval.js
+++ b/app/scripts/controllers/provider-approval.js
@@ -9,29 +9,29 @@ class ProviderApprovalController {
    *
    * @param {Object} [config] - Options to configure controller
    */
-  constructor ({ closePopup, openPopup, keyringController, platform, preferencesController, publicConfigStore } = {}) {
-    this.store = new ObservableStore()
+  constructor ({ closePopup, keyringController, openPopup, platform, preferencesController, publicConfigStore } = {}) {
+    this.approvedOrigins = {}
     this.closePopup = closePopup
+    this.keyringController = keyringController
     this.openPopup = openPopup
     this.platform = platform
-    this.publicConfigStore = publicConfigStore
-    this.approvedOrigins = {}
     this.preferencesController = preferencesController
-    this.keyringController = keyringController
-    platform && platform.addMessageListener && platform.addMessageListener(({ action, origin }) => {
-      if (!action) { return }
+    this.publicConfigStore = publicConfigStore
+    this.store = new ObservableStore()
+
+    platform && platform.addMessageListener && platform.addMessageListener(({ action = '', origin }) => {
       switch (action) {
         case 'init-provider-request':
-          this.handleProviderRequest(origin)
+          this._handleProviderRequest(origin)
           break
         case 'init-is-approved':
-          this.handleIsApproved(origin)
+          this._handleIsApproved(origin)
           break
         case 'init-is-unlocked':
-          this.handleIsUnlocked()
+          this._handleIsUnlocked()
           break
         case 'init-privacy-request':
-          this.handlePrivacyStatusRequest()
+          this._handlePrivacyRequest()
           break
       }
     })
@@ -42,7 +42,7 @@ class ProviderApprovalController {
    *
    * @param {string} origin - Origin of the window requesting full provider access
    */
-  handleProviderRequest (origin) {
+  _handleProviderRequest (origin) {
     this.store.updateState({ providerRequests: [{ origin }] })
     const isUnlocked = this.keyringController.memStore.getState().isUnlocked
     if (isUnlocked && this.isApproved(origin)) {
@@ -53,21 +53,27 @@ class ProviderApprovalController {
   }
 
   /**
-   * Called by a tab to determine if a full Ethereum provider API is exposed
+   * Called by a tab to determine if an origin has been approved in the past
    *
-   * @param {string} origin - Origin of the window requesting provider status
+   * @param {string} origin - Origin of the window
    */
-  async handleIsApproved (origin) {
+  _handleIsApproved (origin) {
     const isApproved = this.isApproved(origin)
     this.platform && this.platform.sendMessage({ action: 'answer-is-approved', isApproved }, { active: true })
   }
 
-  handleIsUnlocked () {
+  /**
+   * Called by a tab to determine if MetaMask is currently locked or unlocked
+   */
+  _handleIsUnlocked () {
     const isUnlocked = this.keyringController.memStore.getState().isUnlocked
     this.platform && this.platform.sendMessage({ action: 'answer-is-unlocked', isUnlocked }, { active: true })
   }
 
-  handlePrivacyStatusRequest () {
+  /**
+   * Called to check privacy mode; if privacy mode is off, this will automatically enable the provider (legacy behavior)
+   */
+  _handlePrivacyRequest () {
     const privacyMode = this.preferencesController.getFeatureFlags().privacyMode
     if (!privacyMode) {
       this.platform && this.platform.sendMessage({ action: 'approve-provider-request' }, { active: true })
@@ -121,6 +127,10 @@ class ProviderApprovalController {
     return !privacyMode || this.approvedOrigins[origin]
   }
 
+  /**
+   * Tells all tabs that MetaMask is now locked. This is primarily used to set
+   * internal flags in the contentscript and inpage script.
+   */
   setLocked () {
     this.platform.sendMessage({ action: 'metamask-set-locked' })
   }
diff --git a/app/scripts/inpage.js b/app/scripts/inpage.js
index 49a18c5e9..a5e0118d4 100644
--- a/app/scripts/inpage.js
+++ b/app/scripts/inpage.js
@@ -6,14 +6,18 @@ const LocalMessageDuplexStream = require('post-message-stream')
 const setupDappAutoReload = require('./lib/auto-reload.js')
 const MetamaskInpageProvider = require('metamask-inpage-provider')
 
+let isEnabled = false
+let warned = false
+
 restoreContextAfterImports()
 
 log.setDefaultLevel(process.env.METAMASK_DEBUG ? 'debug' : 'warn')
 
 console.warn('ATTENTION: In an effort to improve user privacy, MetaMask ' +
-'stopped exposing user accounts to dapps by default beginning November 2nd, 2018. ' +
-'Dapps should call provider.enable() in order to view and use accounts. Please see ' +
-'https://bit.ly/2QQHXvF for complete information and up-to-date example code.')
+'stopped exposing user accounts to dapps if "privacy mode" is enabled on ' +
+'November 2nd, 2018. Dapps should now call provider.enable() in order to view and use ' +
+'accounts. Please see https://bit.ly/2QQHXvF for complete information and up-to-date ' +
+'example code.')
 
 //
 // setup plugin communication
@@ -30,9 +34,8 @@ var inpageProvider = new MetamaskInpageProvider(metamaskStream)
 
 // set a high max listener count to avoid unnecesary warnings
 inpageProvider.setMaxListeners(100)
-var isEnabled = false
-var warned = false
 
+// set up a listener for when MetaMask is locked
 window.addEventListener('metamasksetlocked', () => {
   isEnabled = false
 })
@@ -44,6 +47,7 @@ inpageProvider.enable = function () {
       if (typeof detail.error !== 'undefined') {
         reject(detail.error)
       } else {
+        // wait for the publicConfig store to populate with an account
         const publicConfig = new Promise((resolve) => {
           const { selectedAddress } = inpageProvider.publicConfigStore.getState()
           if (selectedAddress) {
@@ -55,6 +59,7 @@ inpageProvider.enable = function () {
           }
         })
 
+        // wait for the background to update with an accoount
         const ethAccounts = new Promise((resolveAccounts, rejectAccounts) => {
           inpageProvider.sendAsync({ method: 'eth_accounts', params: [] }, (error, response) => {
             if (error) {
@@ -143,7 +148,6 @@ const proxiedInpageProvider = new Proxy(inpageProvider, {
 
 window.ethereum = proxiedInpageProvider
 
-
 //
 // setup web3
 //
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index bf1df7ff5..33278db85 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -222,11 +222,11 @@ module.exports = class MetamaskController extends EventEmitter {
 
     this.providerApprovalController = new ProviderApprovalController({
       closePopup: opts.closePopup,
+      keyringController: this.keyringController,
       openPopup: opts.openPopup,
       platform: opts.platform,
       preferencesController: this.preferencesController,
       publicConfigStore: this.publicConfigStore,
-      keyringController: this.keyringController,
     })
 
     this.store.updateStructure({
@@ -1577,6 +1577,9 @@ module.exports = class MetamaskController extends EventEmitter {
     return this.blacklistController.whitelistDomain(hostname)
   }
 
+  /**
+   * Locks MetaMask
+   */
   setLocked() {
     this.providerApprovalController.setLocked()
     return this.keyringController.setLocked()