From 59a1746afc6d286ab1c14aa9556ef26f1c0a844f Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 29 Jan 2020 14:16:38 -0400 Subject: [PATCH] Validate custom spend limit (#7920) The custom spend limit was previously not validated. It did have a minimum of zero set, but this didn't have any affect (that minimum is used for form constraint validation, and this field wasn't in a form). The field was never checked to ensure the contents didn't exceed the maximum. The field is now checked for values that exceed the maximum, and invalid values in general (including negative values). The parameters to the `showEditApprovalPermissionModal` were also alphabetized to make them easier to read. In the course of doing this, I noticed that the origin was missing from one of the calls. This was responsible for the modal saying "Spend limit requested by undefined" when clicking "Edit" under the transaction details. This has been fixed. --- app/_locales/en/messages.json | 6 +++ .../edit-approval-permission.component.js | 49 +++++++++++++++++-- .../confirm-approve-content.component.js | 16 +++++- .../confirm-approve.component.js | 1 + .../confirm-approve.container.js | 18 ++++--- 5 files changed, 75 insertions(+), 15 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 55b423894..64c95545a 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1307,6 +1307,12 @@ "message": "Spend limit requested by $1", "description": "Origin of the site requesting the spend limit" }, + "spendLimitTooLarge": { + "message": "Spend limit too large" + }, + "spendLimitInvalid": { + "message": "Spend limit invalid; must be a positive number" + }, "switchNetworks": { "message": "Switch Networks" }, diff --git a/ui/app/components/app/modals/edit-approval-permission/edit-approval-permission.component.js b/ui/app/components/app/modals/edit-approval-permission/edit-approval-permission.component.js index f7dad3a50..7c850279b 100644 --- a/ui/app/components/app/modals/edit-approval-permission/edit-approval-permission.component.js +++ b/ui/app/components/app/modals/edit-approval-permission/edit-approval-permission.component.js @@ -1,13 +1,18 @@ import React, { PureComponent } from 'react' import PropTypes from 'prop-types' +import log from 'loglevel' import Modal from '../../modal' import Identicon from '../../../ui/identicon' import TextField from '../../../ui/text-field' +import { calcTokenAmount } from '../../../../helpers/utils/token-util' import classnames from 'classnames' import BigNumber from 'bignumber.js' +const MAX_UNSIGNED_256_INT = new BigNumber(2).pow(256).minus(1).toString(10) + export default class EditApprovalPermission extends PureComponent { static propTypes = { + decimals: PropTypes.number, hideModal: PropTypes.func.isRequired, selectedIdentity: PropTypes.object, tokenAmount: PropTypes.string, @@ -15,7 +20,7 @@ export default class EditApprovalPermission extends PureComponent { tokenSymbol: PropTypes.string, tokenBalance: PropTypes.string, setCustomAmount: PropTypes.func, - origin: PropTypes.string, + origin: PropTypes.string.isRequired, } static contextTypes = { @@ -27,7 +32,7 @@ export default class EditApprovalPermission extends PureComponent { selectedOptionIsUnlimited: !this.props.customTokenAmount, } - renderModalContent () { + renderModalContent (error) { const { t } = this.context const { hideModal, @@ -136,7 +141,6 @@ export default class EditApprovalPermission extends PureComponent {
{ this.setState({ customSpendLimit: event.target.value }) @@ -147,6 +151,7 @@ export default class EditApprovalPermission extends PureComponent { fullWidth margin="dense" value={ this.state.customSpendLimit } + error={error} />
@@ -156,10 +161,44 @@ export default class EditApprovalPermission extends PureComponent { ) } + validateSpendLimit () { + const { t } = this.context + const { decimals } = this.props + const { selectedOptionIsUnlimited, customSpendLimit } = this.state + + if (selectedOptionIsUnlimited || !customSpendLimit) { + return + } + + let customSpendLimitNumber + try { + customSpendLimitNumber = new BigNumber(customSpendLimit) + } catch (error) { + log.debug(`Error converting '${customSpendLimit}' to BigNumber:`, error) + return t('spendLimitInvalid') + } + + if (customSpendLimitNumber.isNegative()) { + return t('spendLimitInvalid') + } + + const maxTokenAmount = calcTokenAmount(MAX_UNSIGNED_256_INT, decimals) + if (customSpendLimitNumber.greaterThan(maxTokenAmount)) { + return t('spendLimitTooLarge') + } + } + render () { const { t } = this.context const { setCustomAmount, hideModal, customTokenAmount } = this.props const { selectedOptionIsUnlimited, customSpendLimit } = this.state + + const error = this.validateSpendLimit() + const disabled = Boolean( + (customSpendLimit === customTokenAmount && !selectedOptionIsUnlimited) || + error + ) + return ( { @@ -170,9 +209,9 @@ export default class EditApprovalPermission extends PureComponent { submitType="primary" contentClass="edit-approval-permission-modal-content" containerClass="edit-approval-permission-modal-container" - submitDisabled={ (customSpendLimit === customTokenAmount) && !selectedOptionIsUnlimited } + submitDisabled={disabled} > - { this.renderModalContent() } + { this.renderModalContent(error) } ) } diff --git a/ui/app/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js b/ui/app/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js index 673bfe415..461d2e942 100644 --- a/ui/app/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js +++ b/ui/app/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js @@ -13,6 +13,7 @@ export default class ConfirmApproveContent extends Component { } static propTypes = { + decimals: PropTypes.number, tokenAmount: PropTypes.string, customTokenAmount: PropTypes.string, tokenSymbol: PropTypes.string, @@ -127,6 +128,7 @@ export default class ConfirmApproveContent extends Component { render () { const { t } = this.context const { + decimals, siteImage, tokenAmount, customTokenAmount, @@ -164,7 +166,15 @@ export default class ConfirmApproveContent extends Component { >
showEditApprovalPermissionModal({ customTokenAmount, tokenAmount, tokenSymbol, setCustomAmount, tokenBalance, origin })} + onClick={() => showEditApprovalPermissionModal({ + customTokenAmount, + decimals, + origin, + setCustomAmount, + tokenAmount, + tokenSymbol, + tokenBalance, + })} > { t('editPermission') }
@@ -209,10 +219,12 @@ export default class ConfirmApproveContent extends Component { showEdit: true, onEditClick: () => showEditApprovalPermissionModal({ customTokenAmount, + decimals, + origin, + setCustomAmount, tokenAmount, tokenSymbol, tokenBalance, - setCustomAmount, }), })} diff --git a/ui/app/pages/confirm-approve/confirm-approve.component.js b/ui/app/pages/confirm-approve/confirm-approve.component.js index bb7f6ceb2..01c93e5a4 100644 --- a/ui/app/pages/confirm-approve/confirm-approve.component.js +++ b/ui/app/pages/confirm-approve/confirm-approve.component.js @@ -87,6 +87,7 @@ export default class ConfirmApprove extends Component { title={tokensText} contentComponent={( { this.setState({ customPermissionAmount: newAmount }) diff --git a/ui/app/pages/confirm-approve/confirm-approve.container.js b/ui/app/pages/confirm-approve/confirm-approve.container.js index 29fec2ff3..92f0eb552 100644 --- a/ui/app/pages/confirm-approve/confirm-approve.container.js +++ b/ui/app/pages/confirm-approve/confirm-approve.container.js @@ -76,20 +76,22 @@ const mapDispatchToProps = (dispatch) => { return { showCustomizeGasModal: (txData) => dispatch(showModal({ name: 'CUSTOMIZE_GAS', txData })), showEditApprovalPermissionModal: ({ - tokenAmount, customTokenAmount, - tokenSymbol, - tokenBalance, - setCustomAmount, + decimals, origin, + setCustomAmount, + tokenAmount, + tokenBalance, + tokenSymbol, }) => dispatch(showModal({ name: 'EDIT_APPROVAL_PERMISSION', - tokenAmount, customTokenAmount, - tokenSymbol, - tokenBalance, - setCustomAmount, + decimals, origin, + setCustomAmount, + tokenAmount, + tokenBalance, + tokenSymbol, })), } }