diff --git a/packages/permission-controller/ARCHITECTURE.md b/packages/permission-controller/ARCHITECTURE.md index 83119eed959..786a4c31cc0 100644 --- a/packages/permission-controller/ARCHITECTURE.md +++ b/packages/permission-controller/ARCHITECTURE.md @@ -326,6 +326,13 @@ const permissionController = new PermissionController({ ### Adding the permission middleware +The permission middleware is created via `createPermissionMiddlewareV2` for +`JsonRpcEngineV2`, or via the deprecated `createPermissionMiddleware` for the +legacy `JsonRpcEngine`. Both factories take a messenger with the +`PermissionController:executeRestrictedMethod` and +`PermissionController:hasUnrestrictedMethod` actions, typically obtained by +delegating them from a root messenger to a subject-scoped messenger. + ```typescript // This should take place where a middleware stack is created for a particular // subject. @@ -333,11 +340,15 @@ const permissionController = new PermissionController({ // The subject could be a port, stream, socket, etc. const origin = getOrigin(subject); -const engine = new JsonRpcEngine(); -engine.push(/* your various middleware*/); -engine.push(permissionController.createPermissionMiddleware({ origin })); -// Your middleware stack is now permissioned -engine.push(/* your other various middleware*/); +// `messenger` is a messenger delegated the two actions listed above, e.g. +// via `rootMessenger.delegate({ actions: [...], messenger: subjectMessenger })`. +const engine = JsonRpcEngineV2.create({ + middleware: [ + /* your various middleware */ + createPermissionMiddlewareV2({ messenger, subject: { origin } }), + /* your other various middleware */ + ], +}); ``` ### Calling a restricted method internally diff --git a/packages/permission-controller/CHANGELOG.md b/packages/permission-controller/CHANGELOG.md index b3bc6c6d099..f909aed1b4a 100644 --- a/packages/permission-controller/CHANGELOG.md +++ b/packages/permission-controller/CHANGELOG.md @@ -9,14 +9,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Expose `createPermissionMiddleware` through the messenger ([#8502](https://github.com/MetaMask/core/pull/8502)) +- Add `createPermissionMiddlewareV2`, a `JsonRpcEngineV2` variant of the standalone permission middleware factory ([#8532](https://github.com/MetaMask/core/pull/8532)) ### Changed +- **BREAKING:** Decouple the permission middleware from `PermissionController` and expose it as a standalone function ([#8532](https://github.com/MetaMask/core/pull/8532)) + - The standalone `createPermissionMiddleware` replaces the former `PermissionController.createPermissionMiddleware`; it is imported from `@metamask/permission-controller` and called with a messenger and subject metadata, and targets the legacy `JsonRpcEngine`. + - New integrations should prefer `createPermissionMiddlewareV2`, which targets `JsonRpcEngineV2`. + - `PermissionController.getRestrictedMethod` no longer serves a purpose, and is removed. Restricted methods should be invoked via the `:executeRestrictedMethod` action instead. - Bump `@metamask/controller-utils` from `^11.19.0` to `^11.20.0` ([#8344](https://github.com/MetaMask/core/pull/8344)) - Bump `@metamask/messenger` from `^1.0.0` to `^1.1.1` ([#8364](https://github.com/MetaMask/core/pull/8364), [#8373](https://github.com/MetaMask/core/pull/8373)) - Bump `@metamask/base-controller` from `^9.0.1` to `^9.1.0` ([#8457](https://github.com/MetaMask/core/pull/8457)) +### Deprecated + +- Deprecate `createPermissionMiddleware` in favor of `createPermissionMiddlewareV2`, which targets `JsonRpcEngineV2` ([#8532](https://github.com/MetaMask/core/pull/8532)) + ## [12.3.0] ### Added @@ -186,7 +194,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ["Are the Types Wrong?"](https://arethetypeswrong.github.io/) tool as ["masquerading as CJS"](https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseCJS.md). All of the ATTW checks now pass. -- Remove chunk files ([#4648](https://github.com/MetaMask/core/pull/4648)). +- Remove chunk files ([#4648](https://github.com/MetaMask/core/pull/4648)) - Previously, the build tool we used to generate JavaScript files extracted common code to "chunk" files. While this was intended to make this package more tree-shakeable, it also made debugging more difficult for our diff --git a/packages/permission-controller/src/PermissionController-method-action-types.ts b/packages/permission-controller/src/PermissionController-method-action-types.ts index ed47c13ab00..76312a01d3a 100644 --- a/packages/permission-controller/src/PermissionController-method-action-types.ts +++ b/packages/permission-controller/src/PermissionController-method-action-types.ts @@ -6,35 +6,25 @@ import type { PermissionController } from './PermissionController'; /** - * Clears the state of the controller. + * Checks whether the given method was declared as unrestricted at + * construction time. Methods unknown to the controller return `false` and + * would be treated as restricted by callers such as the permission + * middleware. + * + * @param method - The name of the method to check. + * @returns Whether the method is unrestricted. */ -export type PermissionControllerClearStateAction = { - type: `PermissionController:clearState`; - handler: PermissionController['clearState']; +export type PermissionControllerHasUnrestrictedMethodAction = { + type: `PermissionController:hasUnrestrictedMethod`; + handler: PermissionController['hasUnrestrictedMethod']; }; /** - * Creates a permission middleware function. Like any {@link JsonRpcEngine} - * middleware, each middleware will only receive requests from a particular - * subject / origin. - * - * The middlewares returned will pass through requests for - * unrestricted methods, and attempt to execute restricted methods. If a method - * is neither restricted nor unrestricted, a "method not found" error will be - * returned. - * If a method is restricted, the middleware will first attempt to retrieve the - * subject's permission for that method. If the permission is found, the method - * will be executed. Otherwise, an "unauthorized" error will be returned. - * - * The middleware **must** be added in the correct place in the middleware - * stack in order for it to work. See the README for an example. - * - * @param subject The permission subject. - * @returns A `json-rpc-engine` middleware. + * Clears the state of the controller. */ -export type PermissionControllerCreatePermissionMiddlewareAction = { - type: `PermissionController:createPermissionMiddleware`; - handler: PermissionController['createPermissionMiddleware']; +export type PermissionControllerClearStateAction = { + type: `PermissionController:clearState`; + handler: PermissionController['clearState']; }; /** @@ -86,7 +76,7 @@ export type PermissionControllerHasPermissionsAction = { /** * Revokes all permissions from the specified origin. * - * Throws an error of the origin has no permissions. + * Throws an error if the origin has no permissions. * * @param origin - The origin whose permissions to revoke. */ @@ -293,12 +283,42 @@ export type PermissionControllerGetEndowmentsAction = { handler: PermissionController['getEndowments']; }; +/** + * Executes a restricted method as the subject with the given origin. + * The specified params, if any, will be passed to the method implementation. + * + * ATTN: Great caution should be exercised in the use of this method. + * Methods that cause side effects or affect application state should + * be avoided. + * + * This method will first attempt to retrieve the requested restricted method + * implementation, throwing if it does not exist. The method will then be + * invoked as though the subject with the specified origin had invoked it with + * the specified parameters. This means that any existing caveats will be + * applied to the restricted method, and this method will throw if the + * restricted method or its caveat decorators throw. + * + * In addition, this method will throw if the subject does not have a + * permission for the specified restricted method. + * + * @param origin - The origin of the subject to execute the method on behalf + * of. + * @param targetName - The name of the method to execute. This must be a valid + * permission target name. + * @param params - The parameters to pass to the method implementation. + * @returns The result of the executed method. + */ +export type PermissionControllerExecuteRestrictedMethodAction = { + type: `PermissionController:executeRestrictedMethod`; + handler: PermissionController['executeRestrictedMethod']; +}; + /** * Union of all PermissionController action types. */ export type PermissionControllerMethodActions = + | PermissionControllerHasUnrestrictedMethodAction | PermissionControllerClearStateAction - | PermissionControllerCreatePermissionMiddlewareAction | PermissionControllerGetSubjectNamesAction | PermissionControllerGetPermissionsAction | PermissionControllerHasPermissionAction @@ -312,4 +332,5 @@ export type PermissionControllerMethodActions = | PermissionControllerGrantPermissionsIncrementalAction | PermissionControllerRequestPermissionsAction | PermissionControllerRequestPermissionsIncrementalAction - | PermissionControllerGetEndowmentsAction; + | PermissionControllerGetEndowmentsAction + | PermissionControllerExecuteRestrictedMethodAction; diff --git a/packages/permission-controller/src/PermissionController.test.ts b/packages/permission-controller/src/PermissionController.test.ts index b7f7460f632..b90e4f3e7ac 100644 --- a/packages/permission-controller/src/PermissionController.test.ts +++ b/packages/permission-controller/src/PermissionController.test.ts @@ -1,6 +1,7 @@ import { deriveStateFromMetadata } from '@metamask/base-controller'; import { isPlainObject } from '@metamask/controller-utils'; import { JsonRpcEngine } from '@metamask/json-rpc-engine'; +import { JsonRpcEngineV2 } from '@metamask/json-rpc-engine/v2'; import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import type { MessengerActions, @@ -26,6 +27,7 @@ import { PermissionControllerMessenger, PermissionControllerOptions, PermissionControllerState, + PermissionMiddlewareActions, PermissionOptions, PermissionsRequest, RestrictedMethodOptions, @@ -35,6 +37,8 @@ import { import { CaveatMutatorOperation, constructPermission, + createPermissionMiddleware, + createPermissionMiddlewareV2, MethodNames, PermissionController, PermissionType, @@ -634,6 +638,32 @@ function getPermissionControllerMessenger( return messenger; } +/** + * Gets a messenger scoped to the actions required by the permission + * middleware, delegated from the given root messenger. + * + * @param rootMessenger - The root messenger to delegate from. + * @returns A messenger suitable for passing to `createPermissionMiddleware`. + */ +function getPermissionMiddlewareMessenger( + rootMessenger: RootMessenger, +): Messenger<'PermissionMiddleware', PermissionMiddlewareActions> { + const messenger = new Messenger< + 'PermissionMiddleware', + PermissionMiddlewareActions, + never, + RootMessenger + >({ namespace: 'PermissionMiddleware', parent: rootMessenger }); + rootMessenger.delegate({ + actions: [ + 'PermissionController:executeRestrictedMethod', + 'PermissionController:hasUnrestrictedMethod', + ], + messenger, + }); + return messenger; +} + /** * Gets the default unrestricted methods array. * Used as a default in {@link getPermissionControllerOptions}. @@ -929,36 +959,6 @@ describe('PermissionController', () => { }); }); - describe('getRestrictedMethod', () => { - it('gets the implementation of a restricted method', async () => { - const controller = getDefaultPermissionController(); - const method = controller.getRestrictedMethod( - PermissionNames.wallet_getSecretArray, - ); - - expect( - await method({ - method: 'wallet_getSecretArray', - context: { origin: 'github.com' }, - }), - ).toStrictEqual(['a', 'b', 'c']); - }); - - it('throws an error if the requested permission target is not a restricted method', () => { - const controller = getDefaultPermissionController(); - expect(() => - controller.getRestrictedMethod(PermissionNames.endowmentAnySubject), - ).toThrow(errors.methodNotFound(PermissionNames.endowmentAnySubject)); - }); - - it('throws an error if the method does not exist', () => { - const controller = getDefaultPermissionController(); - expect(() => controller.getRestrictedMethod('foo')).toThrow( - errors.methodNotFound('foo'), - ); - }); - }); - describe('getSubjectNames', () => { it('gets all subject names', () => { const controller = getDefaultPermissionController(); @@ -5628,7 +5628,7 @@ describe('PermissionController', () => { ), ).rejects.toThrow( new Error( - `Internal request for method "${PermissionNames.wallet_doubleNumber}" as origin "${origin}" returned no result.`, + `Request for method "${PermissionNames.wallet_doubleNumber}" as origin "${origin}" returned no result.`, ), ); }); @@ -6220,11 +6220,105 @@ describe('PermissionController', () => { .caveats[0], ); }); + + it('action: PermissionController:hasUnrestrictedMethod', () => { + const messenger = getRootMessenger(); + const options = getPermissionControllerOptions({ + messenger: getPermissionControllerMessenger(messenger), + }); + // eslint-disable-next-line no-new + new PermissionController< + DefaultPermissionSpecifications, + DefaultCaveatSpecifications + >(options); + + expect( + messenger.call( + 'PermissionController:hasUnrestrictedMethod', + 'wallet_unrestrictedMethod', + ), + ).toBe(true); + + expect( + messenger.call( + 'PermissionController:hasUnrestrictedMethod', + PermissionNames.wallet_getSecretArray, + ), + ).toBe(false); + + expect( + messenger.call( + 'PermissionController:hasUnrestrictedMethod', + 'wallet_unknownMethod', + ), + ).toBe(false); + }); + + it('action: PermissionController:executeRestrictedMethod', async () => { + const messenger = getRootMessenger(); + const options = getPermissionControllerOptions({ + messenger: getPermissionControllerMessenger(messenger), + }); + const controller = new PermissionController< + DefaultPermissionSpecifications, + DefaultCaveatSpecifications + >(options); + const origin = 'metamask.io'; + + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + [PermissionNames.wallet_getSecretArray]: {}, + }, + }); + + const result = await messenger.call( + 'PermissionController:executeRestrictedMethod', + origin, + PermissionNames.wallet_getSecretArray, + ); + + expect(result).toStrictEqual(['a', 'b', 'c']); + }); }); describe('permission middleware', () => { + /** + * Builds a permission controller and a messenger suitable for passing to + * `createPermissionMiddleware` that are wired to the same root messenger. + * + * @param opts - Permission controller options overrides. + * @returns The controller and the middleware messenger. + */ + const setup = ( + opts?: Record, + ): { + controller: PermissionController< + DefaultPermissionSpecifications, + DefaultCaveatSpecifications + >; + middlewareMessenger: Messenger< + 'PermissionMiddleware', + PermissionMiddlewareActions + >; + } => { + const rootMessenger = getRootMessenger(); + const controller = new PermissionController< + DefaultPermissionSpecifications, + DefaultCaveatSpecifications + >( + getPermissionControllerOptions({ + messenger: getPermissionControllerMessenger(rootMessenger), + ...opts, + }), + ); + const middlewareMessenger = + getPermissionMiddlewareMessenger(rootMessenger); + return { controller, middlewareMessenger }; + }; + it('executes a restricted method', async () => { - const controller = getDefaultPermissionController(); + const { controller, middlewareMessenger } = setup(); const origin = 'metamask.io'; controller.grantPermissions({ @@ -6235,7 +6329,12 @@ describe('PermissionController', () => { }); const engine = new JsonRpcEngine(); - engine.push(controller.createPermissionMiddleware({ origin })); + engine.push( + createPermissionMiddleware({ + messenger: middlewareMessenger, + origin, + }), + ); const response = await engine.handle({ jsonrpc: '2.0', @@ -6248,7 +6347,7 @@ describe('PermissionController', () => { }); it('executes a restricted method with a caveat', async () => { - const controller = getDefaultPermissionController(); + const { controller, middlewareMessenger } = setup(); const origin = 'metamask.io'; controller.grantPermissions({ @@ -6261,7 +6360,12 @@ describe('PermissionController', () => { }); const engine = new JsonRpcEngine(); - engine.push(controller.createPermissionMiddleware({ origin })); + engine.push( + createPermissionMiddleware({ + messenger: middlewareMessenger, + origin, + }), + ); const response = await engine.handle({ jsonrpc: '2.0', @@ -6274,7 +6378,7 @@ describe('PermissionController', () => { }); it('executes a restricted method with multiple caveats', async () => { - const controller = getDefaultPermissionController(); + const { controller, middlewareMessenger } = setup(); const origin = 'metamask.io'; controller.grantPermissions({ @@ -6290,7 +6394,12 @@ describe('PermissionController', () => { }); const engine = new JsonRpcEngine(); - engine.push(controller.createPermissionMiddleware({ origin })); + engine.push( + createPermissionMiddleware({ + messenger: middlewareMessenger, + origin, + }), + ); const response = await engine.handle({ jsonrpc: '2.0', @@ -6303,11 +6412,16 @@ describe('PermissionController', () => { }); it('passes through unrestricted methods', async () => { - const controller = getDefaultPermissionController(); + const { middlewareMessenger } = setup(); const origin = 'metamask.io'; const engine = new JsonRpcEngine(); - engine.push(controller.createPermissionMiddleware({ origin })); + engine.push( + createPermissionMiddleware({ + messenger: middlewareMessenger, + origin, + }), + ); engine.push((_req, res, _next, end) => { res.result = 'success'; end(); @@ -6323,27 +6437,17 @@ describe('PermissionController', () => { expect(response.result).toBe('success'); }); - it('throws an error if the subject has an invalid "origin" property', async () => { - const controller = getDefaultPermissionController(); - - ['', null, undefined, 2].forEach((invalidOrigin) => { - expect(() => - controller.createPermissionMiddleware({ - // @ts-expect-error Intentional destructive testing - origin: invalidOrigin, - }), - ).toThrow( - new Error('The subject "origin" must be a non-empty string.'), - ); - }); - }); - it('returns an error if the subject does not have the requisite permission', async () => { - const controller = getDefaultPermissionController(); + const { middlewareMessenger } = setup(); const origin = 'metamask.io'; const engine = new JsonRpcEngine(); - engine.push(controller.createPermissionMiddleware({ origin })); + engine.push( + createPermissionMiddleware({ + messenger: middlewareMessenger, + origin, + }), + ); const request: JsonRpcRequest<[]> = { jsonrpc: '2.0', @@ -6369,11 +6473,16 @@ describe('PermissionController', () => { }); it('returns an error if the method does not exist', async () => { - const controller = getDefaultPermissionController(); + const { middlewareMessenger } = setup(); const origin = 'metamask.io'; const engine = new JsonRpcEngine(); - engine.push(controller.createPermissionMiddleware({ origin })); + engine.push( + createPermissionMiddleware({ + messenger: middlewareMessenger, + origin, + }), + ); const request: JsonRpcRequest<[]> = { jsonrpc: '2.0', @@ -6401,14 +6510,9 @@ describe('PermissionController', () => { permissionSpecifications.wallet_doubleNumber.methodImplementation = (): undefined => undefined; - const controller = new PermissionController< - DefaultPermissionSpecifications, - DefaultCaveatSpecifications - >( - getPermissionControllerOptions({ - permissionSpecifications, - }), - ); + const { controller, middlewareMessenger } = setup({ + permissionSpecifications, + }); const origin = 'metamask.io'; controller.grantPermissions({ @@ -6419,7 +6523,12 @@ describe('PermissionController', () => { }); const engine = new JsonRpcEngine(); - engine.push(controller.createPermissionMiddleware({ origin })); + engine.push( + createPermissionMiddleware({ + messenger: middlewareMessenger, + origin, + }), + ); const request: JsonRpcRequest<[]> = { jsonrpc: '2.0', @@ -6427,21 +6536,222 @@ describe('PermissionController', () => { method: PermissionNames.wallet_doubleNumber, }; - const expectedError = errors.internalError( - `Request for method "${PermissionNames.wallet_doubleNumber}" returned undefined result.`, - { request: { ...request } }, - ); - const response = await engine.handle(request); assertIsJsonRpcFailure(response); const { error } = response; - expect(error.message).toStrictEqual(expectedError.message); - // @ts-expect-error We do expect this property to exist. - expect(error.data?.cause).toBeNull(); - // @ts-expect-error Intentional destructive testing - delete error.data.cause; - expect(error).toMatchObject(expect.objectContaining(expectedError)); + // The public `executeRestrictedMethod` throws a plain `Error` when the + // restricted method returns `undefined`; the JSON-RPC engine wraps it + // as an internal error response. + expect(error.message).toBe( + `Request for method "${PermissionNames.wallet_doubleNumber}" as origin "${origin}" returned no result.`, + ); + expect(error.code).toBe(-32603); + }); + + describe('v2', () => { + it('executes a restricted method', async () => { + const { controller, middlewareMessenger } = setup(); + const origin = 'metamask.io'; + + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + [PermissionNames.wallet_getSecretArray]: {}, + }, + }); + + const engine = JsonRpcEngineV2.create({ + middleware: [ + createPermissionMiddlewareV2({ + messenger: middlewareMessenger, + origin, + }), + ], + }); + + const result = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: PermissionNames.wallet_getSecretArray, + }); + + expect(result).toStrictEqual(['a', 'b', 'c']); + }); + + it('passes through unrestricted methods', async () => { + const { middlewareMessenger } = setup(); + const origin = 'metamask.io'; + + const engine = JsonRpcEngineV2.create({ + middleware: [ + createPermissionMiddlewareV2({ + messenger: middlewareMessenger, + origin, + }), + (): string => 'success', + ], + }); + + const result = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: 'wallet_unrestrictedMethod', + }); + + expect(result).toBe('success'); + }); + + it('throws if the subject does not have the requisite permission', async () => { + const { middlewareMessenger } = setup(); + const origin = 'metamask.io'; + + const engine = JsonRpcEngineV2.create({ + middleware: [ + createPermissionMiddlewareV2({ + messenger: middlewareMessenger, + origin, + }), + ], + }); + + await expect( + engine.handle({ + jsonrpc: '2.0', + id: 1, + method: PermissionNames.wallet_getSecretArray, + }), + ).rejects.toThrow( + 'Unauthorized to perform action. Try requesting the required permission(s) first.', + ); + }); + + it('executes a restricted method with a caveat', async () => { + const { controller, middlewareMessenger } = setup(); + const origin = 'metamask.io'; + + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + [PermissionNames.wallet_getSecretArray]: { + caveats: [ + { type: CaveatTypes.filterArrayResponse, value: ['b'] }, + ], + }, + }, + }); + + const engine = JsonRpcEngineV2.create({ + middleware: [ + createPermissionMiddlewareV2({ + messenger: middlewareMessenger, + origin, + }), + ], + }); + + const result = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: PermissionNames.wallet_getSecretArray, + }); + + expect(result).toStrictEqual(['b']); + }); + + it('executes a restricted method with multiple caveats', async () => { + const { controller, middlewareMessenger } = setup(); + const origin = 'metamask.io'; + + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + [PermissionNames.wallet_getSecretArray]: { + caveats: [ + { type: CaveatTypes.filterArrayResponse, value: ['a', 'c'] }, + { type: CaveatTypes.reverseArrayResponse, value: null }, + ], + }, + }, + }); + + const engine = JsonRpcEngineV2.create({ + middleware: [ + createPermissionMiddlewareV2({ + messenger: middlewareMessenger, + origin, + }), + ], + }); + + const result = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: PermissionNames.wallet_getSecretArray, + }); + + expect(result).toStrictEqual(['c', 'a']); + }); + + it('throws if the method does not exist', async () => { + const { middlewareMessenger } = setup(); + const origin = 'metamask.io'; + + const engine = JsonRpcEngineV2.create({ + middleware: [ + createPermissionMiddlewareV2({ + messenger: middlewareMessenger, + origin, + }), + ], + }); + + await expect( + engine.handle({ + jsonrpc: '2.0', + id: 1, + method: 'wallet_foo', + }), + ).rejects.toThrow(errors.methodNotFound('wallet_foo', { origin })); + }); + + it('throws if the restricted method returns undefined', async () => { + const permissionSpecifications = getDefaultPermissionSpecifications(); + // @ts-expect-error Intentional destructive testing + permissionSpecifications.wallet_doubleNumber.methodImplementation = + (): undefined => undefined; + + const { controller, middlewareMessenger } = setup({ + permissionSpecifications, + }); + const origin = 'metamask.io'; + + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + [PermissionNames.wallet_doubleNumber]: {}, + }, + }); + + const engine = JsonRpcEngineV2.create({ + middleware: [ + createPermissionMiddlewareV2({ + messenger: middlewareMessenger, + origin, + }), + ], + }); + + await expect( + engine.handle({ + jsonrpc: '2.0', + id: 1, + method: PermissionNames.wallet_doubleNumber, + }), + ).rejects.toThrow( + `Request for method "${PermissionNames.wallet_doubleNumber}" as origin "${origin}" returned no result.`, + ); + }); }); }); diff --git a/packages/permission-controller/src/PermissionController.ts b/packages/permission-controller/src/PermissionController.ts index 619fea0ee56..44d1729478a 100644 --- a/packages/permission-controller/src/PermissionController.ts +++ b/packages/permission-controller/src/PermissionController.ts @@ -16,11 +16,6 @@ import { isPlainObject, isValidJson, } from '@metamask/controller-utils'; -import { - AsyncJsonRpcEngineNextCallback, - createAsyncMiddleware, - JsonRpcMiddleware, -} from '@metamask/json-rpc-engine'; import type { Messenger, ActionConstraint, @@ -28,12 +23,7 @@ import type { } from '@metamask/messenger'; import { JsonRpcError } from '@metamask/rpc-errors'; import { hasProperty } from '@metamask/utils'; -import type { - Json, - JsonRpcRequest, - Mutable, - PendingJsonRpcResponse, -} from '@metamask/utils'; +import type { Json, Mutable } from '@metamask/utils'; import deepFreeze from 'deep-freeze-strict'; import { castDraft, produce as immerProduce } from 'immer'; import type { Draft } from 'immer'; @@ -185,11 +175,13 @@ const controllerName = 'PermissionController'; const MESSENGER_EXPOSED_METHODS = [ 'clearState', + 'executeRestrictedMethod', 'getEndowments', 'getSubjectNames', 'getPermissions', 'hasPermission', 'hasPermissions', + 'hasUnrestrictedMethod', 'grantPermissions', 'grantPermissionsIncremental', 'requestPermissions', @@ -199,7 +191,6 @@ const MESSENGER_EXPOSED_METHODS = [ 'revokePermissions', 'updateCaveat', 'getCaveat', - 'createPermissionMiddleware', ] as const; /** @@ -674,6 +665,19 @@ export class PermissionController< return this.#unrestrictedMethods; } + /** + * Checks whether the given method was declared as unrestricted at + * construction time. Methods unknown to the controller return `false` and + * would be treated as restricted by callers such as the permission + * middleware. + * + * @param method - The name of the method to check. + * @returns Whether the method is unrestricted. + */ + hasUnrestrictedMethod(method: string): boolean { + return this.#unrestrictedMethods.has(method); + } + /** * Constructs the PermissionController. * @@ -881,22 +885,19 @@ export class PermissionController< * @template Type - The type of the permission specification to get. * @param permissionType - The type of the permission specification to get. * @param targetName - The name of the permission whose specification to get. - * @param requestingOrigin - The origin of the requesting subject, if any. - * Will be added to any thrown errors. + * @param requestingOrigin - The origin of the requesting subject. Will be + * added to any thrown errors. * @returns The specification object corresponding to the given type and * target name. */ #getTypedPermissionSpecification( permissionType: Type, targetName: string, - requestingOrigin?: string, + requestingOrigin: string, ): ControllerPermissionSpecification & { permissionType: Type } { const failureError = permissionType === PermissionType.RestrictedMethod - ? methodNotFound( - targetName, - requestingOrigin ? { origin: requestingOrigin } : undefined, - ) + ? methodNotFound(targetName, { origin: requestingOrigin }) : new EndowmentPermissionDoesNotExistError( targetName, requestingOrigin, @@ -914,86 +915,20 @@ export class PermissionController< return specification; } - /** - * Creates a permission middleware function. Like any {@link JsonRpcEngine} - * middleware, each middleware will only receive requests from a particular - * subject / origin. - * - * The middlewares returned will pass through requests for - * unrestricted methods, and attempt to execute restricted methods. If a method - * is neither restricted nor unrestricted, a "method not found" error will be - * returned. - * If a method is restricted, the middleware will first attempt to retrieve the - * subject's permission for that method. If the permission is found, the method - * will be executed. Otherwise, an "unauthorized" error will be returned. - * - * The middleware **must** be added in the correct place in the middleware - * stack in order for it to work. See the README for an example. - * - * @param subject The permission subject. - * @returns A `json-rpc-engine` middleware. - */ - public createPermissionMiddleware( - subject: PermissionSubjectMetadata, - ): JsonRpcMiddleware { - const { origin } = subject; - if (typeof origin !== 'string' || !origin) { - throw new Error('The subject "origin" must be a non-empty string.'); - } - - const permissionsMiddleware = async ( - req: JsonRpcRequest, - res: PendingJsonRpcResponse, - next: AsyncJsonRpcEngineNextCallback, - ): Promise => { - const { method, params } = req; - - // Skip registered unrestricted methods. - if (this.#unrestrictedMethods.has(method)) { - return next(); - } - - // This will throw if no restricted method implementation is found. - const methodImplementation = this.getRestrictedMethod(method, origin); - - // This will throw if the permission does not exist. - const result = await this.#executeRestrictedMethod( - methodImplementation, - subject, - method, - params, - ); - - if (result === undefined) { - res.error = internalError( - `Request for method "${req.method}" returned undefined result.`, - { request: req }, - ); - return undefined; - } - - res.result = result; - return undefined; - }; - - return createAsyncMiddleware(permissionsMiddleware); - } - /** * Gets the implementation of the specified restricted method. * * A JSON-RPC error is thrown if the method does not exist. * - * @see {@link PermissionController.executeRestrictedMethod} and - * {@link PermissionController.createPermissionMiddleware} for internal usage. + * @see {@link PermissionController.executeRestrictedMethod} for internal usage. * @param method - The name of the restricted method. * @param origin - The origin associated with the request for the restricted - * method, if any. + * method. * @returns The restricted method implementation. */ - getRestrictedMethod( + #getRestrictedMethod( method: string, - origin?: string, + origin: string, ): RestrictedMethod { return this.#getTypedPermissionSpecification( PermissionType.RestrictedMethod, @@ -1082,7 +1017,7 @@ export class PermissionController< /** * Revokes all permissions from the specified origin. * - * Throws an error of the origin has no permissions. + * Throws an error if the origin has no permissions. * * @param origin - The origin whose permissions to revoke. */ @@ -2830,7 +2765,7 @@ export class PermissionController< params?: RestrictedMethodParameters, ): Promise { // Throws if the method does not exist - const methodImplementation = this.getRestrictedMethod(targetName, origin); + const methodImplementation = this.#getRestrictedMethod(targetName, origin); const result = await this.#executeRestrictedMethod( methodImplementation, @@ -2839,9 +2774,11 @@ export class PermissionController< params, ); + // This is impossible if the restricted method implementation is typed correctly, + // but we maintain it for backwards compatibility. if (result === undefined) { throw new Error( - `Internal request for method "${targetName}" as origin "${origin}" returned no result.`, + `Request for method "${targetName}" as origin "${origin}" returned no result.`, ); } @@ -2849,17 +2786,15 @@ export class PermissionController< } /** - * An internal method used in the controller's `json-rpc-engine` middleware - * and {@link PermissionController.executeRestrictedMethod}. Calls the - * specified restricted method implementation after decorating it with the - * caveats of its permission. Throws if the subject does not have the + * An internal method used in {@link PermissionController.executeRestrictedMethod}. + * Calls the specified restricted method implementation after decorating it + * with the caveats of its permission. Throws if the subject does not have the * requisite permission. * * ATTN: Parameter validation is the responsibility of the caller, or * the restricted method implementation in the case of `params`. * - * @see {@link PermissionController.executeRestrictedMethod} and - * {@link PermissionController.createPermissionMiddleware} for usage. + * @see {@link PermissionController.executeRestrictedMethod} for usage. * @param methodImplementation - The implementation of the method to call. * @param subject - Metadata about the subject that made the request. * @param method - The method name diff --git a/packages/permission-controller/src/index.ts b/packages/permission-controller/src/index.ts index bf76d3151b5..758510cc35a 100644 --- a/packages/permission-controller/src/index.ts +++ b/packages/permission-controller/src/index.ts @@ -4,22 +4,28 @@ export * from './Permission'; export * from './PermissionController'; export type { PermissionControllerClearStateAction, - PermissionControllerGetSubjectNamesAction, + PermissionControllerExecuteRestrictedMethodAction, + PermissionControllerGetCaveatAction, + PermissionControllerGetEndowmentsAction, PermissionControllerGetPermissionsAction, + PermissionControllerGetSubjectNamesAction, + PermissionControllerGrantPermissionsAction, + PermissionControllerGrantPermissionsIncrementalAction, PermissionControllerHasPermissionAction, PermissionControllerHasPermissionsAction, + PermissionControllerHasUnrestrictedMethodAction, + PermissionControllerRequestPermissionsAction, + PermissionControllerRequestPermissionsIncrementalAction, PermissionControllerRevokeAllPermissionsAction, - PermissionControllerRevokePermissionsAction, PermissionControllerRevokePermissionForAllSubjectsAction, - PermissionControllerGetCaveatAction, + PermissionControllerRevokePermissionsAction, PermissionControllerUpdateCaveatAction, - PermissionControllerGrantPermissionsAction, - PermissionControllerGrantPermissionsIncrementalAction, - PermissionControllerRequestPermissionsAction, - PermissionControllerRequestPermissionsIncrementalAction, - PermissionControllerGetEndowmentsAction, - PermissionControllerCreatePermissionMiddlewareAction, } from './PermissionController-method-action-types'; +export { + createPermissionMiddleware, + createPermissionMiddlewareV2, + type PermissionMiddlewareActions, +} from './permission-middleware'; export type { ExtractSpecifications, HandlerMiddlewareFunction, diff --git a/packages/permission-controller/src/permission-middleware.ts b/packages/permission-controller/src/permission-middleware.ts new file mode 100644 index 00000000000..4ae29c519f8 --- /dev/null +++ b/packages/permission-controller/src/permission-middleware.ts @@ -0,0 +1,112 @@ +import { createAsyncMiddleware } from '@metamask/json-rpc-engine'; +import type { + AsyncJsonRpcEngineNextCallback, + JsonRpcMiddleware, +} from '@metamask/json-rpc-engine'; +import type { JsonRpcMiddleware as JsonRpcMiddlewareV2 } from '@metamask/json-rpc-engine/v2'; +import type { Messenger } from '@metamask/messenger'; +import type { + Json, + JsonRpcRequest, + PendingJsonRpcResponse, +} from '@metamask/utils'; + +import type { RestrictedMethodParameters } from './Permission'; +import type { + PermissionControllerExecuteRestrictedMethodAction, + PermissionControllerHasUnrestrictedMethodAction, +} from './PermissionController-method-action-types'; + +/** + * The set of messenger actions required by the permission middleware. + */ +export type PermissionMiddlewareActions = + | PermissionControllerExecuteRestrictedMethodAction + | PermissionControllerHasUnrestrictedMethodAction; + +export type CreatePermissionMiddlewareOptions = { + messenger: Messenger; + origin: string; +}; + +/** + * Creates a JSON-RPC middleware that enforces permissions for a single subject. + * + * The middleware passes through unrestricted methods, and otherwise dispatches + * restricted methods to the `PermissionController` via messenger actions. If + * the subject lacks the required permission, or if the method does not exist, + * the corresponding error is propagated to the JSON-RPC response. + * + * @deprecated Use {@link createPermissionMiddlewareV2} with `JsonRpcEngineV2`. + * @param options - Options bag. + * @param options.messenger - A messenger with the + * `PermissionController:executeRestrictedMethod` and + * `PermissionController:hasUnrestrictedMethod` actions. + * @param options.origin - The origin of the subject for which to create the middleware. + * @returns A `json-rpc-engine` middleware. + */ +export function createPermissionMiddleware({ + messenger, + origin, +}: CreatePermissionMiddlewareOptions): JsonRpcMiddleware< + RestrictedMethodParameters, + Json +> { + const permissionsMiddleware = async ( + request: JsonRpcRequest, + response: PendingJsonRpcResponse, + next: AsyncJsonRpcEngineNextCallback, + ): Promise => { + const { method, params } = request; + + if (messenger.call('PermissionController:hasUnrestrictedMethod', method)) { + return next(); + } + + response.result = await messenger.call( + 'PermissionController:executeRestrictedMethod', + origin, + method, + params, + ); + return undefined; + }; + + return createAsyncMiddleware(permissionsMiddleware); +} + +/** + * Creates a `JsonRpcEngineV2` middleware that enforces permissions for a + * single subject. + * + * The middleware passes through unrestricted methods, and otherwise dispatches + * restricted methods to the `PermissionController` via messenger actions. If + * the subject lacks the required permission, or if the method does not exist, + * the corresponding error is thrown. + * + * @param options - Options bag. + * @param options.messenger - A messenger with the + * `PermissionController:executeRestrictedMethod` and + * `PermissionController:hasUnrestrictedMethod` actions. + * @param options.origin - The origin of the subject for which to create the middleware. + * @returns A `JsonRpcEngineV2` middleware. + */ +export function createPermissionMiddlewareV2({ + messenger, + origin, +}: CreatePermissionMiddlewareOptions): JsonRpcMiddlewareV2 { + return async ({ request, next }) => { + const { method, params } = request; + + if (messenger.call('PermissionController:hasUnrestrictedMethod', method)) { + return next(); + } + + return messenger.call( + 'PermissionController:executeRestrictedMethod', + origin, + method, + params, + ); + }; +}