From c22ce079bdf9c25f9a423a7d16714967c3c4ad85 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 11 Dec 2024 11:28:01 -0500 Subject: [PATCH] address PR comments --- package.json | 2 - .../src/cronjob/CronjobController.test.ts | 201 +++++++++++++++++- .../src/cronjob/CronjobController.ts | 139 +++++------- packages/snaps-rpc-methods/jest.config.js | 6 +- .../src/permitted/getBackgroundEvents.test.ts | 57 ++++- .../src/permitted/getBackgroundEvents.ts | 6 +- ...est.ts => scheduleBackgroundEvent.test.ts} | 70 +++++- .../src/permitted/scheduleBackgroundEvent.ts | 25 ++- .../types/methods/get-background-events.ts | 11 + .../methods/schedule-background-event.ts | 6 + yarn.lock | 2 - 11 files changed, 417 insertions(+), 108 deletions(-) rename packages/snaps-rpc-methods/src/permitted/{scheduleBackground.test.ts => scheduleBackgroundEvent.test.ts} (65%) diff --git a/package.json b/package.json index 5c7d39160a..daad18532d 100644 --- a/package.json +++ b/package.json @@ -84,7 +84,6 @@ "@ts-bridge/cli": "^0.6.1", "@types/jest": "^27.5.1", "@types/lodash": "^4", - "@types/luxon": "^3", "@types/node": "18.14.2", "@typescript-eslint/eslint-plugin": "^5.42.1", "@typescript-eslint/parser": "^6.21.0", @@ -106,7 +105,6 @@ "jest-silent-reporter": "^0.6.0", "lint-staged": "^12.4.1", "lodash": "^4.17.21", - "luxon": "^3.5.0", "minimatch": "^7.4.1", "prettier": "^2.8.8", "prettier-plugin-packagejson": "^2.5.2", diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.test.ts b/packages/snaps-controllers/src/cronjob/CronjobController.test.ts index eb16615bff..5c64c5863f 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.test.ts +++ b/packages/snaps-controllers/src/cronjob/CronjobController.test.ts @@ -312,7 +312,7 @@ describe('CronjobController', () => { [id]: { id, scheduledAt: expect.any(String), ...backgroundEvent }, }); - cronjobController.cancelBackgroundEvent(id); + cronjobController.cancelBackgroundEvent(id, MOCK_SNAP_ID); jest.advanceTimersByTime(inMilliseconds(1, Duration.Day)); @@ -334,6 +334,37 @@ describe('CronjobController', () => { cronjobController.destroy(); }); + it('fails to cancel a background event if the caller is not the scheduler', () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + const backgroundEvent = { + snapId: MOCK_SNAP_ID, + date: '2022-01-01T01:00', + request: { + method: 'handleEvent', + params: ['p1'], + }, + }; + + const id = cronjobController.scheduleBackgroundEvent(backgroundEvent); + + expect(cronjobController.state.events).toStrictEqual({ + [id]: { id, scheduledAt: expect.any(String), ...backgroundEvent }, + }); + + expect(() => cronjobController.cancelBackgroundEvent(id, 'foo')).toThrow( + 'Only the origin that scheduled this event can cancel it', + ); + + cronjobController.destroy(); + }); + it("returns a list of a Snap's background events", () => { const rootMessenger = getRootCronjobControllerMessenger(); const controllerMessenger = @@ -808,4 +839,172 @@ describe('CronjobController', () => { }, ); }); + + describe('CronjobController actions', () => { + describe('CronjobController:scheduleBackgroundEvent', () => { + it('schedules a background event', () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + cronjobController.register(MOCK_SNAP_ID); + + const id = rootMessenger.call( + 'CronjobController:scheduleBackgroundEvent', + { + snapId: MOCK_SNAP_ID, + date: '2022-01-01T01:00', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + ); + + expect(cronjobController.state.events).toStrictEqual({ + [id]: { + id, + snapId: MOCK_SNAP_ID, + scheduledAt: expect.any(String), + date: '2022-01-01T01:00', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + }); + + jest.advanceTimersByTime(inMilliseconds(1, Duration.Day)); + + expect(rootMessenger.call).toHaveBeenCalledWith( + 'SnapController:handleRequest', + { + snapId: MOCK_SNAP_ID, + origin: '', + handler: HandlerType.OnCronjob, + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + ); + + expect(cronjobController.state.events).toStrictEqual({}); + + cronjobController.destroy(); + }); + }); + + describe('CronjobController:cancelBackgroundEvent', () => { + it('cancels a background event', () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + cronjobController.register(MOCK_SNAP_ID); + + const id = rootMessenger.call( + 'CronjobController:scheduleBackgroundEvent', + { + snapId: MOCK_SNAP_ID, + date: '2022-01-01T01:00', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + ); + + expect(cronjobController.state.events).toStrictEqual({ + [id]: { + id, + snapId: MOCK_SNAP_ID, + scheduledAt: expect.any(String), + date: '2022-01-01T01:00', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + }); + + rootMessenger.call( + 'CronjobController:cancelBackgroundEvent', + id, + MOCK_SNAP_ID, + ); + + expect(cronjobController.state.events).toStrictEqual({}); + + cronjobController.destroy(); + }); + }); + + describe('CronjobController:getBackgroundEvents', () => { + it("gets a list of a Snap's background events", () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + cronjobController.register(MOCK_SNAP_ID); + + const id = rootMessenger.call( + 'CronjobController:scheduleBackgroundEvent', + { + snapId: MOCK_SNAP_ID, + date: '2022-01-01T01:00', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + ); + + expect(cronjobController.state.events).toStrictEqual({ + [id]: { + id, + snapId: MOCK_SNAP_ID, + scheduledAt: expect.any(String), + date: '2022-01-01T01:00', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + }); + + const events = rootMessenger.call( + 'CronjobController:getBackgroundEvents', + MOCK_SNAP_ID, + ); + + expect(events).toStrictEqual([ + { + id, + snapId: MOCK_SNAP_ID, + scheduledAt: expect.any(String), + date: '2022-01-01T01:00', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + ]); + + cronjobController.destroy(); + }); + }); + }); }); diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.ts b/packages/snaps-controllers/src/cronjob/CronjobController.ts index 5c68700607..81e8091e8b 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.ts +++ b/packages/snaps-controllers/src/cronjob/CronjobController.ts @@ -19,7 +19,7 @@ import { parseCronExpression, logError, } from '@metamask/snaps-utils'; -import { assert, Duration, hasProperty, inMilliseconds } from '@metamask/utils'; +import { assert, Duration, inMilliseconds } from '@metamask/utils'; import { castDraft } from 'immer'; import { nanoid } from 'nanoid'; @@ -149,11 +149,6 @@ export class CronjobController extends BaseController< this.#snapIds = new Map(); this.#messenger = messenger; - this._handleSnapRegisterEvent = this._handleSnapRegisterEvent.bind(this); - this._handleSnapUnregisterEvent = - this._handleSnapUnregisterEvent.bind(this); - this._handleEventSnapUpdated = this._handleEventSnapUpdated.bind(this); - // Subscribe to Snap events /* eslint-disable @typescript-eslint/unbound-method */ @@ -227,7 +222,7 @@ export class CronjobController extends BaseController< logError(error); }); - this.rescheduleBackgroundEvents(Object.values(this.state.events)).catch( + this.#rescheduleBackgroundEvents(Object.values(this.state.events)).catch( (error) => { logError(error); }, @@ -239,11 +234,11 @@ export class CronjobController extends BaseController< * * @returns Array of Cronjob specifications. */ - private getAllJobs(): Cronjob[] { + #getAllJobs(): Cronjob[] { const snaps = this.messagingSystem.call('SnapController:getAll'); const filteredSnaps = getRunnableSnaps(snaps); - const jobs = filteredSnaps.map((snap) => this.getSnapJobs(snap.id)); + const jobs = filteredSnaps.map((snap) => this.#getSnapJobs(snap.id)); // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion return jobs.flat().filter((job) => job !== undefined) as Cronjob[]; } @@ -254,7 +249,7 @@ export class CronjobController extends BaseController< * @param snapId - ID of a Snap. * @returns Array of Cronjob specifications. */ - private getSnapJobs(snapId: SnapId): Cronjob[] | undefined { + #getSnapJobs(snapId: SnapId): Cronjob[] | undefined { const permissions = this.#messenger.call( 'PermissionController:getPermissions', snapId, @@ -275,8 +270,8 @@ export class CronjobController extends BaseController< * @param snapId - ID of a snap. */ register(snapId: SnapId) { - const jobs = this.getSnapJobs(snapId); - jobs?.forEach((job) => this.schedule(job)); + const jobs = this.#getSnapJobs(snapId); + jobs?.forEach((job) => this.#schedule(job)); } /** @@ -290,7 +285,7 @@ export class CronjobController extends BaseController< * * @param job - Cronjob specification. */ - private schedule(job: Cronjob) { + #schedule(job: Cronjob) { if (this.#timers.has(job.id)) { return; } @@ -307,17 +302,17 @@ export class CronjobController extends BaseController< const timer = new Timer(ms); timer.start(() => { - this.executeCronjob(job).catch((error) => { + this.#executeCronjob(job).catch((error) => { // TODO: Decide how to handle errors. logError(error); }); this.#timers.delete(job.id); - this.schedule(job); + this.#schedule(job); }); if (!this.state.jobs[job.id]?.lastRun) { - this.updateJobLastRunState(job.id, 0); // 0 for init, never ran actually + this.#updateJobLastRunState(job.id, 0); // 0 for init, never ran actually } this.#timers.set(job.id, timer); @@ -329,8 +324,8 @@ export class CronjobController extends BaseController< * * @param job - Cronjob specification. */ - private async executeCronjob(job: Cronjob) { - this.updateJobLastRunState(job.id, Date.now()); + async #executeCronjob(job: Cronjob) { + this.#updateJobLastRunState(job.id, Date.now()); await this.#messenger.call('SnapController:handleRequest', { snapId: job.snapId, origin: '', @@ -348,9 +343,12 @@ export class CronjobController extends BaseController< scheduleBackgroundEvent( backgroundEventWithoutId: Omit, ) { - const event = this.getBackgroundEventWithId(backgroundEventWithoutId); - event.scheduledAt = new Date().toISOString(); - this.setUpBackgroundEvent(event); + const event = { + ...backgroundEventWithoutId, + id: nanoid(), + scheduledAt: new Date().toISOString(), + }; + this.#setUpBackgroundEvent(event); this.update((state) => { state.events[event.id] = castDraft(event); }); @@ -362,14 +360,20 @@ export class CronjobController extends BaseController< * Cancel a background event. * * @param id - The id of the background event to cancel. + * @param origin - The origin making the cancel call. * @throws If the event does not exist. */ - cancelBackgroundEvent(id: string) { + cancelBackgroundEvent(id: string, origin: string) { assert( this.state.events[id], `A background event with the id of "${id}" does not exist.`, ); + assert( + this.state.events[id].snapId === origin, + 'Only the origin that scheduled this event can cancel it', + ); + const timer = this.#timers.get(id); timer?.cancel(); this.#timers.delete(id); @@ -379,42 +383,28 @@ export class CronjobController extends BaseController< }); } - /** - * Assign an id to a background event. - * - * @param backgroundEventWithoutId - A background event with an unassigned id. - * @returns A background event with an id. - */ - private getBackgroundEventWithId( - backgroundEventWithoutId: Omit, - ): BackgroundEvent { - assert( - !hasProperty(backgroundEventWithoutId, 'id'), - `Background event already has an id: ${ - (backgroundEventWithoutId as BackgroundEvent).id - }`, - ); - const event = backgroundEventWithoutId as BackgroundEvent; - const id = this.generateBackgroundEventId(); - event.id = id; - return event; - } - /** * A helper function to handle setup of the background event. * * @param event - A background event. */ - private setUpBackgroundEvent(event: BackgroundEvent) { + #setUpBackgroundEvent(event: BackgroundEvent) { const date = new Date(event.date); const now = new Date(); const ms = date.getTime() - now.getTime(); const timer = new Timer(ms); timer.start(() => { - this.executeBackgroundEvent(event).catch((error) => { - logError(error); - }); + this.#messenger + .call('SnapController:handleRequest', { + snapId: event.snapId, + origin: '', + handler: HandlerType.OnCronjob, + request: event.request, + }) + .catch((error) => { + logError(error); + }); this.#timers.delete(event.id); this.#snapIds.delete(event.id); @@ -427,20 +417,6 @@ export class CronjobController extends BaseController< this.#snapIds.set(event.id, event.snapId); } - /** - * Fire the background event. - * - * @param event - A background event. - */ - private async executeBackgroundEvent(event: BackgroundEvent) { - await this.#messenger.call('SnapController:handleRequest', { - snapId: event.snapId, - origin: '', - handler: HandlerType.OnCronjob, - request: event.request, - }); - } - /** * Get a list of a Snap's background events. * @@ -465,6 +441,7 @@ export class CronjobController extends BaseController< ); if (jobs.length) { + const eventIds: string[] = []; jobs.forEach(([id]) => { const timer = this.#timers.get(id); if (timer) { @@ -472,12 +449,17 @@ export class CronjobController extends BaseController< this.#timers.delete(id); this.#snapIds.delete(id); if (!skipEvents && this.state.events[id]) { - this.update((state) => { - delete state.events[id]; - }); + eventIds.push(id); } } }); + if (eventIds.length > 0) { + this.update((state) => { + eventIds.forEach((id) => { + delete state.events[id]; + }); + }); + } } } @@ -487,7 +469,7 @@ export class CronjobController extends BaseController< * @param jobId - ID of a cron job. * @param lastRun - Unix timestamp when the job was last ran. */ - private updateJobLastRunState(jobId: string, lastRun: number) { + #updateJobLastRunState(jobId: string, lastRun: number) { this.update((state) => { state.jobs[jobId] = { lastRun, @@ -495,26 +477,13 @@ export class CronjobController extends BaseController< }); } - /** - * Generate a unique id for a background event. - * - * @returns An id. - */ - private generateBackgroundEventId(): string { - const id = nanoid(); - if (this.state.events[id]) { - this.generateBackgroundEventId(); - } - return id; - } - /** * Runs every 24 hours to check if new jobs need to be scheduled. * * This is necessary for longer running jobs that execute with more than 24 hours between them. */ async dailyCheckIn() { - const jobs = this.getAllJobs(); + const jobs = this.#getAllJobs(); for (const job of jobs) { const parsed = parseCronExpression(job.expression); @@ -525,11 +494,11 @@ export class CronjobController extends BaseController< parsed.hasPrev() && parsed.prev().getTime() > lastRun ) { - await this.executeCronjob(job); + await this.#executeCronjob(job); } // Try scheduling, will fail if an existing scheduled job is found - this.schedule(job); + this.#schedule(job); } this.#dailyTimer = new Timer(DAILY_TIMEOUT); @@ -546,9 +515,7 @@ export class CronjobController extends BaseController< * * @param backgroundEvents - A list of background events to reschdule. */ - private async rescheduleBackgroundEvents( - backgroundEvents: BackgroundEvent[], - ) { + async #rescheduleBackgroundEvents(backgroundEvents: BackgroundEvent[]) { for (const snapEvent of backgroundEvents) { const { date } = snapEvent; const now = new Date(); @@ -564,7 +531,7 @@ export class CronjobController extends BaseController< ); logError(error); } else { - this.setUpBackgroundEvent(snapEvent); + this.#setUpBackgroundEvent(snapEvent); } } } @@ -624,7 +591,7 @@ export class CronjobController extends BaseController< */ private _handleSnapEnabledEvent(snap: TruncatedSnap) { const events = this.getBackgroundEvents(snap.id); - this.rescheduleBackgroundEvents(events).catch((error) => logError(error)); + this.#rescheduleBackgroundEvents(events).catch((error) => logError(error)); this.register(snap.id); } diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index c5639ad031..73bee7457a 100644 --- a/packages/snaps-rpc-methods/jest.config.js +++ b/packages/snaps-rpc-methods/jest.config.js @@ -10,10 +10,10 @@ module.exports = deepmerge(baseConfig, { ], coverageThreshold: { global: { - branches: 93.02, + branches: 93.05, functions: 97.35, - lines: 97.89, - statements: 97.44, + lines: 97.99, + statements: 97.53, }, }, }); diff --git a/packages/snaps-rpc-methods/src/permitted/getBackgroundEvents.test.ts b/packages/snaps-rpc-methods/src/permitted/getBackgroundEvents.test.ts index 84bf73afcf..682ec51914 100644 --- a/packages/snaps-rpc-methods/src/permitted/getBackgroundEvents.test.ts +++ b/packages/snaps-rpc-methods/src/permitted/getBackgroundEvents.test.ts @@ -1,7 +1,10 @@ import { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import type { GetBackgroundEventsResult } from '@metamask/snaps-sdk'; +import type { + GetBackgroundEventsParams, + GetBackgroundEventsResult, +} from '@metamask/snaps-sdk'; import { MOCK_SNAP_ID } from '@metamask/snaps-utils/test-utils'; -import type { PendingJsonRpcResponse } from '@metamask/utils'; +import type { JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils'; import { getBackgroundEventsHandler } from './getBackgroundEvents'; @@ -47,7 +50,7 @@ describe('snap_getBackgroundEvents', () => { engine.push((request, response, next, end) => { const result = implementation( - request, + request as JsonRpcRequest, response as PendingJsonRpcResponse, next, end, @@ -83,7 +86,7 @@ describe('snap_getBackgroundEvents', () => { engine.push((request, response, next, end) => { const result = implementation( - request, + request as JsonRpcRequest, response as PendingJsonRpcResponse, next, end, @@ -101,5 +104,51 @@ describe('snap_getBackgroundEvents', () => { expect(getBackgroundEvents).toHaveBeenCalled(); }); + + it('will throw if the call to the `getBackgroundEvents` hook fails', async () => { + const { implementation } = getBackgroundEventsHandler; + + const getBackgroundEvents = jest.fn().mockImplementation(() => { + throw new Error('foobar'); + }); + + const hooks = { + getBackgroundEvents, + }; + + const engine = new JsonRpcEngine(); + + engine.push((request, response, next, end) => { + const result = implementation( + request as JsonRpcRequest, + response as PendingJsonRpcResponse, + next, + end, + hooks, + ); + + result?.catch(end); + }); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: 'snap_getBackgroundEvent', + }); + + expect(response).toStrictEqual({ + error: { + code: -32603, + data: { + cause: expect.objectContaining({ + message: 'foobar', + }), + }, + message: 'foobar', + }, + id: 1, + jsonrpc: '2.0', + }); + }); }); }); diff --git a/packages/snaps-rpc-methods/src/permitted/getBackgroundEvents.ts b/packages/snaps-rpc-methods/src/permitted/getBackgroundEvents.ts index 1fc483c1cb..f94e78e470 100644 --- a/packages/snaps-rpc-methods/src/permitted/getBackgroundEvents.ts +++ b/packages/snaps-rpc-methods/src/permitted/getBackgroundEvents.ts @@ -2,8 +2,8 @@ import type { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine'; import type { PermittedHandlerExport } from '@metamask/permission-controller'; import type { BackgroundEvent, + GetBackgroundEventsParams, GetBackgroundEventsResult, - JsonRpcParams, JsonRpcRequest, } from '@metamask/snaps-sdk'; import { type PendingJsonRpcResponse } from '@metamask/utils'; @@ -22,7 +22,7 @@ export type GetBackgroundEventsMethodHooks = { export const getBackgroundEventsHandler: PermittedHandlerExport< GetBackgroundEventsMethodHooks, - JsonRpcParams, + GetBackgroundEventsParams, GetBackgroundEventsResult > = { methodNames: [methodName], @@ -44,7 +44,7 @@ export const getBackgroundEventsHandler: PermittedHandlerExport< * @returns An array of background events. */ async function getGetBackgroundEventsImplementation( - _req: JsonRpcRequest, + _req: JsonRpcRequest, res: PendingJsonRpcResponse, _next: unknown, end: JsonRpcEngineEndCallback, diff --git a/packages/snaps-rpc-methods/src/permitted/scheduleBackground.test.ts b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts similarity index 65% rename from packages/snaps-rpc-methods/src/permitted/scheduleBackground.test.ts rename to packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts index 6cb46262b8..e563510489 100644 --- a/packages/snaps-rpc-methods/src/permitted/scheduleBackground.test.ts +++ b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts @@ -3,8 +3,10 @@ import type { ScheduleBackgroundEventParams, ScheduleBackgroundEventResult, } from '@metamask/snaps-sdk'; +import { MOCK_SNAP_ID } from '@metamask/snaps-utils/test-utils'; import type { JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils'; +import { SnapEndowments } from '../endowments'; import { scheduleBackgroundEventHandler } from './scheduleBackgroundEvent'; describe('snap_scheduleBackgroundEvent', () => { @@ -15,23 +17,34 @@ describe('snap_scheduleBackgroundEvent', () => { implementation: expect.any(Function), hookNames: { scheduleBackgroundEvent: true, + hasPermission: true, }, }); }); }); describe('implementation', () => { + const createOriginMiddleware = + (origin: string) => + (request: any, _response: unknown, next: () => void, _end: unknown) => { + request.origin = origin; + next(); + }; + it('returns an id after calling the `scheduleBackgroundEvent` hook', async () => { const { implementation } = scheduleBackgroundEventHandler; const scheduleBackgroundEvent = jest.fn().mockImplementation(() => 'foo'); + const hasPermission = jest.fn().mockImplementation(() => true); const hooks = { scheduleBackgroundEvent, + hasPermission, }; const engine = new JsonRpcEngine(); + engine.push(createOriginMiddleware(MOCK_SNAP_ID)); engine.push((request, response, next, end) => { const result = implementation( request as JsonRpcRequest, @@ -64,13 +77,16 @@ describe('snap_scheduleBackgroundEvent', () => { const { implementation } = scheduleBackgroundEventHandler; const scheduleBackgroundEvent = jest.fn(); + const hasPermission = jest.fn().mockImplementation(() => true); const hooks = { scheduleBackgroundEvent, + hasPermission, }; const engine = new JsonRpcEngine(); + engine.push(createOriginMiddleware(MOCK_SNAP_ID)); engine.push((request, response, next, end) => { const result = implementation( request as JsonRpcRequest, @@ -97,7 +113,6 @@ describe('snap_scheduleBackgroundEvent', () => { }); expect(scheduleBackgroundEvent).toHaveBeenCalledWith({ - scheduledAt: expect.any(String), date: '2022-01-01T01:00', request: { method: 'handleExport', @@ -106,17 +121,70 @@ describe('snap_scheduleBackgroundEvent', () => { }); }); + it('throws if a snap does not have the "endowment:cronjob" permission', async () => { + const { implementation } = scheduleBackgroundEventHandler; + + const scheduleBackgroundEvent = jest.fn(); + const hasPermission = jest.fn().mockImplementation(() => false); + + const hooks = { + scheduleBackgroundEvent, + hasPermission, + }; + + const engine = new JsonRpcEngine(); + + engine.push(createOriginMiddleware(MOCK_SNAP_ID)); + engine.push((request, response, next, end) => { + const result = implementation( + request as JsonRpcRequest, + response as PendingJsonRpcResponse, + next, + end, + hooks, + ); + + result?.catch(end); + }); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: 'snap_scheduleBackgroundEvent', + params: { + date: 'foobar', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + }); + + expect(response).toStrictEqual({ + error: { + code: -32600, + message: `The snap "${MOCK_SNAP_ID}" does not have the "${SnapEndowments.Cronjob}" permission.`, + stack: expect.any(String), + }, + id: 1, + jsonrpc: '2.0', + }); + }); + it('throws on invalid params', async () => { const { implementation } = scheduleBackgroundEventHandler; const scheduleBackgroundEvent = jest.fn(); + const hasPermission = jest.fn().mockImplementation(() => true); const hooks = { scheduleBackgroundEvent, + hasPermission, }; const engine = new JsonRpcEngine(); + engine.push(createOriginMiddleware(MOCK_SNAP_ID)); engine.push((request, response, next, end) => { const result = implementation( request as JsonRpcRequest, diff --git a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts index a88f93b3c0..a19d440fed 100644 --- a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts +++ b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts @@ -21,17 +21,18 @@ import { import { type PendingJsonRpcResponse } from '@metamask/utils'; import { DateTime } from 'luxon'; +import { SnapEndowments } from '../endowments'; import type { MethodHooksObject } from '../utils'; const methodName = 'snap_scheduleBackgroundEvent'; const hookNames: MethodHooksObject = { scheduleBackgroundEvent: true, + hasPermission: true, }; type ScheduleBackgroundEventHookParams = { date: string; - scheduledAt: string; request: CronjobRpcRequest; }; @@ -39,6 +40,8 @@ export type ScheduleBackgroundEventMethodHooks = { scheduleBackgroundEvent: ( snapEvent: ScheduleBackgroundEventHookParams, ) => string; + + hasPermission: (permissionName: string) => boolean; }; export const scheduleBackgroundEventHandler: PermittedHandlerExport< @@ -77,6 +80,7 @@ export type ScheduleBackgroundEventParameters = InferMatching< * @param end - The `json-rpc-engine` "end" callback. * @param hooks - The RPC method hooks. * @param hooks.scheduleBackgroundEvent - The function to schedule a background event. + * @param hooks.hasPermission - The function to check if a snap has the `endowment:cronjob` permission. * @returns An id representing the background event. */ async function getScheduleBackgroundEventImplementation( @@ -84,18 +88,27 @@ async function getScheduleBackgroundEventImplementation( res: PendingJsonRpcResponse, _next: unknown, end: JsonRpcEngineEndCallback, - { scheduleBackgroundEvent }: ScheduleBackgroundEventMethodHooks, + { + scheduleBackgroundEvent, + hasPermission, + }: ScheduleBackgroundEventMethodHooks, ): Promise { - const { params } = req; + const { params, origin } = req as JsonRpcRequest & { origin: string }; + + if (!hasPermission(SnapEndowments.Cronjob)) { + return end( + rpcErrors.invalidRequest({ + message: `The snap "${origin}" does not have the "${SnapEndowments.Cronjob}" permission.`, + }), + ); + } try { const validatedParams = getValidatedParams(params); const { date, request } = validatedParams; - const scheduledAt = new Date().toISOString(); - - const id = scheduleBackgroundEvent({ date, request, scheduledAt }); + const id = scheduleBackgroundEvent({ date, request }); res.result = id; } catch (error) { return end(error); diff --git a/packages/snaps-sdk/src/types/methods/get-background-events.ts b/packages/snaps-sdk/src/types/methods/get-background-events.ts index 3ce1e154d4..b35240bab6 100644 --- a/packages/snaps-sdk/src/types/methods/get-background-events.ts +++ b/packages/snaps-sdk/src/types/methods/get-background-events.ts @@ -2,6 +2,9 @@ import type { Json } from '@metamask/utils'; import type { SnapId } from '../snap'; +/** + * Backgound event type + */ export type BackgroundEvent = { id: string; scheduledAt: string; @@ -15,4 +18,12 @@ export type BackgroundEvent = { }; }; +/** + * `snap_getBackgroundEvents` result type. + */ export type GetBackgroundEventsResult = BackgroundEvent[]; + +/** + * `snao_getBackgroundEvents` params. + */ +export type GetBackgroundEventsParams = never; diff --git a/packages/snaps-sdk/src/types/methods/schedule-background-event.ts b/packages/snaps-sdk/src/types/methods/schedule-background-event.ts index 70e284fa28..1bb07bb969 100644 --- a/packages/snaps-sdk/src/types/methods/schedule-background-event.ts +++ b/packages/snaps-sdk/src/types/methods/schedule-background-event.ts @@ -1,8 +1,14 @@ import type { Cronjob } from '../permissions'; +/** + * Params for the `snap_scheduleBackgroundEvent` method. + */ export type ScheduleBackgroundEventParams = { date: string; request: Cronjob['request']; }; +/** + * `snap_scheduleBackgroundEvent` return type. + */ export type ScheduleBackgroundEventResult = string; diff --git a/yarn.lock b/yarn.lock index 23ea3b58d6..14be0e30a8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -20490,7 +20490,6 @@ __metadata: "@ts-bridge/cli": "npm:^0.6.1" "@types/jest": "npm:^27.5.1" "@types/lodash": "npm:^4" - "@types/luxon": "npm:^3" "@types/node": "npm:18.14.2" "@typescript-eslint/eslint-plugin": "npm:^5.42.1" "@typescript-eslint/parser": "npm:^6.21.0" @@ -20512,7 +20511,6 @@ __metadata: jest-silent-reporter: "npm:^0.6.0" lint-staged: "npm:^12.4.1" lodash: "npm:^4.17.21" - luxon: "npm:^3.5.0" minimatch: "npm:^7.4.1" prettier: "npm:^2.8.8" prettier-plugin-packagejson: "npm:^2.5.2"