Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Listen to permissions changes and add/remove domains #3969

Merged
merged 7 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/selected-network-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -69,11 +74,6 @@ export type SelectedNetworkControllerSetNetworkClientIdForDomainAction = {
handler: SelectedNetworkController['setNetworkClientIdForDomain'];
};

type PermissionControllerHasPermissions = {
type: `PermissionController:hasPermissions`;
handler: (domain: string) => boolean;
};

export type SelectedNetworkControllerActions =
| SelectedNetworkControllerGetSelectedNetworkStateAction
| SelectedNetworkControllerGetNetworkClientIdForDomainAction
Expand All @@ -82,12 +82,15 @@ export type SelectedNetworkControllerActions =
export type AllowedActions =
| NetworkControllerGetNetworkClientByIdAction
| NetworkControllerGetStateAction
| PermissionControllerHasPermissions;
| PermissionControllerHasPermissions
BelfordZ marked this conversation as resolved.
Show resolved Hide resolved
| PermissionControllerGetSubjectsAction;

export type SelectedNetworkControllerEvents =
SelectedNetworkControllerStateChangeEvent;

export type AllowedEvents = NetworkControllerStateChangeEvent;
export type AllowedEvents =
| NetworkControllerStateChangeEvent
| PermissionControllerStateChange;

export type SelectedNetworkControllerMessenger = RestrictedControllerMessenger<
typeof controllerName,
Expand Down Expand Up @@ -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;
adonesky1 marked this conversation as resolved.
Show resolved Hide resolved
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];
BelfordZ marked this conversation as resolved.
Show resolved Hide resolved
});
}
}
});
},
);

this.messagingSystem.subscribe(
'NetworkController:stateChange',
({ selectedNetworkClientId }, patches) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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',
Expand All @@ -62,25 +65,35 @@ 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'],
});
}

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

const setup = ({
hasPermissions = true,
getSubjectNames = [],
state,
}: {
hasPermissions?: boolean;
state?: SelectedNetworkControllerState;
getSubjectNames?: string[];
} = {}) => {
const mockProviderProxy = {
setTarget: jest.fn(),
Expand Down Expand Up @@ -121,6 +134,7 @@ const setup = ({
buildSelectedNetworkControllerMessenger({
messenger,
hasPermissions,
getSubjectNames,
});
const controller = new SelectedNetworkController({
messenger: selectedNetworkControllerMessenger,
Expand Down Expand Up @@ -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',
);
});
});
});
3 changes: 2 additions & 1 deletion packages/selected-network-controller/tsconfig.build.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
}
3 changes: 3 additions & 0 deletions packages/selected-network-controller/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
},
{
"path": "../json-rpc-engine"
},
{
"path": "../permission-controller"
}
],
"include": ["../../types", "../../tests", "./src", "./tests"]
Expand Down
41 changes: 21 additions & 20 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Loading