Skip to content

Commit

Permalink
Add 'addPermittedAccount' method to permissions controller
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Gudahtt committed Apr 16, 2020
1 parent b35ad4a commit 50c0166
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 0 deletions.
50 changes: 50 additions & 0 deletions app/scripts/controllers/permissions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<string>|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.
Expand Down
1 change: 1 addition & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
18 changes: 18 additions & 0 deletions test/unit/app/controllers/permissions/mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 isn\'t 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
Expand Down
14 changes: 14 additions & 0 deletions ui/app/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 50c0166

Please sign in to comment.