diff --git a/.github/workflows/trigger-metamask-institutional-e2e-ci.yml b/.github/workflows/trigger-metamask-institutional-e2e-ci.yml new file mode 100644 index 000000000000..3701c04241ad --- /dev/null +++ b/.github/workflows/trigger-metamask-institutional-e2e-ci.yml @@ -0,0 +1,28 @@ +name: Trigger MetaMask Institutional E2E CI + +on: + push: + branches: [ Version-v*, master ] + pull_request: + branches: [ Version-v*, master ] + types: + - opened + - reopened + - synchronize + - ready_for_review + +jobs: + trigger-mmi-e2e-ci: + runs-on: ubuntu-latest + if: ${{ (!github.event.pull_request.draft && (startsWith(github.head_ref, 'Version-v') || github.head_ref == 'master')) || (!github.event.pull_request) }} + steps: + - name: Trigger MetaMask Institutional E2E CI + env: + MMI_E2E_CI_TOKEN: ${{ secrets.MMI_E2E_CI_TOKEN }} + run: | + curl -L -X POST \ + -H "Accept: application/vnd.github+json" \ + -H "Authorization: Bearer $MMI_E2E_CI_TOKEN" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + https://api.github.com/repos/consensys-vertical-apps/mmi-extension-e2e/dispatches \ + -d '{"event_type": "mm-triggered", "client_payload":{"ref_name": "${{ github.ref_name }}", "sha": "${{ github.sha }}"}}' diff --git a/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch b/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch similarity index 78% rename from .yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch rename to .yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch index 956b9fe308f7..4b9ccd4f64d9 100644 --- a/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch +++ b/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch @@ -1,5 +1,5 @@ diff --git a/dist/KeyringController.d.ts b/dist/KeyringController.d.ts -index ccfd5ca9accf782b0a612ab1666501898cf2abb9..f772552156491b308f2aa29a734a5138237d951e 100644 +index d6d70eea2de56310b49fa9dcf1e90afae3ddb202..4518aed8e03376d76e5ab6ef0971529e65563fd3 100644 --- a/dist/KeyringController.d.ts +++ b/dist/KeyringController.d.ts @@ -1,10 +1,11 @@ @@ -10,13 +10,12 @@ index ccfd5ca9accf782b0a612ab1666501898cf2abb9..f772552156491b308f2aa29a734a5138 import { BaseControllerV2 } from '@metamask/base-controller'; import type { PersonalMessageParams, TypedMessageParams } from '@metamask/message-manager'; import type { PreferencesController } from '@metamask/preferences-controller'; --import { type Hex, type Keyring, type Json } from '@metamask/utils'; -+import type { Hex, Keyring, Json } from '@metamask/utils'; + import type { Eip1024EncryptedData, Hex, Keyring, Json } from '@metamask/utils'; +import type { KeyringController as EthKeyringController } from '@metamask/eth-keyring-controller'; import type { Patch } from 'immer'; declare const name = "KeyringController"; /** -@@ -135,6 +136,10 @@ export declare class KeyringController extends BaseControllerV2() => }, } as any as jest.Mocked); -describe('EncryptionPublicKeyController', () => { +describe('DecryptMessageController', () => { let decryptMessageController: DecryptMessageController; const decryptMessageManagerConstructorMock = @@ -65,6 +67,19 @@ describe('EncryptionPublicKeyController', () => { const decryptMessageManagerMock = createDecryptMessageManagerMock(); + const mockMessengerAction = ( + action: string, + callback: (actionName: string, ...args: any[]) => any, + ) => { + messengerMock.call.mockImplementation((actionName, ...rest) => { + if (actionName === action) { + return callback(actionName, ...rest); + } + + return Promise.resolve(); + }); + }; + beforeEach(() => { jest.resetAllMocks(); @@ -116,12 +131,18 @@ describe('EncryptionPublicKeyController', () => { it('should decrypt message', async () => { const messageToDecrypt = { ...messageMock, - data: '0x7b22666f6f223a22626172227d', + data: messageDataMock, }; + const decryptMessageActionCallbackMock = jest + .fn() + .mockReturnValue('decryptedMessage'); decryptMessageManagerMock.approveMessage.mockResolvedValue( messageToDecrypt, ); - keyringControllerMock.decryptMessage.mockResolvedValue('decryptedMessage'); + mockMessengerAction( + 'KeyringController:decryptMessage', + decryptMessageActionCallbackMock, + ); getStateMock.mockReturnValue(mockExtState); const result = await decryptMessageController.decryptMessage( @@ -132,8 +153,9 @@ describe('EncryptionPublicKeyController', () => { expect(decryptMessageManagerMock.approveMessage).toBeCalledWith( messageToDecrypt, ); - expect(keyringControllerMock.decryptMessage).toBeCalledTimes(1); - expect(keyringControllerMock.decryptMessage).toBeCalledWith( + expect(decryptMessageActionCallbackMock).toBeCalledTimes(1); + expect(decryptMessageActionCallbackMock).toBeCalledWith( + 'KeyringController:decryptMessage', messageToDecrypt, ); expect(decryptMessageManagerMock.setMessageStatusAndResult).toBeCalledTimes( @@ -147,15 +169,31 @@ describe('EncryptionPublicKeyController', () => { expect(result).toBe(mockExtState); }); + it('should throw when decrypting invalid message', async () => { + const messageToDecrypt = { + ...messageMock, + data: '0x7b2022666f6f223a202262617222207d', + }; + decryptMessageManagerMock.approveMessage.mockResolvedValue( + messageToDecrypt, + ); + + expect( + decryptMessageController.decryptMessage(messageToDecrypt), + ).rejects.toThrow('Invalid encrypted data.'); + }); + it('should cancel decrypt request', async () => { const messageToDecrypt = { ...messageMock, - data: '0x7b22666f6f223a22626172227d', + data: messageDataMock, }; decryptMessageManagerMock.approveMessage.mockResolvedValue( messageToDecrypt, ); - keyringControllerMock.decryptMessage.mockRejectedValue(new Error('error')); + mockMessengerAction('KeyringController:decryptMessage', async () => { + throw new Error('error'); + }); getStateMock.mockReturnValue(mockExtState); return expect( @@ -166,10 +204,13 @@ describe('EncryptionPublicKeyController', () => { it('should decrypt message inline', async () => { const messageToDecrypt = { ...messageMock, - data: '0x7b22666f6f223a22626172227d', + data: messageDataMock, }; decryptMessageManagerMock.getMessage.mockReturnValue(messageToDecrypt); - keyringControllerMock.decryptMessage.mockResolvedValue('decryptedMessage'); + mockMessengerAction( + 'KeyringController:decryptMessage', + async () => 'decryptedMessage', + ); getStateMock.mockReturnValue(mockExtState); const result = await decryptMessageController.decryptMessageInline( @@ -184,6 +225,17 @@ describe('EncryptionPublicKeyController', () => { expect(result).toBe(mockExtState); }); + it('should throw when decrypting invalid message inline', async () => { + const messageToDecrypt = { + ...messageMock, + data: '0x7b2022666f6f223a202262617222207d', + }; + + expect( + decryptMessageController.decryptMessageInline(messageToDecrypt), + ).rejects.toThrow('Invalid encrypted data.'); + }); + it('should be able to cancel decrypt message', async () => { decryptMessageManagerMock.rejectMessage.mockResolvedValue(messageMock); getStateMock.mockReturnValue(mockExtState); diff --git a/app/scripts/controllers/decrypt-message.ts b/app/scripts/controllers/decrypt-message.ts index a0d1808063f1..1ace629dd995 100644 --- a/app/scripts/controllers/decrypt-message.ts +++ b/app/scripts/controllers/decrypt-message.ts @@ -5,7 +5,6 @@ import { DecryptMessageParams, DecryptMessageParamsMetamask, } from '@metamask/message-manager'; -import { KeyringController } from '@metamask/eth-keyring-controller'; import { AbstractMessage, AbstractMessageManager, @@ -25,6 +24,8 @@ import { } from '@metamask/approval-controller'; import { ApprovalType, ORIGIN_METAMASK } from '@metamask/controller-utils'; import { Patch } from 'immer'; +import type { KeyringControllerDecryptMessageAction } from '@metamask/keyring-controller'; +import { Eip1024EncryptedData, hasProperty, isObject } from '@metamask/utils'; import { MetaMetricsEventCategory } from '../../../shared/constants/metametrics'; import { stripHexPrefix } from '../../../shared/modules/hexstring-utils'; @@ -35,6 +36,37 @@ const stateMetadata = { unapprovedDecryptMsgCount: { persist: false, anonymous: false }, }; +/** + * Type guard that checks for the presence of the required properties + * for EIP-1024 encrypted data. + * + * See: https://eips.ethereum.org/EIPS/eip-1024 + * + * @param message - The message to check. + * @param message.from - The sender of the message. + * @param message.data - The EIP-1024 encrypted data. + * @returns Whether the message is an EIP-1024 encrypted message. + */ +export const isEIP1024EncryptedMessage = (message: { + from: string; + data: unknown; +}): message is { + from: string; + data: Eip1024EncryptedData; +} => { + if ( + isObject(message.data) && + hasProperty(message.data, 'version') && + hasProperty(message.data, 'nonce') && + hasProperty(message.data, 'ephemPublicKey') && + hasProperty(message.data, 'ciphertext') + ) { + return true; + } + + return false; +}; + export const getDefaultState = () => ({ unapprovedDecryptMsgs: {}, unapprovedDecryptMsgCount: 0, @@ -70,7 +102,11 @@ export type DecryptMessageControllerActions = GetDecryptMessageState; export type DecryptMessageControllerEvents = DecryptMessageStateChange; -type AllowedActions = AddApprovalRequest | AcceptRequest | RejectRequest; +type AllowedActions = + | AddApprovalRequest + | AcceptRequest + | RejectRequest + | KeyringControllerDecryptMessageAction; export type DecryptMessageControllerMessenger = RestrictedControllerMessenger< typeof controllerName, @@ -82,7 +118,6 @@ export type DecryptMessageControllerMessenger = RestrictedControllerMessenger< export type DecryptMessageControllerOptions = { getState: () => any; - keyringController: KeyringController; messenger: DecryptMessageControllerMessenger; metricsEvent: (payload: any, options?: any) => void; }; @@ -99,8 +134,6 @@ export default class DecryptMessageController extends BaseControllerV2< private _getState: () => any; - private _keyringController: KeyringController; - private _metricsEvent: (payload: any, options?: any) => void; private _decryptMessageManager: DecryptMessageManager; @@ -110,13 +143,11 @@ export default class DecryptMessageController extends BaseControllerV2< * * @param options - The controller options. * @param options.getState - Callback to retrieve all user state. - * @param options.keyringController - An instance of a keyring controller used to decrypt message * @param options.messenger - A reference to the messaging system. * @param options.metricsEvent - A function for emitting a metric event. */ constructor({ getState, - keyringController, metricsEvent, messenger, }: DecryptMessageControllerOptions) { @@ -127,7 +158,6 @@ export default class DecryptMessageController extends BaseControllerV2< state: getDefaultState(), }); this._getState = getState; - this._keyringController = keyringController; this._metricsEvent = metricsEvent; this.hub = new EventEmitter(); @@ -216,7 +246,12 @@ export default class DecryptMessageController extends BaseControllerV2< await this._decryptMessageManager.approveMessage(messageParams); cleanMessageParams.data = this._parseMessageData(cleanMessageParams.data); - const rawMessage = await this._keyringController.decryptMessage( + if (!isEIP1024EncryptedMessage(cleanMessageParams)) { + throw new Error('Invalid encrypted data.'); + } + + const rawMessage = await this.messagingSystem.call( + 'KeyringController:decryptMessage', cleanMessageParams, ); @@ -243,7 +278,11 @@ export default class DecryptMessageController extends BaseControllerV2< async decryptMessageInline(messageParams: DecryptMessageParamsMetamask) { const messageId = messageParams.metamaskId as string; messageParams.data = this._parseMessageData(messageParams.data); - const rawMessage = await this._keyringController.decryptMessage( + if (!isEIP1024EncryptedMessage(messageParams)) { + throw new Error('Invalid encrypted data.'); + } + const rawMessage = await this.messagingSystem.call( + 'KeyringController:decryptMessage', messageParams, ); diff --git a/app/scripts/controllers/detect-tokens.js b/app/scripts/controllers/detect-tokens.js index 46719aebcced..86c8cc222856 100644 --- a/app/scripts/controllers/detect-tokens.js +++ b/app/scripts/controllers/detect-tokens.js @@ -28,7 +28,6 @@ export default class DetectTokensController { * @param config.interval * @param config.preferences * @param config.network - * @param config.keyringMemStore * @param config.tokenList * @param config.tokensController * @param config.assetsContractController @@ -40,7 +39,6 @@ export default class DetectTokensController { interval = DEFAULT_INTERVAL, preferences, network, - keyringMemStore, tokenList, tokensController, assetsContractController = null, @@ -52,7 +50,6 @@ export default class DetectTokensController { this.preferences = preferences; this.interval = interval; this.network = network; - this.keyringMemStore = keyringMemStore; this.tokenList = tokenList; this.useTokenDetection = this.preferences?.store.getState().useTokenDetection; @@ -91,6 +88,8 @@ export default class DetectTokensController { this.restartTokenDetection({ chainId: this.chainId }); } }); + + this.#registerKeyringHandlers(); } /** @@ -236,26 +235,6 @@ export default class DetectTokensController { }, interval); } - /** - * In setter when isUnlocked is updated to true, detectNewTokens and restart polling - * - * @type {object} - */ - set keyringMemStore(keyringMemStore) { - if (!keyringMemStore) { - return; - } - this._keyringMemStore = keyringMemStore; - this._keyringMemStore.subscribe(({ isUnlocked }) => { - if (this.isUnlocked !== isUnlocked) { - this.isUnlocked = isUnlocked; - if (isUnlocked) { - this.restartTokenDetection(); - } - } - }); - } - /** * @type {object} */ @@ -275,4 +254,22 @@ export default class DetectTokensController { return this.isOpen && this.isUnlocked; } /* eslint-enable accessor-pairs */ + + /** + * Constructor helper to register listeners on the keyring + * locked state changes + */ + #registerKeyringHandlers() { + const { isUnlocked } = this.messenger.call('KeyringController:getState'); + this.isUnlocked = isUnlocked; + + this.messenger.subscribe('KeyringController:unlock', () => { + this.isUnlocked = true; + this.restartTokenDetection(); + }); + + this.messenger.subscribe('KeyringController:lock', () => { + this.isUnlocked = false; + }); + } } diff --git a/app/scripts/controllers/detect-tokens.test.js b/app/scripts/controllers/detect-tokens.test.js index 8d1c182c249e..c7306dc83295 100644 --- a/app/scripts/controllers/detect-tokens.test.js +++ b/app/scripts/controllers/detect-tokens.test.js @@ -1,7 +1,6 @@ import { strict as assert } from 'assert'; import sinon from 'sinon'; import nock from 'nock'; -import { ObservableStore } from '@metamask/obs-store'; import BigNumber from 'bignumber.js'; import { ControllerMessenger } from '@metamask/base-controller'; import { @@ -16,25 +15,30 @@ import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils'; import DetectTokensController from './detect-tokens'; import PreferencesController from './preferences'; -function buildMessenger() { - return new ControllerMessenger().getRestricted({ - name: 'DetectTokensController', - allowedEvents: ['NetworkController:stateChange'], - }); -} - describe('DetectTokensController', function () { let sandbox, assetsContractController, - keyringMemStore, network, preferences, provider, tokensController, - tokenListController; + tokenListController, + messenger; const noop = () => undefined; + const getRestrictedMessenger = () => { + return messenger.getRestricted({ + name: 'DetectTokensController', + allowedActions: ['KeyringController:getState'], + allowedEvents: [ + 'NetworkController:stateChange', + 'KeyringController:lock', + 'KeyringController:unlock', + ], + }); + }; + const networkControllerProviderConfig = { getAccounts: noop, }; @@ -198,7 +202,11 @@ describe('DetectTokensController', function () { .reply(200, { error: 'ChainId 3 is not supported' }) .persist(); - keyringMemStore = new ObservableStore({ isUnlocked: false }); + messenger = new ControllerMessenger(); + messenger.registerActionHandler('KeyringController:getState', () => ({ + isUnlocked: true, + })); + const networkControllerMessenger = new ControllerMessenger(); network = new NetworkController({ messenger: networkControllerMessenger, @@ -263,7 +271,11 @@ describe('DetectTokensController', function () { it('should poll on correct interval', async function () { const stub = sinon.stub(global, 'setInterval'); - new DetectTokensController({ messenger: buildMessenger(), interval: 1337 }); // eslint-disable-line no-new + // eslint-disable-next-line no-new + new DetectTokensController({ + messenger: getRestrictedMessenger(), + interval: 1337, + }); assert.strictEqual(stub.getCall(0).args[1], 1337); stub.restore(); }); @@ -272,10 +284,9 @@ describe('DetectTokensController', function () { const clock = sandbox.useFakeTimers(); await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ - messenger: buildMessenger(), + messenger: getRestrictedMessenger(), preferences, network, - keyringMemStore, tokenList: tokenListController, tokensController, assetsContractController, @@ -309,10 +320,9 @@ describe('DetectTokensController', function () { }); await tokenListController.start(); const controller = new DetectTokensController({ - messenger: buildMessenger(), + messenger: getRestrictedMessenger(), preferences, network, - keyringMemStore, tokenList: tokenListController, tokensController, assetsContractController, @@ -333,10 +343,9 @@ describe('DetectTokensController', function () { sandbox.useFakeTimers(); await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ - messenger: buildMessenger(), + messenger: getRestrictedMessenger(), preferences, network, - keyringMemStore, tokenList: tokenListController, tokensController, assetsContractController, @@ -385,10 +394,9 @@ describe('DetectTokensController', function () { sandbox.useFakeTimers(); await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ - messenger: buildMessenger(), + messenger: getRestrictedMessenger(), preferences, network, - keyringMemStore, tokenList: tokenListController, tokensController, assetsContractController, @@ -444,10 +452,9 @@ describe('DetectTokensController', function () { it('should trigger detect new tokens when change address', async function () { sandbox.useFakeTimers(); const controller = new DetectTokensController({ - messenger: buildMessenger(), + messenger: getRestrictedMessenger(), preferences, network, - keyringMemStore, tokenList: tokenListController, tokensController, assetsContractController, @@ -464,10 +471,9 @@ describe('DetectTokensController', function () { it('should trigger detect new tokens when submit password', async function () { sandbox.useFakeTimers(); const controller = new DetectTokensController({ - messenger: buildMessenger(), + messenger: getRestrictedMessenger(), preferences, network, - keyringMemStore, tokenList: tokenListController, tokensController, assetsContractController, @@ -475,18 +481,38 @@ describe('DetectTokensController', function () { controller.isOpen = true; controller.selectedAddress = '0x0'; const stub = sandbox.stub(controller, 'detectNewTokens'); - await controller._keyringMemStore.updateState({ isUnlocked: true }); + + messenger.publish('KeyringController:unlock'); + sandbox.assert.called(stub); + assert.equal(controller.isUnlocked, true); + }); + + it('should not be active after lock event is emitted', async function () { + sandbox.useFakeTimers(); + const controller = new DetectTokensController({ + messenger: getRestrictedMessenger(), + preferences, + network, + tokenList: tokenListController, + tokensController, + assetsContractController, + }); + controller.isOpen = true; + + messenger.publish('KeyringController:lock'); + + assert.equal(controller.isUnlocked, false); + assert.equal(controller.isActive, false); }); it('should not trigger detect new tokens when not unlocked', async function () { const clock = sandbox.useFakeTimers(); await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ - messenger: buildMessenger(), + messenger: getRestrictedMessenger(), preferences, network, - keyringMemStore, tokenList: tokenListController, tokensController, assetsContractController, @@ -505,10 +531,9 @@ describe('DetectTokensController', function () { const clock = sandbox.useFakeTimers(); await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ - messenger: buildMessenger(), + messenger: getRestrictedMessenger(), preferences, network, - keyringMemStore, tokensController, assetsContractController, }); diff --git a/app/scripts/controllers/mmi-controller.js b/app/scripts/controllers/mmi-controller.js index 355d4ad27352..1b259d9f6dd9 100644 --- a/app/scripts/controllers/mmi-controller.js +++ b/app/scripts/controllers/mmi-controller.js @@ -293,7 +293,7 @@ export default class MMIController extends EventEmitter { ); for (let i = 0; i < newAccounts.length; i++) { - await this.keyringController.addNewAccount(keyring); + await this.keyringController.addNewAccountForKeyring(keyring); } const allAccounts = await this.keyringController.getAccounts(); diff --git a/app/scripts/controllers/mmi-controller.test.js b/app/scripts/controllers/mmi-controller.test.js index e4b26dd62f5b..31fe04e9bb0b 100644 --- a/app/scripts/controllers/mmi-controller.test.js +++ b/app/scripts/controllers/mmi-controller.test.js @@ -1,5 +1,5 @@ /* eslint-disable */ -import { KeyringController } from '@metamask/eth-keyring-controller'; +import { KeyringController } from '@metamask/keyring-controller'; import { MmiConfigurationController } from '@metamask-institutional/custody-keyring'; import { TransactionUpdateController } from '@metamask-institutional/transaction-update'; import { SignatureController } from '@metamask/signature-controller'; @@ -13,9 +13,18 @@ describe('MMIController', function () { let mmiController; beforeEach(function () { + const mockMessenger = { + call: jest.fn(() => ({ + catch: jest.fn(), + })), + registerActionHandler: jest.fn(), + publish: jest.fn(), + }; + mmiController = new MMIController({ mmiConfigurationController: new MmiConfigurationController(), keyringController: new KeyringController({ + messenger: mockMessenger, initState: {}, }), transactionUpdateController: new TransactionUpdateController({ @@ -35,13 +44,10 @@ describe('MMIController', function () { onNetworkStateChange: jest.fn(), }), signatureController: new SignatureController({ - messenger: { - registerActionHandler: jest.fn(), - publish: jest.fn(), - call: jest.fn(), - }, + messenger: mockMessenger, keyringController: new KeyringController({ initState: {}, + messenger: mockMessenger, }), isEthSignEnabled: jest.fn(), getAllState: jest.fn(), @@ -73,11 +79,7 @@ describe('MMIController', function () { qrHardwareStore: { subscribe: jest.fn(), }, - messenger: { - call: jest.fn(() => ({ - catch: jest.fn(), - })), - }, + messenger: mockMessenger, }), custodianEventHandlerFactory: jest.fn(), }); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index ee08fe92682b..42f6fd9a19e2 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1142,7 +1142,12 @@ export default class MetamaskController extends EventEmitter { const detectTokensControllerMessenger = this.controllerMessenger.getRestricted({ name: 'DetectTokensController', - allowedEvents: ['NetworkController:stateChange'], + allowedActions: ['KeyringController:getState'], + allowedEvents: [ + 'NetworkController:stateChange', + 'KeyringController:lock', + 'KeyringController:unlock', + ], }); this.detectTokensController = new DetectTokensController({ messenger: detectTokensControllerMessenger, @@ -1150,7 +1155,6 @@ export default class MetamaskController extends EventEmitter { tokensController: this.tokensController, assetsContractController: this.assetsContractController, network: this.networkController, - keyringMemStore: this.keyringController.memStore, tokenList: this.tokenListController, trackMetaMetricsEvent: this.metaMetricsController.trackEvent.bind( this.metaMetricsController, @@ -1369,13 +1373,13 @@ export default class MetamaskController extends EventEmitter { this.networkController.lookupNetwork(); this.decryptMessageController = new DecryptMessageController({ getState: this.getState.bind(this), - keyringController: this.keyringController, messenger: this.controllerMessenger.getRestricted({ name: 'DecryptMessageController', allowedActions: [ `${this.approvalController.name}:addRequest`, `${this.approvalController.name}:acceptRequest`, `${this.approvalController.name}:rejectRequest`, + `${this.coreKeyringController.name}:decryptMessage`, ], }), metricsEvent: this.metaMetricsController.trackEvent.bind( @@ -1438,7 +1442,7 @@ export default class MetamaskController extends EventEmitter { ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) this.mmiController = new MMIController({ mmiConfigurationController: this.mmiConfigurationController, - keyringController: this.keyringController, + keyringController: this.coreKeyringController, txController: this.txController, securityProviderRequest: this.securityProviderRequest.bind(this), preferencesController: this.preferencesController, diff --git a/development/README.md b/development/README.md index 560defc57ad2..d00880addb83 100644 --- a/development/README.md +++ b/development/README.md @@ -76,24 +76,26 @@ To debug in a production Sentry environment: Errors reported whilst using the extension will be displayed in Sentry's `Issues` page. +To debug in test build we need to comment out the IF condition https://github.com/MetaMask/metamask-extension/blob/develop/app/scripts/lib/setupSentry.js#L392 + ## Source Maps ### Debugging production builds using Source Maps To unbundle the extensions compiled and minified JavaScript using Source Maps: -- Open Chrome DevTools to inspect the `background.html` or `home.html` view +- Open Chrome DevTools to inspect the `background.html` or `home.html` view - Click on the `Sources` tab in Chrome DevTools - In the Sources tab, click on the `Page` panel - Expand the file directory in the Page panel until you see the source files you're after -- Select a source file in the Page panel +- Select a source file in the Page panel ``` chrome-extension://{EXTENSION_ID}/common-0.js ``` - Double click the source file to open it in the Workspace - Right click in the body of the source file and select `Add source map...` - Enter the path to the corresponding source map file, and Click `Add` -``` +``` file:///{LOCAL_FILE_SYSTEM}/metamask-extension/dist/sourcemaps/common-0.js.map ``` - Repeat the steps above as necessary adding all the relevant source map files diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 0530d84fa850..e25e02879fd4 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -1677,6 +1677,14 @@ "browserify>url": true } }, + "@metamask/name-controller": { + "globals": { + "fetch": true + }, + "packages": { + "@metamask/base-controller": true + } + }, "@metamask/network-controller": { "globals": { "URL": true, diff --git a/lavamoat/browserify/desktop/policy.json b/lavamoat/browserify/desktop/policy.json index bdd2ba925bb9..7ede24c7bbb7 100644 --- a/lavamoat/browserify/desktop/policy.json +++ b/lavamoat/browserify/desktop/policy.json @@ -1828,6 +1828,14 @@ "browserify>url": true } }, + "@metamask/name-controller": { + "globals": { + "fetch": true + }, + "packages": { + "@metamask/base-controller": true + } + }, "@metamask/network-controller": { "globals": { "URL": true, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 2f97fd964747..d807734aedf6 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -1828,6 +1828,14 @@ "browserify>url": true } }, + "@metamask/name-controller": { + "globals": { + "fetch": true + }, + "packages": { + "@metamask/base-controller": true + } + }, "@metamask/network-controller": { "globals": { "URL": true, diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 8f1f171aa469..9eae2c148db8 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -1677,6 +1677,14 @@ "browserify>url": true } }, + "@metamask/name-controller": { + "globals": { + "fetch": true + }, + "packages": { + "@metamask/base-controller": true + } + }, "@metamask/network-controller": { "globals": { "URL": true, diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index 578f541b1d3e..da430864f696 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -1818,6 +1818,14 @@ "browserify>url": true } }, + "@metamask/name-controller": { + "globals": { + "fetch": true + }, + "packages": { + "@metamask/base-controller": true + } + }, "@metamask/network-controller": { "globals": { "URL": true, diff --git a/package.json b/package.json index c7f3c16c51cb..0dc76d4cae2d 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "user-actions-benchmark:chrome": "SELENIUM_BROWSER=chrome ts-node test/e2e/user-actions-benchmark.js", "benchmark:firefox": "SELENIUM_BROWSER=firefox ts-node test/e2e/benchmark.js", "build:test": "SEGMENT_HOST='https://api.segment.io' SEGMENT_WRITE_KEY='FAKE' SENTRY_DSN_DEV=https://fake@sentry.io/0000000 PORTFOLIO_URL=http://127.0.0.1:8080 yarn build test", - "build:test:flask": "yarn build test --build-type flask", + "build:test:flask": "BLOCKAID_FILE_CDN=storage.googleapis.com/ppom-mock-cdn yarn build test --build-type flask", "build:test:mv3": "ENABLE_MV3=true SEGMENT_HOST='https://api.segment.io' SEGMENT_WRITE_KEY='FAKE' SENTRY_DSN_DEV=https://fake@sentry.io/0000000 PORTFOLIO_URL=http://127.0.0.1:8080 yarn build test", "build:test:dev:mv3": "ENABLE_MV3=true SEGMENT_HOST='https://api.segment.io' SEGMENT_WRITE_KEY='FAKE' SENTRY_DSN_DEV=https://fake@sentry.io/0000000 PORTFOLIO_URL=http://127.0.0.1:8080 yarn build:dev testDev --apply-lavamoat=false", "test": "yarn lint && yarn test:unit && yarn test:unit:jest", @@ -202,7 +202,7 @@ "request@^2.88.2": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch", "request@^2.85.0": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch", "lavamoat-core@^14.2.0": "patch:lavamoat-core@npm%3A14.2.0#./.yarn/patches/lavamoat-core-npm-14.2.0-c453f4f755.patch", - "@metamask/keyring-controller@^7.2.0": "patch:@metamask/keyring-controller@npm%3A7.2.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch", + "@metamask/keyring-controller@^7.4.0": "patch:@metamask/keyring-controller@npm%3A7.4.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch", "@metamask/signature-controller@^5.3.0": "patch:@metamask/signature-controller@npm%3A5.3.0#./.yarn/patches/@metamask-signature-controller-npm-5.3.0-225628460b.patch", "semver@7.3.7": "^7.5.4", "semver@7.3.8": "^7.5.4" @@ -252,10 +252,11 @@ "@metamask/gas-fee-controller": "^6.0.1", "@metamask/jazzicon": "^2.0.0", "@metamask/key-tree": "^9.0.0", - "@metamask/keyring-controller": "^7.2.0", + "@metamask/keyring-controller": "^7.4.0", "@metamask/logo": "^3.1.1", "@metamask/message-manager": "^7.3.0", "@metamask/metamask-eth-abis": "^3.0.0", + "@metamask/name-controller": "^1.0.0", "@metamask/network-controller": "^12.1.1", "@metamask/notification-controller": "^3.0.0", "@metamask/obs-store": "^8.1.0", diff --git a/shared/constants/metametrics.ts b/shared/constants/metametrics.ts index ab43e4a546be..d510db5e7a38 100644 --- a/shared/constants/metametrics.ts +++ b/shared/constants/metametrics.ts @@ -525,7 +525,6 @@ export enum MetaMetricsEventName { NavSendButtonClicked = 'Send Button Clicked', NavSwapButtonClicked = 'Swap Button Clicked', NftAdded = 'NFT Added', - OnboardingWelcome = 'App Installed', OnboardingWalletCreationStarted = 'Wallet Setup Selected', OnboardingWalletImportStarted = 'Wallet Import Started', OnboardingWalletCreationAttempted = 'Wallet Password Created', diff --git a/test/data/mock-send-state.json b/test/data/mock-send-state.json index 8b4cbf16a709..14d4967d70f9 100644 --- a/test/data/mock-send-state.json +++ b/test/data/mock-send-state.json @@ -11,6 +11,7 @@ }, "appState": { "networkDropdownOpen": false, + "importNftsModal": { "open": false }, "gasIsLoading": false, "isLoading": false, "modal": { diff --git a/test/data/mock-state.json b/test/data/mock-state.json index 33a4db866132..7c42087b90fc 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -4,6 +4,7 @@ }, "appState": { "networkDropdownOpen": false, + "importNftsModal": { "open": false }, "gasIsLoading": false, "isLoading": false, "modal": { diff --git a/test/e2e/run-all.js b/test/e2e/run-all.js index 811f6b8fb94c..289d2b94f16b 100644 --- a/test/e2e/run-all.js +++ b/test/e2e/run-all.js @@ -108,6 +108,7 @@ async function main() { 'test-snap-manageAccount.spec.js', 'test-snap-rpc.spec.js', 'test-snap-lifecycle.spec.js', + 'ppom-toggle-settings.spec.js', ]; testPaths = testPaths.filter((p) => filteredTests.every((filteredTest) => !p.endsWith(filteredTest)), diff --git a/test/e2e/snaps/ppom-toggle-settings.spec.js b/test/e2e/snaps/ppom-toggle-settings.spec.js new file mode 100644 index 000000000000..f3a19631aca6 --- /dev/null +++ b/test/e2e/snaps/ppom-toggle-settings.spec.js @@ -0,0 +1,87 @@ +const { strict: assert } = require('assert'); +const { + withFixtures, + unlockWallet, + openDapp, + defaultGanacheOptions, + getWindowHandles, +} = require('../helpers'); +const FixtureBuilder = require('../fixture-builder'); + +const mainnetProviderConfig = { + providerConfig: { + chainId: '0x1', + nickname: '', + rpcUrl: '', + type: 'mainnet', + }, +}; + +describe('PPOM Settings', function () { + it('should not show the PPOM warning when toggle is off', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withNetworkController(mainnetProviderConfig) + .withPermissionControllerConnectedToTestDapp() + .build(), + ganacheOptions: defaultGanacheOptions, + title: this.test.title, + }, + async ({ driver }) => { + await driver.navigate(); + await unlockWallet(driver); + + await openDapp(driver); + await driver.clickElement('#maliciousPermit'); + const windowHandles = await getWindowHandles(driver, 3); + await driver.switchToWindow(windowHandles.popup); + + const blockaidResponseTitle = + '[data-testid="security-provider-banner-alert"]'; + const exists = await driver.isElementPresent(blockaidResponseTitle); + assert.equal(exists, false, 'This is a deceptive request'); + }, + ); + }); + + it('should show the PPOM warning when the toggle is on', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withNetworkController(mainnetProviderConfig) + .withPermissionControllerConnectedToTestDapp() + .build(), + ganacheOptions: defaultGanacheOptions, + title: this.test.title, + }, + async ({ driver }) => { + await driver.navigate(); + await unlockWallet(driver); + + await driver.clickElement( + '[data-testid="account-options-menu-button"]', + ); + + await driver.clickElement({ text: 'Settings', tag: 'div' }); + await driver.clickElement({ text: 'Experimental', tag: 'div' }); + + await driver.clickElement( + '[data-testid="settings-toggle-security-alert-blockaid"] .toggle-button > div', + ); + + await openDapp(driver); + await driver.clickElement('#maliciousPermit'); + const windowHandles = await getWindowHandles(driver, 3); + await driver.switchToWindow(windowHandles.popup); + + const blockaidResponseTitle = + '[data-testid="security-provider-banner-alert"]'; + const exists = await driver.isElementPresent(blockaidResponseTitle); + assert.equal(exists, true, 'This is a deceptive request'); + }, + ); + }); +}); diff --git a/ui/components/app/app-components.scss b/ui/components/app/app-components.scss index c4dc26714527..c5c610d5dc51 100644 --- a/ui/components/app/app-components.scss +++ b/ui/components/app/app-components.scss @@ -95,6 +95,8 @@ @import 'network-account-balance-header/index'; @import 'approve-content-card/index'; @import 'transaction-alerts/transaction-alerts'; +@import 'name/index'; +@import 'name/name-details/index'; ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) @import '../institutional/interactive-replacement-token-notification/index'; @import '../institutional/confirm-remove-jwt-modal/index'; diff --git a/ui/components/app/modals/convert-token-to-nft-modal/convert-token-to-nft-modal.js b/ui/components/app/modals/convert-token-to-nft-modal/convert-token-to-nft-modal.js index 9311153fcdb2..06e6655bc247 100644 --- a/ui/components/app/modals/convert-token-to-nft-modal/convert-token-to-nft-modal.js +++ b/ui/components/app/modals/convert-token-to-nft-modal/convert-token-to-nft-modal.js @@ -36,7 +36,9 @@ const ConvertTokenToNFTModal = ({ hideModal, tokenAddress }) => { pathname: `${ASSET_ROUTE}/${tokenAddress}/${tokenId}`, }); } else { - dispatch(showImportNftsModal()); + dispatch( + showImportNftsModal({ tokenAddress, ignoreErc20Token: true }), + ); } hideModal(); }} diff --git a/ui/components/app/name/__snapshots__/name.test.tsx.snap b/ui/components/app/name/__snapshots__/name.test.tsx.snap new file mode 100644 index 000000000000..5769227e4286 --- /dev/null +++ b/ui/components/app/name/__snapshots__/name.test.tsx.snap @@ -0,0 +1,95 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Name renders address with proposed name 1`] = ` +
+
+
+ + + 0xc0f...4978 + + + “ + TestProposedName + ” + +
+
+
+`; + +exports[`Name renders address with proposed name according to source priority 1`] = ` +
+
+
+ + + 0xc0f...4978 + + + “ + TestProposedName + ” + +
+
+
+`; + +exports[`Name renders address with saved name 1`] = ` +
+
+
+ + + TestName + +
+
+
+`; + +exports[`Name renders address without proposed name 1`] = ` +
+
+
+ + + 0xc0f...4979 + +
+
+
+`; diff --git a/ui/components/app/name/index.scss b/ui/components/app/name/index.scss new file mode 100644 index 000000000000..becff5641d2a --- /dev/null +++ b/ui/components/app/name/index.scss @@ -0,0 +1,37 @@ +.name { + border-radius: 36px; + padding: 6px 9px 6px 9px; + display: inline-flex; + align-items: center; + gap: 5px; + font-size: 12px; + + &__missing { + background-color: var(--color-warning-muted); + } + + &__saved { + background-color: var(--color-info-muted); + } + + &__missing &__icon { + color: var(--color-warning-default); + } + + &__saved &__icon { + color: var(--color-info-default); + } + + &__value, + &__proposed { + color: var(--color-warning-default); + } + + &__name { + color: var(--color-info-default); + } + + &__proposed { + font-style: italic; + } +} diff --git a/ui/components/app/name/index.ts b/ui/components/app/name/index.ts new file mode 100644 index 000000000000..f3e2e4fe0a86 --- /dev/null +++ b/ui/components/app/name/index.ts @@ -0,0 +1 @@ +export { default } from './name'; diff --git a/ui/components/app/name/name-details/__snapshots__/name-details.test.tsx.snap b/ui/components/app/name/name-details/__snapshots__/name-details.test.tsx.snap new file mode 100644 index 000000000000..2f4fc518007d --- /dev/null +++ b/ui/components/app/name/name-details/__snapshots__/name-details.test.tsx.snap @@ -0,0 +1,383 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`NameDetails renders with no saved name 1`] = ` + +
+
+
+
+
+