From 0584988688a471698e9b3ad05cb0597f0270ea9e Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 21 Feb 2017 14:25:47 -0800 Subject: Move sigUtil and keyrings to external modules These external modules now have their own test coverage and build enforcement. This allowed me to somewhat more easily add good tests around our personalSign strategy (held now in [eth-sig-util](https://github.com/flyswatter/eth-sig-util), and allow each of the keyrings to import that, etc. --- app/scripts/metamask-controller.js | 49 +++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) (limited to 'app/scripts/metamask-controller.js') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 29b13dc62..62242bd83 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -16,6 +16,7 @@ const CurrencyController = require('./lib/controllers/currency') const NoticeController = require('./notice-controller') const ShapeShiftController = require('./lib/controllers/shapeshift') const MessageManager = require('./lib/message-manager') +const PersonalMessageManager = require('./lib/personal-message-manager') const TxManager = require('./transaction-manager') const ConfigManager = require('./lib/config-manager') const extension = require('./lib/extension') @@ -23,6 +24,7 @@ const autoFaucet = require('./lib/auto-faucet') const nodeify = require('./lib/nodeify') const IdStoreMigrator = require('./lib/idStore-migrator') const accountImporter = require('./account-import-strategies') +const sigUtil = require('eth-sig-util') const version = require('../manifest.json').version @@ -105,6 +107,7 @@ module.exports = class MetamaskController extends EventEmitter { this.lookupNetwork() this.messageManager = new MessageManager() + this.personalMessageManager = new PersonalMessageManager() this.publicConfigStore = this.initPublicConfigStore() // TEMPORARY UNTIL FULL DEPRECATION: @@ -163,8 +166,13 @@ module.exports = class MetamaskController extends EventEmitter { }, // tx signing processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb), - // msg signing + // old style msg signing processMessage: this.newUnsignedMessage.bind(this), + + // new style msg signing + approvePersonalMessage: this.approvePersonalMessage.bind(this), + signPersonalMessage: this.signPersonalMessage.bind(this), + personalRecoverSigner: this.personalRecoverSigner.bind(this), }) return provider } @@ -209,6 +217,7 @@ module.exports = class MetamaskController extends EventEmitter { this.ethStore.getState(), this.txManager.memStore.getState(), this.messageManager.memStore.getState(), + this.personalMessageManager.memStore.getState(), this.keyringController.memStore.getState(), this.preferencesController.store.getState(), this.currencyController.store.getState(), @@ -449,6 +458,44 @@ module.exports = class MetamaskController extends EventEmitter { )(cb) } + // Prefixed Style Message Signing Methods: + approvePersonalMessage (cb) { + let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) + this.sendUpdate() + this.opts.showUnconfirmedMessage() + this.personalMessageManager.once(`${msgId}:finished`, (data) => { + switch (data.status) { + case 'signed': + return cb(null, data.rawSig) + case 'rejected': + return cb(new Error('MetaMask Message Signature: User denied transaction signature.')) + default: + return cb(new Error(`MetaMask Message Signature: Unknown problem: ${JSON.stringify(msgParams)}`)) + } + }) + } + + signPersonalMessage (msgParams) { + const msgId = msgParams.metamaskId + // sets the status op the message to 'approved' + // and removes the metamaskId for signing + return this.personalMessageManager.approveMessage(msgParams) + .then((cleanMsgParams) => { + // signs the message + return this.keyringController.signPersonalMessage(cleanMsgParams) + }) + .then((rawSig) => { + // tells the listener that the message has been signed + // and can be returned to the dapp + this.personalMessageManager.setMsgStatusSigned(msgId, rawSig) + return rawSig + }) + } + + personalRecoverSigner (msgParams) { + const recovered = sigUtil.recoverPersonalSignature(msgParams) + return Promise.resolve(recovered) + } markAccountsFound (cb) { this.configManager.setLostAccounts([]) -- cgit From 92fb07999a011fa6939c0068f15dd55a6bcd7506 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 21 Feb 2017 14:30:07 -0800 Subject: Point metamask-controller personalSignRecover method to keyring-controller --- app/scripts/metamask-controller.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'app/scripts/metamask-controller.js') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 62242bd83..06c133bb2 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -24,7 +24,6 @@ const autoFaucet = require('./lib/auto-faucet') const nodeify = require('./lib/nodeify') const IdStoreMigrator = require('./lib/idStore-migrator') const accountImporter = require('./account-import-strategies') -const sigUtil = require('eth-sig-util') const version = require('../manifest.json').version @@ -152,6 +151,8 @@ module.exports = class MetamaskController extends EventEmitter { // initializeProvider () { + const keyringController = this.keyringController + let provider = MetaMaskProvider({ static: { eth_syncing: false, @@ -171,8 +172,8 @@ module.exports = class MetamaskController extends EventEmitter { // new style msg signing approvePersonalMessage: this.approvePersonalMessage.bind(this), - signPersonalMessage: this.signPersonalMessage.bind(this), - personalRecoverSigner: this.personalRecoverSigner.bind(this), + signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), + personalRecoverSigner: nodeify(keyringController.recoverPersonalMessage).bind(keyringController), }) return provider } @@ -459,7 +460,7 @@ module.exports = class MetamaskController extends EventEmitter { } // Prefixed Style Message Signing Methods: - approvePersonalMessage (cb) { + approvePersonalMessage (msgParams, cb) { let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) this.sendUpdate() this.opts.showUnconfirmedMessage() @@ -492,11 +493,6 @@ module.exports = class MetamaskController extends EventEmitter { }) } - personalRecoverSigner (msgParams) { - const recovered = sigUtil.recoverPersonalSignature(msgParams) - return Promise.resolve(recovered) - } - markAccountsFound (cb) { this.configManager.setLostAccounts([]) this.sendUpdate() -- cgit From 8684fc40c78cb5293d85f751cf3927c14067d343 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 21 Feb 2017 14:41:55 -0800 Subject: Allow provider to init before keyringController --- app/scripts/metamask-controller.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'app/scripts/metamask-controller.js') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 06c133bb2..d58d1c22d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -151,7 +151,6 @@ module.exports = class MetamaskController extends EventEmitter { // initializeProvider () { - const keyringController = this.keyringController let provider = MetaMaskProvider({ static: { @@ -173,7 +172,7 @@ module.exports = class MetamaskController extends EventEmitter { // new style msg signing approvePersonalMessage: this.approvePersonalMessage.bind(this), signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), - personalRecoverSigner: nodeify(keyringController.recoverPersonalMessage).bind(keyringController), + personalRecoverSigner: nodeify(this.recoverPersonalMessage).bind(this), }) return provider } @@ -493,6 +492,11 @@ module.exports = class MetamaskController extends EventEmitter { }) } + recoverPersonalMessage (msgParams) { + const keyringController = this.keyringController + return keyringController.recoverPersonalMessage(msgParams) + } + markAccountsFound (cb) { this.configManager.setLostAccounts([]) this.sendUpdate() -- cgit From 564f920ae0a1be1aa08905f1b4cf6d081e9a5a0b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 22 Feb 2017 16:23:13 -0800 Subject: Add personal sign actions and template --- app/scripts/metamask-controller.js | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'app/scripts/metamask-controller.js') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index d58d1c22d..b109918cf 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -241,6 +241,7 @@ module.exports = class MetamaskController extends EventEmitter { const preferencesController = this.preferencesController const txManager = this.txManager const messageManager = this.messageManager + const personalMessageManager = this.personalMessageManager const noticeController = this.noticeController return { @@ -285,6 +286,10 @@ module.exports = class MetamaskController extends EventEmitter { signMessage: this.signMessage.bind(this), cancelMessage: messageManager.rejectMsg.bind(messageManager), + // personalMessageManager + signPersonalMessage: this.signPersonalMessage.bind(this), + cancelPersonalMessage: personalMessageManager.rejectMsg.bind(personalMessageManager), + // notices checkNotices: noticeController.updateNoticesList.bind(noticeController), markNoticeRead: noticeController.markNoticeRead.bind(noticeController), -- cgit From 4697aca02c669b1787e72f0648b3043270867799 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 23 Feb 2017 14:23:45 -0800 Subject: Got personal_sign working Also fixed bug where signing would not close popup. --- app/scripts/metamask-controller.js | 55 +++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 19 deletions(-) (limited to 'app/scripts/metamask-controller.js') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index b109918cf..c301e2035 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -170,8 +170,7 @@ module.exports = class MetamaskController extends EventEmitter { processMessage: this.newUnsignedMessage.bind(this), // new style msg signing - approvePersonalMessage: this.approvePersonalMessage.bind(this), - signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), + processPersonalMessage: this.newUnsignedPersonalMessage.bind(this), personalRecoverSigner: nodeify(this.recoverPersonalMessage).bind(this), }) return provider @@ -283,11 +282,11 @@ module.exports = class MetamaskController extends EventEmitter { cancelTransaction: txManager.cancelTransaction.bind(txManager), // messageManager - signMessage: this.signMessage.bind(this), + signMessage: nodeify(this.signMessage).bind(this), cancelMessage: messageManager.rejectMsg.bind(messageManager), // personalMessageManager - signPersonalMessage: this.signPersonalMessage.bind(this), + signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), cancelPersonalMessage: personalMessageManager.rejectMsg.bind(personalMessageManager), // notices @@ -445,22 +444,39 @@ module.exports = class MetamaskController extends EventEmitter { }) } + newUnsignedPersonalMessage (msgParams, cb) { + let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) + this.sendUpdate() + this.opts.showUnconfirmedMessage() + this.personalMessageManager.once(`${msgId}:finished`, (data) => { + switch (data.status) { + case 'signed': + return cb(null, data.rawSig) + case 'rejected': + return cb(new Error('MetaMask Message Signature: User denied transaction signature.')) + default: + return cb(new Error(`MetaMask Message Signature: Unknown problem: ${JSON.stringify(msgParams)}`)) + } + }) + } + signMessage (msgParams, cb) { + log.info('MetaMaskController - signMessage') const msgId = msgParams.metamaskId - promiseToCallback( - // sets the status op the message to 'approved' - // and removes the metamaskId for signing - this.messageManager.approveMessage(msgParams) - .then((cleanMsgParams) => { - // signs the message - return this.keyringController.signMessage(cleanMsgParams) - }) - .then((rawSig) => { - // tells the listener that the message has been signed - // and can be returned to the dapp - this.messageManager.setMsgStatusSigned(msgId, rawSig) - }) - )(cb) + + // sets the status op the message to 'approved' + // and removes the metamaskId for signing + return this.messageManager.approveMessage(msgParams) + .then((cleanMsgParams) => { + // signs the message + return this.keyringController.signMessage(cleanMsgParams) + }) + .then((rawSig) => { + // tells the listener that the message has been signed + // and can be returned to the dapp + this.messageManager.setMsgStatusSigned(msgId, rawSig) + return this.getState() + }) } // Prefixed Style Message Signing Methods: @@ -481,6 +497,7 @@ module.exports = class MetamaskController extends EventEmitter { } signPersonalMessage (msgParams) { + log.info('MetaMaskController - signPersonalMessage') const msgId = msgParams.metamaskId // sets the status op the message to 'approved' // and removes the metamaskId for signing @@ -493,7 +510,7 @@ module.exports = class MetamaskController extends EventEmitter { // tells the listener that the message has been signed // and can be returned to the dapp this.personalMessageManager.setMsgStatusSigned(msgId, rawSig) - return rawSig + return this.getState() }) } -- cgit From 961a83769bd46334f5ecf72d00a32730d19866c3 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 23 Feb 2017 16:00:43 -0800 Subject: Fix cancel msg signing behavior. --- app/scripts/metamask-controller.js | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) (limited to 'app/scripts/metamask-controller.js') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index c301e2035..eace72c24 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -139,6 +139,7 @@ module.exports = class MetamaskController extends EventEmitter { this.ethStore.subscribe(this.sendUpdate.bind(this)) this.txManager.memStore.subscribe(this.sendUpdate.bind(this)) this.messageManager.memStore.subscribe(this.sendUpdate.bind(this)) + this.personalMessageManager.memStore.subscribe(this.sendUpdate.bind(this)) this.keyringController.memStore.subscribe(this.sendUpdate.bind(this)) this.preferencesController.store.subscribe(this.sendUpdate.bind(this)) this.currencyController.store.subscribe(this.sendUpdate.bind(this)) @@ -239,8 +240,6 @@ module.exports = class MetamaskController extends EventEmitter { const keyringController = this.keyringController const preferencesController = this.preferencesController const txManager = this.txManager - const messageManager = this.messageManager - const personalMessageManager = this.personalMessageManager const noticeController = this.noticeController return { @@ -283,11 +282,11 @@ module.exports = class MetamaskController extends EventEmitter { // messageManager signMessage: nodeify(this.signMessage).bind(this), - cancelMessage: messageManager.rejectMsg.bind(messageManager), + cancelMessage: this.cancelMessage.bind(this), // personalMessageManager signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), - cancelPersonalMessage: personalMessageManager.rejectMsg.bind(personalMessageManager), + cancelPersonalMessage: this.cancelPersonalMessage.bind(this), // notices checkNotices: noticeController.updateNoticesList.bind(noticeController), @@ -437,7 +436,7 @@ module.exports = class MetamaskController extends EventEmitter { case 'signed': return cb(null, data.rawSig) case 'rejected': - return cb(new Error('MetaMask Message Signature: User denied transaction signature.')) + return cb(new Error('MetaMask Message Signature: User denied message signature.')) default: return cb(new Error(`MetaMask Message Signature: Unknown problem: ${JSON.stringify(msgParams)}`)) } @@ -445,6 +444,10 @@ module.exports = class MetamaskController extends EventEmitter { } newUnsignedPersonalMessage (msgParams, cb) { + if (!msgParams.from) { + return cb(new Error('MetaMask Message Signature: from field is required.')) + } + let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) this.sendUpdate() this.opts.showUnconfirmedMessage() @@ -453,7 +456,7 @@ module.exports = class MetamaskController extends EventEmitter { case 'signed': return cb(null, data.rawSig) case 'rejected': - return cb(new Error('MetaMask Message Signature: User denied transaction signature.')) + return cb(new Error('MetaMask Message Signature: User denied message signature.')) default: return cb(new Error(`MetaMask Message Signature: Unknown problem: ${JSON.stringify(msgParams)}`)) } @@ -479,6 +482,14 @@ module.exports = class MetamaskController extends EventEmitter { }) } + cancelMessage(msgId, cb) { + const messageManager = this.messageManager + messageManager.rejectMsg(msgId) + if (cb && typeof cb === 'function') { + cb(null, this.getState()) + } + } + // Prefixed Style Message Signing Methods: approvePersonalMessage (msgParams, cb) { let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) @@ -514,6 +525,14 @@ module.exports = class MetamaskController extends EventEmitter { }) } + cancelPersonalMessage(msgId, cb) { + const messageManager = this.personalMessageManager + messageManager.rejectMsg(msgId) + if (cb && typeof cb === 'function') { + cb(null, this.getState()) + } + } + recoverPersonalMessage (msgParams) { const keyringController = this.keyringController return keyringController.recoverPersonalMessage(msgParams) -- cgit From f2851402f39a12c2a11e5dea7312c55b81c481cb Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 24 Feb 2017 16:36:29 -0800 Subject: Mostly fix personal_recover --- app/scripts/metamask-controller.js | 1 + 1 file changed, 1 insertion(+) (limited to 'app/scripts/metamask-controller.js') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index eace72c24..995db1c0a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -534,6 +534,7 @@ module.exports = class MetamaskController extends EventEmitter { } recoverPersonalMessage (msgParams) { + log.debug(`MetaMaskController - recoverPersonalMessage: ${JSON.stringify(msgParams)}`) const keyringController = this.keyringController return keyringController.recoverPersonalMessage(msgParams) } -- cgit From 8c66260bdb903185aaf55944a04b850af2dd64b6 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 24 Feb 2017 17:07:54 -0800 Subject: Removed redundant personal_recover logic --- app/scripts/metamask-controller.js | 7 ------- 1 file changed, 7 deletions(-) (limited to 'app/scripts/metamask-controller.js') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 995db1c0a..f172c67a8 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -172,7 +172,6 @@ module.exports = class MetamaskController extends EventEmitter { // new style msg signing processPersonalMessage: this.newUnsignedPersonalMessage.bind(this), - personalRecoverSigner: nodeify(this.recoverPersonalMessage).bind(this), }) return provider } @@ -533,12 +532,6 @@ module.exports = class MetamaskController extends EventEmitter { } } - recoverPersonalMessage (msgParams) { - log.debug(`MetaMaskController - recoverPersonalMessage: ${JSON.stringify(msgParams)}`) - const keyringController = this.keyringController - return keyringController.recoverPersonalMessage(msgParams) - } - markAccountsFound (cb) { this.configManager.setLostAccounts([]) this.sendUpdate() -- cgit