From 92453f8715b78c0e6e2cdb9b2e1cfe48c0b013ad Mon Sep 17 00:00:00 2001 From: Csaba Solya Date: Sat, 3 Mar 2018 00:32:57 +0100 Subject: seed phrase verifier --- app/scripts/lib/seed-phrase-verifier.js | 43 ++++++++++ app/scripts/metamask-controller.js | 20 ++++- test/unit/seed-phrase-verifier-test.js | 136 ++++++++++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 app/scripts/lib/seed-phrase-verifier.js create mode 100644 test/unit/seed-phrase-verifier-test.js diff --git a/app/scripts/lib/seed-phrase-verifier.js b/app/scripts/lib/seed-phrase-verifier.js new file mode 100644 index 000000000..9bea2910e --- /dev/null +++ b/app/scripts/lib/seed-phrase-verifier.js @@ -0,0 +1,43 @@ +const KeyringController = require('eth-keyring-controller') + +const seedPhraseVerifier = { + + verifyAccounts(createdAccounts, seedWords) { + + return new Promise((resolve, reject) => { + + if (!createdAccounts || createdAccounts.length < 1) { + return reject(new Error('No created accounts defined.')) + } + + let keyringController = new KeyringController({}) + let Keyring = keyringController.getKeyringClassForType('HD Key Tree') + let opts = { + mnemonic: seedWords, + numberOfAccounts: createdAccounts.length, + } + + let 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] !== createdAccounts[i]) { + return reject(new Error('Not identical accounts! Original: ' + createdAccounts[i] + ', Restored: ' + restoredAccounts[i])) + } + } + return resolve() + }) + }) + } +} + +module.exports = seedPhraseVerifier \ No newline at end of file diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index ad4e71792..89bcbd51b 100644 --- a/app/scripts/metamask-controller.js +++ b/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 { @@ -592,8 +593,23 @@ module.exports = class MetamaskController extends EventEmitter { primaryKeyring.serialize() .then((serialized) => { const seedWords = serialized.mnemonic - this.configManager.setSeedWords(seedWords) - cb(null, seedWords) + + primaryKeyring.getAccounts() + .then((accounts) => { + if (accounts.length < 1) { + return cb(new Error('MetamaskController - No accounts found')) + } + + seedPhraseVerifier.verifyAccounts(accounts, seedWords) + .then(() => { + this.configManager.setSeedWords(seedWords) + cb(null, seedWords) + }) + .catch((err) => { + log.error(err) + cb(err) + }) + }) }) } diff --git a/test/unit/seed-phrase-verifier-test.js b/test/unit/seed-phrase-verifier-test.js new file mode 100644 index 000000000..a7a463dd3 --- /dev/null +++ b/test/unit/seed-phrase-verifier-test.js @@ -0,0 +1,136 @@ +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 () { + + var password = 'passw0rd1' + let hdKeyTree = 'HD Key Tree' + + it('should be able to verify created account with seed words', async function () { + + let keyringController = new KeyringController({ + initState: clone(firstTimeState), + encryptor: mockEncryptor, + }) + assert(keyringController) + + let vault = await keyringController.createNewVaultAndKeychain(password) + let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] + + 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 return error with good but different seed words', async function () { + + let keyringController = new KeyringController({ + initState: clone(firstTimeState), + encryptor: mockEncryptor, + }) + assert(keyringController) + + let vault = await keyringController.createNewVaultAndKeychain(password) + let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] + + 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 keyringController = new KeyringController({ + initState: clone(firstTimeState), + encryptor: mockEncryptor, + }) + assert(keyringController) + + let vault = await keyringController.createNewVaultAndKeychain(password) + let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] + + 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 keyringController = new KeyringController({ + initState: clone(firstTimeState), + encryptor: mockEncryptor, + }) + assert(keyringController) + + let vault = await keyringController.createNewVaultAndKeychain(password) + let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] + + 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 () { + + let keyringController = new KeyringController({ + initState: clone(firstTimeState), + encryptor: mockEncryptor, + }) + assert(keyringController) + + let vault = await keyringController.createNewVaultAndKeychain(password) + + let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] + + 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) + }) + }) +}) -- cgit From 4bd7f1a37abcd09dc8816fc5b28ad41bc86b1aea Mon Sep 17 00:00:00 2001 From: Csaba Solya Date: Sat, 3 Mar 2018 00:40:40 +0100 Subject: fix lint issues --- app/scripts/lib/seed-phrase-verifier.js | 18 +++++++++--------- app/scripts/metamask-controller.js | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/scripts/lib/seed-phrase-verifier.js b/app/scripts/lib/seed-phrase-verifier.js index 9bea2910e..97a433fd8 100644 --- a/app/scripts/lib/seed-phrase-verifier.js +++ b/app/scripts/lib/seed-phrase-verifier.js @@ -2,7 +2,7 @@ const KeyringController = require('eth-keyring-controller') const seedPhraseVerifier = { - verifyAccounts(createdAccounts, seedWords) { + verifyAccounts (createdAccounts, seedWords) { return new Promise((resolve, reject) => { @@ -10,23 +10,23 @@ const seedPhraseVerifier = { return reject(new Error('No created accounts defined.')) } - let keyringController = new KeyringController({}) - let Keyring = keyringController.getKeyringClassForType('HD Key Tree') - let opts = { + const keyringController = new KeyringController({}) + const Keyring = keyringController.getKeyringClassForType('HD Key Tree') + const opts = { mnemonic: seedWords, numberOfAccounts: createdAccounts.length, } - let keyring = new Keyring(opts) + 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) { + if (restoredAccounts.length !== createdAccounts.length) { // this should not happen... - return reject(new Error("Wrong number of accounts")) + return reject(new Error('Wrong number of accounts')) } for (let i = 0; i < restoredAccounts.length; i++) { @@ -37,7 +37,7 @@ const seedPhraseVerifier = { return resolve() }) }) - } + }, } -module.exports = seedPhraseVerifier \ No newline at end of file +module.exports = seedPhraseVerifier diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 89bcbd51b..b9231aa3d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -599,9 +599,9 @@ module.exports = class MetamaskController extends EventEmitter { if (accounts.length < 1) { return cb(new Error('MetamaskController - No accounts found')) } - + seedPhraseVerifier.verifyAccounts(accounts, seedWords) - .then(() => { + .then(() => { this.configManager.setSeedWords(seedWords) cb(null, seedWords) }) -- cgit From 3e05b693dbf55ea7ecb791e8f31b7599a6b89ffd Mon Sep 17 00:00:00 2001 From: Csaba Solya Date: Sat, 3 Mar 2018 14:11:02 +0100 Subject: verify addresses regardless case --- app/scripts/lib/seed-phrase-verifier.js | 2 +- test/unit/seed-phrase-verifier-test.js | 44 +++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/app/scripts/lib/seed-phrase-verifier.js b/app/scripts/lib/seed-phrase-verifier.js index 97a433fd8..1f35c2c67 100644 --- a/app/scripts/lib/seed-phrase-verifier.js +++ b/app/scripts/lib/seed-phrase-verifier.js @@ -30,7 +30,7 @@ const seedPhraseVerifier = { } for (let i = 0; i < restoredAccounts.length; i++) { - if (restoredAccounts[i] !== createdAccounts[i]) { + if (restoredAccounts[i].toLowerCase() !== createdAccounts[i].toLowerCase()) { return reject(new Error('Not identical accounts! Original: ' + createdAccounts[i] + ', Restored: ' + restoredAccounts[i])) } } diff --git a/test/unit/seed-phrase-verifier-test.js b/test/unit/seed-phrase-verifier-test.js index a7a463dd3..3e9acfa82 100644 --- a/test/unit/seed-phrase-verifier-test.js +++ b/test/unit/seed-phrase-verifier-test.js @@ -33,6 +33,50 @@ describe('SeedPhraseVerifier', function () { let result = await seedPhraseVerifier.verifyAccounts(createdAccounts, seedWords) }) + it('should be able to verify created account (upper case) with seed words', async function () { + + let keyringController = new KeyringController({ + initState: clone(firstTimeState), + encryptor: mockEncryptor, + }) + assert(keyringController) + + let vault = await keyringController.createNewVaultAndKeychain(password) + let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] + + 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 keyringController = new KeyringController({ + initState: clone(firstTimeState), + encryptor: mockEncryptor, + }) + assert(keyringController) + + let vault = await keyringController.createNewVaultAndKeychain(password) + let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] + + 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 keyringController = new KeyringController({ -- cgit From 2b86d65d0c3266e8ddfe814abe1d1755fbf23fda Mon Sep 17 00:00:00 2001 From: Csaba Solya Date: Sat, 3 Mar 2018 22:08:10 +0100 Subject: verify seedwords on log in --- app/scripts/metamask-controller.js | 19 +++++++++++--- test/unit/seed-phrase-verifier-test.js | 48 ++++++---------------------------- ui/app/actions.js | 7 +++++ 3 files changed, 31 insertions(+), 43 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index b9231aa3d..df9adc248 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -345,6 +345,7 @@ module.exports = class MetamaskController extends EventEmitter { // primary HD keyring management addNewAccount: nodeify(this.addNewAccount, this), placeSeedWords: this.placeSeedWords.bind(this), + verifySeedPhrase: this.verifySeedPhrase.bind(this), clearSeedWordCache: this.clearSeedWordCache.bind(this), resetAccount: this.resetAccount.bind(this), importAccountWithStrategy: this.importAccountWithStrategy.bind(this), @@ -588,6 +589,19 @@ 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((err, seedWords) => { + + if (err) { + return cb(err) + } + this.configManager.setSeedWords(seedWords) + return cb(null, seedWords) + }) + } + + verifySeedPhrase (cb) { + const primaryKeyring = this.keyringController.getKeyringsByType('HD Key Tree')[0] if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found')) primaryKeyring.serialize() @@ -602,12 +616,11 @@ module.exports = class MetamaskController extends EventEmitter { seedPhraseVerifier.verifyAccounts(accounts, seedWords) .then(() => { - this.configManager.setSeedWords(seedWords) - cb(null, seedWords) + return cb(null, seedWords) }) .catch((err) => { log.error(err) - cb(err) + return cb(err) }) }) }) diff --git a/test/unit/seed-phrase-verifier-test.js b/test/unit/seed-phrase-verifier-test.js index 3e9acfa82..654fb5994 100644 --- a/test/unit/seed-phrase-verifier-test.js +++ b/test/unit/seed-phrase-verifier-test.js @@ -9,16 +9,20 @@ describe('SeedPhraseVerifier', function () { describe('verifyAccounts', function () { - var password = 'passw0rd1' + let password = 'passw0rd1' let hdKeyTree = 'HD Key Tree' - it('should be able to verify created account with seed words', async function () { - - let keyringController = new KeyringController({ + let keyringController + beforeEach(function () { + keyringController = new KeyringController({ initState: clone(firstTimeState), encryptor: mockEncryptor, }) + assert(keyringController) + }) + + it('should be able to verify created account with seed words', async function () { let vault = await keyringController.createNewVaultAndKeychain(password) let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] @@ -35,12 +39,6 @@ describe('SeedPhraseVerifier', function () { it('should be able to verify created account (upper case) with seed words', async function () { - let keyringController = new KeyringController({ - initState: clone(firstTimeState), - encryptor: mockEncryptor, - }) - assert(keyringController) - let vault = await keyringController.createNewVaultAndKeychain(password) let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] @@ -57,12 +55,6 @@ describe('SeedPhraseVerifier', function () { it('should be able to verify created account (lower case) with seed words', async function () { - let keyringController = new KeyringController({ - initState: clone(firstTimeState), - encryptor: mockEncryptor, - }) - assert(keyringController) - let vault = await keyringController.createNewVaultAndKeychain(password) let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] @@ -79,12 +71,6 @@ describe('SeedPhraseVerifier', function () { it('should return error with good but different seed words', async function () { - let keyringController = new KeyringController({ - initState: clone(firstTimeState), - encryptor: mockEncryptor, - }) - assert(keyringController) - let vault = await keyringController.createNewVaultAndKeychain(password) let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] @@ -104,12 +90,6 @@ describe('SeedPhraseVerifier', function () { it('should return error with undefined existing accounts', async function () { - let keyringController = new KeyringController({ - initState: clone(firstTimeState), - encryptor: mockEncryptor, - }) - assert(keyringController) - let vault = await keyringController.createNewVaultAndKeychain(password) let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] @@ -129,12 +109,6 @@ describe('SeedPhraseVerifier', function () { it('should return error with empty accounts array', async function () { - let keyringController = new KeyringController({ - initState: clone(firstTimeState), - encryptor: mockEncryptor, - }) - assert(keyringController) - let vault = await keyringController.createNewVaultAndKeychain(password) let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] @@ -154,12 +128,6 @@ describe('SeedPhraseVerifier', function () { it('should be able to verify more than one created account with seed words', async function () { - let keyringController = new KeyringController({ - initState: clone(firstTimeState), - encryptor: mockEncryptor, - }) - assert(keyringController) - let vault = await keyringController.createNewVaultAndKeychain(password) let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] diff --git a/ui/app/actions.js b/ui/app/actions.js index 64d5b67e0..9606841ae 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -296,6 +296,13 @@ function tryUnlockMetamask (password) { dispatch(actions.unlockSucceeded()) dispatch(actions.transitionForward()) forceUpdateMetamaskState(dispatch) + + background.verifySeedPhrase((err) => { + if (err) { + dispatch(actions.displayWarning(err.message)) + } + }) + } }) } -- cgit From f7d4a1080df6d1c8ea5f68f88b01caea065b5e92 Mon Sep 17 00:00:00 2001 From: Csaba Solya Date: Sun, 4 Mar 2018 08:47:46 +0100 Subject: add documentation --- app/scripts/lib/seed-phrase-verifier.js | 5 +++++ app/scripts/metamask-controller.js | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/app/scripts/lib/seed-phrase-verifier.js b/app/scripts/lib/seed-phrase-verifier.js index 1f35c2c67..9cea22029 100644 --- a/app/scripts/lib/seed-phrase-verifier.js +++ b/app/scripts/lib/seed-phrase-verifier.js @@ -2,6 +2,11 @@ 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) => { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index df9adc248..f523e3919 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -600,6 +600,10 @@ module.exports = class MetamaskController extends EventEmitter { }) } + // 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. verifySeedPhrase (cb) { const primaryKeyring = this.keyringController.getKeyringsByType('HD Key Tree')[0] -- cgit From 8fde208f0b1da39ccd63b5f256902786e73e9368 Mon Sep 17 00:00:00 2001 From: Csaba Solya Date: Sun, 4 Mar 2018 08:57:55 +0100 Subject: move more test code to beforeEach --- test/unit/seed-phrase-verifier-test.js | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/test/unit/seed-phrase-verifier-test.js b/test/unit/seed-phrase-verifier-test.js index 654fb5994..4e314806b 100644 --- a/test/unit/seed-phrase-verifier-test.js +++ b/test/unit/seed-phrase-verifier-test.js @@ -13,20 +13,23 @@ describe('SeedPhraseVerifier', function () { let hdKeyTree = 'HD Key Tree' let keyringController - beforeEach(function () { + 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 vault = await keyringController.createNewVaultAndKeychain(password) - let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] - let createdAccounts = await primaryKeyring.getAccounts() assert.equal(createdAccounts.length, 1) @@ -39,11 +42,9 @@ describe('SeedPhraseVerifier', function () { it('should be able to verify created account (upper case) with seed words', async function () { - let vault = await keyringController.createNewVaultAndKeychain(password) - let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] - let createdAccounts = await primaryKeyring.getAccounts() assert.equal(createdAccounts.length, 1) + let upperCaseAccounts = [createdAccounts[0].toUpperCase()] let serialized = await primaryKeyring.serialize() @@ -55,9 +56,6 @@ describe('SeedPhraseVerifier', function () { it('should be able to verify created account (lower case) with seed words', async function () { - let vault = await keyringController.createNewVaultAndKeychain(password) - let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] - let createdAccounts = await primaryKeyring.getAccounts() assert.equal(createdAccounts.length, 1) let lowerCaseAccounts = [createdAccounts[0].toLowerCase()] @@ -71,9 +69,6 @@ describe('SeedPhraseVerifier', function () { it('should return error with good but different seed words', async function () { - let vault = await keyringController.createNewVaultAndKeychain(password) - let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] - let createdAccounts = await primaryKeyring.getAccounts() assert.equal(createdAccounts.length, 1) @@ -90,9 +85,6 @@ describe('SeedPhraseVerifier', function () { it('should return error with undefined existing accounts', async function () { - let vault = await keyringController.createNewVaultAndKeychain(password) - let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] - let createdAccounts = await primaryKeyring.getAccounts() assert.equal(createdAccounts.length, 1) @@ -109,9 +101,6 @@ describe('SeedPhraseVerifier', function () { it('should return error with empty accounts array', async function () { - let vault = await keyringController.createNewVaultAndKeychain(password) - let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] - let createdAccounts = await primaryKeyring.getAccounts() assert.equal(createdAccounts.length, 1) @@ -128,10 +117,6 @@ describe('SeedPhraseVerifier', function () { it('should be able to verify more than one created account with seed words', async function () { - let vault = await keyringController.createNewVaultAndKeychain(password) - - let primaryKeyring = keyringController.getKeyringsByType(hdKeyTree)[0] - const keyState = await keyringController.addNewAccount(primaryKeyring) const keyState2 = await keyringController.addNewAccount(primaryKeyring) -- cgit From 59007a6c36055f9197ad83ccb1741fa186b85f53 Mon Sep 17 00:00:00 2001 From: Csaba Solya Date: Tue, 6 Mar 2018 15:56:27 +0100 Subject: modify verifySeedPhrase to async and call it from addNewAccount also --- app/scripts/metamask-controller.js | 66 ++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index f523e3919..0a5c1d36f 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -345,7 +345,7 @@ module.exports = class MetamaskController extends EventEmitter { // primary HD keyring management addNewAccount: nodeify(this.addNewAccount, this), placeSeedWords: this.placeSeedWords.bind(this), - verifySeedPhrase: this.verifySeedPhrase.bind(this), + verifySeedPhrase: nodeify(this.verifySeedPhrase, this), clearSeedWordCache: this.clearSeedWordCache.bind(this), resetAccount: this.resetAccount.bind(this), importAccountWithStrategy: this.importAccountWithStrategy.bind(this), @@ -567,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) @@ -590,44 +594,42 @@ module.exports = class MetamaskController extends EventEmitter { // Also used when revealing the seed words in the confirmation view. placeSeedWords (cb) { - this.verifySeedPhrase((err, seedWords) => { - - if (err) { + this.verifySeedPhrase() + .then((seedWords) => { + this.configManager.setSeedWords(seedWords) + return cb(null, seedWords) + }) + .catch((err) => { return cb(err) - } - this.configManager.setSeedWords(seedWords) - return cb(null, seedWords) - }) + }) } // 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. - verifySeedPhrase (cb) { + 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 - - primaryKeyring.getAccounts() - .then((accounts) => { - if (accounts.length < 1) { - return cb(new Error('MetamaskController - No accounts found')) - } - - seedPhraseVerifier.verifyAccounts(accounts, seedWords) - .then(() => { - return cb(null, seedWords) - }) - .catch((err) => { - log.error(err) - return cb(err) - }) - }) - }) + 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 -- cgit From cbb65cc4930ff1571a84ac526a29a76010d6003f Mon Sep 17 00:00:00 2001 From: Alexander Tseung Date: Thu, 8 Mar 2018 16:29:29 +0100 Subject: Fix flashing to Log in screen after logging in or restoring from seed phrase (#3466) --- CHANGELOG.md | 1 + ui/app/actions.js | 130 +++++++++++++++++++++++------------ ui/app/keychains/hd/restore-vault.js | 23 +++---- ui/app/reducers/app.js | 14 ++-- ui/app/reducers/metamask.js | 2 - yarn.lock | 74 +++++++------------- 6 files changed, 129 insertions(+), 115 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 823b89b45..177e08bdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Current Master +- Fix flashing to Log in screen after logging in or restoring from seed phrase. ## 4.2.0 Tue Mar 06 2018 diff --git a/ui/app/actions.js b/ui/app/actions.js index 3e26f333b..b56265edc 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -284,27 +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) - background.verifySeedPhrase((err) => { - if (err) { - dispatch(actions.displayWarning(err.message)) - } - }) + 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()) + }) } } @@ -346,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())) } } @@ -396,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() + }) }) }) } @@ -858,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)) } } @@ -1711,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() + }) }) } diff --git a/ui/app/keychains/hd/restore-vault.js b/ui/app/keychains/hd/restore-vault.js index a4ed137f9..d1761f17d 100644 --- a/ui/app/keychains/hd/restore-vault.js +++ b/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)) } diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js index 4dda839a2..74a0f9299 100644 --- a/ui/app/reducers/app.js +++ b/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, diff --git a/ui/app/reducers/metamask.js b/ui/app/reducers/metamask.js index cddcd0c1f..029d830ec 100644 --- a/ui/app/reducers/metamask.js +++ b/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, }) diff --git a/yarn.lock b/yarn.lock index 028ffa44e..cd7ebd844 100644 --- a/yarn.lock +++ b/yarn.lock @@ -221,12 +221,6 @@ ansi-colors@^1.0.1: dependencies: ansi-wrap "^0.1.0" -ansi-cyan@^0.1.1: - version "0.1.1" - resolved "https://registry.yarnpkg.com/ansi-cyan/-/ansi-cyan-0.1.1.tgz#538ae528af8982f28ae30d86f2f17456d2609873" - dependencies: - ansi-wrap "0.1.0" - ansi-escapes@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/ansi-escapes/-/ansi-escapes-3.0.0.tgz#ec3e8b4e9f8064fc02c3ac9b65f1c275bda8ef92" @@ -237,12 +231,6 @@ ansi-gray@^0.1.1: dependencies: ansi-wrap "0.1.0" -ansi-red@^0.1.1: - version "0.1.1" - resolved "https://registry.yarnpkg.com/ansi-red/-/ansi-red-0.1.1.tgz#8c638f9d1080800a353c9c28c8a81ca4705d946c" - dependencies: - ansi-wrap "0.1.0" - ansi-regex@^0.2.0, ansi-regex@^0.2.1: version "0.2.1" resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-0.2.1.tgz#0d8e946967a3d8143f93e24e298525fc1b2235f9" @@ -332,13 +320,6 @@ argparse@^1.0.7: dependencies: sprintf-js "~1.0.2" -arr-diff@^1.0.1: - version "1.1.0" - resolved "https://registry.yarnpkg.com/arr-diff/-/arr-diff-1.1.0.tgz#687c32758163588fef7de7b36fabe495eb1a399a" - dependencies: - arr-flatten "^1.0.1" - array-slice "^0.2.3" - arr-diff@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/arr-diff/-/arr-diff-2.0.0.tgz#8f3b827f955a8bd669697e4a4256ac3ceae356cf" @@ -365,10 +346,6 @@ arr-map@^2.0.0, arr-map@^2.0.2: dependencies: make-iterator "^1.0.0" -arr-union@^2.0.1: - version "2.1.0" - resolved "https://registry.yarnpkg.com/arr-union/-/arr-union-2.1.0.tgz#20f9eab5ec70f5c7d215b1077b1c39161d292c7d" - arr-union@^3.1.0: version "3.1.0" resolved "https://registry.yarnpkg.com/arr-union/-/arr-union-3.1.0.tgz#e39b09aea9def866a8f206e288af63919bae39c4" @@ -475,6 +452,10 @@ asap@~2.0.3: version "2.0.6" resolved "https://registry.yarnpkg.com/asap/-/asap-2.0.6.tgz#e50347611d7e690943208bbdafebcbc2fb866d46" +asmcrypto.js@0.22.0: + version "0.22.0" + resolved "https://registry.yarnpkg.com/asmcrypto.js/-/asmcrypto.js-0.22.0.tgz#38fc1440884d802c7bd37d1d23c2b26a5cd5d2d2" + asn1.js@^4.0.0: version "4.9.2" resolved "https://registry.yarnpkg.com/asn1.js/-/asn1.js-4.9.2.tgz#8117ef4f7ed87cd8f89044b5bff97ac243a16c9a" @@ -2432,6 +2413,10 @@ commander@~2.13.0: version "2.13.0" resolved "https://registry.yarnpkg.com/commander/-/commander-2.13.0.tgz#6964bca67685df7c1f1430c584f07d7597885b9c" +commander@~2.14.1: + version "2.14.1" + resolved "https://registry.yarnpkg.com/commander/-/commander-2.14.1.tgz#2235123e37af8ca3c65df45b026dbd357b01b9aa" + commondir@0.0.1: version "0.0.1" resolved "https://registry.yarnpkg.com/commondir/-/commondir-0.0.1.tgz#89f00fdcd51b519c578733fec563e6a6da7f5be2" @@ -4380,12 +4365,6 @@ express@^4.10.7, express@^4.15.5: utils-merge "1.0.1" vary "~1.1.2" -extend-shallow@^1.1.2: - version "1.1.4" - resolved "https://registry.yarnpkg.com/extend-shallow/-/extend-shallow-1.1.4.tgz#19d6bf94dfc09d76ba711f39b872d21ff4dd9071" - dependencies: - kind-of "^1.1.0" - extend-shallow@^2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/extend-shallow/-/extend-shallow-2.0.1.tgz#51af7d614ad9a9f610ea1bafbb989d6b1c56890f" @@ -5307,13 +5286,13 @@ gulp-stylelint@^4.0.0: stylelint "^8.0.0" through2 "^2.0.3" -gulp-uglify-es@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/gulp-uglify-es/-/gulp-uglify-es-1.0.0.tgz#80b2f8e2fa7211c1706c597f08bbf620c870e545" +gulp-uglify-es@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/gulp-uglify-es/-/gulp-uglify-es-1.0.1.tgz#9f991de31c646fb37fe589086ffd3f6e2f9e20f1" dependencies: o-stream "^0.2.2" - plugin-error "^0.1.2" - uglify-es "^3.2.0" + plugin-error "^1.0.1" + uglify-es "^3.3.9" vinyl "^2.1.0" vinyl-sourcemaps-apply "^0.2.1" @@ -6650,10 +6629,6 @@ keccakjs@^0.2.0: browserify-sha3 "^0.0.1" sha3 "^1.1.0" -kind-of@^1.1.0: - version "1.1.0" - resolved "https://registry.yarnpkg.com/kind-of/-/kind-of-1.1.0.tgz#140a3d2d41a36d2efcfa9377b62c24f8495a5c44" - kind-of@^3.0.2, kind-of@^3.0.3, kind-of@^3.1.0, kind-of@^3.2.0: version "3.2.2" resolved "https://registry.yarnpkg.com/kind-of/-/kind-of-3.2.2.tgz#31ea21a734bab9bbb0f32466d893aea51e4a3c64" @@ -8522,15 +8497,14 @@ plucker@0.0.0: version "0.0.0" resolved "https://registry.yarnpkg.com/plucker/-/plucker-0.0.0.tgz#2ffa24e03ab2cffa4e75adc1df70f25623c45d09" -plugin-error@^0.1.2: - version "0.1.2" - resolved "https://registry.yarnpkg.com/plugin-error/-/plugin-error-0.1.2.tgz#3b9bb3335ccf00f425e07437e19276967da47ace" +plugin-error@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/plugin-error/-/plugin-error-1.0.1.tgz#77016bd8919d0ac377fdcdd0322328953ca5781c" dependencies: - ansi-cyan "^0.1.1" - ansi-red "^0.1.1" - arr-diff "^1.0.1" - arr-union "^2.0.1" - extend-shallow "^1.1.2" + ansi-colors "^1.0.1" + arr-diff "^4.0.0" + arr-union "^3.1.0" + extend-shallow "^3.0.2" plur@^2.0.0, plur@^2.1.0, plur@^2.1.2: version "2.1.2" @@ -11145,11 +11119,11 @@ uglify-es@^3.0.15: commander "~2.12.1" source-map "~0.6.1" -uglify-es@^3.2.0: - version "3.3.7" - resolved "https://registry.yarnpkg.com/uglify-es/-/uglify-es-3.3.7.tgz#d1249af668666aba7cb1163e277455be9eb393cf" +uglify-es@^3.3.9: + version "3.3.10" + resolved "https://registry.yarnpkg.com/uglify-es/-/uglify-es-3.3.10.tgz#8b0b7992cebe20edc26de1bf325cef797b8f3fa5" dependencies: - commander "~2.13.0" + commander "~2.14.1" source-map "~0.6.1" uglify-js@^2.6, uglify-js@^2.8.27: -- cgit