From 8d7698e481b4beb92ff3e283ae0696daa9361dcf Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Thu, 30 Apr 2020 14:56:32 -0400 Subject: [PATCH] Separate idle modal and Bob "lock" from underlying hsd wallet lock. Request user passphrase when needed. (#128) * Use default timeout (60s) for wallet unlock * Removed unused SET_PENDING_TRANSACTIONS * MiniModal onClose() defaults to history.goBack() * Do not lock Bob if hsd wallet is locked Separates Bob's `isLocked` state property from the actual hsd wallet. `isLocked` will now ONLY refer to Bob's state, triggering the enter-passphrase prompt to lock out the wallet, and only after the idle timeout. Actual hsd wallet lock & unlock will be handled separately * Do not sign when using createTX to estimate fee This will prevent hsd from requiring a password just to check the estimated TX size. * Accept optional onClose prop in MiniModal onClose() will be called when user clicks X or outside modal to close. If no function is provided, it will default to history.goBack() unless a specific closeRoute prop is passed. If both onClose and closeRoute are passed, onClose() will be called first instead of "going" to closeRoute. * Display error message on TX failure in OpenBid and Reveal panels * Introduce PassphraseModal Modal is set in App root and displays when getPassphrase is activated in global store. Expects a Promise resolve, reject pair to be passed in via store as well, making it easy to call from any component to wait for password unlock success before continuing. * Deploy PassphraseModal any time private key is needed This includes normal send, OPEN, BID, REVEAL, RENEW, REGISTER * Update changelog --- CHANGELOG.md | 3 + app/background/wallet/service.js | 5 +- app/components/Modal/MiniModal.js | 10 ++- app/ducks/backgroundMonitor.js | 15 +--- app/ducks/myDomains.js | 1 - app/ducks/names.js | 27 +++++- app/ducks/walletActions.js | 23 +++-- app/ducks/walletReducer.js | 14 ++-- app/pages/AcountLogin/PassphraseModal.js | 93 +++++++++++++++++++++ app/pages/App/index.js | 2 + app/pages/Auction/BidActionPanel/OpenBid.js | 2 +- app/pages/Auction/BidActionPanel/Reveal.js | 2 +- 12 files changed, 160 insertions(+), 37 deletions(-) create mode 100644 app/pages/AcountLogin/PassphraseModal.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 18a9b23b9..29383e8bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 with "spendable" balance which is total unconfirmed minus total locked coins - Covenants in portfolio view now display their value as it affects spendable balance - Improvements to maxSend based on spendable balance and cleaner fee estimation +- Bob will ask user for passphrase whenever private key is needed (e.g. send TX) +- Bob will no longer "logout" when underlying hsd wallet is locked, however +Bob will still lock the hsd wallet on logout or idle timeout ## [0.2.8] - 2020-03-17 ### Fixed diff --git a/app/background/wallet/service.js b/app/background/wallet/service.js index babff61fa..1d7495776 100644 --- a/app/background/wallet/service.js +++ b/app/background/wallet/service.js @@ -151,7 +151,8 @@ class WalletService { value: Number(toBaseUnits(amount)), address: to, }], - subtractFee + subtractFee, + sign: false, }); return { feeRate, @@ -222,7 +223,7 @@ class WalletService { unlock = (passphrase) => this._ledgerProxy( () => null, - () => this.client.unlock(WALLET_ID, passphrase, 60 * 60 * 24 * 365), + () => this.client.unlock(WALLET_ID, passphrase), ); isLocked = () => this._ledgerProxy( diff --git a/app/components/Modal/MiniModal.js b/app/components/Modal/MiniModal.js index f6ffd7606..6848b1e41 100644 --- a/app/components/Modal/MiniModal.js +++ b/app/components/Modal/MiniModal.js @@ -7,7 +7,8 @@ import './mini-modal.scss'; export class MiniModal extends Component { static propTypes = { - closeRoute: PropTypes.string.isRequired, + closeRoute: PropTypes.string, + onClose: PropTypes.func, children: PropTypes.node.isRequired, title: PropTypes.string.isRequired, centered: PropTypes.bool, @@ -15,7 +16,12 @@ export class MiniModal extends Component { }; onClose = () => { - this.props.history.push(this.props.closeRoute) + if (this.props.onClose) + return this.props.onClose(); + + this.props.closeRoute + ? this.props.history.push(this.props.closeRoute) + : this.props.history.goBack(); }; render() { diff --git a/app/ducks/backgroundMonitor.js b/app/ducks/backgroundMonitor.js index cf3a61bc6..d252267ba 100644 --- a/app/ducks/backgroundMonitor.js +++ b/app/ducks/backgroundMonitor.js @@ -2,7 +2,7 @@ import walletClient from '../utils/walletClient'; import nodeClient from '../utils/nodeClient'; import * as logger from '../utils/logClient'; import { store } from '../store/configureStore'; -import { LOCK_WALLET, SET_PENDING_TRANSACTIONS } from './walletReducer'; +import { SET_PENDING_TRANSACTIONS } from './walletReducer'; import { getInitializationState } from '../db/system'; import isEqual from 'lodash.isequal'; import { SET_NODE_INFO, SET_FEE_INFO, NEW_BLOCK_STATUS } from './nodeReducer'; @@ -11,7 +11,6 @@ import { getYourBids } from './bids'; import { fetchTransactions, fetchWallet } from './walletActions'; export function createBackgroundMonitor() { - let isLocked; let timeout; let info; let prevNamesWithPendingUpdates = new Set(); @@ -27,18 +26,6 @@ export function createBackgroundMonitor() { return; } - const newIsLocked = await walletClient.isLocked(); - if (newIsLocked) { - if (newIsLocked !== isLocked) { - isLocked = newIsLocked; - store.dispatch({ - type: LOCK_WALLET, - }); - } - - return; - } - const infoRes = await nodeClient.getInfo(); const newInfo = { chain: infoRes.chain, diff --git a/app/ducks/myDomains.js b/app/ducks/myDomains.js index 4e018fdf8..176e2c44e 100644 --- a/app/ducks/myDomains.js +++ b/app/ducks/myDomains.js @@ -1,5 +1,4 @@ import walletClient from '../utils/walletClient'; -import { SET_PENDING_TRANSACTIONS } from './walletActions'; const FETCH_MY_NAMES_START = 'app/myDomains/fetchMyNamesStart'; const FETCH_MY_NAMES_STOP = 'app/myDomains/fetchMyNamesStop'; diff --git a/app/ducks/names.js b/app/ducks/names.js index b0141bab9..0799dbb4c 100644 --- a/app/ducks/names.js +++ b/app/ducks/names.js @@ -6,7 +6,7 @@ import { stopWalletSync, waitForWalletSync, fetchPendingTransactions, - SET_PENDING_TRANSACTIONS + getPassphrase, } from './walletActions'; import { SET_NAME } from './namesReducer'; @@ -139,6 +139,10 @@ async function inflateReveals(nClient, walletClient, bids) { } export const sendOpen = name => async (dispatch) => { + await new Promise((resolve, reject) => { + dispatch(getPassphrase(resolve, reject)); + }); + await walletClient.sendOpen(name); await namesDb.storeName(name); await dispatch(fetchPendingTransactions()); @@ -148,6 +152,9 @@ export const sendBid = (name, amount, lockup, height) => async (dispatch) => { if (!name) { return; } + await new Promise((resolve, reject) => { + dispatch(getPassphrase(resolve, reject)); + }); if (height) { try { @@ -165,34 +172,46 @@ export const sendBid = (name, amount, lockup, height) => async (dispatch) => { await namesDb.storeName(name); }; -export const sendReveal = (name) => async () => { +export const sendReveal = (name) => async (dispatch) => { if (!name) { return; } + await new Promise((resolve, reject) => { + dispatch(getPassphrase(resolve, reject)); + }); await namesDb.storeName(name); await walletClient.sendReveal(name); }; -export const sendRedeem = (name) => async () => { +export const sendRedeem = (name) => async (dispatch) => { if (!name) { return; } + await new Promise((resolve, reject) => { + dispatch(getPassphrase(resolve, reject)); + }); await namesDb.storeName(name); await walletClient.sendRedeem(name); }; -export const sendRenewal = (name) => async () => { +export const sendRenewal = (name) => async (dispatch) => { if (!name) { return; } + await new Promise((resolve, reject) => { + dispatch(getPassphrase(resolve, reject)); + }); await namesDb.storeName(name); await walletClient.sendRenewal(name); }; export const sendUpdate = (name, json) => async (dispatch) => { + await new Promise((resolve, reject) => { + dispatch(getPassphrase(resolve, reject)); + }); await namesDb.storeName(name); await walletClient.sendUpdate(name, json); await dispatch(fetchPendingTransactions()); diff --git a/app/ducks/walletActions.js b/app/ducks/walletActions.js index a681b2f10..8659234b3 100644 --- a/app/ducks/walletActions.js +++ b/app/ducks/walletActions.js @@ -16,6 +16,7 @@ import { START_SYNC_WALLET, STOP_SYNC_WALLET, SYNC_WALLET_PROGRESS, + GET_PASSPHRASE, } from './walletReducer'; import {NEW_BLOCK_STATUS} from './nodeReducer'; @@ -26,7 +27,6 @@ export const setWallet = opts => { initialized = false, address = '', type = NONE, - isLocked = true, balance = {}, } = opts; @@ -36,7 +36,6 @@ export const setWallet = opts => { initialized, address, type, - isLocked, balance, }, }; @@ -61,7 +60,6 @@ export const fetchWallet = () => async (dispatch, getState) => { initialized: false, address: '', type: NONE, - isLocked: true, balance: { ...getInitialState().balance, }, @@ -69,12 +67,10 @@ export const fetchWallet = () => async (dispatch, getState) => { } const accountInfo = await walletClient.getAccountInfo(); - const isLocked = await walletClient.isLocked(); dispatch(setWallet({ initialized: isInitialized, address: accountInfo && accountInfo.receiveAddress, type: NONE, - isLocked, balance: (accountInfo && accountInfo.balance) || { ...getInitialState().balance, }, @@ -87,7 +83,9 @@ export const revealSeed = (passphrase) => async () => { export const unlockWallet = passphrase => async (dispatch) => { await walletClient.unlock(passphrase); - await dispatch(fetchWallet()); + dispatch({ + type: UNLOCK_WALLET, + }); }; export const lockWallet = () => async (dispatch) => { @@ -105,6 +103,9 @@ export const removeWallet = () => async (dispatch, getState) => { }; export const send = (to, amount, fee) => async (dispatch) => { + await new Promise((resolve, reject) => { + dispatch(getPassphrase(resolve, reject)); + }); const res = await walletClient.send(to, amount, fee); await dispatch(fetchWallet()); return res; @@ -218,6 +219,16 @@ export const resetIdle = () => ({ type: RESET_IDLE, }); +export const getPassphrase = (resolve, reject) => ({ + type: GET_PASSPHRASE, + payload: {get: true, resolve, reject}, +}); + +export const closeGetPassphrase = () => ({ + type: GET_PASSPHRASE, + payload: {get: false}, +}); + export const watchActivity = () => dispatch => { if (!idleInterval) { // Increment idle once a minute diff --git a/app/ducks/walletReducer.js b/app/ducks/walletReducer.js index 41165b9ab..3c6b73b9e 100644 --- a/app/ducks/walletReducer.js +++ b/app/ducks/walletReducer.js @@ -13,6 +13,7 @@ export const SET_PENDING_TRANSACTIONS = 'app/wallet/setPendingTransactions'; export const START_SYNC_WALLET = 'app/wallet/startSyncWallet'; export const STOP_SYNC_WALLET = 'app/wallet/stopSyncWallet'; export const SYNC_WALLET_PROGRESS = 'app/wallet/syncWalletProgress'; +export const GET_PASSPHRASE = 'app/wallet/getPassphrase'; export function getInitialState() { return { @@ -31,6 +32,7 @@ export function getInitialState() { idle: 0, walletSync: false, walletSyncProgress: 0, + getPassphrase: {get: false}, }; } @@ -41,7 +43,6 @@ export default function walletReducer(state = getInitialState(), {type, payload} ...state, address: payload.address, type: payload.type, - isLocked: payload.isLocked, balance: { ...state.balance, confirmed: payload.balance.confirmed, @@ -55,11 +56,7 @@ export default function walletReducer(state = getInitialState(), {type, payload} case LOCK_WALLET: return { ...state, - balance: { - ...state.balance - }, - isLocked: true, - transactions: new Map() + isLocked: true }; case UNLOCK_WALLET: return { @@ -96,6 +93,11 @@ export default function walletReducer(state = getInitialState(), {type, payload} ...state, walletSyncProgress: payload, }; + case GET_PASSPHRASE: + return { + ...state, + getPassphrase: payload, + }; default: return state; } diff --git a/app/pages/AcountLogin/PassphraseModal.js b/app/pages/AcountLogin/PassphraseModal.js new file mode 100644 index 000000000..6d378d44e --- /dev/null +++ b/app/pages/AcountLogin/PassphraseModal.js @@ -0,0 +1,93 @@ +import React, { Component } from 'react'; +import { connect } from 'react-redux'; +import PropTypes from 'prop-types'; +import c from 'classnames'; +import * as walletActions from '../../ducks/walletActions'; +import './login.scss'; +import Submittable from '../../components/Submittable'; +import { Link } from 'react-router-dom'; +import MiniModal from '../../components/Modal/MiniModal'; + +@connect( + (state) => ({ + getPassphrase: state.wallet.getPassphrase, + }), + dispatch => ({ + unlockWallet: passphrase => dispatch(walletActions.unlockWallet(passphrase)), + closeGetPassphrase: () => dispatch(walletActions.closeGetPassphrase()), + }) +) +export default class PassphraseModal extends Component { + static propTypes = { + unlockWallet: PropTypes.func.isRequired, + closeGetPassphrase: PropTypes.func.isRequired, + getPassphrase: PropTypes.object.isRequired, + }; + + static defaultProps = { + className: '' + }; + + state = { + passphrase: '', + showError: false + }; + + async handleLogin(passphrase) { + try { + await this.props.unlockWallet(passphrase); + this.setState({passphrase: ''}); + this.props.getPassphrase.resolve(); + this.props.closeGetPassphrase(); + } catch (error) { + return this.setState({ showError: true }); + } + } + + onClose = () => { + this.setState({passphrase:'', showError: false}); + this.props.getPassphrase.reject({message: 'No Password'}); + this.props.closeGetPassphrase(); + }; + + render() { + const { passphrase, showError } = this.state; + + if (!this.props.getPassphrase.get) + return null; + + return ( + + this.handleLogin(passphrase)}> +
+ + this.setState({ passphrase: e.target.value, showError: false }) + } + value={passphrase} + autoFocus + /> +
+
+ {showError && `Invalid password.`} +
+ + + + ); + } +} diff --git a/app/pages/App/index.js b/app/pages/App/index.js index df22a66a5..0ca4ede36 100644 --- a/app/pages/App/index.js +++ b/app/pages/App/index.js @@ -24,6 +24,7 @@ import SearchTLD from '../SearchTLD'; import * as walletActions from '../../ducks/walletActions'; import './app.scss'; import AccountLogin from '../AcountLogin'; +import PassphraseModal from '../AcountLogin/PassphraseModal'; import * as node from '../../ducks/node'; import { onNewBlock } from '../../ducks/backgroundMonitor'; import Notification from '../../components/Notification'; @@ -70,6 +71,7 @@ class App extends Component {
+ {this.renderContent()}
); diff --git a/app/pages/Auction/BidActionPanel/OpenBid.js b/app/pages/Auction/BidActionPanel/OpenBid.js index 1aed96d2e..6336de9bd 100644 --- a/app/pages/Auction/BidActionPanel/OpenBid.js +++ b/app/pages/Auction/BidActionPanel/OpenBid.js @@ -36,7 +36,7 @@ class OpenBid extends Component { } catch (e) { console.error(e); logger.error(`Error received from OpenBid - sendOpen]\n\n${e.message}\n${e.stack}\n`); - this.props.showError('Failed to open bid. Please try again.'); + this.props.showError(`Failed to open bid: ${e.message}`); } }; diff --git a/app/pages/Auction/BidActionPanel/Reveal.js b/app/pages/Auction/BidActionPanel/Reveal.js index efc93c3fc..5b37ea76d 100644 --- a/app/pages/Auction/BidActionPanel/Reveal.js +++ b/app/pages/Auction/BidActionPanel/Reveal.js @@ -60,7 +60,7 @@ class Reveal extends Component { } catch (e) { console.error(e); logger.error(`Error received from Reveal - sendReveal]\n\n${e.message}\n${e.stack}\n`); - this.props.showError('Failed to reveal bid. Please try again.'); + this.props.showError(`Failed to reveal bid: ${e.message}`); } };