Manually connect via the full connect flow

app/scripts/controllers/permissions/index.js

@@ -1,3 +1,4 @@
+import nanoid from 'nanoid'
 import JsonRpcEngine from 'json-rpc-engine'
 import asMiddleware from 'json-rpc-engine/src/asMiddleware'
 import ObservableStore from 'obs-store'
@@ -94,7 +95,7 @@ export class PermissionsController {
       getUnlockPromise: () => this._getUnlockPromise(true),
       hasPermission: this.hasPermission.bind(this, origin),
       requestAccountsPermission: this._requestPermissions.bind(
-        this, origin, { eth_accounts: {} }
+        this, { origin }, { eth_accounts: {} },
       ),
     }))
 
@@ -105,6 +106,17 @@ export class PermissionsController {
     return asMiddleware(engine)
   }
 
+  /**
+   * Request {@code eth_accounts} permissions
+   * @param {string} origin - The origin
+   * @returns {Promise<string>} the request ID
+   */
+  async requestAccountsPermission (origin) {
+    const id = nanoid()
+    this._requestPermissions({ origin, id }, { eth_accounts: {} })
+    return id
+  }
+
   /**
    * Returns the accounts that should be exposed for the given origin domain,
    * if any. This method exists for when a trusted context needs to know
@@ -154,18 +166,18 @@ export class PermissionsController {
   /**
    * Submits a permissions request to rpc-cap. Internal, background use only.
    *
-   * @param {string} origin - The origin string.
+   * @param {IOriginMetadata} metadata - The origin metadata.
    * @param {IRequestedPermissions} permissions - The requested permissions.
    */
-  _requestPermissions (origin, permissions) {
+  _requestPermissions (metadata, permissions) {
     return new Promise((resolve, reject) => {
 
       // rpc-cap assigns an id to the request if there is none, as expected by
       // requestUserApproval below
       const req = { method: 'wallet_requestPermissions', params: [permissions] }
       const res = {}
       this.permissions.providerMiddlewareFunction(
-        { origin }, req, res, () => {}, _end
+        metadata, req, res, () => {}, _end
       )
 
       function _end (_err) {
@@ -243,68 +255,6 @@ export class PermissionsController {
     this._removePendingApproval(id)
   }
 
-  /**
-   * @deprecated
-   * Grants the given origin the eth_accounts permission for the given account(s).
-   * This method should ONLY be called as a result of direct user action in the UI,
-   * with the intention of supporting legacy dapps that don't support EIP 1102.
-   *
-   * @param {string} origin - The origin to expose the account(s) to.
-   * @param {Array<string>} accounts - The account(s) to expose.
-   */
-  async legacyExposeAccounts (origin, accounts) {
-
-    // accounts are validated by finalizePermissionsRequest
-    if (typeof origin !== 'string' || !origin.length) {
-      throw new Error('Must provide non-empty string origin.')
-    }
-
-    const existingAccounts = await this.getAccounts(origin)
-
-    if (existingAccounts.length > 0) {
-      throw new Error(
-        'May not call legacyExposeAccounts on origin with exposed accounts.'
-      )
-    }
-
-    const permissions = await this.finalizePermissionsRequest(
-      { eth_accounts: {} }, accounts
-    )
-
-    try {
-
-      await new Promise((resolve, reject) => {
-        this.permissions.grantNewPermissions(
-          origin, permissions, {}, _end
-        )
-
-        function _end (err) {
-          if (err) {
-            reject(err)
-          } else {
-            resolve()
-          }
-        }
-      })
-
-      this.notifyDomain(origin, {
-        method: NOTIFICATION_NAMES.accountsChanged,
-        result: accounts,
-      })
-      this.permissionsLog.logAccountExposure(origin, accounts)
-
-    } catch (error) {
-
-      throw ethErrors.rpc.internal({
-        message: `Failed to add 'eth_accounts' to '${origin}'.`,
-        data: {
-          originalError: error,
-          accounts,
-        },
-      })
-    }
-  }
-
   /**
    * Expose an account to the given origin. Changes the eth_accounts
    * permissions and emits accountsChanged.

app/scripts/controllers/permissions/permissionsLog.js

@@ -202,33 +202,6 @@ export default class PermissionsLogController {
     this.updateActivityLog(logs)
   }
 
-  /**
-   * Record account exposure and eth_accounts permissions history for the given
-   * origin.
-   *
-   * @param {string} origin - The origin accounts were exposed to.
-   * @param {Array<string>} accounts - The accounts that were exposed.
-   */
-  logAccountExposure (origin, accounts) {
-
-    if (
-      typeof origin !== 'string' || !origin.length ||
-      !Array.isArray(accounts) || accounts.length === 0
-    ) {
-      throw new Error(
-        'Must provide non-empty string origin and array of accounts.'
-      )
-    }
-
-    this.logPermissionsHistory(
-      ['eth_accounts'],
-      origin,
-      accounts,
-      Date.now(),
-      true
-    )
-  }
-
   /**
    * Create new permissions history log entries, if any, and commit them.
    *

app/scripts/metamask-controller.js

@@ -573,7 +573,7 @@ export default class MetamaskController extends EventEmitter {
       removePermissionsFor: permissionsController.removePermissionsFor.bind(permissionsController),
       addPermittedAccount: nodeify(permissionsController.addPermittedAccount, permissionsController),
       removePermittedAccount: nodeify(permissionsController.removePermittedAccount, permissionsController),
-      legacyExposeAccounts: nodeify(permissionsController.legacyExposeAccounts, permissionsController),
+      requestAccountsPermission: nodeify(permissionsController.requestAccountsPermission, permissionsController),
 
       getRequestAccountTabIds: (cb) => cb(null, this.getRequestAccountTabIds()),
       getOpenMetamaskTabsIds: (cb) => cb(null, this.getOpenMetamaskTabsIds()),

test/unit/app/controllers/permissions/mocks.js

@@ -366,20 +366,6 @@ export const getters = deepFreeze({
       },
     },
 
-    legacyExposeAccounts: {
-      badOrigin: () => {
-        return {
-          message: 'Must provide non-empty string origin.',
-        }
-      },
-      forbiddenUsage: () => {
-        return {
-          name: 'Error',
-          message: 'May not call legacyExposeAccounts on origin with exposed accounts.',
-        }
-      },
-    },
-
     _handleAccountSelected: {
       invalidParams: () => {
         return {
@@ -426,14 +412,6 @@ export const getters = deepFreeze({
       },
     },
 
-    logAccountExposure: {
-      invalidParams: () => {
-        return {
-          message: 'Must provide non-empty string origin and array of accounts.',
-        }
-      },
-    },
-
     pendingApprovals: {
       duplicateOriginOrId: (id, origin) => {
         return {

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

@@ -715,119 +715,6 @@ describe('permissions controller', function () {
     })
   })
 
-  describe('legacyExposeAccounts', function () {
-
-    let permController, notifications
-
-    beforeEach(function () {
-      notifications = initNotifications()
-      permController = initPermController(notifications)
-    })
-
-    it('successfully exposes accounts and updates permissions history', async function () {
-
-      let aAccounts = await permController.getAccounts(ORIGINS.a)
-      assert.deepEqual(aAccounts, [], 'origin should have no accounts')
-
-      await permController.legacyExposeAccounts(ORIGINS.a, ACCOUNT_ARRAYS.a)
-
-      aAccounts = await permController.getAccounts(ORIGINS.a)
-      assert.deepEqual(aAccounts, ACCOUNT_ARRAYS.a, 'origin should have correct accounts')
-
-      // now, permissions history should be updated
-      const permissionsHistory = permController.permissionsLog.getHistory()
-      const historyOrigins = Object.keys(permissionsHistory)
-
-      assert.equal(historyOrigins.length, 1, 'should have single origin')
-      assert.equal(historyOrigins[0], ORIGINS.a, 'should have correct origin')
-
-      assert.ok(
-        permissionsHistory[ORIGINS.a].eth_accounts,
-        'history should have eth_accounts entry'
-      )
-
-      assert.deepEqual(
-        Object.keys(permissionsHistory[ORIGINS.a].eth_accounts.accounts),
-        ACCOUNT_ARRAYS.a,
-        'should have expected eth_accounts entry accounts'
-      )
-
-      // notification should also have been sent
-      assert.deepEqual(
-        notifications[ORIGINS.a][0],
-        NOTIFICATIONS.newAccounts(ACCOUNT_ARRAYS.a),
-        'first origin should have correct notification'
-      )
-    })
-
-    it('throws if called on origin with existing exposed accounts', async function () {
-
-      grantPermissions(
-        permController, ORIGINS.a,
-        PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a)
-      )
-
-      const aAccounts = await permController.getAccounts(ORIGINS.a)
-      assert.deepEqual(aAccounts, ACCOUNT_ARRAYS.a, 'origin should have correct accounts')
-
-      await assert.rejects(
-        permController.legacyExposeAccounts(ORIGINS.a, ACCOUNT_ARRAYS.b),
-        ERRORS.legacyExposeAccounts.forbiddenUsage(),
-        'should throw if called on origin with existing exposed accounts'
-      )
-
-      const permissionsHistory = permController.permissionsLog.getHistory()
-      assert.deepEqual(
-        permissionsHistory, {},
-        'should not have modified history'
-      )
-      assert.deepEqual(
-        notifications[ORIGINS.a], [],
-        'should not have sent notification'
-      )
-    })
-
-    it('throws if called with bad accounts', async function () {
-
-      await assert.rejects(
-        permController.legacyExposeAccounts(ORIGINS.a, []),
-        ERRORS.validatePermittedAccounts.invalidParam(),
-        'should throw if called with no accounts'
-      )
-
-      const permissionsHistory = permController.permissionsLog.getHistory()
-      assert.deepEqual(
-        permissionsHistory, {},
-        'should not have modified history'
-      )
-      assert.deepEqual(
-        notifications[ORIGINS.a], [],
-        'should not have sent notification'
-      )
-    })
-
-    it('throws if called with bad origin', async function () {
-
-      await assert.rejects(
-        permController.legacyExposeAccounts(null, ACCOUNT_ARRAYS.a),
-        ERRORS.legacyExposeAccounts.badOrigin(),
-        'should throw if called with invalid origin'
-      )
-
-      const permissionsHistory = permController.permissionsLog.getHistory()
-      assert.deepEqual(
-        permissionsHistory, {},
-        'should not have modified history'
-      )
-      Object.keys(notifications).forEach((domain) => {
-        assert.deepEqual(
-          notifications[domain], [],
-          'should not have sent notification'
-        )
-      })
-    })
-  })
-
   describe('preferences state update', function () {
 
     let permController, notifications, preferences, identities
@@ -1506,6 +1393,16 @@ describe('permissions controller', function () {
       permController = initPermController()
     })
 
+    it('requestAccountsPermission calls _requestAccountsPermission with an explicit request ID', async function () {
+      const _requestPermissions = sinon.stub(permController, '_requestPermissions').resolves()
+      await permController.requestAccountsPermission('example.com')
+      assert.ok(_requestPermissions.calledOnceWithExactly(
+        sinon.match.object.and(sinon.match.has('origin')).and(sinon.match.has('id')),
+        { eth_accounts: {} },
+      ))
+      _requestPermissions.restore()
+    })
+
     it('_addPendingApproval: should throw if adding origin twice', function () {
 
       const id = nanoid()

test/unit/app/controllers/permissions/permissions-log-controller-test.js

@@ -22,7 +22,6 @@ import {
 } from './mocks'
 
 const {
-  ERRORS,
   PERMS,
   RPC_REQUESTS,
 } = getters
@@ -647,44 +646,4 @@ describe('permissions log', function () {
       )
     })
   })
-
-  describe('instance method edge cases', function () {
-
-    it('logAccountExposure errors on invalid params', function () {
-
-      const permLog = initPermLog()
-
-      assert.throws(
-        () => {
-          permLog.logAccountExposure('', ACCOUNT_ARRAYS.a)
-        },
-        ERRORS.logAccountExposure.invalidParams(),
-        'should throw expected error'
-      )
-
-      assert.throws(
-        () => {
-          permLog.logAccountExposure(null, ACCOUNT_ARRAYS.a)
-        },
-        ERRORS.logAccountExposure.invalidParams(),
-        'should throw expected error'
-      )
-
-      assert.throws(
-        () => {
-          permLog.logAccountExposure('foo', {})
-        },
-        ERRORS.logAccountExposure.invalidParams(),
-        'should throw expected error'
-      )
-
-      assert.throws(
-        () => {
-          permLog.logAccountExposure('foo', [])
-        },
-        ERRORS.logAccountExposure.invalidParams(),
-        'should throw expected error'
-      )
-    })
-  })
 })