From 15616a33cabe28a38b6debc526d288f470a75a74 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 16 Apr 2020 19:23:36 -0300 Subject: [PATCH] Add 'addPermittedAccount' method to permissions controller (#8344) This method adds the given account to the given origin's list of exposed accounts. This method is not yet used, but it will be in subsequent PRs (e.g. #8312) This method has been added to the background API, and a wrapper action creator has been written as well. --- app/scripts/controllers/permissions/index.js | 50 +++++++++++ app/scripts/metamask-controller.js | 1 + .../unit/app/controllers/permissions/mocks.js | 18 ++++ .../permissions-controller-test.js | 87 +++++++++++++++++++ ui/app/store/actions.js | 14 +++ 5 files changed, 170 insertions(+) diff --git a/app/scripts/controllers/permissions/index.js b/app/scripts/controllers/permissions/index.js index 97503ec78..9f5f6cd45 100644 --- a/app/scripts/controllers/permissions/index.js +++ b/app/scripts/controllers/permissions/index.js @@ -306,6 +306,40 @@ export class PermissionsController { } } + /** + * Expose an account to the given origin. Changes the eth_accounts + * permissions and emits accountsChanged. + * + * Throws error if the origin or account is invalid, or if the update fails. + * + * @param {string} origin - The origin to expose the account to. + * @param {string} account - The new account to expose. + */ + async addPermittedAccount (origin, account) { + const domains = this.permissions.getDomains() + if (!domains[origin]) { + throw new Error('Unrecognized domain') + } + this.validatePermittedAccounts([account]) + + const oldPermittedAccounts = this._getPermittedAccounts(origin) + if (!oldPermittedAccounts) { + throw new Error('Origin does not have \'eth_accounts\' permission') + } else if (oldPermittedAccounts.includes(account)) { + throw new Error('Account is already permitted') + } + + this.permissions.updateCaveatFor( + origin, 'eth_accounts', CAVEAT_NAMES.exposedAccounts, [...oldPermittedAccounts, account] + ) + const permittedAccounts = await this.getAccounts(origin) + + this.notifyDomain(origin, { + method: NOTIFICATION_NAMES.accountsChanged, + result: permittedAccounts, + }) + } + /** * Finalizes a permissions request. Throws if request validation fails. * Clones the passed-in parameters to prevent inadvertent modification. @@ -419,6 +453,22 @@ export class PermissionsController { }) } + /** + * Get current set of permitted accounts for the given origin + * + * @param {string} origin - The origin to obtain permitted accounts for + * @returns {Array|null} The list of permitted accounts + */ + _getPermittedAccounts (origin) { + const permittedAccounts = this.permissions + .getPermission(origin, 'eth_accounts') + ?.caveats + ?.find((caveat) => caveat.name === CAVEAT_NAMES.exposedAccounts) + ?.value + + return permittedAccounts || null + } + /** * When a new account is selected in the UI, emit accountsChanged to each origin * where the selected account is exposed. diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 63f9039ed..370e58251 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -576,6 +576,7 @@ export default class MetamaskController extends EventEmitter { getApprovedAccounts: nodeify(permissionsController.getAccounts.bind(permissionsController)), rejectPermissionsRequest: nodeify(permissionsController.rejectPermissionsRequest, permissionsController), removePermissionsFor: permissionsController.removePermissionsFor.bind(permissionsController), + addPermittedAccount: nodeify(permissionsController.addPermittedAccount, permissionsController), legacyExposeAccounts: nodeify(permissionsController.legacyExposeAccounts, permissionsController), getRequestAccountTabIds: (cb) => cb(null, this.getRequestAccountTabIds()), diff --git a/test/unit/app/controllers/permissions/mocks.js b/test/unit/app/controllers/permissions/mocks.js index 7e44dc62b..dbc08dfc0 100644 --- a/test/unit/app/controllers/permissions/mocks.js +++ b/test/unit/app/controllers/permissions/mocks.js @@ -330,6 +330,24 @@ export const getters = deepFreeze({ }, }, + addPermittedAccount: { + alreadyPermitted: () => { + return { + message: 'Account is already permitted', + } + }, + invalidOrigin: () => { + return { + message: 'Unrecognized domain', + } + }, + noEthAccountsPermission: () => { + return { + message: 'Origin does not have \'eth_accounts\' permission', + } + }, + }, + legacyExposeAccounts: { badOrigin: () => { return { diff --git a/test/unit/app/controllers/permissions/permissions-controller-test.js b/test/unit/app/controllers/permissions/permissions-controller-test.js index 58cd0f552..80c1e7341 100644 --- a/test/unit/app/controllers/permissions/permissions-controller-test.js +++ b/test/unit/app/controllers/permissions/permissions-controller-test.js @@ -461,6 +461,93 @@ describe('permissions controller', function () { }) }) + describe('addPermittedAccount', function () { + let permController, notifications + + beforeEach(function () { + notifications = initNotifications() + permController = initPermController(notifications) + grantPermissions( + permController, ORIGINS.a, + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a) + ) + grantPermissions( + permController, ORIGINS.b, + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.b) + ) + }) + + it('should throw if account is not a string', async function () { + await assert.rejects( + () => permController.addPermittedAccount(ORIGINS.a, {}), + ERRORS.validatePermittedAccounts.nonKeyringAccount({}), + 'should throw on non-string account param' + ) + }) + + it('should throw if given account is not in keyring', async function () { + await assert.rejects( + () => permController.addPermittedAccount(ORIGINS.a, DUMMY_ACCOUNT), + ERRORS.validatePermittedAccounts.nonKeyringAccount(DUMMY_ACCOUNT), + 'should throw on non-keyring account' + ) + }) + + it('should throw if origin is invalid', async function () { + await assert.rejects( + () => permController.addPermittedAccount(false, EXTRA_ACCOUNT), + ERRORS.addPermittedAccount.invalidOrigin(), + 'should throw on invalid origin' + ) + }) + + it('should throw if origin lacks any permissions', async function () { + await assert.rejects( + () => permController.addPermittedAccount(ORIGINS.c, EXTRA_ACCOUNT), + ERRORS.addPermittedAccount.invalidOrigin(), + 'should throw on origin without permissions' + ) + }) + + it('should throw if origin lacks eth_accounts permission', async function () { + grantPermissions( + permController, ORIGINS.c, + PERMS.finalizedRequests.test_method() + ) + + await assert.rejects( + () => permController.addPermittedAccount(ORIGINS.c, EXTRA_ACCOUNT), + ERRORS.addPermittedAccount.noEthAccountsPermission(), + 'should throw on origin without eth_accounts permission' + ) + }) + + it('should throw if account is already permitted', async function () { + await assert.rejects( + () => permController.addPermittedAccount(ORIGINS.a, ACCOUNT_ARRAYS.c[0]), + ERRORS.addPermittedAccount.alreadyPermitted(), + 'should throw if account is already permitted' + ) + }) + + it('should successfully add permitted account', async function () { + await permController.addPermittedAccount(ORIGINS.a, EXTRA_ACCOUNT) + + const accounts = await permController.getAccounts(ORIGINS.a) + + assert.deepEqual( + accounts, [...ACCOUNT_ARRAYS.a, EXTRA_ACCOUNT], + 'origin should have correct accounts' + ) + + assert.deepEqual( + notifications[ORIGINS.a][0], + NOTIFICATIONS.newAccounts([...ACCOUNT_ARRAYS.a, EXTRA_ACCOUNT]), + 'origin should have correct notification' + ) + }) + }) + describe('finalizePermissionsRequest', function () { let permController diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index cc8d8e19a..318572a70 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -1227,6 +1227,20 @@ export function showAccountDetail (address) { } } +export function addPermittedAccount (origin, address) { + return async (dispatch) => { + await new Promise((resolve, reject) => { + background.addPermittedAccount(origin, address, (error) => { + if (error) { + return reject(error) + } + resolve() + }) + }) + await forceUpdateMetamaskState(dispatch) + } +} + export function showAccountsPage () { return { type: actionConstants.SHOW_ACCOUNTS_PAGE,