From 4ef908aeb2410a8cb4c4c1c0304a8da00c0672d9 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Mon, 9 Nov 2020 10:20:24 -0330 Subject: [PATCH] Revert "Remove unnecessary lock page (#9793)" (#9825) This reverts commit f5265c24ab2359f7d51b5f527dde23a7132c4bd1. Apparently it wasn't unnecessary after all. The Lock page served a few different purposes. First, it was used to safeguard the seed phrase, in case the user was interrupted after setting a password. Otherwise anyone could open MetaMask and see the seed phrase without verifying the password. Second, the submit function for the initialization unlock screen also returned the seed phrase, so that it could be set in React state for the confirmation step. Third, the submit function was also responsible for navigating back to the seed phrase reveal page. Removing the lock page had the effect of causing an infinite render loop if onboarding was interrupted in the "Create" flow after setting a password but before seed phrase confirmation. That redirect loop has now been fixed. --- ui/app/helpers/constants/routes.js | 3 ++ .../first-time-flow-switch.component.js | 8 +++- .../first-time-flow-switch.container.js | 3 +- .../tests/first-time-flow-switch.test.js | 17 ++++++++ ui/app/pages/lock/index.js | 1 + ui/app/pages/lock/lock.component.js | 26 ++++++++++++ ui/app/pages/lock/lock.container.js | 26 ++++++++++++ ui/app/pages/lock/tests/lock.test.js | 40 +++++++++++++++++++ ui/app/pages/routes/routes.component.js | 3 ++ 9 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 ui/app/pages/lock/index.js create mode 100644 ui/app/pages/lock/lock.component.js create mode 100644 ui/app/pages/lock/lock.container.js create mode 100644 ui/app/pages/lock/tests/lock.test.js diff --git a/ui/app/helpers/constants/routes.js b/ui/app/helpers/constants/routes.js index 4824a9b13..efd09c151 100644 --- a/ui/app/helpers/constants/routes.js +++ b/ui/app/helpers/constants/routes.js @@ -1,5 +1,6 @@ const DEFAULT_ROUTE = '/' const UNLOCK_ROUTE = '/unlock' +const LOCK_ROUTE = '/lock' const ASSET_ROUTE = '/asset' const SETTINGS_ROUTE = '/settings' const GENERAL_ROUTE = '/settings/general' @@ -67,6 +68,7 @@ const ENCRYPTION_PUBLIC_KEY_REQUEST_PATH = '/encryption-public-key-request' const PATH_NAME_MAP = { [DEFAULT_ROUTE]: 'Home', [UNLOCK_ROUTE]: 'Unlock Page', + [LOCK_ROUTE]: 'Lock Page', [`${ASSET_ROUTE}/:asset`]: `Asset Page`, [SETTINGS_ROUTE]: 'Settings Page', [GENERAL_ROUTE]: 'General Settings Page', @@ -133,6 +135,7 @@ export { ALERTS_ROUTE, ASSET_ROUTE, UNLOCK_ROUTE, + LOCK_ROUTE, SETTINGS_ROUTE, REVEAL_SEED_ROUTE, MOBILE_SYNC_ROUTE, diff --git a/ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.component.js b/ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.component.js index 45c7d311d..54503bf1d 100644 --- a/ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.component.js +++ b/ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.component.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types' import { Redirect } from 'react-router-dom' import { DEFAULT_ROUTE, + LOCK_ROUTE, INITIALIZE_WELCOME_ROUTE, INITIALIZE_UNLOCK_ROUTE, } from '../../../helpers/constants/routes' @@ -11,15 +12,20 @@ export default class FirstTimeFlowSwitch extends PureComponent { static propTypes = { completedOnboarding: PropTypes.bool, isInitialized: PropTypes.bool, + isUnlocked: PropTypes.bool, } render() { - const { completedOnboarding, isInitialized } = this.props + const { completedOnboarding, isInitialized, isUnlocked } = this.props if (completedOnboarding) { return } + if (isUnlocked) { + return + } + if (!isInitialized) { return } diff --git a/ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.container.js b/ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.container.js index 2f1293b31..1b35418b5 100644 --- a/ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.container.js +++ b/ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.container.js @@ -2,11 +2,12 @@ import { connect } from 'react-redux' import FirstTimeFlowSwitch from './first-time-flow-switch.component' const mapStateToProps = ({ metamask }) => { - const { completedOnboarding, isInitialized } = metamask + const { completedOnboarding, isInitialized, isUnlocked } = metamask return { completedOnboarding, isInitialized, + isUnlocked, } } diff --git a/ui/app/pages/first-time-flow/first-time-flow-switch/tests/first-time-flow-switch.test.js b/ui/app/pages/first-time-flow/first-time-flow-switch/tests/first-time-flow-switch.test.js index 950f87e94..98a0f1103 100644 --- a/ui/app/pages/first-time-flow/first-time-flow-switch/tests/first-time-flow-switch.test.js +++ b/ui/app/pages/first-time-flow/first-time-flow-switch/tests/first-time-flow-switch.test.js @@ -3,6 +3,7 @@ import React from 'react' import { mountWithRouter } from '../../../../../../test/lib/render-helpers' import { DEFAULT_ROUTE, + LOCK_ROUTE, INITIALIZE_WELCOME_ROUTE, INITIALIZE_UNLOCK_ROUTE, } from '../../../../helpers/constants/routes' @@ -34,6 +35,22 @@ describe('FirstTimeFlowSwitch', function () { ) }) + it('redirects to /lock route when isUnlocked is true ', function () { + const props = { + completedOnboarding: false, + isUnlocked: true, + } + + const wrapper = mountWithRouter( + , + ) + + assert.equal( + wrapper.find('Lifecycle').find({ to: { pathname: LOCK_ROUTE } }).length, + 1, + ) + }) + it('redirects to /welcome route when isInitialized is false', function () { const props = { completedOnboarding: false, diff --git a/ui/app/pages/lock/index.js b/ui/app/pages/lock/index.js new file mode 100644 index 000000000..7bfe2a61f --- /dev/null +++ b/ui/app/pages/lock/index.js @@ -0,0 +1 @@ +export { default } from './lock.container' diff --git a/ui/app/pages/lock/lock.component.js b/ui/app/pages/lock/lock.component.js new file mode 100644 index 000000000..a415f5b55 --- /dev/null +++ b/ui/app/pages/lock/lock.component.js @@ -0,0 +1,26 @@ +import React, { PureComponent } from 'react' +import PropTypes from 'prop-types' +import Loading from '../../components/ui/loading-screen' +import { DEFAULT_ROUTE } from '../../helpers/constants/routes' + +export default class Lock extends PureComponent { + static propTypes = { + history: PropTypes.object, + isUnlocked: PropTypes.bool, + lockMetamask: PropTypes.func, + } + + componentDidMount() { + const { lockMetamask, isUnlocked, history } = this.props + + if (isUnlocked) { + lockMetamask().then(() => history.push(DEFAULT_ROUTE)) + } else { + history.replace(DEFAULT_ROUTE) + } + } + + render() { + return + } +} diff --git a/ui/app/pages/lock/lock.container.js b/ui/app/pages/lock/lock.container.js new file mode 100644 index 000000000..b1da86f67 --- /dev/null +++ b/ui/app/pages/lock/lock.container.js @@ -0,0 +1,26 @@ +import { compose } from 'redux' +import { connect } from 'react-redux' +import { withRouter } from 'react-router-dom' +import { lockMetamask } from '../../store/actions' +import Lock from './lock.component' + +const mapStateToProps = (state) => { + const { + metamask: { isUnlocked }, + } = state + + return { + isUnlocked, + } +} + +const mapDispatchToProps = (dispatch) => { + return { + lockMetamask: () => dispatch(lockMetamask()), + } +} + +export default compose( + withRouter, + connect(mapStateToProps, mapDispatchToProps), +)(Lock) diff --git a/ui/app/pages/lock/tests/lock.test.js b/ui/app/pages/lock/tests/lock.test.js new file mode 100644 index 000000000..92a46af6e --- /dev/null +++ b/ui/app/pages/lock/tests/lock.test.js @@ -0,0 +1,40 @@ +import assert from 'assert' +import React from 'react' +import sinon from 'sinon' +import { mountWithRouter } from '../../../../../test/lib/render-helpers' +import Lock from '..' + +describe('Lock', function () { + it('replaces history with default route when isUnlocked false', function () { + const props = { + isUnlocked: false, + history: { + replace: sinon.spy(), + }, + } + + mountWithRouter() + + assert.equal(props.history.replace.getCall(0).args[0], '/') + }) + + it('locks and pushes history with default route when isUnlocked true', function (done) { + const props = { + isUnlocked: true, + lockMetamask: sinon.stub(), + history: { + push: sinon.spy(), + }, + } + + props.lockMetamask.resolves() + + mountWithRouter() + + assert(props.lockMetamask.calledOnce) + setImmediate(() => { + assert.equal(props.history.push.getCall(0).args[0], '/') + done() + }) + }) +}) diff --git a/ui/app/pages/routes/routes.component.js b/ui/app/pages/routes/routes.component.js index abebf1554..04db4f0e9 100644 --- a/ui/app/pages/routes/routes.component.js +++ b/ui/app/pages/routes/routes.component.js @@ -13,6 +13,7 @@ import Home from '../home' import Settings from '../settings' import Authenticated from '../../helpers/higher-order-components/authenticated' import Initialized from '../../helpers/higher-order-components/initialized' +import Lock from '../lock' import PermissionsConnect from '../permissions-connect' import RestoreVaultPage from '../keychains/restore-vault' import RevealSeedConfirmation from '../keychains/reveal-seed' @@ -42,6 +43,7 @@ import { DEFAULT_ROUTE, INITIALIZE_ROUTE, INITIALIZE_UNLOCK_ROUTE, + LOCK_ROUTE, MOBILE_SYNC_ROUTE, NEW_ACCOUNT_ROUTE, RESTORE_VAULT_ROUTE, @@ -112,6 +114,7 @@ export default class Routes extends Component { const routes = ( +