Skip to content

Commit

Permalink
add account support check in validator (#4816)
Browse files Browse the repository at this point in the history
## Explanation

Mirrors the [wallet_createSession handler

](/~https://github.com/MetaMask/metamask-extension/pull/27782/files#diff-107459889087f2776c6db636bd45498bef6749302f9d2dc633b4de17fede40a3R96-R108)
in how eth account support is checked/asserted. Opted to do this rather
than modify `assertScopeSupported` because the `bucketScopes` helper
also relies on `assertScopedSupported` but doesn't care about accounts
(which is why eth accounts are checked outside of assertScopeSupported
in the wallet_createSession handler currently)

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/package-a`

- **<CATEGORY>**: Your change here
- **<CATEGORY>**: Your change here

### `@metamask/package-b`

- **<CATEGORY>**: Your change here
- **<CATEGORY>**: Your change here

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

---------

Co-authored-by: Alex Donesky <adonesky@gmail.com>
  • Loading branch information
jiexi and adonesky1 authored Oct 17, 2024
1 parent 32b213a commit dae4f73
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 2 deletions.
71 changes: 69 additions & 2 deletions packages/multichain/src/caip25Permission.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('endowment:caip25', () => {
const specification = caip25EndowmentBuilder.specificationBuilder({
methodHooks: {
findNetworkClientIdByChainId: jest.fn(),
listAccounts: jest.fn(),
},
});
expect(specification).toStrictEqual({
Expand Down Expand Up @@ -227,9 +228,11 @@ describe('endowment:caip25', () => {

describe('permission validator', () => {
const findNetworkClientIdByChainId = jest.fn();
const listAccounts = jest.fn();
const { validator } = caip25EndowmentBuilder.specificationBuilder({
methodHooks: {
findNetworkClientIdByChainId,
listAccounts,
},
});

Expand Down Expand Up @@ -493,7 +496,7 @@ describe('endowment:caip25', () => {
},
},
normalizedOptionalScopes: {
'eip155:1': {
'eip155:5': {
methods: ['normalized_optional'],
notifications: [],
accounts: [],
Expand Down Expand Up @@ -534,7 +537,7 @@ describe('endowment:caip25', () => {
}
expect(MockScopeAssert.assertScopesSupported).toHaveBeenCalledWith(
{
'eip155:1': {
'eip155:5': {
methods: ['normalized_optional'],
notifications: [],
accounts: [],
Expand All @@ -549,6 +552,61 @@ describe('endowment:caip25', () => {
expect(isChainIdSupportedBody).toContain('findNetworkClientIdByChainId');
});

it('throws if the eth accounts specified in the normalized scopeObjects are not found in the wallet keyring', () => {
MockScopeAuthorization.validateAndNormalizeScopes.mockReturnValue({
normalizedRequiredScopes: {
'eip155:1': {
methods: ['eth_chainId'],
notifications: [],
accounts: ['eip155:1:0xdead'],
},
},
normalizedOptionalScopes: {
'eip155:5': {
methods: [],
notifications: [],
accounts: ['eip155:5:0xbeef'],
},
},
});
listAccounts.mockReturnValue([{ address: '0xdead' }]); // missing '0xbeef'

expect(() => {
validator({
caveats: [
{
type: Caip25CaveatType,
value: {
requiredScopes: {
'eip155:1': {
methods: ['eth_chainId'],
notifications: [],
accounts: ['eip155:1:0xdead'],
},
},
optionalScopes: {
'eip155:5': {
methods: [],
notifications: [],
accounts: ['eip155:5:0xbeef'],
},
},
isMultichainOrigin: true,
},
},
],
date: 1234,
id: '1',
invoker: 'test.com',
parentCapability: Caip25EndowmentPermissionName,
});
}).toThrow(
new Error(
`${Caip25EndowmentPermissionName} error: Received eip155 account value(s) for caveat of type "${Caip25CaveatType}" that were not found in the wallet keyring.`,
),
);
});

it('throws if the input requiredScopes does not match the output of validateAndNormalizeScopes', () => {
MockScopeAuthorization.validateAndNormalizeScopes.mockReturnValue({
normalizedRequiredScopes: {},
Expand All @@ -560,6 +618,8 @@ describe('endowment:caip25', () => {
},
},
});
listAccounts.mockReturnValue([{ address: '0xbeef' }]);

expect(() => {
validator({
caveats: [
Expand Down Expand Up @@ -603,6 +663,8 @@ describe('endowment:caip25', () => {
},
normalizedOptionalScopes: {},
});
listAccounts.mockReturnValue([{ address: '0xdead' }]);

expect(() => {
validator({
caveats: [
Expand Down Expand Up @@ -652,6 +714,11 @@ describe('endowment:caip25', () => {
},
},
});
listAccounts.mockReturnValue([
{ address: '0xdead' },
{ address: '0xbeef' },
]);

expect(
validator({
caveats: [
Expand Down
22 changes: 22 additions & 0 deletions packages/multichain/src/caip25Permission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
import { strict as assert } from 'assert';
import { cloneDeep, isEqual } from 'lodash';

import { getEthAccounts } from './adapters/caip-permission-adapter-eth-accounts';
import { assertScopesSupported } from './scope/assert';
import { validateAndNormalizeScopes } from './scope/authorization';
import type {
Expand Down Expand Up @@ -56,6 +57,7 @@ type Caip25EndowmentSpecification = ValidPermissionSpecification<{
type Caip25EndowmentSpecificationBuilderOptions = {
methodHooks: {
findNetworkClientIdByChainId: (chainId: Hex) => NetworkClientId;
listAccounts: () => { address: Hex }[];
};
};

Expand Down Expand Up @@ -120,6 +122,26 @@ const specificationBuilder: PermissionSpecificationBuilder<
isChainIdSupported,
});

// Fetch EVM accounts from native wallet keyring
// These addresses are lowercased already
const existingEvmAddresses = methodHooks
.listAccounts()
.map((account) => account.address);
const ethAccounts = getEthAccounts({
requiredScopes: normalizedRequiredScopes,
optionalScopes: normalizedOptionalScopes,
isMultichainOrigin,
}).map((address) => address.toLowerCase() as Hex);

const allEthAccountsSupported = ethAccounts.every((address) =>
existingEvmAddresses.includes(address),
);
if (!allEthAccountsSupported) {
throw new Error(
`${Caip25EndowmentPermissionName} error: Received eip155 account value(s) for caveat of type "${Caip25CaveatType}" that were not found in the wallet keyring.`,
);
}

assert.deepEqual(requiredScopes, normalizedRequiredScopes);
assert.deepEqual(optionalScopes, normalizedOptionalScopes);
},
Expand Down

0 comments on commit dae4f73

Please sign in to comment.