From 0df0dc5a8e54a1d59f2c76cda0eaa68c988d7333 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Fri, 23 Feb 2024 15:06:00 -0800 Subject: [PATCH 1/7] SelectedNetworkController: Listen to permissions changes and add/remove `domains` --- .../selected-network-controller/package.json | 1 + .../src/SelectedNetworkController.ts | 53 ++++++++++++++++++- .../tsconfig.build.json | 3 +- .../selected-network-controller/tsconfig.json | 3 ++ yarn.lock | 41 +++++++------- 5 files changed, 78 insertions(+), 23 deletions(-) diff --git a/packages/selected-network-controller/package.json b/packages/selected-network-controller/package.json index dbe3cc81af..66891b7802 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.0", "@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..558e8a0e3a 100644 --- a/packages/selected-network-controller/src/SelectedNetworkController.ts +++ b/packages/selected-network-controller/src/SelectedNetworkController.ts @@ -8,6 +8,10 @@ import type { NetworkControllerStateChangeEvent, ProviderProxy, } from '@metamask/network-controller'; +import type { + PermissionControllerStateChange, + GetSubjects as PermissionControllerGetSubjectsAction, +} from '@metamask/permission-controller'; import { createEventEmitterProxy } from '@metamask/swappable-obj-proxy'; import type { Patch } from 'immer'; @@ -82,12 +86,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 +143,48 @@ export class SelectedNetworkController extends BaseController< }); this.#registerMessageHandlers(); + this.messagingSystem + .call('PermissionController:getSubjectNames') + .filter( + (domain) => + this.state.domains[domain] === undefined && + 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/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..705b4beb0b 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.0, @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.0 "@metamask/swappable-obj-proxy": ^2.2.0 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 From 0de01217a638c3720fb92a3616ce8afa0acf936a Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Tue, 27 Feb 2024 11:06:07 -0800 Subject: [PATCH 2/7] Update packages/selected-network-controller/src/SelectedNetworkController.ts Co-authored-by: Alex Donesky --- .../src/SelectedNetworkController.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/selected-network-controller/src/SelectedNetworkController.ts b/packages/selected-network-controller/src/SelectedNetworkController.ts index 558e8a0e3a..70f0c13e00 100644 --- a/packages/selected-network-controller/src/SelectedNetworkController.ts +++ b/packages/selected-network-controller/src/SelectedNetworkController.ts @@ -147,8 +147,7 @@ export class SelectedNetworkController extends BaseController< .call('PermissionController:getSubjectNames') .filter( (domain) => - this.state.domains[domain] === undefined && - this.state.domains[domain] === undefined, + this.state.domains[domain] === undefined ) .forEach((domain) => this.setNetworkClientIdForDomain( From 199bda76884383549f2f2623ab2c04eedbd8f0b5 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Tue, 27 Feb 2024 12:07:18 -0800 Subject: [PATCH 3/7] add tests --- .../src/SelectedNetworkController.ts | 5 +- .../tests/SelectedNetworkController.test.ts | 83 ++++++++++++++++++- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/packages/selected-network-controller/src/SelectedNetworkController.ts b/packages/selected-network-controller/src/SelectedNetworkController.ts index 70f0c13e00..eb1e821d86 100644 --- a/packages/selected-network-controller/src/SelectedNetworkController.ts +++ b/packages/selected-network-controller/src/SelectedNetworkController.ts @@ -145,10 +145,7 @@ export class SelectedNetworkController extends BaseController< this.messagingSystem .call('PermissionController:getSubjectNames') - .filter( - (domain) => - this.state.domains[domain] === undefined - ) + .filter((domain) => this.state.domains[domain] === undefined) .forEach((domain) => this.setNetworkClientIdForDomain( domain, 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', + ); + }); + }); }); From eee57c5363a4c52f2d183666ab1dfd693514593f Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Tue, 27 Feb 2024 13:20:26 -0800 Subject: [PATCH 4/7] bump permission controller in selected network controller --- packages/selected-network-controller/package.json | 2 +- yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/selected-network-controller/package.json b/packages/selected-network-controller/package.json index 66891b7802..0f7d979b44 100644 --- a/packages/selected-network-controller/package.json +++ b/packages/selected-network-controller/package.json @@ -39,7 +39,7 @@ }, "devDependencies": { "@metamask/auto-changelog": "^3.4.4", - "@metamask/permission-controller": "^8.0.0", + "@metamask/permission-controller": "^8.0.1", "@types/jest": "^27.4.1", "deepmerge": "^4.2.2", "immer": "^9.0.6", diff --git a/yarn.lock b/yarn.lock index 705b4beb0b..74711dc673 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2417,7 +2417,7 @@ __metadata: languageName: node linkType: hard -"@metamask/permission-controller@^8.0.0, @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: @@ -2673,7 +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.0 + "@metamask/permission-controller": ^8.0.1 "@metamask/swappable-obj-proxy": ^2.2.0 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 From d8a24b2ef23faad74cf710d246b3e21cbab8cc7d Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Tue, 27 Feb 2024 13:31:33 -0800 Subject: [PATCH 5/7] fixups --- .../src/SelectedNetworkController.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/selected-network-controller/src/SelectedNetworkController.ts b/packages/selected-network-controller/src/SelectedNetworkController.ts index eb1e821d86..79623e071b 100644 --- a/packages/selected-network-controller/src/SelectedNetworkController.ts +++ b/packages/selected-network-controller/src/SelectedNetworkController.ts @@ -11,6 +11,7 @@ import type { 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'; @@ -73,11 +74,6 @@ export type SelectedNetworkControllerSetNetworkClientIdForDomainAction = { handler: SelectedNetworkController['setNetworkClientIdForDomain']; }; -type PermissionControllerHasPermissions = { - type: `PermissionController:hasPermissions`; - handler: (domain: string) => boolean; -}; - export type SelectedNetworkControllerActions = | SelectedNetworkControllerGetSelectedNetworkStateAction | SelectedNetworkControllerGetNetworkClientIdForDomainAction From 36ffaca814c1c60f72a876aa785d2b10f2bbecb4 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Tue, 27 Feb 2024 13:33:19 -0800 Subject: [PATCH 6/7] add comment --- .../selected-network-controller/src/SelectedNetworkController.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/selected-network-controller/src/SelectedNetworkController.ts b/packages/selected-network-controller/src/SelectedNetworkController.ts index 79623e071b..283b98cb3d 100644 --- a/packages/selected-network-controller/src/SelectedNetworkController.ts +++ b/packages/selected-network-controller/src/SelectedNetworkController.ts @@ -139,6 +139,7 @@ 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 adding 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) From 6802f49047b4731d52d181bb15b01cc626ceac1a Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Tue, 27 Feb 2024 13:36:35 -0800 Subject: [PATCH 7/7] Update packages/selected-network-controller/src/SelectedNetworkController.ts Co-authored-by: Alex Donesky --- .../src/SelectedNetworkController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/selected-network-controller/src/SelectedNetworkController.ts b/packages/selected-network-controller/src/SelectedNetworkController.ts index 283b98cb3d..9093ae4633 100644 --- a/packages/selected-network-controller/src/SelectedNetworkController.ts +++ b/packages/selected-network-controller/src/SelectedNetworkController.ts @@ -139,7 +139,7 @@ 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 adding them to state here, setting it with the globally selected networkClientId (fetched from the NetworkController) + // 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)