From 4a1853cf7b66ad1a9da4ef43b23a52a1db7cca6e Mon Sep 17 00:00:00 2001 From: David Walsh Date: Thu, 2 May 2024 12:15:17 -0500 Subject: [PATCH] Fix #24322 - Disable edits for all dapp initiated transactions (#24334) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Ensures only transactions started within MetaMask are editable [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24334?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/24322 ## **Manual testing steps** 1. Go to the test dapp (https://metamask.github.io/test-dapp/) 2. Trigger any number of transactions 3. Do not see "Edit" in header 4. Open MetaMask, manually start a send, see the "Edit" button ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Dan J Miller --- .../dapp-interactions/dapp-tx-edit.spec.js | 6 ++-- ...confirm-page-container-header.component.js | 33 +++++++++++-------- ...rm-page-container-header.component.test.js | 3 ++ .../confirm-page-container.component.js | 1 + .../confirm-send-ether.test.js.snap | 15 --------- 5 files changed, 26 insertions(+), 32 deletions(-) diff --git a/test/e2e/tests/dapp-interactions/dapp-tx-edit.spec.js b/test/e2e/tests/dapp-interactions/dapp-tx-edit.spec.js index 94e4b0084766..df98799a462d 100644 --- a/test/e2e/tests/dapp-interactions/dapp-tx-edit.spec.js +++ b/test/e2e/tests/dapp-interactions/dapp-tx-edit.spec.js @@ -56,7 +56,7 @@ describe('Editing confirmations of dapp initiated contract interactions', functi ); }); - it('should show an edit button on a simple ETH send initiated by a dapp', async function () { + it('should NOT show an edit button on a simple ETH send initiated by a dapp', async function () { await withFixtures( { dapp: true, @@ -88,8 +88,8 @@ describe('Editing confirmations of dapp initiated contract interactions', functi ); assert.equal( editTransactionButton, - true, - `Edit transaction button should be visible on a contract interaction created by a dapp`, + false, + `Edit transaction button should not be visible on a simple send transaction created by a dapp`, ); }, ); diff --git a/ui/pages/confirmations/components/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.js b/ui/pages/confirmations/components/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.js index f5e2574a688d..0ee751357114 100644 --- a/ui/pages/confirmations/components/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.js +++ b/ui/pages/confirmations/components/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import { ENVIRONMENT_TYPE_POPUP, ENVIRONMENT_TYPE_NOTIFICATION, + ORIGIN_METAMASK, } from '../../../../../../shared/constants/app'; import { getEnvironmentType } from '../../../../../../app/scripts/lib/util'; import NetworkDisplay from '../../../../../components/app/network-display'; @@ -18,6 +19,7 @@ export default function ConfirmPageContainerHeader({ accountAddress, showAccountInHeader, children, + origin, }) { const t = useI18nContext(); const windowType = getEnvironmentType(); @@ -48,21 +50,23 @@ export default function ConfirmPageContainerHeader({ ) : ( -
- - onEdit()} + origin === ORIGIN_METAMASK && ( +
- {t('edit')} - -
+ + onEdit()} + > + {t('edit')} + +
+ ) )} @@ -77,4 +81,5 @@ ConfirmPageContainerHeader.propTypes = { showEdit: PropTypes.bool, onEdit: PropTypes.func, children: PropTypes.node, + origin: PropTypes.string, }; diff --git a/ui/pages/confirmations/components/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.test.js b/ui/pages/confirmations/components/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.test.js index e7043c283d19..6a48d8473a8e 100644 --- a/ui/pages/confirmations/components/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.test.js +++ b/ui/pages/confirmations/components/confirm-page-container/confirm-page-container-header/confirm-page-container-header.component.test.js @@ -2,6 +2,7 @@ import React from 'react'; import configureStore from 'redux-mock-store'; import { NetworkStatus } from '@metamask/network-controller'; import { NetworkType } from '@metamask/controller-utils'; +import { ORIGIN_METAMASK } from '../../../../../../shared/constants/app'; import { renderWithProvider } from '../../../../../../test/lib/render-helpers'; import { getEnvironmentType } from '../../../../../../app/scripts/lib/util'; import ConfirmPageContainerHeader from '.'; @@ -38,6 +39,7 @@ describe('Confirm Detail Row Component', () => { onEdit: jest.fn(), showAccountInHeader: false, accountAddress: '0xmockAccountAddress', + origin: ORIGIN_METAMASK, }; const { container } = renderWithProvider( @@ -56,6 +58,7 @@ describe('Confirm Detail Row Component', () => { onEdit: jest.fn(), showAccountInHeader: false, accountAddress: '0xmockAccountAddress', + origin: ORIGIN_METAMASK, }; const { container } = renderWithProvider( diff --git a/ui/pages/confirmations/components/confirm-page-container/confirm-page-container.component.js b/ui/pages/confirmations/components/confirm-page-container/confirm-page-container.component.js index 82fd617131b7..ad546c81992f 100644 --- a/ui/pages/confirmations/components/confirm-page-container/confirm-page-container.component.js +++ b/ui/pages/confirmations/components/confirm-page-container/confirm-page-container.component.js @@ -217,6 +217,7 @@ const ConfirmPageContainer = (props) => { onEdit={() => onEdit()} showAccountInHeader={showAccountInHeader} accountAddress={fromAddress} + origin={origin} > {hideSenderToRecipient ? null : ( -
- - - Edit - -