diff --git a/shared/constants/security-provider.ts b/shared/constants/security-provider.ts new file mode 100644 index 000000000..1862e8181 --- /dev/null +++ b/shared/constants/security-provider.ts @@ -0,0 +1,13 @@ +/** + * @typedef {object} SecurityProviderMessageSeverity + * @property {0} NOT_MALICIOUS - Indicates message is not malicious + * @property {1} MALICIOUS - Indicates message is malicious + * @property {2} NOT_SAFE - Indicates message is not safe + */ + +/** @type {SecurityProviderMessageSeverity} */ +export const SECURITY_PROVIDER_MESSAGE_SEVERITY = { + NOT_MALICIOUS: 0, + MALICIOUS: 1, + NOT_SAFE: 2, +}; diff --git a/shared/modules/security-provider.utils.test.ts b/shared/modules/security-provider.utils.test.ts new file mode 100644 index 000000000..c32ea7d38 --- /dev/null +++ b/shared/modules/security-provider.utils.test.ts @@ -0,0 +1,30 @@ +import { SECURITY_PROVIDER_MESSAGE_SEVERITY } from '../constants/security-provider'; +import { isSuspiciousResponse } from './security-provider.utils'; + +describe('security-provider util', () => { + describe('isSuspiciousResponse', () => { + it('should return false if the response does not exist', () => { + const result = isSuspiciousResponse(undefined); + expect(result).toBeFalsy(); + }); + + it('should return false when flagAsDangerous exists and is not malicious', () => { + const result = isSuspiciousResponse({ + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS, + }); + expect(result).toBeFalsy(); + }); + + it('should return true when flagAsDangerous exists and is malicious or not safe', () => { + const result = isSuspiciousResponse({ + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_SAFE, + }); + expect(result).toBeTruthy(); + }); + + it('should return true if the response exists but is empty', () => { + const result = isSuspiciousResponse({}); + expect(result).toBeTruthy(); + }); + }); +}); diff --git a/shared/modules/security-provider.utils.ts b/shared/modules/security-provider.utils.ts new file mode 100644 index 000000000..3c341b4aa --- /dev/null +++ b/shared/modules/security-provider.utils.ts @@ -0,0 +1,19 @@ +import { Json } from '@metamask/utils'; +import { SECURITY_PROVIDER_MESSAGE_SEVERITY } from '../constants/security-provider'; + +export function isSuspiciousResponse( + securityProviderResponse: Record | undefined, +): boolean { + if (!securityProviderResponse) { + return false; + } + + const isFlagged = + securityProviderResponse.flagAsDangerous !== undefined && + securityProviderResponse.flagAsDangerous !== + SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS; + + const isNotVerified = Object.keys(securityProviderResponse).length === 0; + + return isFlagged || isNotVerified; +} diff --git a/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.js b/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.js index fd0651e13..713cd9293 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.js +++ b/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.js @@ -10,8 +10,9 @@ import { INSUFFICIENT_FUNDS_ERROR_KEY } from '../../../../helpers/constants/erro import Typography from '../../../ui/typography'; import { TypographyVariant } from '../../../../helpers/constants/design-system'; +import { isSuspiciousResponse } from '../../../../../shared/modules/security-provider.utils'; import SecurityProviderBannerMessage from '../../security-provider-banner-message/security-provider-banner-message'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from '../../security-provider-banner-message/security-provider-banner-message.constants'; + import { ConfirmPageContainerSummary, ConfirmPageContainerWarning } from '.'; export default class ConfirmPageContainerContent extends Component { @@ -214,15 +215,11 @@ export default class ConfirmPageContainerContent extends Component { {ethGasPriceWarning && ( )} - {(txData?.securityProviderResponse?.flagAsDangerous !== undefined && - txData?.securityProviderResponse?.flagAsDangerous !== - SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) || - (txData?.securityProviderResponse && - Object.keys(txData.securityProviderResponse).length === 0) ? ( + {isSuspiciousResponse(txData?.securityProviderResponse) && ( - ) : null} + )} { @@ -155,7 +155,7 @@ describe('Confirm Page Container Content', () => { it('should not render SecurityProviderBannerMessage component when flagAsDangerous is not malicious', () => { props.txData.securityProviderResponse = { - flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS, }; const { queryByText } = renderWithProvider( diff --git a/ui/components/app/security-provider-banner-message/security-provider-banner-message.constants.js b/ui/components/app/security-provider-banner-message/security-provider-banner-message.constants.js deleted file mode 100644 index fc30d0b5c..000000000 --- a/ui/components/app/security-provider-banner-message/security-provider-banner-message.constants.js +++ /dev/null @@ -1,5 +0,0 @@ -export const SECURITY_PROVIDER_MESSAGE_SEVERITIES = { - NOT_MALICIOUS: 0, - MALICIOUS: 1, - NOT_SAFE: 2, -}; diff --git a/ui/components/app/security-provider-banner-message/security-provider-banner-message.js b/ui/components/app/security-provider-banner-message/security-provider-banner-message.js index 71c928edd..fdaba451f 100644 --- a/ui/components/app/security-provider-banner-message/security-provider-banner-message.js +++ b/ui/components/app/security-provider-banner-message/security-provider-banner-message.js @@ -6,9 +6,9 @@ import { Size, TextVariant, } from '../../../helpers/constants/design-system'; +import { SECURITY_PROVIDER_MESSAGE_SEVERITY } from '../../../../shared/constants/security-provider'; import { I18nContext } from '../../../../.storybook/i18n'; import { BannerAlert, ButtonLink, Text } from '../../component-library'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from './security-provider-banner-message.constants'; export default function SecurityProviderBannerMessage({ securityProviderResponse, @@ -21,7 +21,7 @@ export default function SecurityProviderBannerMessage({ if ( securityProviderResponse.flagAsDangerous === - SECURITY_PROVIDER_MESSAGE_SEVERITIES.MALICIOUS + SECURITY_PROVIDER_MESSAGE_SEVERITY.MALICIOUS ) { messageTitle = securityProviderResponse.reason_header === '' @@ -34,7 +34,7 @@ export default function SecurityProviderBannerMessage({ severity = SEVERITIES.DANGER; } else if ( securityProviderResponse.flagAsDangerous === - SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_SAFE + SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_SAFE ) { messageTitle = t('requestMayNotBeSafe'); messageText = t('requestMayNotBeSafeError'); diff --git a/ui/components/app/security-provider-banner-message/security-provider-banner-message.test.js b/ui/components/app/security-provider-banner-message/security-provider-banner-message.test.js index 4fd21f7e9..f131fad16 100644 --- a/ui/components/app/security-provider-banner-message/security-provider-banner-message.test.js +++ b/ui/components/app/security-provider-banner-message/security-provider-banner-message.test.js @@ -2,8 +2,8 @@ import { fireEvent } from '@testing-library/react'; import React from 'react'; import configureMockStore from 'redux-mock-store'; import { renderWithProvider } from '../../../../test/lib/render-helpers'; +import { SECURITY_PROVIDER_MESSAGE_SEVERITY } from '../../../../shared/constants/security-provider'; import SecurityProviderBannerMessage from './security-provider-banner-message'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from './security-provider-banner-message.constants'; describe('Security Provider Banner Message', () => { const store = configureMockStore()({}); @@ -12,7 +12,7 @@ describe('Security Provider Banner Message', () => { it('should render SecurityProviderBannerMessage component properly when flagAsDangerous is malicious', () => { const securityProviderResponse = { - flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITIES.MALICIOUS, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.MALICIOUS, reason: 'Approval is to an unverified smart contract known for stealing NFTs in the past.', reason_header: 'This could be a scam', @@ -34,7 +34,7 @@ describe('Security Provider Banner Message', () => { it('should render SecurityProviderBannerMessage component properly when flagAsDangerous is not safe', () => { const securityProviderResponse = { - flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_SAFE, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_SAFE, reason: 'Some reason...', reason_header: 'Some reason header...', }; @@ -99,7 +99,7 @@ describe('Security Provider Banner Message', () => { it('should navigate to the OpenSea web page when clicked on the OpenSea button', () => { const securityProviderResponse = { - flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_SAFE, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_SAFE, reason: 'Some reason...', reason_header: 'Some reason header...', }; @@ -122,7 +122,7 @@ describe('Security Provider Banner Message', () => { it('should render SecurityProviderBannerMessage component properly, with predefined reason message, when a request is malicious and there is no reason given', () => { const securityProviderResponse = { - flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITIES.MALICIOUS, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.MALICIOUS, reason: '', reason_header: 'Some reason header...', }; @@ -145,7 +145,7 @@ describe('Security Provider Banner Message', () => { it('should render SecurityProviderBannerMessage component properly, with predefined reason_header message, when a request is malicious and there is no reason header given', () => { const securityProviderResponse = { - flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITIES.MALICIOUS, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.MALICIOUS, reason: 'Some reason...', reason_header: '', }; @@ -166,7 +166,7 @@ describe('Security Provider Banner Message', () => { it('should render SecurityProviderBannerMessage component properly, with predefined reason and reason_header messages, when a request is malicious and there are no reason and reason header given', () => { const securityProviderResponse = { - flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITIES.MALICIOUS, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.MALICIOUS, reason: '', reason_header: '', }; diff --git a/ui/components/app/signature-request-original/signature-request-original.component.js b/ui/components/app/signature-request-original/signature-request-original.component.js index 4b72ea529..b61bcf79e 100644 --- a/ui/components/app/signature-request-original/signature-request-original.component.js +++ b/ui/components/app/signature-request-original/signature-request-original.component.js @@ -13,6 +13,7 @@ import { ///: END:ONLY_INCLUDE_IN } from '../../../helpers/utils/util'; import { stripHexPrefix } from '../../../../shared/modules/hexstring-utils'; +import { isSuspiciousResponse } from '../../../../shared/modules/security-provider.utils'; import Button from '../../ui/button'; import SiteOrigin from '../../ui/site-origin'; import Typography from '../../ui/typography/typography'; @@ -32,7 +33,6 @@ import { } from '../../../helpers/constants/design-system'; import ConfirmPageContainerNavigation from '../confirm-page-container/confirm-page-container-navigation'; import SecurityProviderBannerMessage from '../security-provider-banner-message/security-provider-banner-message'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from '../security-provider-banner-message/security-provider-banner-message.constants'; ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) import { Icon, IconName, Text } from '../../component-library'; import Box from '../../ui/box/box'; @@ -133,15 +133,11 @@ export default class SignatureRequestOriginal extends Component { return (
- {(txData?.securityProviderResponse?.flagAsDangerous !== undefined && - txData?.securityProviderResponse?.flagAsDangerous !== - SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) || - (txData?.securityProviderResponse && - Object.keys(txData.securityProviderResponse).length === 0) ? ( + {isSuspiciousResponse(txData?.securityProviderResponse) && ( - ) : null} + )} { ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) diff --git a/ui/components/app/signature-request-original/signature-request-original.test.js b/ui/components/app/signature-request-original/signature-request-original.test.js index d9c136a86..030451514 100644 --- a/ui/components/app/signature-request-original/signature-request-original.test.js +++ b/ui/components/app/signature-request-original/signature-request-original.test.js @@ -3,10 +3,10 @@ import configureMockStore from 'redux-mock-store'; import { fireEvent, screen } from '@testing-library/react'; import { act } from 'react-dom/test-utils'; import { MESSAGE_TYPE } from '../../../../shared/constants/app'; +import { SECURITY_PROVIDER_MESSAGE_SEVERITY } from '../../../../shared/constants/security-provider'; import mockState from '../../../../test/data/mock-state.json'; import { renderWithProvider } from '../../../../test/lib/render-helpers'; import configureStore from '../../../store/store'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from '../security-provider-banner-message/security-provider-banner-message.constants'; import { resolvePendingApproval, rejectPendingApproval, @@ -168,7 +168,7 @@ describe('SignatureRequestOriginal', () => { it('should not render SecurityProviderBannerMessage component when flagAsDangerous is not malicious', () => { props.txData.securityProviderResponse = { - flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS, }; render(); diff --git a/ui/components/app/signature-request-siwe/signature-request-siwe.js b/ui/components/app/signature-request-siwe/signature-request-siwe.js index 33529e300..ddd4319e5 100644 --- a/ui/components/app/signature-request-siwe/signature-request-siwe.js +++ b/ui/components/app/signature-request-siwe/signature-request-siwe.js @@ -19,6 +19,7 @@ import { unconfirmedMessagesHashSelector, } from '../../../selectors'; import { getAccountByAddress, valuesFor } from '../../../helpers/utils/util'; +import { isSuspiciousResponse } from '../../../../shared/modules/security-provider.utils'; import { formatMessageParams } from '../../../../shared/modules/siwe'; import { clearConfirmTransaction } from '../../../ducks/confirm-transaction/confirm-transaction.duck'; @@ -35,7 +36,6 @@ import { } from '../../../store/actions'; import SecurityProviderBannerMessage from '../security-provider-banner-message/security-provider-banner-message'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from '../security-provider-banner-message/security-provider-banner-message.constants'; import ConfirmPageContainerNavigation from '../confirm-page-container/confirm-page-container-navigation'; import { getMostRecentOverviewPage } from '../../../ducks/history/history'; import LedgerInstructionField from '../ledger-instruction-field'; @@ -78,12 +78,9 @@ export default function SignatureRequestSIWE({ txData }) { const [hasAgreedToDomainWarning, setHasAgreedToDomainWarning] = useState(false); - const showSecurityProviderBanner = - (txData?.securityProviderResponse?.flagAsDangerous !== undefined && - txData?.securityProviderResponse?.flagAsDangerous !== - SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) || - (txData?.securityProviderResponse && - Object.keys(txData.securityProviderResponse).length === 0); + const showSecurityProviderBanner = isSuspiciousResponse( + txData?.securityProviderResponse, + ); const onSign = useCallback(async () => { try { diff --git a/ui/components/app/signature-request/signature-request.component.js b/ui/components/app/signature-request/signature-request.component.js index d03cf9fa8..8b6f1eaff 100644 --- a/ui/components/app/signature-request/signature-request.component.js +++ b/ui/components/app/signature-request/signature-request.component.js @@ -31,10 +31,10 @@ import { } from '../../../helpers/constants/design-system'; import NetworkAccountBalanceHeader from '../network-account-balance-header'; import { Numeric } from '../../../../shared/modules/Numeric'; +import { isSuspiciousResponse } from '../../../../shared/modules/security-provider.utils'; import { EtherDenomination } from '../../../../shared/constants/common'; import ConfirmPageContainerNavigation from '../confirm-page-container/confirm-page-container-navigation'; import SecurityProviderBannerMessage from '../security-provider-banner-message/security-provider-banner-message'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from '../security-provider-banner-message/security-provider-banner-message.constants'; import { formatCurrency } from '../../../helpers/utils/confirm-tx.util'; import { getValueFromWeiHex } from '../../../../shared/modules/conversion.utils'; ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) @@ -284,15 +284,11 @@ export default class SignatureRequest extends PureComponent { />
- {(txData?.securityProviderResponse?.flagAsDangerous !== undefined && - txData?.securityProviderResponse?.flagAsDangerous !== - SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) || - (txData?.securityProviderResponse && - Object.keys(txData.securityProviderResponse).length === 0) ? ( + {isSuspiciousResponse(txData?.securityProviderResponse) && ( - ) : null} + )} { ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) diff --git a/ui/components/app/signature-request/signature-request.component.test.js b/ui/components/app/signature-request/signature-request.component.test.js index eebae4d46..81f18932a 100644 --- a/ui/components/app/signature-request/signature-request.component.test.js +++ b/ui/components/app/signature-request/signature-request.component.test.js @@ -3,7 +3,7 @@ import { fireEvent } from '@testing-library/react'; import configureMockStore from 'redux-mock-store'; import mockState from '../../../../test/data/mock-state.json'; import { renderWithProvider } from '../../../../test/lib/render-helpers'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from '../security-provider-banner-message/security-provider-banner-message.constants'; +import { SECURITY_PROVIDER_MESSAGE_SEVERITY } from '../../../../shared/constants/security-provider'; import SignatureRequest from './signature-request.component'; const baseProps = { @@ -308,8 +308,7 @@ describe('Signature Request Component', () => { txData={{ msgParams, securityProviderResponse: { - flagAsDangerous: - SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS, }, }} unapprovedMessagesCount={2} @@ -345,8 +344,7 @@ describe('Signature Request Component', () => { txData={{ msgParams, securityProviderResponse: { - flagAsDangerous: - SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS, + flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS, }, }} unapprovedMessagesCount={2} diff --git a/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js b/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js index c25e097fc..12f07dd94 100644 --- a/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js +++ b/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js @@ -12,7 +12,6 @@ import SimulationErrorMessage from '../../../components/ui/simulation-error-mess import EditGasFeeButton from '../../../components/app/edit-gas-fee-button'; import MultiLayerFeeMessage from '../../../components/app/multilayer-fee-message'; import SecurityProviderBannerMessage from '../../../components/app/security-provider-banner-message/security-provider-banner-message'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from '../../../components/app/security-provider-banner-message/security-provider-banner-message.constants'; import { BLOCK_SIZES, JustifyContent, @@ -24,6 +23,8 @@ import { } from '../../../helpers/constants/design-system'; import { ConfirmPageContainerWarning } from '../../../components/app/confirm-page-container/confirm-page-container-content'; import LedgerInstructionField from '../../../components/app/ledger-instruction-field'; +import { isSuspiciousResponse } from '../../../../shared/modules/security-provider.utils'; + import { TokenStandard } from '../../../../shared/constants/transaction'; import { CHAIN_IDS, TEST_CHAINS } from '../../../../shared/constants/network'; import ContractDetailsModal from '../../../components/app/modals/contract-details-modal/contract-details-modal'; @@ -594,15 +595,11 @@ export default class ConfirmApproveContent extends Component { 'confirm-approve-content--full': showFullTxDetails, })} > - {(txData?.securityProviderResponse?.flagAsDangerous !== undefined && - txData?.securityProviderResponse?.flagAsDangerous !== - SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) || - (txData?.securityProviderResponse && - Object.keys(txData.securityProviderResponse).length === 0) ? ( + {isSuspiciousResponse(txData?.securityProviderResponse) && ( - ) : null} + )} {warning && (
diff --git a/ui/pages/token-allowance/token-allowance.js b/ui/pages/token-allowance/token-allowance.js index a6634ed8f..28fef706f 100644 --- a/ui/pages/token-allowance/token-allowance.js +++ b/ui/pages/token-allowance/token-allowance.js @@ -57,11 +57,11 @@ import { MAX_TOKEN_ALLOWANCE_AMOUNT, NUM_W_OPT_DECIMAL_COMMA_OR_DOT_REGEX, } from '../../../shared/constants/tokens'; +import { isSuspiciousResponse } from '../../../shared/modules/security-provider.utils'; import { ConfirmPageContainerNavigation } from '../../components/app/confirm-page-container'; import { useSimulationFailureWarning } from '../../hooks/useSimulationFailureWarning'; import SimulationErrorMessage from '../../components/ui/simulation-error-message'; import LedgerInstructionField from '../../components/app/ledger-instruction-field/ledger-instruction-field'; -import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from '../../components/app/security-provider-banner-message/security-provider-banner-message.constants'; import SecurityProviderBannerMessage from '../../components/app/security-provider-banner-message/security-provider-banner-message'; import { Text, Icon, IconName } from '../../components/component-library'; @@ -273,15 +273,11 @@ export default function TokenAllowance({ - {(txData?.securityProviderResponse?.flagAsDangerous !== undefined && - txData?.securityProviderResponse?.flagAsDangerous !== - SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) || - (txData?.securityProviderResponse && - Object.keys(txData.securityProviderResponse).length === 0) ? ( + {isSuspiciousResponse(txData?.securityProviderResponse) && ( - ) : null} + )}