Skip to content

Commit

Permalink
Use verifyPassword instead of submitPassword when exporting priv key (#…
Browse files Browse the repository at this point in the history
…9288)

* Use verifyPassword instead of submitPassword when exporting priv key

Fixes #9287 which was when submitPassword is called will fully clear the keyring state

https://github.com/MetaMask/KeyringController/blob/master/index.js#L155
https://github.com/MetaMask/KeyringController/blob/ad823d0ac15560d2bd22f737516736361f3ea107/index.js#L562
https://github.com/MetaMask/KeyringController/blob/ad823d0ac15560d2bd22f737516736361f3ea107/index.js#L726

Also, pass hideWarning action prop so it will clear the appState.warning if a correct password is never provided on componentWillUnmount

* Hide Warning on componentWillUnmount

* Update exportAccount tests to verifyPassword

* Update ui/app/store/actions.js
Co-authored-by: Mark Stacey <[email protected]>
  • Loading branch information
tmashuang and Gudahtt authored Aug 26, 2020
1 parent 20b0e66 commit 79d4770
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 9 deletions.
12 changes: 6 additions & 6 deletions test/unit/ui/app/actions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1144,10 +1144,10 @@ describe('Actions', function () {
})

describe('#exportAccount', function () {
let submitPasswordSpy, exportAccountSpy
let verifyPasswordSpy, exportAccountSpy

afterEach(function () {
submitPasswordSpy.restore()
verifyPasswordSpy.restore()
exportAccountSpy.restore()
})

Expand All @@ -1159,11 +1159,11 @@ describe('Actions', function () {
{ type: 'SHOW_PRIVATE_KEY', value: '7ec73b91bb20f209a7ff2d32f542c3420b4fccf14abcc7840d2eff0ebcb18505' },
]

submitPasswordSpy = sinon.spy(background, 'submitPassword')
verifyPasswordSpy = sinon.spy(background, 'verifyPassword')
exportAccountSpy = sinon.spy(background, 'exportAccount')

await store.dispatch(actions.exportAccount(password, '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'))
assert(submitPasswordSpy.calledOnce)
assert(verifyPasswordSpy.calledOnce)
assert(exportAccountSpy.calledOnce)
assert.deepEqual(store.getActions(), expectedActions)
})
Expand All @@ -1176,8 +1176,8 @@ describe('Actions', function () {
{ type: 'DISPLAY_WARNING', value: 'Incorrect Password.' },
]

submitPasswordSpy = sinon.stub(background, 'submitPassword')
submitPasswordSpy.callsFake((_, callback) => {
verifyPasswordSpy = sinon.stub(background, 'verifyPassword')
verifyPasswordSpy.callsFake((_, callback) => {
callback(new Error('error'))
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export default class ExportPrivateKeyModal extends Component {
warning: PropTypes.node,
showAccountDetailModal: PropTypes.func.isRequired,
hideModal: PropTypes.func.isRequired,
hideWarning: PropTypes.func.isRequired,
clearAccountDetails: PropTypes.func.isRequired,
previousModalState: PropTypes.string,
}
Expand All @@ -37,6 +38,7 @@ export default class ExportPrivateKeyModal extends Component {

componentWillUnmount () {
this.props.clearAccountDetails()
this.props.hideWarning()
}

exportAccountAndGetPrivateKey = (password, address) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ function mapDispatchToProps (dispatch) {
},
showAccountDetailModal: () => dispatch(showModal({ name: 'ACCOUNT_DETAILS' })),
hideModal: () => dispatch(hideModal()),
hideWarning: () => dispatch(hideWarning()),
clearAccountDetails: () => dispatch(clearAccountDetails()),
}
}
Expand Down
6 changes: 3 additions & 3 deletions ui/app/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1707,11 +1707,11 @@ export function exportAccount (password, address) {
return function (dispatch) {
dispatch(showLoadingIndication())

log.debug(`background.submitPassword`)
log.debug(`background.verifyPassword`)
return new Promise((resolve, reject) => {
background.submitPassword(password, function (err) {
background.verifyPassword(password, function (err) {
if (err) {
log.error('Error in submitting password.')
log.error('Error in verifying password.')
dispatch(hideLoadingIndication())
dispatch(displayWarning('Incorrect Password.'))
reject(err)
Expand Down

0 comments on commit 79d4770

Please sign in to comment.