From 0a5c46b1565eec77c3cad9e886377293df9e2ed4 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 15 Nov 2022 12:28:39 -0330 Subject: [PATCH] Prevent user from editing a contract interaction created by a dapp (#16498) * Prevent user from editing a contract interaction created by a dapp * Code cleanup * Fix e2e test selector * Fix e2e test * Fix e2e test * Update snapshot --- test/e2e/tests/dapp-tx-edit.spec.js | 107 ++++++++++++++++++ test/e2e/webdriver/driver.js | 9 ++ ...ge-container-header.component.test.js.snap | 1 + ...confirm-page-container-header.component.js | 1 + .../confirm-transaction-base.component.js | 10 +- 5 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 test/e2e/tests/dapp-tx-edit.spec.js diff --git a/test/e2e/tests/dapp-tx-edit.spec.js b/test/e2e/tests/dapp-tx-edit.spec.js new file mode 100644 index 000000000..3b937f4ea --- /dev/null +++ b/test/e2e/tests/dapp-tx-edit.spec.js @@ -0,0 +1,107 @@ +const { strict: assert } = require('assert'); +const { convertToHexValue, withFixtures } = require('../helpers'); +const { SMART_CONTRACTS } = require('../seeder/smart-contracts'); +const FixtureBuilder = require('../fixture-builder'); + +describe('Editing confirmations of dapp initiated contract interactions', function () { + const ganacheOptions = { + accounts: [ + { + secretKey: + '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC', + balance: convertToHexValue(25000000000000000000), + }, + ], + }; + const smartContract = SMART_CONTRACTS.PIGGYBANK; + it('should NOT show an edit button on a contract interaction confirmation iniated by a dapp', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withPermissionControllerConnectedToTestDapp() + .build(), + ganacheOptions, + smartContract, + title: this.test.title, + }, + async ({ driver, contractRegistry }) => { + const contractAddress = await contractRegistry.getContractAddress( + smartContract, + ); + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + + // deploy contract + await driver.openNewPage( + `http://127.0.0.1:8080/?contract=${contractAddress}`, + ); + + // wait for deployed contract, calls and confirms a contract method where ETH is sent + await driver.findClickableElement('#deployButton'); + await driver.clickElement('#depositButton'); + await driver.waitUntilXWindowHandles(3); + const windowHandles = await driver.getAllWindowHandles(); + + await driver.switchToWindowWithTitle( + 'MetaMask Notification', + windowHandles, + ); + await driver.waitForSelector({ + css: '.confirm-page-container-summary__action__name', + text: 'Deposit', + }); + const editTransactionButton = await driver.isElementPresentAndVisible( + '[data-testid="confirm-page-back-edit-button"]', + ); + assert.equal( + editTransactionButton, + false, + `Edit transaction button should not be visible on a contract interaction created by a dapp`, + ); + }, + ); + }); + + it('should show an edit button on a simple ETH send iniated by a dapp', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withPermissionControllerConnectedToTestDapp() + .build(), + ganacheOptions, + smartContract, + title: this.test.title, + }, + async ({ driver }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + + await driver.openNewPage(`http://127.0.0.1:8080/`); + await driver.clickElement('#sendButton'); + await driver.waitUntilXWindowHandles(3); + const windowHandles = await driver.getAllWindowHandles(); + + await driver.switchToWindowWithTitle( + 'MetaMask Notification', + windowHandles, + ); + await driver.waitForSelector({ + css: '.confirm-page-container-summary__action__name', + text: 'Sending ETH', + }); + const editTransactionButton = await driver.isElementPresentAndVisible( + '[data-testid="confirm-page-back-edit-button"]', + ); + assert.equal( + editTransactionButton, + true, + `Edit transaction button should be visible on a contract interaction created by a dapp`, + ); + }, + ); + }); +}); diff --git a/test/e2e/webdriver/driver.js b/test/e2e/webdriver/driver.js index 4507d4905..28a9ac115 100644 --- a/test/e2e/webdriver/driver.js +++ b/test/e2e/webdriver/driver.js @@ -272,6 +272,15 @@ class Driver { } } + async isElementPresentAndVisible(rawLocator) { + try { + await this.findVisibleElement(rawLocator); + return true; + } catch (err) { + return false; + } + } + /** * Paste a string into a field. * diff --git a/ui/components/app/confirm-page-container/confirm-page-container-header/__snapshots__/confirm-page-container-header.component.test.js.snap b/ui/components/app/confirm-page-container/confirm-page-container-header/__snapshots__/confirm-page-container-header.component.test.js.snap index 9129dba7e..80aeb929f 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container-header/__snapshots__/confirm-page-container-header.component.test.js.snap +++ b/ui/components/app/confirm-page-container/confirm-page-container-header/__snapshots__/confirm-page-container-header.component.test.js.snap @@ -26,6 +26,7 @@ exports[`Confirm Detail Row Component should match snapshot 1`] = ` Edit diff --git a/ui/components/app/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.js b/ui/components/app/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.js index 1477e309b..40a9f3519 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.js +++ b/ui/components/app/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.js @@ -56,6 +56,7 @@ export default function ConfirmPageContainerHeader({ > onEdit()} > diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js index 01b6070ec..29755bbd6 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -1157,11 +1157,11 @@ export default class ConfirmTransactionBase extends Component { requestsWaitingText, } = this.getNavigateTxData(); - let functionType; - if ( + const isContractInteractionFromDapp = txData.type === TRANSACTION_TYPES.CONTRACT_INTERACTION && - txData.origin !== 'metamask' - ) { + txData.origin !== 'metamask'; + let functionType; + if (isContractInteractionFromDapp) { functionType = getMethodName(name); } @@ -1183,7 +1183,7 @@ export default class ConfirmTransactionBase extends Component { toAddress={toAddress} toEns={toEns} toNickname={toNickname} - showEdit={Boolean(onEdit)} + showEdit={!isContractInteractionFromDapp && Boolean(onEdit)} action={functionType} title={title} image={image}