Update accounts permission history on accountsChanged event

app/scripts/controllers/permissions/enums.js

@@ -0,0 +1,72 @@
+
+export const WALLET_PREFIX = 'wallet_'
+
+export const HISTORY_STORE_KEY = 'permissionsHistory'
+
+export const LOG_STORE_KEY = 'permissionsLog'
+
+export const METADATA_STORE_KEY = 'domainMetadata'
+
+export const CAVEAT_NAMES = {
+  exposedAccounts: 'exposedAccounts',
+}
+
+export const NOTIFICATION_NAMES = {
+  accountsChanged: 'wallet_accountsChanged',
+}
+
+export const LOG_IGNORE_METHODS = [
+  'wallet_sendDomainMetadata',
+]
+
+export const SAFE_METHODS = [
+  'web3_sha3',
+  'net_listening',
+  'net_peerCount',
+  'net_version',
+  'eth_blockNumber',
+  'eth_call',
+  'eth_chainId',
+  'eth_coinbase',
+  'eth_estimateGas',
+  'eth_gasPrice',
+  'eth_getBalance',
+  'eth_getBlockByHash',
+  'eth_getBlockByNumber',
+  'eth_getBlockTransactionCountByHash',
+  'eth_getBlockTransactionCountByNumber',
+  'eth_getCode',
+  'eth_getFilterChanges',
+  'eth_getFilterLogs',
+  'eth_getLogs',
+  'eth_getStorageAt',
+  'eth_getTransactionByBlockHashAndIndex',
+  'eth_getTransactionByBlockNumberAndIndex',
+  'eth_getTransactionByHash',
+  'eth_getTransactionCount',
+  'eth_getTransactionReceipt',
+  'eth_getUncleByBlockHashAndIndex',
+  'eth_getUncleByBlockNumberAndIndex',
+  'eth_getUncleCountByBlockHash',
+  'eth_getUncleCountByBlockNumber',
+  'eth_getWork',
+  'eth_hashrate',
+  'eth_mining',
+  'eth_newBlockFilter',
+  'eth_newFilter',
+  'eth_newPendingTransactionFilter',
+  'eth_protocolVersion',
+  'eth_sendRawTransaction',
+  'eth_sendTransaction',
+  'eth_sign',
+  'personal_sign',
+  'eth_signTypedData',
+  'eth_signTypedData_v1',
+  'eth_signTypedData_v3',
+  'eth_submitHashrate',
+  'eth_submitWork',
+  'eth_syncing',
+  'eth_uninstallFilter',
+  'metamask_watchAsset',
+  'wallet_watchAsset',
+]

app/scripts/controllers/permissions/index.js

@@ -4,42 +4,46 @@ import ObservableStore from 'obs-store'
 import log from 'loglevel'
 import { CapabilitiesController as RpcCap } from 'rpc-cap'
 import { ethErrors } from 'eth-json-rpc-errors'
+
 import getRestrictedMethods from './restrictedMethods'
 import createMethodMiddleware from './methodMiddleware'
-import createLoggerMiddleware from './loggerMiddleware'
+import PermissionsLogController from './permissionsLog'
 
 // Methods that do not require any permissions to use:
-import SAFE_METHODS from './permissions-safe-methods.json'
-
-// some constants
-const METADATA_STORE_KEY = 'domainMetadata'
-const LOG_STORE_KEY = 'permissionsLog'
-const HISTORY_STORE_KEY = 'permissionsHistory'
-const WALLET_METHOD_PREFIX = 'wallet_'
-const ACCOUNTS_CHANGED_NOTIFICATION = 'wallet_accountsChanged'
-
-export const CAVEAT_NAMES = {
-  exposedAccounts: 'exposedAccounts',
-}
+import {
+  SAFE_METHODS, // methods that do not require any permissions to use
+  WALLET_PREFIX,
+  METADATA_STORE_KEY,
+  LOG_STORE_KEY,
+  HISTORY_STORE_KEY,
+  CAVEAT_NAMES,
+  NOTIFICATION_NAMES,
+} from './enums'
 
 export class PermissionsController {
 
   constructor (
     {
-      platform, notifyDomain, notifyAllDomains, keyringController,
+      platform, notifyDomain, notifyAllDomains, getKeyringAccounts,
     } = {},
     restoredPermissions = {},
     restoredState = {}) {
+
     this.store = new ObservableStore({
       [METADATA_STORE_KEY]: restoredState[METADATA_STORE_KEY] || {},
       [LOG_STORE_KEY]: restoredState[LOG_STORE_KEY] || [],
       [HISTORY_STORE_KEY]: restoredState[HISTORY_STORE_KEY] || {},
     })
-    this.notifyDomain = notifyDomain
+
+    this._notifyDomain = notifyDomain
     this.notifyAllDomains = notifyAllDomains
-    this.keyringController = keyringController
+    this.getKeyringAccounts = getKeyringAccounts
     this._platform = platform
     this._restrictedMethods = getRestrictedMethods(this)
+    this.permissionsLogController = new PermissionsLogController({
+      restrictedMethods: Object.keys(this._restrictedMethods),
+      store: this.store,
+    })
     this._initializePermissions(restoredPermissions)
   }
 
@@ -56,14 +60,7 @@ export class PermissionsController {
 
     const engine = new JsonRpcEngine()
 
-    engine.push(createLoggerMiddleware({
-      walletPrefix: WALLET_METHOD_PREFIX,
-      restrictedMethods: Object.keys(this._restrictedMethods),
-      ignoreMethods: [ 'wallet_sendDomainMetadata' ],
-      store: this.store,
-      logStoreKey: LOG_STORE_KEY,
-      historyStoreKey: HISTORY_STORE_KEY,
-    }))
+    engine.push(this.permissionsLogController.createMiddleware())
 
     engine.push(createMethodMiddleware({
       store: this.store,
@@ -107,7 +104,7 @@ export class PermissionsController {
   }
 
   /**
-   * Submits a permissions request to rpc-cap. Internal use only.
+   * Submits a permissions request to rpc-cap. Internal, background use only.
    *
    * @param {string} origin - The origin string.
    * @param {IRequestedPermissions} permissions - The requested permissions.
@@ -222,22 +219,26 @@ export class PermissionsController {
   }
 
   /**
-   * Update the accounts exposed to the given origin.
+   * Update the accounts exposed to the given origin. Changes the eth_accounts
+   * permissions and emits accountsChanged.
+   * At least one account must be exposed. If no accounts are to be exposed, the
+   * eth_accounts permissions should be removed completely.
+   *
    * Throws error if the update fails.
    *
    * @param {string} origin - The origin to change the exposed accounts for.
    * @param {string[]} accounts - The new account(s) to expose.
    */
-  async updateExposedAccounts (origin, accounts) {
+  async updatePermittedAccounts (origin, accounts) {
 
-    await this.validateExposedAccounts(accounts)
+    await this.validatePermittedAccounts(accounts)
 
     this.permissions.updateCaveatFor(
       origin, 'eth_accounts', CAVEAT_NAMES.exposedAccounts, accounts
     )
 
     this.notifyDomain(origin, {
-      method: ACCOUNTS_CHANGED_NOTIFICATION,
+      method: NOTIFICATION_NAMES.accountsChanged,
       result: accounts,
     })
   }
@@ -255,7 +256,7 @@ export class PermissionsController {
 
     if (ethAccounts) {
 
-      await this.validateExposedAccounts(accounts)
+      await this.validatePermittedAccounts(accounts)
 
       if (!ethAccounts.caveats) {
         ethAccounts.caveats = []
@@ -282,21 +283,44 @@ export class PermissionsController {
    *
    * @param {string[]} accounts - An array of addresses.
    */
-  async validateExposedAccounts (accounts) {
+  async validatePermittedAccounts (accounts) {
 
     if (!Array.isArray(accounts) || accounts.length === 0) {
       throw new Error('Must provide non-empty array of account(s).')
     }
 
     // assert accounts exist
-    const allAccounts = await this.keyringController.getAccounts()
+    const allAccounts = await this.getKeyringAccounts()
     accounts.forEach(acc => {
       if (!allAccounts.includes(acc)) {
         throw new Error(`Unknown account: ${acc}`)
       }
     })
   }
 
+  notifyDomain (origin, payload) {
+
+    // if the accounts changed from the perspective of the dapp,
+    // update "last seen" time for the origin and account(s)
+    // exception: no accounts -> no times to update
+    if (
+      payload.method === NOTIFICATION_NAMES.accountsChanged &&
+      Array.isArray(payload.result)
+    ) {
+      this.permissionsLogController.updateAccountsHistory(
+        origin, payload.result
+      )
+    }
+
+    this._notifyDomain(origin, payload)
+
+    // NOTE:
+    // we don't check for accounts changing in the notifyAllDomains case,
+    // because the log only records when accounts were last seen,
+    // and the accounts only change for all domains at once when permissions
+    // are removed
+  }
+
   /**
    * Removes the given permissions for the given domain.
    * @param {Object} domains { origin: [permissions] }
@@ -312,7 +336,7 @@ export class PermissionsController {
           if (methodName === 'eth_accounts') {
             this.notifyDomain(
               origin,
-              { method: ACCOUNTS_CHANGED_NOTIFICATION, result: [] }
+              { method: NOTIFICATION_NAMES.accountsChanged, result: [] }
             )
           }
 
@@ -322,13 +346,41 @@ export class PermissionsController {
     })
   }
 
+  /**
+   * When a new account is selected in the UI for 'origin', emit accountsChanged
+   * to 'origin' if the selected account is permitted.
+   * @param {string} origin - The origin.
+   * @param {string} account - The newly selected account's address.
+   */
+  async handleNewAccountSelected (origin, account) {
+
+    const permittedAccounts = await this.getAccounts(origin)
+
+    // 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[0] === account
+    ) {
+      return
+    }
+
+    const newPermittedAccounts = [account].concat(
+      permittedAccounts.filter(_account => _account !== account)
+    )
+
+    // update permitted accounts to ensure that accounts are returned
+    // in the same order every time
+    this.updatePermittedAccounts(origin, newPermittedAccounts)
+  }
+
   /**
    * Removes all known domains and their related permissions.
    */
   clearPermissions () {
     this.permissions.clearDomains()
     this.notifyAllDomains({
-      method: ACCOUNTS_CHANGED_NOTIFICATION,
+      method: NOTIFICATION_NAMES.accountsChanged,
       result: [],
     })
   }
@@ -352,7 +404,7 @@ export class PermissionsController {
       safeMethods: SAFE_METHODS,
 
       // optional prefix for internal methods
-      methodPrefix: WALLET_METHOD_PREFIX,
+      methodPrefix: WALLET_PREFIX,
 
       restrictedMethods: this._restrictedMethods,
 
@@ -379,5 +431,5 @@ export class PermissionsController {
 }
 
 export function addInternalMethodPrefix (method) {
-  return WALLET_METHOD_PREFIX + method
+  return WALLET_PREFIX + method
 }