From 1880cda9b95f3274d56e1f4abc513d1d7ddd59c2 Mon Sep 17 00:00:00 2001
From: Dan Finlay <dan@danfinlay.com>
Date: Wed, 30 Nov 2016 19:34:17 -0800
Subject: Fix vault encrypting & unlocking bug

This is only a bug in dev, but was committed yesterday.

Sometimes the `encrypt` method was being passed values other than the password as the encryption key, leading to un-unlockable vaults.

To find this, and avoid it for all time hereafter, I added several more steps to our oft-neglected integration test suite, which now fully initializes a vault, locks it, and unlocks it again, to make sure all of those steps definitely work always.
---
 app/scripts/keyring-controller.js | 16 ++++++++++++----
 app/scripts/lib/config-manager.js |  5 +++--
 2 files changed, 15 insertions(+), 6 deletions(-)

(limited to 'app')

diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js
index ac9409dbb..9cfc3af97 100644
--- a/app/scripts/keyring-controller.js
+++ b/app/scripts/keyring-controller.js
@@ -167,7 +167,7 @@ module.exports = class KeyringController extends EventEmitter {
       this.configManager.setSelectedAccount(hexAccount)
       return this.setupAccounts(accounts)
     })
-    .then(this.persistAllKeyrings.bind(this))
+    .then(this.persistAllKeyrings.bind(this, password))
     .then(this.fullUpdate.bind(this))
   }
 
@@ -226,9 +226,11 @@ module.exports = class KeyringController extends EventEmitter {
     })
     .then((keyrings) => {
       this.keyrings = keyrings
-      return this.setupAccounts()
+      return this.fullUpdate()
+    })
+    .catch((reason) => {
+      return reason
     })
-    .then(this.fullUpdate.bind(this))
   }
 
   // Add New Keyring
@@ -250,6 +252,7 @@ module.exports = class KeyringController extends EventEmitter {
       this.keyrings.push(keyring)
       return this.setupAccounts(accounts)
     })
+    .then(() => { return this.password })
     .then(this.persistAllKeyrings.bind(this))
     .then(() => {
       return keyring
@@ -692,6 +695,9 @@ module.exports = class KeyringController extends EventEmitter {
   // Takes an account address and an iterator representing
   // the current number of named accounts.
   getBalanceAndNickname (account) {
+    if (!account) {
+      throw new Error('Problem loading account.')
+    }
     const address = normalize(account)
     this.ethStore.addAccount(address)
     return this.createNickname(address)
@@ -725,7 +731,9 @@ module.exports = class KeyringController extends EventEmitter {
   // encrypts that array with the provided `password`,
   // and persists that encrypted string to storage.
   persistAllKeyrings (password = this.password) {
-    this.password = password
+    if (typeof password === 'string') {
+      this.password = password
+    }
     return Promise.all(this.keyrings.map((keyring) => {
       return Promise.all([keyring.type, keyring.serialize()])
       .then((serializedKeyringArray) => {
diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js
index 8e63265d2..59cc2b63c 100644
--- a/app/scripts/lib/config-manager.js
+++ b/app/scripts/lib/config-manager.js
@@ -3,6 +3,7 @@ const MetamaskConfig = require('../config.js')
 const migrations = require('./migrations')
 const rp = require('request-promise')
 const ethUtil = require('ethereumjs-util')
+const normalize = require('./sig-util').normalize
 
 const TESTNET_RPC = MetamaskConfig.network.testnet
 const MAINNET_RPC = MetamaskConfig.network.mainnet
@@ -273,13 +274,13 @@ ConfigManager.prototype.getWalletNicknames = function () {
 }
 
 ConfigManager.prototype.nicknameForWallet = function (account) {
-  const address = ethUtil.addHexPrefix(account.toLowerCase())
+  const address = normalize(account)
   const nicknames = this.getWalletNicknames()
   return nicknames[address]
 }
 
 ConfigManager.prototype.setNicknameForWallet = function (account, nickname) {
-  const address = ethUtil.addHexPrefix(account.toLowerCase())
+  const address = normalize(account)
   const nicknames = this.getWalletNicknames()
   nicknames[address] = nickname
   var data = this.getData()
-- 
cgit