Skip to content

Commit

Permalink
feat: Ensure unsupported method rejection occurs before permission mi…
Browse files Browse the repository at this point in the history
…ddleware
  • Loading branch information
rekmarks committed May 10, 2024
1 parent ce29250 commit 9fb45c4
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { permissionRpcMethods } from '@metamask/permission-controller';
import { selectHooks } from '@metamask/snaps-rpc-methods';
import { hasProperty } from '@metamask/utils';
import { ethErrors } from 'eth-rpc-errors';
import { UNSUPPORTED_RPC_METHODS } from '../../../../shared/constants/network';
import localHandlers from './handlers';

const allHandlers = [...localHandlers, ...permissionRpcMethods.handlers];
Expand Down Expand Up @@ -32,11 +31,6 @@ export function createMethodMiddleware(hooks) {
assertExpectedHook(hooks);

return async function methodMiddleware(req, res, next, end) {
// Reject unsupported methods.
if (UNSUPPORTED_RPC_METHODS.has(req.method)) {
return end(ethErrors.rpc.methodNotSupported());
}

const handler = handlerMap[req.method];
if (handler) {
const { implementation, hookNames } = handler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,21 +122,6 @@ describe('createMethodMiddleware', () => {
});
});

it('should reject unsupported methods', async () => {
const middleware = createMethodMiddleware(getDefaultHooks());
const engine = new JsonRpcEngine();
engine.push(middleware);

const response = await engine.handle({
jsonrpc: '2.0',
id: 1,
method: 'eth_signTransaction',
});
assertIsJsonRpcFailure(response);

expect(response.error.message).toBe('Method not supported.');
});

it('should handle errors returned by the implementation', async () => {
const middleware = createMethodMiddleware(getDefaultHooks());
const engine = new JsonRpcEngine();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { jsonrpc2 } from '@metamask/utils';
import { UNSUPPORTED_RPC_METHODS } from '../../../../shared/constants/network';
import { createUnsupportedMethodMiddleware } from '.';

describe('createDupeReqFilterMiddleware', () => {
const getMockRequest = (method: string) => ({
jsonrpc: jsonrpc2,
id: 1,
method,
});
const getMockResponse = () => ({ jsonrpc: jsonrpc2, id: 'foo' });

it('forwards requests whose methods are not on the list of unsupported methods', () => {
const middleware = createUnsupportedMethodMiddleware();
const nextMock = jest.fn();
const endMock = jest.fn();

middleware(getMockRequest('foo'), getMockResponse(), nextMock, endMock);

expect(nextMock).toHaveBeenCalledTimes(1);
expect(endMock).not.toHaveBeenCalled();
});

it.each([...UNSUPPORTED_RPC_METHODS.keys()])(
'ends requests for methods that are on the list of unsupported methods: %s',
(method) => {
const middleware = createUnsupportedMethodMiddleware();
const nextMock = jest.fn();
const endMock = jest.fn();

const response = getMockResponse();
middleware(getMockRequest(method), response, nextMock, endMock);

expect('result' in response).toBe(false);
expect(nextMock).not.toHaveBeenCalled();
expect(endMock).toHaveBeenCalledTimes(1);
},
);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { ethErrors } from 'eth-rpc-errors';
import type { JsonRpcMiddleware } from 'json-rpc-engine';
import { UNSUPPORTED_RPC_METHODS } from '../../../../shared/constants/network';

/**
* Creates a middleware that rejects explicitly unsupported RPC methods with the
* appropriate error.
*/
export function createUnsupportedMethodMiddleware(): JsonRpcMiddleware<
unknown,
void
> {
return async function unsupportedMethodMiddleware(req, _res, next, end) {
if ((UNSUPPORTED_RPC_METHODS as Set<string>).has(req.method)) {
return end(ethErrors.rpc.methodNotSupported());
}
return next();
};
}
1 change: 1 addition & 0 deletions app/scripts/lib/rpc-method-middleware/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './createMethodMiddleware';
export * from './createUnsupportedMethodMiddleware';
7 changes: 6 additions & 1 deletion app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,10 @@ import ComposableObservableStore from './lib/ComposableObservableStore';
import AccountTracker from './lib/account-tracker';
import createDupeReqFilterMiddleware from './lib/createDupeReqFilterMiddleware';
import createLoggerMiddleware from './lib/createLoggerMiddleware';
import { createMethodMiddleware } from './lib/rpc-method-middleware';
import {
createMethodMiddleware,
createUnsupportedMethodMiddleware,
} from './lib/rpc-method-middleware';
import createOriginMiddleware from './lib/createOriginMiddleware';
import createTabIdMiddleware from './lib/createTabIdMiddleware';
import { NetworkOrderController } from './controllers/network-order';
Expand Down Expand Up @@ -4923,6 +4926,8 @@ export default class MetamaskController extends EventEmitter {
}),
);

engine.push(createUnsupportedMethodMiddleware());

if (subjectType !== SubjectType.Internal) {
engine.push(
this.permissionController.createPermissionMiddleware({
Expand Down
3 changes: 3 additions & 0 deletions app/scripts/metamask-controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ const rpcMethodMiddlewareMock = {
createMethodMiddleware: () => (_req, _res, next, _end) => {
next();
},
createUnsupportedMethodMiddleware: () => (_req, _res, next, _end) => {
next();
},
};
jest.mock('./lib/rpc-method-middleware', () => rpcMethodMiddlewareMock);

Expand Down

0 comments on commit 9fb45c4

Please sign in to comment.