From 79d920947375854b383cd7b1caef2d2227f1e175 Mon Sep 17 00:00:00 2001
From: Erik Marks <25517051+rekmarks@users.noreply.github.com>
Date: Tue, 5 May 2020 07:03:21 -0700
Subject: [PATCH] Open notification UI when eth_requestAccounts waits for
 unlock (#8508)

---
 app/scripts/background.js                     |  3 ++-
 app/scripts/controllers/app-state.js          | 27 ++++++++++++++++---
 app/scripts/controllers/permissions/index.js  |  6 ++---
 app/scripts/metamask-controller.js            |  1 +
 .../unit/app/controllers/permissions/mocks.js |  2 +-
 5 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/app/scripts/background.js b/app/scripts/background.js
index 41f1809bb..17e65796c 100644
--- a/app/scripts/background.js
+++ b/app/scripts/background.js
@@ -236,7 +236,8 @@ function setupController (initState, initLangCode) {
     showUnconfirmedMessage: triggerUi,
     showUnapprovedTx: triggerUi,
     showPermissionRequest: triggerUi,
-    openPopup: openPopup,
+    showUnlockRequest: triggerUi,
+    openPopup,
     // initial state
     initState,
     // initial locale code
diff --git a/app/scripts/controllers/app-state.js b/app/scripts/controllers/app-state.js
index 10f234b44..4febe9456 100644
--- a/app/scripts/controllers/app-state.js
+++ b/app/scripts/controllers/app-state.js
@@ -12,6 +12,7 @@ class AppStateController extends EventEmitter {
       isUnlocked,
       initState,
       onInactiveTimeout,
+      showUnlockRequest,
       preferencesStore,
     } = opts
     const { preferences } = preferencesStore.getState()
@@ -29,6 +30,8 @@ class AppStateController extends EventEmitter {
     this.waitingForUnlock = []
     addUnlockListener(this.handleUnlock.bind(this))
 
+    this._showUnlockRequest = showUnlockRequest
+
     preferencesStore.subscribe((state) => {
       this._setInactiveTimeout(state.preferences.autoLockTimeLimit)
     })
@@ -40,20 +43,38 @@ class AppStateController extends EventEmitter {
    * Get a Promise that resolves when the extension is unlocked.
    * This Promise will never reject.
    *
+   * @param {boolean} shouldShowUnlockRequest - Whether the extension notification
+   * popup should be opened.
    * @returns {Promise<void>} A promise that resolves when the extension is
    * unlocked, or immediately if the extension is already unlocked.
    */
-  getUnlockPromise () {
+  getUnlockPromise (shouldShowUnlockRequest) {
     return new Promise((resolve) => {
       if (this.isUnlocked()) {
         resolve()
       } else {
-        this.waitingForUnlock.push({ resolve })
-        this.emit('updateBadge')
+        this.waitForUnlock(resolve, shouldShowUnlockRequest)
       }
     })
   }
 
+  /**
+   * Adds a Promise's resolve function to the waitingForUnlock queue.
+   * Also opens the extension popup if specified.
+   *
+   * @param {Promise.resolve} resolve - A Promise's resolve function that will
+   * be called when the extension is unlocked.
+   * @param {boolean} shouldShowUnlockRequest - Whether the extension notification
+   * popup should be opened.
+   */
+  waitForUnlock (resolve, shouldShowUnlockRequest) {
+    this.waitingForUnlock.push({ resolve })
+    this.emit('updateBadge')
+    if (shouldShowUnlockRequest) {
+      this._showUnlockRequest()
+    }
+  }
+
   /**
    * Drains the waitingForUnlock queue, resolving all the related Promises.
    */
diff --git a/app/scripts/controllers/permissions/index.js b/app/scripts/controllers/permissions/index.js
index f5852687a..4f0b5dff2 100644
--- a/app/scripts/controllers/permissions/index.js
+++ b/app/scripts/controllers/permissions/index.js
@@ -42,7 +42,7 @@ export class PermissionsController {
     })
 
     this.getKeyringAccounts = getKeyringAccounts
-    this.getUnlockPromise = getUnlockPromise
+    this._getUnlockPromise = getUnlockPromise
     this._notifyDomain = notifyDomain
     this.notifyAllDomains = notifyAllDomains
     this._showPermissionRequest = showPermissionRequest
@@ -92,7 +92,7 @@ export class PermissionsController {
       store: this.store,
       storeKey: METADATA_STORE_KEY,
       getAccounts: this.getAccounts.bind(this, origin),
-      getUnlockPromise: this.getUnlockPromise,
+      getUnlockPromise: () => this._getUnlockPromise(true),
       hasPermission: this.hasPermission.bind(this, origin),
       requestAccountsPermission: this._requestPermissions.bind(
         this, origin, { eth_accounts: {} }
@@ -602,7 +602,7 @@ export class PermissionsController {
       this.pendingApprovals.has(id)
     ) {
       throw new Error(
-        `Pending approval with id ${id} or origin ${origin} already exists.`
+        `Pending approval with id '${id}' or origin '${origin}' already exists.`
       )
     }
 
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index b2a25b81d..e3364cece 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -125,6 +125,7 @@ export default class MetamaskController extends EventEmitter {
       isUnlocked: this.isUnlocked.bind(this),
       initState: initState.AppStateController,
       onInactiveTimeout: () => this.setLocked(),
+      showUnlockRequest: opts.showUnlockRequest,
       preferencesStore: this.preferencesController.store,
     })
 
diff --git a/test/unit/app/controllers/permissions/mocks.js b/test/unit/app/controllers/permissions/mocks.js
index 4220e8119..60fc87144 100644
--- a/test/unit/app/controllers/permissions/mocks.js
+++ b/test/unit/app/controllers/permissions/mocks.js
@@ -437,7 +437,7 @@ export const getters = deepFreeze({
     pendingApprovals: {
       duplicateOriginOrId: (id, origin) => {
         return {
-          message: `Pending approval with id ${id} or origin ${origin} already exists.`,
+          message: `Pending approval with id '${id}' or origin '${origin}' already exists.`,
         }
       },
       requestAlreadyPending: () => {