Skip to content

Commit

Permalink
Add getAllowedKeyringMethods hook for Account Snaps (MetaMask#21246)
Browse files Browse the repository at this point in the history
This PR adds the `getAllowedKeyringMethods` hook. It is used in the `rpc-methods` packed to get the list of allowed Keyring methods for a given origin.

---------

Co-authored-by: Frederik Bolding <[email protected]>
Co-authored-by: Maarten Zuidhoorn <[email protected]>
Co-authored-by: MetaMask Bot <[email protected]>
  • Loading branch information
4 people authored Oct 10, 2023
1 parent b8b3dd6 commit 1e5578e
Show file tree
Hide file tree
Showing 10 changed files with 256 additions and 173 deletions.
75 changes: 75 additions & 0 deletions app/scripts/lib/keyring-snaps-permissions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import {
SubjectMetadataController,
SubjectType,
} from '@metamask/permission-controller';
import { KeyringRpcMethod } from '@metamask/keyring-api';
import { keyringSnapPermissionsBuilder } from './keyring-snaps-permissions';

describe('keyringSnapPermissionsBuilder', () => {
const mockController = new SubjectMetadataController({
subjectCacheLimit: 100,
messenger: {
registerActionHandler: jest.fn(),
publish: jest.fn(),
} as any,
state: {},
});
mockController.addSubjectMetadata({
origin: 'https://some-dapp.com',
subjectType: SubjectType.Website,
});

it('returns the methods metamask can call', () => {
const permissions = keyringSnapPermissionsBuilder(mockController);
expect(permissions('metamask')).toStrictEqual([
KeyringRpcMethod.ListAccounts,
KeyringRpcMethod.GetAccount,
KeyringRpcMethod.FilterAccountChains,
KeyringRpcMethod.DeleteAccount,
KeyringRpcMethod.ListRequests,
KeyringRpcMethod.GetRequest,
KeyringRpcMethod.SubmitRequest,
KeyringRpcMethod.RejectRequest,
]);
});

it('returns the methods a known origin can call', () => {
const permissions = keyringSnapPermissionsBuilder(mockController);
expect(permissions('https://some-dapp.com')).toStrictEqual([
KeyringRpcMethod.ListAccounts,
KeyringRpcMethod.GetAccount,
KeyringRpcMethod.CreateAccount,
KeyringRpcMethod.FilterAccountChains,
KeyringRpcMethod.UpdateAccount,
KeyringRpcMethod.DeleteAccount,
KeyringRpcMethod.ExportAccount,
KeyringRpcMethod.ListRequests,
KeyringRpcMethod.GetRequest,
KeyringRpcMethod.ApproveRequest,
KeyringRpcMethod.RejectRequest,
]);
});

it('returns the methods an unknown origin can call', () => {
const permissions = keyringSnapPermissionsBuilder(mockController);
expect(permissions('https://some-other-dapp.com')).toStrictEqual([]);
});

it.each([
'',
'null',
'sftp://some-dapp.com',
'http://some-dapp.com',
'0',
undefined,
null,
true,
false,
1,
0,
-1,
])('"%s" cannot call any methods', (origin) => {
const permissions = keyringSnapPermissionsBuilder(mockController);
expect(permissions(origin as any)).toStrictEqual([]);
});
});
82 changes: 82 additions & 0 deletions app/scripts/lib/keyring-snaps-permissions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import {
SubjectType,
SubjectMetadataController,
} from '@metamask/permission-controller';
import { KeyringRpcMethod } from '@metamask/keyring-api';

/**
* List of keyring methods MetaMask can call.
*/
const METAMASK_ALLOWED_METHODS: string[] = [
KeyringRpcMethod.ListAccounts,
KeyringRpcMethod.GetAccount,
KeyringRpcMethod.FilterAccountChains,
KeyringRpcMethod.DeleteAccount,
KeyringRpcMethod.ListRequests,
KeyringRpcMethod.GetRequest,
KeyringRpcMethod.SubmitRequest,
KeyringRpcMethod.RejectRequest,
];

/**
* List of keyring methods a dapp can call.
*/
const WEBSITE_ALLOWED_METHODS: string[] = [
KeyringRpcMethod.ListAccounts,
KeyringRpcMethod.GetAccount,
KeyringRpcMethod.CreateAccount,
KeyringRpcMethod.FilterAccountChains,
KeyringRpcMethod.UpdateAccount,
KeyringRpcMethod.DeleteAccount,
KeyringRpcMethod.ExportAccount,
KeyringRpcMethod.ListRequests,
KeyringRpcMethod.GetRequest,
KeyringRpcMethod.ApproveRequest,
KeyringRpcMethod.RejectRequest,
];

/**
* List of allowed protocols. On Flask, HTTP is also allowed for testing.
*/
const ALLOWED_PROTOCOLS: string[] = [
'https:',
///: BEGIN:ONLY_INCLUDE_IN(build-flask)
'http:',
///: END:ONLY_INCLUDE_IN
];

/**
* Checks if the protocol of the origin is allowed.
*
* @param origin - The origin to check.
* @returns `true` if the protocol of the origin is allowed, `false` otherwise.
*/
function isProtocolAllowed(origin: string): boolean {
const url = new URL(origin);
return ALLOWED_PROTOCOLS.includes(url.protocol);
}

/**
* Builds a function that returns the list of keyring methods an origin can
* call.
*
* @param controller - Reference to the `SubjectMetadataController`.
* @returns A function that returns the list of keyring methods an origin can
* call.
*/
export function keyringSnapPermissionsBuilder(
controller: SubjectMetadataController,
): (origin: string) => string[] {
return (origin: string) => {
if (origin === 'metamask') {
return METAMASK_ALLOWED_METHODS;
}

const originMetadata = controller.getSubjectMetadata(origin);
if (originMetadata?.subjectType === SubjectType.Website) {
return isProtocolAllowed(origin) ? WEBSITE_ALLOWED_METHODS : [];
}

return [];
};
}
21 changes: 21 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ import { getTokenValueParam } from '../../shared/lib/metamask-controller-utils';
import { isManifestV3 } from '../../shared/modules/mv3.utils';
import { hexToDecimal } from '../../shared/modules/conversion.utils';
import { ACTION_QUEUE_METRICS_E2E_TEST } from '../../shared/constants/test-flags';
///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps)
import { keyringSnapPermissionsBuilder } from './lib/keyring-snaps-permissions';
///: END:ONLY_INCLUDE_IN

///: BEGIN:ONLY_INCLUDE_IN(petnames)
import { SnapsNameProvider } from './lib/SnapsNameProvider';
Expand Down Expand Up @@ -4426,6 +4429,24 @@ export default class MetamaskController extends EventEmitter {
'SnapController:install',
origin,
),
///: END:ONLY_INCLUDE_IN
///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps)
hasPermission: this.permissionController.hasPermission.bind(
this.permissionController,
),
getSnap: this.controllerMessenger.call.bind(
this.controllerMessenger,
'SnapController:get',
),
handleSnapRpcRequest: this.controllerMessenger.call.bind(
this.controllerMessenger,
'SnapController:handleRequest',
),
getAllowedKeyringMethods: keyringSnapPermissionsBuilder(
this.subjectMetadataController,
),
///: END:ONLY_INCLUDE_IN
///: BEGIN:ONLY_INCLUDE_IN(snaps)
}),
);
///: END:ONLY_INCLUDE_IN
Expand Down
52 changes: 26 additions & 26 deletions lavamoat/browserify/desktop/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1233,9 +1233,9 @@
"packages": {
"@ethereumjs/tx": true,
"@metamask/eth-snap-keyring>@metamask/eth-sig-util": true,
"@metamask/eth-snap-keyring>@metamask/keyring-api": true,
"@metamask/eth-snap-keyring>@metamask/utils": true,
"@metamask/eth-snap-keyring>uuid": true,
"@metamask/keyring-api": true,
"superstruct": true,
"webpack>events": true
}
Expand All @@ -1252,31 +1252,6 @@
"eth-sig-util>tweetnacl-util": true
}
},
"@metamask/eth-snap-keyring>@metamask/keyring-api": {
"packages": {
"@metamask/eth-snap-keyring>@metamask/keyring-api>@metamask/utils": true,
"@metamask/eth-snap-keyring>@metamask/keyring-api>uuid": true,
"superstruct": true
}
},
"@metamask/eth-snap-keyring>@metamask/keyring-api>@metamask/utils": {
"globals": {
"TextDecoder": true,
"TextEncoder": true
},
"packages": {
"@metamask/key-tree>@noble/hashes": true,
"browserify>buffer": true,
"nock>debug": true,
"semver": true,
"superstruct": true
}
},
"@metamask/eth-snap-keyring>@metamask/keyring-api>uuid": {
"globals": {
"crypto": true
}
},
"@metamask/eth-snap-keyring>@metamask/utils": {
"globals": {
"TextDecoder": true,
Expand Down Expand Up @@ -1736,6 +1711,31 @@
"TextEncoder": true
}
},
"@metamask/keyring-api": {
"packages": {
"@metamask/keyring-api>@metamask/utils": true,
"@metamask/keyring-api>uuid": true,
"superstruct": true
}
},
"@metamask/keyring-api>@metamask/utils": {
"globals": {
"TextDecoder": true,
"TextEncoder": true
},
"packages": {
"@metamask/key-tree>@noble/hashes": true,
"browserify>buffer": true,
"nock>debug": true,
"semver": true,
"superstruct": true
}
},
"@metamask/keyring-api>uuid": {
"globals": {
"crypto": true
}
},
"@metamask/keyring-controller": {
"packages": {
"@metamask/base-controller": true,
Expand Down
52 changes: 26 additions & 26 deletions lavamoat/browserify/flask/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1233,9 +1233,9 @@
"packages": {
"@ethereumjs/tx": true,
"@metamask/eth-snap-keyring>@metamask/eth-sig-util": true,
"@metamask/eth-snap-keyring>@metamask/keyring-api": true,
"@metamask/eth-snap-keyring>@metamask/utils": true,
"@metamask/eth-snap-keyring>uuid": true,
"@metamask/keyring-api": true,
"superstruct": true,
"webpack>events": true
}
Expand All @@ -1252,31 +1252,6 @@
"eth-sig-util>tweetnacl-util": true
}
},
"@metamask/eth-snap-keyring>@metamask/keyring-api": {
"packages": {
"@metamask/eth-snap-keyring>@metamask/keyring-api>@metamask/utils": true,
"@metamask/eth-snap-keyring>@metamask/keyring-api>uuid": true,
"superstruct": true
}
},
"@metamask/eth-snap-keyring>@metamask/keyring-api>@metamask/utils": {
"globals": {
"TextDecoder": true,
"TextEncoder": true
},
"packages": {
"@metamask/key-tree>@noble/hashes": true,
"browserify>buffer": true,
"nock>debug": true,
"semver": true,
"superstruct": true
}
},
"@metamask/eth-snap-keyring>@metamask/keyring-api>uuid": {
"globals": {
"crypto": true
}
},
"@metamask/eth-snap-keyring>@metamask/utils": {
"globals": {
"TextDecoder": true,
Expand Down Expand Up @@ -1736,6 +1711,31 @@
"TextEncoder": true
}
},
"@metamask/keyring-api": {
"packages": {
"@metamask/keyring-api>@metamask/utils": true,
"@metamask/keyring-api>uuid": true,
"superstruct": true
}
},
"@metamask/keyring-api>@metamask/utils": {
"globals": {
"TextDecoder": true,
"TextEncoder": true
},
"packages": {
"@metamask/key-tree>@noble/hashes": true,
"browserify>buffer": true,
"nock>debug": true,
"semver": true,
"superstruct": true
}
},
"@metamask/keyring-api>uuid": {
"globals": {
"crypto": true
}
},
"@metamask/keyring-controller": {
"packages": {
"@metamask/base-controller": true,
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,15 @@
"@metamask/eth-json-rpc-middleware": "^11.0.0",
"@metamask/eth-keyring-controller": "^13.0.1",
"@metamask/eth-ledger-bridge-keyring": "^0.15.0",
"@metamask/eth-snap-keyring": "0.3.1",
"@metamask/eth-snap-keyring": "^1.0.0-rc.1",
"@metamask/eth-token-tracker": "^4.0.0",
"@metamask/eth-trezor-keyring": "^1.1.0",
"@metamask/etherscan-link": "^2.2.0",
"@metamask/ethjs-query": "^0.5.0",
"@metamask/gas-fee-controller": "^6.0.1",
"@metamask/jazzicon": "^2.0.0",
"@metamask/key-tree": "^9.0.0",
"@metamask/keyring-api": "^1.0.0-rc.1",
"@metamask/keyring-controller": "^8.0.1",
"@metamask/logging-controller": "^1.0.1",
"@metamask/logo": "^3.1.2",
Expand Down
3 changes: 1 addition & 2 deletions shared/constants/snaps/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@ export const ExcludedSnapPermissions = Object.freeze({
});

export const ExcludedSnapEndowments = Object.freeze({
// Move to below fence once implemented
///: BEGIN:ONLY_INCLUDE_IN(build-main)
'endowment:keyring':
'This endowment is still in development therefore not available.',
///: BEGIN:ONLY_INCLUDE_IN(build-main)
'endowment:lifecycle-hooks':
'This endowment is experimental and therefore not available.',
'endowment:name-lookup':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ describe('Add snap account experimental settings', function () {

// Make sure the "Add snap account" button is not visible.
await driver.clickElement('[data-testid="account-menu-icon"]');
await driver.clickElement(
'[data-testid="multichain-account-menu-popover-action-button"]',
);
await driver.assertElementNotPresent({
text: 'Add snap account',
tag: 'button',
Expand All @@ -39,6 +42,9 @@ describe('Add snap account experimental settings', function () {

// Make sure the "Add snap account" button is visible.
await driver.clickElement('[data-testid="account-menu-icon"]');
await driver.clickElement(
'[data-testid="multichain-account-menu-popover-action-button"]',
);
assert.equal(
await driver.isElementPresentAndVisible({
text: 'Add snap account',
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/accounts/utils.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export const TEST_SNAPS_SIMPLE_KEYRING_WEBSITE_URL =
'https://metamask.github.io/snap-simple-keyring/0.2.3/';
'https://metamask.github.io/snap-simple-keyring/0.3.0/';
Loading

0 comments on commit 1e5578e

Please sign in to comment.