From b1d090ac4d2136739e934f9dd8caefc35335971b Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 16 Mar 2020 10:13:22 -0700 Subject: [PATCH] Add permissions controller unit tests (#7969) * add permissions controller, log, middleware, and restricted method unit tests * fix permissions-related bugs * convert permissions log to controller-like class * add permissions unit test coverage requirements * update rpc-cap Co-Authored-By: Whymarrh Whitby Co-Authored-By: Mark Stacey --- app/scripts/controllers/permissions/enums.js | 7 + app/scripts/controllers/permissions/index.js | 161 ++- .../permissions/methodMiddleware.js | 17 +- .../controllers/permissions/permissionsLog.js | 188 +-- .../permissions/restrictedMethods.js | 12 +- app/scripts/metamask-controller.js | 2 + nyc.config.js | 6 + package.json | 7 +- test/e2e/permissions.spec.js | 2 +- .../app/controllers/permissions/helpers.js | 117 ++ .../unit/app/controllers/permissions/mocks.js | 691 ++++++++++ .../permissions-controller-test.js | 1191 +++++++++++++++++ .../permissions-log-controller-test.js | 690 ++++++++++ .../permissions-middleware-test.js | 752 +++++++++++ .../permissions/restricted-methods-test.js | 35 + yarn.lock | 8 +- 16 files changed, 3726 insertions(+), 160 deletions(-) create mode 100644 nyc.config.js create mode 100644 test/unit/app/controllers/permissions/helpers.js create mode 100644 test/unit/app/controllers/permissions/mocks.js create mode 100644 test/unit/app/controllers/permissions/permissions-controller-test.js create mode 100644 test/unit/app/controllers/permissions/permissions-log-controller-test.js create mode 100644 test/unit/app/controllers/permissions/permissions-middleware-test.js create mode 100644 test/unit/app/controllers/permissions/restricted-methods-test.js diff --git a/app/scripts/controllers/permissions/enums.js b/app/scripts/controllers/permissions/enums.js index 0d30827d6..0e68232f4 100644 --- a/app/scripts/controllers/permissions/enums.js +++ b/app/scripts/controllers/permissions/enums.js @@ -19,6 +19,13 @@ export const LOG_IGNORE_METHODS = [ 'wallet_sendDomainMetadata', ] +export const LOG_METHOD_TYPES = { + restricted: 'restricted', + internal: 'internal', +} + +export const LOG_LIMIT = 100 + export const SAFE_METHODS = [ 'web3_sha3', 'net_listening', diff --git a/app/scripts/controllers/permissions/index.js b/app/scripts/controllers/permissions/index.js index e8b1b17e8..eb62bb219 100644 --- a/app/scripts/controllers/permissions/index.js +++ b/app/scripts/controllers/permissions/index.js @@ -4,8 +4,8 @@ import ObservableStore from 'obs-store' import log from 'loglevel' import { CapabilitiesController as RpcCap } from 'rpc-cap' import { ethErrors } from 'eth-json-rpc-errors' +import { cloneDeep } from 'lodash' -import getRestrictedMethods from './restrictedMethods' import createMethodMiddleware from './methodMiddleware' import PermissionsLogController from './permissionsLog' @@ -24,7 +24,8 @@ export class PermissionsController { constructor ( { - platform, notifyDomain, notifyAllDomains, getKeyringAccounts, + platform, notifyDomain, notifyAllDomains, + getKeyringAccounts, getRestrictedMethods, } = {}, restoredPermissions = {}, restoredState = {}) { @@ -40,7 +41,7 @@ export class PermissionsController { this.getKeyringAccounts = getKeyringAccounts this._platform = platform this._restrictedMethods = getRestrictedMethods(this) - this.permissionsLogController = new PermissionsLogController({ + this.permissionsLog = new PermissionsLogController({ restrictedMethods: Object.keys(this._restrictedMethods), store: this.store, }) @@ -51,6 +52,10 @@ export class PermissionsController { createMiddleware ({ origin, extensionId }) { + if (typeof origin !== 'string' || !origin.length) { + throw new Error('Must provide non-empty string origin.') + } + if (extensionId) { this.store.updateState({ [METADATA_STORE_KEY]: { @@ -62,7 +67,7 @@ export class PermissionsController { const engine = new JsonRpcEngine() - engine.push(this.permissionsLogController.createMiddleware()) + engine.push(this.permissionsLog.createMiddleware()) engine.push(createMethodMiddleware({ store: this.store, @@ -76,6 +81,7 @@ export class PermissionsController { engine.push(this.permissions.providerMiddlewareFunction.bind( this.permissions, { origin } )) + return asMiddleware(engine) } @@ -114,15 +120,18 @@ export class PermissionsController { _requestPermissions (origin, 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 ) - function _end (err) { - if (err || res.error) { - reject(err || res.error) + function _end (_err) { + const err = _err || res.error + if (err) { + reject(err) } else { resolve(res.result) } @@ -131,9 +140,11 @@ export class PermissionsController { } /** - * User approval callback. The request can fail if the request is invalid. + * User approval callback. Resolves the Promise for the permissions request + * waited upon by rpc-cap, see requestUserApproval in _initializePermissions. + * The request will be rejected if finalizePermissionsRequest fails. * - * @param {Object} approved - the approved request object + * @param {Object} approved - The request object approved by the user * @param {Array} accounts - The accounts to expose, if any */ async approvePermissionsRequest (approved, accounts) { @@ -142,16 +153,27 @@ export class PermissionsController { const approval = this.pendingApprovals.get(id) if (!approval) { - log.warn(`Permissions request with id '${id}' not found`) + log.error(`Permissions request with id '${id}' not found`) return } try { - // attempt to finalize the request and resolve it - await this.finalizePermissionsRequest(approved.permissions, accounts) - approval.resolve(approved.permissions) + if (Object.keys(approved.permissions).length === 0) { + approval.reject(ethErrors.rpc.invalidRequest({ + message: 'Must request at least one permission.', + })) + + } else { + + // attempt to finalize the request and resolve it, + // settings caveats as necessary + approved.permissions = await this.finalizePermissionsRequest( + approved.permissions, accounts + ) + approval.resolve(approved.permissions) + } } catch (err) { // if finalization fails, reject the request @@ -164,15 +186,16 @@ export class PermissionsController { } /** - * User rejection callback. + * User rejection callback. Rejects the Promise for the permissions request + * waited upon by rpc-cap, see requestUserApproval in _initializePermissions. * - * @param {string} id - the id of the rejected request + * @param {string} id - The id of the request rejected by the user */ async rejectPermissionsRequest (id) { const approval = this.pendingApprovals.get(id) if (!approval) { - log.warn(`Permissions request with id '${id}' not found`) + log.error(`Permissions request with id '${id}' not found`) return } @@ -181,6 +204,7 @@ export class PermissionsController { } /** + * @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. @@ -190,33 +214,54 @@ export class PermissionsController { */ async legacyExposeAccounts (origin, accounts) { - const permissions = { - eth_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.' + ) } - await this.finalizePermissionsRequest(permissions, accounts) + const permissions = await this.finalizePermissionsRequest( + { eth_accounts: {} }, accounts + ) - let error try { + await new Promise((resolve, reject) => { - this.permissions.grantNewPermissions(origin, permissions, {}, (err) => (err ? resolve() : reject(err))) + this.permissions.grantNewPermissions( + origin, permissions, {}, _end + ) + + function _end (err) { + if (err) { + reject(err) + } else { + resolve() + } + } }) - } catch (err) { - error = err - } - if (error) { - if (error.code === 4001) { - throw error - } else { - throw ethErrors.rpc.internal({ - message: `Failed to add 'eth_accounts' to '${origin}'.`, - data: { - originalError: error, - accounts, - }, - }) - } + 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, + }, + }) } } @@ -246,19 +291,25 @@ export class PermissionsController { } /** - * Finalizes a permissions request. - * Throws if request validation fails. + * Finalizes a permissions request. Throws if request validation fails. + * Clones the passed-in parameters to prevent inadvertent modification. + * Sets (adds or replaces) caveats for the following permissions: + * - eth_accounts: the permitted accounts caveat * * @param {Object} requestedPermissions - The requested permissions. - * @param {string[]} accounts - The accounts to expose, if any. + * @param {string[]} requestedAccounts - The accounts to expose, if any. + * @returns {Object} The finalized permissions request object. */ - async finalizePermissionsRequest (requestedPermissions, accounts) { + async finalizePermissionsRequest (requestedPermissions, requestedAccounts) { + + const finalizedPermissions = cloneDeep(requestedPermissions) + const finalizedAccounts = cloneDeep(requestedAccounts) - const { eth_accounts: ethAccounts } = requestedPermissions + const { eth_accounts: ethAccounts } = finalizedPermissions if (ethAccounts) { - await this.validatePermittedAccounts(accounts) + await this.validatePermittedAccounts(finalizedAccounts) if (!ethAccounts.caveats) { ethAccounts.caveats = [] @@ -272,11 +323,13 @@ export class PermissionsController { ethAccounts.caveats.push( { type: 'filterResponse', - value: accounts, + value: finalizedAccounts, name: CAVEAT_NAMES.exposedAccounts, }, ) } + + return finalizedPermissions } /** @@ -309,7 +362,7 @@ export class PermissionsController { payload.method === NOTIFICATION_NAMES.accountsChanged && Array.isArray(payload.result) ) { - this.permissionsLogController.updateAccountsHistory( + this.permissionsLog.updateAccountsHistory( origin, payload.result ) } @@ -325,6 +378,9 @@ export class PermissionsController { /** * Removes the given permissions for the given domain. + * Should only be called after confirming that the permissions exist, to + * avoid sending unnecessary notifications. + * * @param {Object} domains { origin: [permissions] } */ removePermissionsFor (domains) { @@ -351,6 +407,10 @@ export class PermissionsController { /** * When a new account is selected in the UI for 'origin', emit accountsChanged * to 'origin' if the selected account is permitted. + * + * Note: This will emit "false positive" accountsChanged events, but they are + * handled by the inpage provider. + * * @param {string} origin - The origin. * @param {string} account - The newly selected account's address. */ @@ -358,10 +418,17 @@ export class PermissionsController { const permittedAccounts = await this.getAccounts(origin) + if ( + typeof origin !== 'string' || !origin.length || + typeof account !== 'string' || !account.length + ) { + throw new Error('Should provide non-empty origin and account strings.') + } + // do nothing if the account is not permitted for the origin, or // if it's already first in the array of permitted accounts if ( - !account || !permittedAccounts.includes(account) || + !permittedAccounts.includes(account) || permittedAccounts[0] === account ) { return @@ -373,7 +440,7 @@ export class PermissionsController { // update permitted accounts to ensure that accounts are returned // in the same order every time - this.updatePermittedAccounts(origin, newPermittedAccounts) + await this.updatePermittedAccounts(origin, newPermittedAccounts) } /** @@ -454,7 +521,7 @@ export class PermissionsController { if (this.pendingApprovalOrigins.has(origin)) { throw ethErrors.rpc.resourceUnavailable( - 'Permission request already pending; please wait.' + 'Permissions request already pending; please wait.' ) } diff --git a/app/scripts/controllers/permissions/methodMiddleware.js b/app/scripts/controllers/permissions/methodMiddleware.js index 61464cc71..0656981ff 100644 --- a/app/scripts/controllers/permissions/methodMiddleware.js +++ b/app/scripts/controllers/permissions/methodMiddleware.js @@ -9,15 +9,11 @@ export default function createMethodMiddleware ({ }) { return createAsyncMiddleware(async (req, res, next) => { - if (typeof req.method !== 'string') { - res.error = ethErrors.rpc.invalidRequest({ data: req }) - return - } - switch (req.method) { - // intercepting eth_accounts requests for backwards compatibility, - // i.e. return an empty array instead of an error + // Intercepting eth_accounts requests for backwards compatibility: + // The getAccounts call below wraps the rpc-cap middleware, and returns + // an empty array in case of errors (such as 4100:unauthorized) case 'eth_accounts': res.result = await getAccounts() @@ -42,10 +38,12 @@ export default function createMethodMiddleware ({ // get the accounts again accounts = await getAccounts() + /* istanbul ignore else: too hard to induce, see below comment */ if (accounts.length > 0) { res.result = accounts } else { - // this should never happen + // this should never happen, because it should be caught in the + // above catch clause res.error = ethErrors.rpc.internal( 'Accounts unexpectedly unavailable. Please report this bug.' ) @@ -53,7 +51,8 @@ export default function createMethodMiddleware ({ return - // custom method for getting metadata from the requesting domain + // custom method for getting metadata from the requesting domain, + // sent automatically by the inpage provider case 'wallet_sendDomainMetadata': const storeState = store.getState()[storeKey] diff --git a/app/scripts/controllers/permissions/permissionsLog.js b/app/scripts/controllers/permissions/permissionsLog.js index e574b6cc3..71db901dc 100644 --- a/app/scripts/controllers/permissions/permissionsLog.js +++ b/app/scripts/controllers/permissions/permissionsLog.js @@ -1,16 +1,14 @@ - import { cloneDeep } from 'lodash' -import { isValidAddress } from 'ethereumjs-util' import { CAVEAT_NAMES, HISTORY_STORE_KEY, - LOG_STORE_KEY, LOG_IGNORE_METHODS, + LOG_LIMIT, + LOG_METHOD_TYPES, + LOG_STORE_KEY, WALLET_PREFIX, } from './enums' -const LOG_LIMIT = 100 - /** * Controller with middleware for logging requests and responses to restricted * and permissions-related methods. @@ -25,7 +23,7 @@ export default class PermissionsLogController { /** * Get the activity log. * - * @returns {Array} - The activity log. + * @returns {Array} The activity log. */ getActivityLog () { return this.store.getState()[LOG_STORE_KEY] || [] @@ -43,7 +41,7 @@ export default class PermissionsLogController { /** * Get the permissions history log. * - * @returns {Object} - The permissions history log. + * @returns {Object} The permissions history log. */ getHistory () { return this.store.getState()[HISTORY_STORE_KEY] || {} @@ -81,15 +79,20 @@ export default class PermissionsLogController { } /** - * Create a permissions log middleware. + * Create a permissions log middleware. Records permissions activity and history: + * + * Activity: requests and responses for restricted and most wallet_ methods. + * + * History: for each origin, the last time a permission was granted, including + * which accounts were exposed, if any. * - * @returns {JsonRpcEngineMiddleware} - The permissions log middleware. + * @returns {JsonRpcEngineMiddleware} The permissions log middleware. */ createMiddleware () { return (req, res, next, _end) => { - let requestedMethods - const { origin, method, id: requestId } = req + let activityEntry, requestedMethods + const { origin, method } = req const isInternal = method.startsWith(WALLET_PREFIX) // we only log certain methods @@ -98,17 +101,18 @@ export default class PermissionsLogController { (isInternal || this.restrictedMethods.includes(method)) ) { - this.logActivityRequest(req, isInternal) + activityEntry = this.logRequest(req, isInternal) if (method === `${WALLET_PREFIX}requestPermissions`) { - // get the corresponding methods from the requested permissions + // get the corresponding methods from the requested permissions so + // that we can record permissions history requestedMethods = this.getRequestedMethods(req) } } else if (method === 'eth_requestAccounts') { // eth_requestAccounts is a special case; we need to extract the accounts // from it - this.logActivityRequest(req, isInternal) + activityEntry = this.logRequest(req, isInternal) requestedMethods = [ 'eth_accounts' ] } else { // no-op @@ -119,9 +123,9 @@ export default class PermissionsLogController { next((cb) => { const time = Date.now() - this.logActivityResponse(requestId, res, time) + this.logResponse(activityEntry, res, time) - if (!res.error && requestedMethods) { + if (requestedMethods && !res.error && res.result) { // any permissions or accounts changes will be recorded on the response, // so we only log permissions history here this.logPermissionsHistory( @@ -140,46 +144,41 @@ export default class PermissionsLogController { * @param {Object} request - The request object. * @param {boolean} isInternal - Whether the request is internal. */ - logActivityRequest (request, isInternal) { + logRequest (request, isInternal) { const activityEntry = { id: request.id, method: request.method, - methodType: isInternal ? 'internal' : 'restricted', + methodType: ( + isInternal ? LOG_METHOD_TYPES.internal : LOG_METHOD_TYPES.restricted + ), origin: request.origin, - request: cloneObj(request), + request: cloneDeep(request), requestTime: Date.now(), response: null, responseTime: null, success: null, } this.commitNewActivity(activityEntry) + return activityEntry } /** - * Adds response data to an existing activity log entry and re-commits it. + * Adds response data to an existing activity log entry. + * Entry assumed already committed (i.e., in the log). * - * @param {string} id - The original request id. + * @param {Object} entry - The entry to add a response to. * @param {Object} response - The response object. * @param {number} time - Output from Date.now() */ - logActivityResponse (id, response, time) { + logResponse (entry, response, time) { - if (!id || !response) { + if (!entry || !response) { return } - const logs = this.getActivityLog() - const index = getLastIndexOfObjectArray(logs, 'id', id) - if (index === -1) { - return - } - - const entry = logs[index] - entry.response = cloneObj(response) + entry.response = cloneDeep(response) entry.responseTime = time entry.success = !response.error - - this.updateActivityLog(logs) } /** @@ -203,6 +202,33 @@ 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} 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. * @@ -212,7 +238,10 @@ export default class PermissionsLogController { * @param {string} time - The time of the request, i.e. Date.now(). * @param {boolean} isEthRequestAccounts - Whether the permissions request was 'eth_requestAccounts'. */ - logPermissionsHistory (requestedMethods, origin, result, time, isEthRequestAccounts) { + logPermissionsHistory ( + requestedMethods, origin, result, + time, isEthRequestAccounts + ) { let accounts, newEntries @@ -233,35 +262,35 @@ export default class PermissionsLogController { // Special handling for eth_accounts, in order to record the time the // accounts were last seen or approved by the origin. newEntries = result - ? result - .map((perm) => { + .map((perm) => { - if (perm.parentCapability === 'eth_accounts') { - accounts = this.getAccountsFromPermission(perm) - } + if (perm.parentCapability === 'eth_accounts') { + accounts = this.getAccountsFromPermission(perm) + } - return perm.parentCapability - }) - .reduce((acc, method) => { + return perm.parentCapability + }) + .reduce((acc, method) => { - if (requestedMethods.includes(method)) { + // all approved permissions will be included in the response, + // not just the newly requested ones + if (requestedMethods.includes(method)) { - if (method === 'eth_accounts') { + if (method === 'eth_accounts') { - const accountToTimeMap = getAccountToTimeMap(accounts, time) + const accountToTimeMap = getAccountToTimeMap(accounts, time) - acc[method] = { - lastApproved: time, - accounts: accountToTimeMap, - } - } else { - acc[method] = { lastApproved: time } + acc[method] = { + lastApproved: time, + accounts: accountToTimeMap, } + } else { + acc[method] = { lastApproved: time } } + } - return acc - }, {}) - : {} // no result (e.g. in case of error), no log + return acc + }, {}) } if (Object.keys(newEntries).length > 0) { @@ -292,6 +321,7 @@ export default class PermissionsLogController { history[origin] && history[origin]['eth_accounts'] ) const newEthAccountsEntry = newEntries['eth_accounts'] + if (existingEthAccountsEntry && newEthAccountsEntry) { // we may intend to update just the accounts, not the permission @@ -320,7 +350,7 @@ export default class PermissionsLogController { * Get all requested methods from a permissions request. * * @param {Object} request - The request object. - * @returns {Array} - The names of the requested permissions. + * @returns {Array} The names of the requested permissions. */ getRequestedMethods (request) { if ( @@ -339,7 +369,7 @@ export default class PermissionsLogController { * Returns an empty array if the permission is not eth_accounts. * * @param {Object} perm - The permissions object. - * @returns {Array} - The permitted accounts. + * @returns {Array} The permitted accounts. */ getAccountsFromPermission (perm) { @@ -347,7 +377,7 @@ export default class PermissionsLogController { return [] } - const accounts = {} + const accounts = new Set() for (const caveat of perm.caveats) { if ( @@ -356,51 +386,25 @@ export default class PermissionsLogController { ) { for (const value of caveat.value) { - if (isValidAddress(value)) { - accounts[value] = true - } + accounts.add(value) } } } - return Object.keys(accounts) + return [ ...accounts ] } } // helper functions -// the call to clone is set to disallow circular references -// we attempt cloning at a depth of 3 and 2, then return a -// shallow copy of the object -function cloneObj (obj) { - - for (let i = 3; i > 1; i--) { - try { - return cloneDeep(obj, false, i) - } catch (_) {} - } - return { ...obj } -} - +/** + * Get a map from account addresses to the given time. + * + * @param {Array} accounts - An array of addresses. + * @param {number} time - A time, e.g. Date.now(). + * @returns {Object} A string:number map of addresses to time. + */ function getAccountToTimeMap (accounts, time) { return accounts.reduce( (acc, account) => ({ ...acc, [account]: time }), {} ) } - -function getLastIndexOfObjectArray (array, key, value) { - - if (Array.isArray(array) && array.length > 0) { - - for (let i = array.length - 1; i >= 0; i--) { - - if (!array[i] || typeof array[i] !== 'object') { - throw new Error(`Encountered non-Object element at index ${i}`) - } - - if (array[i][key] === value) { - return i - } - } - } - return -1 -} diff --git a/app/scripts/controllers/permissions/restrictedMethods.js b/app/scripts/controllers/permissions/restrictedMethods.js index b77336cfe..73ac7d94e 100644 --- a/app/scripts/controllers/permissions/restrictedMethods.js +++ b/app/scripts/controllers/permissions/restrictedMethods.js @@ -2,17 +2,19 @@ export default function getRestrictedMethods (permissionsController) { return { 'eth_accounts': { - description: 'View the address of the selected account', + description: `View the addresses of the user's chosen accounts.`, method: (_, res, __, end) => { permissionsController.getKeyringAccounts() .then((accounts) => { res.result = accounts end() }) - .catch((err) => { - res.error = err - end(err) - }) + .catch( + (err) => { + res.error = err + end(err) + } + ) }, }, } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 8c28fa858..bd1d39b83 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -45,6 +45,7 @@ import TokenRatesController from './controllers/token-rates' import DetectTokensController from './controllers/detect-tokens' import ABTestController from './controllers/ab-test' import { PermissionsController } from './controllers/permissions' +import getRestrictedMethods from './controllers/permissions/restrictedMethods' import nodeify from './lib/nodeify' import accountImporter from './account-import-strategies' import getBuyEthUrl from './lib/buy-eth-url' @@ -211,6 +212,7 @@ export default class MetamaskController extends EventEmitter { platform: opts.platform, notifyDomain: this.notifyConnections.bind(this), notifyAllDomains: this.notifyAllConnections.bind(this), + getRestrictedMethods, }, initState.PermissionsController, initState.PermissionsMetadata) this.detectTokensController = new DetectTokensController({ diff --git a/nyc.config.js b/nyc.config.js new file mode 100644 index 000000000..8902e0571 --- /dev/null +++ b/nyc.config.js @@ -0,0 +1,6 @@ +module.exports = { + branches: 95, + lines: 95, + functions: 95, + statements: 95, +} diff --git a/package.json b/package.json index 90600a71e..b27298d30 100644 --- a/package.json +++ b/package.json @@ -19,13 +19,16 @@ "sendwithprivatedapp": "node development/static-server.js test/e2e/send-eth-with-private-key-test --port 8080", "test:unit": "mocha --exit --require test/env.js --require test/setup.js --recursive \"test/unit/**/*.js\" \"ui/app/**/*.test.js\"", "test:unit:global": "mocha --exit --require test/env.js --require test/setup.js --recursive mocha test/unit-global/*", + "test:unit:lax": "mocha --exit --require test/env.js --require test/setup.js --recursive \"test/unit/{,**/!(permissions)}/*.js\" \"ui/app/**/*.test.js\"", + "test:unit:strict": "mocha --exit --require test/env.js --require test/setup.js --recursive \"test/unit/**/permissions/*.js\"", "test:integration": "yarn test:integration:build && yarn test:flat", "test:integration:build": "yarn build styles", "test:e2e:chrome": "SELENIUM_BROWSER=chrome test/e2e/run-all.sh", "test:web3:chrome": "SELENIUM_BROWSER=chrome test/e2e/run-web3.sh", "test:web3:firefox": "SELENIUM_BROWSER=firefox test/e2e/run-web3.sh", "test:e2e:firefox": "SELENIUM_BROWSER=firefox test/e2e/run-all.sh", - "test:coverage": "nyc --reporter=text --reporter=html yarn test:unit", + "test:coverage": "nyc --silent --check-coverage yarn test:unit:strict && nyc --silent --no-clean yarn test:unit:lax && nyc report --reporter=text --reporter=html", + "test:coverage:strict": "nyc --check-coverage yarn test:unit:strict", "test:coveralls-upload": "if [ \"$COVERALLS_REPO_TOKEN\" ]; then nyc report --reporter=text-lcov | coveralls; fi", "test:flat": "yarn test:flat:build && karma start test/flat.conf.js", "test:flat:build": "yarn test:flat:build:ui && yarn test:flat:build:tests && yarn test:flat:build:locales", @@ -156,7 +159,7 @@ "redux": "^4.0.5", "redux-thunk": "^2.3.0", "reselect": "^3.0.1", - "rpc-cap": "^1.0.5", + "rpc-cap": "^2.0.0", "safe-event-emitter": "^1.0.1", "safe-json-stringify": "^1.2.0", "single-call-balance-checker-abi": "^1.0.0", diff --git a/test/e2e/permissions.spec.js b/test/e2e/permissions.spec.js index d287cff6d..8a18b5bf4 100644 --- a/test/e2e/permissions.spec.js +++ b/test/e2e/permissions.spec.js @@ -146,7 +146,7 @@ describe('MetaMask', function () { await domains[0].click() const permissionDescription = await driver.findElement(By.css('.connected-sites-list__permission-description')) - assert.equal(await permissionDescription.getText(), 'View the address of the selected account') + assert.equal(await permissionDescription.getText(), `View the addresses of the user's chosen accounts.`) }) it('can get accounts within the dapp', async function () { diff --git a/test/unit/app/controllers/permissions/helpers.js b/test/unit/app/controllers/permissions/helpers.js new file mode 100644 index 000000000..d0b81fef9 --- /dev/null +++ b/test/unit/app/controllers/permissions/helpers.js @@ -0,0 +1,117 @@ +import { strict as assert } from 'assert' + +import { noop } from './mocks' + +/** + * Grants the given permissions to the given origin, using the given permissions + * controller. + * + * Just a wrapper for an rpc-cap middleware function. + * + * @param {PermissionsController} permController - The permissions controller. + * @param {string} origin - The origin to grant permissions to. + * @param {Object} permissions - The permissions to grant. + */ +export function grantPermissions (permController, origin, permissions) { + permController.permissions.grantNewPermissions( + origin, permissions, {}, noop + ) +} + +/** + * Sets the underlying rpc-cap requestUserApproval function, and returns + * a promise that's resolved once it has been set. + * + * This function must be called on the given permissions controller every + * time you want such a Promise. As of writing, it's only called once per test. + * + * @param {PermissionsController} - A permissions controller. + * @returns {Promise} A Promise that resolves once a pending approval + * has been set. + */ +export function getUserApprovalPromise (permController) { + return new Promise((resolveForCaller) => { + permController.permissions.requestUserApproval = async (req) => { + const { origin, metadata: { id } } = req + + return new Promise((resolve, reject) => { + permController.pendingApprovals.set(id, { origin, resolve, reject }) + resolveForCaller() + }) + } + }) +} + +/** + * Validates an activity log entry with respect to a request, response, and + * relevant metadata. + * + * @param {Object} entry - The activity log entry to validate. + * @param {Object} req - The request that generated the entry. + * @param {Object} [res] - The response for the request, if any. + * @param {'restricted'|'internal'} methodType - The method log controller method type of the request. + * @param {boolean} success - Whether the request succeeded or not. + */ +export function validateActivityEntry ( + entry, req, res, methodType, success +) { + assert.doesNotThrow( + () => { + _validateActivityEntry( + entry, req, res, methodType, success + ) + }, + 'should have expected activity entry' + ) +} + +function _validateActivityEntry ( + entry, req, res, methodType, success +) { + + assert.ok(entry, 'entry should exist') + + assert.equal(entry.id, req.id) + assert.equal(entry.method, req.method) + assert.equal(entry.origin, req.origin) + assert.equal(entry.methodType, methodType) + assert.deepEqual( + entry.request, req, + 'entry.request should equal the request' + ) + + if (res) { + + assert.ok( + ( + Number.isInteger(entry.requestTime) && + Number.isInteger(entry.responseTime) + ), + 'request and response times should be numbers' + ) + assert.ok( + (entry.requestTime <= entry.responseTime), + 'request time should be less than response time' + ) + + assert.equal(entry.success, success) + assert.deepEqual( + entry.response, res, + 'entry.response should equal the response' + ) + } else { + + assert.ok( + Number.isInteger(entry.requestTime) && entry.requestTime > 0, + 'entry should have non-zero request time' + ) + assert.ok( + ( + entry.success === null && + entry.responseTime === null && + entry.response === null + ), + 'entry response values should be null' + ) + } +} diff --git a/test/unit/app/controllers/permissions/mocks.js b/test/unit/app/controllers/permissions/mocks.js new file mode 100644 index 000000000..448de3fc9 --- /dev/null +++ b/test/unit/app/controllers/permissions/mocks.js @@ -0,0 +1,691 @@ +import { ethErrors, ERROR_CODES } from 'eth-json-rpc-errors' +import deepFreeze from 'deep-freeze-strict' + +import _getRestrictedMethods + from '../../../../../app/scripts/controllers/permissions/restrictedMethods' + +import { + CAVEAT_NAMES, + NOTIFICATION_NAMES, +} from '../../../../../app/scripts/controllers/permissions/enums' + +/** + * README + * This file contains three primary kinds of mocks: + * - Mocks for initializing a permissions controller and getting a permissions + * middleware + * - Functions for getting various mock objects consumed or produced by + * permissions controller methods + * - Immutable mock values like Ethereum accounts and expected states + */ + +export const noop = () => {} + +/** + * Mock Permissions Controller and Middleware + */ + +const platform = { + openExtensionInBrowser: noop, +} + +const keyringAccounts = deepFreeze([ + '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc', + '0xc42edfcc21ed14dda456aa0756c153f7985d8813', +]) + +const getKeyringAccounts = async () => [ ...keyringAccounts ] + +// perm controller initialization helper +const getRestrictedMethods = (permController) => { + return { + + // the actual, production restricted methods + ..._getRestrictedMethods(permController), + + // our own dummy method for testing + 'test_method': { + description: `This method is only for testing.`, + method: (req, res, __, end) => { + if (req.params[0]) { + res.result = 1 + } else { + res.result = 0 + } + end() + }, + }, + } +} + +/** + * Gets default mock constructor options for a permissions controller. + * + * @returns {Object} A PermissionsController constructor options object. + */ +export function getPermControllerOpts () { + return { + platform, + getKeyringAccounts, + notifyDomain: noop, + notifyAllDomains: noop, + getRestrictedMethods, + } +} + +/** + * Gets a Promise-wrapped permissions controller middleware function. + * + * @param {PermissionsController} permController - The permissions controller to get a + * middleware for. + * @param {string} origin - The origin for the middleware. + * @param {string} extensionId - The extension id for the middleware. + * @returns {Function} A Promise-wrapped middleware function with convenient default args. + */ +export function getPermissionsMiddleware (permController, origin, extensionId) { + const middleware = permController.createMiddleware({ origin, extensionId }) + return (req, res = {}, next = noop, end) => { + return new Promise((resolve, reject) => { + + end = end || _end + + middleware(req, res, next, end) + + // emulates json-rpc-engine error handling + function _end (err) { + if (err || res.error) { + reject(err || res.error) + } else { + resolve(res) + } + } + }) + } +} + +/** + * @param {Object} notifications - An object that will store notifications produced + * by the permissions controller. + * @returns {Function} A function passed to the permissions controller at initialization, + * for recording notifications. + */ +export const getNotifyDomain = (notifications = {}) => (origin, notification) => { + notifications[origin].push(notification) +} + +/** + * @param {Object} notifications - An object that will store notifications produced + * by the permissions controller. + * @returns {Function} A function passed to the permissions controller at initialization, + * for recording notifications. + */ +export const getNotifyAllDomains = (notifications = {}) => (notification) => { + Object.keys(notifications).forEach((origin) => { + notifications[origin].push(notification) + }) +} + +/** + * Constants and Mock Objects + * - e.g. permissions, caveats, and permission requests + */ + +const ORIGINS = { + a: 'foo.xyz', + b: 'bar.abc', + c: 'baz.def', +} + +const PERM_NAMES = { + eth_accounts: 'eth_accounts', + test_method: 'test_method', + does_not_exist: 'does_not_exist', +} + +const ACCOUNT_ARRAYS = { + a: [ ...keyringAccounts ], + b: [keyringAccounts[0]], + c: [keyringAccounts[1]], +} + +/** + * Helpers for getting mock caveats. + */ +const CAVEATS = { + + /** + * Gets a correctly formatted eth_accounts exposedAccounts caveat. + * + * @param {Array} accounts - The accounts for the caveat + * @returns {Object} An eth_accounts exposedAccounts caveats + */ + eth_accounts: (accounts) => { + return { + type: 'filterResponse', + value: accounts, + name: CAVEAT_NAMES.exposedAccounts, + } + }, +} + +/** + * Each function here corresponds to what would be a type or interface consumed + * by permissions controller functions if we used TypeScript. + */ +const PERMS = { + + /** + * The argument to approvePermissionsRequest + * @param {string} id - The rpc-cap permissions request id. + * @param {Object} permissions - The approved permissions, request-formatted. + */ + approvedRequest: (id, permissions = {}) => { + return { + permissions: { ...permissions }, + metadata: { id }, + } + }, + + /** + * Requested permissions objects, as passed to wallet_requestPermissions. + */ + requests: { + + /** + * @returns {Object} A permissions request object with eth_accounts + */ + eth_accounts: () => { + return { eth_accounts: {} } + }, + + /** + * @returns {Object} A permissions request object with test_method + */ + test_method: () => { + return { test_method: {} } + }, + + /** + * @returns {Object} A permissions request object with does_not_exist + */ + does_not_exist: () => { + return { does_not_exist: {} } + }, + }, + + /** + * Finalized permission requests, as returned by finalizePermissionsRequest + */ + finalizedRequests: { + + /** + * @param {Array} accounts - The accounts for the eth_accounts permission caveat + * @returns {Object} A finalized permissions request object with eth_accounts and its caveat + */ + eth_accounts: (accounts) => { + return { + eth_accounts: { + caveats: [CAVEATS.eth_accounts(accounts)], + } } + }, + + /** + * @returns {Object} A finalized permissions request object with test_method + */ + test_method: () => { + return { + test_method: {}, + } + }, + }, + + /** + * Partial members of res.result for successful: + * - wallet_requestPermissions + * - wallet_getPermissions + */ + granted: { + + /** + * @param {Array} accounts - The accounts for the eth_accounts permission caveat + * @returns {Object} A granted permissions object with eth_accounts and its caveat + */ + eth_accounts: (accounts) => { + return { + parentCapability: PERM_NAMES.eth_accounts, + caveats: [CAVEATS.eth_accounts(accounts)], + } + }, + + /** + * @returns {Object} A granted permissions object with test_method + */ + test_method: () => { + return { + parentCapability: PERM_NAMES.test_method, + } + }, + }, +} + +/** + * Objects with function values for getting correctly formatted permissions, + * caveats, errors, permissions requests etc. + */ +export const getters = deepFreeze({ + + CAVEATS, + + PERMS, + + /** + * Getters for errors by the method or workflow that throws them. + */ + ERRORS: { + + validatePermittedAccounts: { + + invalidParam: () => { + return { + name: 'Error', + message: 'Must provide non-empty array of account(s).', + } + }, + + nonKeyringAccount: (account) => { + return { + name: 'Error', + message: `Unknown account: ${account}`, + } + }, + }, + + finalizePermissionsRequest: { + grantEthAcountsFailure: (origin) => { + return { + // name: 'EthereumRpcError', + message: `Failed to add 'eth_accounts' to '${origin}'.`, + code: ERROR_CODES.rpc.internal, + } + }, + }, + + updatePermittedAccounts: { + invalidOrigin: () => { + return { + message: 'No such permission exists for the given domain.', + } + }, + }, + + legacyExposeAccounts: { + badOrigin: () => { + return { + message: 'Must provide non-empty string origin.', + } + }, + forbiddenUsage: () => { + return { + name: 'Error', + message: 'May not call legacyExposeAccounts on origin with exposed accounts.', + } + }, + }, + + handleNewAccountSelected: { + invalidParams: () => { + return { + name: 'Error', + message: 'Should provide non-empty origin and account strings.', + } + }, + }, + + approvePermissionsRequest: { + noPermsRequested: () => { + return { + message: 'Must request at least one permission.', + } + }, + }, + + rejectPermissionsRequest: { + rejection: () => { + return { + message: ethErrors.provider.userRejectedRequest().message, + } + }, + methodNotFound: (methodName) => { + return { + message: `The method '${methodName}' does not exist / is not available.`, + } + }, + }, + + createMiddleware: { + badOrigin: () => { + return { + message: 'Must provide non-empty string origin.', + } + }, + }, + + rpcCap: { + unauthorized: () => { + return { + code: 4100, + } + }, + }, + + logAccountExposure: { + invalidParams: () => { + return { + message: 'Must provide non-empty string origin and array of accounts.', + } + }, + }, + + pendingApprovals: { + duplicateOriginOrId: (id, origin) => { + return { + message: `Pending approval with id ${id} or origin ${origin} already exists.`, + } + }, + requestAlreadyPending: () => { + return { + message: 'Permissions request already pending; please wait.', + } + }, + }, + }, + + /** + * Getters for notifications produced by the permissions controller. + */ + NOTIFICATIONS: { + + /** + * Gets a removed accounts notification. + * + * @returns {Object} An accountsChanged notification with an empty array as its result + */ + removedAccounts: () => { + return { + method: NOTIFICATION_NAMES.accountsChanged, + result: [], + } + }, + + /** + * Gets a new accounts notification. + * + * @param {Array} accounts - The accounts added to the notification. + * @returns {Object} An accountsChanged notification with the given accounts as its result + */ + newAccounts: (accounts) => { + return { + method: NOTIFICATION_NAMES.accountsChanged, + result: accounts, + } + }, + + /** + * Gets a test notification that doesn't occur in practice. + * + * @returns {Object} A notification with the 'test_notification' method name + */ + test: () => { + return { + method: 'test_notification', + result: true, + } + }, + }, + + /** + * Getters for mock RPC request objects. + */ + RPC_REQUESTS: { + + /** + * Gets an arbitrary RPC request object. + * + * @param {string} origin - The origin of the request + * @param {string} method - The request method + * @param {Array} params - The request parameters + * @param {string} [id] - The request id + * @returns {Object} An RPC request object + */ + custom: (origin, method, params = [], id) => { + const req = { + origin, + method, + params, + } + if (id !== undefined) { + req.id = id + } + return req + }, + + /** + * Gets an eth_accounts RPC request object. + * + * @param {string} origin - The origin of the request + * @returns {Object} An RPC request object + */ + eth_accounts: (origin) => { + return { + origin, + method: 'eth_accounts', + params: [], + } + }, + + /** + * Gets a test_method RPC request object. + * + * @param {string} origin - The origin of the request + * @param {boolean} param - The request param + * @returns {Object} An RPC request object + */ + test_method: (origin, param = false) => { + return { + origin, + method: 'test_method', + params: [param], + } + }, + + /** + * Gets an eth_requestAccounts RPC request object. + * + * @param {string} origin - The origin of the request + * @returns {Object} An RPC request object + */ + eth_requestAccounts: (origin) => { + return { + origin, + method: 'eth_requestAccounts', + params: [], + } + }, + + /** + * Gets a wallet_requestPermissions RPC request object, + * for a single permission. + * + * @param {string} origin - The origin of the request + * @param {string} permissionName - The name of the permission to request + * @returns {Object} An RPC request object + */ + requestPermission: (origin, permissionName) => { + return { + origin, + method: 'wallet_requestPermissions', + params: [ PERMS.requests[permissionName]() ], + } + }, + + /** + * Gets a wallet_requestPermissions RPC request object, + * for multiple permissions. + * + * @param {string} origin - The origin of the request + * @param {Object} permissions - A permission request object + * @returns {Object} An RPC request object + */ + requestPermissions: (origin, permissions = {}) => { + return { + origin, + method: 'wallet_requestPermissions', + params: [ permissions ], + } + }, + + /** + * Gets a wallet_sendDomainMetadata RPC request object. + * + * @param {string} origin - The origin of the request + * @param {Object} name - The domainMetadata name + * @param {Array} [args] - Any other data for the request's domainMetadata + * @returns {Object} An RPC request object + */ + wallet_sendDomainMetadata: (origin, name, ...args) => { + return { + origin, + method: 'wallet_sendDomainMetadata', + domainMetadata: { + ...args, + name, + }, + } + }, + }, +}) + +/** + * Objects with immutable mock values. + */ +export const constants = deepFreeze({ + + DUMMY_ACCOUNT: '0xabc', + + REQUEST_IDS: { + a: '1', + b: '2', + c: '3', + }, + + ORIGINS: { ...ORIGINS }, + + ACCOUNT_ARRAYS: { ...ACCOUNT_ARRAYS }, + + PERM_NAMES: { ...PERM_NAMES }, + + RESTRICTED_METHODS: [ + 'eth_accounts', + 'test_method', + ], + + /** + * Mock permissions history objects. + */ + EXPECTED_HISTORIES: { + + case1: [ + { + [ORIGINS.a]: { + [PERM_NAMES.eth_accounts]: { + lastApproved: 1, + accounts: { + [ACCOUNT_ARRAYS.a[0]]: 1, + [ACCOUNT_ARRAYS.a[1]]: 1, + }, + }, + }, + }, + { + [ORIGINS.a]: { + [PERM_NAMES.eth_accounts]: { + lastApproved: 2, + accounts: { + [ACCOUNT_ARRAYS.a[0]]: 2, + [ACCOUNT_ARRAYS.a[1]]: 1, + }, + }, + }, + }, + ], + + case2: [ + { + [ORIGINS.a]: { + [PERM_NAMES.eth_accounts]: { + lastApproved: 1, + accounts: {}, + }, + }, + }, + ], + + case3: [ + { + [ORIGINS.a]: { + [PERM_NAMES.test_method]: { lastApproved: 1 }, + }, + [ORIGINS.b]: { + [PERM_NAMES.eth_accounts]: { + lastApproved: 1, + accounts: { + [ACCOUNT_ARRAYS.b[0]]: 1, + }, + }, + }, + [ORIGINS.c]: { + [PERM_NAMES.test_method]: { lastApproved: 1 }, + [PERM_NAMES.eth_accounts]: { + lastApproved: 1, + accounts: { + [ACCOUNT_ARRAYS.c[0]]: 1, + }, + }, + }, + }, + { + [ORIGINS.a]: { + [PERM_NAMES.test_method]: { lastApproved: 2 }, + }, + [ORIGINS.b]: { + [PERM_NAMES.eth_accounts]: { + lastApproved: 1, + accounts: { + [ACCOUNT_ARRAYS.b[0]]: 1, + }, + }, + }, + [ORIGINS.c]: { + [PERM_NAMES.test_method]: { lastApproved: 1 }, + [PERM_NAMES.eth_accounts]: { + lastApproved: 2, + accounts: { + [ACCOUNT_ARRAYS.c[0]]: 1, + [ACCOUNT_ARRAYS.b[0]]: 2, + }, + }, + }, + }, + ], + + case4: [ + { + [ORIGINS.a]: { + [PERM_NAMES.test_method]: { + lastApproved: 1, + }, + }, + }, + ], + }, +}) diff --git a/test/unit/app/controllers/permissions/permissions-controller-test.js b/test/unit/app/controllers/permissions/permissions-controller-test.js new file mode 100644 index 000000000..b2327b5fb --- /dev/null +++ b/test/unit/app/controllers/permissions/permissions-controller-test.js @@ -0,0 +1,1191 @@ +import { strict as assert } from 'assert' +import { find } from 'lodash' +import nanoid from 'nanoid' +import sinon from 'sinon' + +import { + METADATA_STORE_KEY, + WALLET_PREFIX, +} from '../../../../../app/scripts/controllers/permissions/enums' + +import { + PermissionsController, + addInternalMethodPrefix, +} from '../../../../../app/scripts/controllers/permissions' + +import { + grantPermissions, +} from './helpers' + +import { + noop, + constants, + getters, + getNotifyDomain, + getNotifyAllDomains, + getPermControllerOpts, +} from './mocks' + +const { + ERRORS, + NOTIFICATIONS, + PERMS, +} = getters + +const { + ACCOUNT_ARRAYS, + DUMMY_ACCOUNT, + ORIGINS, + PERM_NAMES, + REQUEST_IDS, +} = constants + +const initNotifications = () => { + return Object.values(ORIGINS).reduce((acc, domain) => { + acc[domain] = [] + return acc + }, {}) +} + +const initPermController = (notifications = initNotifications()) => { + return new PermissionsController({ + ...getPermControllerOpts(), + notifyDomain: getNotifyDomain(notifications), + notifyAllDomains: getNotifyAllDomains(notifications), + }) +} + +const getMockRequestUserApprovalFunction = (permController) => (id, origin) => { + return new Promise((resolve, reject) => { + permController.pendingApprovals.set(id, { origin, resolve, reject }) + }) +} + +describe('permissions controller', function () { + + describe('getAccounts', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + grantPermissions( + permController, ORIGINS.a, + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a) + ) + grantPermissions( + permController, ORIGINS.b, + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.b) + ) + }) + + it('gets permitted accounts for permitted origins', async function () { + + const aAccounts = await permController.getAccounts(ORIGINS.a) + const bAccounts = await permController.getAccounts(ORIGINS.b) + + assert.deepEqual( + aAccounts, ACCOUNT_ARRAYS.a, + 'first origin should have correct accounts' + ) + assert.deepEqual( + bAccounts, ACCOUNT_ARRAYS.b, + 'second origin should have correct accounts' + ) + }) + + it('does not get accounts for unpermitted origins', async function () { + const cAccounts = await permController.getAccounts(ORIGINS.c) + assert.deepEqual(cAccounts, [], 'origin should have no accounts') + }) + + it('does not handle "MetaMask" origin as special case', async function () { + const metamaskAccounts = await permController.getAccounts('MetaMask') + assert.deepEqual(metamaskAccounts, [], 'origin should have no accounts') + }) + }) + + describe('clearPermissions', function () { + + it('notifies all appropriate domains and removes permissions', async function () { + + const notifications = initNotifications() + const 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) + ) + grantPermissions( + permController, ORIGINS.c, + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.c) + ) + + let aAccounts = await permController.getAccounts(ORIGINS.a) + let bAccounts = await permController.getAccounts(ORIGINS.b) + let cAccounts = await permController.getAccounts(ORIGINS.c) + + assert.deepEqual( + aAccounts, ACCOUNT_ARRAYS.a, + 'first origin should have correct accounts' + ) + assert.deepEqual( + bAccounts, ACCOUNT_ARRAYS.b, + 'second origin should have correct accounts' + ) + assert.deepEqual( + cAccounts, ACCOUNT_ARRAYS.c, + 'third origin should have correct accounts' + ) + + permController.clearPermissions() + + Object.keys(notifications).forEach((origin) => { + assert.deepEqual( + notifications[origin], + [ NOTIFICATIONS.removedAccounts() ], + 'origin should have single wallet_accountsChanged:[] notification', + ) + }) + + aAccounts = await permController.getAccounts(ORIGINS.a) + bAccounts = await permController.getAccounts(ORIGINS.b) + cAccounts = await permController.getAccounts(ORIGINS.c) + + assert.deepEqual(aAccounts, [], 'first origin should have no accounts') + assert.deepEqual(bAccounts, [], 'second origin should have no accounts') + assert.deepEqual(cAccounts, [], 'third origin should have no accounts') + + Object.keys(notifications).forEach((origin) => { + assert.deepEqual( + permController.permissions.getPermissionsForDomain(origin), + [], + 'origin should have no permissions' + ) + }) + + assert.deepEqual( + Object.keys(permController.permissions.getDomains()), [], + 'all domains should be deleted' + ) + }) + }) + + describe('removePermissionsFor', 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('removes permissions for multiple domains', async function () { + + let aAccounts = await permController.getAccounts(ORIGINS.a) + let bAccounts = await permController.getAccounts(ORIGINS.b) + + assert.deepEqual( + aAccounts, ACCOUNT_ARRAYS.a, + 'first origin should have correct accounts' + ) + assert.deepEqual( + bAccounts, ACCOUNT_ARRAYS.b, + 'second origin should have correct accounts' + ) + + permController.removePermissionsFor({ + [ORIGINS.a]: [PERM_NAMES.eth_accounts], + [ORIGINS.b]: [PERM_NAMES.eth_accounts], + }) + + aAccounts = await permController.getAccounts(ORIGINS.a) + bAccounts = await permController.getAccounts(ORIGINS.b) + + assert.deepEqual(aAccounts, [], 'first origin should have no accounts') + assert.deepEqual(bAccounts, [], 'second origin should have no accounts') + + assert.deepEqual( + notifications[ORIGINS.a], [NOTIFICATIONS.removedAccounts()], + 'first origin should have correct notification' + ) + assert.deepEqual( + notifications[ORIGINS.b], [NOTIFICATIONS.removedAccounts()], + 'second origin should have correct notification' + ) + + assert.deepEqual( + Object.keys(permController.permissions.getDomains()), [], + 'all domains should be deleted' + ) + }) + + it('only removes targeted permissions from single domain', async function () { + + grantPermissions( + permController, ORIGINS.b, PERMS.finalizedRequests.test_method() + ) + + let bPermissions = permController.permissions.getPermissionsForDomain(ORIGINS.b) + + assert.ok( + ( + bPermissions.length === 2 && + find(bPermissions, { parentCapability: PERM_NAMES.eth_accounts }) && + find(bPermissions, { parentCapability: PERM_NAMES.test_method }) + ), + 'origin should have correct permissions' + ) + + permController.removePermissionsFor({ + [ORIGINS.b]: [PERM_NAMES.test_method], + }) + + bPermissions = permController.permissions.getPermissionsForDomain(ORIGINS.b) + + assert.ok( + ( + bPermissions.length === 1 && + find(bPermissions, { parentCapability: PERM_NAMES.eth_accounts }) + ), + 'only targeted permission should have been removed' + ) + }) + + it('removes permissions for a single domain, without affecting another', async function () { + + permController.removePermissionsFor({ + [ORIGINS.b]: [PERM_NAMES.eth_accounts], + }) + + const aAccounts = await permController.getAccounts(ORIGINS.a) + const bAccounts = await permController.getAccounts(ORIGINS.b) + + assert.deepEqual( + aAccounts, ACCOUNT_ARRAYS.a, + 'first origin should have correct accounts' + ) + assert.deepEqual(bAccounts, [], 'second origin should have no accounts') + + assert.deepEqual( + notifications[ORIGINS.a], [], + 'first origin should have no notifications' + ) + assert.deepEqual( + notifications[ORIGINS.b], [NOTIFICATIONS.removedAccounts()], + 'second origin should have correct notification' + ) + + assert.deepEqual( + Object.keys(permController.permissions.getDomains()), [ORIGINS.a], + 'only first origin should remain' + ) + }) + + it('send notification but does not affect permissions for unknown domain', async function () { + + // it knows nothing of this origin + permController.removePermissionsFor({ + [ORIGINS.c]: [PERM_NAMES.eth_accounts], + }) + + assert.deepEqual( + notifications[ORIGINS.c], [NOTIFICATIONS.removedAccounts()], + 'unknown origin should have notification' + ) + + const aAccounts = await permController.getAccounts(ORIGINS.a) + const bAccounts = await permController.getAccounts(ORIGINS.b) + + assert.deepEqual( + aAccounts, ACCOUNT_ARRAYS.a, + 'first origin should have correct accounts' + ) + assert.deepEqual( + bAccounts, ACCOUNT_ARRAYS.b, + 'second origin should have correct accounts' + ) + + assert.deepEqual( + Object.keys(permController.permissions.getDomains()), + [ORIGINS.a, ORIGINS.b], + 'should have correct domains' + ) + }) + }) + + describe('validatePermittedAccounts', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + grantPermissions( + permController, ORIGINS.a, + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a) + ) + grantPermissions( + permController, ORIGINS.b, + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.b) + ) + }) + + it('throws error on non-array accounts', async function () { + + await assert.rejects( + permController.validatePermittedAccounts(undefined), + ERRORS.validatePermittedAccounts.invalidParam(), + 'should throw on undefined' + ) + + await assert.rejects( + permController.validatePermittedAccounts(false), + ERRORS.validatePermittedAccounts.invalidParam(), + 'should throw on false' + ) + + await assert.rejects( + permController.validatePermittedAccounts(true), + ERRORS.validatePermittedAccounts.invalidParam(), + 'should throw on true' + ) + + await assert.rejects( + permController.validatePermittedAccounts({}), + ERRORS.validatePermittedAccounts.invalidParam(), + 'should throw on non-array object' + ) + }) + + it('throws error on empty array of accounts', async function () { + + await assert.rejects( + permController.validatePermittedAccounts([]), + ERRORS.validatePermittedAccounts.invalidParam(), + 'should throw on empty array' + ) + }) + + it('throws error if any account value is not in keyring', async function () { + + const keyringAccounts = await permController.getKeyringAccounts() + + await assert.rejects( + permController.validatePermittedAccounts([DUMMY_ACCOUNT]), + ERRORS.validatePermittedAccounts.nonKeyringAccount(DUMMY_ACCOUNT), + 'should throw on non-keyring account' + ) + + await assert.rejects( + permController.validatePermittedAccounts(keyringAccounts.concat(DUMMY_ACCOUNT)), + ERRORS.validatePermittedAccounts.nonKeyringAccount(DUMMY_ACCOUNT), + 'should throw on non-keyring account with other accounts' + ) + }) + + it('succeeds if all accounts are in keyring', async function () { + + const keyringAccounts = await permController.getKeyringAccounts() + + await assert.doesNotReject( + permController.validatePermittedAccounts(keyringAccounts), + 'should not throw on all keyring accounts' + ) + + await assert.doesNotReject( + permController.validatePermittedAccounts([ keyringAccounts[0] ]), + 'should not throw on single keyring account' + ) + + await assert.doesNotReject( + permController.validatePermittedAccounts([ keyringAccounts[1] ]), + 'should not throw on single keyring account' + ) + }) + }) + + describe('updatePermittedAccounts', 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('throws on invalid accounts', async function () { + + await assert.rejects( + permController.updatePermittedAccounts(ORIGINS.a, {}), + ERRORS.validatePermittedAccounts.invalidParam(), + 'should throw on non-array accounts param' + ) + + await assert.rejects( + permController.updatePermittedAccounts(ORIGINS.a, []), + ERRORS.validatePermittedAccounts.invalidParam(), + 'should throw on empty array accounts param' + ) + + await assert.rejects( + permController.updatePermittedAccounts(ORIGINS.a, [DUMMY_ACCOUNT]), + ERRORS.validatePermittedAccounts.nonKeyringAccount(DUMMY_ACCOUNT), + 'should throw on non-keyring account' + ) + }) + + it('throws if origin invalid or lacks eth_accounts permission', async function () { + + await assert.rejects( + permController.updatePermittedAccounts(false, ACCOUNT_ARRAYS.a), + ERRORS.updatePermittedAccounts.invalidOrigin(), + 'should throw on invalid origin' + ) + + await assert.rejects( + permController.updatePermittedAccounts(ORIGINS.c, ACCOUNT_ARRAYS.a), + ERRORS.updatePermittedAccounts.invalidOrigin(), + 'should throw on origin without eth_accounts permission' + ) + }) + + it('successfully updates permitted accounts', async function () { + + await permController.updatePermittedAccounts(ORIGINS.a, ACCOUNT_ARRAYS.b) + await permController.updatePermittedAccounts(ORIGINS.b, ACCOUNT_ARRAYS.c) + + let aAccounts = await permController.getAccounts(ORIGINS.a) + let bAccounts = await permController.getAccounts(ORIGINS.b) + + assert.deepEqual( + aAccounts, ACCOUNT_ARRAYS.b, + 'first origin should have correct accounts' + ) + assert.deepEqual( + bAccounts, ACCOUNT_ARRAYS.c, + 'first origin should have correct accounts' + ) + + assert.deepEqual( + notifications[ORIGINS.a][0], + NOTIFICATIONS.newAccounts(ACCOUNT_ARRAYS.b), + 'first origin should have correct notification' + ) + assert.deepEqual( + notifications[ORIGINS.b][0], + NOTIFICATIONS.newAccounts(ACCOUNT_ARRAYS.c), + 'second origin should have correct notification' + ) + + await permController.updatePermittedAccounts(ORIGINS.a, ACCOUNT_ARRAYS.c) + await permController.updatePermittedAccounts(ORIGINS.b, ACCOUNT_ARRAYS.a) + + aAccounts = await permController.getAccounts(ORIGINS.a) + bAccounts = await permController.getAccounts(ORIGINS.b) + + assert.deepEqual( + aAccounts, ACCOUNT_ARRAYS.c, + 'first origin should have correct accounts' + ) + assert.deepEqual( + bAccounts, ACCOUNT_ARRAYS.a, + 'first origin should have correct accounts' + ) + + assert.deepEqual( + notifications[ORIGINS.a][1], + NOTIFICATIONS.newAccounts(ACCOUNT_ARRAYS.c), + 'first origin should have correct notification' + ) + assert.deepEqual( + notifications[ORIGINS.b][1], + NOTIFICATIONS.newAccounts(ACCOUNT_ARRAYS.a), + 'second origin should have correct notification' + ) + }) + }) + + describe('finalizePermissionsRequest', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + }) + + it('throws on non-keyring accounts', async function () { + + await assert.rejects( + permController.finalizePermissionsRequest( + PERMS.requests.eth_accounts(), [DUMMY_ACCOUNT] + ), + ERRORS.validatePermittedAccounts.nonKeyringAccount(DUMMY_ACCOUNT), + 'should throw on non-keyring account' + ) + }) + + it('adds caveat to eth_accounts permission', async function () { + + const perm = await permController.finalizePermissionsRequest( + PERMS.requests.eth_accounts(), + ACCOUNT_ARRAYS.a, + ) + + assert.deepEqual(perm, PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a)) + }) + + it('replaces caveat of eth_accounts permission', async function () { + + const perm = await permController.finalizePermissionsRequest( + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a), + ACCOUNT_ARRAYS.b, + ) + + assert.deepEqual( + perm, PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.b), + 'permission should have correct caveat' + ) + }) + + it('handles non-eth_accounts permission', async function () { + + const perm = await permController.finalizePermissionsRequest( + PERMS.finalizedRequests.test_method(), + ACCOUNT_ARRAYS.b, + ) + + assert.deepEqual( + perm, PERMS.finalizedRequests.test_method(), + 'permission should have correct caveat' + ) + }) + }) + + 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('handleNewAccountSelected', 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('throws if invalid origin or account', async function () { + + await assert.rejects( + permController.handleNewAccountSelected({}, DUMMY_ACCOUNT), + ERRORS.handleNewAccountSelected.invalidParams(), + 'should throw if origin non-string' + ) + + await assert.rejects( + permController.handleNewAccountSelected('', DUMMY_ACCOUNT), + ERRORS.handleNewAccountSelected.invalidParams(), + 'should throw if origin empty string' + ) + + await assert.rejects( + permController.handleNewAccountSelected(ORIGINS.a, {}), + ERRORS.handleNewAccountSelected.invalidParams(), + 'should throw if account non-string' + ) + + await assert.rejects( + permController.handleNewAccountSelected(ORIGINS.a, ''), + ERRORS.handleNewAccountSelected.invalidParams(), + 'should throw if account empty string' + ) + }) + + it('does nothing if account not permitted for origin', async function () { + + await permController.handleNewAccountSelected( + ORIGINS.b, ACCOUNT_ARRAYS.c[0] + ) + + assert.deepEqual( + notifications[ORIGINS.b], [], + 'should not have emitted notification' + ) + }) + + it('does nothing if account already first in array', async function () { + + await permController.handleNewAccountSelected( + ORIGINS.a, ACCOUNT_ARRAYS.a[0] + ) + + assert.deepEqual( + notifications[ORIGINS.a], [], + 'should not have emitted notification' + ) + }) + + it('emits notification if selected account not first in array', async function () { + + await permController.handleNewAccountSelected( + ORIGINS.a, ACCOUNT_ARRAYS.a[1] + ) + + assert.deepEqual( + notifications[ORIGINS.a], + [NOTIFICATIONS.newAccounts([ ...ACCOUNT_ARRAYS.a ].reverse())], + 'should have emitted notification' + ) + }) + }) + + describe('approvePermissionsRequest', function () { + + let permController, mockRequestUserApproval + + beforeEach(function () { + permController = initPermController() + mockRequestUserApproval = getMockRequestUserApprovalFunction( + permController + ) + }) + + it('does nothing if called on non-existing request', async function () { + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should be empty on init', + ) + + sinon.spy(permController, 'finalizePermissionsRequest') + + const request = PERMS.approvedRequest(REQUEST_IDS.a, null) + + await assert.doesNotReject( + permController.approvePermissionsRequest(request, null), + 'should not throw on non-existing request' + ) + + assert.ok( + permController.finalizePermissionsRequest.notCalled, + 'should not call finalizePermissionRequest' + ) + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should still be empty after request', + ) + }) + + it('rejects request with bad accounts param', async function () { + + const request = PERMS.approvedRequest( + REQUEST_IDS.a, + PERMS.requests.eth_accounts() + ) + + const requestRejection = assert.rejects( + mockRequestUserApproval(REQUEST_IDS.a), + ERRORS.validatePermittedAccounts.invalidParam(), + 'should reject bad accounts' + ) + + await permController.approvePermissionsRequest(request, null) + await requestRejection + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should be empty after rejection', + ) + }) + + it('rejects request with no permissions', async function () { + + const request = PERMS.approvedRequest(REQUEST_IDS.a, {}) + + const requestRejection = assert.rejects( + mockRequestUserApproval(REQUEST_IDS.a), + ERRORS.approvePermissionsRequest.noPermsRequested(), + 'should reject if no permissions in request' + ) + + await permController.approvePermissionsRequest(request, ACCOUNT_ARRAYS.a) + await requestRejection + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should be empty after rejection', + ) + }) + + it('approves valid request', async function () { + + const request = PERMS.approvedRequest(REQUEST_IDS.a, PERMS.requests.eth_accounts()) + + let perms + + const requestApproval = assert.doesNotReject( + async () => { + perms = await mockRequestUserApproval(REQUEST_IDS.a) + }, + 'should not reject single valid request' + ) + + await permController.approvePermissionsRequest(request, ACCOUNT_ARRAYS.a) + await requestApproval + + assert.deepEqual( + perms, PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a), + 'should produce expected approved permissions' + ) + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should be empty after approval', + ) + }) + + it('approves valid requests regardless of order', async function () { + + const request1 = PERMS.approvedRequest(REQUEST_IDS.a, PERMS.requests.eth_accounts()) + const request2 = PERMS.approvedRequest(REQUEST_IDS.b, PERMS.requests.eth_accounts()) + const request3 = PERMS.approvedRequest(REQUEST_IDS.c, PERMS.requests.eth_accounts()) + + let perms1, perms2 + + const approval1 = assert.doesNotReject( + async () => { + perms1 = await mockRequestUserApproval(REQUEST_IDS.a) + }, + 'should not reject request' + ) + + const approval2 = assert.doesNotReject( + async () => { + perms2 = await mockRequestUserApproval(REQUEST_IDS.b) + }, + 'should not reject request' + ) + + // approve out of order + await permController.approvePermissionsRequest(request2, ACCOUNT_ARRAYS.b) + // add a non-existing request to the mix + await permController.approvePermissionsRequest(request3, ACCOUNT_ARRAYS.c) + await permController.approvePermissionsRequest(request1, ACCOUNT_ARRAYS.a) + + await approval1 + await approval2 + + assert.deepEqual( + perms1, PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a), + 'first request should produce expected approved permissions' + ) + + assert.deepEqual( + perms2, PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.b), + 'second request should produce expected approved permissions' + ) + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should be empty after approvals', + ) + }) + }) + + describe('rejectPermissionsRequest', function () { + + let permController, mockRequestUserApproval + + beforeEach(async function () { + permController = initPermController() + mockRequestUserApproval = getMockRequestUserApprovalFunction( + permController + ) + }) + + it('does nothing if called on non-existing request', async function () { + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should be empty on init', + ) + + await assert.doesNotReject( + permController.rejectPermissionsRequest(REQUEST_IDS.a), + 'should not throw on non-existing request' + ) + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should still be empty after request', + ) + }) + + it('rejects single existing request', async function () { + + const requestRejection = assert.rejects( + mockRequestUserApproval(REQUEST_IDS.a), + ERRORS.rejectPermissionsRequest.rejection(), + 'should reject with expected error' + ) + + await permController.rejectPermissionsRequest(REQUEST_IDS.a) + await requestRejection + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should be empty after rejection', + ) + }) + + it('rejects requests regardless of order', async function () { + + const requestRejection1 = assert.rejects( + mockRequestUserApproval(REQUEST_IDS.b), + ERRORS.rejectPermissionsRequest.rejection(), + 'should reject with expected error' + ) + + const requestRejection2 = assert.rejects( + mockRequestUserApproval(REQUEST_IDS.c), + ERRORS.rejectPermissionsRequest.rejection(), + 'should reject with expected error' + ) + + // reject out of order + await permController.rejectPermissionsRequest(REQUEST_IDS.c) + // add a non-existing request to the mix + await permController.rejectPermissionsRequest(REQUEST_IDS.a) + await permController.rejectPermissionsRequest(REQUEST_IDS.b) + + await requestRejection1 + await requestRejection2 + + assert.equal( + permController.pendingApprovals.size, 0, + 'pending approvals should be empty after approval', + ) + }) + }) + + // see permissions-middleware-test for testing the middleware itself + describe('createMiddleware', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + }) + + it('should throw on bad origin', function () { + + assert.throws( + () => permController.createMiddleware({ origin: {} }), + ERRORS.createMiddleware.badOrigin(), + 'should throw expected error' + ) + + assert.throws( + () => permController.createMiddleware({ origin: '' }), + ERRORS.createMiddleware.badOrigin(), + 'should throw expected error' + ) + + assert.throws( + () => permController.createMiddleware({}), + ERRORS.createMiddleware.badOrigin(), + 'should throw expected error' + ) + }) + + it('should create a middleware', function () { + + let middleware + assert.doesNotThrow( + () => { + middleware = permController.createMiddleware({ origin: ORIGINS.a }) + }, + 'should not throw' + ) + + assert.equal( + typeof middleware, 'function', + 'should return function' + ) + + assert.equal( + middleware.name, 'engineAsMiddleware', + 'function name should be "engineAsMiddleware"' + ) + }) + + it('should create a middleware with extensionId', function () { + + const extensionId = 'fooExtension' + + let middleware + assert.doesNotThrow( + () => { + middleware = permController.createMiddleware({ + origin: ORIGINS.a, + extensionId, + }) + }, + 'should not throw' + ) + + assert.equal( + typeof middleware, 'function', + 'should return function' + ) + + assert.equal( + middleware.name, 'engineAsMiddleware', + 'function name should be "engineAsMiddleware"' + ) + + const metadataStore = permController.store.getState()[METADATA_STORE_KEY] + + assert.deepEqual( + metadataStore[ORIGINS.a], { extensionId }, + 'metadata should be stored' + ) + }) + }) + + describe('notifyDomain', function () { + + let notifications, permController + + beforeEach(function () { + notifications = initNotifications() + permController = initPermController(notifications) + sinon.spy(permController.permissionsLog, 'updateAccountsHistory') + }) + + it('notifyDomain handles accountsChanged', async function () { + + permController.notifyDomain( + ORIGINS.a, + NOTIFICATIONS.newAccounts(ACCOUNT_ARRAYS.a), + ) + + assert.ok( + permController.permissionsLog.updateAccountsHistory.calledOnce, + 'permissionsLog.updateAccountsHistory should have been called once' + ) + + assert.deepEqual( + notifications[ORIGINS.a], + [ NOTIFICATIONS.newAccounts(ACCOUNT_ARRAYS.a) ], + 'origin should have correct notification' + ) + }) + + it('notifyDomain handles notifications other than accountsChanged', async function () { + + permController.notifyDomain(ORIGINS.a, NOTIFICATIONS.test()) + + assert.ok( + permController.permissionsLog.updateAccountsHistory.notCalled, + 'permissionsLog.updateAccountsHistory should not have been called' + ) + + assert.deepEqual( + notifications[ORIGINS.a], + [ NOTIFICATIONS.test() ], + 'origin should have correct notification' + ) + }) + }) + + describe('miscellanea and edge cases', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + }) + + it('_addPendingApproval: should throw if adding origin twice', function () { + + const id = nanoid() + const origin = ORIGINS.a + + permController._addPendingApproval(id, origin, noop, noop) + + const otherId = nanoid() + + assert.throws( + () => permController._addPendingApproval(otherId, origin, noop, noop), + ERRORS.pendingApprovals.duplicateOriginOrId(otherId, origin), + 'should throw expected error' + ) + + assert.equal( + permController.pendingApprovals.size, 1, + 'pending approvals should have single entry', + ) + + assert.equal( + permController.pendingApprovalOrigins.size, 1, + 'pending approval origins should have single item', + ) + + assert.deepEqual( + permController.pendingApprovals.get(id), + { origin, resolve: noop, reject: noop }, + 'pending approvals should have expected entry' + ) + + assert.ok( + permController.pendingApprovalOrigins.has(origin), + 'pending approval origins should have expected item', + ) + }) + + it('addInternalMethodPrefix', function () { + const str = 'foo' + const res = addInternalMethodPrefix(str) + assert.equal(res, WALLET_PREFIX + str, 'should prefix correctly') + }) + }) +}) diff --git a/test/unit/app/controllers/permissions/permissions-log-controller-test.js b/test/unit/app/controllers/permissions/permissions-log-controller-test.js new file mode 100644 index 000000000..80225d51b --- /dev/null +++ b/test/unit/app/controllers/permissions/permissions-log-controller-test.js @@ -0,0 +1,690 @@ +import { strict as assert } from 'assert' +import ObservableStore from 'obs-store' +import nanoid from 'nanoid' +import { useFakeTimers } from 'sinon' + +import PermissionsLogController + from '../../../../../app/scripts/controllers/permissions/permissionsLog' + +import { + LOG_LIMIT, + LOG_METHOD_TYPES, +} from '../../../../../app/scripts/controllers/permissions/enums' + +import { + validateActivityEntry, +} from './helpers' + +import { + constants, + getters, + noop, +} from './mocks' + +const { + ERRORS, + PERMS, + RPC_REQUESTS, +} = getters + +const { + ACCOUNT_ARRAYS, + EXPECTED_HISTORIES, + ORIGINS, + PERM_NAMES, + REQUEST_IDS, + RESTRICTED_METHODS, +} = constants + +let clock + +const initPermLog = () => { + return new PermissionsLogController({ + store: new ObservableStore(), + restrictedMethods: RESTRICTED_METHODS, + }) +} + +const mockNext = (handler) => { + if (handler) { + handler(noop) + } +} + +const initMiddleware = (permLog) => { + const middleware = permLog.createMiddleware() + return (req, res, next = mockNext) => { + middleware(req, res, next) + } +} + +const initClock = () => { + clock = useFakeTimers(1) +} + +const tearDownClock = () => { + clock.restore() +} + +const getSavedMockNext = (arr) => (handler) => { + arr.push(handler) +} + +describe('permissions log', function () { + + describe('activity log', function () { + + let permLog, logMiddleware + + beforeEach(function () { + permLog = initPermLog() + logMiddleware = initMiddleware(permLog) + }) + + it('records activity for restricted methods', function () { + + let log, req, res + + // test_method, success + + req = RPC_REQUESTS.test_method(ORIGINS.a) + req.id = REQUEST_IDS.a + res = { foo: 'bar' } + + logMiddleware({ ...req }, res) + + log = permLog.getActivityLog() + const entry1 = log[0] + + assert.equal(log.length, 1, 'log should have single entry') + validateActivityEntry( + entry1, { ...req }, { ...res }, + LOG_METHOD_TYPES.restricted, true + ) + + // eth_accounts, failure + + req = RPC_REQUESTS.eth_accounts(ORIGINS.b) + req.id = REQUEST_IDS.b + res = { error: new Error('Unauthorized.') } + + logMiddleware({ ...req }, res) + + log = permLog.getActivityLog() + const entry2 = log[1] + + assert.equal(log.length, 2, 'log should have 2 entries') + validateActivityEntry( + entry2, { ...req }, { ...res }, + LOG_METHOD_TYPES.restricted, false + ) + + // eth_requestAccounts, success + + req = RPC_REQUESTS.eth_requestAccounts(ORIGINS.c) + req.id = REQUEST_IDS.c + res = { result: ACCOUNT_ARRAYS.c } + + logMiddleware({ ...req }, res) + + log = permLog.getActivityLog() + const entry3 = log[2] + + assert.equal(log.length, 3, 'log should have 3 entries') + validateActivityEntry( + entry3, { ...req }, { ...res }, + LOG_METHOD_TYPES.restricted, true + ) + + // test_method, no response + + req = RPC_REQUESTS.test_method(ORIGINS.a) + req.id = REQUEST_IDS.a + res = null + + logMiddleware({ ...req }, res) + + log = permLog.getActivityLog() + const entry4 = log[3] + + assert.equal(log.length, 4, 'log should have 4 entries') + validateActivityEntry( + entry4, { ...req }, null, + LOG_METHOD_TYPES.restricted, false + ) + + // validate final state + + assert.equal(entry1, log[0], 'first log entry should remain') + assert.equal(entry2, log[1], 'second log entry should remain') + assert.equal(entry3, log[2], 'third log entry should remain') + assert.equal(entry4, log[3], 'fourth log entry should remain') + }) + + it('handles responses added out of order', function () { + + let log + + const handlerArray = [] + + const id1 = nanoid() + const id2 = nanoid() + const id3 = nanoid() + + const req = RPC_REQUESTS.test_method(ORIGINS.a) + + // get make requests + req.id = id1 + const res1 = { foo: id1 } + logMiddleware({ ...req }, { ...res1 }, getSavedMockNext(handlerArray)) + + req.id = id2 + const res2 = { foo: id2 } + logMiddleware({ ...req }, { ...res2 }, getSavedMockNext(handlerArray)) + + req.id = id3 + const res3 = { foo: id3 } + logMiddleware({ ...req }, { ...res3 }, getSavedMockNext(handlerArray)) + + // verify log state + log = permLog.getActivityLog() + assert.equal(log.length, 3, 'log should have 3 entries') + const entry1 = log[0] + const entry2 = log[1] + const entry3 = log[2] + assert.ok( + ( + entry1.id === id1 && entry1.response === null && + entry2.id === id2 && entry2.response === null && + entry3.id === id3 && entry3.response === null + ), + 'all entries should be in correct order and without responses' + ) + + // call response handlers + for (const i of [1, 2, 0]) { + handlerArray[i](noop) + } + + // verify log state again + log = permLog.getActivityLog() + assert.equal(log.length, 3, 'log should have 3 entries') + + // verify all entries + log = permLog.getActivityLog() + + validateActivityEntry( + log[0], { ...req, id: id1 }, { ...res1 }, + LOG_METHOD_TYPES.restricted, true + ) + + validateActivityEntry( + log[1], { ...req, id: id2 }, { ...res2 }, + LOG_METHOD_TYPES.restricted, true + ) + + validateActivityEntry( + log[2], { ...req, id: id3 }, { ...res3 }, + LOG_METHOD_TYPES.restricted, true + ) + }) + + it('handles a lack of response', function () { + + let req = RPC_REQUESTS.test_method(ORIGINS.a) + req.id = REQUEST_IDS.a + let res = { foo: 'bar' } + + // noop for next handler prevents recording of response + logMiddleware({ ...req }, res, noop) + + let log = permLog.getActivityLog() + const entry1 = log[0] + + assert.equal(log.length, 1, 'log should have single entry') + validateActivityEntry( + entry1, { ...req }, null, + LOG_METHOD_TYPES.restricted, true + ) + + // next request should be handled as normal + req = RPC_REQUESTS.eth_accounts(ORIGINS.b) + req.id = REQUEST_IDS.b + res = { result: ACCOUNT_ARRAYS.b } + + logMiddleware({ ...req }, res) + + log = permLog.getActivityLog() + const entry2 = log[1] + assert.equal(log.length, 2, 'log should have 2 entries') + validateActivityEntry( + entry2, { ...req }, { ...res }, + LOG_METHOD_TYPES.restricted, true + ) + + // validate final state + assert.equal(entry1, log[0], 'first log entry remains') + assert.equal(entry2, log[1], 'second log entry remains') + }) + + it('ignores expected methods', function () { + + let log = permLog.getActivityLog() + assert.equal(log.length, 0, 'log should be empty') + + const res = { foo: 'bar' } + const req1 = RPC_REQUESTS.wallet_sendDomainMetadata(ORIGINS.c, 'foobar') + const req2 = RPC_REQUESTS.custom(ORIGINS.b, 'eth_getBlockNumber') + const req3 = RPC_REQUESTS.custom(ORIGINS.b, 'net_version') + + logMiddleware(req1, res) + logMiddleware(req2, res) + logMiddleware(req3, res) + + log = permLog.getActivityLog() + assert.equal(log.length, 0, 'log should still be empty') + }) + + it('enforces log limit', function () { + + const req = RPC_REQUESTS.test_method(ORIGINS.a) + const res = { foo: 'bar' } + + // max out log + let lastId + for (let i = 0; i < LOG_LIMIT; i++) { + lastId = nanoid() + logMiddleware({ ...req, id: lastId }, { ...res }) + } + + // check last entry valid + let log = permLog.getActivityLog() + assert.equal( + log.length, LOG_LIMIT, 'log should have LOG_LIMIT num entries' + ) + + validateActivityEntry( + log[LOG_LIMIT - 1], { ...req, id: lastId }, res, + LOG_METHOD_TYPES.restricted, true + ) + + // store the id of the current second entry + const nextFirstId = log[1]['id'] + + // add one more entry to log, putting it over the limit + lastId = nanoid() + logMiddleware({ ...req, id: lastId }, { ...res }) + + // check log length + log = permLog.getActivityLog() + assert.equal( + log.length, LOG_LIMIT, 'log should have LOG_LIMIT num entries' + ) + + // check first and last entries + validateActivityEntry( + log[0], { ...req, id: nextFirstId }, res, + LOG_METHOD_TYPES.restricted, true + ) + + validateActivityEntry( + log[LOG_LIMIT - 1], { ...req, id: lastId }, res, + LOG_METHOD_TYPES.restricted, true + ) + }) + }) + + describe('permissions history', function () { + + let permLog, logMiddleware + + beforeEach(function () { + permLog = initPermLog() + logMiddleware = initMiddleware(permLog) + initClock() + }) + + afterEach(function () { + tearDownClock() + }) + + it('only updates history on responses', function () { + + let permHistory + + const req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.test_method + ) + const res = { result: [ PERMS.granted.test_method() ] } + + // noop => no response + logMiddleware({ ...req }, { ...res }, noop) + + permHistory = permLog.getHistory() + assert.deepEqual(permHistory, {}, 'history should not have been updated') + + // response => records granted permissions + logMiddleware({ ...req }, { ...res }) + + permHistory = permLog.getHistory() + assert.equal( + Object.keys(permHistory).length, 1, + 'history should have single origin' + ) + assert.ok( + Boolean(permHistory[ORIGINS.a]), + 'history should have expected origin' + ) + }) + + it('ignores malformed permissions requests', function () { + + const req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.test_method + ) + delete req.params + const res = { result: [ PERMS.granted.test_method() ] } + + // no params => no response + logMiddleware({ ...req }, { ...res }) + + assert.deepEqual(permLog.getHistory(), {}, 'history should not have been updated') + }) + + it('records and updates account history as expected', async function () { + + let permHistory + + const req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.eth_accounts + ) + const res = { + result: [ PERMS.granted.eth_accounts(ACCOUNT_ARRAYS.a) ], + } + + logMiddleware({ ...req }, { ...res }) + + // validate history + + permHistory = permLog.getHistory() + + assert.deepEqual( + permHistory, + EXPECTED_HISTORIES.case1[0], + 'should have correct history' + ) + + // mock permission requested again, with another approved account + + clock.tick(1) + + res.result = [ PERMS.granted.eth_accounts([ ACCOUNT_ARRAYS.a[0] ]) ] + + logMiddleware({ ...req }, { ...res }) + + permHistory = permLog.getHistory() + + assert.deepEqual( + permHistory, + EXPECTED_HISTORIES.case1[1], + 'should have correct history' + ) + }) + + it('handles eth_accounts response without caveats', async function () { + + const req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.eth_accounts + ) + const res = { + result: [ PERMS.granted.eth_accounts(ACCOUNT_ARRAYS.a) ], + } + delete res.result[0].caveats + + logMiddleware({ ...req }, { ...res }) + + // validate history + + assert.deepEqual( + permLog.getHistory(), EXPECTED_HISTORIES.case2[0], + 'should have expected history' + ) + }) + + it('handles extra caveats for eth_accounts', async function () { + + const req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.eth_accounts + ) + const res = { + result: [ PERMS.granted.eth_accounts(ACCOUNT_ARRAYS.a) ], + } + res.result[0].caveats.push({ foo: 'bar' }) + + logMiddleware({ ...req }, { ...res }) + + // validate history + + assert.deepEqual( + permLog.getHistory(), + EXPECTED_HISTORIES.case1[0], + 'should have correct history' + ) + }) + + // wallet_requestPermissions returns all permissions approved for the + // requesting origin, including old ones + it('handles unrequested permissions on the response', async function () { + + const req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.eth_accounts + ) + const res = { + result: [ + PERMS.granted.eth_accounts(ACCOUNT_ARRAYS.a), + PERMS.granted.test_method(), + ], + } + + logMiddleware({ ...req }, { ...res }) + + // validate history + + assert.deepEqual( + permLog.getHistory(), + EXPECTED_HISTORIES.case1[0], + 'should have correct history' + ) + }) + + it('does not update history if no new permissions are approved', async function () { + + let req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.test_method + ) + let res = { + result: [ + PERMS.granted.test_method(), + ], + } + + logMiddleware({ ...req }, { ...res }) + + // validate history + + assert.deepEqual( + permLog.getHistory(), + EXPECTED_HISTORIES.case4[0], + 'should have correct history' + ) + + // new permission requested, but not approved + + clock.tick(1) + + req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.eth_accounts + ) + res = { + result: [ + PERMS.granted.test_method(), + ], + } + + logMiddleware({ ...req }, { ...res }) + + // validate history + + assert.deepEqual( + permLog.getHistory(), + EXPECTED_HISTORIES.case4[0], + 'should have same history as before' + ) + }) + + it('records and updates history for multiple origins, regardless of response order', async function () { + + let permHistory + + // make first round of requests + + const round1 = [] + const handlers1 = [] + + // first origin + round1.push({ + req: RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.test_method + ), + res: { + result: [ PERMS.granted.test_method() ], + }, + }) + + // second origin + round1.push({ + req: RPC_REQUESTS.requestPermission( + ORIGINS.b, PERM_NAMES.eth_accounts + ), + res: { + result: [ PERMS.granted.eth_accounts(ACCOUNT_ARRAYS.b) ], + }, + }) + + // third origin + round1.push({ + req: RPC_REQUESTS.requestPermissions(ORIGINS.c, { + [PERM_NAMES.test_method]: {}, + [PERM_NAMES.eth_accounts]: {}, + }), + res: { + result: [ + PERMS.granted.test_method(), + PERMS.granted.eth_accounts(ACCOUNT_ARRAYS.c), + ], + }, + }) + + // make requests and process responses out of order + round1.forEach((x) => { + logMiddleware({ ...x.req }, { ...x.res }, getSavedMockNext(handlers1)) + }) + + for (const i of [1, 2, 0]) { + handlers1[i](noop) + } + + // validate history + permHistory = permLog.getHistory() + + assert.deepEqual( + permHistory, EXPECTED_HISTORIES.case3[0], + 'should have expected history' + ) + + // make next round of requests + + clock.tick(1) + + const round2 = [] + // we're just gonna process these in order + + // first origin + round2.push({ + req: RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.test_method + ), + res: { + result: [ PERMS.granted.test_method() ], + }, + }) + + // nothing for second origin + + // third origin + round2.push({ + req: RPC_REQUESTS.requestPermissions(ORIGINS.c, { + [PERM_NAMES.eth_accounts]: {}, + }), + res: { + result: [ + PERMS.granted.eth_accounts(ACCOUNT_ARRAYS.b), + ], + }, + }) + + // make requests + round2.forEach((x) => { + logMiddleware({ ...x.req }, { ...x.res }) + }) + + // validate history + permHistory = permLog.getHistory() + + assert.deepEqual( + permHistory, EXPECTED_HISTORIES.case3[1], + 'should have expected history' + ) + }) + }) + + 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' + ) + }) + }) +}) diff --git a/test/unit/app/controllers/permissions/permissions-middleware-test.js b/test/unit/app/controllers/permissions/permissions-middleware-test.js new file mode 100644 index 000000000..b27669072 --- /dev/null +++ b/test/unit/app/controllers/permissions/permissions-middleware-test.js @@ -0,0 +1,752 @@ +import { strict as assert } from 'assert' + +import { + METADATA_STORE_KEY, +} from '../../../../../app/scripts/controllers/permissions/enums' + +import { + PermissionsController, +} from '../../../../../app/scripts/controllers/permissions' + +import { + getUserApprovalPromise, + grantPermissions, +} from './helpers' + +import { + constants, + getters, + getPermControllerOpts, + getPermissionsMiddleware, +} from './mocks' + +const { + CAVEATS, + ERRORS, + PERMS, + RPC_REQUESTS, +} = getters + +const { + ACCOUNT_ARRAYS, + ORIGINS, + PERM_NAMES, +} = constants + +const validatePermission = (perm, name, origin, caveats) => { + assert.equal(name, perm.parentCapability, 'should have expected permission name') + assert.equal(origin, perm.invoker, 'should have expected permission origin') + if (caveats) { + assert.deepEqual(caveats, perm.caveats, 'should have expected permission caveats') + } else { + assert.ok(!perm.caveats, 'should not have any caveats') + } +} + +const initPermController = () => { + return new PermissionsController({ + ...getPermControllerOpts(), + }) +} + +describe('permissions middleware', function () { + + describe('wallet_requestPermissions', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + }) + + it('grants permissions on user approval', async function () { + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + + const req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.eth_accounts + ) + const res = {} + + const pendingApproval = assert.doesNotReject( + aMiddleware(req, res), + 'should not reject permissions request' + ) + + assert.equal( + permController.pendingApprovals.size, 1, + 'perm controller should have single pending approval', + ) + + const id = permController.pendingApprovals.keys().next().value + const approvedReq = PERMS.approvedRequest(id, PERMS.requests.eth_accounts()) + + await permController.approvePermissionsRequest(approvedReq, ACCOUNT_ARRAYS.a) + await pendingApproval + + assert.ok( + res.result && !res.error, + 'response should have result and no error' + ) + + assert.equal( + res.result.length, 1, + 'origin should have single approved permission' + ) + + validatePermission( + res.result[0], + PERM_NAMES.eth_accounts, + ORIGINS.a, + [CAVEATS.eth_accounts(ACCOUNT_ARRAYS.a)] + ) + + const aAccounts = await permController.getAccounts(ORIGINS.a) + assert.deepEqual( + aAccounts, ACCOUNT_ARRAYS.a, + 'origin should have correct accounts' + ) + }) + + it('handles serial approved requests that overwrite existing permissions', async function () { + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + + // create first request + + const req1 = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.eth_accounts + ) + const res1 = {} + + // send, approve, and validate first request + // note use of ACCOUNT_ARRAYS.a + + const pendingApproval1 = assert.doesNotReject( + aMiddleware(req1, res1), + 'should not reject permissions request' + ) + + const id1 = permController.pendingApprovals.keys().next().value + const approvedReq1 = PERMS.approvedRequest(id1, PERMS.requests.eth_accounts()) + + await permController.approvePermissionsRequest(approvedReq1, ACCOUNT_ARRAYS.a) + await pendingApproval1 + + assert.ok( + res1.result && !res1.error, + 'response should have result and no error' + ) + + assert.equal( + res1.result.length, 1, + 'origin should have single approved permission' + ) + + validatePermission( + res1.result[0], + PERM_NAMES.eth_accounts, + ORIGINS.a, + [CAVEATS.eth_accounts(ACCOUNT_ARRAYS.a)] + ) + + const accounts1 = await permController.getAccounts(ORIGINS.a) + assert.deepEqual( + accounts1, ACCOUNT_ARRAYS.a, + 'origin should have correct accounts' + ) + + // create second request + + const requestedPerms2 = { + ...PERMS.requests.eth_accounts(), + ...PERMS.requests.test_method(), + } + + const req2 = RPC_REQUESTS.requestPermissions( + ORIGINS.a, { ...requestedPerms2 } + ) + const res2 = {} + + // send, approve, and validate second request + // note use of ACCOUNT_ARRAYS.b + + const pendingApproval2 = assert.doesNotReject( + aMiddleware(req2, res2), + 'should not reject permissions request' + ) + + const id2 = permController.pendingApprovals.keys().next().value + const approvedReq2 = PERMS.approvedRequest(id2, { ...requestedPerms2 }) + + await permController.approvePermissionsRequest(approvedReq2, ACCOUNT_ARRAYS.b) + await pendingApproval2 + + assert.ok( + res2.result && !res2.error, + 'response should have result and no error' + ) + + assert.equal( + res2.result.length, 2, + 'origin should have single approved permission' + ) + + validatePermission( + res2.result[0], + PERM_NAMES.eth_accounts, + ORIGINS.a, + [CAVEATS.eth_accounts(ACCOUNT_ARRAYS.b)] + ) + + validatePermission( + res2.result[1], + PERM_NAMES.test_method, + ORIGINS.a, + ) + + const accounts2 = await permController.getAccounts(ORIGINS.a) + assert.deepEqual( + accounts2, ACCOUNT_ARRAYS.b, + 'origin should have correct accounts' + ) + }) + + it('rejects permissions on user rejection', async function () { + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + + const req = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.eth_accounts + ) + const res = {} + + const expectedError = ERRORS.rejectPermissionsRequest.rejection() + + const requestRejection = assert.rejects( + aMiddleware(req, res), + expectedError, + 'request should be rejected with correct error', + ) + + assert.equal( + permController.pendingApprovals.size, 1, + 'perm controller should have single pending approval', + ) + + const id = permController.pendingApprovals.keys().next().value + + await permController.rejectPermissionsRequest(id) + await requestRejection + + assert.ok( + ( + !res.result && res.error && + res.error.message === expectedError.message + ), + 'response should have expected error and no result' + ) + + const aAccounts = await permController.getAccounts(ORIGINS.a) + assert.deepEqual( + aAccounts, [], 'origin should have have correct accounts' + ) + }) + + it('rejects requests with unknown permissions', async function () { + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + + const req = RPC_REQUESTS.requestPermissions( + ORIGINS.a, { + ...PERMS.requests.does_not_exist(), + ...PERMS.requests.test_method(), + } + ) + const res = {} + + const expectedError = ERRORS.rejectPermissionsRequest.methodNotFound( + PERM_NAMES.does_not_exist + ) + + await assert.rejects( + aMiddleware(req, res), + expectedError, + 'request should be rejected with correct error', + ) + + assert.equal( + permController.pendingApprovals.size, 0, + 'perm controller should have no pending approvals', + ) + + assert.ok( + ( + !res.result && res.error && + res.error.message === expectedError.message + ), + 'response should have expected error and no result' + ) + }) + + it('accepts only a single pending permissions request per origin', async function () { + + const expectedError = ERRORS.pendingApprovals.requestAlreadyPending() + + // two middlewares for two origins + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + const bMiddleware = getPermissionsMiddleware(permController, ORIGINS.b) + + // create and start processing first request for first origin + + const reqA1 = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.test_method + ) + const resA1 = {} + + const requestApproval1 = assert.doesNotReject( + aMiddleware(reqA1, resA1), + 'should not reject permissions request' + ) + + // create and start processing first request for second origin + + const reqB1 = RPC_REQUESTS.requestPermission( + ORIGINS.b, PERM_NAMES.test_method + ) + const resB1 = {} + + const requestApproval2 = assert.doesNotReject( + bMiddleware(reqB1, resB1), + 'should not reject permissions request' + ) + + assert.equal( + permController.pendingApprovals.size, 2, + 'perm controller should have expected number of pending approvals', + ) + + // create and start processing second request for first origin, + // which should throw + + const reqA2 = RPC_REQUESTS.requestPermission( + ORIGINS.a, PERM_NAMES.test_method + ) + const resA2 = {} + + await assert.rejects( + aMiddleware(reqA2, resA2), + expectedError, + 'request should be rejected with correct error', + ) + + assert.ok( + ( + !resA2.result && resA2.error && + resA2.error.message === expectedError.message + ), + 'response should have expected error and no result' + ) + + // first requests for both origins should remain + + assert.equal( + permController.pendingApprovals.size, 2, + 'perm controller should have expected number of pending approvals', + ) + + // now, remaining pending requests should be approved without issue + + for (const id of permController.pendingApprovals.keys()) { + await permController.approvePermissionsRequest( + PERMS.approvedRequest(id, PERMS.requests.test_method()) + ) + } + await requestApproval1 + await requestApproval2 + + assert.ok( + resA1.result && !resA1.error, + 'first response should have result and no error' + ) + assert.equal( + resA1.result.length, 1, + 'first origin should have single approved permission' + ) + + assert.ok( + resB1.result && !resB1.error, + 'second response should have result and no error' + ) + assert.equal( + resB1.result.length, 1, + 'second origin should have single approved permission' + ) + + assert.equal( + permController.pendingApprovals.size, 0, + 'perm controller should have expected number of pending approvals', + ) + }) + }) + + describe('restricted methods', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + }) + + it('prevents restricted method access for unpermitted domain', async function () { + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + + const req = RPC_REQUESTS.test_method(ORIGINS.a) + const res = {} + + const expectedError = ERRORS.rpcCap.unauthorized() + + await assert.rejects( + aMiddleware(req, res), + expectedError, + 'request should be rejected with correct error', + ) + + assert.ok( + ( + !res.result && res.error && + res.error.code === expectedError.code + ), + 'response should have expected error and no result' + ) + }) + + it('allows restricted method access for permitted domain', async function () { + + const bMiddleware = getPermissionsMiddleware(permController, ORIGINS.b) + + grantPermissions(permController, ORIGINS.b, PERMS.finalizedRequests.test_method()) + + const req = RPC_REQUESTS.test_method(ORIGINS.b, true) + const res = {} + + await assert.doesNotReject( + bMiddleware(req, res), + 'should not reject' + ) + + assert.ok( + res.result && res.result === 1, + 'response should have correct result' + ) + }) + }) + + describe('eth_accounts', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + }) + + it('returns empty array for non-permitted domain', async function () { + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + + const req = RPC_REQUESTS.eth_accounts(ORIGINS.a) + const res = {} + + await assert.doesNotReject( + aMiddleware(req, res), + 'should not reject' + ) + + assert.ok( + res.result && !res.error, + 'response should have result and no error' + ) + assert.deepEqual( + res.result, [], + 'response should have correct result' + ) + }) + + it('returns correct accounts for permitted domain', async function () { + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + + grantPermissions( + permController, ORIGINS.a, + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a) + ) + + const req = RPC_REQUESTS.eth_accounts(ORIGINS.a) + const res = {} + + await assert.doesNotReject( + aMiddleware(req, res), + 'should not reject' + ) + + assert.ok( + res.result && !res.error, + 'response should have result and no error' + ) + assert.deepEqual( + res.result, ACCOUNT_ARRAYS.a, + 'response should have correct result' + ) + }) + }) + + describe('eth_requestAccounts', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + }) + + it('requests accounts for unpermitted origin, and approves on user approval', async function () { + + const userApprovalPromise = getUserApprovalPromise(permController) + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + + const req = RPC_REQUESTS.eth_requestAccounts(ORIGINS.a) + const res = {} + + const pendingApproval = assert.doesNotReject( + aMiddleware(req, res), + 'should not reject permissions request' + ) + + await userApprovalPromise + + assert.equal( + permController.pendingApprovals.size, 1, + 'perm controller should have single pending approval', + ) + + const id = permController.pendingApprovals.keys().next().value + const approvedReq = PERMS.approvedRequest(id, PERMS.requests.eth_accounts()) + + await permController.approvePermissionsRequest(approvedReq, ACCOUNT_ARRAYS.a) + + // at this point, the permission should have been granted + const perms = permController.permissions.getPermissionsForDomain(ORIGINS.a) + + assert.equal( + perms.length, 1, + 'domain should have correct number of permissions' + ) + + validatePermission( + perms[0], + PERM_NAMES.eth_accounts, + ORIGINS.a, + [CAVEATS.eth_accounts(ACCOUNT_ARRAYS.a)] + ) + + await pendingApproval + + // we should also see the accounts on the response + assert.ok( + res.result && !res.error, + 'response should have result and no error' + ) + + assert.deepEqual( + res.result, ACCOUNT_ARRAYS.a, + 'result should have correct accounts' + ) + + // we should also be able to get the accounts independently + const aAccounts = await permController.getAccounts(ORIGINS.a) + assert.deepEqual( + aAccounts, ACCOUNT_ARRAYS.a, 'origin should have have correct accounts' + ) + }) + + it('requests accounts for unpermitted origin, and rejects on user rejection', async function () { + + const userApprovalPromise = getUserApprovalPromise(permController) + + const aMiddleware = getPermissionsMiddleware(permController, ORIGINS.a) + + const req = RPC_REQUESTS.eth_requestAccounts(ORIGINS.a) + const res = {} + + const expectedError = ERRORS.rejectPermissionsRequest.rejection() + + const requestRejection = assert.rejects( + aMiddleware(req, res), + expectedError, + 'request should be rejected with correct error', + ) + + await userApprovalPromise + + assert.equal( + permController.pendingApprovals.size, 1, + 'perm controller should have single pending approval', + ) + + const id = permController.pendingApprovals.keys().next().value + + await permController.rejectPermissionsRequest(id) + await requestRejection + + assert.ok( + ( + !res.result && res.error && + res.error.message === expectedError.message + ), + 'response should have expected error and no result' + ) + + const aAccounts = await permController.getAccounts(ORIGINS.a) + assert.deepEqual( + aAccounts, [], 'origin should have have correct accounts' + ) + }) + + it('directly returns accounts for permitted domain', async function () { + + const cMiddleware = getPermissionsMiddleware(permController, ORIGINS.c) + + grantPermissions( + permController, ORIGINS.c, + PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.c) + ) + + const req = RPC_REQUESTS.eth_requestAccounts(ORIGINS.c) + const res = {} + + await assert.doesNotReject( + cMiddleware(req, res), + 'should not reject' + ) + + assert.ok( + res.result && !res.error, + 'response should have result and no error' + ) + assert.deepEqual( + res.result, ACCOUNT_ARRAYS.c, + 'response should have correct result' + ) + }) + }) + + describe('wallet_sendDomainMetadata', function () { + + let permController + + beforeEach(function () { + permController = initPermController() + }) + + it('records domain metadata', async function () { + + const name = 'BAZ' + + const cMiddleware = getPermissionsMiddleware(permController, ORIGINS.c) + + const req = RPC_REQUESTS.wallet_sendDomainMetadata(ORIGINS.c, name) + const res = {} + + await assert.doesNotReject( + cMiddleware(req, res), + 'should not reject' + ) + + assert.ok(res.result, 'result should be true') + + const metadataStore = permController.store.getState()[METADATA_STORE_KEY] + + assert.deepEqual( + metadataStore, + { [ORIGINS.c]: { name, extensionId: undefined } }, + 'metadata should have been added to store' + ) + }) + + it('records domain metadata and preserves extensionId', async function () { + + const extensionId = 'fooExtension' + + const name = 'BAZ' + + const cMiddleware = getPermissionsMiddleware(permController, ORIGINS.c, extensionId) + + const req = RPC_REQUESTS.wallet_sendDomainMetadata(ORIGINS.c, name) + const res = {} + + await assert.doesNotReject( + cMiddleware(req, res), + 'should not reject' + ) + + assert.ok(res.result, 'result should be true') + + const metadataStore = permController.store.getState()[METADATA_STORE_KEY] + + assert.deepEqual( + metadataStore, + { [ORIGINS.c]: { name, extensionId } }, + 'metadata should have been added to store' + ) + }) + + it('should not record domain metadata if no name', async function () { + + const name = null + + const cMiddleware = getPermissionsMiddleware(permController, ORIGINS.c) + + const req = RPC_REQUESTS.wallet_sendDomainMetadata(ORIGINS.c, name) + const res = {} + + await assert.doesNotReject( + cMiddleware(req, res), + 'should not reject' + ) + + assert.ok(res.result, 'result should be true') + + const metadataStore = permController.store.getState()[METADATA_STORE_KEY] + + assert.deepEqual( + metadataStore, {}, + 'metadata should not have been added to store' + ) + }) + + it('should not record domain metadata if no metadata', async function () { + + const cMiddleware = getPermissionsMiddleware(permController, ORIGINS.c) + + const req = RPC_REQUESTS.wallet_sendDomainMetadata(ORIGINS.c) + delete req.domainMetadata + const res = {} + + await assert.doesNotReject( + cMiddleware(req, res), + 'should not reject' + ) + + assert.ok(res.result, 'result should be true') + + const metadataStore = permController.store.getState()[METADATA_STORE_KEY] + + assert.deepEqual( + metadataStore, {}, + 'metadata should not have been added to store' + ) + }) + }) +}) diff --git a/test/unit/app/controllers/permissions/restricted-methods-test.js b/test/unit/app/controllers/permissions/restricted-methods-test.js new file mode 100644 index 000000000..26f477bed --- /dev/null +++ b/test/unit/app/controllers/permissions/restricted-methods-test.js @@ -0,0 +1,35 @@ +import { strict as assert } from 'assert' + +import getRestrictedMethods + from '../../../../../app/scripts/controllers/permissions/restrictedMethods' + +describe('restricted methods', function () { + + // this method is tested extensively in other permissions tests + describe('eth_accounts', function () { + + it('handles failure', async function () { + const restrictedMethods = getRestrictedMethods({ + getKeyringAccounts: async () => { + throw new Error('foo') + }, + }) + + const res = {} + restrictedMethods.eth_accounts.method(null, res, null, (err) => { + + const fooError = new Error('foo') + + assert.deepEqual( + err, fooError, + 'should end with expected error' + ) + + assert.deepEqual( + res, { error: fooError }, + 'response should have expected error and no result' + ) + }) + }) + }) +}) diff --git a/yarn.lock b/yarn.lock index 7ab987cef..8684f9c0d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -24638,10 +24638,10 @@ rn-host-detect@^1.1.5: resolved "https://registry.yarnpkg.com/rn-host-detect/-/rn-host-detect-1.1.5.tgz#fbecb982b73932f34529e97932b9a63e58d8deb6" integrity sha512-ufk2dFT3QeP9HyZ/xTuMtW27KnFy815CYitJMqQm+pgG3ZAtHBsrU8nXizNKkqXGy3bQmhEoloVbrfbvMJMqkg== -rpc-cap@^1.0.5: - version "1.0.5" - resolved "https://registry.yarnpkg.com/rpc-cap/-/rpc-cap-1.0.5.tgz#6f5ec41a7f0f85eb9aca8ccc7e618625109e6430" - integrity sha512-uZLjb609EbR+STeiLg27CVRWSKn/iKWeSkVYH5D1dj34ffpYcq6ByQBT7IN0AXozLUxmLulgpETB3gOS7fsH2w== +rpc-cap@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/rpc-cap/-/rpc-cap-2.0.0.tgz#575ff22417bdea9526292b8989c7b7af5e664bfa" + integrity sha512-P78tE+fIOxIkxcfZ8S1ge+Mt02AH4vMXdcFjr2uWLOdouDosgJOvJP5oROYx6qiUYbECuyKrsmETU7e+MA8vpg== dependencies: clone "^2.1.2" eth-json-rpc-errors "^2.0.2"