From 57d3212c6131abfe857c851244b8344dc243e36d Mon Sep 17 00:00:00 2001 From: Owen Craston Date: Thu, 12 Oct 2023 13:08:02 -0700 Subject: [PATCH] feat: Snap account async transaction redirect (#21312) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Progresses: https://github.com/MetaMask/accounts-planning/issues/35 This PR implements the `redirectUser` callback that was added in the [eth-snap-keyring](https://github.com/MetaMask/eth-snap-keyring/pull/136). This callbacks allows us to redirect the user to a given url or direct them with custom instructions to sign an async transaction. [Design](https://www.figma.com/file/K0csZegZ5XQ6TjfKk3n4Z8/Add-dialog-to-inform-about-redirection-to-snap-site-%2335?type=design&node-id=604-2586&mode=design&t=tRpxcs4qeRiRKxPl-0) ## **Manual testing steps** 1. checkout this [branch](https://github.com/MetaMask/snap-simple-keyring/pull/98) in the snap simple keyring repo (this branch is wip but it will work for testing) 2. run `yarn install` and `yarn start` 3. checkout this branch and run `yarn install && yarn start --build-type flask` 4. in your browser load the dist folder into the extensions via `load unpacked` 5. setup a wallet in MM 6. 3. navigate to http://localhost:8000/ 7. click `create account` in the dapp site and approve the creation flow 8. toggle the `Use Synchronous Approval` button at the top of the page so that it is grey. This will turn on async approvals 9. navigate to https://metamask.github.io/test-dapp/ 10. connect your MM to the snap 11. scroll down and click `Personal Sign` 12. a pop up should open in MM, accept that sign 13. a new pop up should open that matches the [above designs](https://www.figma.com/file/K0csZegZ5XQ6TjfKk3n4Z8/Add-dialog-to-inform-about-redirection-to-snap-site-%2335?type=design&node-id=604-2586&mode=design&t=tRpxcs4qeRiRKxPl-0) ## **Screenshots/Recordings** Blocked URL Screenshot 2023-10-12 at 11 16 35 AM redirect with message and URL Screenshot 2023-10-12 at 11 14 55 AM without message Screenshot 2023-10-12 at 11 18 23 AM without URL Screenshot 2023-10-12 at 11 17 33 AM ### **Before** N/A ### **After** https://github.com/MetaMask/metamask-extension/assets/22918444/05213130-ac5d-4d65-ac10-2fe32a96115a ## **Related issues** https://github.com/MetaMask/accounts-planning/issues/35 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained: - [ ] What problem this PR is solving. - [ ] How this problem was solved. - [ ] How reviewers can test my changes. - [ ] I’ve indicated what issue this PR is linked to: Fixes #??? - [ ] I’ve included tests if applicable. - [ ] I’ve documented any added code. - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: MetaMask Bot Co-authored-by: Monte Lai Co-authored-by: Daniel Rocha <68558152+danroc@users.noreply.github.com> --- app/_locales/en/messages.json | 12 + app/scripts/background.js | 3 +- .../lib/keyring-snaps-permissions.test.ts | 25 +- app/scripts/lib/keyring-snaps-permissions.ts | 10 +- app/scripts/lib/snap-keyring/snap-keyring.ts | 28 +++ .../snap-keyring/utils/isBlockedUrl.test.ts | 38 +++ .../lib/snap-keyring/utils/isBlockedUrl.ts | 32 +++ app/scripts/metamask-controller.js | 2 + lavamoat/browserify/desktop/policy.json | 3 +- lavamoat/browserify/flask/policy.json | 3 +- package.json | 3 +- privacy-snapshot.json | 1 + shared/constants/app.ts | 1 + test/e2e/accounts/test-snap-accounts.spec.js | 63 ++++- test/e2e/accounts/utils.ts | 2 +- .../safe-component-list.js | 2 + .../confirmation-footer.js | 24 +- .../snap-account-redirect.test.js.snap | 220 ++++++++++++++++++ ui/pages/confirmation/templates/index.js | 3 + .../templates/snap-account-redirect.js | 44 ++++ .../templates/snap-account-redirect.test.js | 68 ++++++ .../create-snap-redirect.test.tsx.snap | 88 +++++++ .../snap-account-redirect/components/index.ts | 1 + .../components/redirect-url-icon.tsx | 27 +++ .../snap-account-redirect-context.tsx | 99 ++++++++ .../snap-account-redirect-message.tsx | 36 +++ .../components/url-display-box.tsx | 39 ++++ .../create-snap-redirect.test.tsx | 97 ++++++++ ui/pages/snap-account-redirect/index.ts | 1 + .../snap-account-redirect.stories.tsx | 38 +++ .../snap-account-redirect.tsx | 53 +++++ yarn.lock | 18 +- 32 files changed, 1051 insertions(+), 33 deletions(-) create mode 100644 app/scripts/lib/snap-keyring/utils/isBlockedUrl.test.ts create mode 100644 app/scripts/lib/snap-keyring/utils/isBlockedUrl.ts create mode 100644 ui/pages/confirmation/templates/__snapshots__/snap-account-redirect.test.js.snap create mode 100644 ui/pages/confirmation/templates/snap-account-redirect.js create mode 100644 ui/pages/confirmation/templates/snap-account-redirect.test.js create mode 100644 ui/pages/snap-account-redirect/__snapshots__/create-snap-redirect.test.tsx.snap create mode 100644 ui/pages/snap-account-redirect/components/index.ts create mode 100644 ui/pages/snap-account-redirect/components/redirect-url-icon.tsx create mode 100644 ui/pages/snap-account-redirect/components/snap-account-redirect-context.tsx create mode 100644 ui/pages/snap-account-redirect/components/snap-account-redirect-message.tsx create mode 100644 ui/pages/snap-account-redirect/components/url-display-box.tsx create mode 100644 ui/pages/snap-account-redirect/create-snap-redirect.test.tsx create mode 100644 ui/pages/snap-account-redirect/index.ts create mode 100644 ui/pages/snap-account-redirect/snap-account-redirect.stories.tsx create mode 100644 ui/pages/snap-account-redirect/snap-account-redirect.tsx diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index cafe489e14d0..0ed8f24abf99 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1827,6 +1827,9 @@ "goBack": { "message": "Go back" }, + "goToSite": { + "message": "Go to site" + }, "goerli": { "message": "Goerli test network" }, @@ -4167,6 +4170,12 @@ "snapAccountCreated": { "message": "Your account is ready!" }, + "snapAccountRedirectFinishSigningTitle": { + "message": "Finish signing" + }, + "snapAccountRedirectSiteDescription": { + "message": "Follow the instructions from $1" + }, "snapAccountRemoved": { "message": "Account removed" }, @@ -4298,6 +4307,9 @@ "snapUpdateSuccess": { "message": "Update complete" }, + "snapUrlIsBlocked": { + "message": "This Snap wants to take you to a blocked site. $1." + }, "snaps": { "message": "Snaps" }, diff --git a/app/scripts/background.js b/app/scripts/background.js index 4ae4ecbbce87..a42e3f17c652 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -806,9 +806,8 @@ export function setupController( ///: END:ONLY_INCLUDE_IN ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) case SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.confirmAccountCreation: - controller.approvalController.accept(id, false); - break; case SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.confirmAccountRemoval: + case SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.showSnapAccountRedirect: controller.approvalController.accept(id, false); break; ///: END:ONLY_INCLUDE_IN diff --git a/app/scripts/lib/keyring-snaps-permissions.test.ts b/app/scripts/lib/keyring-snaps-permissions.test.ts index 4bbce54b346c..c2707914ecbd 100644 --- a/app/scripts/lib/keyring-snaps-permissions.test.ts +++ b/app/scripts/lib/keyring-snaps-permissions.test.ts @@ -3,7 +3,10 @@ import { SubjectType, } from '@metamask/permission-controller'; import { KeyringRpcMethod } from '@metamask/keyring-api'; -import { keyringSnapPermissionsBuilder } from './keyring-snaps-permissions'; +import { + isProtocolAllowed, + keyringSnapPermissionsBuilder, +} from './keyring-snaps-permissions'; describe('keyringSnapPermissionsBuilder', () => { const mockController = new SubjectMetadataController({ @@ -73,3 +76,23 @@ describe('keyringSnapPermissionsBuilder', () => { expect(permissions(origin as any)).toStrictEqual([]); }); }); + +describe('isProtocolAllowed', () => { + it.each([ + ['http://some-dapp.com', true], + ['https://some-dapp.com', true], + ['sftp://some-dapp.com', false], + ['', false], + ['null', false], + ['0', false], + [undefined, false], + [null, false], + [true, false], + [false, false], + [1, false], + [0, false], + [-1, false], + ])('"%s" cannot call any methods', (origin: any, expected: boolean) => { + expect(isProtocolAllowed(origin)).toBe(expected); + }); +}); diff --git a/app/scripts/lib/keyring-snaps-permissions.ts b/app/scripts/lib/keyring-snaps-permissions.ts index fc3dba14a3f9..d6df442e8966 100644 --- a/app/scripts/lib/keyring-snaps-permissions.ts +++ b/app/scripts/lib/keyring-snaps-permissions.ts @@ -51,9 +51,13 @@ const ALLOWED_PROTOCOLS: string[] = [ * @param origin - The origin to check. * @returns `true` if the protocol of the origin is allowed, `false` otherwise. */ -function isProtocolAllowed(origin: string): boolean { - const url = new URL(origin); - return ALLOWED_PROTOCOLS.includes(url.protocol); +export function isProtocolAllowed(origin: string): boolean { + try { + const url = new URL(origin); + return ALLOWED_PROTOCOLS.includes(url.protocol); + } catch (error) { + return false; + } } /** diff --git a/app/scripts/lib/snap-keyring/snap-keyring.ts b/app/scripts/lib/snap-keyring/snap-keyring.ts index 6d853dfdbf54..92c1de692cc8 100644 --- a/app/scripts/lib/snap-keyring/snap-keyring.ts +++ b/app/scripts/lib/snap-keyring/snap-keyring.ts @@ -5,10 +5,13 @@ import type { ResultComponent, } from '@metamask/approval-controller'; import type { KeyringController } from '@metamask/keyring-controller'; +import { PhishingController } from '@metamask/phishing-controller'; +import browser from 'webextension-polyfill'; import { SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES } from '../../../../shared/constants/app'; import { t } from '../../translate'; import MetamaskController from '../../metamask-controller'; import PreferencesController from '../../controllers/preferences'; +import { isBlockedUrl } from './utils/isBlockedUrl'; /** * Get the addresses of the accounts managed by a given Snap. @@ -32,6 +35,7 @@ export const getAccountsBySnapId = async ( * @param getApprovalController - A function that retrieves the Approval Controller instance. * @param getKeyringController - A function that retrieves the Keyring Controller instance. * @param getPreferencesController - A function that retrieves the Preferences Controller instance. + * @param getPhishingController - A function that retrieves the Phishing Controller instance * @param removeAccountHelper - A function to help remove an account based on its address. * @returns The constructed SnapKeyring builder instance with the following methods: * - `saveState`: Persists all keyrings in the keyring controller. @@ -43,6 +47,7 @@ export const snapKeyringBuilder = ( getApprovalController: () => ApprovalController, getKeyringController: () => KeyringController, getPreferencesController: () => PreferencesController, + getPhishingController: () => PhishingController, removeAccountHelper: (address: string) => Promise, ) => { const builder = (() => { @@ -51,6 +56,29 @@ export const snapKeyringBuilder = ( const addresses = await getKeyringController().getAccounts(); return addresses.includes(address.toLowerCase()); }, + redirectUser: async (snapId: string, url: string, message: string) => { + // Either url or message must be defined + if (url.length > 0 || message.length > 0) { + const isBlocked = await isBlockedUrl(url, getPhishingController()); + + const confirmationResult: boolean = + (await getApprovalController().addAndShowApprovalRequest({ + origin: snapId, + requestData: { url, message, isBlockedUrl: isBlocked }, + type: SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.showSnapAccountRedirect, + })) as boolean; + + if (confirmationResult && url.length > 0) { + browser.tabs.create({ url }); + } else { + console.log('User refused snap account redirection to:', url); + } + } else { + console.log( + 'Error occurred when redirecting snap account. url or message must be defined', + ); + } + }, saveState: async () => { await getKeyringController().persistAllKeyrings(); }, diff --git a/app/scripts/lib/snap-keyring/utils/isBlockedUrl.test.ts b/app/scripts/lib/snap-keyring/utils/isBlockedUrl.test.ts new file mode 100644 index 000000000000..0ac199636d67 --- /dev/null +++ b/app/scripts/lib/snap-keyring/utils/isBlockedUrl.test.ts @@ -0,0 +1,38 @@ +import { ListNames, PhishingController } from '@metamask/phishing-controller'; +import { isBlockedUrl } from './isBlockedUrl'; + +describe('isBlockedUrl', () => { + const phishingController = new PhishingController( + {}, + { + phishingLists: [ + { + blocklist: ['https://metamask.test'], + allowlist: [], + fuzzylist: [], + tolerance: 0, + version: 1, + lastUpdated: 0, + name: ListNames.MetaMask, + }, + ], + }, + ); + + it.each([ + ['http://metamask.io', false], + ['https://metamask.io', false], + ['https://metamask.test', true], + ['sftp://metamask.io', true], + ['', true], + ['1', true], + [undefined, true], + [null, true], + [1, true], + [0, true], + [-1, true], + ])('"%s" is blocked: %s', async (url: any, expected: boolean) => { + const result = await isBlockedUrl(url, phishingController); + expect(result).toEqual(expected); + }); +}); diff --git a/app/scripts/lib/snap-keyring/utils/isBlockedUrl.ts b/app/scripts/lib/snap-keyring/utils/isBlockedUrl.ts new file mode 100644 index 000000000000..8a4f527a1bf7 --- /dev/null +++ b/app/scripts/lib/snap-keyring/utils/isBlockedUrl.ts @@ -0,0 +1,32 @@ +import { PhishingController } from '@metamask/phishing-controller'; +import { isProtocolAllowed } from '../../keyring-snaps-permissions'; + +/** + * Checks whether a given URL is blocked due to not using HTTPS or being + * recognized as a phishing URL. + * + * @param url - The URL to check. + * @param phishingController - An instance of PhishingController to verify + * against known phishing URLs. + * @returns Returns a promise which resolves to `true` if the URL is blocked + * either due to using an insecure protocol (not HTTPS) or being recognized as + * a phishing URL. Otherwise, resolves to `false`. + */ +export const isBlockedUrl = async ( + url: string, + phishingController: PhishingController, +): Promise => { + try { + // check if the URL is HTTPS + if (!isProtocolAllowed(url)) { + return true; + } + + // check if the url is in the phishing list + await phishingController.maybeUpdateState(); + return phishingController.test(url).result; + } catch (error) { + console.error('Invalid URL passed into snap-keyring:', error); + return false; + } +}; diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 11190df27f4c..9f0b162b207c 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -877,6 +877,7 @@ export default class MetamaskController extends EventEmitter { const getApprovalController = () => this.approvalController; const getKeyringController = () => this.keyringController; const getPreferencesController = () => this.preferencesController; + const getPhishingController = () => this.phishingController; additionalKeyrings.push( snapKeyringBuilder( @@ -884,6 +885,7 @@ export default class MetamaskController extends EventEmitter { getApprovalController, getKeyringController, getPreferencesController, + getPhishingController, (address) => this.removeAccount(address), ), ); diff --git a/lavamoat/browserify/desktop/policy.json b/lavamoat/browserify/desktop/policy.json index bb227f6be7a1..534de2371187 100644 --- a/lavamoat/browserify/desktop/policy.json +++ b/lavamoat/browserify/desktop/policy.json @@ -1227,8 +1227,7 @@ }, "@metamask/eth-snap-keyring": { "globals": { - "console.error": true, - "console.log": true + "console.error": true }, "packages": { "@ethereumjs/tx": true, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 5d2e9e9db48f..eb0da21f56ef 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -1227,8 +1227,7 @@ }, "@metamask/eth-snap-keyring": { "globals": { - "console.error": true, - "console.log": true + "console.error": true }, "packages": { "@ethereumjs/tx": true, diff --git a/package.json b/package.json index 219dd5f0bd98..ee7ad8ccb66f 100644 --- a/package.json +++ b/package.json @@ -248,7 +248,7 @@ "@metamask/eth-json-rpc-middleware": "^11.0.0", "@metamask/eth-keyring-controller": "^13.0.1", "@metamask/eth-ledger-bridge-keyring": "^0.15.0", - "@metamask/eth-snap-keyring": "^1.0.0-rc.1", + "@metamask/eth-snap-keyring": "1.0.0-rc.2", "@metamask/eth-token-tracker": "^4.0.0", "@metamask/eth-trezor-keyring": "^1.1.0", "@metamask/etherscan-link": "^2.2.0", @@ -436,6 +436,7 @@ "@types/sinon": "^10.0.13", "@types/w3c-web-hid": "^1.0.3", "@types/watchify": "^3.11.1", + "@types/webextension-polyfill": "^0.10.4", "@types/yargs": "^17.0.8", "@typescript-eslint/eslint-plugin": "^5.30.7", "@typescript-eslint/parser": "^5.30.7", diff --git a/privacy-snapshot.json b/privacy-snapshot.json index bd6d6ae7a7b4..35e80a9dcd3b 100644 --- a/privacy-snapshot.json +++ b/privacy-snapshot.json @@ -4,6 +4,7 @@ "api.lens.dev", "api.segment.io", "arbitrum-mainnet.infura.io", + "aus5.mozilla.org", "bafkreifvhjdf6ve4jfv6qytqtux5nd4nwnelioeiqx5x2ez5yrgrzk7ypi.ipfs.dweb.link", "bafybeidxfmwycgzcp4v2togflpqh2gnibuexjy4m4qqwxp7nh3jx5zlh4y.ipfs.dweb.link", "cdnjs.cloudflare.com", diff --git a/shared/constants/app.ts b/shared/constants/app.ts index 47bb37e4b5ae..60dd772ae1f7 100644 --- a/shared/constants/app.ts +++ b/shared/constants/app.ts @@ -81,6 +81,7 @@ export const SNAP_DIALOG_TYPES = { export const SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES = { confirmAccountCreation: 'snap_manageAccounts:confirmAccountCreation', confirmAccountRemoval: 'snap_manageAccounts:confirmAccountRemoval', + showSnapAccountRedirect: 'showSnapAccountRedirect', }; ///: END:ONLY_INCLUDE_IN diff --git a/test/e2e/accounts/test-snap-accounts.spec.js b/test/e2e/accounts/test-snap-accounts.spec.js index 6a22aa829a1e..97dbf69e75fc 100644 --- a/test/e2e/accounts/test-snap-accounts.spec.js +++ b/test/e2e/accounts/test-snap-accounts.spec.js @@ -2,7 +2,6 @@ const { strict: assert } = require('assert'); const util = require('ethereumjs-util'); const FixtureBuilder = require('../fixture-builder'); const { - clickSignOnSignatureConfirmation, convertETHToHexGwei, openDapp, PRIVATE_KEY, @@ -286,14 +285,25 @@ describe('Test Snap Account', function () { await switchToOrOpenDapp(driver); await driver.clickElement(locatorID); - await switchToNotificationWindow(driver, 4); + + // behaviour of chrome and firefox is different, + // chrome needs extra time to load the popup + if (driver.browser === 'chrome') { + await driver.delay(500); + } + const handles = await driver.getAllWindowHandles(); + await driver.switchToWindowWithTitle( + WINDOW_TITLES.Notification, + handles, + 2000, + ); // these two don't have a contract details page if (locatorID !== '#personalSign' && locatorID !== '#signTypedData') { await validateContractDetails(driver); } - await clickSignOnSignatureConfirmation(driver, 3); + await driver.clickElement({ text: 'Sign', tag: 'button' }); if (isAsyncFlow) { await approveOrRejectRequest(driver, flowType); @@ -422,7 +432,12 @@ describe('Test Snap Account', function () { }); // Click "Create" on the Snap's confirmation popup - await switchToNotificationWindow(driver, 3); + const handles = await driver.getAllWindowHandles(); + await driver.switchToWindowWithTitle( + WINDOW_TITLES.Notification, + handles, + 2000, + ); await driver.clickElement('[data-testid="confirmation-submit-button"]'); await driver.clickElement('[data-testid="confirmation-submit-button"]'); await driver.switchToWindowWithTitle('SSK - Simple Snap Keyring'); @@ -466,7 +481,12 @@ describe('Test Snap Account', function () { async function connectAccountToTestDapp(driver) { await switchToOrOpenDapp(driver); await driver.clickElement('#connectButton'); - await switchToNotificationWindow(driver, 4); + const handles = await driver.getAllWindowHandles(); + await driver.switchToWindowWithTitle( + WINDOW_TITLES.Notification, + handles, + 2000, + ); await driver.clickElement('[data-testid="page-container-footer-next"]'); await driver.clickElement('[data-testid="page-container-footer-next"]'); } @@ -489,6 +509,39 @@ describe('Test Snap Account', function () { * @param {string} flowType */ async function approveOrRejectRequest(driver, flowType) { + // Click redirect button for async flows + if (flowType === 'approve' || flowType === 'reject') { + // There is a try catch here because when using the send eth flow, + // MetaMask is still active, and therefore the popup notification will + // no appear. The workaround is to reload the extension and + // force the notification to appear in the full window. + try { + // Adding a delay here because there is a race condition where + // the driver tries to switch to the first notification window + // and not the second notification window with the redirect button + await driver.delay(500); + const handles = await driver.getAllWindowHandles(); + await driver.switchToWindowWithTitle( + WINDOW_TITLES.Notification, + handles, + 2000, + ); + } catch (error) { + console.log('SNAPS/ error switching to notification window', error); + + await driver.switchToWindowWithTitle( + WINDOW_TITLES.ExtensionInFullScreenView, + ); + await driver.navigate(); + } + await driver.clickElement({ + text: 'Go to site', + tag: 'button', + }); + + await driver.delay(500); + } + await driver.switchToWindowWithTitle('SSK - Simple Snap Keyring'); await driver.clickElementUsingMouseMove({ diff --git a/test/e2e/accounts/utils.ts b/test/e2e/accounts/utils.ts index 8d844f329326..f8652465faa5 100644 --- a/test/e2e/accounts/utils.ts +++ b/test/e2e/accounts/utils.ts @@ -1,2 +1,2 @@ export const TEST_SNAPS_SIMPLE_KEYRING_WEBSITE_URL = - 'https://metamask.github.io/snap-simple-keyring/0.3.0/'; + 'https://metamask.github.io/snap-simple-keyring/0.4.0/'; diff --git a/ui/components/app/metamask-template-renderer/safe-component-list.js b/ui/components/app/metamask-template-renderer/safe-component-list.js index 0c6574f71190..623c945e40c1 100644 --- a/ui/components/app/metamask-template-renderer/safe-component-list.js +++ b/ui/components/app/metamask-template-renderer/safe-component-list.js @@ -24,6 +24,7 @@ import { SnapUIImage } from '../snaps/snap-ui-image'; ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) import { CreateSnapAccount } from '../../../pages/create-snap-account'; import { RemoveSnapAccount } from '../../../pages/remove-snap-account'; +import { SnapAccountRedirect } from '../../../pages/snap-account-redirect'; import SnapAuthorshipHeader from '../snaps/snap-authorship-header'; ///: END:ONLY_INCLUDE_IN @@ -61,5 +62,6 @@ export const safeComponentList = { CreateSnapAccount, RemoveSnapAccount, SnapAuthorshipHeader, + SnapAccountRedirect, ///: END:ONLY_INCLUDE_IN }; diff --git a/ui/pages/confirmation/components/confirmation-footer/confirmation-footer.js b/ui/pages/confirmation/components/confirmation-footer/confirmation-footer.js index 69523038d774..5177801cdadc 100644 --- a/ui/pages/confirmation/components/confirmation-footer/confirmation-footer.js +++ b/ui/pages/confirmation/components/confirmation-footer/confirmation-footer.js @@ -29,17 +29,19 @@ export default function ConfirmationFooter({ {cancelText} ) : null} - + {onSubmit && submitText ? ( + + ) : null} ); diff --git a/ui/pages/confirmation/templates/__snapshots__/snap-account-redirect.test.js.snap b/ui/pages/confirmation/templates/__snapshots__/snap-account-redirect.test.js.snap new file mode 100644 index 000000000000..53a20ffea165 --- /dev/null +++ b/ui/pages/confirmation/templates/__snapshots__/snap-account-redirect.test.js.snap @@ -0,0 +1,220 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`snap-account-redirect confirmation should match snapshot 1`] = ` +
+
+
+
+
+
+
+ ? +
+
+
+ +
+
+
+
+
+

+

+

+
+ +
+
+ +
+
+
+ + +
+
+
+
+`; diff --git a/ui/pages/confirmation/templates/index.js b/ui/pages/confirmation/templates/index.js index 7199833c2b54..4698e66f18e5 100644 --- a/ui/pages/confirmation/templates/index.js +++ b/ui/pages/confirmation/templates/index.js @@ -10,6 +10,7 @@ import { import { SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES } from '../../../../shared/constants/app'; import createSnapAccount from './create-snap-account'; import removeSnapAccount from './remove-snap-account'; +import snapAccountRedirect from './snap-account-redirect'; ///: END:ONLY_INCLUDE_IN import addEthereumChain from './add-ethereum-chain'; import switchEthereumChain from './switch-ethereum-chain'; @@ -37,6 +38,8 @@ const APPROVAL_TEMPLATES = { createSnapAccount, [SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.confirmAccountRemoval]: removeSnapAccount, + [SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.showSnapAccountRedirect]: + snapAccountRedirect, ///: END:ONLY_INCLUDE_IN }; diff --git a/ui/pages/confirmation/templates/snap-account-redirect.js b/ui/pages/confirmation/templates/snap-account-redirect.js new file mode 100644 index 000000000000..a878613d47b8 --- /dev/null +++ b/ui/pages/confirmation/templates/snap-account-redirect.js @@ -0,0 +1,44 @@ +function getValues(pendingApproval, t, actions, _history) { + const { snapName } = pendingApproval; + const { url, message, isBlockedUrl } = pendingApproval.requestData; + + const getConditionalProps = () => { + if ( + url !== undefined && + url !== null && + url.length > 0 && + isBlockedUrl === false + ) { + return { + submitText: t('goToSite'), + onSubmit: () => + actions.resolvePendingApproval(pendingApproval.id, true), + }; + } + return {}; + }; + + return { + content: [ + { + element: 'SnapAccountRedirect', + key: 'snap-account-redirect', + props: { + url, + message, + snapName, + isBlockedUrl, + }, + }, + ], + cancelText: t('close'), + onCancel: () => actions.resolvePendingApproval(pendingApproval.id, false), + ...getConditionalProps(), + }; +} + +const createSnapAccount = { + getValues, +}; + +export default createSnapAccount; diff --git a/ui/pages/confirmation/templates/snap-account-redirect.test.js b/ui/pages/confirmation/templates/snap-account-redirect.test.js new file mode 100644 index 000000000000..261dcc7af6d8 --- /dev/null +++ b/ui/pages/confirmation/templates/snap-account-redirect.test.js @@ -0,0 +1,68 @@ +import React from 'react'; +import configureMockStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; +import { waitFor } from '@testing-library/react'; + +import Confirmation from '../confirmation'; +import { renderWithProvider } from '../../../../test/lib/render-helpers'; +import { SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES } from '../../../../shared/constants/app'; + +const middleware = [thunk]; + +const mockApprovalId = 1; +const mockSnapName = 'Test Snap Account Name'; +const mockUrl = 'https://metamask.github.io/test-snap'; +const mockMessage = 'Test Snap Account Message'; +const mockIsBlockedUrl = false; +const providerConfig = { + chainId: '0x5', +}; +const mockApproval = { + id: mockApprovalId, + snapName: mockSnapName, + requestData: { + url: mockUrl, + message: mockMessage, + isBlockedUrl: mockIsBlockedUrl, + }, +}; +const mockBaseStore = { + metamask: { + pendingApprovals: { + [mockApprovalId]: mockApproval, + }, + approvalFlows: [], + subjectMetadata: {}, + providerConfig, + }, +}; + +describe('snap-account-redirect confirmation', () => { + it('should match snapshot', async () => { + const testStore = { + metamask: { + ...mockBaseStore.metamask, + pendingApprovals: { + [mockApprovalId]: { + ...mockApproval, + type: SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.showSnapAccountRedirect, + }, + }, + }, + }; + const store = configureMockStore(middleware)(testStore); + const { container, getByText } = renderWithProvider( + , + store, + ); + await waitFor(() => { + expect(getByText(`Finish signing`)).toBeInTheDocument(); + expect( + getByText(`Follow the instructions from ${mockSnapName}`), + ).toBeInTheDocument(); + expect(getByText('Test Snap Account Message')).toBeInTheDocument(); + expect(container.querySelector('.callout')).toBeDefined(); + expect(container).toMatchSnapshot(); + }); + }); +}); 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 new file mode 100644 index 000000000000..e6c61312d708 --- /dev/null +++ b/ui/pages/snap-account-redirect/__snapshots__/create-snap-redirect.test.tsx.snap @@ -0,0 +1,88 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders the url and message when provided and isBlockedUrl is false 1`] = ` +
+ +
+`; diff --git a/ui/pages/snap-account-redirect/components/index.ts b/ui/pages/snap-account-redirect/components/index.ts new file mode 100644 index 000000000000..1e98846345b8 --- /dev/null +++ b/ui/pages/snap-account-redirect/components/index.ts @@ -0,0 +1 @@ +export { default as SnapAccountRedirectContent } from './snap-account-redirect-context'; diff --git a/ui/pages/snap-account-redirect/components/redirect-url-icon.tsx b/ui/pages/snap-account-redirect/components/redirect-url-icon.tsx new file mode 100644 index 000000000000..4d5e605dde24 --- /dev/null +++ b/ui/pages/snap-account-redirect/components/redirect-url-icon.tsx @@ -0,0 +1,27 @@ +import React from 'react'; +import { + ButtonIcon, + ButtonIconSize, + IconName, +} from '../../../components/component-library'; +import { IconColor } from '../../../helpers/constants/design-system'; + +interface RedirectUrlIconProps { + url: string; +} + +const RedirectUrlIcon = ({ url }: RedirectUrlIconProps) => { + return ( + { + global.platform.openTab({ url }); + }} + iconName={IconName.Export} + color={IconColor.primaryDefault} + size={ButtonIconSize.Sm} + ariaLabel={''} + /> + ); +}; + +export default React.memo(RedirectUrlIcon); diff --git a/ui/pages/snap-account-redirect/components/snap-account-redirect-context.tsx b/ui/pages/snap-account-redirect/components/snap-account-redirect-context.tsx new file mode 100644 index 000000000000..bda5d5e5625a --- /dev/null +++ b/ui/pages/snap-account-redirect/components/snap-account-redirect-context.tsx @@ -0,0 +1,99 @@ +import React from 'react'; +import { + BannerAlert, + BannerAlertSeverity, + Box, + Button, + ButtonSize, + ButtonVariant, + Text, +} from '../../../components/component-library'; +import { SnapAccountRedirectProps } from '../snap-account-redirect'; +import { + AlignItems, + Display, + FlexDirection, + JustifyContent, + TextAlign, + TextVariant, +} from '../../../helpers/constants/design-system'; +import { useI18nContext } from '../../../hooks/useI18nContext'; +import SnapAccountRedirectMessage from './snap-account-redirect-message'; + +const SnapAccountRedirectContent = ({ + url, + snapName, + isBlockedUrl, + message, +}: SnapAccountRedirectProps) => { + const t = useI18nContext(); + const learnMoreAboutBlockedUrls = + 'https://support.metamask.io/hc/en-us/articles/4428045875483--Deceptive-site-ahead-when-trying-to-connect-to-a-site'; + + return ( + + + + {t('snapAccountRedirectFinishSigningTitle')} + + {isBlockedUrl ? ( + + + + {t('snapUrlIsBlocked', [ + , + ])} + + + + ) : null} + {isBlockedUrl === false ? ( + + {t('snapAccountRedirectSiteDescription', [snapName])} + + ) : null} + {(url.length > 0 || message.length > 0) && isBlockedUrl === false ? ( + + ) : null} + + + ); +}; + +export default SnapAccountRedirectContent; diff --git a/ui/pages/snap-account-redirect/components/snap-account-redirect-message.tsx b/ui/pages/snap-account-redirect/components/snap-account-redirect-message.tsx new file mode 100644 index 000000000000..6446172c133e --- /dev/null +++ b/ui/pages/snap-account-redirect/components/snap-account-redirect-message.tsx @@ -0,0 +1,36 @@ +import React from 'react'; +import { Display, TextVariant } from '../../../helpers/constants/design-system'; +import { Box, Text } from '../../../components/component-library'; +import { SnapAccountRedirectProps } from '../snap-account-redirect'; +import { SnapDelineator } from '../../../components/app/snaps/snap-delineator'; +import UrlDisplayBox from './url-display-box'; + +const SnapAccountRedirectMessage = ({ + snapName, + url, + message, +}: Pick) => { + /* eslint-disable no-negated-condition */ + return ( + + {message !== '' ? ( + + {message} + + ) : null} + {url.length > 0 ? ( + + + + ) : null} + + ); +}; + +export default React.memo(SnapAccountRedirectMessage); diff --git a/ui/pages/snap-account-redirect/components/url-display-box.tsx b/ui/pages/snap-account-redirect/components/url-display-box.tsx new file mode 100644 index 000000000000..221844f7efb9 --- /dev/null +++ b/ui/pages/snap-account-redirect/components/url-display-box.tsx @@ -0,0 +1,39 @@ +import React from 'react'; +import { SnapAccountRedirectProps } from '../snap-account-redirect'; +import { + AlignItems, + BackgroundColor, + BorderRadius, + BorderColor, + Display, + TextColor, + TextVariant, +} from '../../../helpers/constants/design-system'; +import { Box, Text } from '../../../components/component-library'; +import RedirectUrlIcon from './redirect-url-icon'; + +const UrlDisplayBox = ({ url }: Pick) => { + return ( + + + {url} + + + + ); +}; + +export default React.memo(UrlDisplayBox); diff --git a/ui/pages/snap-account-redirect/create-snap-redirect.test.tsx b/ui/pages/snap-account-redirect/create-snap-redirect.test.tsx new file mode 100644 index 000000000000..1683b954812a --- /dev/null +++ b/ui/pages/snap-account-redirect/create-snap-redirect.test.tsx @@ -0,0 +1,97 @@ +import React from 'react'; +import { render } from '@testing-library/react'; +import { SnapAccountRedirect } from '.'; + +// If you're using some kind of global variable (like `global.platform` in your component), you might want to mock it. +global.platform = { + openTab: jest.fn(), +}; + +const mockUrl = 'https://metamask.github.io/snap-simple-keyring/1.0.0/'; +const mockSnapName = 'Snap Simple Keyring'; +const mockMessage = 'Redirecting to Snap Simple Keyring'; + +describe('', () => { + it('renders the url and message when provided and isBlockedUrl is false', () => { + const { getByTestId, container } = render( + , + ); + + expect(getByTestId('snap-account-redirect-content-title')); + expect(getByTestId('snap-account-redirect-message')).toHaveTextContent( + mockMessage, + ); + expect( + getByTestId('snap-account-redirect-url-display-box'), + ).toHaveTextContent(mockUrl); + expect(container).toMatchSnapshot(); + }); + + it('renders alert banner and does not render message or url when isBlockedUrl is true', () => { + const { queryByTestId } = render( + , + ); + + expect( + queryByTestId('snap-account-redirect-content-description'), + ).toBeNull(); + expect(queryByTestId('snap-account-redirect-message')).toBeNull(); + expect(queryByTestId('snap-account-redirect-url-display-box')).toBeNull(); + expect( + queryByTestId('snap-account-redirect-content-blocked-url-banner'), + ).toBeDefined(); + }); + + it('does not render URL display box when URL is empty', () => { + const { queryByTestId } = render( + , + ); + expect(queryByTestId('snap-account-redirect-message')).toHaveTextContent( + mockMessage, + ); + expect(queryByTestId('snap-account-redirect-url-display-box')).toBeNull(); + }); + + it('does not render message when message is empty', () => { + const { queryByTestId } = render( + , + ); + + expect( + queryByTestId('snap-account-redirect-url-display-box'), + ).toHaveTextContent(mockUrl); + expect(queryByTestId('snap-account-redirect-message')).toBeNull(); + }); + + it('does not render message/url box when message and url are empty', () => { + const { queryByTestId } = render( + , + ); + expect(queryByTestId('snap-account-redirect-message-container')).toBeNull(); + }); +}); diff --git a/ui/pages/snap-account-redirect/index.ts b/ui/pages/snap-account-redirect/index.ts new file mode 100644 index 000000000000..2c62a3b40670 --- /dev/null +++ b/ui/pages/snap-account-redirect/index.ts @@ -0,0 +1 @@ +export { default as SnapAccountRedirect } from './snap-account-redirect'; diff --git a/ui/pages/snap-account-redirect/snap-account-redirect.stories.tsx b/ui/pages/snap-account-redirect/snap-account-redirect.stories.tsx new file mode 100644 index 000000000000..a9cb210597f2 --- /dev/null +++ b/ui/pages/snap-account-redirect/snap-account-redirect.stories.tsx @@ -0,0 +1,38 @@ +import React from 'react'; +import { Meta } from '@storybook/react'; +import SnapAccountRedirect from './snap-account-redirect'; + +export default { + title: 'Components/UI/SnapAccountRedirect', + argTypes: { + snapName: { + control: { + type: 'text', + }, + }, + isBlockedUrl: { + control: { + type: 'boolean', + }, + }, + url: { + control: { + type: 'text', + }, + }, + message: { + control: { + type: 'text', + }, + }, + }, + args: { + snapName: 'Snap Simple Keyring', + isBlockedUrl: false, + url: 'https://metamask.github.io/snap-simple-keyring/0.2.4/', + message: 'Redirecting to Snap Simple Keyring', + }, +} as Meta; + +export const DefaultStory = (args) => ; +DefaultStory.storyName = 'Default'; diff --git a/ui/pages/snap-account-redirect/snap-account-redirect.tsx b/ui/pages/snap-account-redirect/snap-account-redirect.tsx new file mode 100644 index 000000000000..fccfe2c5acd1 --- /dev/null +++ b/ui/pages/snap-account-redirect/snap-account-redirect.tsx @@ -0,0 +1,53 @@ +import React from 'react'; +import { Box } from '../../components/component-library'; +import { + AlignItems, + BlockSize, + BorderStyle, + Display, + FlexDirection, +} from '../../helpers/constants/design-system'; +import { SnapAccountRedirectContent } from './components'; + +export interface SnapAccountRedirectProps { + url: string; + snapName: string; + isBlockedUrl: boolean; + message: string; +} + +const SnapAccountRedirect = ({ + url, + snapName, + isBlockedUrl, + message, +}: SnapAccountRedirectProps) => { + return ( + + + + + + ); +}; + +export default SnapAccountRedirect; diff --git a/yarn.lock b/yarn.lock index 1fc9841661e5..81ad6956cf55 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4144,9 +4144,9 @@ __metadata: languageName: node linkType: hard -"@metamask/eth-snap-keyring@npm:^1.0.0-rc.1": - version: 1.0.0-rc.1 - resolution: "@metamask/eth-snap-keyring@npm:1.0.0-rc.1" +"@metamask/eth-snap-keyring@npm:1.0.0-rc.2": + version: 1.0.0-rc.2 + resolution: "@metamask/eth-snap-keyring@npm:1.0.0-rc.2" dependencies: "@ethereumjs/tx": "npm:^4.2.0" "@metamask/eth-sig-util": "npm:^7.0.0" @@ -4156,7 +4156,7 @@ __metadata: "@types/uuid": "npm:^9.0.1" superstruct: "npm:^1.0.3" uuid: "npm:^9.0.0" - checksum: afb50a755823e249e80731da7e2f3ab4246b900020135eb4c3c657d663f2d44a7a6281f29a9a453f5975bd493e4322bede3d5bea0c94871dda8ce0ceced5f186 + checksum: 02e3392db1e0bbea0aac8fa99e55ad2490d99d23568fb1587a31a1510ff41aad3bc29ec5514256e727ec2c6a34c111327600cee8a2d48e4745bdc8665d30770b languageName: node linkType: hard @@ -8240,6 +8240,13 @@ __metadata: languageName: node linkType: hard +"@types/webextension-polyfill@npm:^0.10.4": + version: 0.10.4 + resolution: "@types/webextension-polyfill@npm:0.10.4" + checksum: f5aa3d24d3214a1d82328dc14c2f9b8d07d36e149281968959992f29ef78017dc1cf6055759d18ce46d29528d22b87fcfb30a289432382704869c7f3e1962f52 + languageName: node + linkType: hard + "@types/ws@npm:*": version: 8.2.2 resolution: "@types/ws@npm:8.2.2" @@ -23816,7 +23823,7 @@ __metadata: "@metamask/eth-json-rpc-middleware": "npm:^11.0.0" "@metamask/eth-keyring-controller": "npm:^13.0.1" "@metamask/eth-ledger-bridge-keyring": "npm:^0.15.0" - "@metamask/eth-snap-keyring": "npm:^1.0.0-rc.1" + "@metamask/eth-snap-keyring": "npm:1.0.0-rc.2" "@metamask/eth-token-tracker": "npm:^4.0.0" "@metamask/eth-trezor-keyring": "npm:^1.1.0" "@metamask/etherscan-link": "npm:^2.2.0" @@ -23908,6 +23915,7 @@ __metadata: "@types/sinon": "npm:^10.0.13" "@types/w3c-web-hid": "npm:^1.0.3" "@types/watchify": "npm:^3.11.1" + "@types/webextension-polyfill": "npm:^0.10.4" "@types/yargs": "npm:^17.0.8" "@typescript-eslint/eslint-plugin": "npm:^5.30.7" "@typescript-eslint/parser": "npm:^5.30.7"