diff --git a/README.md b/README.md index 3122d17adc4..ab10b22fdc0 100644 --- a/README.md +++ b/README.md @@ -386,6 +386,7 @@ linkStyle default opacity:0.5 multichain_account_service --> keyring_controller; multichain_account_service --> messenger; multichain_account_service --> controller_utils; + multichain_api_middleware --> accounts_controller; multichain_api_middleware --> chain_agnostic_permission; multichain_api_middleware --> controller_utils; multichain_api_middleware --> json_rpc_engine; diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 793f8d99be9..829215c5187 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1334,7 +1334,7 @@ }, "packages/multichain-api-middleware/src/handlers/wallet-createSession.ts": { "@typescript-eslint/explicit-function-return-type": { - "count": 2 + "count": 1 }, "@typescript-eslint/prefer-nullish-coalescing": { "count": 2 diff --git a/packages/eip1193-permission-middleware/CHANGELOG.md b/packages/eip1193-permission-middleware/CHANGELOG.md index bd8c25f51fb..23244ecd9b3 100644 --- a/packages/eip1193-permission-middleware/CHANGELOG.md +++ b/packages/eip1193-permission-middleware/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** Consolidate method handlers into a single `methodHandlers` export ([#8583](https://github.com/MetaMask/core/pull/8583)) + - The individual handler exports have been removed. They can still be accessed as properties on the `methodHandlers` export. + - The new handlers follow the format expected by `createMethodMiddleware` from `@metamask/json-rpc-engine@10.3.0`. + - The hook types have been updated to cohere with the corresponding `@metamask/permission-controller` methods. - Bump `@metamask/chain-agnostic-permission` from `^1.4.0` to `^1.5.0` ([#8290](https://github.com/MetaMask/core/pull/8290)) - Bump `@metamask/json-rpc-engine` from `^10.2.0` to `^10.2.4` ([#7642](https://github.com/MetaMask/core/pull/7642), [#7856](https://github.com/MetaMask/core/pull/7856), [#8078](https://github.com/MetaMask/core/pull/8078), [#8317](https://github.com/MetaMask/core/pull/8317)) - Upgrade `@metamask/utils` from `^11.8.1` to `^11.9.0` ([#7511](https://github.com/MetaMask/core/pull/7511)) diff --git a/packages/eip1193-permission-middleware/src/index.test.ts b/packages/eip1193-permission-middleware/src/index.test.ts index 63fa4531016..feed1294273 100644 --- a/packages/eip1193-permission-middleware/src/index.test.ts +++ b/packages/eip1193-permission-middleware/src/index.test.ts @@ -1,13 +1,40 @@ +import { createMethodMiddleware } from '@metamask/json-rpc-engine'; + import * as allExports from '.'; +import type { GetPermissionsHooks } from './wallet-getPermissions'; +import type { RequestPermissionsHooks } from './wallet-requestPermissions'; +import type { RevokePermissionsHooks } from './wallet-revokePermissions'; + +type Hooks = GetPermissionsHooks & + RequestPermissionsHooks & + RevokePermissionsHooks; + +/* eslint-disable @typescript-eslint/explicit-function-return-type */ +const makeMockHooks = () => + ({ + getPermissionsForOrigin: () => ({}), + getAccounts: () => ['0x123'], + requestPermissionsForOrigin: () => + Promise.resolve([{}, { id: '1', origin: 'test' }]), + revokePermissionsForOrigin: () => undefined, + getCaip25PermissionFromLegacyPermissionsForOrigin: () => ({}), + }) satisfies Hooks; +/* eslint-enable @typescript-eslint/explicit-function-return-type */ describe('@metamask/eip1193-permission-middleware', () => { it('has expected JavaScript exports', () => { expect(Object.keys(allExports)).toMatchInlineSnapshot(` [ - "getPermissionsHandler", - "requestPermissionsHandler", - "revokePermissionsHandler", + "methodHandlers", ] `); }); + + it('constructs a method middleware from the handlers', () => { + const middleware = createMethodMiddleware({ + handlers: allExports.methodHandlers, + hooks: makeMockHooks(), + }); + expect(middleware).toBeDefined(); + }); }); diff --git a/packages/eip1193-permission-middleware/src/index.ts b/packages/eip1193-permission-middleware/src/index.ts index 3cf3a13c49a..ef0b2336e84 100644 --- a/packages/eip1193-permission-middleware/src/index.ts +++ b/packages/eip1193-permission-middleware/src/index.ts @@ -1,3 +1,17 @@ -export { getPermissionsHandler } from './wallet-getPermissions'; -export { requestPermissionsHandler } from './wallet-requestPermissions'; -export { revokePermissionsHandler } from './wallet-revokePermissions'; +import { MethodNames } from '@metamask/permission-controller'; + +import { getPermissionsHandler } from './wallet-getPermissions'; +import { requestPermissionsHandler } from './wallet-requestPermissions'; +import { revokePermissionsHandler } from './wallet-revokePermissions'; + +type MethodHandlers = { + [MethodNames.GetPermissions]: typeof getPermissionsHandler; + [MethodNames.RequestPermissions]: typeof requestPermissionsHandler; + [MethodNames.RevokePermissions]: typeof revokePermissionsHandler; +}; + +export const methodHandlers: Readonly = { + [MethodNames.GetPermissions]: getPermissionsHandler, + [MethodNames.RequestPermissions]: requestPermissionsHandler, + [MethodNames.RevokePermissions]: revokePermissionsHandler, +}; diff --git a/packages/eip1193-permission-middleware/src/wallet-getPermissions.ts b/packages/eip1193-permission-middleware/src/wallet-getPermissions.ts index 705899e16ab..52914bc1ff4 100644 --- a/packages/eip1193-permission-middleware/src/wallet-getPermissions.ts +++ b/packages/eip1193-permission-middleware/src/wallet-getPermissions.ts @@ -5,15 +5,11 @@ import { getPermittedEthChainIds, } from '@metamask/chain-agnostic-permission'; import type { - AsyncJsonRpcEngineNextCallback, JsonRpcEngineEndCallback, + JsonRpcEngineNextCallback, + MethodHandler, } from '@metamask/json-rpc-engine'; -import { MethodNames } from '@metamask/permission-controller'; -import type { - CaveatSpecificationConstraint, - PermissionController, - PermissionSpecificationConstraint, -} from '@metamask/permission-controller'; +import type { GenericPermissionController } from '@metamask/permission-controller'; import type { Json, JsonRpcRequest, @@ -22,14 +18,28 @@ import type { import { CaveatTypes, EndowmentTypes, RestrictedMethods } from './types'; +export type GetPermissionsHooks = { + getPermissionsForOrigin: () => ReturnType< + GenericPermissionController['getPermissions'] + >; + getAccounts: (options?: { ignoreLock?: boolean }) => string[]; +}; + +export type GetPermissionsHandler = MethodHandler< + GetPermissionsHooks, + never, + Json[], + Json, + { origin: string } +>; + export const getPermissionsHandler = { - methodNames: [MethodNames.GetPermissions], implementation: getPermissionsImplementation, hookNames: { getPermissionsForOrigin: true, getAccounts: true, }, -}; +} satisfies GetPermissionsHandler; /** * Get Permissions implementation to be used in JsonRpcEngine middleware, specifically for `wallet_getPermissions` RPC method. @@ -47,20 +57,9 @@ export const getPermissionsHandler = { async function getPermissionsImplementation( _req: JsonRpcRequest, res: PendingJsonRpcResponse, - _next: AsyncJsonRpcEngineNextCallback, + _next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, - { - getPermissionsForOrigin, - getAccounts, - }: { - getPermissionsForOrigin: () => ReturnType< - PermissionController< - PermissionSpecificationConstraint, - CaveatSpecificationConstraint - >['getPermissions'] - >; - getAccounts: (options?: { ignoreLock?: boolean }) => string[]; - }, + { getPermissionsForOrigin, getAccounts }: GetPermissionsHooks, ) { const permissions = { ...getPermissionsForOrigin() }; const caip25Endowment = permissions[Caip25EndowmentPermissionName]; diff --git a/packages/eip1193-permission-middleware/src/wallet-requestPermissions.ts b/packages/eip1193-permission-middleware/src/wallet-requestPermissions.ts index 6b882e3dc07..ec723df7ba3 100644 --- a/packages/eip1193-permission-middleware/src/wallet-requestPermissions.ts +++ b/packages/eip1193-permission-middleware/src/wallet-requestPermissions.ts @@ -6,15 +6,14 @@ import { } from '@metamask/chain-agnostic-permission'; import { isPlainObject } from '@metamask/controller-utils'; import type { - AsyncJsonRpcEngineNextCallback, + JsonRpcEngineNextCallback, JsonRpcEngineEndCallback, + MethodHandler, } from '@metamask/json-rpc-engine'; -import { invalidParams, MethodNames } from '@metamask/permission-controller'; +import { invalidParams } from '@metamask/permission-controller'; import type { Caveat, - CaveatSpecificationConstraint, - PermissionController, - PermissionSpecificationConstraint, + GenericPermissionController, RequestedPermissions, ValidPermission, } from '@metamask/permission-controller'; @@ -27,23 +26,35 @@ import { pick } from 'lodash'; import { CaveatTypes, EndowmentTypes, RestrictedMethods } from './types'; +export type RequestPermissionsHooks = { + getAccounts: () => string[]; + requestPermissionsForOrigin: ( + requestedPermissions: RequestedPermissions, + ) => ReturnType; + getCaip25PermissionFromLegacyPermissionsForOrigin: ( + requestedPermissions?: RequestedPermissions, + ) => RequestedPermissions; +}; + +export type RequestPermissionsHandler = MethodHandler< + RequestPermissionsHooks, + never, + [RequestedPermissions], + Json, + { origin: string } +>; + export const requestPermissionsHandler = { - methodNames: [MethodNames.RequestPermissions], implementation: requestPermissionsImplementation, hookNames: { getAccounts: true, requestPermissionsForOrigin: true, getCaip25PermissionFromLegacyPermissionsForOrigin: true, }, -}; - -type AbstractPermissionController = PermissionController< - PermissionSpecificationConstraint, - CaveatSpecificationConstraint ->; +} satisfies RequestPermissionsHandler; type GrantedPermissions = Awaited< - ReturnType + ReturnType >[0]; /** @@ -63,21 +74,13 @@ type GrantedPermissions = Awaited< async function requestPermissionsImplementation( req: JsonRpcRequest<[RequestedPermissions]> & { origin: string }, res: PendingJsonRpcResponse, - _next: AsyncJsonRpcEngineNextCallback, + _next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, { getAccounts, requestPermissionsForOrigin, getCaip25PermissionFromLegacyPermissionsForOrigin, - }: { - getAccounts: () => string[]; - requestPermissionsForOrigin: ( - requestedPermissions: RequestedPermissions, - ) => Promise<[GrantedPermissions]>; - getCaip25PermissionFromLegacyPermissionsForOrigin: ( - requestedPermissions?: RequestedPermissions, - ) => RequestedPermissions; - }, + }: RequestPermissionsHooks, ) { const { params } = req; diff --git a/packages/eip1193-permission-middleware/src/wallet-revokePermissions.ts b/packages/eip1193-permission-middleware/src/wallet-revokePermissions.ts index 828679498bb..8f051393a16 100644 --- a/packages/eip1193-permission-middleware/src/wallet-revokePermissions.ts +++ b/packages/eip1193-permission-middleware/src/wallet-revokePermissions.ts @@ -1,9 +1,11 @@ import { Caip25EndowmentPermissionName } from '@metamask/chain-agnostic-permission'; import type { - AsyncJsonRpcEngineNextCallback, + JsonRpcEngineNextCallback, JsonRpcEngineEndCallback, + MethodHandler, } from '@metamask/json-rpc-engine'; -import { invalidParams, MethodNames } from '@metamask/permission-controller'; +import { invalidParams } from '@metamask/permission-controller'; +import type { GenericPermissionController } from '@metamask/permission-controller'; import { isNonEmptyArray } from '@metamask/utils'; import type { Json, @@ -13,14 +15,26 @@ import type { import { EndowmentTypes, RestrictedMethods } from './types'; +export type RevokePermissionsHooks = { + revokePermissionsForOrigin: ( + permissionKeys: string[], + ) => ReturnType; +}; + +export type RevokePermissionsHandler = MethodHandler< + RevokePermissionsHooks, + never, + Json[], + Json, + { origin: string } +>; + export const revokePermissionsHandler = { - methodNames: [MethodNames.RevokePermissions], implementation: revokePermissionsImplementation, hookNames: { revokePermissionsForOrigin: true, - updateCaveat: true, }, -}; +} satisfies RevokePermissionsHandler; /** * Revoke Permissions implementation to be used in JsonRpcEngine middleware. @@ -36,7 +50,7 @@ export const revokePermissionsHandler = { function revokePermissionsImplementation( req: JsonRpcRequest, res: PendingJsonRpcResponse, - _next: AsyncJsonRpcEngineNextCallback, + _next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, { revokePermissionsForOrigin, diff --git a/packages/json-rpc-engine/CHANGELOG.md b/packages/json-rpc-engine/CHANGELOG.md index e4b3e9433c4..6efb15160d5 100644 --- a/packages/json-rpc-engine/CHANGELOG.md +++ b/packages/json-rpc-engine/CHANGELOG.md @@ -10,8 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `createOriginMiddleware` utility to `v2` ([#8522](https://github.com/MetaMask/core/pull/8522)) -- Add `createMethodMiddleware` utility to `v2` ([#8506](https://github.com/MetaMask/core/pull/8506)) +- Add `createMethodMiddleware` utility to `v2` ([#8506](https://github.com/MetaMask/core/pull/8506), [#8583](https://github.com/MetaMask/core/pull/8583)) - This utility allows JSON-RPC method implementations to use both the hooks pattern and the messenger. +- Add legacy `createMethodMiddleware` ([#8583](https://github.com/MetaMask/core/pull/8583)) + - Consolidates bespoke `makeMethodMiddlewareMaker` implementations from the MetaMask extension and mobile clients. + - Handlers may now declare `actionNames` and receive a delegated messenger as the sixth argument to `implementation`, mirroring the v2 `createMethodMiddleware`. + - Deprecated in favor of the v2 `createMethodMiddleware`. ## [10.2.4] diff --git a/packages/json-rpc-engine/src/createMethodMiddleware.test.ts b/packages/json-rpc-engine/src/createMethodMiddleware.test.ts new file mode 100644 index 00000000000..9e5f60563a1 --- /dev/null +++ b/packages/json-rpc-engine/src/createMethodMiddleware.test.ts @@ -0,0 +1,360 @@ +import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; +import { + assertIsJsonRpcFailure, + assertIsJsonRpcSuccess, + Json, + JsonRpcParams, + JsonRpcRequest, +} from '@metamask/utils'; + +import { + MethodHandler, + MethodHandlerImplementation, + createMethodMiddleware, +} from './createMethodMiddleware'; +import { JsonRpcEngine, JsonRpcMiddleware } from './JsonRpcEngine'; + +type AllHooks = { + hook1: () => number; + hook2: () => number; +}; + +const getDefaultHooks = (): AllHooks => ({ + hook1: () => 42, + hook2: () => 99, +}); + +const makeHandler = >( + implementation: MethodHandlerImplementation, + hookNames: { [Name in keyof Hooks]: true }, + // eslint-disable-next-line @typescript-eslint/explicit-function-return-type +) => ({ implementation, hookNames }); + +const method1 = 'method1'; + +const baseRequest = { + jsonrpc: '2.0' as const, + id: 1, + method: method1, +}; + +describe('createMethodMiddleware', () => { + it('calls the handler for the matching method (uses hook1)', async () => { + const handler = makeHandler( + (_req, res, _next, end, hooks) => { + res.result = hooks.hook1(); + return end(); + }, + { hook1: true, hook2: true }, + ); + + const middleware = createMethodMiddleware({ + handlers: { method1: handler, method2: handler }, + hooks: getDefaultHooks(), + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle(baseRequest); + assertIsJsonRpcSuccess(response); + + expect(response.result).toBe(42); + }); + + it('calls the handler for the matching method (uses hook2)', async () => { + const handler = makeHandler( + (_req, res, _next, end, hooks) => { + res.result = hooks.hook2(); + return end(); + }, + { hook1: true, hook2: true }, + ); + + const middleware = createMethodMiddleware({ + handlers: { method1: handler, method2: handler }, + hooks: getDefaultHooks(), + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle(baseRequest); + assertIsJsonRpcSuccess(response); + + expect(response.result).toBe(99); + }); + + it('does not call the handler for a non-matching method', async () => { + const handler = makeHandler( + (_req, res, _next, end) => { + res.result = 'unreachable'; + return end(); + }, + { hook1: true, hook2: true }, + ); + + const middleware = createMethodMiddleware({ + handlers: { method1: handler, method2: handler }, + hooks: getDefaultHooks(), + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + ...baseRequest, + method: 'nonMatchingMethod', + }); + assertIsJsonRpcFailure(response); + + expect(response.error).toMatchObject({ + message: expect.stringMatching( + /Response has no error or result for request/u, + ), + }); + }); + + it('handles errors returned by the implementation', async () => { + const handler = makeHandler( + (_req, _res, _next, end) => end(new Error('test error')), + { hook1: true, hook2: true }, + ); + + const middleware = createMethodMiddleware({ + handlers: { method1: handler, method2: handler }, + hooks: getDefaultHooks(), + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle(baseRequest); + assertIsJsonRpcFailure(response); + + expect(response.error.message).toBe('test error'); + expect( + (response.error.data as { cause: { message: string } }).cause.message, + ).toBe('test error'); + }); + + it('handles errors thrown by the implementation', async () => { + const handler = makeHandler( + () => { + throw new Error('test error'); + }, + { hook1: true, hook2: true }, + ); + + const middleware = createMethodMiddleware({ + handlers: { method1: handler, method2: handler }, + hooks: getDefaultHooks(), + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle(baseRequest); + assertIsJsonRpcFailure(response); + + expect(response.error.message).toBe('test error'); + expect( + (response.error.data as { cause: { message: string } }).cause.message, + ).toBe('test error'); + }); + + it('handles non-errors thrown by the implementation', async () => { + const handler = makeHandler( + () => { + // eslint-disable-next-line @typescript-eslint/only-throw-error + throw 'foo'; + }, + { hook1: true, hook2: true }, + ); + + const middleware = createMethodMiddleware({ + handlers: { method1: handler, method2: handler }, + hooks: getDefaultHooks(), + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle(baseRequest); + assertIsJsonRpcFailure(response); + + expect(response.error).toMatchObject({ + message: 'Internal JSON-RPC error.', + data: 'foo', + }); + }); + + it('invokes onError when a handler throws', async () => { + const onError = jest.fn(); + const handler = makeHandler( + () => { + throw new Error('test error'); + }, + { hook1: true, hook2: true }, + ); + + const middleware = createMethodMiddleware({ + handlers: { method1: handler, method2: handler }, + hooks: getDefaultHooks(), + onError, + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + await engine.handle(baseRequest); + + expect(onError).toHaveBeenCalledTimes(1); + const [error, receivedRequest] = onError.mock.calls[0]; + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe('test error'); + expect(receivedRequest).toMatchObject(baseRequest); + }); + + it('works when no hooks are configured', async () => { + const noDepsHandler = { + implementation: ((_req, res, _next, end) => { + res.result = 'no-deps'; + return end(); + }) as JsonRpcMiddleware, + }; + + const middleware = createMethodMiddleware({ + handlers: { noDeps: noDepsHandler }, + hooks: {}, + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ ...baseRequest, method: 'noDeps' }); + assertIsJsonRpcSuccess(response); + + expect(response.result).toBe('no-deps'); + }); + + it('allows typing non-standard request fields via RequestExtras', async () => { + const originHandler = { + implementation: (req, res, _next, end): void => { + res.result = req.origin ?? 'missing'; + return end(); + }, + } satisfies MethodHandler< + never, + never, + JsonRpcParams, + Json, + { origin: string } + >; + + const middleware = createMethodMiddleware({ + handlers: { reportOrigin: originHandler }, + hooks: {}, + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + ...baseRequest, + method: 'reportOrigin', + origin: 'https://example.com', + } as JsonRpcRequest & { origin: string }); + assertIsJsonRpcSuccess(response); + + expect(response.result).toBe('https://example.com'); + }); + + it('throws if handler actionNames are configured without a messenger', () => { + const actionHandler = { + implementation: (() => undefined) as JsonRpcMiddleware< + JsonRpcRequest, + Json + >, + actionNames: ['Example:TestAction'] as const, + }; + + expect(() => + createMethodMiddleware({ + handlers: { callAction: actionHandler }, + hooks: {}, + }), + ).toThrow('A messenger is required when a handler declares actionNames.'); + }); + + it('passes a delegated messenger to the handler', async () => { + type TestAction = { + type: 'Example:TestAction'; + handler: () => Promise; + }; + + const messengerHandler = { + implementation: async ( + _req, + res, + _next, + end, + _hooks, + messenger, + ): Promise => { + res.result = await messenger.call('Example:TestAction'); + return end(); + }, + actionNames: ['Example:TestAction'] as const, + } satisfies MethodHandler; + + const rootMessenger = new Messenger({ + namespace: MOCK_ANY_NAMESPACE, + }); + rootMessenger.registerActionHandler( + 'Example:TestAction', + async () => 'action-result', + ); + + const middleware = createMethodMiddleware({ + handlers: { callAction: messengerHandler }, + messenger: rootMessenger, + hooks: {}, + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + ...baseRequest, + method: 'callAction', + }); + assertIsJsonRpcSuccess(response); + + expect(response.result).toBe('action-result'); + }); + + it('throws an error if a required hook is missing', () => { + const handler = makeHandler((_req, _res, _next, end) => end(), { + hook1: true, + hook2: true, + }); + const hooks = { hook1: (): number => 42 }; + + expect(() => + createMethodMiddleware({ + handlers: { method1: handler, method2: handler }, + // @ts-expect-error Intentionally missing a required hook. + hooks, + }), + ).toThrow('Missing expected hooks'); + }); + + it('throws an error if an extraneous hook is provided', () => { + const handler = makeHandler((_req, _res, _next, end) => end(), { + hook1: true, + hook2: true, + }); + const hooks = { + ...getDefaultHooks(), + extraneousHook: (): number => 100, + }; + + expect(() => + createMethodMiddleware({ + handlers: { method1: handler, method2: handler }, + hooks, + }), + ).toThrow('Received unexpected hooks'); + }); +}); diff --git a/packages/json-rpc-engine/src/createMethodMiddleware.ts b/packages/json-rpc-engine/src/createMethodMiddleware.ts new file mode 100644 index 00000000000..5425fcec67d --- /dev/null +++ b/packages/json-rpc-engine/src/createMethodMiddleware.ts @@ -0,0 +1,225 @@ +import type { ActionConstraint } from '@metamask/messenger'; +import type { Messenger } from '@metamask/messenger'; +import { rpcErrors } from '@metamask/rpc-errors'; +import type { + Json, + JsonRpcParams, + JsonRpcRequest, + PendingJsonRpcResponse, +} from '@metamask/utils'; + +import type { + JsonRpcEngineEndCallback, + JsonRpcEngineNextCallback, + JsonRpcMiddleware, +} from './JsonRpcEngine'; +import { + assertExpectedHooks, + createHandlerMessenger, + selectHooks, + UnionToIntersection, +} from './v2/utils'; + +type HandlerActions = Handler extends { + implementation: (...args: infer Args) => unknown; +} + ? Args extends [ + unknown, + unknown, + unknown, + unknown, + unknown, + infer HandlerMessenger, + ] + ? HandlerMessenger extends Messenger + ? Actions + : never + : never + : never; + +type HandlerHooks = Handler extends { + implementation: (...args: infer Args) => unknown; +} + ? Args extends [ + unknown, + unknown, + unknown, + unknown, + infer ArgHooks, + ...unknown[], + ] + ? ArgHooks extends Record + ? ArgHooks + : never + : never + : never; + +/** + * A {@link MethodHandler} implementation. + * + * @deprecated Use the v2 `createMethodMiddleware` instead. + */ +export type MethodHandlerImplementation< + Hooks extends Record = never, + MessengerActions extends ActionConstraint = never, + Params extends JsonRpcParams = JsonRpcParams, + Result extends Json = Json, + RequestExtras extends Record = Record, +> = ( + req: JsonRpcRequest & RequestExtras, + res: PendingJsonRpcResponse, + next: JsonRpcEngineNextCallback, + end: JsonRpcEngineEndCallback, + hooks: Hooks, + messenger: Messenger, +) => Promise | void; + +/** + * A handler for {@link createMethodMiddleware}. + * + * @deprecated Use the v2 `createMethodMiddleware` instead. + */ +export type MethodHandler< + Hooks extends Record = never, + MessengerActions extends ActionConstraint = never, + Params extends JsonRpcParams = JsonRpcParams, + Result extends Json = Json, + RequestExtras extends Record = Record, +> = { + implementation: MethodHandlerImplementation< + Hooks, + MessengerActions, + Params, + Result, + RequestExtras + >; +} & ([Hooks] extends [never] + ? { hookNames?: undefined } + : { hookNames: { [Key in keyof Hooks]: true } }) & + ([MessengerActions] extends [never] + ? { actionNames?: undefined } + : { actionNames: readonly MessengerActions['type'][] }); + +type AnyMethodHandler = { + implementation( + this: void, + req: JsonRpcRequest, + res: PendingJsonRpcResponse, + next: JsonRpcEngineNextCallback, + end: JsonRpcEngineEndCallback, + hooks: unknown, + messenger: unknown, + ): Promise | void; + hookNames?: Record; + actionNames?: readonly string[]; +}; + +type CreateMethodMiddlewareBaseOptions< + Handlers extends Record, +> = { + handlers: Handlers; + // Due to a quirk of TypeScript's inference over generics, the hooks property must + // be present even if no hooks are needed. Otherwise, TypeScript will fail to infer + // the correct type for the messenger property. `Record` is the + // (hopefully) least confusing way to satisfy this requirement. + hooks: [HandlerHooks] extends [never] + ? Record + : UnionToIntersection>; + /** + * Called when a handler throws, before the error is forwarded to `end`. + * Intended for logging; must not throw. + */ + onError?: (error: unknown, request: JsonRpcRequest) => void; +}; + +/** + * Options for {@link createMethodMiddleware}. + * + * @deprecated Use the v2 `createMethodMiddleware` instead. + */ +export type CreateMethodMiddlewareOptions< + Handlers extends Record, +> = CreateMethodMiddlewareBaseOptions & + ([HandlerActions] extends [never] + ? { + messenger?: undefined; + } + : { + messenger: Messenger>; + }); + +type ResolvedHandler = { + implementation: AnyMethodHandler['implementation']; + hooks: Record; + messenger?: Messenger | undefined; +}; + +/** + * Create a JSON-RPC middleware that handles the passed JSON-RPC method handlers using the messenger and hooks. + * + * @deprecated Use the v2 `createMethodMiddleware` instead. + * @param options The options. + * @param options.handlers - The JSON-RPC method handler implementations. + * @param options.messenger - The messenger to be used by the handlers. + * @param options.hooks - The hooks to be used by the handlers. + * @returns A JsonRpcEngineV2 middleware. + */ +export function createMethodMiddleware< + Handlers extends Record, +>( + options: CreateMethodMiddlewareOptions, +): JsonRpcMiddleware { + const { messenger: rootMessenger, onError } = options; + const allHooks = options.hooks as Record; + + const expectedHookNames = new Set( + Object.values(options.handlers).flatMap((handler) => + handler.hookNames ? Object.getOwnPropertyNames(handler.hookNames) : [], + ), + ); + assertExpectedHooks(allHooks, expectedHookNames); + + const handlers = Object.entries(options.handlers).reduce< + Record + >((accumulator, [handlerName, handler]) => { + const handlerHooks = selectHooks(allHooks, handler.hookNames) ?? {}; + const handlerMessenger = createHandlerMessenger< + HandlerActions + >({ + namespace: handlerName, + actionNames: handler.actionNames as + | readonly HandlerActions['type'][] + | undefined, + rootMessenger, + }); + + accumulator[handlerName] = { + implementation: handler.implementation, + hooks: handlerHooks, + messenger: handlerMessenger, + }; + return accumulator; + }, {}); + + // This should technically use createAsyncMiddleware, but we get around this by catching + // all handler errors. + // eslint-disable-next-line @typescript-eslint/no-misused-promises + return async (req, res, next, end) => { + const handler = handlers[req.method]; + if (!handler) { + return next(); + } + + const { implementation, hooks: handlerHooks, messenger } = handler; + try { + return await implementation(req, res, next, end, handlerHooks, messenger); + } catch (error) { + onError?.(error, req); + return end( + error instanceof Error + ? error + : rpcErrors.internal({ data: error as Json }), + ); + } + }; +} diff --git a/packages/json-rpc-engine/src/index.test.ts b/packages/json-rpc-engine/src/index.test.ts index ac4e8008419..e2b811b55aa 100644 --- a/packages/json-rpc-engine/src/index.test.ts +++ b/packages/json-rpc-engine/src/index.test.ts @@ -6,6 +6,7 @@ describe('@metamask/json-rpc-engine', () => { [ "asV2Middleware", "createAsyncMiddleware", + "createMethodMiddleware", "createScaffoldMiddleware", "getUniqueId", "createIdRemapMiddleware", diff --git a/packages/json-rpc-engine/src/index.ts b/packages/json-rpc-engine/src/index.ts index 57e69b8d09b..746cd0c3628 100644 --- a/packages/json-rpc-engine/src/index.ts +++ b/packages/json-rpc-engine/src/index.ts @@ -4,6 +4,12 @@ export type { AsyncJsonrpcMiddleware, } from './createAsyncMiddleware'; export { createAsyncMiddleware } from './createAsyncMiddleware'; +export type { + CreateMethodMiddlewareOptions, + MethodHandler, + MethodHandlerImplementation, +} from './createMethodMiddleware'; +export { createMethodMiddleware } from './createMethodMiddleware'; export { createScaffoldMiddleware } from './createScaffoldMiddleware'; export { getUniqueId } from './getUniqueId'; export { createIdRemapMiddleware } from './idRemapMiddleware'; diff --git a/packages/json-rpc-engine/src/v2/createMethodMiddleware.test.ts b/packages/json-rpc-engine/src/v2/createMethodMiddleware.test.ts index 168744a3fec..7b11bc47d88 100644 --- a/packages/json-rpc-engine/src/v2/createMethodMiddleware.test.ts +++ b/packages/json-rpc-engine/src/v2/createMethodMiddleware.test.ts @@ -6,13 +6,14 @@ import { MethodHandler, } from './createMethodMiddleware'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; +import { JsonRpcRequest } from './utils'; type TestAction = { type: 'Example:TestAction'; handler: () => Promise; }; -function setup(): { engine: JsonRpcEngineV2 } { +function setup(): { engine: JsonRpcEngineV2 } { const getValueA = { hookNames: { testHook: true }, implementation: ({ hooks }): Promise => hooks.testHook(), @@ -41,9 +42,25 @@ function setup(): { engine: JsonRpcEngineV2 } { return { engine }; } +function setupWithoutMessenger(): { engine: JsonRpcEngineV2 } { + const getValueA = { + hookNames: { testHook: true }, + implementation: ({ hooks }): Promise => hooks.testHook(), + } satisfies MethodHandler<{ testHook: () => Promise }>; + + const middleware = createMethodMiddleware({ + handlers: { getValueA }, + hooks: { testHook: async () => 'A' }, + }); + + const engine = JsonRpcEngineV2.create({ middleware: [middleware] }); + + return { engine }; +} + describe('createMethodMiddleware', () => { - it('passes in the requested hooks', async () => { - const { engine } = setup(); + it('passes in the requested hooks without a messenger', async () => { + const { engine } = setupWithoutMessenger(); const result = await engine.handle(makeRequest({ method: 'getValueA' })); expect(result).toBe('A'); @@ -63,4 +80,85 @@ describe('createMethodMiddleware', () => { engine.handle(makeRequest({ method: 'getValueC' })), ).rejects.toThrow('Nothing ended request'); }); + + it('handles a handler with no hooks or actions', async () => { + const noDeps = { + implementation: (): Promise => Promise.resolve('ok'), + } satisfies MethodHandler; + + const middleware = createMethodMiddleware({ + handlers: { noDeps }, + hooks: {}, + }); + const engine = JsonRpcEngineV2.create({ middleware: [middleware] }); + + const result = await engine.handle(makeRequest({ method: 'noDeps' })); + expect(result).toBe('ok'); + }); + + it('propagates errors thrown by the implementation', async () => { + const failing = { + implementation: (): Promise => { + throw new Error('test error'); + }, + } satisfies MethodHandler; + + const middleware = createMethodMiddleware({ + handlers: { failing }, + hooks: {}, + }); + const engine = JsonRpcEngineV2.create({ middleware: [middleware] }); + + await expect( + engine.handle(makeRequest({ method: 'failing' })), + ).rejects.toThrow('test error'); + }); + + it('throws if handler actionNames are configured without a messenger', () => { + const getValueB = { + actionNames: ['Example:TestAction'], + implementation: (): Promise => Promise.resolve('B'), + } satisfies MethodHandler; + + expect(() => + createMethodMiddleware({ + handlers: { getValueB }, + hooks: {}, + }), + ).toThrow('A messenger is required when a handler declares actionNames.'); + }); + + it('throws if a required hook is missing', () => { + const getValueA = { + hookNames: { testHook: true }, + implementation: ({ hooks }): Promise => hooks.testHook(), + } satisfies MethodHandler<{ testHook: () => Promise }>; + + expect(() => + createMethodMiddleware({ + handlers: { getValueA }, + // @ts-expect-error Intentionally missing a required hook. + hooks: {}, + }), + ).toThrow('Missing expected hooks'); + }); + + it('throws if an extraneous hook is provided', () => { + const getValueA = { + hookNames: { testHook: true }, + implementation: ({ hooks }): Promise => hooks.testHook(), + } satisfies MethodHandler<{ testHook: () => Promise }>; + + const hooks = { + testHook: async (): Promise => 'A', + extraneousHook: (): number => 100, + }; + + expect(() => + createMethodMiddleware({ + handlers: { getValueA }, + hooks, + }), + ).toThrow('Received unexpected hooks'); + }); }); diff --git a/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts b/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts index dfd87035a1d..d1310b9c1bd 100644 --- a/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts +++ b/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts @@ -3,13 +3,15 @@ import { ActionConstraint, Messenger } from '@metamask/messenger'; import { JsonRpcMiddleware, Next } from './JsonRpcEngineV2'; import { ContextConstraint } from './MiddlewareContext'; import { + assertExpectedHooks, + createHandlerMessenger, + selectHooks, Json, JsonRpcParams, JsonRpcRequest, UnionToIntersection, } from './utils'; -// The helpers below seem excessive, but they are required for inference of hooks/actions. type HandlerActions = Handler extends { implementation: (options: infer Options) => unknown; } @@ -26,6 +28,9 @@ type HandlerHooks = Handler extends { : never : never; +/** + * A `JsonRpcEngineV2` method middleware handler. + */ export type MethodHandler< Hooks extends Record = never, MessengerActions extends ActionConstraint = never, @@ -62,18 +67,33 @@ type AnyMethodHandler = { actionNames?: readonly string[]; }; -export type CreateMethodMiddlewareOptions< +type CreateMethodMiddlewareBaseOptions< Handlers extends Record, > = { handlers: Handlers; - messenger: Messenger>; - hooks: UnionToIntersection>; + hooks: [HandlerHooks] extends [never] + ? Record + : UnionToIntersection>; }; +/** + * Options for {@link createMethodMiddleware}. + */ +export type CreateMethodMiddlewareOptions< + Handlers extends Record, +> = CreateMethodMiddlewareBaseOptions & + ([HandlerActions] extends [never] + ? { + messenger?: undefined; + } + : { + messenger: Messenger>; + }); + type ResolvedHandler = { implementation: AnyMethodHandler['implementation']; hooks: Record; - messenger: Messenger; + messenger?: Messenger | undefined; }; /** @@ -94,25 +114,25 @@ export function createMethodMiddleware< const { messenger: rootMessenger } = options; const allHooks = options.hooks as Record; + const expectedHookNames = new Set( + Object.values(options.handlers).flatMap((handler) => + handler.hookNames ? Object.getOwnPropertyNames(handler.hookNames) : [], + ), + ); + assertExpectedHooks(allHooks, expectedHookNames); + const handlers = Object.entries(options.handlers).reduce< Record >((accumulator, [handlerName, handler]) => { const handlerHooks = selectHooks(allHooks, handler.hookNames) ?? {}; - const handlerMessenger = new Messenger< - string, - HandlerActions, - never, - typeof rootMessenger + const handlerMessenger = createHandlerMessenger< + HandlerActions >({ namespace: handlerName, - parent: rootMessenger, - }); - - rootMessenger.delegate({ - actions: (handler.actionNames ?? []) as HandlerActions< - Handlers[keyof Handlers] - >['type'][], - messenger: handlerMessenger, + actionNames: handler.actionNames as + | readonly HandlerActions['type'][] + | undefined, + rootMessenger, }); accumulator[handlerName] = { @@ -134,35 +154,3 @@ export function createMethodMiddleware< return implementation({ request, context, next, hooks, messenger }); }; } - -/** - * Returns the subset of the specified `hooks` that are included in the - * `hookNames` object. This is a Principle of Least Authority (POLA) measure - * to ensure that each RPC method implementation only has access to the - * API "hooks" it needs to do its job. - * - * @param hooks - The hooks to select from. - * @param hookNames - The names of the hooks to select. - * @returns The selected hooks. - * @template Hooks - The hooks to select from. - * @template HookName - The names of the hooks to select. - */ -export function selectHooks< - Hooks extends Record, - HookName extends keyof Hooks, ->( - hooks: Hooks, - hookNames?: Record, -): Pick | undefined { - if (hookNames) { - return Object.keys(hookNames).reduce>>( - (hookSubset, _hookName) => { - const hookName = _hookName as HookName; - hookSubset[hookName] = hooks[hookName]; - return hookSubset; - }, - {}, - ) as Pick; - } - return undefined; -} diff --git a/packages/json-rpc-engine/src/v2/index.ts b/packages/json-rpc-engine/src/v2/index.ts index 9f0906a4139..ba83b5158e8 100644 --- a/packages/json-rpc-engine/src/v2/index.ts +++ b/packages/json-rpc-engine/src/v2/index.ts @@ -1,6 +1,6 @@ export { asLegacyMiddleware } from './asLegacyMiddleware'; export { getUniqueId } from '../getUniqueId'; -export { selectHooks, createMethodMiddleware } from './createMethodMiddleware'; +export { createMethodMiddleware } from './createMethodMiddleware'; export type { MethodHandler } from './createMethodMiddleware'; export { createOriginMiddleware } from './createOriginMiddleware'; export { createScaffoldMiddleware } from './createScaffoldMiddleware'; @@ -18,11 +18,17 @@ export type { export { JsonRpcServer } from './JsonRpcServer'; export { MiddlewareContext } from './MiddlewareContext'; export type { EmptyContext, ContextConstraint } from './MiddlewareContext'; -export { isNotification, isRequest, JsonRpcEngineError } from './utils'; +export { + isNotification, + isRequest, + JsonRpcEngineError, + selectHooks, +} from './utils'; export type { Json, JsonRpcCall, JsonRpcNotification, JsonRpcParams, JsonRpcRequest, + UnionToIntersection, } from './utils'; diff --git a/packages/json-rpc-engine/src/v2/utils.ts b/packages/json-rpc-engine/src/v2/utils.ts index 75aeb3c073d..5cc38239040 100644 --- a/packages/json-rpc-engine/src/v2/utils.ts +++ b/packages/json-rpc-engine/src/v2/utils.ts @@ -1,3 +1,5 @@ +import type { ActionConstraint } from '@metamask/messenger'; +import { Messenger } from '@metamask/messenger'; import { hasProperty, isObject } from '@metamask/utils'; import type { JsonRpcNotification, @@ -87,3 +89,110 @@ export class JsonRpcEngineError extends Error { return isInstance(value, JsonRpcEngineErrorSymbol); } } + +// Method middleware utils + +/** + * Returns the subset of the specified `hooks` that are included in the + * `hookNames` object. This is a Principle of Least Authority (POLA) measure + * to ensure that each RPC method implementation only has access to the + * API "hooks" it needs to do its job. + * + * @param hooks - The hooks to select from. + * @param hookNames - The names of the hooks to select. + * @returns The selected hooks, or `undefined` if `hookNames` is not provided. + * @template Hooks - The hooks to select from. + * @template HookName - The names of the hooks to select. + */ +export function selectHooks( + hooks: Hooks, + hookNames?: Record, +): Pick | undefined { + if (hookNames) { + return Object.keys(hookNames).reduce>>( + (subset, name) => { + const hookName = name as HookName; + subset[hookName] = hooks[hookName]; + return subset; + }, + {}, + ) as Pick; + } + return undefined; +} + +/** + * Asserts that `hooks` contains exactly the hook names in `expectedHookNames`. + * Throws on any missing hooks, then on any extraneous hooks. + * + * @param hooks - The hooks object to validate. + * @param expectedHookNames - The expected hook names. + */ +export function assertExpectedHooks( + hooks: Record, + expectedHookNames: Set, +): void { + const missingHookNames = Array.from(expectedHookNames).filter( + (hookName) => !hasProperty(hooks, hookName), + ); + if (missingHookNames.length > 0) { + throw new Error( + `Missing expected hooks:\n\n${missingHookNames.join('\n')}\n`, + ); + } + + const extraneousHookNames = Object.getOwnPropertyNames(hooks).filter( + (hookName) => !expectedHookNames.has(hookName), + ); + if (extraneousHookNames.length > 0) { + throw new Error( + `Received unexpected hooks:\n\n${extraneousHookNames.join('\n')}\n`, + ); + } +} + +/** + * Creates a per-handler messenger namespaced to `namespace`, and delegates the + * specified `actionNames` from `rootMessenger` to it. This lets each handler + * call only the actions it declared, per POLA. + * + * @param options - The options. + * @param options.namespace - The namespace for the handler messenger. + * @param options.actionNames - Actions to delegate from the root messenger. + * @param options.rootMessenger - The root messenger to delegate from. Required + * when `actionNames` are provided. + * @returns The per-handler messenger. + */ +export function createHandlerMessenger({ + namespace, + actionNames, + rootMessenger, +}: { + namespace: string; + actionNames: readonly Actions['type'][] | undefined; + rootMessenger?: Messenger | undefined; +}): Messenger | undefined { + if (!actionNames) { + return undefined; + } + + if (!rootMessenger) { + throw new Error( + 'A messenger is required when a handler declares actionNames.', + ); + } + + const handlerMessenger = new Messenger< + string, + Actions, + never, + typeof rootMessenger + >({ namespace, parent: rootMessenger }); + + rootMessenger.delegate({ + actions: actionNames as Actions['type'][], + messenger: handlerMessenger, + }); + + return handlerMessenger; +} diff --git a/packages/multichain-api-middleware/CHANGELOG.md b/packages/multichain-api-middleware/CHANGELOG.md index 5c651f7f6af..9f9d3430997 100644 --- a/packages/multichain-api-middleware/CHANGELOG.md +++ b/packages/multichain-api-middleware/CHANGELOG.md @@ -7,14 +7,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `MethodHandlerHooks` type, the intersection of all method handler hook types ([#8583](https://github.com/MetaMask/core/pull/8583)) + - Consumers can use this to type the hooks object passed to `createMethodMiddleware` without restating each handler's hooks individually. + ### Changed +- **BREAKING:** Consolidate method handlers into a single `methodHandlers` export ([#8583](https://github.com/MetaMask/core/pull/8583)) + - The individual handler exports have been removed. They can still be accessed as properties on the `methodHandlers` export. + - The new handlers follow the format expected by `createMethodMiddleware` from `@metamask/json-rpc-engine@10.3.0`. + - The hook types have been updated to cohere with their corresponding MetaMask controller methods. +- **BREAKING:** Make `trackSessionCreatedEvent` hook required in `wallet_createSession` handler ([#8583](https://github.com/MetaMask/core/pull/8583)) + - If the hook is not required, `null` can be passed instead. - Bump `@metamask/json-rpc-engine` from `^10.2.3` to `^10.2.4` ([#8317](https://github.com/MetaMask/core/pull/8317)) - Bump `@metamask/network-controller` from `^30.0.0` to `^30.0.1` ([#8317](https://github.com/MetaMask/core/pull/8317)) - Bump `@metamask/permission-controller` from `^12.2.1` to `^12.3.0` ([#8317](https://github.com/MetaMask/core/pull/8317)) - Bump `@metamask/multichain-transactions-controller` from `^7.0.3` to `^7.0.4` ([#8325](https://github.com/MetaMask/core/pull/8325)) - Bump `@metamask/controller-utils` from `^11.19.0` to `^11.20.0` ([#8344](https://github.com/MetaMask/core/pull/8344)) +### Fixed + +- `wallet_invokeMethod` fails early with an `invalidParams` error when the `params` object is not an object ([#8583](https://github.com/MetaMask/core/pull/8583)) + - Previously it would fail with a less specific error. +- `wallet_revokeSession` now returns `true` when no active session exists and specific scopes are requested, consistent with its full-revoke behavior ([#8583](https://github.com/MetaMask/core/pull/8583)) + - Previously it would return an internal error. + ## [2.0.0] ### Added diff --git a/packages/multichain-api-middleware/package.json b/packages/multichain-api-middleware/package.json index 8bcf9794487..0a7b74eb7f3 100644 --- a/packages/multichain-api-middleware/package.json +++ b/packages/multichain-api-middleware/package.json @@ -51,6 +51,7 @@ "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" }, "dependencies": { + "@metamask/accounts-controller": "^37.2.0", "@metamask/api-specs": "^0.14.0", "@metamask/chain-agnostic-permission": "^1.5.0", "@metamask/controller-utils": "^11.20.0", @@ -58,6 +59,7 @@ "@metamask/network-controller": "^30.0.1", "@metamask/permission-controller": "^12.3.0", "@metamask/rpc-errors": "^7.0.2", + "@metamask/snaps-controllers": "^19.0.0", "@metamask/utils": "^11.9.0", "@open-rpc/meta-schema": "^1.14.6", "@open-rpc/schema-utils-js": "^2.0.5", diff --git a/packages/multichain-api-middleware/src/handlers/index.test.ts b/packages/multichain-api-middleware/src/handlers/index.test.ts new file mode 100644 index 00000000000..e51ed606980 --- /dev/null +++ b/packages/multichain-api-middleware/src/handlers/index.test.ts @@ -0,0 +1,57 @@ +import { createMethodMiddleware } from '@metamask/json-rpc-engine'; + +import { methodHandlers } from '.'; +import type { WalletCreateSessionHooks } from './wallet-createSession'; +import type { WalletGetSessionHooks } from './wallet-getSession'; +import type { WalletInvokeMethodHooks } from './wallet-invokeMethod'; +import type { WalletRevokeSessionHooks } from './wallet-revokeSession'; + +type Hooks = WalletCreateSessionHooks & + WalletGetSessionHooks & + WalletInvokeMethodHooks & + WalletRevokeSessionHooks; + +/* eslint-disable @typescript-eslint/explicit-function-return-type */ +const makeMockHooks = () => + ({ + listAccounts: () => [ + { + type: 'eip155:eoa', + address: '0x123', + id: '1', + options: {}, + scopes: [], + methods: [], + metadata: { + name: 'Account 1', + importTime: Date.now(), + keyring: { type: 'HD Key Tree' }, + }, + }, + ], + findNetworkClientIdByChainId: (() => + '1') as Hooks['findNetworkClientIdByChainId'], + requestPermissionsForOrigin: () => + Promise.resolve([{}, { id: '1', origin: 'test' }]), + getNonEvmSupportedMethods: () => [], + isNonEvmScopeSupported: () => false, + getNonEvmAccountAddresses: () => [], + sortAccountIdsByLastSelected: () => [], + getCaveatForOrigin: (() => ({}) as unknown) as Hooks['getCaveatForOrigin'], + getSelectedNetworkClientId: () => 'mainnet', + handleNonEvmRequestForOrigin: () => Promise.resolve(null), + revokePermissionForOrigin: () => undefined, + updateCaveat: () => undefined, + trackSessionCreatedEvent: null, + }) satisfies Hooks; +/* eslint-enable @typescript-eslint/explicit-function-return-type */ + +describe('methodHandlers', () => { + it('constructs a method middleware from the handlers', () => { + const middleware = createMethodMiddleware({ + handlers: methodHandlers, + hooks: makeMockHooks(), + }); + expect(middleware).toBeDefined(); + }); +}); diff --git a/packages/multichain-api-middleware/src/handlers/index.ts b/packages/multichain-api-middleware/src/handlers/index.ts new file mode 100644 index 00000000000..ab8ad4ea991 --- /dev/null +++ b/packages/multichain-api-middleware/src/handlers/index.ts @@ -0,0 +1,38 @@ +import type { UnionToIntersection } from '@metamask/json-rpc-engine/v2'; + +import type { WalletCreateSessionHooks } from './wallet-createSession'; +import { walletCreateSessionHandler } from './wallet-createSession'; +import type { WalletGetSessionHooks } from './wallet-getSession'; +import { walletGetSessionHandler } from './wallet-getSession'; +import type { WalletInvokeMethodHooks } from './wallet-invokeMethod'; +import { walletInvokeMethodHandler } from './wallet-invokeMethod'; +import type { WalletRevokeSessionHooks } from './wallet-revokeSession'; +import { walletRevokeSessionHandler } from './wallet-revokeSession'; + +export type MethodHandlerHooks = UnionToIntersection< + | WalletCreateSessionHooks + | WalletGetSessionHooks + | WalletInvokeMethodHooks + | WalletRevokeSessionHooks +>; + +const MethodNames = { + WalletCreateSession: 'wallet_createSession', + WalletGetSession: 'wallet_getSession', + WalletInvokeMethod: 'wallet_invokeMethod', + WalletRevokeSession: 'wallet_revokeSession', +} as const; + +type MethodHandlers = { + [MethodNames.WalletCreateSession]: typeof walletCreateSessionHandler; + [MethodNames.WalletGetSession]: typeof walletGetSessionHandler; + [MethodNames.WalletInvokeMethod]: typeof walletInvokeMethodHandler; + [MethodNames.WalletRevokeSession]: typeof walletRevokeSessionHandler; +}; + +export const methodHandlers: Readonly = { + [MethodNames.WalletCreateSession]: walletCreateSessionHandler, + [MethodNames.WalletGetSession]: walletGetSessionHandler, + [MethodNames.WalletInvokeMethod]: walletInvokeMethodHandler, + [MethodNames.WalletRevokeSession]: walletRevokeSessionHandler, +}; diff --git a/packages/multichain-api-middleware/src/handlers/types.ts b/packages/multichain-api-middleware/src/handlers/types.ts index b5a4bec7c83..1e7114a1979 100644 --- a/packages/multichain-api-middleware/src/handlers/types.ts +++ b/packages/multichain-api-middleware/src/handlers/types.ts @@ -1,13 +1,13 @@ -import type { +import { Caip25CaveatType, Caip25CaveatValue, } from '@metamask/chain-agnostic-permission'; import type { + GenericPermissionController, Caveat, - CaveatSpecificationConstraint, - PermissionController, - PermissionSpecificationConstraint, } from '@metamask/permission-controller'; +import type { MultichainRoutingService } from '@metamask/snaps-controllers'; +import type { CaipAccountId } from '@metamask/utils'; /** * Multichain API notifications currently supported by/known to the wallet. @@ -16,24 +16,20 @@ export enum MultichainApiNotifications { sessionChanged = 'wallet_sessionChanged', walletNotify = 'wallet_notify', } -type AbstractPermissionController = PermissionController< - PermissionSpecificationConstraint, - CaveatSpecificationConstraint ->; -export type GrantedPermissions = Awaited< - ReturnType ->[0]; +export type Caip25Caveat = Caveat; -export type WalletRevokeSessionHooks = { - revokePermissionForOrigin: (permissionName: string) => void; - updateCaveat: ( - target: string, - caveatType: string, - caveatValue: Caip25CaveatValue, - ) => void; +export type GetCaveatForOriginHook = { getCaveatForOrigin: ( endowmentPermissionName: string, caveatType: string, - ) => Caveat; + ) => ReturnType; +}; + +export type GetNonEvmSupportedMethodsHook = { + getNonEvmSupportedMethods: MultichainRoutingService['getSupportedMethods']; +}; + +export type SortAccountIdsByLastSelectedHook = { + sortAccountIdsByLastSelected: (accounts: CaipAccountId[]) => CaipAccountId[]; }; diff --git a/packages/multichain-api-middleware/src/handlers/wallet-createSession.test.ts b/packages/multichain-api-middleware/src/handlers/wallet-createSession.test.ts index cc9711b0aa1..5ca1de503fd 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-createSession.test.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-createSession.test.ts @@ -18,7 +18,7 @@ import type { JsonRpcSuccess, } from '@metamask/utils'; -import { walletCreateSession } from './wallet-createSession'; +import { walletCreateSessionHandler } from './wallet-createSession'; jest.mock('@metamask/rpc-errors', () => ({ ...jest.requireActual('@metamask/rpc-errors'), @@ -63,7 +63,7 @@ const baseRequest = { }, }; -const createMockedHandler = () => { +const createMockedHandler = (trackSessionEvents: boolean = true) => { const next = jest.fn(); const end = jest.fn(); const requestPermissionsForOrigin = jest.fn().mockResolvedValue([ @@ -92,7 +92,9 @@ const createMockedHandler = () => { }, ]); const findNetworkClientIdByChainId = jest.fn().mockReturnValue('mainnet'); - const trackSessionCreatedEvent = jest.fn().mockImplementation(undefined); + const trackSessionCreatedEvent = trackSessionEvents + ? jest.fn().mockImplementation(undefined) + : null; const listAccounts = jest.fn().mockReturnValue([]); const getNonEvmSupportedMethods = jest.fn().mockReturnValue([]); const isNonEvmScopeSupported = jest.fn().mockReturnValue(false); @@ -108,7 +110,7 @@ const createMockedHandler = () => { const handler = ( request: JsonRpcRequest & { origin: string }, ) => - walletCreateSession.implementation(request, response, next, end, { + walletCreateSessionHandler.implementation(request, response, next, end, { findNetworkClientIdByChainId, requestPermissionsForOrigin, listAccounts, @@ -722,9 +724,17 @@ describe('wallet_createSession', () => { ); }); - it('calls trackSessionCreatedEvent hook if defined', async () => { + it('ignores trackSessionCreatedEvent hook if it is null', async () => { + const { handler, trackSessionCreatedEvent } = createMockedHandler(false); + await handler(baseRequest); + + expect(trackSessionCreatedEvent).toBeNull(); + }); + it('calls trackSessionCreatedEvent hook if not null', async () => { const { handler, trackSessionCreatedEvent } = createMockedHandler(); - trackSessionCreatedEvent.mockImplementation(() => { + expect(trackSessionCreatedEvent).not.toBeNull(); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + trackSessionCreatedEvent!.mockImplementation(() => { // mock implementation }); await handler(baseRequest); diff --git a/packages/multichain-api-middleware/src/handlers/wallet-createSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-createSession.ts index 5844d6cfdda..2041367bee6 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-createSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-createSession.ts @@ -1,3 +1,4 @@ +import type { AccountsController } from '@metamask/accounts-controller'; import { Caip25CaveatType, Caip25EndowmentPermissionName, @@ -19,13 +20,18 @@ import type { } from '@metamask/chain-agnostic-permission'; import { isEqualCaseInsensitive } from '@metamask/controller-utils'; import type { + MethodHandler, JsonRpcEngineEndCallback, JsonRpcEngineNextCallback, } from '@metamask/json-rpc-engine'; import type { NetworkController } from '@metamask/network-controller'; import { invalidParams } from '@metamask/permission-controller'; -import type { RequestedPermissions } from '@metamask/permission-controller'; +import type { + GenericPermissionController, + RequestedPermissions, +} from '@metamask/permission-controller'; import { JsonRpcError, rpcErrors } from '@metamask/rpc-errors'; +import type { MultichainRoutingService } from '@metamask/snaps-controllers'; import { isPlainObject, KnownCaipNamespace, @@ -33,17 +39,41 @@ import { } from '@metamask/utils'; import type { CaipAccountId, - CaipChainId, Hex, Json, JsonRpcRequest, - JsonRpcSuccess, + PendingJsonRpcResponse, } from '@metamask/utils'; -import type { GrantedPermissions } from './types'; +import type { + GetNonEvmSupportedMethodsHook, + SortAccountIdsByLastSelectedHook, +} from './types'; const SOLANA_CAIP_CHAIN_ID = 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'; +export type WalletCreateSessionHooks = GetNonEvmSupportedMethodsHook & + SortAccountIdsByLastSelectedHook & { + listAccounts: AccountsController['listAccounts']; + findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; + requestPermissionsForOrigin: ( + requestedPermissions: RequestedPermissions, + metadata?: Record, + ) => ReturnType; + isNonEvmScopeSupported: MultichainRoutingService['isSupportedScope']; + getNonEvmAccountAddresses: MultichainRoutingService['getSupportedAccounts']; + trackSessionCreatedEvent: + | ((approvedCaip25CaveatValue: Caip25CaveatValue) => void) + | null; + }; + +type Params = Caip25Authorization; + +type Result = { + sessionScopes: NormalizedScopesObject; + sessionProperties?: Record; +}; + /** * Handler for the `wallet_createSession` RPC method which is responsible * for prompting for approval and granting a CAIP-25 permission. @@ -67,35 +97,16 @@ const SOLANA_CAIP_CHAIN_ID = 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'; * @param hooks.isNonEvmScopeSupported - The hook that returns true if a non EVM scope is supported. * @param hooks.getNonEvmAccountAddresses - The hook that returns a list of CaipAccountIds that are supported for a CaipChainId. * @param hooks.sortAccountIdsByLastSelected - A function that accepts an array of CaipAccountId and returns an array of CaipAccountId sorted by last selected. - * @param hooks.trackSessionCreatedEvent - An optional hook for platform specific logic to run. Can be undefined. + * @param hooks.trackSessionCreatedEvent - An optional hook for platform specific logic to run. * @returns A promise with wallet_createSession handler */ -async function walletCreateSessionHandler( - req: JsonRpcRequest & { origin: string }, - res: JsonRpcSuccess<{ - sessionScopes: NormalizedScopesObject; - sessionProperties?: Record; - }>, +async function handleWalletCreateSession( + req: JsonRpcRequest & { origin: string }, + res: PendingJsonRpcResponse, _next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, - hooks: { - listAccounts: () => { address: string }[]; - findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; - requestPermissionsForOrigin: ( - requestedPermissions: RequestedPermissions, - metadata?: Record, - ) => Promise<[GrantedPermissions]>; - getNonEvmSupportedMethods: (scope: CaipChainId) => string[]; - isNonEvmScopeSupported: (scope: CaipChainId) => boolean; - getNonEvmAccountAddresses: (scope: CaipChainId) => CaipAccountId[]; - sortAccountIdsByLastSelected: ( - accounts: CaipAccountId[], - ) => CaipAccountId[]; - trackSessionCreatedEvent?: ( - approvedCaip25CaveatValue: Caip25CaveatValue, - ) => void; - }, -) { + hooks: WalletCreateSessionHooks, +): Promise { if (!isPlainObject(req.params)) { return end(invalidParams({ data: { request: req } })); } @@ -285,9 +296,16 @@ async function walletCreateSessionHandler( } } -export const walletCreateSession = { - methodNames: ['wallet_createSession'], - implementation: walletCreateSessionHandler, +export type WalletCreateSessionHandler = MethodHandler< + WalletCreateSessionHooks, + never, + Params, + Result, + { origin: string } +>; + +export const walletCreateSessionHandler = { + implementation: handleWalletCreateSession, hookNames: { findNetworkClientIdByChainId: true, listAccounts: true, @@ -298,4 +316,4 @@ export const walletCreateSession = { sortAccountIdsByLastSelected: true, trackSessionCreatedEvent: true, }, -}; +} satisfies WalletCreateSessionHandler; diff --git a/packages/multichain-api-middleware/src/handlers/wallet-getSession.test.ts b/packages/multichain-api-middleware/src/handlers/wallet-getSession.test.ts index 54709e7089e..d3805893d5a 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-getSession.test.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-getSession.test.ts @@ -1,7 +1,7 @@ import * as chainAgnosticPermissionModule from '@metamask/chain-agnostic-permission'; import type { JsonRpcRequest } from '@metamask/utils'; -import { walletGetSession } from './wallet-getSession'; +import { walletGetSessionHandler } from './wallet-getSession'; jest.mock('@metamask/chain-agnostic-permission', () => ({ ...jest.requireActual('@metamask/chain-agnostic-permission'), @@ -52,7 +52,7 @@ const createMockedHandler = () => { jsonrpc: '2.0' as const, }; const handler = (request: JsonRpcRequest & { origin: string }) => - walletGetSession.implementation(request, response, next, end, { + walletGetSessionHandler.implementation(request, response, next, end, { getCaveatForOrigin, getNonEvmSupportedMethods, sortAccountIdsByLastSelected, diff --git a/packages/multichain-api-middleware/src/handlers/wallet-getSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-getSession.ts index cd1b62bcd71..f093e832931 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-getSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-getSession.ts @@ -1,20 +1,33 @@ -import type { - Caip25CaveatValue, - NormalizedScopesObject, -} from '@metamask/chain-agnostic-permission'; +import type { NormalizedScopesObject } from '@metamask/chain-agnostic-permission'; import { Caip25CaveatType, Caip25EndowmentPermissionName, getSessionScopes, } from '@metamask/chain-agnostic-permission'; -import type { Caveat } from '@metamask/permission-controller'; -import type { CaipAccountId } from '@metamask/utils'; import type { - CaipChainId, + JsonRpcEngineEndCallback, + JsonRpcEngineNextCallback, + MethodHandler, +} from '@metamask/json-rpc-engine'; +import type { + JsonRpcParams, JsonRpcRequest, - JsonRpcSuccess, + PendingJsonRpcResponse, } from '@metamask/utils'; +import type { + Caip25Caveat, + GetCaveatForOriginHook, + GetNonEvmSupportedMethodsHook, + SortAccountIdsByLastSelectedHook, +} from './types'; + +type WalletGetSessionResult = { sessionScopes: NormalizedScopesObject }; + +export type WalletGetSessionHooks = GetCaveatForOriginHook & + GetNonEvmSupportedMethodsHook & + SortAccountIdsByLastSelectedHook; + /** * Handler for the `wallet_getSession` RPC method as specified by [CAIP-312](https://chainagnostic.org/CAIPs/caip-312). * The implementation below deviates from the linked spec in that it ignores the `sessionId` param entirely, @@ -31,28 +44,19 @@ import type { * @param hooks.sortAccountIdsByLastSelected - A function that accepts an array of CaipAccountId and returns an array of CaipAccountId sorted by corresponding last selected account in the wallet. * @returns Nothing. */ -async function walletGetSessionHandler( +async function handleWalletGetSession( _request: JsonRpcRequest & { origin: string }, - response: JsonRpcSuccess<{ sessionScopes: NormalizedScopesObject }>, - _next: () => void, - end: () => void, - hooks: { - getCaveatForOrigin: ( - endowmentPermissionName: string, - caveatType: string, - ) => Caveat; - getNonEvmSupportedMethods: (scope: CaipChainId) => string[]; - sortAccountIdsByLastSelected: ( - accounts: CaipAccountId[], - ) => CaipAccountId[]; - }, + response: PendingJsonRpcResponse, + _next: JsonRpcEngineNextCallback, + end: JsonRpcEngineEndCallback, + hooks: WalletGetSessionHooks, ) { - let caveat; + let caveat: Caip25Caveat | undefined; try { caveat = hooks.getCaveatForOrigin( Caip25EndowmentPermissionName, Caip25CaveatType, - ); + ) as Caip25Caveat | undefined; } catch { // noop } @@ -71,12 +75,19 @@ async function walletGetSessionHandler( return end(); } -export const walletGetSession = { - methodNames: ['wallet_getSession'], - implementation: walletGetSessionHandler, +export type WalletGetSessionHandler = MethodHandler< + WalletGetSessionHooks, + never, + JsonRpcParams, + WalletGetSessionResult, + { origin: string } +>; + +export const walletGetSessionHandler = { + implementation: handleWalletGetSession, hookNames: { getCaveatForOrigin: true, getNonEvmSupportedMethods: true, sortAccountIdsByLastSelected: true, }, -}; +} satisfies WalletGetSessionHandler; diff --git a/packages/multichain-api-middleware/src/handlers/wallet-invokeMethod.test.ts b/packages/multichain-api-middleware/src/handlers/wallet-invokeMethod.test.ts index 42e4fb84a91..ad4f83f9e78 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-invokeMethod.test.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-invokeMethod.test.ts @@ -2,7 +2,7 @@ import * as chainAgnosticPermissionModule from '@metamask/chain-agnostic-permiss import { providerErrors, rpcErrors } from '@metamask/rpc-errors'; import type { WalletInvokeMethodRequest } from './wallet-invokeMethod'; -import { walletInvokeMethod } from './wallet-invokeMethod'; +import { walletInvokeMethodHandler } from './wallet-invokeMethod'; // Allow individual modules to be mocked jest.mock('@metamask/chain-agnostic-permission', () => ({ @@ -62,7 +62,7 @@ const createMockedHandler = () => { const handleNonEvmRequestForOrigin = jest.fn().mockResolvedValue(null); const response = { jsonrpc: '2.0' as const, id: 1 }; const handler = (request: WalletInvokeMethodRequest) => - walletInvokeMethod.implementation(request, response, next, end, { + walletInvokeMethodHandler.implementation(request, response, next, end, { getCaveatForOrigin, findNetworkClientIdByChainId, getSelectedNetworkClientId, @@ -118,6 +118,22 @@ describe('wallet_invokeMethod', () => { }); }); + it('returns invalid params when params is not a plain object', async () => { + const { handler, end, getCaveatForOrigin, next } = createMockedHandler(); + const request = { + ...createMockedRequest(), + params: [ + 'not-a-plain-object', + ] as unknown as WalletInvokeMethodRequest['params'], + }; + await handler(request); + expect(end).toHaveBeenCalledWith( + rpcErrors.invalidParams({ data: { request } }), + ); + expect(getCaveatForOrigin).not.toHaveBeenCalled(); + expect(next).not.toHaveBeenCalled(); + }); + it('gets the authorized scopes from the CAIP-25 endowment permission', async () => { const request = createMockedRequest(); const { handler, getCaveatForOrigin } = createMockedHandler(); diff --git a/packages/multichain-api-middleware/src/handlers/wallet-invokeMethod.ts b/packages/multichain-api-middleware/src/handlers/wallet-invokeMethod.ts index c485109aba1..1305d43ae87 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-invokeMethod.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-invokeMethod.ts @@ -1,7 +1,4 @@ -import type { - Caip25CaveatValue, - ExternalScopeString, -} from '@metamask/chain-agnostic-permission'; +import type { ExternalScopeString } from '@metamask/chain-agnostic-permission'; import { Caip25CaveatType, Caip25EndowmentPermissionName, @@ -9,27 +6,55 @@ import { getSessionScopes, parseScopeString, } from '@metamask/chain-agnostic-permission'; -import type { NetworkClientId } from '@metamask/network-controller'; -import type { Caveat } from '@metamask/permission-controller'; +import type { + JsonRpcEngineEndCallback, + JsonRpcEngineNextCallback, + MethodHandler, +} from '@metamask/json-rpc-engine'; +import type { + NetworkClientId, + NetworkController, +} from '@metamask/network-controller'; import { providerErrors, rpcErrors } from '@metamask/rpc-errors'; +import type { MultichainRoutingService } from '@metamask/snaps-controllers'; +import { isObject, KnownCaipNamespace, numberToHex } from '@metamask/utils'; import type { CaipAccountId, CaipChainId, - Hex, Json, JsonRpcRequest, PendingJsonRpcResponse, } from '@metamask/utils'; -import { KnownCaipNamespace, numberToHex } from '@metamask/utils'; -export type WalletInvokeMethodRequest = JsonRpcRequest & { - origin: string; - params: { - scope: ExternalScopeString; - request: Pick; - }; +import type { + Caip25Caveat, + GetCaveatForOriginHook, + GetNonEvmSupportedMethodsHook, + SortAccountIdsByLastSelectedHook, +} from './types'; + +export type WalletInvokeMethodParams = { + scope: ExternalScopeString; + request: Pick; }; +export type WalletInvokeMethodRequest = + JsonRpcRequest & { + origin: string; + }; + +export type WalletInvokeMethodHooks = GetCaveatForOriginHook & + GetNonEvmSupportedMethodsHook & + SortAccountIdsByLastSelectedHook & { + findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; + getSelectedNetworkClientId: () => NetworkClientId; + handleNonEvmRequestForOrigin: (params: { + connectedAddresses: CaipAccountId[]; + scope: CaipChainId; + request: JsonRpcRequest; + }) => ReturnType; + }; + /** * Handler for the `wallet_invokeMethod` RPC method as specified by [CAIP-27](https://chainagnostic.org/CAIPs/caip-27). * The implementation below deviates from the linked spec in that it ignores the `sessionId` param @@ -48,39 +73,26 @@ export type WalletInvokeMethodRequest = JsonRpcRequest & { * @param hooks.handleNonEvmRequestForOrigin - A function that sends a request to the MultichainRouter for processing. * @returns Nothing. */ -async function walletInvokeMethodHandler( +async function handleWalletInvokeMethod( request: WalletInvokeMethodRequest, - response: PendingJsonRpcResponse, - next: () => void, - end: (error?: Error) => void, - hooks: { - getCaveatForOrigin: ( - endowmentPermissionName: string, - caveatType: string, - ) => Caveat; - findNetworkClientIdByChainId: (chainId: Hex) => NetworkClientId | undefined; - getSelectedNetworkClientId: () => NetworkClientId; - getNonEvmSupportedMethods: (scope: CaipChainId) => string[]; - sortAccountIdsByLastSelected: ( - accounts: CaipAccountId[], - ) => CaipAccountId[]; - handleNonEvmRequestForOrigin: (params: { - connectedAddresses: CaipAccountId[]; - scope: CaipChainId; - request: JsonRpcRequest; - }) => Promise; - }, + response: PendingJsonRpcResponse, + next: JsonRpcEngineNextCallback, + end: JsonRpcEngineEndCallback, + hooks: WalletInvokeMethodHooks, ) { - const { scope, request: wrappedRequest } = request.params; + if (!isObject(request.params)) { + return end(rpcErrors.invalidParams({ data: { request } })); + } + const { scope, request: wrappedRequest } = request.params; assertIsInternalScopeString(scope); - let caveat; + let caveat: Caip25Caveat | undefined; try { caveat = hooks.getCaveatForOrigin( Caip25EndowmentPermissionName, Caip25CaveatType, - ); + ) as Caip25Caveat | undefined; } catch { // noop } @@ -151,9 +163,17 @@ async function walletInvokeMethodHandler( } return end(); } -export const walletInvokeMethod = { - methodNames: ['wallet_invokeMethod'], - implementation: walletInvokeMethodHandler, + +export type WalletInvokeMethodHandler = MethodHandler< + WalletInvokeMethodHooks, + never, + WalletInvokeMethodParams, + Json, + { origin: string } +>; + +export const walletInvokeMethodHandler = { + implementation: handleWalletInvokeMethod, hookNames: { getCaveatForOrigin: true, findNetworkClientIdByChainId: true, @@ -162,4 +182,4 @@ export const walletInvokeMethod = { sortAccountIdsByLastSelected: true, handleNonEvmRequestForOrigin: true, }, -}; +} satisfies WalletInvokeMethodHandler; diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts index 26ca5e7e16d..9c14f205366 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts @@ -9,7 +9,7 @@ import { import { rpcErrors } from '@metamask/rpc-errors'; import type { JsonRpcRequest } from '@metamask/utils'; -import { walletRevokeSession } from './wallet-revokeSession'; +import { walletRevokeSessionHandler } from './wallet-revokeSession'; const baseRequest: JsonRpcRequest & { origin: string; @@ -39,7 +39,7 @@ const createMockedHandler = () => { params: { scopes?: string[] }; }, ) => - walletRevokeSession.implementation(request, response, next, end, { + walletRevokeSessionHandler.implementation(request, response, next, end, { revokePermissionForOrigin, updateCaveat, getCaveatForOrigin, @@ -86,6 +86,23 @@ describe('wallet_revokeSession', () => { expect(response.result).toBe(true); }); + it('returns true without revoking if there is no active session and scopes are specified', async () => { + const { + handler, + getCaveatForOrigin, + revokePermissionForOrigin, + updateCaveat, + response, + } = createMockedHandler(); + getCaveatForOrigin.mockReturnValue(undefined); + + await handler({ ...baseRequest, params: { scopes: ['eip155:1'] } }); + + expect(revokePermissionForOrigin).not.toHaveBeenCalled(); + expect(updateCaveat).not.toHaveBeenCalled(); + expect(response.result).toBe(true); + }); + it('partially revokes the CAIP-25 endowment permission if `scopes` param is passed in', async () => { const { handler, getCaveatForOrigin, updateCaveat } = createMockedHandler(); getCaveatForOrigin.mockImplementation(() => ({ diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts index cf1a4752c28..ce22f546a68 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts @@ -4,20 +4,40 @@ import { Caip25EndowmentPermissionName, getCaipAccountIdsFromCaip25CaveatValue, } from '@metamask/chain-agnostic-permission'; +import type { Caip25CaveatValue } from '@metamask/chain-agnostic-permission'; import type { - JsonRpcEngineNextCallback, JsonRpcEngineEndCallback, + JsonRpcEngineNextCallback, + MethodHandler, } from '@metamask/json-rpc-engine'; import { CaveatMutatorOperation, + GenericPermissionController, PermissionDoesNotExistError, UnrecognizedSubjectError, } from '@metamask/permission-controller'; import { rpcErrors } from '@metamask/rpc-errors'; import { isObject } from '@metamask/utils'; -import type { JsonRpcSuccess, JsonRpcRequest } from '@metamask/utils'; +import type { + Json, + JsonRpcRequest, + PendingJsonRpcResponse, +} from '@metamask/utils'; + +import type { Caip25Caveat, GetCaveatForOriginHook } from './types'; + +export type WalletRevokeSessionHooks = GetCaveatForOriginHook & { + revokePermissionForOrigin: ( + permissionName: string, + ) => ReturnType; + updateCaveat: ( + target: string, + caveatType: string, + caveatValue: Caip25CaveatValue, + ) => ReturnType; +}; -import type { WalletRevokeSessionHooks } from './types'; +type WalletRevokeSessionParams = { scopes?: string[] }; /** * Check whether the given error is a permission error. @@ -54,10 +74,15 @@ function partialRevokePermissions( scopes: string[], hooks: WalletRevokeSessionHooks, ) { - let updatedCaveatValue = hooks.getCaveatForOrigin( + const caveat = hooks.getCaveatForOrigin( Caip25EndowmentPermissionName, Caip25CaveatType, - ).value; + ) as Caip25Caveat | undefined; + if (!caveat) { + return; + } + + let updatedCaveatValue = caveat.value; for (const scopeString of scopes) { const result = Caip25CaveatMutators[Caip25CaveatType].removeScope( @@ -110,12 +135,9 @@ function partialRevokePermissions( * @param hooks.getCaveatForOrigin - The hook to fetch an existing caveat for the origin of the request. * @returns Nothing. */ -async function walletRevokeSessionHandler( - request: JsonRpcRequest & { - origin: string; - params: { scopes?: string[] }; - }, - response: JsonRpcSuccess, +async function handleWalletRevokeSession( + request: JsonRpcRequest & { origin: string }, + response: PendingJsonRpcResponse, _next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, hooks: WalletRevokeSessionHooks, @@ -136,12 +158,20 @@ async function walletRevokeSessionHandler( response.result = true; return end(); } -export const walletRevokeSession = { - methodNames: ['wallet_revokeSession'], - implementation: walletRevokeSessionHandler, + +export type WalletRevokeSessionHandler = MethodHandler< + WalletRevokeSessionHooks, + never, + WalletRevokeSessionParams, + Json, + { origin: string } +>; + +export const walletRevokeSessionHandler = { + implementation: handleWalletRevokeSession, hookNames: { revokePermissionForOrigin: true, updateCaveat: true, getCaveatForOrigin: true, }, -}; +} satisfies WalletRevokeSessionHandler; diff --git a/packages/multichain-api-middleware/src/index.test.ts b/packages/multichain-api-middleware/src/index.test.ts index 5792d47e342..58bef74e359 100644 --- a/packages/multichain-api-middleware/src/index.test.ts +++ b/packages/multichain-api-middleware/src/index.test.ts @@ -4,10 +4,7 @@ describe('@metamask/multichain-api-middleware', () => { it('has expected JavaScript exports', () => { expect(Object.keys(allExports)).toMatchInlineSnapshot(` [ - "walletCreateSession", - "walletGetSession", - "walletInvokeMethod", - "walletRevokeSession", + "methodHandlers", "multichainMethodCallValidatorMiddleware", "MultichainMiddlewareManager", "MultichainSubscriptionManager", diff --git a/packages/multichain-api-middleware/src/index.ts b/packages/multichain-api-middleware/src/index.ts index 739547ee4cf..d126c158795 100644 --- a/packages/multichain-api-middleware/src/index.ts +++ b/packages/multichain-api-middleware/src/index.ts @@ -1,7 +1,5 @@ -export { walletCreateSession } from './handlers/wallet-createSession'; -export { walletGetSession } from './handlers/wallet-getSession'; -export { walletInvokeMethod } from './handlers/wallet-invokeMethod'; -export { walletRevokeSession } from './handlers/wallet-revokeSession'; +export { methodHandlers } from './handlers'; +export type { MethodHandlerHooks } from './handlers'; export { multichainMethodCallValidatorMiddleware } from './middlewares/multichainMethodCallValidatorMiddleware'; export { MultichainMiddlewareManager } from './middlewares/MultichainMiddlewareManager'; diff --git a/packages/multichain-api-middleware/tsconfig.build.json b/packages/multichain-api-middleware/tsconfig.build.json index d3f977a6176..6674054205c 100644 --- a/packages/multichain-api-middleware/tsconfig.build.json +++ b/packages/multichain-api-middleware/tsconfig.build.json @@ -7,6 +7,7 @@ "rootDir": "./src" }, "references": [ + { "path": "../accounts-controller/tsconfig.build.json" }, { "path": "../chain-agnostic-permission/tsconfig.build.json" }, { "path": "../json-rpc-engine/tsconfig.build.json" }, { "path": "../network-controller/tsconfig.build.json" }, diff --git a/packages/multichain-api-middleware/tsconfig.json b/packages/multichain-api-middleware/tsconfig.json index d38c2522293..65fc1ac134f 100644 --- a/packages/multichain-api-middleware/tsconfig.json +++ b/packages/multichain-api-middleware/tsconfig.json @@ -6,6 +6,7 @@ "rootDir": "../.." }, "references": [ + { "path": "../accounts-controller" }, { "path": "../chain-agnostic-permission" }, { "path": "../json-rpc-engine" }, { "path": "../network-controller" }, diff --git a/packages/permission-controller/.eslintrc.js b/packages/permission-controller/.eslintrc.js index 29de5a0688d..1e60b9c9bd4 100644 --- a/packages/permission-controller/.eslintrc.js +++ b/packages/permission-controller/.eslintrc.js @@ -3,10 +3,7 @@ module.exports = { overrides: [ { - files: [ - 'src/PermissionController.test.ts', - 'src/rpc-methods/revokePermissions.test.ts', - ], + files: ['src/PermissionController.test.ts'], rules: { // This is taken directly from @metamask/eslint-config-typescript@12.1.0 '@typescript-eslint/naming-convention': [ diff --git a/packages/permission-controller/CHANGELOG.md b/packages/permission-controller/CHANGELOG.md index c9e6f6ab06a..5e5f9b556c4 100644 --- a/packages/permission-controller/CHANGELOG.md +++ b/packages/permission-controller/CHANGELOG.md @@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed - **BREAKING:** Remove `factoryHooks`, `validatorHooks`, and related fields from permission specification builders ([#8551](https://github.com/MetaMask/core/pull/8551)) +- **BREAKING:** Remove permitted method handlers and types ([#8583](https://github.com/MetaMask/core/pull/8583)) + - The permitted method handlers were unused in practice. Replacement types for generic RPC method implementations are available in `@metamask/json-rpc-engine@10.3.0`. ## [12.3.0] diff --git a/packages/permission-controller/src/index.ts b/packages/permission-controller/src/index.ts index 36292db1874..ae79f7cacdb 100644 --- a/packages/permission-controller/src/index.ts +++ b/packages/permission-controller/src/index.ts @@ -27,14 +27,8 @@ export { createPermissionMiddlewareV2, type PermissionMiddlewareActions, } from './permission-middleware'; -export type { - ExtractSpecifications, - HandlerMiddlewareFunction, - HookNames, - PermittedHandlerExport, -} from './utils'; +export type { ExtractSpecifications } from './utils'; export { MethodNames } from './utils'; -export * as permissionRpcMethods from './rpc-methods'; export * from './SubjectMetadataController'; export type { SubjectMetadataControllerClearStateAction, diff --git a/packages/permission-controller/src/rpc-methods/getPermissions.test.ts b/packages/permission-controller/src/rpc-methods/getPermissions.test.ts deleted file mode 100644 index f7724f4617b..00000000000 --- a/packages/permission-controller/src/rpc-methods/getPermissions.test.ts +++ /dev/null @@ -1,74 +0,0 @@ -import { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import type { JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils'; -import { assertIsJsonRpcSuccess } from '@metamask/utils'; - -import type { PermissionConstraint } from '../Permission'; -import { getPermissionsHandler } from './getPermissions'; - -describe('getPermissions RPC method', () => { - it('returns the values of the object returned by getPermissionsForOrigin', async () => { - const { implementation } = getPermissionsHandler; - const mockGetPermissionsForOrigin = jest.fn().mockImplementationOnce(() => { - return { a: 'a', b: 'b', c: 'c' }; - }); - - const engine = new JsonRpcEngine(); - engine.push( - ( - req: JsonRpcRequest<[]>, - res: PendingJsonRpcResponse, - next, - end, - ) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - getPermissionsForOrigin: mockGetPermissionsForOrigin, - }); - }, - ); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - }); - assertIsJsonRpcSuccess(response); - expect(response.result).toStrictEqual(['a', 'b', 'c']); - expect(mockGetPermissionsForOrigin).toHaveBeenCalledTimes(1); - }); - - it('returns an empty array if getPermissionsForOrigin returns a falsy value', async () => { - const { implementation } = getPermissionsHandler; - const mockGetPermissionsForOrigin = jest - .fn() - .mockImplementationOnce(() => null); - - const engine = new JsonRpcEngine(); - engine.push( - ( - req: JsonRpcRequest<[]>, - res: PendingJsonRpcResponse, - next, - end, - ) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - getPermissionsForOrigin: mockGetPermissionsForOrigin, - }); - }, - ); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - }); - assertIsJsonRpcSuccess(response); - expect(response.result).toStrictEqual([]); - expect(mockGetPermissionsForOrigin).toHaveBeenCalledTimes(1); - }); -}); diff --git a/packages/permission-controller/src/rpc-methods/getPermissions.ts b/packages/permission-controller/src/rpc-methods/getPermissions.ts deleted file mode 100644 index b7119973227..00000000000 --- a/packages/permission-controller/src/rpc-methods/getPermissions.ts +++ /dev/null @@ -1,46 +0,0 @@ -import type { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine'; -import type { PendingJsonRpcResponse } from '@metamask/utils'; - -import type { PermissionConstraint } from '../Permission'; -import type { SubjectPermissions } from '../PermissionController'; -import type { PermittedHandlerExport } from '../utils'; -import { MethodNames } from '../utils'; - -export const getPermissionsHandler: PermittedHandlerExport< - GetPermissionsHooks, - [], - PermissionConstraint[] -> = { - methodNames: [MethodNames.GetPermissions], - implementation: getPermissionsImplementation, - hookNames: { - getPermissionsForOrigin: true, - }, -}; - -export type GetPermissionsHooks = { - // This must be bound to the requesting origin. - getPermissionsForOrigin: () => SubjectPermissions; -}; - -/** - * Get Permissions implementation to be used in JsonRpcEngine middleware. - * - * @param _req - The JsonRpcEngine request - unused - * @param res - The JsonRpcEngine result object - * @param _next - JsonRpcEngine next() callback - unused - * @param end - JsonRpcEngine end() callback - * @param options - Method hooks passed to the method implementation - * @param options.getPermissionsForOrigin - The specific method hook needed for this method implementation - * @returns A promise that resolves to nothing - */ -async function getPermissionsImplementation( - _req: unknown, - res: PendingJsonRpcResponse, - _next: unknown, - end: JsonRpcEngineEndCallback, - { getPermissionsForOrigin }: GetPermissionsHooks, -): Promise { - res.result = Object.values(getPermissionsForOrigin() || {}); - return end(); -} diff --git a/packages/permission-controller/src/rpc-methods/index.ts b/packages/permission-controller/src/rpc-methods/index.ts deleted file mode 100644 index 7f13a06cd31..00000000000 --- a/packages/permission-controller/src/rpc-methods/index.ts +++ /dev/null @@ -1,16 +0,0 @@ -import type { GetPermissionsHooks } from './getPermissions'; -import { getPermissionsHandler } from './getPermissions'; -import type { RequestPermissionsHooks } from './requestPermissions'; -import { requestPermissionsHandler } from './requestPermissions'; -import type { RevokePermissionsHooks } from './revokePermissions'; -import { revokePermissionsHandler } from './revokePermissions'; - -export type PermittedRpcMethodHooks = RequestPermissionsHooks & - GetPermissionsHooks & - RevokePermissionsHooks; - -export const handlers = [ - requestPermissionsHandler, - getPermissionsHandler, - revokePermissionsHandler, -] as const; diff --git a/packages/permission-controller/src/rpc-methods/requestPermissions.test.ts b/packages/permission-controller/src/rpc-methods/requestPermissions.test.ts deleted file mode 100644 index 4422ed67a7e..00000000000 --- a/packages/permission-controller/src/rpc-methods/requestPermissions.test.ts +++ /dev/null @@ -1,143 +0,0 @@ -import { - JsonRpcEngine, - createAsyncMiddleware, -} from '@metamask/json-rpc-engine'; -import { rpcErrors, serializeError } from '@metamask/rpc-errors'; -import { - assertIsJsonRpcFailure, - assertIsJsonRpcSuccess, - hasProperty, -} from '@metamask/utils'; -import type { - PermissionConstraint, - RequestedPermissions, -} from 'src/Permission'; - -import { requestPermissionsHandler } from './requestPermissions'; - -describe('requestPermissions RPC method', () => { - it('returns the values of the object returned by requestPermissionsForOrigin', async () => { - const { implementation } = requestPermissionsHandler; - const mockRequestPermissionsForOrigin = jest - .fn() - .mockImplementationOnce(() => { - // Resolve this promise after a timeout to ensure that the function - // is awaited properly. - return new Promise((resolve) => { - setTimeout(() => { - resolve([{ a: 'a', b: 'b', c: 'c' }]); - }, 10); - }); - }); - - const engine = new JsonRpcEngine(); - engine.push<[RequestedPermissions], PermissionConstraint[]>( - (req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - requestPermissionsForOrigin: mockRequestPermissionsForOrigin, - }); - }, - ); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - params: [{}], - }); - assertIsJsonRpcSuccess(response); - - expect(response.result).toStrictEqual(['a', 'b', 'c']); - expect(mockRequestPermissionsForOrigin).toHaveBeenCalledTimes(1); - expect(mockRequestPermissionsForOrigin).toHaveBeenCalledWith({}); - }); - - it('returns an error if requestPermissionsForOrigin rejects', async () => { - const { implementation } = requestPermissionsHandler; - const mockRequestPermissionsForOrigin = jest - .fn() - .mockImplementationOnce(async () => { - throw new Error('foo'); - }); - - const engine = new JsonRpcEngine(); - const end = (): undefined => undefined; // this won't be called - - // Pass the middleware function to createAsyncMiddleware so the error - // is catched. - engine.push<[RequestedPermissions], PermissionConstraint[]>( - createAsyncMiddleware(async (req, res, next) => - // This promise will be awaited by the createAsyncMiddleware wrapper. - // eslint-disable-next-line @typescript-eslint/no-misused-promises - implementation(req, res, next, end, { - requestPermissionsForOrigin: mockRequestPermissionsForOrigin, - }), - ), - ); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - params: [{}], - }); - assertIsJsonRpcFailure(response); - - expect(hasProperty(response, 'result')).toBe(false); - delete response.error.stack; - // @ts-expect-error We do expect this property to exist. - delete response.error.data.cause.stack; - const expectedError = new Error('foo'); - delete expectedError.stack; - expect(response.error).toStrictEqual( - serializeError(expectedError, { shouldIncludeStack: false }), - ); - expect(mockRequestPermissionsForOrigin).toHaveBeenCalledTimes(1); - expect(mockRequestPermissionsForOrigin).toHaveBeenCalledWith({}); - }); - - it('returns an error if the request params are invalid', async () => { - const { implementation } = requestPermissionsHandler; - const mockRequestPermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push<[RequestedPermissions], PermissionConstraint[]>( - (req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - requestPermissionsForOrigin: mockRequestPermissionsForOrigin, - }); - }, - ); - - for (const invalidParams of ['foo', ['bar']]) { - const request = { - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - params: invalidParams, - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...request } }, - }) - .serialize(); - delete expectedError.stack; - - // @ts-expect-error Intentional destructive testing - // ESLint is confused; this signature is async. - // eslint-disable-next-line @typescript-eslint/await-thenable - const response = await engine.handle(request); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRequestPermissionsForOrigin).not.toHaveBeenCalled(); - } - }); -}); diff --git a/packages/permission-controller/src/rpc-methods/requestPermissions.ts b/packages/permission-controller/src/rpc-methods/requestPermissions.ts deleted file mode 100644 index ae77a2dfae5..00000000000 --- a/packages/permission-controller/src/rpc-methods/requestPermissions.ts +++ /dev/null @@ -1,63 +0,0 @@ -import { isPlainObject } from '@metamask/controller-utils'; -import type { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine'; -import type { JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils'; - -import { invalidParams } from '../errors'; -import type { PermissionConstraint, RequestedPermissions } from '../Permission'; -import type { PermittedHandlerExport } from '../utils'; -import { MethodNames } from '../utils'; - -export const requestPermissionsHandler: PermittedHandlerExport< - RequestPermissionsHooks, - [RequestedPermissions], - PermissionConstraint[] -> = { - methodNames: [MethodNames.RequestPermissions], - implementation: requestPermissionsImplementation, - hookNames: { - requestPermissionsForOrigin: true, - }, -}; - -type RequestPermissions = ( - requestedPermissions: RequestedPermissions, -) => Promise< - [Record, { id: string; origin: string }] ->; - -export type RequestPermissionsHooks = { - requestPermissionsForOrigin: RequestPermissions; -}; - -/** - * Request Permissions implementation to be used in JsonRpcEngine middleware. - * - * @param req - The JsonRpcEngine request - * @param res - The JsonRpcEngine result object - * @param _next - JsonRpcEngine next() callback - unused - * @param end - JsonRpcEngine end() callback - * @param options - Method hooks passed to the method implementation - * @param options.requestPermissionsForOrigin - The specific method hook needed for this method implementation - * @returns A promise that resolves to nothing - */ -async function requestPermissionsImplementation( - req: JsonRpcRequest<[RequestedPermissions]>, - res: PendingJsonRpcResponse, - _next: unknown, - end: JsonRpcEngineEndCallback, - { requestPermissionsForOrigin }: RequestPermissionsHooks, -): Promise { - const { params } = req; - - if (!Array.isArray(params) || !isPlainObject(params[0])) { - return end(invalidParams({ data: { request: req } })); - } - - const [requestedPermissions] = params; - const [grantedPermissions] = - await requestPermissionsForOrigin(requestedPermissions); - - // `wallet_requestPermission` is specified to return an array. - res.result = Object.values(grantedPermissions); - return end(); -} diff --git a/packages/permission-controller/src/rpc-methods/revokePermissions.test.ts b/packages/permission-controller/src/rpc-methods/revokePermissions.test.ts deleted file mode 100644 index 60465a1b475..00000000000 --- a/packages/permission-controller/src/rpc-methods/revokePermissions.test.ts +++ /dev/null @@ -1,196 +0,0 @@ -import { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import { rpcErrors } from '@metamask/rpc-errors'; -import type { Json, JsonRpcRequest } from '@metamask/utils'; -import { - assertIsJsonRpcFailure, - assertIsJsonRpcSuccess, -} from '@metamask/utils'; - -import type { RevokePermissionArgs } from './revokePermissions'; -import { revokePermissionsHandler } from './revokePermissions'; - -describe('revokePermissionsHandler', () => { - it('has the expected shape', () => { - expect(revokePermissionsHandler).toStrictEqual({ - methodNames: ['wallet_revokePermissions'], - implementation: expect.any(Function), - hookNames: { - revokePermissionsForOrigin: true, - }, - }); - }); -}); - -describe('revokePermissions RPC method', () => { - it('revokes permissions using revokePermissionsForOrigin', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - params: [ - { - snap_dialog: {}, - }, - ], - }); - assertIsJsonRpcSuccess(response); - - expect(response.result).toBeNull(); - expect(mockRevokePermissionsForOrigin).toHaveBeenCalledTimes(1); - expect(mockRevokePermissionsForOrigin).toHaveBeenCalledWith([ - 'snap_dialog', - ]); - }); - - it('returns an error if the request params is a plain object', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const req: JsonRpcRequest> = { - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - params: {}, - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...req } }, - }) - .serialize(); - delete expectedError.stack; - - const response = await engine.handle(req); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRevokePermissionsForOrigin).not.toHaveBeenCalled(); - }); - - it('returns an error if the permissionKeys is a plain object', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const req: JsonRpcRequest<[Record]> = { - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - params: [{}], - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...req } }, - }) - .serialize(); - delete expectedError.stack; - - const response = await engine.handle(req); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRevokePermissionsForOrigin).not.toHaveBeenCalled(); - }); - - it('returns an error if the params are not set', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const req: JsonRpcRequest<[]> = { - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...req } }, - }) - .serialize(); - delete expectedError.stack; - - const response = await engine.handle(req); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRevokePermissionsForOrigin).not.toHaveBeenCalled(); - }); - - it('returns an error if the request params is an empty array', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const req: JsonRpcRequest = { - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - params: [], - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...req } }, - }) - .serialize(); - delete expectedError.stack; - - const response = await engine.handle(req); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRevokePermissionsForOrigin).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/permission-controller/src/rpc-methods/revokePermissions.ts b/packages/permission-controller/src/rpc-methods/revokePermissions.ts deleted file mode 100644 index 34b9b6352ef..00000000000 --- a/packages/permission-controller/src/rpc-methods/revokePermissions.ts +++ /dev/null @@ -1,79 +0,0 @@ -import type { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine'; -import { isNonEmptyArray } from '@metamask/utils'; -import type { - Json, - JsonRpcRequest, - NonEmptyArray, - PendingJsonRpcResponse, -} from '@metamask/utils'; - -import { invalidParams } from '../errors'; -import type { PermissionConstraint } from '../Permission'; -import type { PermittedHandlerExport } from '../utils'; -import { MethodNames } from '../utils'; - -export const revokePermissionsHandler: PermittedHandlerExport< - RevokePermissionsHooks, - RevokePermissionArgs, - null -> = { - methodNames: [MethodNames.RevokePermissions], - implementation: revokePermissionsImplementation, - hookNames: { - revokePermissionsForOrigin: true, - }, -}; - -export type RevokePermissionArgs = Record< - PermissionConstraint['parentCapability'], - Json ->; - -type RevokePermissions = ( - permissions: NonEmptyArray, -) => void; - -export type RevokePermissionsHooks = { - revokePermissionsForOrigin: RevokePermissions; -}; - -/** - * Revoke Permissions implementation to be used in JsonRpcEngine middleware. - * - * @param req - The JsonRpcEngine request - * @param res - The JsonRpcEngine result object - * @param _next - JsonRpcEngine next() callback - unused - * @param end - JsonRpcEngine end() callback - * @param options - Method hooks passed to the method implementation - * @param options.revokePermissionsForOrigin - A hook that revokes given permission keys for an origin - * @returns A promise that resolves to nothing - */ -async function revokePermissionsImplementation( - req: JsonRpcRequest, - res: PendingJsonRpcResponse, - _next: unknown, - end: JsonRpcEngineEndCallback, - { revokePermissionsForOrigin }: RevokePermissionsHooks, -): Promise { - const { params } = req; - - const param = params?.[0]; - - if (!param) { - return end(invalidParams({ data: { request: req } })); - } - - // For now, this API revokes the entire permission key - // even if caveats are specified. - const permissionKeys = Object.keys(param); - - if (!isNonEmptyArray(permissionKeys)) { - return end(invalidParams({ data: { request: req } })); - } - - revokePermissionsForOrigin(permissionKeys); - - res.result = null; - - return end(); -} diff --git a/packages/permission-controller/src/utils.ts b/packages/permission-controller/src/utils.ts index e26ac47efe1..d576924ad6d 100644 --- a/packages/permission-controller/src/utils.ts +++ b/packages/permission-controller/src/utils.ts @@ -1,14 +1,3 @@ -import type { - JsonRpcEngineEndCallback, - JsonRpcEngineNextCallback, -} from '@metamask/json-rpc-engine'; -import type { - Json, - JsonRpcParams, - JsonRpcRequest, - PendingJsonRpcResponse, -} from '@metamask/utils'; - import type { CaveatConstraint, CaveatSpecificationConstraint, @@ -40,44 +29,6 @@ export type ExtractSpecifications< | PermissionSpecificationMap, > = SpecificationsMap[keyof SpecificationsMap]; -/** - * A middleware function for handling a permitted method. - */ -export type HandlerMiddlewareFunction< - Hooks, - Params extends JsonRpcParams, - Result extends Json, -> = ( - req: JsonRpcRequest, - res: PendingJsonRpcResponse, - next: JsonRpcEngineNextCallback, - end: JsonRpcEngineEndCallback, - hooks: Hooks, -) => void | Promise; - -/** - * We use a mapped object type in order to create a type that requires the - * presence of the names of all hooks for the given handler. - * This can then be used to select only the necessary hooks whenever a method - * is called for purposes of POLA. - */ -export type HookNames = { - [Property in keyof HookMap]: true; -}; - -/** - * A handler for a permitted method. - */ -export type PermittedHandlerExport< - Hooks, - Params extends JsonRpcParams, - Result extends Json, -> = { - implementation: HandlerMiddlewareFunction; - hookNames: HookNames; - methodNames: string[]; -}; - /** * Given two permission objects, computes 3 sets: * - The set of caveat pairs that are common to both permissions. diff --git a/yarn.lock b/yarn.lock index 207f7f147ad..d0ef7d43d66 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4587,6 +4587,7 @@ __metadata: version: 0.0.0-use.local resolution: "@metamask/multichain-api-middleware@workspace:packages/multichain-api-middleware" dependencies: + "@metamask/accounts-controller": "npm:^37.2.0" "@metamask/api-specs": "npm:^0.14.0" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/chain-agnostic-permission": "npm:^1.5.0" @@ -4598,6 +4599,7 @@ __metadata: "@metamask/permission-controller": "npm:^12.3.0" "@metamask/rpc-errors": "npm:^7.0.2" "@metamask/safe-event-emitter": "npm:^3.0.0" + "@metamask/snaps-controllers": "npm:^19.0.0" "@metamask/utils": "npm:^11.9.0" "@open-rpc/meta-schema": "npm:^1.14.6" "@open-rpc/schema-utils-js": "npm:^2.0.5"