Security Provider cleanup (#19694)

* security-prov: isFlaggedSecurityProviderResponse

* security-prov: create shared/modules/util

* security prov: rn isFlagged -> isSuspcious
- util fn returns true if response is not verified and flagged

* security prov: add util test
and support undefined param

* security prov: reorg util fn
- no logic changes

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,
+};

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();
+    });
+  });
+});

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<string, Json> | 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;
+}

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 && (
           <ConfirmPageContainerWarning warning={ethGasPriceWarning} />
         )}
-        {(txData?.securityProviderResponse?.flagAsDangerous !== undefined &&
+        {isSuspiciousResponse(txData?.securityProviderResponse) && (
-          txData?.securityProviderResponse?.flagAsDangerous !==
-            SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) ||
-        (txData?.securityProviderResponse &&
-          Object.keys(txData.securityProviderResponse).length === 0) ? (
           <SecurityProviderBannerMessage
             securityProviderResponse={txData.securityProviderResponse}
           />
-        ) : null}
+        )}
         <ConfirmPageContainerSummary
           className={classnames({
             'confirm-page-container-summary--border':

ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.test.js

@@ -1,13 +1,13 @@
 import { fireEvent } from '@testing-library/react';
 import React from 'react';
 import configureMockStore from 'redux-mock-store';
+import { SECURITY_PROVIDER_MESSAGE_SEVERITY } from '../../../../../shared/constants/security-provider';
 import { TransactionType } from '../../../../../shared/constants/transaction';
 import { renderWithProvider } from '../../../../../test/lib/render-helpers';
 import {
   INSUFFICIENT_FUNDS_ERROR_KEY,
   TRANSACTION_ERROR_KEY,
 } from '../../../../helpers/constants/error-keys';
-import { SECURITY_PROVIDER_MESSAGE_SEVERITIES } from '../../security-provider-banner-message/security-provider-banner-message.constants';
 import ConfirmPageContainerContent from './confirm-page-container-content.component';
 
 describe('Confirm Page Container Content', () => {
@@ -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(

ui/components/app/security-provider-banner-message/security-provider-banner-message.constants.js

@@ -1,5 +0,0 @@
-export const SECURITY_PROVIDER_MESSAGE_SEVERITIES = {
-  NOT_MALICIOUS: 0,
-  MALICIOUS: 1,
-  NOT_SAFE: 2,
-};

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');

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: '',
     };

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 (
       <div className="request-signature__body">
-        {(txData?.securityProviderResponse?.flagAsDangerous !== undefined &&
+        {isSuspiciousResponse(txData?.securityProviderResponse) && (
-          txData?.securityProviderResponse?.flagAsDangerous !==
-            SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) ||
-        (txData?.securityProviderResponse &&
-          Object.keys(txData.securityProviderResponse).length === 0) ? (
           <SecurityProviderBannerMessage
             securityProviderResponse={txData.securityProviderResponse}
           />
-        ) : null}
+        )}
 
         {
           ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)

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();

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 =
+  const showSecurityProviderBanner = isSuspiciousResponse(
-    (txData?.securityProviderResponse?.flagAsDangerous !== undefined &&
+    txData?.securityProviderResponse,
-      txData?.securityProviderResponse?.flagAsDangerous !==
+  );
-        SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) ||
-    (txData?.securityProviderResponse &&
-      Object.keys(txData.securityProviderResponse).length === 0);
 
   const onSign = useCallback(async () => {
     try {

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 {
           />
         </div>
         <div className="signature-request-content">
-          {(txData?.securityProviderResponse?.flagAsDangerous !== undefined &&
+          {isSuspiciousResponse(txData?.securityProviderResponse) && (
-            txData?.securityProviderResponse?.flagAsDangerous !==
-              SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) ||
-          (txData?.securityProviderResponse &&
-            Object.keys(txData.securityProviderResponse).length === 0) ? (
             <SecurityProviderBannerMessage
               securityProviderResponse={txData.securityProviderResponse}
             />
-          ) : null}
+          )}
 
           {
             ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)

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:
+              flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS,
-                SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS,
             },
           }}
           unapprovedMessagesCount={2}
@@ -345,8 +344,7 @@ describe('Signature Request Component', () => {
           txData={{
             msgParams,
             securityProviderResponse: {
-              flagAsDangerous:
+              flagAsDangerous: SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS,
-                SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS,
             },
           }}
           unapprovedMessagesCount={2}

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 &&
+        {isSuspiciousResponse(txData?.securityProviderResponse) && (
-          txData?.securityProviderResponse?.flagAsDangerous !==
-            SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) ||
-        (txData?.securityProviderResponse &&
-          Object.keys(txData.securityProviderResponse).length === 0) ? (
           <SecurityProviderBannerMessage
             securityProviderResponse={txData.securityProviderResponse}
           />
-        ) : null}
+        )}
         {warning && (
           <div className="confirm-approve-content__custom-nonce-warning">
             <ConfirmPageContainerWarning warning={warning} />

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({
       <Box>
         <ConfirmPageContainerNavigation />
       </Box>
-      {(txData?.securityProviderResponse?.flagAsDangerous !== undefined &&
+      {isSuspiciousResponse(txData?.securityProviderResponse) && (
-        txData?.securityProviderResponse?.flagAsDangerous !==
-          SECURITY_PROVIDER_MESSAGE_SEVERITIES.NOT_MALICIOUS) ||
-      (txData?.securityProviderResponse &&
-        Object.keys(txData.securityProviderResponse).length === 0) ? (
         <SecurityProviderBannerMessage
           securityProviderResponse={txData.securityProviderResponse}
         />
-      ) : null}
+      )}
       <Box
         paddingLeft={4}
         paddingRight={4}