diff --git a/packages/message-manager/src/AbstractMessageManager.test.ts b/packages/message-manager/src/AbstractMessageManager.test.ts index da11beb542..c6362815ec 100644 --- a/packages/message-manager/src/AbstractMessageManager.test.ts +++ b/packages/message-manager/src/AbstractMessageManager.test.ts @@ -1,4 +1,10 @@ -import type { SecurityProviderRequest } from './AbstractMessageManager'; +import { ApprovalType } from '@metamask/controller-utils'; + +import type { + AbstractMessageParams, + OriginalRequest, + SecurityProviderRequest, +} from './AbstractMessageManager'; import { AbstractMessageManager } from './AbstractMessageManager'; import type { TypedMessage, @@ -11,6 +17,21 @@ class AbstractTestManager extends AbstractMessageManager< TypedMessageParams, TypedMessageParamsMetamask > { + addRequestToMessageParams( + messageParams: MessageParams, + req?: OriginalRequest, + ) { + return super.addRequestToMessageParams(messageParams, req); + } + + createUnapprovedMessage( + messageParams: MessageParams, + type: ApprovalType, + req?: OriginalRequest, + ) { + return super.createUnapprovedMessage(messageParams, type, req); + } + prepMessageForSigning( messageParams: TypedMessageParamsMetamask, ): Promise { @@ -50,6 +71,14 @@ const rawSigMock = '0xsignaturemocked'; const messageIdMock = 'message-id-mocked'; const fromMock = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; +const mockSecurityProviderResponse = { flagAsDangerous: 2 }; +const mockRequest = { + origin: 'example.com', + id: 123, + securityAlertResponse: mockSecurityProviderResponse, +}; +const mockMessageParams = { from, data: 'test' }; + describe('AbstractTestManager', () => { it('should set default state', () => { const controller = new AbstractTestManager(); @@ -317,12 +346,54 @@ describe('AbstractTestManager', () => { expect(message.status).toBe('approved'); }); + describe('addRequestToMessageParams', () => { + it('adds original request id and origin to messageParams', () => { + const controller = new AbstractTestManager(); + + const result = controller.addRequestToMessageParams( + mockMessageParams, + mockRequest, + ); + + expect(result).toStrictEqual({ + ...mockMessageParams, + origin: mockRequest.origin, + requestId: mockRequest.id, + }); + }); + }); + + describe('createUnapprovedMessage', () => { + it('creates a Message object with an unapproved status', () => { + const controller = new AbstractTestManager(); + + const result = controller.createUnapprovedMessage( + mockMessageParams, + ApprovalType.PersonalSign, + mockRequest, + ); + + expect(result.messageParams).toBe(mockMessageParams); + expect(result.securityAlertResponse).toBe( + mockRequest.securityAlertResponse, + ); + expect(result.status).toBe('unapproved'); + expect(result.type).toBe(ApprovalType.PersonalSign); + expect(typeof result.time).toBe('number'); + expect(typeof result.id).toBe('string'); + }); + }); + describe('setMessageStatus', () => { - it('should set the given message status', async () => { + it('updates the status of a message', async () => { + jest.mock('events', () => ({ + emit: jest.fn(), + })); + const controller = new AbstractTestManager(); await controller.addMessage({ id: messageId, - messageParams: { from: '0x1234', data: 'test' }, + messageParams: { ...mockMessageParams }, status: 'status', time: 10, type: 'type', @@ -331,11 +402,12 @@ describe('AbstractTestManager', () => { expect(messageBefore?.status).toBe('status'); controller.setMessageStatus(messageId, 'newstatus'); + const messageAfter = controller.getMessage(messageId); expect(messageAfter?.status).toBe('newstatus'); }); - it('should throw an error if message is not found', () => { + it('throws an error if the message is not found', async () => { const controller = new AbstractTestManager(); expect(() => controller.setMessageStatus(messageId, 'newstatus')).toThrow( @@ -353,7 +425,7 @@ describe('AbstractTestManager', () => { const controller = new AbstractTestManager(); await controller.addMessage({ id: messageId, - messageParams: { from: '0x1234', data: 'test' }, + messageParams: { ...mockMessageParams }, status: 'status', time: 10, type: 'type', @@ -374,7 +446,7 @@ describe('AbstractTestManager', () => { const controller = new AbstractTestManager(); await controller.addMessage({ id: messageId, - messageParams: { from: '0x1234', data: 'test' }, + messageParams: { ...mockMessageParams }, status: 'status', time: 10, type: 'type', diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index 9582e87a8b..9ede9ce7a1 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -1,7 +1,9 @@ import type { BaseConfig, BaseState } from '@metamask/base-controller'; import { BaseControllerV1 } from '@metamask/base-controller'; +import type { ApprovalType } from '@metamask/controller-utils'; import type { Hex, Json } from '@metamask/utils'; import { EventEmitter } from 'events'; +import { v1 as random } from 'uuid'; /** * @type OriginalRequest @@ -13,6 +15,7 @@ import { EventEmitter } from 'events'; // Convert to a `type` in a future major version. // eslint-disable-next-line @typescript-eslint/consistent-type-definitions export interface OriginalRequest { + id?: number; origin?: string; securityAlertResponse?: Record; } @@ -49,6 +52,7 @@ export interface AbstractMessage { * Represents the parameters to pass to the signing method once the signature request is approved. * @property from - Address from which the message is processed * @property origin? - Added for request origin identification + * @property requestId? - Original request id * @property deferSetAsSigned? - Whether to defer setting the message as signed immediately after the keyring is told to sign it */ // This interface was created before this ESLint rule was added. @@ -57,6 +61,7 @@ export interface AbstractMessage { export interface AbstractMessageParams { from: string; origin?: string; + requestId?: number; deferSetAsSigned?: boolean; } @@ -127,6 +132,49 @@ export abstract class AbstractMessageManager< private readonly additionalFinishStatuses: string[]; + /** + * Adds request props to the messsage params and returns a new messageParams object. + * @param messageParams - The messageParams to add the request props to. + * @param req - The original request object. + * @returns The messageParams with the request props added. + */ + protected addRequestToMessageParams< + MessageParams extends AbstractMessageParams, + >(messageParams: MessageParams, req?: OriginalRequest) { + const updatedMessageParams = { + ...messageParams, + }; + + if (req) { + updatedMessageParams.requestId = req.id; + updatedMessageParams.origin = req.origin; + } + + return updatedMessageParams; + } + + /** + * Creates a new Message with a random id and an 'unapproved' status. + * @param messageParams - The messageParams to add the request props to. + * @param type - The approval type of the message. + * @param req - The original request object. + * @returns The new unapproved message for a specified type. + */ + protected createUnapprovedMessage< + MessageParams extends AbstractMessageParams, + >(messageParams: MessageParams, type: ApprovalType, req?: OriginalRequest) { + const messageId = random(); + + return { + id: messageId, + messageParams, + securityAlertResponse: req?.securityAlertResponse, + status: 'unapproved', + time: Date.now(), + type, + }; + } + /** * Saves the unapproved messages, and their count to state. * diff --git a/packages/message-manager/src/DecryptMessageManager.test.ts b/packages/message-manager/src/DecryptMessageManager.test.ts index 6d68212912..d81b336141 100644 --- a/packages/message-manager/src/DecryptMessageManager.test.ts +++ b/packages/message-manager/src/DecryptMessageManager.test.ts @@ -140,7 +140,7 @@ describe('DecryptMessageManager', () => { const messageStatus = 'unapproved'; const messageType = 'eth_decrypt'; const messageParams = { from: fromMock, data: dataMock }; - const originalRequest = { origin: 'origin' }; + const originalRequest = { id: 111, origin: 'origin' }; const messageId = await controller.addUnapprovedMessage( messageParams, originalRequest, @@ -151,6 +151,7 @@ describe('DecryptMessageManager', () => { throw new Error('"message" is falsy'); } expect(message.messageParams.from).toBe(messageParams.from); + expect(message.messageParams.requestId).toBe(originalRequest.id); expect(message.time).toBeDefined(); expect(message.status).toBe(messageStatus); expect(message.type).toBe(messageType); diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index dbfd3a4a39..4036e79d90 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -1,4 +1,4 @@ -import { v1 as random } from 'uuid'; +import { ApprovalType } from '@metamask/controller-utils'; import type { AbstractMessage, @@ -129,22 +129,24 @@ export class DecryptMessageManager extends AbstractMessageManager< messageParams: DecryptMessageParams, req?: OriginalRequest, ) { - if (req) { - messageParams.origin = req.origin; - } - messageParams.data = normalizeMessageData(messageParams.data); - const messageId = random(); - const messageData: DecryptMessage = { - id: messageId, + const updatedMessageParams = this.addRequestToMessageParams( messageParams, - status: 'unapproved', - time: Date.now(), - type: 'eth_decrypt', - }; + req, + ) satisfies DecryptMessageParams; + messageParams.data = normalizeMessageData(messageParams.data); + + const messageData = this.createUnapprovedMessage( + updatedMessageParams, + ApprovalType.EthDecrypt, + req, + ) satisfies DecryptMessage; + + const messageId = messageData.id; + await this.addMessage(messageData); this.hub.emit(`unapprovedMessage`, { - ...messageParams, - ...{ metamaskId: messageId }, + ...updatedMessageParams, + metamaskId: messageId, }); return messageId; } diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts index 219dba663d..8617c16534 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts @@ -120,7 +120,7 @@ describe('EncryptionPublicKeyManager', () => { const messageParams = { from: fromMock, }; - const originalRequest = { origin: 'origin' }; + const originalRequest = { id: 111, origin: 'origin' }; const messageId = await controller.addUnapprovedMessage( messageParams, originalRequest, @@ -131,6 +131,7 @@ describe('EncryptionPublicKeyManager', () => { throw new Error('"message" is falsy'); } expect(message.messageParams.from).toBe(messageParams.from); + expect(message.messageParams.requestId).toBe(originalRequest.id); expect(message.time).toBeDefined(); expect(message.status).toBe(messageStatus); expect(message.type).toBe(messageType); diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index ab4f40f7d1..6fdf1518f7 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -1,4 +1,4 @@ -import { v1 as random } from 'uuid'; +import { ApprovalType } from '@metamask/controller-utils'; import type { AbstractMessage, @@ -120,21 +120,23 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< messageParams: EncryptionPublicKeyParams, req?: OriginalRequest, ): Promise { - if (req) { - messageParams.origin = req.origin; - } - const messageId = random(); - const messageData: EncryptionPublicKey = { - id: messageId, + const updatedMessageParams = this.addRequestToMessageParams( messageParams, - status: 'unapproved', - time: Date.now(), - type: 'eth_getEncryptionPublicKey', - }; + req, + ) satisfies EncryptionPublicKeyParams; + + const messageData = this.createUnapprovedMessage( + updatedMessageParams, + ApprovalType.EthGetEncryptionPublicKey, + req, + ) satisfies EncryptionPublicKey; + + const messageId = messageData.id; + await this.addMessage(messageData); this.hub.emit(`unapprovedMessage`, { - ...messageParams, - ...{ metamaskId: messageId }, + ...updatedMessageParams, + metamaskId: messageId, }); return messageId; } diff --git a/packages/message-manager/src/PersonalMessageManager.test.ts b/packages/message-manager/src/PersonalMessageManager.test.ts index c7a082c848..f5b6494758 100644 --- a/packages/message-manager/src/PersonalMessageManager.test.ts +++ b/packages/message-manager/src/PersonalMessageManager.test.ts @@ -79,6 +79,7 @@ describe('PersonalMessageManager', () => { from: fromMock, }; const originalRequest = { + id: 111, origin: 'origin', // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention @@ -95,6 +96,7 @@ describe('PersonalMessageManager', () => { } expect(message.messageParams.from).toBe(messageParams.from); expect(message.messageParams.data).toBe(messageParams.data); + expect(message.messageParams.requestId).toBe(originalRequest.id); expect(message.time).toBeDefined(); expect(message.status).toBe(messageStatus); expect(message.type).toBe(messageType); diff --git a/packages/message-manager/src/PersonalMessageManager.ts b/packages/message-manager/src/PersonalMessageManager.ts index f0f58e17ba..c9fe134db4 100644 --- a/packages/message-manager/src/PersonalMessageManager.ts +++ b/packages/message-manager/src/PersonalMessageManager.ts @@ -1,6 +1,5 @@ import type { SIWEMessage } from '@metamask/controller-utils'; -import { detectSIWE } from '@metamask/controller-utils'; -import { v1 as random } from 'uuid'; +import { detectSIWE, ApprovalType } from '@metamask/controller-utils'; import type { AbstractMessage, @@ -91,27 +90,27 @@ export class PersonalMessageManager extends AbstractMessageManager< req?: OriginalRequest, ): Promise { validateSignMessageData(messageParams); - if (req) { - messageParams.origin = req.origin; - } - messageParams.data = normalizeMessageData(messageParams.data); const ethereumSignInData = detectSIWE(messageParams); - const finalMsgParams = { ...messageParams, siwe: ethereumSignInData }; + const updatedMessageParams = this.addRequestToMessageParams( + messageParams, + req, + ) satisfies PersonalMessageParams; - const messageId = random(); - const messageData: PersonalMessage = { - id: messageId, - messageParams: finalMsgParams, - securityAlertResponse: req?.securityAlertResponse, - status: 'unapproved', - time: Date.now(), - type: 'personal_sign', - }; + updatedMessageParams.data = normalizeMessageData(messageParams.data); + updatedMessageParams.siwe = ethereumSignInData; + + const messageData = this.createUnapprovedMessage( + updatedMessageParams, + ApprovalType.PersonalSign, + req, + ) satisfies PersonalMessage; + + const messageId = messageData.id; await this.addMessage(messageData); this.hub.emit(`unapprovedMessage`, { - ...finalMsgParams, - ...{ metamaskId: messageId }, + ...updatedMessageParams, + metamaskId: messageId, }); return messageId; } diff --git a/packages/message-manager/src/TypedMessageManager.test.ts b/packages/message-manager/src/TypedMessageManager.test.ts index 99539ce645..942fdde99b 100644 --- a/packages/message-manager/src/TypedMessageManager.test.ts +++ b/packages/message-manager/src/TypedMessageManager.test.ts @@ -109,7 +109,7 @@ describe('TypedMessageManager', () => { data: messageData, from: fromMock, }; - const originalRequest = { origin: 'origin' }; + const originalRequest = { id: 111, origin: 'origin' }; await expect( controller.addUnapprovedMessage(messageParams, originalRequest, version), @@ -126,6 +126,7 @@ describe('TypedMessageManager', () => { from: fromMock, }; const originalRequest = { + id: 111, origin: 'origin', // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention @@ -143,6 +144,8 @@ describe('TypedMessageManager', () => { } expect(message.messageParams.from).toBe(messageParams.from); expect(message.messageParams.data).toBe(messageParams.data); + expect(message.messageParams.origin).toBe(originalRequest.origin); + expect(message.messageParams.requestId).toBe(originalRequest.id); expect(message.time).toBeDefined(); expect(message.status).toBe(messageStatus); expect(message.type).toBe(messageType); @@ -160,7 +163,7 @@ describe('TypedMessageManager', () => { data: messageData, from: fromMock, }; - const originalRequest = { origin: 'origin' }; + const originalRequest = { id: 111, origin: 'origin' }; const messageId = await controller.addUnapprovedMessage( messageParams, originalRequest, @@ -173,6 +176,7 @@ describe('TypedMessageManager', () => { } expect(message.messageParams.from).toBe(messageParams.from); expect(message.messageParams.data).toBe(messageParams.data); + expect(message.messageParams.requestId).toBe(originalRequest.id); expect(message.time).toBeDefined(); expect(message.status).toBe(messageStatus); expect(message.type).toBe(messageType); @@ -188,7 +192,7 @@ describe('TypedMessageManager', () => { data: messageData, from: fromMock, }; - const originalRequest = { origin: 'origin' }; + const originalRequest = { id: 111, origin: 'origin' }; const messageId = await controller.addUnapprovedMessage( messageParams, originalRequest, @@ -201,6 +205,7 @@ describe('TypedMessageManager', () => { } expect(message.messageParams.from).toBe(messageParams.from); expect(message.messageParams.data).toBe(messageParams.data); + expect(message.messageParams.requestId).toBe(originalRequest.id); expect(message.time).toBeDefined(); expect(message.status).toBe(messageStatus); expect(message.type).toBe(messageType); diff --git a/packages/message-manager/src/TypedMessageManager.ts b/packages/message-manager/src/TypedMessageManager.ts index 75682dedcc..cc1f03f720 100644 --- a/packages/message-manager/src/TypedMessageManager.ts +++ b/packages/message-manager/src/TypedMessageManager.ts @@ -1,4 +1,4 @@ -import { v1 as random } from 'uuid'; +import { ApprovalType } from '@metamask/controller-utils'; import type { AbstractMessage, @@ -129,25 +129,31 @@ export class TypedMessageManager extends AbstractMessageManager< messageParams.data = JSON.stringify(messageParams.data); } - const messageId = random(); - const messageParamsMetamask = { + const updatedMessageParams = this.addRequestToMessageParams( + messageParams, + req, + ) satisfies TypedMessageParams; + + const messageData = this.createUnapprovedMessage( + updatedMessageParams, + ApprovalType.EthSignTypedData, + req, + ) satisfies TypedMessage; + + const messageId = messageData.id; + + await this.addMessage(messageData); + + /** + * This intentionally splays messageParams rather than updatedMessageParams. I'm unsure if this + * is exactly what we want, but I am preserving existing logic. + */ + this.hub.emit(`unapprovedMessage`, { ...messageParams, metamaskId: messageId, version, - }; - if (req) { - messageParams.origin = req.origin; - } - const messageData: TypedMessage = { - id: messageId, - messageParams, - securityAlertResponse: req?.securityAlertResponse, - status: 'unapproved', - time: Date.now(), - type: 'eth_signTypedData', - }; - await this.addMessage(messageData); - this.hub.emit(`unapprovedMessage`, messageParamsMetamask); + }); + return messageId; }