From d47ae272c92a19952232f9de4d37eeb5e0ff0003 Mon Sep 17 00:00:00 2001 From: Hassan Malik <41640681+hmalik88@users.noreply.github.com> Date: Tue, 13 Feb 2024 10:43:02 -0500 Subject: [PATCH] [FLASK] Add signature insights (#22485) Per [SIP-16](https://github.com/MetaMask/SIPs/blob/main/SIPS/sip-16.md), the signature insights implementation is being integrated. Closes https://github.com/MetaMask/MetaMask-planning/issues/1778 Demo: https://github.com/MetaMask/metamask-extension/assets/41640681/a238e720-c614-4be8-b272-5257c1ccfad2 --- app/_locales/en/messages.json | 46 ++- package.json | 2 +- shared/constants/permissions.test.js | 1 + shared/constants/snaps/permissions.ts | 5 +- ui/components/app/app-components.scss | 2 +- .../app/snaps/insight-warnings/index.js | 1 + .../app/snaps/insight-warnings/index.scss | 6 + .../insight-warnings.js} | 44 ++- .../app/snaps/snap-delineator/index.scss | 5 + .../snaps/snap-delineator/snap-delineator.js | 10 +- .../app/snaps/tx-insight-warnings/index.js | 1 - .../app/snaps/tx-insight-warnings/index.scss | 5 - ui/helpers/constants/snaps/index.js | 1 + ui/helpers/constants/snaps/insights.ts | 21 ++ ui/helpers/utils/permission.js | 38 ++ ui/hooks/snaps/useSignatureInsights.js | 113 ++++++ .../confirm-page-container.component.js | 4 +- .../signature-request-original.component.js | 67 +++- .../signature-request-siwe.js | 278 +++++++++------ .../signature-request/signature-request.js | 327 ++++++++++-------- .../confirm-signature-request/index.js | 9 + .../confirm-signature-request/index.test.js | 73 ++++ .../snap-account-redirect.test.js.snap | 6 +- .../create-snap-redirect.test.tsx.snap | 6 +- ui/selectors/selectors.js | 15 + yarn.lock | 10 +- 26 files changed, 767 insertions(+), 329 deletions(-) create mode 100644 ui/components/app/snaps/insight-warnings/index.js create mode 100644 ui/components/app/snaps/insight-warnings/index.scss rename ui/components/app/snaps/{tx-insight-warnings/tx-insight-warnings.js => insight-warnings/insight-warnings.js} (82%) delete mode 100644 ui/components/app/snaps/tx-insight-warnings/index.js delete mode 100644 ui/components/app/snaps/tx-insight-warnings/index.scss create mode 100644 ui/helpers/constants/snaps/insights.ts create mode 100644 ui/hooks/snaps/useSignatureInsights.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 3af1c1b4e964..fdf7b19c68b5 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -2082,6 +2082,21 @@ "inputLogicHigherNumber": { "message": "This allows the third party to spend all your token balance until it reaches the cap or you revoke the spending cap. If this is not intended, consider setting a lower spending cap." }, + "insightWarningCheckboxMessage": { + "message": "$1 the request by $2", + "description": "$1 is the action i.e. sign, confirm. $2 is the origin making the request." + }, + "insightWarningContentPlural": { + "message": "Review $1 warnings before $2. Once made, your $3 is irreversible.", + "description": "$1 is the number of warnings, $2 is the action (i.e. signing) and $3 is the result (i.e. signature, transaction)" + }, + "insightWarningContentSingular": { + "message": "Review 1 warning before $1. Once made, your $2 is irreversible.", + "description": "$1 is the action (i.e. signing) and $2 is the result (i.e. signature, transaction)" + }, + "insightWarningHeader": { + "message": "This request may be risky" + }, "insightsFromSnap": { "message": "Insights from $1", "description": "$1 represents the name of the snap" @@ -3603,6 +3618,22 @@ "message": "$1 and $2", "description": "A list of allowed origins where $2 is the last origin of the list and $1 is the rest of the list separated by ','." }, + "permission_signatureInsight": { + "message": "Display signature insights modal.", + "description": "The description for the `endowment:signature-insight` permission" + }, + "permission_signatureInsightDescription": { + "message": "Allow $1 to display a modal with insights on any signature request before approval. This can be used for anti-phishing and security solutions.", + "description": "An extended description for the `endowment:signature-insight` permission. $1 is the snap name." + }, + "permission_signatureInsightOrigin": { + "message": "See the origins of websites that initiate a signature request", + "description": "The description for the `signatureOrigin` caveat, to be used with the `endowment:signature-insight` permission" + }, + "permission_signatureInsightOriginDescription": { + "message": "Allow $1 to see the origin (URI) of websites that initiate signature requests. This can be used for anti-phishing and security solutions.", + "description": "An extended description for the `signatureOrigin` caveat, to be used with the `endowment:signature-insight` permission. $1 is the snap name." + }, "permission_transactionInsight": { "message": "Fetch and display transaction insights.", "description": "The description for the `endowment:transaction-insight` permission" @@ -5492,21 +5523,6 @@ "transactionHistoryTotalGasFee": { "message": "Total gas fee" }, - "transactionInsightWarningCheckboxMessage": { - "message": "$1 the request by $2", - "description": "$1 is the action i.e. sign, confirm. $2 is the origin making the request." - }, - "transactionInsightWarningContentPlural": { - "message": "Review $1 warnings before $2. Once made, your $3 is irreversible.", - "description": "$1 is the number of warnings, $2 is the action (i.e. signing) and $3 is the result (i.e. signature, transaction)" - }, - "transactionInsightWarningContentSingular": { - "message": "Review 1 warning before $1. Once made, your $2 is irreversible.", - "description": "$1 is the action (i.e. signing) and $2 is the result (i.e. signature, transaction)" - }, - "transactionInsightWarningHeader": { - "message": "This request may be risky" - }, "transactionNote": { "message": "Transaction note" }, diff --git a/package.json b/package.json index 1b1aeb1656a0..805f5f152eb0 100644 --- a/package.json +++ b/package.json @@ -255,7 +255,7 @@ "@metamask/controller-utils": "^8.0.1", "@metamask/design-tokens": "^1.13.0", "@metamask/desktop": "^0.3.0", - "@metamask/eth-json-rpc-middleware": "^12.0.1", + "@metamask/eth-json-rpc-middleware": "^12.1.0", "@metamask/eth-keyring-controller": "^15.1.0", "@metamask/eth-ledger-bridge-keyring": "^2.0.1", "@metamask/eth-query": "^4.0.0", diff --git a/shared/constants/permissions.test.js b/shared/constants/permissions.test.js index 99afcaa9208e..a5976d058c45 100644 --- a/shared/constants/permissions.test.js +++ b/shared/constants/permissions.test.js @@ -14,6 +14,7 @@ describe('EndowmentPermissions', () => { [ 'endowment:name-lookup', 'endowment:page-home', + 'endowment:signature-insight', ...Object.keys(endowmentPermissionBuilders).filter( (targetName) => !Object.keys(ExcludedSnapEndowments).includes(targetName), diff --git a/shared/constants/snaps/permissions.ts b/shared/constants/snaps/permissions.ts index 44693e8fa09a..160a02f7507b 100644 --- a/shared/constants/snaps/permissions.ts +++ b/shared/constants/snaps/permissions.ts @@ -8,6 +8,7 @@ export const EndowmentPermissions = Object.freeze({ 'endowment:lifecycle-hooks': 'endowment:lifecycle-hooks', ///: BEGIN:ONLY_INCLUDE_IF(build-flask) 'endowment:page-home': 'endowment:page-home', + 'endowment:signature-insight': 'endowment:signature-insight', 'endowment:name-lookup': 'endowment:name-lookup', ///: END:ONLY_INCLUDE_IF ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) @@ -22,13 +23,13 @@ export const ExcludedSnapPermissions = Object.freeze({ }); export const ExcludedSnapEndowments = Object.freeze({ - 'endowment:signature-insight': - 'This endowment is in development and therefore not available.', ///: BEGIN:ONLY_INCLUDE_IF(build-main) 'endowment:page-home': 'This endowment is experimental and therefore not available.', 'endowment:name-lookup': 'This endowment is experimental and therefore not available.', + 'endowment:signature-insight': + 'This endowment is experimental and therefore not available.', ///: END:ONLY_INCLUDE_IF }); diff --git a/ui/components/app/app-components.scss b/ui/components/app/app-components.scss index fe6d538877ab..1595e3e15111 100644 --- a/ui/components/app/app-components.scss +++ b/ui/components/app/app-components.scss @@ -25,7 +25,7 @@ @import 'snaps/copyable/index'; @import 'snaps/snap-version/index'; @import 'snaps/show-more/index'; -@import 'snaps/tx-insight-warnings/index'; +@import 'snaps/insight-warnings/index'; @import 'hold-to-reveal-button/index'; @import 'home-notification/index'; @import 'info-box/index'; diff --git a/ui/components/app/snaps/insight-warnings/index.js b/ui/components/app/snaps/insight-warnings/index.js new file mode 100644 index 000000000000..c8b0def81f4e --- /dev/null +++ b/ui/components/app/snaps/insight-warnings/index.js @@ -0,0 +1 @@ +export { default } from './insight-warnings'; diff --git a/ui/components/app/snaps/insight-warnings/index.scss b/ui/components/app/snaps/insight-warnings/index.scss new file mode 100644 index 000000000000..21e229275e0c --- /dev/null +++ b/ui/components/app/snaps/insight-warnings/index.scss @@ -0,0 +1,6 @@ +.insights-warnings-modal { + &__content { + overflow-y: auto; + overflow-x: hidden; + } +} diff --git a/ui/components/app/snaps/tx-insight-warnings/tx-insight-warnings.js b/ui/components/app/snaps/insight-warnings/insight-warnings.js similarity index 82% rename from ui/components/app/snaps/tx-insight-warnings/tx-insight-warnings.js rename to ui/components/app/snaps/insight-warnings/insight-warnings.js index 63e36fdf8605..ff0e87e63d85 100644 --- a/ui/components/app/snaps/tx-insight-warnings/tx-insight-warnings.js +++ b/ui/components/app/snaps/insight-warnings/insight-warnings.js @@ -29,12 +29,15 @@ import { } from '../../../../helpers/constants/design-system'; import { useI18nContext } from '../../../../hooks/useI18nContext'; import { SnapUIRenderer } from '../snap-ui-renderer'; -import { DelineatorType } from '../../../../helpers/constants/snaps'; +import { + DelineatorType, + InsightWarningLanguage, +} from '../../../../helpers/constants/snaps'; import { stripHttpSchemes } from '../../../../helpers/utils/util'; -export default function TxInsightWarnings({ +export default function InsightWarnings({ warnings, - type = 'confirming', + action = 'confirming', origin, onCancel, onSubmit, @@ -67,7 +70,7 @@ export default function TxInsightWarnings({ const Warnings = () => { const lastWarningIdx = warnings.length - 1; return ( - + {warnings.map((warning, idx) => { const { snapId, content } = warning; return ( @@ -87,19 +90,12 @@ export default function TxInsightWarnings({ ); }; - // move this to an enum that defines the language to be used in this modal on the various - // screens that will offer transaction insights, should be indexed on "type". - const results = { - confirming: { noun: 'confirmation', imperative: 'confirm' }, - signing: { noun: 'signature', imperative: 'sign' }, - }; - return ( @@ -122,19 +118,19 @@ export default function TxInsightWarnings({ paddingTop={4} paddingBottom={4} > - {t('transactionInsightWarningHeader')} + {t('insightWarningHeader')} {warnings.length === 1 - ? t('transactionInsightWarningContentSingular', [ - type, - results[type].noun, + ? t('insightWarningContentSingular', [ + action, + InsightWarningLanguage[action].noun, ]) - : t('transactionInsightWarningContentPlural', [ + : t('insightWarningContentPlural', [ warnings.length, - type, - results[type].noun, + action, + InsightWarningLanguage[action].noun, ])} @@ -156,8 +152,8 @@ export default function TxInsightWarnings({ variant={TextVariant.bodySm} isChecked={isChecked} onChange={handleOnChange} - label={t('transactionInsightWarningCheckboxMessage', [ - t(results[type].imperative), + label={t('insightWarningCheckboxMessage', [ + t(InsightWarningLanguage[action].imperative), stripHttpSchemes(origin), ])} /> @@ -182,7 +178,7 @@ export default function TxInsightWarnings({ onClick={onSubmit} disabled={!isChecked} > - {t(results[type].imperative)} + {t(InsightWarningLanguage[action].imperative)} @@ -190,7 +186,7 @@ export default function TxInsightWarnings({ ); } -TxInsightWarnings.propTypes = { +InsightWarnings.propTypes = { /** * An array of warnings returned from tx-insight snaps that deem their content 'critical' */ @@ -198,7 +194,7 @@ TxInsightWarnings.propTypes = { /** * A limited set of actions defining the type of transaction */ - type: PropTypes.oneOf(['confirming', 'signing']), + action: PropTypes.oneOf(Object.keys(InsightWarningLanguage)), /** * Origin initiating the transaction */ diff --git a/ui/components/app/snaps/snap-delineator/index.scss b/ui/components/app/snaps/snap-delineator/index.scss index 9876d5dc6540..c296bf09c2d1 100644 --- a/ui/components/app/snaps/snap-delineator/index.scss +++ b/ui/components/app/snaps/snap-delineator/index.scss @@ -8,8 +8,13 @@ &__text { white-space: nowrap; text-overflow: ellipsis; + width: calc(100% - 16px); overflow: hidden; } + + &__container { + width: calc(100% - 16px); + } } &__expansion-icon { diff --git a/ui/components/app/snaps/snap-delineator/snap-delineator.js b/ui/components/app/snaps/snap-delineator/snap-delineator.js index d19a37ebad25..d933c9cc227a 100644 --- a/ui/components/app/snaps/snap-delineator/snap-delineator.js +++ b/ui/components/app/snaps/snap-delineator/snap-delineator.js @@ -65,9 +65,14 @@ export const SnapDelineator = ({ padding={1} style={{ borderBottomWidth: isCollapsed ? 0 : 1 }} > - + {t(getDelineatorTitle(type), [snapName])} @@ -97,6 +104,7 @@ export const SnapDelineator = ({ /> )} + { + const baseDescription = { + leftIcon: IconName.Warning, + weight: 3, + }; + + const result = [ + { + ...baseDescription, + label: t('permission_signatureInsight'), + description: t('permission_signatureInsightDescription', [ + getSnapNameComponent(targetSubjectMetadata), + ]), + }, + ]; + + if ( + isNonEmptyArray(permissionValue.caveats) && + permissionValue.caveats.find((caveat) => { + return caveat.type === SnapCaveatType.SignatureOrigin && caveat.value; + }) + ) { + result.push({ + ...baseDescription, + label: t('permission_signatureInsightOrigin'), + description: t('permission_signatureInsightOriginDescription', [ + getSnapNameComponent(targetSubjectMetadata), + ]), + leftIcon: IconName.Explore, + }); + } + + return result; + }, ///: END:ONLY_INCLUDE_IF [UNKNOWN_PERMISSION]: ({ t, permissionName }) => ({ label: t('permission_unknown', [permissionName ?? 'undefined']), diff --git a/ui/hooks/snaps/useSignatureInsights.js b/ui/hooks/snaps/useSignatureInsights.js new file mode 100644 index 000000000000..9c0a418613ea --- /dev/null +++ b/ui/hooks/snaps/useSignatureInsights.js @@ -0,0 +1,113 @@ +import { useEffect, useState } from 'react'; +import { useSelector } from 'react-redux'; +import { getSignatureOriginCaveat } from '@metamask/snaps-controllers'; +import { SeverityLevel } from '@metamask/snaps-sdk'; +import { handleSnapRequest } from '../../store/actions'; +import { + getSignatureInsightSnapIds, + getPermissionSubjectsDeepEqual, +} from '../../selectors'; + +const SIGNATURE_INSIGHT_PERMISSION = 'endowment:signature-insight'; + +export function useSignatureInsights({ txData }) { + const subjects = useSelector(getPermissionSubjectsDeepEqual); + const snapIds = useSelector(getSignatureInsightSnapIds); + const [loading, setLoading] = useState(true); + const [data, setData] = useState(undefined); + const [warnings, setWarnings] = useState([]); + + useEffect(() => { + let cancelled = false; + + async function fetchInsight() { + setLoading(true); + + const { + msgParams: { from, data: msgData, signatureMethod, origin }, + } = txData; + + /** + * Both eth_signTypedData_v3 and eth_signTypedData_v4 methods + * need to be parsed because their data is stringified. All other + * signature methods do not, so they are ignored. + */ + const shouldParse = + signatureMethod === 'eth_signTypedData_v3' || + signatureMethod === 'eth_signTypedData_v4'; + + const signature = { + from, + data: shouldParse ? JSON.parse(msgData) : msgData, + signatureMethod, + }; + + const newData = await Promise.allSettled( + snapIds.map((snapId) => { + const permission = + subjects[snapId]?.permissions[SIGNATURE_INSIGHT_PERMISSION]; + if (!permission) { + return Promise.reject( + new Error( + 'This Snap does not have the signature insight endowment.', + ), + ); + } + + const hasSignatureOriginCaveat = getSignatureOriginCaveat(permission); + const signatureOrigin = hasSignatureOriginCaveat ? origin : null; + return handleSnapRequest({ + snapId, + origin: '', + handler: 'onSignature', + request: { + jsonrpc: '2.0', + method: '', + params: { signature, signatureOrigin }, + }, + }); + }), + ); + + const reformattedData = newData.map((promise, idx) => { + const snapId = snapIds[idx]; + if (promise.status === 'rejected') { + return { + error: promise.reason, + snapId, + }; + } + return { + snapId, + response: promise.value, + }; + }); + + const insightWarnings = reformattedData.reduce((warningsArr, promise) => { + if (promise.response?.severity === SeverityLevel.Critical) { + const { + snapId, + response: { content }, + } = promise; + warningsArr.push({ snapId, content }); + } + return warningsArr; + }, []); + + if (!cancelled) { + setData(reformattedData); + setWarnings(insightWarnings); + setLoading(false); + } + } + if (Object.keys(txData).length > 0) { + fetchInsight(); + } + return () => { + cancelled = true; + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [txData, JSON.stringify(snapIds), subjects]); + + return { data, loading, warnings }; +} 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 87b15c0b0d92..0b77d2708202 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 @@ -41,7 +41,7 @@ import { useI18nContext } from '../../../../hooks/useI18nContext'; import useTransactionInsights from '../../../../hooks/useTransactionInsights'; ///: END:ONLY_INCLUDE_IF ///: BEGIN:ONLY_INCLUDE_IF(build-flask) -import TxInsightWarnings from '../../../../components/app/snaps/tx-insight-warnings/tx-insight-warnings'; +import InsightWarnings from '../../../../components/app/snaps/insight-warnings'; ///: END:ONLY_INCLUDE_IF import { getAccountName, @@ -378,7 +378,7 @@ const ConfirmPageContainer = (props) => { ///: BEGIN:ONLY_INCLUDE_IF(build-flask) } {isShowingTxInsightWarnings && ( - setIsShowingTxInsightWarnings(false)} diff --git a/ui/pages/confirmations/components/signature-request-original/signature-request-original.component.js b/ui/pages/confirmations/components/signature-request-original/signature-request-original.component.js index 671253cfc01b..e0c63ebe51d4 100644 --- a/ui/pages/confirmations/components/signature-request-original/signature-request-original.component.js +++ b/ui/pages/confirmations/components/signature-request-original/signature-request-original.component.js @@ -55,6 +55,9 @@ import SignatureRequestHeader from '../signature-request-header'; ///: BEGIN:ONLY_INCLUDE_IF(snaps) import SnapLegacyAuthorshipHeader from '../../../../components/app/snaps/snap-legacy-authorship-header'; ///: END:ONLY_INCLUDE_IF +///: BEGIN:ONLY_INCLUDE_IF(build-flask) +import InsightWarnings from '../../../../components/app/snaps/insight-warnings'; +///: END:ONLY_INCLUDE_IF import SignatureRequestOriginalWarning from './signature-request-original-warning'; export default class SignatureRequestOriginal extends Component { @@ -87,10 +90,16 @@ export default class SignatureRequestOriginal extends Component { selectedAccount: PropTypes.object, mmiOnSignCallback: PropTypes.func, ///: END:ONLY_INCLUDE_IF + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + warnings: PropTypes.array, + ///: END:ONLY_INCLUDE_IF }; state = { showSignatureRequestWarning: false, + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + showSignatureInsights: false, + ///: END:ONLY_INCLUDE_IF }; msgHexToText = (hex) => { @@ -308,7 +317,9 @@ export default class SignatureRequestOriginal extends Component { txData, hardwareWalletRequiresConnection, rejectPendingApproval, - resolvePendingApproval, + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + warnings, + ///: END:ONLY_INCLUDE_IF } = this.props; const { t } = this.context; @@ -326,19 +337,14 @@ export default class SignatureRequestOriginal extends Component { }} onSubmit={async () => { if (txData.type === MESSAGE_TYPE.ETH_SIGN) { - this.setState({ showSignatureRequestWarning: true }); - } else { - ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) - if (this.props.mmiOnSignCallback) { - await this.props.mmiOnSignCallback(txData); - return; - } - ///: END:ONLY_INCLUDE_IF - - await resolvePendingApproval(txData.id); - clearConfirmTransaction(); - history.push(mostRecentOverviewPage); + return this.setState({ showSignatureRequestWarning: true }); } + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + if (warnings?.length >= 1) { + return this.setState({ showSignatureInsights: true }); + } + ///: END:ONLY_INCLUDE_IF + return await this.onSubmit(); }} disabled={ ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) @@ -376,6 +382,9 @@ export default class SignatureRequestOriginal extends Component { messagesCount, fromAccount: { address, name }, txData, + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + warnings, + ///: END:ONLY_INCLUDE_IF } = this.props; const { showSignatureRequestWarning } = this.state; const { t } = this.context; @@ -400,10 +409,40 @@ export default class SignatureRequestOriginal extends Component { await this.onSubmit(event)} + onSubmit={async () => { + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + if (warnings?.length >= 1) { + return this.setState({ + showSignatureInsights: true, + showSignatureRequestWarning: false, + }); + } + ///: END:ONLY_INCLUDE_IF + return await this.onSubmit(); + }} onCancel={async (event) => await this.onCancel(event)} /> )} + { + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + } + {this.state.showSignatureInsights && ( + { + this.setState({ showSignatureInsights: false }); + }} + onSubmit={async () => { + await this.onSubmit(); + this.setState({ showSignatureInsights: false }); + }} + /> + )} + { + ///: END:ONLY_INCLUDE_IF + } {this.renderFooter()} {messagesCount > 1 ? ( { try { await dispatch(resolvePendingApproval(id, null)); @@ -125,120 +138,159 @@ export default function SignatureRequestSIWE({ txData }) { const rejectNText = t('rejectRequestsN', [messagesCount]); return ( -
-
- -
- + <> +
+
+ +
+ + { + ///: BEGIN:ONLY_INCLUDE_IF(blockaid) + + ///: END:ONLY_INCLUDE_IF + } + {showSecurityProviderBanner && ( + + )} +
+ + {!isMatchingAddress && ( + + {t('SIWEAddressInvalid', [ + parsedMessage.address, + fromAccount.address, + ])} + + )} + {isLedgerWallet && ( +
+ +
+ )} + {!isSIWEDomainValid && ( + + + {t('SIWEDomainInvalidTitle')} + {' '} + {t('SIWEDomainInvalidText')} + + )} + { + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + if (warnings?.length >= 1) { + return isSIWEDomainValid + ? setIsShowingSigInsightWarnings(true) + : setIsShowingDomainWarning(true); + } + ///: END:ONLY_INCLUDE_IF + if (isSIWEDomainValid) { + return onSign(); + } + return setIsShowingDomainWarning(true); + }} + cancelText={t('cancel')} + submitText={t('signin')} + submitButtonType={isSIWEDomainValid ? 'primary' : 'danger-primary'} + /> + {messagesCount > 1 ? ( + + ) : null} + {isShowingDomainWarning && ( + setIsShowingDomainWarning(false)} + title={t('SIWEWarningTitle')} + subtitle={t('SIWEWarningSubtitle')} + className="signature-request-siwe__warning-popover" + footerClassName="signature-request-siwe__warning-popover__footer" + footer={ + setIsShowingDomainWarning(false)} + cancelText={t('cancel')} + cancelButtonType="default" + onSubmit={() => { + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + if (warnings?.length >= 1) { + return setIsShowingSigInsightWarnings(true); + } + ///: END:ONLY_INCLUDE_IF + onSign(); + return setIsShowingDomainWarning(false); + }} + submitText={t('confirm')} + submitButtonType="danger-primary" + disabled={!hasAgreedToDomainWarning} + /> + } + > +
+ + setHasAgreedToDomainWarning((checked) => !checked) + } + /> + +
+ + )} +
{ - ///: BEGIN:ONLY_INCLUDE_IF(blockaid) - - ///: END:ONLY_INCLUDE_IF + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) } - {showSecurityProviderBanner && ( - - )} - -
- - {!isMatchingAddress && ( - - {t('SIWEAddressInvalid', [ - parsedMessage.address, - fromAccount.address, - ])} - - )} - {isLedgerWallet && ( -
- -
- )} - {!isSIWEDomainValid && ( - - - {t('SIWEDomainInvalidTitle')} - {' '} - {t('SIWEDomainInvalidText')} - - )} - setIsShowingDomainWarning(true) - } - cancelText={t('cancel')} - submitText={t('signin')} - submitButtonType={isSIWEDomainValid ? 'primary' : 'danger-primary'} - /> - {messagesCount > 1 ? ( - - ) : null} - {isShowingDomainWarning && ( - setIsShowingDomainWarning(false)} - title={t('SIWEWarningTitle')} - subtitle={t('SIWEWarningSubtitle')} - className="signature-request-siwe__warning-popover" - footerClassName="signature-request-siwe__warning-popover__footer" - footer={ - setIsShowingDomainWarning(false)} - cancelText={t('cancel')} - cancelButtonType="default" - onSubmit={onSign} - submitText={t('confirm')} - submitButtonType="danger-primary" - disabled={!hasAgreedToDomainWarning} - /> - } - > -
- setHasAgreedToDomainWarning((checked) => !checked)} - /> - -
- + /> )} -
+ { + ///: END:ONLY_INCLUDE_IF + } + ); } @@ -247,4 +299,10 @@ SignatureRequestSIWE.propTypes = { * The display content of transaction data */ txData: PropTypes.object.isRequired, + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + /** + * Signature insights array + */ + warnings: PropTypes.array, + ///: END:ONLY_INCLUDE_IF }; diff --git a/ui/pages/confirmations/components/signature-request/signature-request.js b/ui/pages/confirmations/components/signature-request/signature-request.js index 3ca4a98cb98c..30d8ae039268 100644 --- a/ui/pages/confirmations/components/signature-request/signature-request.js +++ b/ui/pages/confirmations/components/signature-request/signature-request.js @@ -79,10 +79,18 @@ import { useMMICustodySignMessage } from '../../../../hooks/useMMICustodySignMes import BlockaidBannerAlert from '../security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert'; ///: END:ONLY_INCLUDE_IF +///: BEGIN:ONLY_INCLUDE_IF(build-flask) +import InsightWarnings from '../../../../components/app/snaps/insight-warnings'; +///: END:ONLY_INCLUDE_IF import Message from './signature-request-message'; import Footer from './signature-request-footer'; -const SignatureRequest = ({ txData }) => { +const SignatureRequest = ({ + txData, + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + warnings, + ///: END:ONLY_INCLUDE_IF +}) => { const trackEvent = useContext(MetaMetricsContext); const dispatch = useDispatch(); const t = useI18nContext(); @@ -121,6 +129,11 @@ const SignatureRequest = ({ txData }) => { const { custodySignFn } = useMMICustodySignMessage(); ///: END:ONLY_INCLUDE_IF + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + const [isShowingSigInsightWarnings, setIsShowingSigInsightWarnings] = + useState(false); + ///: END:ONLY_INCLUDE_IF + useEffect(() => { setMessageIsScrollable( messageRootRef?.scrollHeight > messageRootRef?.clientHeight, @@ -188,160 +201,190 @@ const SignatureRequest = ({ txData }) => { } = parseMessage(data); return ( -
- -
- -
-
- { - ///: BEGIN:ONLY_INCLUDE_IF(blockaid) - - ///: END:ONLY_INCLUDE_IF - } - {(txData?.securityProviderResponse?.flagAsDangerous !== undefined && - txData?.securityProviderResponse?.flagAsDangerous !== - SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS) || - (txData?.securityProviderResponse && - Object.keys(txData.securityProviderResponse).length === 0) ? ( - - ) : null} - { - ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) - selectedAccount.address === address ? null : ( - +
+ +
+ +
+
+ { + ///: BEGIN:ONLY_INCLUDE_IF(blockaid) + - - + ///: END:ONLY_INCLUDE_IF + } + {(txData?.securityProviderResponse?.flagAsDangerous !== undefined && + txData?.securityProviderResponse?.flagAsDangerous !== + SECURITY_PROVIDER_MESSAGE_SEVERITY.NOT_MALICIOUS) || + (txData?.securityProviderResponse && + Object.keys(txData.securityProviderResponse).length === 0) ? ( + + ) : null} + { + ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) + selectedAccount.address === address ? null : ( + - {t('mismatchAccount', [ - shortenAddress(selectedAccount.address), - shortenAddress(address), - ])} - - - ) - ///: END:ONLY_INCLUDE_IF - } -
- -
- - {t('sigRequest')} - - - {t('signatureRequestGuidance')} - - {verifyingContract ? ( -
- + + {t('verifyContractDetails')} + + +
+ ) : null} +
+ {isLedgerWallet ? ( +
+
) : null} + setHasScrolledMessage(true)} + setMessageRootRef={setMessageRootRef} + messageRootRef={messageRootRef} + messageIsScrollable={messageIsScrollable} + primaryType={primaryType} + /> +
{ + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + if (warnings?.length >= 1) { + return setIsShowingSigInsightWarnings(true); + } + ///: END:ONLY_INCLUDE_IF + return onSign(); + }} + disabled={ + ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) + Boolean(txData?.custodyId) || + ///: END:ONLY_INCLUDE_IF + hardwareWalletRequiresConnection || + (messageIsScrollable && !hasScrolledMessage) + } + /> + {showContractDetails && ( + setShowContractDetails(false)} + isContractRequestingSignature + /> + )} + {unapprovedMessagesCount > 1 ? ( + + {t('rejectRequestsN', [unapprovedMessagesCount])} + + ) : null}
- {isLedgerWallet ? ( -
- -
- ) : null} - setHasScrolledMessage(true)} - setMessageRootRef={setMessageRootRef} - messageRootRef={messageRootRef} - messageIsScrollable={messageIsScrollable} - primaryType={primaryType} - /> -
- {showContractDetails && ( - setShowContractDetails(false)} - isContractRequestingSignature + { + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + } + {isShowingSigInsightWarnings && ( + setIsShowingSigInsightWarnings(false)} + onSubmit={() => { + onSign(); + setIsShowingSigInsightWarnings(false); + }} /> )} - {unapprovedMessagesCount > 1 ? ( - - {t('rejectRequestsN', [unapprovedMessagesCount])} - - ) : null} -
+ { + ///: END:ONLY_INCLUDE_IF + } + ); }; SignatureRequest.propTypes = { txData: PropTypes.object, + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + warnings: PropTypes.array, + ///: END:ONLY_INCLUDE_IF }; export default SignatureRequest; diff --git a/ui/pages/confirmations/confirm-signature-request/index.js b/ui/pages/confirmations/confirm-signature-request/index.js index d045a5dd29f1..75487e4e052b 100644 --- a/ui/pages/confirmations/confirm-signature-request/index.js +++ b/ui/pages/confirmations/confirm-signature-request/index.js @@ -27,6 +27,9 @@ import { getMemoizedCurrentChainId, getMemoizedTxId, } from '../../../selectors'; +///: BEGIN:ONLY_INCLUDE_IF(build-flask) +import { useSignatureInsights } from '../../../hooks/snaps/useSignatureInsights'; +///: END:ONLY_INCLUDE_IF import { MESSAGE_TYPE } from '../../../../shared/constants/app'; import { getSendTo } from '../../../ducks/send'; @@ -199,6 +202,9 @@ const ConfirmTxScreen = ({ match }) => { unapprovedTypedMessages, ]); + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + const { warnings } = useSignatureInsights({ txData }); + ///: END:ONLY_INCLUDE_IF const resolvedSecurityAlertResponse = signatureSecurityAlertResponses?.[ txData.securityAlertResponse?.securityAlertId @@ -229,6 +235,9 @@ const ConfirmTxScreen = ({ match }) => { ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) selectedAccount={selectedAccount} ///: END:ONLY_INCLUDE_IF + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) + warnings={warnings} + ///: END:ONLY_INCLUDE_IF /> ); }; diff --git a/ui/pages/confirmations/confirm-signature-request/index.test.js b/ui/pages/confirmations/confirm-signature-request/index.test.js index b4ce7db6ad36..829b4aa6c705 100644 --- a/ui/pages/confirmations/confirm-signature-request/index.test.js +++ b/ui/pages/confirmations/confirm-signature-request/index.test.js @@ -25,6 +25,7 @@ const mockState = { from: '0x8eeee1781fd885ff5ddef7789486676961873d12', version: 'V4', origin: 'https://metamask.github.io', + signatureMethod: 'eth_signTypedData_v4', }, time: 1674533844299, status: 'unapproved', @@ -67,6 +68,78 @@ const mockState = { addressBook: {}, tokenList: {}, preferences: {}, + snaps: { + 'local:http://localhost:8080/': { + enabled: true, + id: 'local:http://localhost:8080/', + initialPermissions: { + snap_dialog: {}, + }, + manifest: { + description: 'An example MetaMask Snap.', + initialPermissions: { + snap_dialog: {}, + }, + manifestVersion: '0.1', + proposedName: 'MetaMask Example Snap', + repository: { + type: 'git', + url: 'https://github.com/MetaMask/snaps-skunkworks.git', + }, + source: { + location: { + npm: { + filePath: 'dist/bundle.js', + iconPath: 'images/icon.svg', + packageName: '@metamask/example-snap', + registry: 'https://registry.npmjs.org/', + }, + }, + shasum: '3lEt0yUu080DwV78neROaAAIQWXukSkMnP4OBhOhBnE=', + }, + version: '0.6.0', + }, + sourceCode: '(...)', + status: 'stopped', + svgIcon: '...', + version: '0.6.0', + }, + 'npm:@metamask/test-snap-bip44': { + id: 'npm:@metamask/test-snap-bip44', + origin: 'npm:@metamask/test-snap-bip44', + version: '5.1.2', + iconUrl: null, + initialPermissions: { + 'endowment:ethereum-provider': {}, + }, + manifest: { + description: 'An example Snap that signs messages using BLS.', + proposedName: 'BIP-44 Test Snap', + repository: { + type: 'git', + url: 'https://github.com/MetaMask/test-snaps.git', + }, + source: { + location: { + npm: { + filePath: 'dist/bundle.js', + packageName: '@metamask/test-snap-bip44', + registry: 'https://registry.npmjs.org', + }, + }, + shasum: 'L1k+dT9Q+y3KfIqzaH09MpDZVPS9ZowEh9w01ZMTWMU=', + }, + version: '5.1.2', + }, + versionHistory: [ + { + date: 1680686075921, + origin: 'https://metamask.github.io', + version: '5.1.2', + }, + ], + }, + }, }, appState: { warning: null, diff --git a/ui/pages/confirmations/confirmation/templates/__snapshots__/snap-account-redirect.test.js.snap b/ui/pages/confirmations/confirmation/templates/__snapshots__/snap-account-redirect.test.js.snap index 70be56b8335d..e5a231ba7b0c 100644 --- a/ui/pages/confirmations/confirmation/templates/__snapshots__/snap-account-redirect.test.js.snap +++ b/ui/pages/confirmations/confirmation/templates/__snapshots__/snap-account-redirect.test.js.snap @@ -142,10 +142,10 @@ exports[`snap-account-redirect confirmation should match snapshot 1`] = ` style="border-bottom-width: 1px;" >

Content from Test Snap Account Name

diff --git a/ui/pages/snap-account-redirect/__snapshots__/create-snap-redirect.test.tsx.snap b/ui/pages/snap-account-redirect/__snapshots__/create-snap-redirect.test.tsx.snap index 97937e411816..3b200146348c 100644 --- a/ui/pages/snap-account-redirect/__snapshots__/create-snap-redirect.test.tsx.snap +++ b/ui/pages/snap-account-redirect/__snapshots__/create-snap-redirect.test.tsx.snap @@ -141,10 +141,10 @@ exports[` renders the url and message when provided and i style="border-bottom-width: 1px;" >
renders the url and message when provided and i />

Content from Snap Simple Keyring

diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index d7c244f3544d..95d803c2bd32 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -1161,6 +1161,21 @@ export const getInsightSnaps = createDeepEqualSelector( }, ); +export const getSignatureInsightSnaps = createDeepEqualSelector( + getEnabledSnaps, + getPermissionSubjects, + (snaps, subjects) => { + return Object.values(snaps).filter( + ({ id }) => subjects[id]?.permissions['endowment:signature-insight'], + ); + }, +); + +export const getSignatureInsightSnapIds = createDeepEqualSelector( + getSignatureInsightSnaps, + (snaps) => snaps.map((snap) => snap.id), +); + export const getInsightSnapIds = createDeepEqualSelector( getInsightSnaps, (snaps) => snaps.map((snap) => snap.id), diff --git a/yarn.lock b/yarn.lock index 4da47447e281..ef9140413087 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3912,9 +3912,9 @@ __metadata: languageName: node linkType: hard -"@metamask/eth-json-rpc-middleware@npm:^12.0.1": - version: 12.0.1 - resolution: "@metamask/eth-json-rpc-middleware@npm:12.0.1" +"@metamask/eth-json-rpc-middleware@npm:^12.0.1, @metamask/eth-json-rpc-middleware@npm:^12.1.0": + version: 12.1.0 + resolution: "@metamask/eth-json-rpc-middleware@npm:12.1.0" dependencies: "@metamask/eth-json-rpc-provider": "npm:^2.1.0" "@metamask/eth-sig-util": "npm:^7.0.0" @@ -3925,7 +3925,7 @@ __metadata: klona: "npm:^2.0.6" pify: "npm:^5.0.0" safe-stable-stringify: "npm:^2.4.3" - checksum: 3ff40940d118042cfa957222effcd30035b1eb8f412724c3e60e87eef7b9cdcaf4cd9af743eb7c9b1846a3af896830b1522c73f57bcc791a82aa7d1ee96d294c + checksum: 35a34dba839b957068030880328b43aabca615cc9f9fbf2d11fd998f0cc622ff3eec87582861b6cc517ee18cbd652f5c237647d651aa939151ac3f8738abae73 languageName: node linkType: hard @@ -24052,7 +24052,7 @@ __metadata: "@metamask/eslint-config-mocha": "npm:^9.0.0" "@metamask/eslint-config-nodejs": "npm:^9.0.0" "@metamask/eslint-config-typescript": "npm:^9.0.1" - "@metamask/eth-json-rpc-middleware": "npm:^12.0.1" + "@metamask/eth-json-rpc-middleware": "npm:^12.1.0" "@metamask/eth-keyring-controller": "npm:^15.1.0" "@metamask/eth-ledger-bridge-keyring": "npm:^2.0.1" "@metamask/eth-query": "npm:^4.0.0"