From f7a0c1327dc44e4714182a41fe2ea74beda168f1 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Tue, 27 Feb 2024 13:40:17 -0800 Subject: [PATCH] Listen to permissions changes and add/remove `domains` (#3969) See here for more info: https://app.zenhub.com/workspaces/wallet-api-platform-63bee08a4e3b9d001108416e/issues/gh/metamask/metamask-planning/2142 ## Explanation When there exists a permission for a domain, we will then start saving their network selection. We also retroactively add network selections for domains which already have permissions. ## References ## Changelog ### `@metamask/selected-network-controller` - **CHANGED**: Domain selection is written/deleted when permissions are added/removed ## 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 --------- Co-authored-by: Alex Donesky --- .../selected-network-controller/package.json | 1 + .../src/SelectedNetworkController.ts | 56 +++++++++++-- .../tests/SelectedNetworkController.test.ts | 83 ++++++++++++++++++- .../tsconfig.build.json | 3 +- .../selected-network-controller/tsconfig.json | 3 + yarn.lock | 41 ++++----- 6 files changed, 158 insertions(+), 29 deletions(-) diff --git a/packages/selected-network-controller/package.json b/packages/selected-network-controller/package.json index dbe3cc81af..0f7d979b44 100644 --- a/packages/selected-network-controller/package.json +++ b/packages/selected-network-controller/package.json @@ -39,6 +39,7 @@ }, "devDependencies": { "@metamask/auto-changelog": "^3.4.4", + "@metamask/permission-controller": "^8.0.1", "@types/jest": "^27.4.1", "deepmerge": "^4.2.2", "immer": "^9.0.6", diff --git a/packages/selected-network-controller/src/SelectedNetworkController.ts b/packages/selected-network-controller/src/SelectedNetworkController.ts index 20cccbbbd2..9093ae4633 100644 --- a/packages/selected-network-controller/src/SelectedNetworkController.ts +++ b/packages/selected-network-controller/src/SelectedNetworkController.ts @@ -8,6 +8,11 @@ import type { NetworkControllerStateChangeEvent, ProviderProxy, } from '@metamask/network-controller'; +import type { + PermissionControllerStateChange, + GetSubjects as PermissionControllerGetSubjectsAction, + HasPermissions as PermissionControllerHasPermissions, +} from '@metamask/permission-controller'; import { createEventEmitterProxy } from '@metamask/swappable-obj-proxy'; import type { Patch } from 'immer'; @@ -69,11 +74,6 @@ export type SelectedNetworkControllerSetNetworkClientIdForDomainAction = { handler: SelectedNetworkController['setNetworkClientIdForDomain']; }; -type PermissionControllerHasPermissions = { - type: `PermissionController:hasPermissions`; - handler: (domain: string) => boolean; -}; - export type SelectedNetworkControllerActions = | SelectedNetworkControllerGetSelectedNetworkStateAction | SelectedNetworkControllerGetNetworkClientIdForDomainAction @@ -82,12 +82,15 @@ export type SelectedNetworkControllerActions = export type AllowedActions = | NetworkControllerGetNetworkClientByIdAction | NetworkControllerGetStateAction - | PermissionControllerHasPermissions; + | PermissionControllerHasPermissions + | PermissionControllerGetSubjectsAction; export type SelectedNetworkControllerEvents = SelectedNetworkControllerStateChangeEvent; -export type AllowedEvents = NetworkControllerStateChangeEvent; +export type AllowedEvents = + | NetworkControllerStateChangeEvent + | PermissionControllerStateChange; export type SelectedNetworkControllerMessenger = RestrictedControllerMessenger< typeof controllerName, @@ -136,6 +139,45 @@ export class SelectedNetworkController extends BaseController< }); this.#registerMessageHandlers(); + // this is fetching all the dapp permissions from the PermissionsController and looking for any domains that are not in domains state in this controller. Then we take any missing domains and add them to state here, setting it with the globally selected networkClientId (fetched from the NetworkController) + this.messagingSystem + .call('PermissionController:getSubjectNames') + .filter((domain) => this.state.domains[domain] === undefined) + .forEach((domain) => + this.setNetworkClientIdForDomain( + domain, + this.messagingSystem.call('NetworkController:getState') + .selectedNetworkClientId, + ), + ); + + this.messagingSystem.subscribe( + 'PermissionController:stateChange', + (_, patches) => { + patches.forEach(({ op, path }) => { + const isChangingSubject = + path[0] === 'subjects' && path[1] !== undefined; + if (isChangingSubject && typeof path[1] === 'string') { + const domain = path[1]; + if (op === 'add' && this.state.domains[domain] === undefined) { + this.setNetworkClientIdForDomain( + domain, + this.messagingSystem.call('NetworkController:getState') + .selectedNetworkClientId, + ); + } else if ( + op === 'remove' && + this.state.domains[domain] !== undefined + ) { + this.update(({ domains }) => { + delete domains[domain]; + }); + } + } + }); + }, + ); + this.messagingSystem.subscribe( 'NetworkController:stateChange', ({ selectedNetworkClientId }, patches) => { diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index 9a202639e1..a452945634 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -32,6 +32,7 @@ function buildMessenger() { * @param options - The options bag. * @param options.messenger - A controller messenger. * @param options.hasPermissions - Whether the requesting domain has permissions. + * @param options.getSubjectNames - Permissions controller list of domains with permissions * @returns The network controller restricted messenger. */ export function buildSelectedNetworkControllerMessenger({ @@ -40,12 +41,14 @@ export function buildSelectedNetworkControllerMessenger({ SelectedNetworkControllerEvents | AllowedEvents >(), hasPermissions, + getSubjectNames, }: { messenger?: ControllerMessenger< SelectedNetworkControllerActions | AllowedActions, SelectedNetworkControllerEvents | AllowedEvents >; hasPermissions?: boolean; + getSubjectNames?: string[]; } = {}): SelectedNetworkControllerMessenger { messenger.registerActionHandler( 'NetworkController:getNetworkClientById', @@ -62,14 +65,22 @@ export function buildSelectedNetworkControllerMessenger({ 'PermissionController:hasPermissions', jest.fn().mockReturnValue(hasPermissions), ); + messenger.registerActionHandler( + 'PermissionController:getSubjectNames', + jest.fn().mockReturnValue(getSubjectNames), + ); return messenger.getRestricted({ name: controllerName, allowedActions: [ 'NetworkController:getNetworkClientById', 'NetworkController:getState', 'PermissionController:hasPermissions', + 'PermissionController:getSubjectNames', + ], + allowedEvents: [ + 'NetworkController:stateChange', + 'PermissionController:stateChange', ], - allowedEvents: ['NetworkController:stateChange'], }); } @@ -77,10 +88,12 @@ jest.mock('@metamask/swappable-obj-proxy'); const setup = ({ hasPermissions = true, + getSubjectNames = [], state, }: { hasPermissions?: boolean; state?: SelectedNetworkControllerState; + getSubjectNames?: string[]; } = {}) => { const mockProviderProxy = { setTarget: jest.fn(), @@ -121,6 +134,7 @@ const setup = ({ buildSelectedNetworkControllerMessenger({ messenger, hasPermissions, + getSubjectNames, }); const controller = new SelectedNetworkController({ messenger: selectedNetworkControllerMessenger, @@ -432,4 +446,71 @@ describe('SelectedNetworkController', () => { }); }); }); + describe('When a permission is added or removed', () => { + it('should add new domain to domains list on permission add', async () => { + const { controller, messenger } = setup(); + const mockPermission = { + parentCapability: 'eth_accounts', + id: 'example.com', + date: Date.now(), + caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }], + }; + + messenger.publish('PermissionController:stateChange', { subjects: {} }, [ + { + op: 'add', + path: ['subjects', 'example.com', 'permissions'], + value: mockPermission, + }, + ]); + + const { domains } = controller.state; + expect(domains['example.com']).toBeDefined(); + }); + + it('should remove domain from domains list on permission removal', async () => { + const { controller, messenger } = setup({ + state: { perDomainNetwork: true, domains: { 'example.com': 'foo' } }, + }); + + messenger.publish('PermissionController:stateChange', { subjects: {} }, [ + { + op: 'remove', + path: ['subjects', 'example.com', 'permissions'], + }, + ]); + + const { domains } = controller.state; + expect(domains['example.com']).toBeUndefined(); + }); + }); + describe('Constructor checks for domains in permissions', () => { + it('should set networkClientId for domains not already in state', async () => { + const getSubjectNamesMock = ['newdomain.com']; + const { controller } = setup({ + state: { perDomainNetwork: true, domains: {} }, + getSubjectNames: getSubjectNamesMock, + }); + + // Now, 'newdomain.com' should have the selectedNetworkClientId set + expect(controller.state.domains['newdomain.com']).toBe('mainnet'); + }); + + it('should not modify domains already in state', async () => { + const { controller } = setup({ + state: { + perDomainNetwork: true, + domains: { + 'existingdomain.com': 'initialNetworkId', + }, + }, + getSubjectNames: ['existingdomain.com'], + }); + + // The 'existingdomain.com' should retain its initial networkClientId + expect(controller.state.domains['existingdomain.com']).toBe( + 'initialNetworkId', + ); + }); + }); }); diff --git a/packages/selected-network-controller/tsconfig.build.json b/packages/selected-network-controller/tsconfig.build.json index 51944fc30a..0113f47641 100644 --- a/packages/selected-network-controller/tsconfig.build.json +++ b/packages/selected-network-controller/tsconfig.build.json @@ -8,7 +8,8 @@ "references": [ { "path": "../base-controller/tsconfig.build.json" }, { "path": "../network-controller/tsconfig.build.json" }, - { "path": "../json-rpc-engine/tsconfig.build.json" } + { "path": "../json-rpc-engine/tsconfig.build.json" }, + { "path": "../permission-controller/tsconfig.build.json" } ], "include": ["../../types", "./src"] } diff --git a/packages/selected-network-controller/tsconfig.json b/packages/selected-network-controller/tsconfig.json index 5293b22cfb..9e391177a6 100644 --- a/packages/selected-network-controller/tsconfig.json +++ b/packages/selected-network-controller/tsconfig.json @@ -13,6 +13,9 @@ }, { "path": "../json-rpc-engine" + }, + { + "path": "../permission-controller" } ], "include": ["../../types", "../../tests", "./src", "./tests"] diff --git a/yarn.lock b/yarn.lock index a6d71e8b27..74711dc673 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2417,26 +2417,7 @@ __metadata: languageName: node linkType: hard -"@metamask/permission-controller@npm:^7.0.0, @metamask/permission-controller@npm:^7.1.0": - version: 7.1.0 - resolution: "@metamask/permission-controller@npm:7.1.0" - dependencies: - "@metamask/base-controller": ^4.0.1 - "@metamask/controller-utils": ^8.0.1 - "@metamask/json-rpc-engine": ^7.3.1 - "@metamask/rpc-errors": ^6.1.0 - "@metamask/utils": ^8.2.0 - "@types/deep-freeze-strict": ^1.1.0 - deep-freeze-strict: ^1.1.1 - immer: ^9.0.6 - nanoid: ^3.1.31 - peerDependencies: - "@metamask/approval-controller": ^5.1.1 - checksum: 889213cca32cbf5b32b7e71c70ded0aeea32eae169ec67fb0d0bc8dcaa183b222f9d5417f657e331d7fb21ecb71f250cf1c932110d4b1e2167972b30bd012098 - languageName: node - linkType: hard - -"@metamask/permission-controller@workspace:packages/permission-controller": +"@metamask/permission-controller@^8.0.1, @metamask/permission-controller@workspace:packages/permission-controller": version: 0.0.0-use.local resolution: "@metamask/permission-controller@workspace:packages/permission-controller" dependencies: @@ -2463,6 +2444,25 @@ __metadata: languageName: unknown linkType: soft +"@metamask/permission-controller@npm:^7.0.0, @metamask/permission-controller@npm:^7.1.0": + version: 7.1.0 + resolution: "@metamask/permission-controller@npm:7.1.0" + dependencies: + "@metamask/base-controller": ^4.0.1 + "@metamask/controller-utils": ^8.0.1 + "@metamask/json-rpc-engine": ^7.3.1 + "@metamask/rpc-errors": ^6.1.0 + "@metamask/utils": ^8.2.0 + "@types/deep-freeze-strict": ^1.1.0 + deep-freeze-strict: ^1.1.1 + immer: ^9.0.6 + nanoid: ^3.1.31 + peerDependencies: + "@metamask/approval-controller": ^5.1.1 + checksum: 889213cca32cbf5b32b7e71c70ded0aeea32eae169ec67fb0d0bc8dcaa183b222f9d5417f657e331d7fb21ecb71f250cf1c932110d4b1e2167972b30bd012098 + languageName: node + linkType: hard + "@metamask/permission-log-controller@workspace:packages/permission-log-controller": version: 0.0.0-use.local resolution: "@metamask/permission-log-controller@workspace:packages/permission-log-controller" @@ -2673,6 +2673,7 @@ __metadata: "@metamask/base-controller": ^4.1.1 "@metamask/json-rpc-engine": ^7.3.2 "@metamask/network-controller": ^17.2.0 + "@metamask/permission-controller": ^8.0.1 "@metamask/swappable-obj-proxy": ^2.2.0 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1