Skip to content

Commit 09d848c

Browse files
authored
feat: decode permissions in SignatureController (#6619)
## Summary Add execution-permission decoding to `SignatureController` for `eth_signTypedData_v4` delegation requests. If a delegation request is unable to be decoded into a coherent Execution Permission, reject external (not from `origin: metamask`) attempts to sign delegations for internal accounts. Adds decoded permission in signature request state for rendering downstream. ## Why Readable Permissions requires the ability to sign delegations, only when the delegation can be decoded to a permission that is then shown to the user. [The architecture is described in this document](https://www.notion.so/metamask-consensys/SignTypedData-with-Metadata-Specification-22bf86d67d688023be67e2ee06e3a56a). Decoding has been added to `GatorPermissionsController` in #6556 Note: the request origin is always `@metamask/gator-permission-snap` (any other origin is rejected). Because of this, `metadata` is added to the 712 payload, defining the `justification` and `origin` which is the _original_ origin of the request (the dapp).
1 parent 680b813 commit 09d848c

16 files changed

+777
-43
lines changed

eslint-warning-thresholds.json

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -349,21 +349,10 @@
349349
"jest/no-conditional-in-test": 1
350350
},
351351
"packages/signature-controller/src/SignatureController.ts": {
352-
"@typescript-eslint/no-unsafe-enum-comparison": 4
353-
},
354-
"packages/signature-controller/src/utils/decoding-api.test.ts": {
355-
"import-x/order": 1,
356-
"jsdoc/tag-lines": 1
357-
},
358-
"packages/signature-controller/src/utils/decoding-api.ts": {
359-
"import-x/order": 1
360-
},
361-
"packages/signature-controller/src/utils/normalize.test.ts": {
362-
"import-x/order": 1
352+
"@typescript-eslint/no-unsafe-enum-comparison": 3
363353
},
364354
"packages/signature-controller/src/utils/normalize.ts": {
365-
"@typescript-eslint/no-unused-vars": 1,
366-
"jsdoc/tag-lines": 2
355+
"@typescript-eslint/no-unused-vars": 1
367356
},
368357
"packages/signature-controller/src/utils/validation.ts": {
369358
"@typescript-eslint/no-base-to-string": 1,

packages/signature-controller/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- Add two new controller state metadata properties: `includeInStateLogs` and `usedInUi` ([#6473](https://github.com/MetaMask/core/pull/6473))
13+
- Decode delegation permissions using `@metamask/gator-permissions-controller` when calling `newUnsignedTypedMessage` ([#6619](https://github.com/MetaMask/core/pull/6619))
1314

1415
### Changed
1516

packages/signature-controller/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
"@metamask/accounts-controller": "^33.1.0",
6060
"@metamask/approval-controller": "^7.1.3",
6161
"@metamask/auto-changelog": "^3.4.4",
62+
"@metamask/gator-permissions-controller": "^0.2.0",
6263
"@metamask/keyring-controller": "^23.1.0",
6364
"@metamask/logging-controller": "^6.0.4",
6465
"@metamask/network-controller": "^24.2.0",
@@ -73,6 +74,7 @@
7374
"peerDependencies": {
7475
"@metamask/accounts-controller": "^33.0.0",
7576
"@metamask/approval-controller": "^7.0.0",
77+
"@metamask/gator-permissions-controller": "^0.2.0",
7678
"@metamask/keyring-controller": "^23.0.0",
7779
"@metamask/logging-controller": "^6.0.0",
7880
"@metamask/network-controller": "^24.0.0"

packages/signature-controller/src/SignatureController.test.ts

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,27 @@ const PERMIT_REQUEST_MOCK = {
8989
traceContext: null,
9090
};
9191

92+
const DELEGATION_PARAMS_MOCK = {
93+
data: '{"types":{"EIP712Domain":[{"name":"chainId","type":"uint256"}],"Delegation":[{"name":"delegate","type":"address"},{"name":"delegator","type":"address"},{"name":"authority","type":"bytes"},{"name":"caveats","type":"bytes"}]},"primaryType":"Delegation","domain":{"chainId":1},"message":{"delegate":"0x5B38Da6a701c568545dCfcB03FcB875f56beddC4","delegator":"0x975e73efb9ff52e23bac7f7e043a1ecd06d05477","authority":"0x1234abcd","caveats":[]},"metadata":{"origin":"https://metamask.github.io","justification":"Testing delegation"}}',
94+
from: '0x975e73efb9ff52e23bac7f7e043a1ecd06d05477',
95+
version: 'V4',
96+
signatureMethod: 'eth_signTypedData_v4',
97+
};
98+
99+
const DELEGATION_REQUEST_MOCK = {
100+
method: 'eth_signTypedData_v4',
101+
params: [
102+
'0x975e73efb9ff52e23bac7f7e043a1ecd06d05477',
103+
DELEGATION_PARAMS_MOCK.data,
104+
],
105+
jsonrpc: '2.0',
106+
id: 1680528591,
107+
origin: 'npm:@metamask/gator-permissions-snap',
108+
networkClientId: 'mainnet',
109+
tabId: 1048807182,
110+
traceContext: null,
111+
};
112+
92113
/**
93114
* Create a mock messenger instance.
94115
*
@@ -101,6 +122,7 @@ function createMessengerMock() {
101122
const keyringControllerSignTypedMessageMock = jest.fn();
102123
const loggingControllerAddMock = jest.fn();
103124
const networkControllerGetNetworkClientByIdMock = jest.fn();
125+
const decodePermissionFromPermissionContextForOriginMock = jest.fn();
104126

105127
// eslint-disable-next-line @typescript-eslint/no-explicit-any
106128
const callMock = (method: string, ...args: any[]) => {
@@ -117,6 +139,8 @@ function createMessengerMock() {
117139
return loggingControllerAddMock(...args);
118140
case 'NetworkController:getNetworkClientById':
119141
return networkControllerGetNetworkClientByIdMock(...args);
142+
case 'GatorPermissionsController:decodePermissionFromPermissionContextForOrigin':
143+
return decodePermissionFromPermissionContextForOriginMock(...args);
120144
default:
121145
throw new Error(`Messenger method not recognised: ${method}`);
122146
}
@@ -144,12 +168,17 @@ function createMessengerMock() {
144168
},
145169
});
146170

171+
decodePermissionFromPermissionContextForOriginMock.mockReturnValue({
172+
kind: 'decoded-permission',
173+
});
174+
147175
return {
148176
accountsControllerGetStateMock,
149177
approvalControllerAddRequestMock,
150178
keyringControllerSignPersonalMessageMock,
151179
keyringControllerSignTypedMessageMock,
152180
loggingControllerAddMock,
181+
decodePermissionFromPermissionContextForOriginMock,
153182
messenger,
154183
};
155184
}
@@ -933,6 +962,201 @@ describe('SignatureController', () => {
933962
).toBe(SignTypedDataVersion.V3);
934963
});
935964

965+
describe('delegations', () => {
966+
it('invokes decodePermissionFromRequest to get execution permission', async () => {
967+
const {
968+
controller,
969+
decodePermissionFromPermissionContextForOriginMock,
970+
} = createController();
971+
972+
await controller.newUnsignedTypedMessage(
973+
DELEGATION_PARAMS_MOCK,
974+
DELEGATION_REQUEST_MOCK,
975+
SignTypedDataVersion.V4,
976+
{ parseJsonData: false },
977+
);
978+
979+
expect(
980+
decodePermissionFromPermissionContextForOriginMock,
981+
).toHaveBeenCalledWith({
982+
origin: 'npm:@metamask/gator-permissions-snap',
983+
chainId: 1,
984+
delegation: {
985+
delegate: '0x5B38Da6a701c568545dCfcB03FcB875f56beddC4',
986+
delegator: '0x975e73efb9ff52e23bac7f7e043a1ecd06d05477',
987+
authority: '0x1234abcd',
988+
caveats: [],
989+
},
990+
metadata: {
991+
origin: 'https://metamask.github.io',
992+
justification: 'Testing delegation',
993+
},
994+
});
995+
});
996+
997+
it('does not invoke decodePermissionFromRequest if version is not V4', async () => {
998+
const {
999+
controller,
1000+
decodePermissionFromPermissionContextForOriginMock,
1001+
} = createController();
1002+
1003+
await controller.newUnsignedTypedMessage(
1004+
DELEGATION_PARAMS_MOCK,
1005+
DELEGATION_REQUEST_MOCK,
1006+
SignTypedDataVersion.V3,
1007+
{ parseJsonData: false },
1008+
);
1009+
1010+
expect(
1011+
decodePermissionFromPermissionContextForOriginMock,
1012+
).not.toHaveBeenCalled();
1013+
});
1014+
1015+
it('sets decodedPermission on the message state', async () => {
1016+
const { controller } = createController();
1017+
1018+
await controller.newUnsignedTypedMessage(
1019+
DELEGATION_PARAMS_MOCK,
1020+
DELEGATION_REQUEST_MOCK,
1021+
SignTypedDataVersion.V4,
1022+
{ parseJsonData: false },
1023+
);
1024+
1025+
const { decodedPermission } =
1026+
controller.state.signatureRequests[ID_MOCK];
1027+
1028+
expect(decodedPermission).toStrictEqual({
1029+
kind: 'decoded-permission',
1030+
});
1031+
});
1032+
1033+
it('does not invoke decodePermissionFromRequest if data is not a delegation request', async () => {
1034+
const {
1035+
controller,
1036+
decodePermissionFromPermissionContextForOriginMock,
1037+
} = createController();
1038+
1039+
await controller.newUnsignedTypedMessage(
1040+
{
1041+
...DELEGATION_PARAMS_MOCK,
1042+
data: '{primaryType:"not-a-delegation"}',
1043+
},
1044+
DELEGATION_REQUEST_MOCK,
1045+
SignTypedDataVersion.V4,
1046+
{ parseJsonData: false },
1047+
);
1048+
1049+
expect(
1050+
decodePermissionFromPermissionContextForOriginMock,
1051+
).not.toHaveBeenCalled();
1052+
});
1053+
1054+
it('does not set decodedPermission if data is not a delegation request', async () => {
1055+
const { controller } = createController();
1056+
1057+
await controller.newUnsignedTypedMessage(
1058+
{
1059+
...DELEGATION_PARAMS_MOCK,
1060+
data: '{primaryType:"not-a-delegation"}',
1061+
},
1062+
DELEGATION_REQUEST_MOCK,
1063+
SignTypedDataVersion.V4,
1064+
{ parseJsonData: false },
1065+
);
1066+
1067+
const { decodedPermission } =
1068+
controller.state.signatureRequests[ID_MOCK];
1069+
1070+
expect(decodedPermission).toBeUndefined();
1071+
});
1072+
1073+
it('does not set decodedPermission if decoding throws an error', async () => {
1074+
const {
1075+
controller,
1076+
decodePermissionFromPermissionContextForOriginMock,
1077+
} = createController();
1078+
1079+
decodePermissionFromPermissionContextForOriginMock.mockImplementation(
1080+
() => {
1081+
throw new Error('An error occurred');
1082+
},
1083+
);
1084+
1085+
await controller.newUnsignedTypedMessage(
1086+
DELEGATION_PARAMS_MOCK,
1087+
DELEGATION_REQUEST_MOCK,
1088+
SignTypedDataVersion.V4,
1089+
{ parseJsonData: false },
1090+
);
1091+
1092+
const { decodedPermission } =
1093+
controller.state.signatureRequests[ID_MOCK];
1094+
1095+
expect(decodedPermission).toBeUndefined();
1096+
});
1097+
1098+
it('does not set decodedPermission if decoding returns undefined', async () => {
1099+
const {
1100+
controller,
1101+
decodePermissionFromPermissionContextForOriginMock,
1102+
} = createController();
1103+
1104+
decodePermissionFromPermissionContextForOriginMock.mockReturnValue(
1105+
undefined,
1106+
);
1107+
1108+
await controller.newUnsignedTypedMessage(
1109+
DELEGATION_PARAMS_MOCK,
1110+
DELEGATION_REQUEST_MOCK,
1111+
SignTypedDataVersion.V4,
1112+
{ parseJsonData: false },
1113+
);
1114+
1115+
const { decodedPermission } =
1116+
controller.state.signatureRequests[ID_MOCK];
1117+
1118+
expect(decodedPermission).toBeUndefined();
1119+
});
1120+
1121+
it('does not set decodedPermission if metadata is invalid', async () => {
1122+
const { controller } = createController();
1123+
1124+
const delegationParamsMock = {
1125+
...DELEGATION_PARAMS_MOCK,
1126+
data: {
1127+
...JSON.parse(DELEGATION_PARAMS_MOCK.data),
1128+
metadata: {},
1129+
},
1130+
};
1131+
1132+
const delegationRequestMock = {
1133+
method: 'eth_signTypedData_v4',
1134+
params: [
1135+
'0x975e73efb9ff52e23bac7f7e043a1ecd06d05477',
1136+
delegationParamsMock.data,
1137+
],
1138+
jsonrpc: '2.0',
1139+
id: 1680528591,
1140+
origin: 'npm:@metamask/gator-permissions-snap',
1141+
networkClientId: 'mainnet',
1142+
tabId: 1048807182,
1143+
traceContext: null,
1144+
};
1145+
1146+
await controller.newUnsignedTypedMessage(
1147+
delegationParamsMock,
1148+
delegationRequestMock,
1149+
SignTypedDataVersion.V4,
1150+
{ parseJsonData: false },
1151+
);
1152+
1153+
const { decodedPermission } =
1154+
controller.state.signatureRequests[ID_MOCK];
1155+
1156+
expect(decodedPermission).toBeUndefined();
1157+
});
1158+
});
1159+
9361160
describe('decodeSignature', () => {
9371161
it('invoke decodeSignature to get decoding data', async () => {
9381162
const MOCK_STATE_CHANGES = {

0 commit comments

Comments
 (0)