Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions eslint-warning-thresholds.json
Original file line number Diff line number Diff line change
Expand Up @@ -492,14 +492,8 @@
"packages/selected-network-controller/tests/SelectedNetworkController.test.ts": {
"jest/no-conditional-in-test": 1
},
"packages/signature-controller/src/SignatureController.test.ts": {
"import-x/order": 1,
"jsdoc/tag-lines": 3
},
"packages/signature-controller/src/SignatureController.ts": {
"@typescript-eslint/no-unsafe-enum-comparison": 4,
"@typescript-eslint/prefer-readonly": 3,
"jsdoc/tag-lines": 8
"@typescript-eslint/no-unsafe-enum-comparison": 4
},
"packages/signature-controller/src/utils/decoding-api.test.ts": {
"import-x/order": 1,
Expand All @@ -515,13 +509,9 @@
"@typescript-eslint/no-unused-vars": 1,
"jsdoc/tag-lines": 2
},
"packages/signature-controller/src/utils/validation.test.ts": {
"import-x/order": 1
},
"packages/signature-controller/src/utils/validation.ts": {
"@typescript-eslint/no-base-to-string": 1,
"@typescript-eslint/no-unused-vars": 2,
"jsdoc/tag-lines": 4
"@typescript-eslint/no-unused-vars": 2
},
"packages/user-operation-controller/src/UserOperationController.test.ts": {
"jsdoc/tag-lines": 4
Expand Down
2 changes: 2 additions & 0 deletions packages/signature-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"uuid": "^8.3.2"
},
"devDependencies": {
"@metamask/accounts-controller": "^26.0.0",
"@metamask/approval-controller": "^7.1.3",
"@metamask/auto-changelog": "^3.4.4",
"@metamask/keyring-controller": "^21.0.0",
Expand All @@ -70,6 +71,7 @@
"typescript": "~5.2.2"
},
"peerDependencies": {
"@metamask/accounts-controller": "^26.0.0",
"@metamask/approval-controller": "^7.0.0",
"@metamask/keyring-controller": "^21.0.0",
"@metamask/logging-controller": "^6.0.0",
Expand Down
76 changes: 72 additions & 4 deletions packages/signature-controller/src/SignatureController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { SignTypedDataVersion } from '@metamask/keyring-controller';
import { LogType, SigningStage } from '@metamask/logging-controller';
import { v1 } from 'uuid';

import { flushPromises } from '../../../tests/helpers';
import type {
SignatureControllerMessenger,
SignatureControllerOptions,
Expand All @@ -23,6 +22,8 @@ import {
normalizePersonalMessageParams,
normalizeTypedMessageParams,
} from './utils/normalize';
import { validateTypedSignatureRequest } from './utils/validation';
import { flushPromises } from '../../../tests/helpers';

jest.mock('uuid');
jest.mock('./utils/validation');
Expand Down Expand Up @@ -89,26 +90,30 @@ const PERMIT_REQUEST_MOCK = {

/**
* Create a mock messenger instance.
*
* @returns The mock messenger instance plus individual mock functions for each action.
*/
function createMessengerMock() {
const loggingControllerAddMock = jest.fn();
const accountsControllerGetStateMock = jest.fn();
const approvalControllerAddRequestMock = jest.fn();
const keyringControllerSignPersonalMessageMock = jest.fn();
const keyringControllerSignTypedMessageMock = jest.fn();
const loggingControllerAddMock = jest.fn();
const networkControllerGetNetworkClientByIdMock = jest.fn();

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const callMock = (method: string, ...args: any[]) => {
switch (method) {
case 'LoggingController:add':
return loggingControllerAddMock(...args);
case 'AccountsController:getState':
return accountsControllerGetStateMock(...args);
case 'ApprovalController:addRequest':
return approvalControllerAddRequestMock(...args);
case 'KeyringController:signPersonalMessage':
return keyringControllerSignPersonalMessageMock(...args);
case 'KeyringController:signTypedMessage':
return keyringControllerSignTypedMessageMock(...args);
case 'LoggingController:add':
return loggingControllerAddMock(...args);
case 'NetworkController:getNetworkClientById':
return networkControllerGetNetworkClientByIdMock(...args);
default:
Expand All @@ -123,6 +128,12 @@ function createMessengerMock() {
call: callMock,
} as unknown as jest.Mocked<SignatureControllerMessenger>;

accountsControllerGetStateMock.mockReturnValue({
internalAccounts: {
accounts: [],
},
});

approvalControllerAddRequestMock.mockResolvedValue({});
loggingControllerAddMock.mockResolvedValue({});

Expand All @@ -133,6 +144,7 @@ function createMessengerMock() {
});

return {
accountsControllerGetStateMock,
approvalControllerAddRequestMock,
keyringControllerSignPersonalMessageMock,
keyringControllerSignTypedMessageMock,
Expand All @@ -143,6 +155,7 @@ function createMessengerMock() {

/**
* Create a new instance of the SignatureController.
*
* @param options - Optional overrides for the default options.
* @returns The controller instance plus individual mock functions for each action.
*/
Expand All @@ -159,6 +172,7 @@ function createController(options?: Partial<SignatureControllerOptions>) {

/**
* Create a mock error.
*
* @returns The mock error instance.
*/
function createErrorMock(): Error {
Expand All @@ -177,6 +191,10 @@ describe('SignatureController', () => {
normalizeTypedMessageParams,
);

const validateTypedSignatureRequestMock = jest.mocked(
validateTypedSignatureRequest,
);

const detectSIWEMock = jest.mocked(detectSIWE);
const uuidV1Mock = jest.mocked(v1);

Expand Down Expand Up @@ -1068,6 +1086,56 @@ describe('SignatureController', () => {
controller.state.signatureRequests[ID_MOCK].decodingLoading,
).toBe(true);
});

it('validates the request', async () => {
const { controller } = createController();

await controller.newUnsignedTypedMessage(
PARAMS_MOCK,
REQUEST_MOCK,
SignTypedDataVersion.V4,
{ parseJsonData: false },
);

expect(validateTypedSignatureRequestMock).toHaveBeenCalledTimes(1);
});

it('validates the request using EOA internal accounts', async () => {
const { controller, accountsControllerGetStateMock } =
createController();

accountsControllerGetStateMock.mockReturnValue({
internalAccounts: {
accounts: [
{
type: 'eip155:eoa',
address: '0x123',
},
{
type: 'invalid',
address: '0x321',
},
{
type: 'eip155:eoa',
address: '0xabc',
},
],
},
});

await controller.newUnsignedTypedMessage(
PARAMS_MOCK,
REQUEST_MOCK,
SignTypedDataVersion.V4,
{ parseJsonData: false },
);

expect(validateTypedSignatureRequestMock).toHaveBeenCalledWith(
expect.objectContaining({
internalAccounts: ['0x123', '0xabc'],
}),
);
});
});
});

Expand Down
38 changes: 30 additions & 8 deletions packages/signature-controller/src/SignatureController.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { AccountsControllerGetStateAction } from '@metamask/accounts-controller';
import type {
AddApprovalRequest,
AcceptResultCallbacks,
Expand Down Expand Up @@ -89,30 +90,35 @@ export type SignatureControllerState = {

/**
* Map of personal messages with the unapproved status, keyed by ID.
*
* @deprecated - Use `signatureRequests` instead.
*/
unapprovedPersonalMsgs: Record<string, LegacyStateMessage>;

/**
* Map of typed messages with the unapproved status, keyed by ID.
*
* @deprecated - Use `signatureRequests` instead.
*/
unapprovedTypedMessages: Record<string, LegacyStateMessage>;

/**
* Number of unapproved personal messages.
*
* @deprecated - Use `signatureRequests` instead.
*/
unapprovedPersonalMsgCount: number;

/**
* Number of unapproved typed messages.
*
* @deprecated - Use `signatureRequests` instead.
*/
unapprovedTypedMessagesCount: number;
};

type AllowedActions =
| AccountsControllerGetStateAction
| AddApprovalRequest
| KeyringControllerSignMessageAction
| KeyringControllerSignPersonalMessageAction
Expand Down Expand Up @@ -189,11 +195,11 @@ export class SignatureController extends BaseController<
> {
hub: EventEmitter;

#decodingApiUrl?: string;
readonly #decodingApiUrl?: string;

#isDecodeSignatureRequestEnabled?: () => boolean;
readonly #isDecodeSignatureRequestEnabled?: () => boolean;

#trace: TraceCallback;
readonly #trace: TraceCallback;

/**
* Construct a Sign controller.
Expand Down Expand Up @@ -230,6 +236,7 @@ export class SignatureController extends BaseController<

/**
* A getter for the number of 'unapproved' PersonalMessages in this.messages.
*
* @deprecated Use `signatureRequests` state instead.
* @returns The number of 'unapproved' PersonalMessages in this.messages
*/
Expand All @@ -239,6 +246,7 @@ export class SignatureController extends BaseController<

/**
* A getter for the number of 'unapproved' TypedMessages in this.messages.
*
* @deprecated Use `signatureRequests` state instead.
* @returns The number of 'unapproved' TypedMessages in this.messages
*/
Expand All @@ -248,6 +256,7 @@ export class SignatureController extends BaseController<

/**
* A getter for returning all messages.
*
* @deprecated Use `signatureRequests` state instead.
* @returns The object containing all messages.
*/
Expand Down Expand Up @@ -346,12 +355,15 @@ export class SignatureController extends BaseController<
options: { traceContext?: TraceContext } = {},
): Promise<string> {
const chainId = this.#getChainId(request);
const internalAccounts = this.#getInternalAccounts();

validateTypedSignatureRequest(
messageParams,
version as SignTypedDataVersion,
chainId,
);
validateTypedSignatureRequest({
currentChainId: chainId,
internalAccounts,
messageData: messageParams,
request,
version: version as SignTypedDataVersion,
});

const normalizedMessageParams = normalizeTypedMessageParams(
messageParams,
Expand Down Expand Up @@ -386,6 +398,7 @@ export class SignatureController extends BaseController<

/**
* Set custom metadata on a signature request.
*
* @param signatureRequestId - The ID of the signature request.
* @param metadata - The custom metadata to set.
*/
Expand Down Expand Up @@ -938,4 +951,13 @@ export class SignatureController extends BaseController<
}),
);
}

#getInternalAccounts(): Hex[] {
const state = this.messagingSystem.call('AccountsController:getState');

/* istanbul ignore next */
return Object.values(state.internalAccounts?.accounts ?? {})
.filter((account) => account.type === 'eip155:eoa')
.map((account) => account.address as Hex);
}
}
18 changes: 9 additions & 9 deletions packages/signature-controller/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,18 @@ export type MessageParamsPersonal = MessageParams & {
siwe?: StateSIWEMessage;
};

/** Typed data used in the signTypedData request. */
export type MessageParamsTypedData = {
types: Record<string, Json>;
domain: Record<string, Json>;
primaryType: string;
message: Json;
};

/** Typed message parameters that were requested to be signed. */
export type MessageParamsTyped = MessageParams & {
/** Structured data to sign. */
data:
| Record<string, Json>[]
| string
| {
types: Record<string, Json>;
domain: Record<string, Json>;
primaryType: string;
message: Json;
};
data: Record<string, Json>[] | string | MessageParamsTypedData;
/** Version of the signTypedData request. */
version?: string;
};
Expand Down
Loading