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

feat: handle accountNameSuggestion and displayConfirmation options in #handleAccountCreated #300

Merged
merged 9 commits into from
May 28, 2024
197 changes: 141 additions & 56 deletions src/SnapKeyring.test.ts
k-g-j marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,18 @@ describe('SnapKeyring', () => {
const mockCallbacks = {
saveState: jest.fn(),
addressExists: jest.fn(),
addAccount: jest.fn(async (_address, _snapId, handleUserInput) => {
await handleUserInput(true);
return Promise.resolve();
}),
addAccount: jest.fn(
async (
_address,
_snapId,
handleUserInput,
_accountNameSuggestion,
_displayConfirmation,
) => {
await handleUserInput(true);
return Promise.resolve();
},
),
removeAccount: jest.fn(async (address, _snapId, handleUserInput) => {
await keyring.removeAccount(address);
await handleUserInput(true);
Expand All @@ -64,6 +72,13 @@ describe('SnapKeyring', () => {
methods: [...Object.values(EthMethod)],
type: EthAccountType.Eoa,
};
const ethEoaAccount3 = {
id: 'c6697bcf-5710-4751-a1cb-340e4b50617a',
address: '0xab1G3q98V7C67T9103g30C0417610237A137d763'.toLowerCase(),
options: {},
methods: [...Object.values(EthMethod)],
type: EthAccountType.Eoa,
};

const ethErc4337Account = {
id: 'fc926fff-f515-4eb5-9952-720bbd9b9849',
Expand All @@ -83,6 +98,7 @@ describe('SnapKeyring', () => {
const accounts = [
ethEoaAccount1,
ethEoaAccount2,
ethEoaAccount3,
ethErc4337Account,
btcP2wpkhAccount,
] as const;
Expand All @@ -97,7 +113,13 @@ describe('SnapKeyring', () => {
mockCallbacks,
);
mockCallbacks.addAccount.mockImplementation(
async (_address, _snapId, handleUserInput) => {
async (
_address,
_snapId,
handleUserInput,
_accountNameSuggestion,
_displayConfirmation,
) => {
await handleUserInput(true);
},
);
Expand Down Expand Up @@ -144,16 +166,63 @@ describe('SnapKeyring', () => {
).rejects.toThrow(`Account '${ethEoaAccount1.id}' already exists`);
});

it('cannot update an account owned by another Snap', async () => {
await expect(
keyring.handleKeyringSnapMessage('a-different-snap-id' as SnapId, {
method: KeyringEvent.AccountCreated,
params: {
account: { ...(ethEoaAccount1 as unknown as KeyringAccount) },
},
}),
).rejects.toThrow(
'Snap "a-different-snap-id" is not allowed to set "b05d918a-b37c-497a-bb28-3d15c0d56b7a"',
describe('with options', () => {
it.each([
[
'handles account creation with accountNameSuggestion',
{ ...ethEoaAccount1 },
'New Account',
undefined,
],
[
'handles account creation with displayConfirmation',
{ ...ethEoaAccount2 },
undefined,
false,
],
[
'handles account creation with both accountNameSuggestion and displayConfirmation',
{ ...ethEoaAccount3 },
'New Account',
false,
],
])(
'%s',
async (
_description,
account,
accountNameSuggestion,
displayConfirmation,
) => {
// Reset mock
mockCallbacks.addAccount.mockClear();
// Reset the keyring so it's empty.
keyring = new SnapKeyring(
mockSnapController as unknown as SnapController,
mockCallbacks,
);

const params = {
account: account as unknown as KeyringAccount,
...(displayConfirmation !== undefined && { displayConfirmation }),
...(accountNameSuggestion !== undefined && {
accountNameSuggestion,
}),
};

await keyring.handleKeyringSnapMessage(snapId, {
method: KeyringEvent.AccountCreated,
params,
});

expect(mockCallbacks.addAccount).toHaveBeenCalledWith(
account.address.toLowerCase(),
snapId,
expect.any(Function),
accountNameSuggestion,
displayConfirmation,
);
},
);
});
});
Expand Down Expand Up @@ -203,6 +272,19 @@ describe('SnapKeyring', () => {
);
});

it('cannot update an account owned by another Snap', async () => {
await expect(
keyring.handleKeyringSnapMessage('a-different-snap-id' as SnapId, {
method: KeyringEvent.AccountCreated,
k-g-j marked this conversation as resolved.
Show resolved Hide resolved
params: {
account: { ...(ethEoaAccount1 as unknown as KeyringAccount) },
},
}),
).rejects.toThrow(
'Snap "a-different-snap-id" is not allowed to set "b05d918a-b37c-497a-bb28-3d15c0d56b7a"',
);
});

it('cannot change the address of an account', async () => {
await expect(
keyring.handleKeyringSnapMessage(snapId, {
Expand All @@ -219,7 +301,7 @@ describe('SnapKeyring', () => {
);
});

it('cannot update an account owned by another Snap', async () => {
it('cannot update an account owned by another snap', async () => {
await expect(
keyring.handleKeyringSnapMessage('invalid-snap-id' as SnapId, {
method: KeyringEvent.AccountUpdated,
Expand Down Expand Up @@ -311,19 +393,21 @@ describe('SnapKeyring', () => {
});
expect(await keyring.getAccounts()).toStrictEqual([
ethEoaAccount2.address.toLowerCase(),
ethEoaAccount3.address.toLowerCase(),
ethErc4337Account.address.toLowerCase(),
btcP2wpkhAccount.address.toLowerCase(),
]);
});

it('cannot delete an account owned by another Snap', async () => {
it('cannot delete an account owned by another snap', async () => {
await keyring.handleKeyringSnapMessage('invalid-snap-id' as SnapId, {
method: KeyringEvent.AccountDeleted,
params: { id: ethEoaAccount1.id },
});
expect(await keyring.getAccounts()).toStrictEqual([
ethEoaAccount1.address.toLowerCase(),
ethEoaAccount2.address.toLowerCase(),
ethEoaAccount3.address.toLowerCase(),
ethErc4337Account.address.toLowerCase(),
btcP2wpkhAccount.address.toLowerCase(),
]);
Expand Down Expand Up @@ -393,7 +477,7 @@ describe('SnapKeyring', () => {
);
});

it("cannot approve another Snap's request", async () => {
it("cannot approve another snap's request", async () => {
mockSnapController.handleRequest.mockResolvedValue({
pending: true,
});
Expand Down Expand Up @@ -465,7 +549,7 @@ describe('SnapKeyring', () => {
);
});

it("cannot reject another Snap's request", async () => {
it("cannot reject another snap's request", async () => {
mockSnapController.handleRequest.mockResolvedValue({
pending: true,
});
Expand Down Expand Up @@ -507,6 +591,7 @@ describe('SnapKeyring', () => {
accounts: {
[ethEoaAccount1.id]: { account: ethEoaAccount1, snapId },
[ethEoaAccount2.id]: { account: ethEoaAccount2, snapId },
[ethEoaAccount3.id]: { account: ethEoaAccount3, snapId },
[ethErc4337Account.id]: { account: ethErc4337Account, snapId },
[btcP2wpkhAccount.id]: { account: btcP2wpkhAccount, snapId },
},
Expand Down Expand Up @@ -552,41 +637,7 @@ describe('SnapKeyring', () => {
expect(await keyring.getAccounts()).toStrictEqual([]);
});
});

describe('async request redirect', () => {
const isNotAllowedOrigin = async (
allowedOrigins: string[],
redirectUrl: string,
) => {
const { origin } = new URL(redirectUrl);
const snapObject = {
id: snapId,
manifest: {
initialPermissions:
allowedOrigins.length > 0
? { 'endowment:keyring': { allowedOrigins } }
: {},
},
enabled: true,
};
mockSnapController.get.mockReturnValue(snapObject);
mockSnapController.handleRequest.mockResolvedValue({
pending: true,
redirect: {
message: 'Go to dapp to continue.',
url: redirectUrl,
},
});
const requestPromise = keyring.signPersonalMessage(
ethEoaAccount1.address,
'hello',
);

await expect(requestPromise).rejects.toThrow(
`Redirect URL domain '${origin}' is not an allowed origin by snap '${snapId}'`,
);
};

it.each([
[{ message: 'Go to dapp to continue.' }],
[{ url: 'https://example.com/sign?tx=1234' }],
Expand Down Expand Up @@ -643,6 +694,38 @@ describe('SnapKeyring', () => {
);
spy.mockRestore();
});
const isNotAllowedOrigin = async (
k-g-j marked this conversation as resolved.
Show resolved Hide resolved
allowedOrigins: string[],
redirectUrl: string,
) => {
const { origin } = new URL(redirectUrl);
const snapObject = {
id: snapId,
manifest: {
initialPermissions:
allowedOrigins.length > 0
? { 'endowment:keyring': { allowedOrigins } }
: {},
},
enabled: true,
};
mockSnapController.get.mockReturnValue(snapObject);
mockSnapController.handleRequest.mockResolvedValue({
pending: true,
redirect: {
message: 'Go to dapp to continue.',
url: redirectUrl,
},
});
const requestPromise = keyring.signPersonalMessage(
ethEoaAccount1.address,
'hello',
);

await expect(requestPromise).rejects.toThrow(
`Redirect URL domain '${origin}' is not an allowed origin by snap '${snapId}'`,
);
};

it('throws an error if async request redirect url is not an allowed origin', async () => {
expect.hasAssertions();
Expand All @@ -657,7 +740,7 @@ describe('SnapKeyring', () => {
await isNotAllowedOrigin([], 'https://example.com/sign?tx=1234');
});

it('throws an error if the Snap is undefined', async () => {
it('throws an error if the snap is undefined', async () => {
const redirect = {
message: 'Go to dapp to continue.',
url: 'https://example.com/sign?tx=1234',
Expand Down Expand Up @@ -1179,9 +1262,10 @@ describe('SnapKeyring', () => {
mockSnapController.handleRequest.mockResolvedValue(null);
await keyring.removeAccount(ethEoaAccount1.address);
expect(await keyring.getAccounts()).toStrictEqual([
ethEoaAccount2.address,
accounts[1].address,
accounts[2].address,
accounts[3].address,
accounts[4].address,
]);
});

Expand All @@ -1190,9 +1274,10 @@ describe('SnapKeyring', () => {
mockSnapController.handleRequest.mockRejectedValue('some error');
await keyring.removeAccount(ethEoaAccount1.address);
expect(await keyring.getAccounts()).toStrictEqual([
ethEoaAccount2.address,
accounts[1].address,
accounts[2].address,
accounts[3].address,
accounts[4].address,
]);
expect(console.error).toHaveBeenCalledWith(
"Account '0xc728514df8a7f9271f4b7a4dd2aa6d2d723d3ee3' may not have been removed from snap 'local:snap.mock':",
Expand Down
Loading
Loading