Skip to content

Commit

Permalink
feat(permission-controller): Re-throw all well-formed errors
Browse files Browse the repository at this point in the history
Re-throws all well-formed errors during that occur during permission
validation in `requestPermissions()`. Also restores 100% unit test
coverage.
  • Loading branch information
rekmarks committed Apr 16, 2024
1 parent 69c78bb commit 391be86
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 4 deletions.
101 changes: 99 additions & 2 deletions packages/permission-controller/src/PermissionController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3776,7 +3776,7 @@ describe('PermissionController', () => {
expect(callActionSpy).not.toHaveBeenCalled();
});

it('throws if subjectTypes do not match', async () => {
it('throws if subjectTypes do not match (restricted method)', async () => {
const options = getPermissionControllerOptions();
const { messenger } = options;
const origin = 'metamask.io';
Expand Down Expand Up @@ -3812,6 +3812,42 @@ describe('PermissionController', () => {
);
});

it('throws if subjectTypes do not match (endowment)', async () => {
const options = getPermissionControllerOptions();
const { messenger } = options;
const origin = 'metamask.io';

const callActionSpy = jest
.spyOn(messenger, 'call')
.mockImplementationOnce(() => {
return {
origin,
name: origin,
subjectType: SubjectType.Website,
iconUrl: null,
extensionId: null,
};
});

const controller = getDefaultPermissionController(options);
await expect(
controller.requestPermissions(
{ origin },
{
[PermissionNames.endowmentSnapsOnly]: {},
},
),
).rejects.toThrow(
'Subject "metamask.io" has no permission for "endowmentSnapsOnly".',
);

expect(callActionSpy).toHaveBeenCalledTimes(1);
expect(callActionSpy).toHaveBeenCalledWith(
'SubjectMetadataController:getSubjectMetadata',
origin,
);
});

it('does not throw if subjectTypes match', async () => {
const options = getPermissionControllerOptions();
const { messenger } = options;
Expand Down Expand Up @@ -3900,7 +3936,7 @@ describe('PermissionController', () => {
);
});

it('throws if the "caveat" property of a requested permission is invalid', async () => {
it('throws if the "caveats" property of a requested permission is invalid', async () => {
const options = getPermissionControllerOptions();
const { messenger } = options;
const origin = 'metamask.io';
Expand Down Expand Up @@ -4299,6 +4335,67 @@ describe('PermissionController', () => {
true,
);
});

it('correctly throws errors that do not inherit from JsonRpcError', async () => {
const options = getPermissionControllerOptions();
const { messenger } = options;
const origin = 'metamask.io';
const controller = getDefaultPermissionController(options);

const callActionSpy = jest
.spyOn(messenger, 'call')
// eslint-disable-next-line @typescript-eslint/no-explicit-any
.mockImplementationOnce(async (...args: any) => {
const [, { requestData }] = args;
return {
metadata: { ...requestData.metadata },
permissions: {
[PermissionNames.wallet_getSecretArray]: {
parentCapability: PermissionNames.wallet_getSecretArray,
},
[PermissionNames.wallet_getSecretObject]: {
parentCapability: PermissionNames.wallet_getSecretObject,
caveats: 'foo', // invalid
},
},
};
});

await expect(
async () =>
await controller.requestPermissions(
{ origin },
{
[PermissionNames.wallet_getSecretArray]: {
parentCapability: PermissionNames.wallet_getSecretArray,
},
},
),
).rejects.toThrow(
errors.internalError(
`Invalid approved permissions request: The "caveats" property of permission for "${PermissionNames.wallet_getSecretObject}" of subject "${origin}" is invalid. It must be a non-empty array if specified.`,
),
);

expect(callActionSpy).toHaveBeenCalledTimes(1);
expect(callActionSpy).toHaveBeenCalledWith(
'ApprovalController:addRequest',
{
id: expect.any(String),
origin,
requestData: {
metadata: { id: expect.any(String), origin },
permissions: {
[PermissionNames.wallet_getSecretArray]: {
parentCapability: PermissionNames.wallet_getSecretArray,
},
},
},
type: MethodNames.requestPermissions,
},
true,
);
});
});

describe('acceptPermissionsRequest', () => {
Expand Down
5 changes: 3 additions & 2 deletions packages/permission-controller/src/PermissionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2155,14 +2155,15 @@ export class PermissionController<
try {
this.validateRequestedPermissions(origin, permissions);
} catch (error) {
if (error instanceof JsonRpcError) {
if (error instanceof Error) {
// Re-throw as an internal error; we should never receive invalid approved
// permissions.
throw internalError(
`Invalid approved permissions request: ${error.message}`,
error.data,
error instanceof JsonRpcError ? error.data : undefined,
);
}
/* istanbul ignore next: We should only throw well-formed errors */
throw internalError('Unrecognized error type', { error });
}
}
Expand Down

0 comments on commit 391be86

Please sign in to comment.