1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-12-22 17:33:23 +01:00

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.
This commit is contained in:
Dan Finlay 2016-11-30 19:34:17 -08:00
parent 049e351c9d
commit 1880cda9b9
6 changed files with 53 additions and 13 deletions

View File

@ -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) {
if (typeof password === 'string') {
this.password = password
}
return Promise.all(this.keyrings.map((keyring) => {
return Promise.all([keyring.type, keyring.serialize()])
.then((serializedKeyringArray) => {

View File

@ -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()

View File

@ -1,5 +1,7 @@
var encryptor = require('../../../app/scripts/lib/encryptor')
QUnit.module('encryptor')
QUnit.test('encryptor:serializeBufferForStorage', function (assert) {
assert.expect(1)
var buf = new Buffer(2)
@ -65,3 +67,5 @@ QUnit.test('encryptor:encrypt & decrypt with wrong password', function(assert) {
done()
})
})

View File

@ -1,5 +1,7 @@
const PASSWORD = 'password123'
QUnit.module('first time usage')
QUnit.test('agree to terms', function (assert) {
var done = assert.async()
let app

View File

@ -5,7 +5,11 @@ var actions = {
goHome: goHome,
// menu state
getNetworkStatus: 'getNetworkStatus',
// transition state
TRANSITION_FORWARD: 'TRANSITION_FORWARD',
TRANSITION_BACKWARD: 'TRANSITION_BACKWARD',
transitionForward,
transitionBackward,
// remote state
UPDATE_METAMASK_STATE: 'UPDATE_METAMASK_STATE',
updateMetamaskState: updateMetamaskState,
@ -166,16 +170,25 @@ function tryUnlockMetamask (password) {
if (err) {
dispatch(actions.unlockFailed(err.message))
} else {
let selectedAccount
try {
selectedAccount = newState.metamask.selectedAccount
} catch (e) {}
dispatch(actions.unlockMetamask(selectedAccount))
dispatch(actions.transitionForward())
dispatch(actions.updateMetamaskState(newState))
}
})
}
}
function transitionForward() {
return {
type: this.TRANSITION_FORWARD,
}
}
function transitionBackward() {
return {
type: this.TRANSITION_BACKWARD,
}
}
function confirmSeedWords () {
return (dispatch) => {
dispatch(actions.showLoadingIndication())

View File

@ -43,6 +43,18 @@ function reduceApp (state, action) {
switch (action.type) {
// transition methods
case actions.TRANSITION_FORWARD:
return extend(appState, {
transForward: true,
})
case actions.TRANSITION_BACKWARD:
return extend(appState, {
transForward: false,
})
// intialize
case actions.SHOW_CREATE_VAULT: