Skip to content

Commit f7a0c13

Browse files
BelfordZadonesky1
BelfordZ
and
adonesky1
authored
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 <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> 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 <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? 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/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 <adonesky@gmail.com>
1 parent 969dfd5 commit f7a0c13

6 files changed

Lines changed: 158 additions & 29 deletions

File tree

‎packages/selected-network-controller/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
},
4040
"devDependencies": {
4141
"@metamask/auto-changelog": "^3.4.4",
42+
"@metamask/permission-controller": "^8.0.1",
4243
"@types/jest": "^27.4.1",
4344
"deepmerge": "^4.2.2",
4445
"immer": "^9.0.6",

‎packages/selected-network-controller/src/SelectedNetworkController.ts‎

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ import type {
88
NetworkControllerStateChangeEvent,
99
ProviderProxy,
1010
} from '@metamask/network-controller';
11+
import type {
12+
PermissionControllerStateChange,
13+
GetSubjects as PermissionControllerGetSubjectsAction,
14+
HasPermissions as PermissionControllerHasPermissions,
15+
} from '@metamask/permission-controller';
1116
import { createEventEmitterProxy } from '@metamask/swappable-obj-proxy';
1217
import type { Patch } from 'immer';
1318

@@ -69,11 +74,6 @@ export type SelectedNetworkControllerSetNetworkClientIdForDomainAction = {
6974
handler: SelectedNetworkController['setNetworkClientIdForDomain'];
7075
};
7176

72-
type PermissionControllerHasPermissions = {
73-
type: `PermissionController:hasPermissions`;
74-
handler: (domain: string) => boolean;
75-
};
76-
7777
export type SelectedNetworkControllerActions =
7878
| SelectedNetworkControllerGetSelectedNetworkStateAction
7979
| SelectedNetworkControllerGetNetworkClientIdForDomainAction
@@ -82,12 +82,15 @@ export type SelectedNetworkControllerActions =
8282
export type AllowedActions =
8383
| NetworkControllerGetNetworkClientByIdAction
8484
| NetworkControllerGetStateAction
85-
| PermissionControllerHasPermissions;
85+
| PermissionControllerHasPermissions
86+
| PermissionControllerGetSubjectsAction;
8687

8788
export type SelectedNetworkControllerEvents =
8889
SelectedNetworkControllerStateChangeEvent;
8990

90-
export type AllowedEvents = NetworkControllerStateChangeEvent;
91+
export type AllowedEvents =
92+
| NetworkControllerStateChangeEvent
93+
| PermissionControllerStateChange;
9194

9295
export type SelectedNetworkControllerMessenger = RestrictedControllerMessenger<
9396
typeof controllerName,
@@ -136,6 +139,45 @@ export class SelectedNetworkController extends BaseController<
136139
});
137140
this.#registerMessageHandlers();
138141

142+
// 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)
143+
this.messagingSystem
144+
.call('PermissionController:getSubjectNames')
145+
.filter((domain) => this.state.domains[domain] === undefined)
146+
.forEach((domain) =>
147+
this.setNetworkClientIdForDomain(
148+
domain,
149+
this.messagingSystem.call('NetworkController:getState')
150+
.selectedNetworkClientId,
151+
),
152+
);
153+
154+
this.messagingSystem.subscribe(
155+
'PermissionController:stateChange',
156+
(_, patches) => {
157+
patches.forEach(({ op, path }) => {
158+
const isChangingSubject =
159+
path[0] === 'subjects' && path[1] !== undefined;
160+
if (isChangingSubject && typeof path[1] === 'string') {
161+
const domain = path[1];
162+
if (op === 'add' && this.state.domains[domain] === undefined) {
163+
this.setNetworkClientIdForDomain(
164+
domain,
165+
this.messagingSystem.call('NetworkController:getState')
166+
.selectedNetworkClientId,
167+
);
168+
} else if (
169+
op === 'remove' &&
170+
this.state.domains[domain] !== undefined
171+
) {
172+
this.update(({ domains }) => {
173+
delete domains[domain];
174+
});
175+
}
176+
}
177+
});
178+
},
179+
);
180+
139181
this.messagingSystem.subscribe(
140182
'NetworkController:stateChange',
141183
({ selectedNetworkClientId }, patches) => {

‎packages/selected-network-controller/tests/SelectedNetworkController.test.ts‎

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ function buildMessenger() {
3232
* @param options - The options bag.
3333
* @param options.messenger - A controller messenger.
3434
* @param options.hasPermissions - Whether the requesting domain has permissions.
35+
* @param options.getSubjectNames - Permissions controller list of domains with permissions
3536
* @returns The network controller restricted messenger.
3637
*/
3738
export function buildSelectedNetworkControllerMessenger({
@@ -40,12 +41,14 @@ export function buildSelectedNetworkControllerMessenger({
4041
SelectedNetworkControllerEvents | AllowedEvents
4142
>(),
4243
hasPermissions,
44+
getSubjectNames,
4345
}: {
4446
messenger?: ControllerMessenger<
4547
SelectedNetworkControllerActions | AllowedActions,
4648
SelectedNetworkControllerEvents | AllowedEvents
4749
>;
4850
hasPermissions?: boolean;
51+
getSubjectNames?: string[];
4952
} = {}): SelectedNetworkControllerMessenger {
5053
messenger.registerActionHandler(
5154
'NetworkController:getNetworkClientById',
@@ -62,25 +65,35 @@ export function buildSelectedNetworkControllerMessenger({
6265
'PermissionController:hasPermissions',
6366
jest.fn().mockReturnValue(hasPermissions),
6467
);
68+
messenger.registerActionHandler(
69+
'PermissionController:getSubjectNames',
70+
jest.fn().mockReturnValue(getSubjectNames),
71+
);
6572
return messenger.getRestricted({
6673
name: controllerName,
6774
allowedActions: [
6875
'NetworkController:getNetworkClientById',
6976
'NetworkController:getState',
7077
'PermissionController:hasPermissions',
78+
'PermissionController:getSubjectNames',
79+
],
80+
allowedEvents: [
81+
'NetworkController:stateChange',
82+
'PermissionController:stateChange',
7183
],
72-
allowedEvents: ['NetworkController:stateChange'],
7384
});
7485
}
7586

7687
jest.mock('@metamask/swappable-obj-proxy');
7788

7889
const setup = ({
7990
hasPermissions = true,
91+
getSubjectNames = [],
8092
state,
8193
}: {
8294
hasPermissions?: boolean;
8395
state?: SelectedNetworkControllerState;
96+
getSubjectNames?: string[];
8497
} = {}) => {
8598
const mockProviderProxy = {
8699
setTarget: jest.fn(),
@@ -121,6 +134,7 @@ const setup = ({
121134
buildSelectedNetworkControllerMessenger({
122135
messenger,
123136
hasPermissions,
137+
getSubjectNames,
124138
});
125139
const controller = new SelectedNetworkController({
126140
messenger: selectedNetworkControllerMessenger,
@@ -432,4 +446,71 @@ describe('SelectedNetworkController', () => {
432446
});
433447
});
434448
});
449+
describe('When a permission is added or removed', () => {
450+
it('should add new domain to domains list on permission add', async () => {
451+
const { controller, messenger } = setup();
452+
const mockPermission = {
453+
parentCapability: 'eth_accounts',
454+
id: 'example.com',
455+
date: Date.now(),
456+
caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }],
457+
};
458+
459+
messenger.publish('PermissionController:stateChange', { subjects: {} }, [
460+
{
461+
op: 'add',
462+
path: ['subjects', 'example.com', 'permissions'],
463+
value: mockPermission,
464+
},
465+
]);
466+
467+
const { domains } = controller.state;
468+
expect(domains['example.com']).toBeDefined();
469+
});
470+
471+
it('should remove domain from domains list on permission removal', async () => {
472+
const { controller, messenger } = setup({
473+
state: { perDomainNetwork: true, domains: { 'example.com': 'foo' } },
474+
});
475+
476+
messenger.publish('PermissionController:stateChange', { subjects: {} }, [
477+
{
478+
op: 'remove',
479+
path: ['subjects', 'example.com', 'permissions'],
480+
},
481+
]);
482+
483+
const { domains } = controller.state;
484+
expect(domains['example.com']).toBeUndefined();
485+
});
486+
});
487+
describe('Constructor checks for domains in permissions', () => {
488+
it('should set networkClientId for domains not already in state', async () => {
489+
const getSubjectNamesMock = ['newdomain.com'];
490+
const { controller } = setup({
491+
state: { perDomainNetwork: true, domains: {} },
492+
getSubjectNames: getSubjectNamesMock,
493+
});
494+
495+
// Now, 'newdomain.com' should have the selectedNetworkClientId set
496+
expect(controller.state.domains['newdomain.com']).toBe('mainnet');
497+
});
498+
499+
it('should not modify domains already in state', async () => {
500+
const { controller } = setup({
501+
state: {
502+
perDomainNetwork: true,
503+
domains: {
504+
'existingdomain.com': 'initialNetworkId',
505+
},
506+
},
507+
getSubjectNames: ['existingdomain.com'],
508+
});
509+
510+
// The 'existingdomain.com' should retain its initial networkClientId
511+
expect(controller.state.domains['existingdomain.com']).toBe(
512+
'initialNetworkId',
513+
);
514+
});
515+
});
435516
});

‎packages/selected-network-controller/tsconfig.build.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
"references": [
99
{ "path": "../base-controller/tsconfig.build.json" },
1010
{ "path": "../network-controller/tsconfig.build.json" },
11-
{ "path": "../json-rpc-engine/tsconfig.build.json" }
11+
{ "path": "../json-rpc-engine/tsconfig.build.json" },
12+
{ "path": "../permission-controller/tsconfig.build.json" }
1213
],
1314
"include": ["../../types", "./src"]
1415
}

‎packages/selected-network-controller/tsconfig.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
},
1414
{
1515
"path": "../json-rpc-engine"
16+
},
17+
{
18+
"path": "../permission-controller"
1619
}
1720
],
1821
"include": ["../../types", "../../tests", "./src", "./tests"]

‎yarn.lock‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2417,26 +2417,7 @@ __metadata:
24172417
languageName: node
24182418
linkType: hard
24192419

2420-
"@metamask/permission-controller@npm:^7.0.0, @metamask/permission-controller@npm:^7.1.0":
2421-
version: 7.1.0
2422-
resolution: "@metamask/permission-controller@npm:7.1.0"
2423-
dependencies:
2424-
"@metamask/base-controller": ^4.0.1
2425-
"@metamask/controller-utils": ^8.0.1
2426-
"@metamask/json-rpc-engine": ^7.3.1
2427-
"@metamask/rpc-errors": ^6.1.0
2428-
"@metamask/utils": ^8.2.0
2429-
"@types/deep-freeze-strict": ^1.1.0
2430-
deep-freeze-strict: ^1.1.1
2431-
immer: ^9.0.6
2432-
nanoid: ^3.1.31
2433-
peerDependencies:
2434-
"@metamask/approval-controller": ^5.1.1
2435-
checksum: 889213cca32cbf5b32b7e71c70ded0aeea32eae169ec67fb0d0bc8dcaa183b222f9d5417f657e331d7fb21ecb71f250cf1c932110d4b1e2167972b30bd012098
2436-
languageName: node
2437-
linkType: hard
2438-
2439-
"@metamask/permission-controller@workspace:packages/permission-controller":
2420+
"@metamask/permission-controller@^8.0.1, @metamask/permission-controller@workspace:packages/permission-controller":
24402421
version: 0.0.0-use.local
24412422
resolution: "@metamask/permission-controller@workspace:packages/permission-controller"
24422423
dependencies:
@@ -2463,6 +2444,25 @@ __metadata:
24632444
languageName: unknown
24642445
linkType: soft
24652446

2447+
"@metamask/permission-controller@npm:^7.0.0, @metamask/permission-controller@npm:^7.1.0":
2448+
version: 7.1.0
2449+
resolution: "@metamask/permission-controller@npm:7.1.0"
2450+
dependencies:
2451+
"@metamask/base-controller": ^4.0.1
2452+
"@metamask/controller-utils": ^8.0.1
2453+
"@metamask/json-rpc-engine": ^7.3.1
2454+
"@metamask/rpc-errors": ^6.1.0
2455+
"@metamask/utils": ^8.2.0
2456+
"@types/deep-freeze-strict": ^1.1.0
2457+
deep-freeze-strict: ^1.1.1
2458+
immer: ^9.0.6
2459+
nanoid: ^3.1.31
2460+
peerDependencies:
2461+
"@metamask/approval-controller": ^5.1.1
2462+
checksum: 889213cca32cbf5b32b7e71c70ded0aeea32eae169ec67fb0d0bc8dcaa183b222f9d5417f657e331d7fb21ecb71f250cf1c932110d4b1e2167972b30bd012098
2463+
languageName: node
2464+
linkType: hard
2465+
24662466
"@metamask/permission-log-controller@workspace:packages/permission-log-controller":
24672467
version: 0.0.0-use.local
24682468
resolution: "@metamask/permission-log-controller@workspace:packages/permission-log-controller"
@@ -2673,6 +2673,7 @@ __metadata:
26732673
"@metamask/base-controller": ^4.1.1
26742674
"@metamask/json-rpc-engine": ^7.3.2
26752675
"@metamask/network-controller": ^17.2.0
2676+
"@metamask/permission-controller": ^8.0.1
26762677
"@metamask/swappable-obj-proxy": ^2.2.0
26772678
"@metamask/utils": ^8.3.0
26782679
"@types/jest": ^27.4.1

0 commit comments

Comments
 (0)