Merge branch 'master' of github.com:MetaMask/metamask-extension into i3076-UseStorageLocalInstead

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Changelog
 
 ## Current Master
+- Fix flashing to Log in screen after logging in or restoring from seed phrase.
 
 - Fix bug that could cause MetaMask to lose all of its local data.
 ## 4.2.0 Tue Mar 06 2018

app/scripts/lib/seed-phrase-verifier.js

@@ -0,0 +1,48 @@
+const KeyringController = require('eth-keyring-controller')
+
+const seedPhraseVerifier = {
+
+  // Verifies if the seed words can restore the accounts.
+  //
+  // The seed words can recreate the primary keyring and the accounts belonging to it.
+  // The created accounts in the primary keyring are always the same.
+  // The keyring always creates the accounts in the same sequence.
+  verifyAccounts (createdAccounts, seedWords) {
+
+    return new Promise((resolve, reject) => {
+
+      if (!createdAccounts || createdAccounts.length < 1) {
+        return reject(new Error('No created accounts defined.'))
+      }
+
+      const keyringController = new KeyringController({})
+      const Keyring = keyringController.getKeyringClassForType('HD Key Tree')
+      const opts = {
+        mnemonic: seedWords,
+        numberOfAccounts: createdAccounts.length,
+      }
+
+      const keyring = new Keyring(opts)
+      keyring.getAccounts()
+        .then((restoredAccounts) => {
+
+          log.debug('Created accounts: ' + JSON.stringify(createdAccounts))
+          log.debug('Restored accounts: ' + JSON.stringify(restoredAccounts))
+
+          if (restoredAccounts.length !== createdAccounts.length) {
+            // this should not happen...
+            return reject(new Error('Wrong number of accounts'))
+          }
+
+          for (let i = 0; i < restoredAccounts.length; i++) {
+            if (restoredAccounts[i].toLowerCase() !== createdAccounts[i].toLowerCase()) {
+              return reject(new Error('Not identical accounts! Original: ' + createdAccounts[i] + ', Restored: ' + restoredAccounts[i]))
+            }
+          }
+          return resolve()
+        })
+    })
+  },
+}
+
+module.exports = seedPhraseVerifier

app/scripts/metamask-controller.js

@@ -37,6 +37,7 @@ const version = require('../manifest.json').version
 const BN = require('ethereumjs-util').BN
 const GWEI_BN = new BN('1000000000')
 const percentile = require('percentile')
+const seedPhraseVerifier = require('./lib/seed-phrase-verifier')
 
 module.exports = class MetamaskController extends EventEmitter {
 
@@ -344,6 +345,7 @@ module.exports = class MetamaskController extends EventEmitter {
       // primary HD keyring management
       addNewAccount: nodeify(this.addNewAccount, this),
       placeSeedWords: this.placeSeedWords.bind(this),
+      verifySeedPhrase: nodeify(this.verifySeedPhrase, this),
       clearSeedWordCache: this.clearSeedWordCache.bind(this),
       resetAccount: this.resetAccount.bind(this),
       importAccountWithStrategy: this.importAccountWithStrategy.bind(this),
@@ -565,14 +567,18 @@ module.exports = class MetamaskController extends EventEmitter {
   // Opinionated Keyring Management
   //
 
-  async addNewAccount (cb) {
+  async addNewAccount () {
     const primaryKeyring = this.keyringController.getKeyringsByType('HD Key Tree')[0]
-    if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found'))
+    if (!primaryKeyring) {
+      throw new Error('MetamaskController - No HD Key Tree found')
+    }
     const keyringController = this.keyringController
     const oldAccounts = await keyringController.getAccounts()
     const keyState = await keyringController.addNewAccount(primaryKeyring)
     const newAccounts = await keyringController.getAccounts()
 
+    await this.verifySeedPhrase()
+
     newAccounts.forEach((address) => {
       if (!oldAccounts.includes(address)) {
         this.preferencesController.setSelectedAddress(address)
@@ -587,14 +593,43 @@ module.exports = class MetamaskController extends EventEmitter {
   // Used when creating a first vault, to allow confirmation.
   // Also used when revealing the seed words in the confirmation view.
   placeSeedWords (cb) {
+
+    this.verifySeedPhrase()
+      .then((seedWords) => {
+        this.configManager.setSeedWords(seedWords)
+        return cb(null, seedWords)
+      })
+      .catch((err) => {
+        return cb(err)
+      })
+  }
+
+  // Verifies the current vault's seed words if they can restore the
+  // accounts belonging to the current vault.
+  //
+  // Called when the first account is created and on unlocking the vault.
+  async verifySeedPhrase () {
+
     const primaryKeyring = this.keyringController.getKeyringsByType('HD Key Tree')[0]
-    if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found'))
-    primaryKeyring.serialize()
-    .then((serialized) => {
-      const seedWords = serialized.mnemonic
-      this.configManager.setSeedWords(seedWords)
-      cb(null, seedWords)
-    })
+    if (!primaryKeyring) {
+      throw new Error('MetamaskController - No HD Key Tree found')
+    }
+
+    const serialized = await primaryKeyring.serialize()
+    const seedWords = serialized.mnemonic
+
+    const accounts = await primaryKeyring.getAccounts()
+    if (accounts.length < 1) {
+      throw new Error('MetamaskController - No accounts found')
+    }
+
+    try {
+      await seedPhraseVerifier.verifyAccounts(accounts, seedWords)
+      return seedWords
+    } catch (err) {
+      log.error(err.message)
+      throw err
+    }
   }
 
   // ClearSeedWordCache

test/unit/seed-phrase-verifier-test.js

@@ -0,0 +1,133 @@
+const assert = require('assert')
+const clone = require('clone')
+const KeyringController = require('eth-keyring-controller')
+const firstTimeState = require('../../app/scripts/first-time-state')
+const seedPhraseVerifier = require('../../app/scripts/lib/seed-phrase-verifier')
+const mockEncryptor = require('../lib/mock-encryptor')
+
+describe('SeedPhraseVerifier', function () {
+
+  describe('verifyAccounts', function () {
+
+    let password = 'passw0rd1'
+    let hdKeyTree = 'HD Key Tree'
+
+    let keyringController
+    let vault
+    let primaryKeyring
+
+    beforeEach(async function () {
+      keyringController = new KeyringController({
+        initState: clone(firstTimeState),
+        encryptor: mockEncryptor,
+      })
+
+      assert(keyringController)
+
+      vault = await keyringController.createNewVaultAndKeychain(password)
+      primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0]
+    })
+
+    it('should be able to verify created account with seed words', async function () {
+
+      let createdAccounts = await primaryKeyring.getAccounts()
+      assert.equal(createdAccounts.length, 1)
+
+      let serialized = await primaryKeyring.serialize()
+      let seedWords = serialized.mnemonic
+      assert.notEqual(seedWords.length, 0)
+    
+      let result = await seedPhraseVerifier.verifyAccounts(createdAccounts, seedWords)
+    })
+
+    it('should be able to verify created account (upper case) with seed words', async function () {
+
+      let createdAccounts = await primaryKeyring.getAccounts()
+      assert.equal(createdAccounts.length, 1)
+
+      let upperCaseAccounts = [createdAccounts[0].toUpperCase()]
+
+      let serialized = await primaryKeyring.serialize()
+      let seedWords = serialized.mnemonic
+      assert.notEqual(seedWords.length, 0)
+    
+      let result = await seedPhraseVerifier.verifyAccounts(upperCaseAccounts, seedWords)
+    })
+
+    it('should be able to verify created account (lower case) with seed words', async function () {
+
+      let createdAccounts = await primaryKeyring.getAccounts()
+      assert.equal(createdAccounts.length, 1)
+      let lowerCaseAccounts = [createdAccounts[0].toLowerCase()]
+
+      let serialized = await primaryKeyring.serialize()
+      let seedWords = serialized.mnemonic
+      assert.notEqual(seedWords.length, 0)
+    
+      let result = await seedPhraseVerifier.verifyAccounts(lowerCaseAccounts, seedWords)
+    })
+
+    it('should return error with good but different seed words', async function () {
+
+      let createdAccounts = await primaryKeyring.getAccounts()
+      assert.equal(createdAccounts.length, 1)
+
+      let serialized = await primaryKeyring.serialize()
+      let seedWords = 'debris dizzy just program just float decrease vacant alarm reduce speak stadium'
+      
+      try { 
+        let result = await seedPhraseVerifier.verifyAccounts(createdAccounts, seedWords)
+        assert.fail("Should reject")
+      } catch (err) {
+        assert.ok(err.message.indexOf('Not identical accounts!') >= 0, 'Wrong error message')
+      }
+    })
+
+    it('should return error with undefined existing accounts', async function () {
+
+      let createdAccounts = await primaryKeyring.getAccounts()
+      assert.equal(createdAccounts.length, 1)
+
+      let serialized = await primaryKeyring.serialize()
+      let seedWords = 'debris dizzy just program just float decrease vacant alarm reduce speak stadium'
+
+      try { 
+        let result = await seedPhraseVerifier.verifyAccounts(undefined, seedWords)
+        assert.fail("Should reject")
+      } catch (err) {
+        assert.equal(err.message, 'No created accounts defined.')
+      }
+    })
+
+    it('should return error with empty accounts array', async function () {
+
+      let createdAccounts = await primaryKeyring.getAccounts()
+      assert.equal(createdAccounts.length, 1)
+
+      let serialized = await primaryKeyring.serialize()
+      let seedWords = 'debris dizzy just program just float decrease vacant alarm reduce speak stadium'
+
+      try { 
+        let result = await seedPhraseVerifier.verifyAccounts([], seedWords)
+        assert.fail("Should reject")
+      } catch (err) {
+        assert.equal(err.message, 'No created accounts defined.')
+      }
+    })
+
+    it('should be able to verify more than one created account with seed words', async function () {
+
+      const keyState = await keyringController.addNewAccount(primaryKeyring)
+      const keyState2 = await keyringController.addNewAccount(primaryKeyring)
+
+      let createdAccounts = await primaryKeyring.getAccounts()
+      assert.equal(createdAccounts.length, 3)
+
+      let serialized = await primaryKeyring.serialize()
+      let seedWords = serialized.mnemonic
+      assert.notEqual(seedWords.length, 0)
+    
+      let result = await seedPhraseVerifier.verifyAccounts(createdAccounts, seedWords)
+    })
+  })
+})

ui/app/actions.js

@@ -284,20 +284,43 @@ function goHome () {
 // async actions
 
 function tryUnlockMetamask (password) {
-  return (dispatch) => {
+  return dispatch => {
     dispatch(actions.showLoadingIndication())
     dispatch(actions.unlockInProgress())
     log.debug(`background.submitPassword`)
-    background.submitPassword(password, (err) => {
-      dispatch(actions.hideLoadingIndication())
-      if (err) {
-        dispatch(actions.unlockFailed(err.message))
-      } else {
-        dispatch(actions.unlockSucceeded())
-        dispatch(actions.transitionForward())
-        forceUpdateMetamaskState(dispatch)
-      }
+
+    return new Promise((resolve, reject) => {
+      background.submitPassword(password, error => {
+        if (error) {
+          return reject(error)
+        }
+
+        resolve()
+      })
     })
+      .then(() => {
+        dispatch(actions.unlockSucceeded())
+        return forceUpdateMetamaskState(dispatch)
+      })
+      .then(() => {
+        return new Promise((resolve, reject) => {
+          background.verifySeedPhrase(err => {
+            if (err) {
+              dispatch(actions.displayWarning(err.message))
+            }
+
+            resolve()
+          })
+        })
+      })
+      .then(() => {
+        dispatch(actions.transitionForward())
+        dispatch(actions.hideLoadingIndication())
+      })
+      .catch(err => {
+        dispatch(actions.unlockFailed(err.message))
+        dispatch(actions.hideLoadingIndication())
+      })
   }
 }
 
@@ -339,46 +362,53 @@ function createNewVaultAndRestore (password, seed) {
     log.debug(`background.createNewVaultAndRestore`)
 
     return new Promise((resolve, reject) => {
-      background.createNewVaultAndRestore(password, seed, (err) => {
-
-        dispatch(actions.hideLoadingIndication())
-
+      background.createNewVaultAndRestore(password, seed, err => {
         if (err) {
-          dispatch(actions.displayWarning(err.message))
           return reject(err)
         }
 
-        dispatch(actions.showAccountsPage())
         resolve()
       })
     })
+      .then(() => dispatch(actions.unMarkPasswordForgotten()))
+      .then(() => {
+        dispatch(actions.showAccountsPage())
+        dispatch(actions.hideLoadingIndication())
+      })
+      .catch(err => {
+        dispatch(actions.displayWarning(err.message))
+        dispatch(actions.hideLoadingIndication())
+      })
   }
 }
 
 function createNewVaultAndKeychain (password) {
-  return (dispatch) => {
+  return dispatch => {
     dispatch(actions.showLoadingIndication())
     log.debug(`background.createNewVaultAndKeychain`)
 
     return new Promise((resolve, reject) => {
-      background.createNewVaultAndKeychain(password, (err) => {
+      background.createNewVaultAndKeychain(password, err => {
         if (err) {
           dispatch(actions.displayWarning(err.message))
           return reject(err)
         }
+
         log.debug(`background.placeSeedWords`)
+
         background.placeSeedWords((err) => {
           if (err) {
             dispatch(actions.displayWarning(err.message))
             return reject(err)
           }
-          dispatch(actions.hideLoadingIndication())
-          forceUpdateMetamaskState(dispatch)
+
           resolve()
         })
       })
     })
-
+      .then(() => forceUpdateMetamaskState(dispatch))
+      .then(() => dispatch(actions.hideLoadingIndication()))
+      .catch(() => dispatch(actions.hideLoadingIndication()))
   }
 }
 
@@ -389,18 +419,27 @@ function revealSeedConfirmation () {
 }
 
 function requestRevealSeed (password) {
-  return (dispatch) => {
+  return dispatch => {
     dispatch(actions.showLoadingIndication())
     log.debug(`background.submitPassword`)
-    background.submitPassword(password, (err) => {
-      if (err) {
-        return dispatch(actions.displayWarning(err.message))
-      }
-      log.debug(`background.placeSeedWords`)
-      background.placeSeedWords((err, result) => {
-        if (err) return dispatch(actions.displayWarning(err.message))
-        dispatch(actions.hideLoadingIndication())
-        dispatch(actions.showNewVaultSeed(result))
+    return new Promise((resolve, reject) => {
+      background.submitPassword(password, err => {
+        if (err) {
+          dispatch(actions.displayWarning(err.message))
+          return reject(err)
+        }
+
+        log.debug(`background.placeSeedWords`)
+        background.placeSeedWords((err, result) => {
+          if (err) {
+            dispatch(actions.displayWarning(err.message))
+            return reject(err)
+          }
+
+          dispatch(actions.showNewVaultSeed(result))
+          dispatch(actions.hideLoadingIndication())
+          resolve()
+        })
       })
     })
   }
@@ -851,11 +890,14 @@ function markPasswordForgotten () {
 }
 
 function unMarkPasswordForgotten () {
-  return (dispatch) => {
-    return background.unMarkPasswordForgotten(() => {
-      dispatch(actions.forgotPassword(false))
-      forceUpdateMetamaskState(dispatch)
+  return dispatch => {
+    return new Promise(resolve => {
+      background.unMarkPasswordForgotten(() => {
+        dispatch(actions.forgotPassword(false))
+        resolve()
+      })
     })
+      .then(() => forceUpdateMetamaskState(dispatch))
   }
 }
 
@@ -1704,11 +1746,16 @@ function callBackgroundThenUpdate (method, ...args) {
 
 function forceUpdateMetamaskState (dispatch) {
   log.debug(`background.getState`)
-  background.getState((err, newState) => {
-    if (err) {
-      return dispatch(actions.displayWarning(err.message))
-    }
-    dispatch(actions.updateMetamaskState(newState))
+  return new Promise((resolve, reject) => {
+    background.getState((err, newState) => {
+      if (err) {
+        dispatch(actions.displayWarning(err.message))
+        return reject(err)
+      }
+
+      dispatch(actions.updateMetamaskState(newState))
+      resolve()
+    })
   })
 }
 

ui/app/keychains/hd/restore-vault.js

@@ -107,12 +107,15 @@ RestoreVaultScreen.prototype.render = function () {
 }
 
 RestoreVaultScreen.prototype.showInitializeMenu = function () {
-  this.props.dispatch(actions.unMarkPasswordForgotten())
-  if (this.props.forgottenPassword) {
-    this.props.dispatch(actions.backToUnlockView())
-  } else {
-    this.props.dispatch(actions.showInitializeMenu())
-  }
+  const { dispatch, forgottenPassword } = this.props
+  dispatch(actions.unMarkPasswordForgotten())
+    .then(() => {
+      if (forgottenPassword) {
+        dispatch(actions.backToUnlockView())
+      } else {
+        dispatch(actions.showInitializeMenu())
+      }
+    })
 }
 
 RestoreVaultScreen.prototype.createOnEnter = function (event) {
@@ -150,11 +153,5 @@ RestoreVaultScreen.prototype.createNewVaultAndRestore = function () {
   this.warning = null
   this.props.dispatch(actions.displayWarning(this.warning))
   this.props.dispatch(actions.createNewVaultAndRestore(password, seed))
-  .then(() => {
-    this.props.dispatch(actions.unMarkPasswordForgotten())
-  })
-  .catch((err) => {
-    log.error(err.message)
-  })
-
+    .catch(err => log.error(err.message))
 }

ui/app/reducers/app.js

@@ -138,14 +138,18 @@ function reduceApp (state, action) {
       })
 
     case actions.FORGOT_PASSWORD:
-      return extend(appState, {
-        currentView: {
-          name: action.value ? 'restoreVault' : 'accountDetail',
-        },
-        transForward: false,
+      const newState = extend(appState, {
         forgottenPassword: action.value,
       })
 
+      if (action.value) {
+        newState.currentView = {
+          name: 'restoreVault',
+        }
+      }
+
+      return newState
+
     case actions.SHOW_INIT_MENU:
       return extend(appState, {
         currentView: defaultView,

ui/app/reducers/metamask.js

@@ -130,8 +130,6 @@ function reduceMetamask (state, action) {
 
     case actions.SHOW_NEW_VAULT_SEED:
       return extend(metamaskState, {
-        isUnlocked: true,
-        isInitialized: false,
         isRevealingSeedWords: true,
         seedWords: action.value,
       })