Adding blockaid alert on confirmation pages

MetaMask · Jul 26, 2023 · 6603bf7 · 6603bf7
1 parent 1fbd8bc
commit 6603bf7
Show file tree

Hide file tree

Showing 19 changed files with 227 additions and 51 deletions.
diff --git a/app/scripts/lib/ppom/ppom-middleware.test.ts b/app/scripts/lib/ppom/ppom-middleware.test.ts
@@ -30,9 +30,12 @@ describe('PPOMMiddleware', () => {
       usePPOM: async () => Promise.resolve('VALIDATION_RESULT'),
     };
     const middlewareFunction = createPPOMMiddleware(controller as any);
-    const req = { method: 'eth_sendTransaction', ppomResponse: undefined };
+    const req = {
+      method: 'eth_sendTransaction',
+      securityAlertResponse: undefined,
+    };
     await middlewareFunction(req, undefined, () => undefined);
-    expect(req.ppomResponse).toBeDefined();
+    expect(req.securityAlertResponse).toBeDefined();
   });
 
   it('should call next method when ppomController.usePPOM completes', async () => {

diff --git a/package.json b/package.json
@@ -203,7 +203,7 @@
   },
   "dependencies": {
     "@babel/runtime": "^7.18.9",
-    "@blockaid/ppom": "^0.1.2",
+    "@blockaid/ppom": "^1.0.1",
     "@download/blockies": "^1.0.3",
     "@ensdomains/content-hash": "^2.5.6",
     "@ethereumjs/common": "^3.1.1",

diff --git a/...page-container/confirm-page-container-content/confirm-page-container-content.component.js b/...page-container/confirm-page-container-content/confirm-page-container-content.component.js
@@ -13,6 +13,9 @@ import Typography from '../../../ui/typography';
 import { TypographyVariant } from '../../../../helpers/constants/design-system';
 
 import { isSuspiciousResponse } from '../../../../../shared/modules/security-provider.utils';
+///: BEGIN:ONLY_INCLUDE_IN(blockaid)
+import BlockaidBannerAlert from '../../security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert';
+///: END:ONLY_INCLUDE_IN
 import SecurityProviderBannerMessage from '../../security-provider-banner-message/security-provider-banner-message';
 
 import { ConfirmPageContainerSummary, ConfirmPageContainerWarning } from '.';
@@ -222,6 +225,13 @@ export default class ConfirmPageContainerContent extends Component {
         {ethGasPriceWarning && (
           <ConfirmPageContainerWarning warning={ethGasPriceWarning} />
         )}
+        {
+          ///: BEGIN:ONLY_INCLUDE_IN(blockaid)
+          <BlockaidBannerAlert
+            securityAlertResponse={txData?.securityAlertResponse}
+          />
+          ///: END:ONLY_INCLUDE_IN
+        }
         {isSuspiciousResponse(txData?.securityProviderResponse) && (
           <SecurityProviderBannerMessage
             securityProviderResponse={txData.securityProviderResponse}

diff --git a/...container/confirm-page-container-content/confirm-page-container-content.component.test.js b/...container/confirm-page-container-content/confirm-page-container-content.component.test.js
@@ -197,4 +197,26 @@ describe('Confirm Page Container Content', () => {
     );
     expect(getByRole('button', { name: 'Buy' })).toBeInTheDocument();
   });
+
+  it('should display security alert if present', () => {
+    const { getByText } = renderWithProvider(
+      <ConfirmPageContainerContent
+        {...props}
+        txData={{
+          securityAlertResponse: {
+            resultType: 'Malicious',
+            reason: 'blur_farming',
+            description:
+              'A SetApprovalForAll request was made on {contract}. We found the operator {operator} to be malicious',
+            args: {
+              contract: '0xa7206d878c5c3871826dfdb42191c49b1d11f466',
+              operator: '0x92a3b9773b1763efa556f55ccbeb20441962d9b2',
+            },
+          },
+        }}
+      />,
+      store,
+    );
+    expect(getByText('This is a deceptive request')).toBeInTheDocument();
+  });
 });
diff --git a/.../security-provider-banner-alert/__snapshots__/security-provider-banner-alert.test.js.snap b/.../security-provider-banner-alert/__snapshots__/security-provider-banner-alert.test.js.snap
@@ -3,7 +3,7 @@
 exports[`Security Provider Banner Alert should match snapshot 1`] = `
 <div>
   <div
-    class="box mm-banner-base mm-banner-alert mm-banner-alert--severity-danger box--margin-top-4 box--margin-right-4 box--margin-left-4 box--padding-3 box--padding-left-2 box--display-flex box--gap-2 box--flex-direction-row box--background-color-error-muted box--rounded-sm"
+    class="box mm-banner-base mm-banner-alert mm-banner-alert--severity-danger box--margin-4 box--padding-3 box--padding-left-2 box--display-flex box--gap-2 box--flex-direction-row box--background-color-error-muted box--rounded-sm"
   >
     <span
       class="mm-box mm-icon mm-icon--size-lg mm-box--display-inline-block mm-box--color-error-default"

diff --git a/...vider-banner-alert/blockaid-banner-alert/__snapshots__/blockaid-banner-alert.test.js.snap b/...vider-banner-alert/blockaid-banner-alert/__snapshots__/blockaid-banner-alert.test.js.snap
@@ -1,8 +1,8 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Blockaid Banner Alert should render 'danger' UI when ppomResponse.resultType is 'Malicious 1`] = `
+exports[`Blockaid Banner Alert should render 'danger' UI when securityAlertResponse.resultType is 'Malicious 1`] = `
 <div
-  class="box mm-banner-base mm-banner-alert mm-banner-alert--severity-danger box--margin-top-4 box--margin-right-4 box--margin-left-4 box--padding-3 box--padding-left-2 box--display-flex box--gap-2 box--flex-direction-row box--background-color-error-muted box--rounded-sm"
+  class="box mm-banner-base mm-banner-alert mm-banner-alert--severity-danger box--margin-4 box--padding-3 box--padding-left-2 box--display-flex box--gap-2 box--flex-direction-row box--background-color-error-muted box--rounded-sm"
 >
   <span
     class="mm-box mm-icon mm-icon--size-lg mm-box--display-inline-block mm-box--color-error-default"
@@ -46,9 +46,9 @@ exports[`Blockaid Banner Alert should render 'danger' UI when ppomResponse.resul
 </div>
 `;
 
-exports[`Blockaid Banner Alert should render 'warning' UI when ppomResponse.resultType is 'Warning 1`] = `
+exports[`Blockaid Banner Alert should render 'warning' UI when securityAlertResponse.resultType is 'Warning 1`] = `
 <div
-  class="box mm-banner-base mm-banner-alert mm-banner-alert--severity-warning box--margin-top-4 box--margin-right-4 box--margin-left-4 box--padding-3 box--padding-left-2 box--display-flex box--gap-2 box--flex-direction-row box--background-color-warning-muted box--rounded-sm"
+  class="box mm-banner-base mm-banner-alert mm-banner-alert--severity-warning box--margin-4 box--padding-3 box--padding-left-2 box--display-flex box--gap-2 box--flex-direction-row box--background-color-warning-muted box--rounded-sm"
 >
   <span
     class="mm-box mm-icon mm-icon--size-lg mm-box--display-inline-block mm-box--color-warning-default"
@@ -95,7 +95,7 @@ exports[`Blockaid Banner Alert should render 'warning' UI when ppomResponse.resu
 exports[`Blockaid Banner Alert should render details when provided 1`] = `
 <div>
   <div
-    class="box mm-banner-base mm-banner-alert mm-banner-alert--severity-warning box--margin-top-4 box--margin-right-4 box--margin-left-4 box--padding-3 box--padding-left-2 box--display-flex box--gap-2 box--flex-direction-row box--background-color-warning-muted box--rounded-sm"
+    class="box mm-banner-base mm-banner-alert mm-banner-alert--severity-warning box--margin-4 box--padding-3 box--padding-left-2 box--display-flex box--gap-2 box--flex-direction-row box--background-color-warning-muted box--rounded-sm"
   >
     <span
       class="mm-box mm-icon mm-icon--size-lg mm-box--display-inline-block mm-box--color-warning-default"

diff --git a/...ponents/app/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.js b/...ponents/app/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.js
@@ -39,11 +39,15 @@ const REASON_TO_DESCRIPTION_TKEY = Object.freeze({
 /** List of suspicious reason(s). Other reasons will be deemed as deceptive. */
 const SUSPCIOUS_REASON = [BlockaidReason.rawSignatureFarming];
 
-function BlockaidBannerAlert({
-  ppomResponse: { reason, resultType, features },
-}) {
+function BlockaidBannerAlert({ securityAlertResponse }) {
   const t = useContext(I18nContext);
 
+  if (!securityAlertResponse) {
+    return null;
+  }
+
+  const { reason, resultType, features } = securityAlertResponse;
+
   if (resultType === BlockaidResultType.Benign) {
     return null;
   }
@@ -84,7 +88,7 @@ function BlockaidBannerAlert({
 }
 
 BlockaidBannerAlert.propTypes = {
-  ppomResponse: PropTypes.object,
+  securityAlertResponse: PropTypes.object,
 };
 
 export default BlockaidBannerAlert;
diff --git a/...app/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.stories.js b/...app/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.stories.js
@@ -16,17 +16,17 @@ export default {
     features: {
       control: 'array',
       description:
-        'ppomResponse.features value which is a list displayed as SecurityProviderBannerAlert details',
+        'securityAlertResponse.features value which is a list displayed as SecurityProviderBannerAlert details',
     },
     reason: {
       control: 'select',
       options: Object.values(BlockaidReason),
-      description: 'ppomResponse.reason value',
+      description: 'securityAlertResponse.reason value',
     },
     resultType: {
       control: 'select',
       options: Object.values(BlockaidResultType),
-      description: 'ppomResponse.resultType value',
+      description: 'securityAlertResponse.resultType value',
     },
   },
   args: {
@@ -37,6 +37,6 @@ export default {
 };
 
 export const DefaultStory = (args) => (
-  <BlockaidBannerAlert ppomResponse={args} />
+  <BlockaidBannerAlert securityAlertResponse={args} />
 );
 DefaultStory.storyName = 'Default';
diff --git a/...ts/app/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.test.js b/...ts/app/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.test.js
@@ -7,7 +7,7 @@ import {
 } from '../../../../../shared/constants/security-provider';
 import BlockaidBannerAlert from '.';
 
-const mockPpomResponse = {
+const mockSecurityAlertResponse = {
   resultType: BlockaidResultType.Warning,
   reason: BlockaidReason.setApprovalForAll,
   description:
@@ -19,11 +19,19 @@ const mockPpomResponse = {
 };
 
 describe('Blockaid Banner Alert', () => {
-  it(`should not render when ppomResponse.resultType is '${BlockaidResultType.Benign}'`, () => {
+  it('should not render when securityAlertResponse is not present', () => {
+    const { container } = renderWithLocalization(
+      <BlockaidBannerAlert securityAlertResponse={undefined} />,
+    );
+
+    expect(container.querySelector('.mm-banner-alert')).toBeNull();
+  });
+
+  it(`should not render when securityAlertResponse.resultType is '${BlockaidResultType.Benign}'`, () => {
     const { container } = renderWithLocalization(
       <BlockaidBannerAlert
-        ppomResponse={{
-          ...mockPpomResponse,
+        securityAlertResponse={{
+          ...mockSecurityAlertResponse,
           resultType: BlockaidResultType.Benign,
         }}
       />,
@@ -32,11 +40,11 @@ describe('Blockaid Banner Alert', () => {
     expect(container.querySelector('.mm-banner-alert')).toBeNull();
   });
 
-  it(`should render '${Severity.Danger}' UI when ppomResponse.resultType is '${BlockaidResultType.Malicious}`, () => {
+  it(`should render '${Severity.Danger}' UI when securityAlertResponse.resultType is '${BlockaidResultType.Malicious}`, () => {
     const { container } = renderWithLocalization(
       <BlockaidBannerAlert
-        ppomResponse={{
-          ...mockPpomResponse,
+        securityAlertResponse={{
+          ...mockSecurityAlertResponse,
           resultType: BlockaidResultType.Malicious,
         }}
       />,
@@ -49,9 +57,9 @@ describe('Blockaid Banner Alert', () => {
     expect(dangerBannerAlert).toMatchSnapshot();
   });
 
-  it(`should render '${Severity.Warning}' UI when ppomResponse.resultType is '${BlockaidResultType.Warning}`, () => {
+  it(`should render '${Severity.Warning}' UI when securityAlertResponse.resultType is '${BlockaidResultType.Warning}`, () => {
     const { container } = renderWithLocalization(
-      <BlockaidBannerAlert ppomResponse={mockPpomResponse} />,
+      <BlockaidBannerAlert securityAlertResponse={mockSecurityAlertResponse} />,
     );
     const warningBannerAlert = container.querySelector(
       '.mm-banner-alert--severity-warning',
@@ -63,7 +71,7 @@ describe('Blockaid Banner Alert', () => {
 
   it('should render title, "This is a deceptive request"', () => {
     const { getByText } = renderWithLocalization(
-      <BlockaidBannerAlert ppomResponse={mockPpomResponse} />,
+      <BlockaidBannerAlert securityAlertResponse={mockSecurityAlertResponse} />,
     );
 
     expect(getByText('This is a deceptive request')).toBeInTheDocument();
@@ -72,8 +80,8 @@ describe('Blockaid Banner Alert', () => {
   it('should render title, "This is a suspicious request", when the reason is "raw_signature_farming"', () => {
     const { getByText } = renderWithLocalization(
       <BlockaidBannerAlert
-        ppomResponse={{
-          ...mockPpomResponse,
+        securityAlertResponse={{
+          ...mockSecurityAlertResponse,
           reason: BlockaidReason.rawSignatureFarming,
         }}
       />,
@@ -90,7 +98,10 @@ describe('Blockaid Banner Alert', () => {
 
     const { container, getByText } = renderWithLocalization(
       <BlockaidBannerAlert
-        ppomResponse={{ ...mockPpomResponse, features: mockFeatures }}
+        securityAlertResponse={{
+          ...mockSecurityAlertResponse,
+          features: mockFeatures,
+        }}
       />,
     );
 
@@ -133,7 +144,7 @@ describe('Blockaid Banner Alert', () => {
       it(`should render for '${reason}' correctly`, () => {
         const { getByText } = renderWithLocalization(
           <BlockaidBannerAlert
-            ppomResponse={{ ...mockPpomResponse, reason }}
+            securityAlertResponse={{ ...mockSecurityAlertResponse, reason }}
           />,
         );
 

diff --git a/ui/components/app/security-provider-banner-alert/security-provider-banner-alert.js b/ui/components/app/security-provider-banner-alert/security-provider-banner-alert.js
@@ -36,13 +36,7 @@ function SecurityProviderBannerAlert({
   const t = useContext(I18nContext);
 
   return (
-    <BannerAlert
-      title={title}
-      severity={severity}
-      marginTop={4}
-      marginRight={4}
-      marginLeft={4}
-    >
+    <BannerAlert title={title} severity={severity} margin={4}>
       <Text marginTop={2}>{description}</Text>
 
       {details && (

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
@@ -42,6 +42,9 @@ import {
   Text,
   ///: END:ONLY_INCLUDE_IN
 } from '../../component-library';
+///: BEGIN:ONLY_INCLUDE_IN(blockaid)
+import BlockaidBannerAlert from '../security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert';
+///: END:ONLY_INCLUDE_IN
 ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)
 import Box from '../../ui/box/box';
 ///: END:ONLY_INCLUDE_IN
@@ -150,12 +153,18 @@ export default class SignatureRequestOriginal extends Component {
 
     return (
       <div className="request-signature__body">
+        {
+          ///: BEGIN:ONLY_INCLUDE_IN(blockaid)
+          <BlockaidBannerAlert
+            securityAlertResponse={txData?.securityAlertResponse}
+          />
+          ///: END:ONLY_INCLUDE_IN
+        }
         {isSuspiciousResponse(txData?.securityProviderResponse) && (
           <SecurityProviderBannerMessage
             securityProviderResponse={txData.securityProviderResponse}
           />
         )}
-
         {
           ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)
           this.props.selectedAccount.address ===
@@ -183,7 +192,6 @@ export default class SignatureRequestOriginal extends Component {
           )
           ///: END:ONLY_INCLUDE_IN
         }
-
         <div className="request-signature__origin">
           {
             // Use legacy authorship header for snaps
@@ -211,7 +219,6 @@ export default class SignatureRequestOriginal extends Component {
             ///: END:ONLY_INCLUDE_IN
           }
         </div>
-
         <Typography
           className="request-signature__content__title"
           variant={TypographyVariant.H3}
@@ -229,7 +236,6 @@ export default class SignatureRequestOriginal extends Component {
         >
           {this.context.t('signatureRequestGuidance')}
         </Typography>
-
         <div className={classnames('request-signature__notice')}>{notice}</div>
         <div className="request-signature__rows">
           {rows.map(({ name, value }, index) => {

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
@@ -180,4 +180,20 @@ describe('SignatureRequestOriginal', () => {
     ).toBeNull();
     expect(screen.queryByText('OpenSea')).toBeNull();
   });
+
+  it('should display security alert if present', () => {
+    props.txData.securityAlertResponse = {
+      resultType: 'Malicious',
+      reason: 'blur_farming',
+      description:
+        'A SetApprovalForAll request was made on {contract}. We found the operator {operator} to be malicious',
+      args: {
+        contract: '0xa7206d878c5c3871826dfdb42191c49b1d11f466',
+        operator: '0x92a3b9773b1763efa556f55ccbeb20441962d9b2',
+      },
+    };
+
+    render();
+    expect(screen.getByText('This is a deceptive request')).toBeInTheDocument();
+  });
 });