Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

fix: reject unsupported account methods #190

Merged
merged 5 commits into from
Dec 18, 2023
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ node_modules/
# Stores VSCode versions used for testing VSCode extensions
.vscode-test

# IntelliJ
.idea

# yarn v3 (w/o zero-install)
# See: https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
.pnp.*
Expand Down
62 changes: 59 additions & 3 deletions src/SnapKeyring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ describe('SnapKeyring', () => {
id: 'b05d918a-b37c-497a-bb28-3d15c0d56b7a',
address: '0xC728514Df8A7F9271f4B7a4dd2Aa6d2D723d3eE3'.toLowerCase(),
options: {},
methods: [EthMethod.PersonalSign, EthMethod.SignTransaction],
methods: [...Object.values(EthMethod)],
type: EthAccountType.Eoa,
},
{
id: '33c96b60-2237-488e-a7bb-233576f3d22f',
address: '0x34b13912eAc00152bE0Cb409A301Ab8E55739e63'.toLowerCase(),
options: {},
methods: [EthMethod.SignTransaction, EthMethod.SignTypedDataV1],
methods: [...Object.values(EthMethod)],
type: EthAccountType.Eoa,
},
] as const;
Expand Down Expand Up @@ -224,14 +224,70 @@ describe('SnapKeyring', () => {
).toBeNull();
});

it('fails when the method is not supported', async () => {
it('fails when the method is invalid', async () => {
await expect(
keyring.handleKeyringSnapMessage(snapId, {
method: 'invalid',
}),
).rejects.toThrow('Method not supported: invalid');
});

it('fails when the EthMethod is not supported', async () => {
// Update first account to remove `EthMethod.PersonalSign`
let updatedMethods = Object.values(EthMethod).filter(
(method) => method !== EthMethod.PersonalSign,
);
expect(
await keyring.handleKeyringSnapMessage(snapId, {
method: KeyringEvent.AccountUpdated,
params: {
account: {
...accounts[0],
methods: updatedMethods,
},
},
}),
).toBeNull();
expect(keyring.listAccounts()[0]?.methods).toStrictEqual(updatedMethods);
await expect(
keyring.signPersonalMessage(accounts[0].address, 'hello'),
).rejects.toThrow(
`Method '${EthMethod.PersonalSign}' not supported for account ${accounts[0].address}`,
);
// Restore `EthMethod.PersonalSign` and remove `EthMethod.SignTransaction`
updatedMethods = Object.values(EthMethod).filter(
(method) => method !== EthMethod.SignTransaction,
);
expect(
await keyring.handleKeyringSnapMessage(snapId, {
method: KeyringEvent.AccountUpdated,
params: {
account: {
...accounts[0],
methods: updatedMethods,
},
},
}),
).toBeNull();
expect(keyring.listAccounts()[0]?.methods).toStrictEqual(updatedMethods);
const mockTx = {
data: '0x0',
gasLimit: '0x26259fe',
gasPrice: '0x1',
nonce: '0xfffffffe',
to: '0xccccccccccccd000000000000000000000000000',
value: '0x1869e',
chainId: '0x1',
type: '0x00',
};
const tx = TransactionFactory.fromTxData(mockTx);
await expect(
keyring.signTransaction(accounts[0].address, tx),
).rejects.toThrow(
`Method '${EthMethod.SignTransaction}' not supported for account ${accounts[0].address}`,
);
});

it('approves an async request', async () => {
mockSnapController.handleRequest.mockResolvedValue({
pending: true,
Expand Down
25 changes: 15 additions & 10 deletions src/SnapKeyring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,32 @@ import { TransactionFactory } from '@ethereumjs/tx';
import type { TypedDataV1, TypedMessage } from '@metamask/eth-sig-util';
import { SignTypedDataVersion } from '@metamask/eth-sig-util';
import type {
KeyringAccount,
InternalAccount,
EthBaseTransaction,
EthBaseUserOperation,
EthUserOperation,
EthUserOperationPatch,
InternalAccount,
KeyringAccount,
} from '@metamask/keyring-api';
import {
EthMethod,
KeyringSnapControllerClient,
KeyringEvent,
AccountCreatedEventStruct,
AccountUpdatedEventStruct,
AccountDeletedEventStruct,
RequestApprovedEventStruct,
RequestRejectedEventStruct,
AccountUpdatedEventStruct,
EthBaseUserOperationStruct,
EthUserOperationPatchStruct,
EthBytesStruct,
EthMethod,
EthUserOperationPatchStruct,
KeyringEvent,
KeyringSnapControllerClient,
RequestApprovedEventStruct,
RequestRejectedEventStruct,
} from '@metamask/keyring-api';
import type { SnapController } from '@metamask/snaps-controllers';
import type { SnapId } from '@metamask/snaps-sdk';
import type { Json } from '@metamask/utils';
import { bigIntToHex } from '@metamask/utils';
import { EventEmitter } from 'events';
import { assert, object, string, mask } from 'superstruct';
import { assert, mask, object, string } from 'superstruct';
import { v4 as uuid } from 'uuid';

import { DeferredPromise } from './DeferredPromise';
Expand Down Expand Up @@ -396,6 +396,11 @@ export class SnapKeyring extends EventEmitter {
chainId?: string;
}): Promise<Json> {
const { account, snapId } = this.#resolveAddress(address);
if (!account.methods.includes(method as EthMethod)) {
throw new Error(
`Method '${method}' not supported for account ${account.address}`,
);
}
const requestId = uuid();

// Create the promise before calling the snap to prevent a race condition
Expand Down