Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: filter eth requests for non-EVM accounts #25038

Merged
merged 20 commits into from
Jun 14, 2024
Merged

feat: filter eth requests for non-EVM accounts #25038

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e47ea35
feat: add ethMethod filter for nonevm accounts
montelaidev Jun 4, 2024
be11e02
fix: permission test
montelaidev Jun 4, 2024
437d527
fix: use older action until v16
montelaidev Jun 4, 2024
621f65d
fix: middleware to use RestrictedEthMethods instead of RestrictedMethods
montelaidev Jun 6, 2024
bb824e8
Merge remote-tracking branch 'origin/develop' into feat/filter-eth-re…
montelaidev Jun 6, 2024
703bc6c
fix: update to use restricted messenger
montelaidev Jun 6, 2024
8668fee
fix: lint
montelaidev Jun 6, 2024
e902361
fix: eth_requestAccounts
montelaidev Jun 7, 2024
a843ae1
Merge branch 'develop' into feat/filter-eth-requests-for-non-evm-acco…
montelaidev Jun 7, 2024
e621af6
refactor: test and comments
montelaidev Jun 11, 2024
8293cc0
Merge branch 'develop' into feat/filter-eth-requests-for-non-evm-acco…
montelaidev Jun 11, 2024
bc608ed
fix: wallet_requestPermissions test case
montelaidev Jun 11, 2024
e7f1904
fix: styling
montelaidev Jun 12, 2024
182f676
fix: remove lines and add comment
montelaidev Jun 13, 2024
2880da4
Merge branch 'develop' into feat/filter-eth-requests-for-non-evm-acco…
montelaidev Jun 13, 2024
25e2a67
chore: add todo comment
montelaidev Jun 13, 2024
0b65eae
fix: lint
montelaidev Jun 13, 2024
796f872
fix: test
montelaidev Jun 13, 2024
8a9bf36
fix: move code closer
montelaidev Jun 13, 2024
c4b6c1f
Merge remote-tracking branch 'origin/develop' into feat/filter-eth-re…
montelaidev Jun 13, 2024
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
13 changes: 12 additions & 1 deletion app/scripts/controllers/permissions/specifications.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
endowmentCaveatSpecifications as snapsEndowmentCaveatSpecifications,
} from '@metamask/snaps-rpc-methods';
///: END:ONLY_INCLUDE_IF
import { isValidHexAddress } from '@metamask/utils';
import {
CaveatTypes,
RestrictedMethods,
Expand Down Expand Up @@ -141,7 +142,7 @@ export const getPermissionSpecifications = ({
});
},
methodImplementation: async (_args) => {
const accounts = await getAllAccounts();
const accounts = (await getAllAccounts()).filter(isValidHexAddress);
ccharly marked this conversation as resolved.
Show resolved Hide resolved
const internalAccounts = getInternalAccounts();

return accounts.sort((firstAddress, secondAddress) => {
Expand Down Expand Up @@ -278,6 +279,16 @@ function validateCaveatAccounts(accounts, getInternalAccounts) {
});
}

export const UnrestrictedEthSigningMethods = Object.freeze([
montelaidev marked this conversation as resolved.
Show resolved Hide resolved
'eth_sendRawTransaction',
'eth_sendTransaction',
'eth_sign',
'eth_signTypedData',
'eth_signTypedData_v1',
'eth_signTypedData_v3',
'eth_signTypedData_v4',
]);

ccharly marked this conversation as resolved.
Show resolved Hide resolved
/**
* Validates the networks associated with a caveat. Ensures that
* the networks value is an array of valid chain IDs.
Expand Down
47 changes: 30 additions & 17 deletions app/scripts/controllers/permissions/specifications.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ describe('PermissionController specifications', () => {
const getInternalAccounts = jest.fn().mockImplementationOnce(() => {
return [
{
address: '0x1',
address: '0x7A2Bd22810088523516737b4Dc238A4bC37c23F2',
id: '21066553-d8c8-4cdc-af33-efc921cd3ca9',
metadata: {
name: 'Test Account',
Expand All @@ -375,7 +375,7 @@ describe('PermissionController specifications', () => {
type: EthAccountType.Eoa,
},
{
address: '0x2',
address: '0x7152f909e5EB3EF198f17e5Cb087c5Ced88294e3',
id: '0bd7348e-bdfe-4f67-875c-de831a583857',
metadata: {
name: 'Test Account',
Expand All @@ -388,7 +388,7 @@ describe('PermissionController specifications', () => {
type: EthAccountType.Eoa,
},
{
address: '0x3',
address: '0xDe70d2FF1995DC03EF1a3b584e3ae14da020C616',
id: 'ff8fda69-d416-4d25-80a2-efb77bc7d4ad',
metadata: {
name: 'Test Account',
Expand All @@ -402,7 +402,7 @@ describe('PermissionController specifications', () => {
type: EthAccountType.Eoa,
},
{
address: '0x4',
address: '0x04eBa9B766477d8eCA77F5f0e67AE1863C95a7E3',
id: '0bd7348e-bdfe-4f67-875c-de831a583857',
metadata: {
name: 'Test Account',
Expand All @@ -419,26 +419,31 @@ describe('PermissionController specifications', () => {
});
const getAllAccounts = jest
.fn()
.mockImplementationOnce(() => ['0x1', '0x2', '0x3', '0x4']);
.mockImplementationOnce(() => [
'0x7A2Bd22810088523516737b4Dc238A4bC37c23F2',
'0x7152f909e5EB3EF198f17e5Cb087c5Ced88294e3',
'0xDe70d2FF1995DC03EF1a3b584e3ae14da020C616',
'0x04eBa9B766477d8eCA77F5f0e67AE1863C95a7E3',
]);

const { methodImplementation } = getPermissionSpecifications({
getInternalAccounts,
getAllAccounts,
})[RestrictedMethods.eth_accounts];

expect(await methodImplementation()).toStrictEqual([
'0x3',
'0x4',
'0x1',
'0x2',
'0xDe70d2FF1995DC03EF1a3b584e3ae14da020C616',
'0x04eBa9B766477d8eCA77F5f0e67AE1863C95a7E3',
'0x7A2Bd22810088523516737b4Dc238A4bC37c23F2',
'0x7152f909e5EB3EF198f17e5Cb087c5Ced88294e3',
]);
});

it('throws if a keyring account is missing an address (case 1)', async () => {
const getInternalAccounts = jest.fn().mockImplementationOnce(() => {
return [
{
address: '0x2',
address: '0x7152f909e5EB3EF198f17e5Cb087c5Ced88294e3',
id: '0bd7348e-bdfe-4f67-875c-de831a583857',
metadata: {
name: 'Test Account',
Expand All @@ -452,7 +457,7 @@ describe('PermissionController specifications', () => {
type: EthAccountType.Eoa,
},
{
address: '0x3',
address: '0xDe70d2FF1995DC03EF1a3b584e3ae14da020C616',
id: 'ff8fda69-d416-4d25-80a2-efb77bc7d4ad',
metadata: {
name: 'Test Account',
Expand All @@ -469,7 +474,11 @@ describe('PermissionController specifications', () => {
});
const getAllAccounts = jest
.fn()
.mockImplementationOnce(() => ['0x1', '0x2', '0x3']);
.mockImplementationOnce(() => [
'0x7A2Bd22810088523516737b4Dc238A4bC37c23F2',
'0x7152f909e5EB3EF198f17e5Cb087c5Ced88294e3',
'0xDe70d2FF1995DC03EF1a3b584e3ae14da020C616',
]);

const { methodImplementation } = getPermissionSpecifications({
getInternalAccounts,
Expand All @@ -478,15 +487,15 @@ describe('PermissionController specifications', () => {
})[RestrictedMethods.eth_accounts];

await expect(() => methodImplementation()).rejects.toThrow(
'Missing identity for address: "0x1".',
'Missing identity for address: "0x7A2Bd22810088523516737b4Dc238A4bC37c23F2".',
);
});

it('throws if a keyring account is missing an address (case 2)', async () => {
const getInternalAccounts = jest.fn().mockImplementationOnce(() => {
return [
{
address: '0x1',
address: '0x7A2Bd22810088523516737b4Dc238A4bC37c23F2',
id: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3',
metadata: {
name: 'Test Account',
Expand All @@ -500,7 +509,7 @@ describe('PermissionController specifications', () => {
type: EthAccountType.Eoa,
},
{
address: '0x3',
address: '0xDe70d2FF1995DC03EF1a3b584e3ae14da020C616',
id: 'ff8fda69-d416-4d25-80a2-efb77bc7d4ad',
metadata: {
name: 'Test Account',
Expand All @@ -517,7 +526,11 @@ describe('PermissionController specifications', () => {
});
const getAllAccounts = jest
.fn()
.mockImplementationOnce(() => ['0x1', '0x2', '0x3']);
.mockImplementationOnce(() => [
'0x7A2Bd22810088523516737b4Dc238A4bC37c23F2',
'0x7152f909e5EB3EF198f17e5Cb087c5Ced88294e3',
'0xDe70d2FF1995DC03EF1a3b584e3ae14da020C616',
]);

const { methodImplementation } = getPermissionSpecifications({
getInternalAccounts,
Expand All @@ -526,7 +539,7 @@ describe('PermissionController specifications', () => {
})[RestrictedMethods.eth_accounts];

await expect(() => methodImplementation()).rejects.toThrow(
'Missing identity for address: "0x2".',
'Missing identity for address: "0x7152f909e5EB3EF198f17e5Cb087c5Ced88294e3".',
);
});
});
Expand Down
Loading