From c26d27264981fa39c86cbea1d972a9955a0f2ef8 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Thu, 16 Apr 2020 15:58:36 -0300
Subject: [PATCH] Synchronously validate accounts (#8343)

Now that identities are available synchronously in the permissions
controller, accounts can be validated synchronously as well. Any
account the user wants to give permissions to should already be tracked
as an identity in the preferences controller.
---
 app/scripts/controllers/permissions/index.js  |  9 ++---
 .../permissions-controller-test.js            | 40 +++++++++----------
 2 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/app/scripts/controllers/permissions/index.js b/app/scripts/controllers/permissions/index.js
index b33349063..97503ec78 100644
--- a/app/scripts/controllers/permissions/index.js
+++ b/app/scripts/controllers/permissions/index.js
@@ -325,7 +325,7 @@ export class PermissionsController {
 
     if (ethAccounts) {
 
-      await this.validatePermittedAccounts(finalizedAccounts)
+      this.validatePermittedAccounts(finalizedAccounts)
 
       if (!ethAccounts.caveats) {
         ethAccounts.caveats = []
@@ -354,16 +354,15 @@ export class PermissionsController {
    *
    * @param {string[]} accounts - An array of addresses.
    */
-  async validatePermittedAccounts (accounts) {
-
+  validatePermittedAccounts (accounts) {
     if (!Array.isArray(accounts) || accounts.length === 0) {
       throw new Error('Must provide non-empty array of account(s).')
     }
 
     // assert accounts exist
-    const allAccounts = await this.getKeyringAccounts()
+    const allIdentities = this._getIdentities()
     accounts.forEach((acc) => {
-      if (!allAccounts.includes(acc)) {
+      if (!allIdentities[acc]) {
         throw new Error(`Unknown account: ${acc}`)
       }
     })
diff --git a/test/unit/app/controllers/permissions/permissions-controller-test.js b/test/unit/app/controllers/permissions/permissions-controller-test.js
index ac3b54865..58cd0f552 100644
--- a/test/unit/app/controllers/permissions/permissions-controller-test.js
+++ b/test/unit/app/controllers/permissions/permissions-controller-test.js
@@ -389,26 +389,26 @@ describe('permissions controller', function () {
 
     it('throws error on non-array accounts', async function () {
 
-      await assert.rejects(
-        permController.validatePermittedAccounts(undefined),
+      await assert.throws(
+        () => permController.validatePermittedAccounts(undefined),
         ERRORS.validatePermittedAccounts.invalidParam(),
         'should throw on undefined'
       )
 
-      await assert.rejects(
-        permController.validatePermittedAccounts(false),
+      await assert.throws(
+        () => permController.validatePermittedAccounts(false),
         ERRORS.validatePermittedAccounts.invalidParam(),
         'should throw on false'
       )
 
-      await assert.rejects(
-        permController.validatePermittedAccounts(true),
+      await assert.throws(
+        () => permController.validatePermittedAccounts(true),
         ERRORS.validatePermittedAccounts.invalidParam(),
         'should throw on true'
       )
 
-      await assert.rejects(
-        permController.validatePermittedAccounts({}),
+      await assert.throws(
+        () => permController.validatePermittedAccounts({}),
         ERRORS.validatePermittedAccounts.invalidParam(),
         'should throw on non-array object'
       )
@@ -416,8 +416,8 @@ describe('permissions controller', function () {
 
     it('throws error on empty array of accounts', async function () {
 
-      await assert.rejects(
-        permController.validatePermittedAccounts([]),
+      await assert.throws(
+        () => permController.validatePermittedAccounts([]),
         ERRORS.validatePermittedAccounts.invalidParam(),
         'should throw on empty array'
       )
@@ -427,14 +427,14 @@ describe('permissions controller', function () {
 
       const keyringAccounts = await permController.getKeyringAccounts()
 
-      await assert.rejects(
-        permController.validatePermittedAccounts([DUMMY_ACCOUNT]),
+      await assert.throws(
+        () => permController.validatePermittedAccounts([DUMMY_ACCOUNT]),
         ERRORS.validatePermittedAccounts.nonKeyringAccount(DUMMY_ACCOUNT),
         'should throw on non-keyring account'
       )
 
-      await assert.rejects(
-        permController.validatePermittedAccounts(keyringAccounts.concat(DUMMY_ACCOUNT)),
+      await assert.throws(
+        () => permController.validatePermittedAccounts(keyringAccounts.concat(DUMMY_ACCOUNT)),
         ERRORS.validatePermittedAccounts.nonKeyringAccount(DUMMY_ACCOUNT),
         'should throw on non-keyring account with other accounts'
       )
@@ -444,18 +444,18 @@ describe('permissions controller', function () {
 
       const keyringAccounts = await permController.getKeyringAccounts()
 
-      await assert.doesNotReject(
-        permController.validatePermittedAccounts(keyringAccounts),
+      await assert.doesNotThrow(
+        () => permController.validatePermittedAccounts(keyringAccounts),
         'should not throw on all keyring accounts'
       )
 
-      await assert.doesNotReject(
-        permController.validatePermittedAccounts([ keyringAccounts[0] ]),
+      await assert.doesNotThrow(
+        () => permController.validatePermittedAccounts([ keyringAccounts[0] ]),
         'should not throw on single keyring account'
       )
 
-      await assert.doesNotReject(
-        permController.validatePermittedAccounts([ keyringAccounts[1] ]),
+      await assert.doesNotThrow(
+        () => permController.validatePermittedAccounts([ keyringAccounts[1] ]),
         'should not throw on single keyring account'
       )
     })