Manually connect via the full connect flow (#8666)

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'
@@ -95,7 +96,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: {} },
       ),
     }))
 
@@ -106,6 +107,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
@@ -155,10 +167,10 @@ 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
@@ -166,7 +178,7 @@ export class PermissionsController {
       const req = { method: 'wallet_requestPermissions', params: [permissions] }
       const res = {}
       this.permissions.providerMiddlewareFunction(
-        { origin }, req, res, () => {}, _end
+        metadata, req, res, () => {}, _end
       )
 
       function _end (_err) {
@@ -244,70 +256,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()
-          }
-        }
-      })
-
-      const newPermittedAccounts = await this.getAccounts(origin)
-
-      this.notifyDomain(origin, {
-        method: NOTIFICATION_NAMES.accountsChanged,
-        result: newPermittedAccounts,
-      })
-      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

@@ -380,20 +380,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 {
@@ -440,14 +426,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

@@ -716,119 +716,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, ACCOUNTS.a.permitted)
-
-      aAccounts = await permController.getAccounts(ORIGINS.a)
-      assert.deepEqual(aAccounts, [ACCOUNTS.a.primary], '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),
-        ACCOUNTS.a.permitted,
-        'should have expected eth_accounts entry accounts'
-      )
-
-      // notification should also have been sent
-      assert.deepEqual(
-        notifications[ORIGINS.a][0],
-        NOTIFICATIONS.newAccounts([ACCOUNTS.a.primary]),
-        '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(ACCOUNTS.a.permitted)
-      )
-
-      const aAccounts = await permController.getAccounts(ORIGINS.a)
-      assert.deepEqual(aAccounts, [ACCOUNTS.a.primary], 'origin should have correct accounts')
-
-      await assert.rejects(
-        permController.legacyExposeAccounts(ORIGINS.a, ACCOUNTS.b.permitted),
-        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, ACCOUNTS.a.permitted),
-        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
@@ -1504,6 +1391,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('', ACCOUNTS.a.permitted)
-        },
-        ERRORS.logAccountExposure.invalidParams(),
-        'should throw expected error'
-      )
-
-      assert.throws(
-        () => {
-          permLog.logAccountExposure(null, ACCOUNTS.a.permitted)
-        },
-        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'
-      )
-    })
-  })
 })

ui/app/pages/connected-sites/connected-sites.component.js

@@ -20,11 +20,11 @@ export default class ConnectedSites extends Component {
     disconnectAllAccounts: PropTypes.func.isRequired,
     disconnectAccount: PropTypes.func.isRequired,
     getOpenMetamaskTabsIds: PropTypes.func.isRequired,
-    legacyExposeAccount: PropTypes.func.isRequired,
     permittedAccountsByOrigin: PropTypes.objectOf(
       PropTypes.arrayOf(PropTypes.string),
     ).isRequired,
     tabToConnect: PropTypes.object,
+    requestAccountsPermission: PropTypes.func.isRequired,
   }
 
   state = {
@@ -76,13 +76,12 @@ export default class ConnectedSites extends Component {
   }
 
   renderConnectedSitesPopover () {
-
     const {
       accountLabel,
       closePopover,
       connectedDomains,
-      legacyExposeAccount,
       tabToConnect,
+      requestAccountsPermission,
     } = this.props
     const { t } = this.context
 
@@ -100,7 +99,7 @@ export default class ConnectedSites extends Component {
             ? (
               <a
                 className="connected-sites__text-button"
-                onClick={legacyExposeAccount}
+                onClick={requestAccountsPermission}
               >
                 {t('connectManually')}
               </a>

ui/app/pages/connected-sites/connected-sites.container.js

@@ -2,7 +2,7 @@ import { connect } from 'react-redux'
 import ConnectedSites from './connected-sites.component'
 import {
   getOpenMetamaskTabsIds,
-  legacyExposeAccounts,
+  requestAccountsPermission,
   removePermissionsFor,
   removePermittedAccount,
 } from '../../store/actions'
@@ -14,7 +14,7 @@ import {
   getPermittedAccountsByOrigin,
   getSelectedAddress,
 } from '../../selectors'
-import { DEFAULT_ROUTE } from '../../helpers/constants/routes'
+import { CONNECT_ROUTE, DEFAULT_ROUTE } from '../../helpers/constants/routes'
 
 const mapStateToProps = (state) => {
   const { openMetaMaskTabs } = state.appState
@@ -57,7 +57,7 @@ const mapDispatchToProps = (dispatch) => {
         [domainKey]: permissionMethodNames,
       }))
     },
-    legacyExposeAccounts: (origin, account) => dispatch(legacyExposeAccounts(origin, [account])),
+    requestAccountsPermission: (origin) => dispatch(requestAccountsPermission(origin)),
   }
 }
 
@@ -71,7 +71,7 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => {
   const {
     disconnectAccount,
     disconnectAllAccounts,
-    legacyExposeAccounts: dispatchLegacyExposeAccounts,
+    requestAccountsPermission: dispatchRequestAccountsPermission,
   } = dispatchProps
   const { history } = ownProps
 
@@ -94,7 +94,10 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => {
         closePopover()
       }
     },
-    legacyExposeAccount: () => dispatchLegacyExposeAccounts(tabToConnect.origin, selectedAddress),
+    requestAccountsPermission: async () => {
+      const id = await dispatchRequestAccountsPermission(tabToConnect.origin)
+      history.push(`${CONNECT_ROUTE}/${id}`)
+    },
   }
 }
 

ui/app/store/actions.js

@@ -2035,6 +2035,14 @@ export function setPendingTokens (pendingTokens) {
 
 // Permissions
 
+export function requestAccountsPermission (origin) {
+  return async (dispatch) => {
+    const id = await promisifiedBackground.requestAccountsPermission(origin)
+    await forceUpdateMetamaskState(dispatch)
+    return id
+  }
+}
+
 /**
  * Approves the permissions request.
  * @param {Object} request - The permissions request to approve
@@ -2064,16 +2072,6 @@ export function rejectPermissionsRequest (requestId) {
   }
 }
 
-/**
- * Exposes the given account(s) to the given origin.
- * Call ONLY as a result of direct user action.
- */
-export function legacyExposeAccounts (origin, accounts) {
-  return () => {
-    return background.legacyExposeAccounts(origin, accounts)
-  }
-}
-
 /**
  * Clears the given permissions for the given origin.
  */