Skip to content

Commit

Permalink
Make authorize() throw on excluded permissions (MetaMask#1125)
Browse files Browse the repository at this point in the history
  • Loading branch information
GuillaumeRx authored Jan 13, 2023
1 parent 16abbca commit bd9c71e
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 0 deletions.
28 changes: 28 additions & 0 deletions packages/snaps-controllers/src/snaps/SnapController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2029,6 +2029,34 @@ describe('SnapController', () => {
);
});

it('throws an error if a forbidden permission is requested', async () => {
const initialPermissions = {
// eslint-disable-next-line @typescript-eslint/naming-convention
'endowment:long-running': {},
};

const manifest = {
...getSnapManifest(),
initialPermissions,
};

const messenger = getSnapControllerMessenger();
const controller = getSnapController(
getSnapControllerOptions({
messenger,
detectSnapLocation: loopbackDetect({ manifest }),
// eslint-disable-next-line @typescript-eslint/naming-convention
excludedPermissions: { 'endowment:long-running': 'foobar' },
}),
);

await expect(
controller.installSnaps(MOCK_ORIGIN, {
[MOCK_SNAP_ID]: {},
}),
).rejects.toThrow('One or more permissions are not allowed:\nfoobar');
});

it('maps permission caveats to the proper format', async () => {
const initialPermissions = {
// eslint-disable-next-line @typescript-eslint/naming-convention
Expand Down
27 changes: 27 additions & 0 deletions packages/snaps-controllers/src/snaps/SnapController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,11 @@ type SnapControllerArgs = {
*/
environmentEndowmentPermissions: string[];

/**
* Excluded permissions with its associated error message used to forbid certain permssions.
*/
excludedPermissions: Record<string, string>;

/**
* The function that will be used by the controller fo make network requests.
* Should be compatible with {@link fetch}.
Expand Down Expand Up @@ -616,6 +621,8 @@ export class SnapController extends BaseController<

#environmentEndowmentPermissions: string[];

#excludedPermissions: Record<string, string>;

#featureFlags: FeatureFlags;

#fetchFunction: typeof fetch;
Expand Down Expand Up @@ -649,6 +656,7 @@ export class SnapController extends BaseController<
messenger,
state,
environmentEndowmentPermissions = [],
excludedPermissions = {},
idleTimeCheckInterval = inMilliseconds(5, Duration.Second),
registry = new JsonSnapsRegistry(),
maxIdleTime = inMilliseconds(30, Duration.Second),
Expand Down Expand Up @@ -714,6 +722,7 @@ export class SnapController extends BaseController<

this.#closeAllConnections = closeAllConnections;
this.#environmentEndowmentPermissions = environmentEndowmentPermissions;
this.#excludedPermissions = excludedPermissions;
this.#featureFlags = featureFlags;
this.#fetchFunction = fetchFunction;
this.#idleTimeCheckInterval = idleTimeCheckInterval;
Expand Down Expand Up @@ -2124,6 +2133,24 @@ export class SnapController extends BaseController<
try {
const processedPermissions =
this.#processSnapPermissions(initialPermissions);

const excludedPermissionErrors = Object.keys(processedPermissions).reduce<
string[]
>((errors, permission) => {
if (hasProperty(this.#excludedPermissions, permission)) {
errors.push(this.#excludedPermissions[permission]);
}

return errors;
}, []);

assert(
excludedPermissionErrors.length === 0,
`One or more permissions are not allowed:\n${excludedPermissionErrors.join(
'\n',
)}`,
);

const id = nanoid();
const { permissions: approvedPermissions, ...requestData } =
(await this.messagingSystem.call(
Expand Down

0 comments on commit bd9c71e

Please sign in to comment.