From 58e98b14dac14ff7293f83418ce2b981563114d3 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 | 53 ++++++++++-- .../confirm-approve-content.component.js | 82 ++++++++++--------- .../confirm-approve.component.js | 1 + .../confirm-approve.container.js | 29 ++++--- 5 files changed, 110 insertions(+), 61 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index bf84c91dc5d7..15604d2e41d0 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1313,6 +1313,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 b492010eabaa..e9720fec82da 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, @@ -141,7 +146,6 @@ export default class EditApprovalPermission extends PureComponent {
{ this.setState({ customSpendLimit: event.target.value }) @@ -151,7 +155,8 @@ export default class EditApprovalPermission extends PureComponent { }} fullWidth margin="dense" - value={this.state.customSpendLimit} + value={ this.state.customSpendLimit } + error={error} />
@@ -161,10 +166,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 ( { @@ -175,11 +214,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 291849c88551..c4436b66b60c 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 @@ -11,6 +11,7 @@ export default class ConfirmApproveContent extends Component { } static propTypes = { + decimals: PropTypes.number, tokenAmount: PropTypes.string, customTokenAmount: PropTypes.string, tokenSymbol: PropTypes.string, @@ -146,6 +147,7 @@ export default class ConfirmApproveContent extends Component { render () { const { t } = this.context const { + decimals, siteImage, tokenAmount, customTokenAmount, @@ -181,16 +183,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')}
@@ -228,34 +229,39 @@ export default class ConfirmApproveContent extends Component { })}
- {showFullTxDetails ? ( -
-
- {this.renderApproveContentCard({ - symbol: , - title: 'Permission', - content: this.renderPermissionContent(), - showEdit: true, - onEditClick: () => - showEditApprovalPermissionModal({ - customTokenAmount, - tokenAmount, - tokenSymbol, - tokenBalance, - setCustomAmount, - }), - })} -
-
- {this.renderApproveContentCard({ - symbol: , - title: 'Data', - content: this.renderDataContent(), - noBorder: true, - })} -
-
- ) : null} + { + showFullTxDetails + ? ( +
+
+ {this.renderApproveContentCard({ + symbol: , + title: 'Permission', + content: this.renderPermissionContent(), + showEdit: true, + onEditClick: () => showEditApprovalPermissionModal({ + customTokenAmount, + decimals, + origin, + setCustomAmount, + tokenAmount, + tokenSymbol, + tokenBalance, + }), + })} +
+
+ {this.renderApproveContentCard({ + symbol: , + title: 'Data', + content: this.renderDataContent(), + noBorder: true, + })} +
+
+ ) + : null + } ) } diff --git a/ui/app/pages/confirm-approve/confirm-approve.component.js b/ui/app/pages/confirm-approve/confirm-approve.component.js index bf3af94a8ef7..484423fc874b 100644 --- a/ui/app/pages/confirm-approve/confirm-approve.component.js +++ b/ui/app/pages/confirm-approve/confirm-approve.component.js @@ -97,6 +97,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 92223142f29c..b14935d7987a 100644 --- a/ui/app/pages/confirm-approve/confirm-approve.container.js +++ b/ui/app/pages/confirm-approve/confirm-approve.container.js @@ -90,24 +90,23 @@ const mapDispatchToProps = dispatch => { showCustomizeGasModal: txData => dispatch(showModal({ name: 'CUSTOMIZE_GAS', txData })), showEditApprovalPermissionModal: ({ - tokenAmount, customTokenAmount, - tokenSymbol, - tokenBalance, + decimals, + origin, setCustomAmount, + tokenAmount, + tokenBalance, + tokenSymbol, + }) => dispatch(showModal({ + name: 'EDIT_APPROVAL_PERMISSION', + customTokenAmount, + decimals, origin, - }) => - dispatch( - showModal({ - name: 'EDIT_APPROVAL_PERMISSION', - tokenAmount, - customTokenAmount, - tokenSymbol, - tokenBalance, - setCustomAmount, - origin, - }) - ), + setCustomAmount, + tokenAmount, + tokenBalance, + tokenSymbol, + })), } }