From c2499d63aae44239679219024f866139397b28f2 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 f947c9e5c..c9e1b02db 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1261,6 +1261,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 28f9fb1f9..f627ddaef 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, @@ -128,7 +133,6 @@ export default class EditApprovalPermission extends PureComponent {
{ this.setState({ customSpendLimit: event.target.value }) @@ -139,6 +143,7 @@ export default class EditApprovalPermission extends PureComponent { fullWidth margin="dense" value={ this.state.customSpendLimit } + error={error} />
@@ -148,10 +153,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 ( { @@ -162,9 +201,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 d1acc6681..75608c16c 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 @@ -15,6 +15,7 @@ export default class ConfirmApproveContent extends Component { static propTypes = { amount: PropTypes.string, txFeeTotal: PropTypes.string, + decimals: PropTypes.number, tokenAmount: PropTypes.string, customTokenAmount: PropTypes.string, tokenSymbol: PropTypes.string, @@ -124,6 +125,7 @@ export default class ConfirmApproveContent extends Component { render () { const { t } = this.context const { + decimals, siteImage, tokenAmount, customTokenAmount, @@ -159,7 +161,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') }
@@ -201,10 +211,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 2e02f6ba3..786a854e7 100644 --- a/ui/app/pages/confirm-approve/confirm-approve.component.js +++ b/ui/app/pages/confirm-approve/confirm-approve.component.js @@ -86,6 +86,7 @@ export default class ConfirmApprove extends Component { showAccountInHeader title={tokensText} contentComponent={ { diff --git a/ui/app/pages/confirm-approve/confirm-approve.container.js b/ui/app/pages/confirm-approve/confirm-approve.container.js index 5a3223564..185c0f9c6 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, })), } }