diff --git a/packages/examples/packages/browserify-plugin/snap.manifest.json b/packages/examples/packages/browserify-plugin/snap.manifest.json index 4eea98f356..aef3381149 100644 --- a/packages/examples/packages/browserify-plugin/snap.manifest.json +++ b/packages/examples/packages/browserify-plugin/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snaps.git" }, "source": { - "shasum": "cpGbAa5I7Y1mBlhvQlq5Sj8KkB/UcsD5MEgaeFeyMmM=", + "shasum": "j2iSyKR8FljepPZvGpP82aJlN4lq/LbCOmEkj18F8OM=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/examples/packages/browserify/snap.manifest.json b/packages/examples/packages/browserify/snap.manifest.json index d554d738c3..aa616394d8 100644 --- a/packages/examples/packages/browserify/snap.manifest.json +++ b/packages/examples/packages/browserify/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snaps.git" }, "source": { - "shasum": "v1nVnxArJyII7qfswvNyO/rW2hG89ZQgyttgPAUp1r8=", + "shasum": "SHtsXe9bzVAg3N1wepgK06JIiZvDKoKxC+Acys6fzY4=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index b8739fd7e2..266c39bf23 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: 92.91, + branches: 92.85, functions: 97.23, - lines: 97.83, - statements: 97.35, + lines: 97.8, + statements: 97.31, }, }, }); diff --git a/packages/snaps-rpc-methods/src/restricted/notify.test.tsx b/packages/snaps-rpc-methods/src/restricted/notify.test.ts similarity index 56% rename from packages/snaps-rpc-methods/src/restricted/notify.test.tsx rename to packages/snaps-rpc-methods/src/restricted/notify.test.ts index cb1482ee8b..fc62b2854a 100644 --- a/packages/snaps-rpc-methods/src/restricted/notify.test.tsx +++ b/packages/snaps-rpc-methods/src/restricted/notify.test.ts @@ -1,6 +1,5 @@ import { PermissionType, SubjectType } from '@metamask/permission-controller'; import { NotificationType } from '@metamask/snaps-sdk'; -import { Box, Text } from '@metamask/snaps-sdk/jsx'; import { getImplementation, @@ -21,7 +20,6 @@ describe('snap_notify', () => { showInAppNotification: jest.fn(), isOnPhishingList: jest.fn(), maybeUpdatePhishingList: jest.fn(), - createInterface: jest.fn(), getSnap: jest.fn(), }; @@ -46,14 +44,12 @@ describe('snap_notify', () => { const isOnPhishingList = jest.fn().mockResolvedValueOnce(false); const getSnap = jest.fn(); const maybeUpdatePhishingList = jest.fn(); - const createInterface = jest.fn(); const notificationImplementation = getImplementation({ showNativeNotification, showInAppNotification, isOnPhishingList, maybeUpdatePhishingList, - createInterface, getSnap, }); @@ -74,55 +70,11 @@ describe('snap_notify', () => { }); }); - it('shows inApp notifications with a detailed view', async () => { - const showNativeNotification = jest.fn().mockResolvedValueOnce(true); - const showInAppNotification = jest.fn().mockResolvedValueOnce(true); - const isOnPhishingList = jest.fn().mockResolvedValueOnce(false); - const maybeUpdatePhishingList = jest.fn(); - const createInterface = jest.fn().mockResolvedValueOnce(1); - const getSnap = jest.fn(); - - const notificationImplementation = getImplementation({ - showNativeNotification, - showInAppNotification, - isOnPhishingList, - maybeUpdatePhishingList, - createInterface, - getSnap, - }); - - await notificationImplementation({ - context: { - origin: 'extension', - }, - method: 'snap_notify', - params: { - type: NotificationType.InApp, - message: 'Some message', - title: 'Detailed view title', - content: Hello, - }, - }); - - expect(showInAppNotification).toHaveBeenCalledWith('extension', { - type: NotificationType.InApp, - message: 'Some message', - title: 'Detailed view title', - content: 1, - }); - - expect(createInterface).toHaveBeenCalledWith( - 'extension', - Hello, - ); - }); - it('shows native notification', async () => { const showNativeNotification = jest.fn().mockResolvedValueOnce(true); const showInAppNotification = jest.fn().mockResolvedValueOnce(true); const isOnPhishingList = jest.fn().mockResolvedValueOnce(false); const maybeUpdatePhishingList = jest.fn(); - const createInterface = jest.fn(); const getSnap = jest.fn(); const notificationImplementation = getImplementation({ @@ -130,7 +82,6 @@ describe('snap_notify', () => { showInAppNotification, isOnPhishingList, maybeUpdatePhishingList, - createInterface, getSnap, }); @@ -156,7 +107,6 @@ describe('snap_notify', () => { const showInAppNotification = jest.fn().mockResolvedValueOnce(true); const isOnPhishingList = jest.fn().mockResolvedValueOnce(false); const maybeUpdatePhishingList = jest.fn(); - const createInterface = jest.fn(); const getSnap = jest.fn(); const notificationImplementation = getImplementation({ @@ -164,7 +114,6 @@ describe('snap_notify', () => { showInAppNotification, isOnPhishingList, maybeUpdatePhishingList, - createInterface, getSnap, }); @@ -190,7 +139,6 @@ describe('snap_notify', () => { const showInAppNotification = jest.fn().mockResolvedValueOnce(true); const isOnPhishingList = jest.fn().mockResolvedValueOnce(false); const maybeUpdatePhishingList = jest.fn(); - const createInterface = jest.fn(); const getSnap = jest.fn(); const notificationImplementation = getImplementation({ @@ -198,7 +146,6 @@ describe('snap_notify', () => { showInAppNotification, isOnPhishingList, maybeUpdatePhishingList, - createInterface, getSnap, }); @@ -216,91 +163,12 @@ describe('snap_notify', () => { }), ).rejects.toThrow('Must specify a valid notification "type".'); }); - }); - - describe('getValidatedParams', () => { - it('throws an error if the params is not an object', () => { - const isOnPhishingList = jest.fn().mockResolvedValue(true); - expect(() => getValidatedParams([], isOnPhishingList, jest.fn())).toThrow( - 'Expected params to be a single object.', - ); - }); - - it('throws an error if the type is missing from params object', () => { - const isOnPhishingList = jest.fn().mockResolvedValue(true); - expect(() => - getValidatedParams( - { type: undefined, message: 'Something happened.' }, - isOnPhishingList, - jest.fn(), - ), - ).toThrow('Must specify a valid notification "type".'); - }); - - it('throws an error if the message is empty', () => { - const isOnPhishingList = jest.fn().mockResolvedValue(true); - expect(() => - getValidatedParams( - { type: NotificationType.InApp, message: '' }, - isOnPhishingList, - jest.fn(), - ), - ).toThrow( - 'Must specify a non-empty string "message" less than 500 characters long.', - ); - }); - - it('throws an error if the message is not a string', () => { - const isOnPhishingList = jest.fn().mockResolvedValue(true); - expect(() => - getValidatedParams( - { type: NotificationType.InApp, message: 123 }, - isOnPhishingList, - jest.fn(), - ), - ).toThrow( - 'Must specify a non-empty string "message" less than 500 characters long.', - ); - }); - - it('throws an error if the message is larger than or equal to 50 characters for native notifications', () => { - const isOnPhishingList = jest.fn().mockResolvedValue(true); - expect(() => - getValidatedParams( - { - type: NotificationType.Native, - message: 'test'.repeat(20), - }, - isOnPhishingList, - jest.fn(), - ), - ).toThrow( - 'Must specify a non-empty string "message" less than 50 characters long.', - ); - }); - it('throws an error if the message is larger than or equal to 500 characters for in app notifications', () => { - const isOnPhishingList = jest.fn().mockResolvedValue(true); - expect(() => - getValidatedParams( - { - type: NotificationType.InApp, - message: 'test'.repeat(150), - }, - isOnPhishingList, - jest.fn(), - ), - ).toThrow( - 'Must specify a non-empty string "message" less than 500 characters long.', - ); - }); - - it('throws an error if a link in the `message` property is on the phishing list', async () => { + it('throws an error if a link is on the phishing list', async () => { const showNativeNotification = jest.fn().mockResolvedValueOnce(true); const showInAppNotification = jest.fn().mockResolvedValueOnce(true); - const isOnPhishingList = jest.fn().mockResolvedValue(true); + const isOnPhishingList = jest.fn().mockResolvedValueOnce(true); const maybeUpdatePhishingList = jest.fn(); - const createInterface = jest.fn(); const getSnap = jest.fn(); const notificationImplementation = getImplementation({ @@ -308,7 +176,6 @@ describe('snap_notify', () => { showInAppNotification, isOnPhishingList, maybeUpdatePhishingList, - createInterface, getSnap, }); @@ -319,19 +186,18 @@ describe('snap_notify', () => { }, method: 'snap_notify', params: { - type: 'inApp', + type: 'native', message: '[test link](https://foo.bar)', }, }), ).rejects.toThrow('Invalid URL: The specified URL is not allowed.'); }); - it('throws an error if a link in the `message` property is invalid', async () => { + it('throws an error if a link is invalid', async () => { const showNativeNotification = jest.fn().mockResolvedValueOnce(true); const showInAppNotification = jest.fn().mockResolvedValueOnce(true); - const isOnPhishingList = jest.fn().mockResolvedValue(true); + const isOnPhishingList = jest.fn().mockResolvedValueOnce(true); const maybeUpdatePhishingList = jest.fn(); - const createInterface = jest.fn(); const getSnap = jest.fn(); const notificationImplementation = getImplementation({ @@ -339,7 +205,6 @@ describe('snap_notify', () => { showInAppNotification, isOnPhishingList, maybeUpdatePhishingList, - createInterface, getSnap, }); @@ -350,7 +215,7 @@ describe('snap_notify', () => { }, method: 'snap_notify', params: { - type: 'inApp', + type: 'native', message: '[test](http://foo.bar)', }, }), @@ -358,94 +223,63 @@ describe('snap_notify', () => { 'Invalid URL: Protocol must be one of: https:, mailto:, metamask:.', ); }); + }); - it('throws an error if a link in the `footerLink` property is on the phishing list', async () => { - const showNativeNotification = jest.fn().mockResolvedValueOnce(true); - const showInAppNotification = jest.fn().mockResolvedValueOnce(true); - const isOnPhishingList = jest.fn().mockResolvedValue(true); - const maybeUpdatePhishingList = jest.fn(); - const createInterface = jest.fn(); - const getSnap = jest.fn(); - - const notificationImplementation = getImplementation({ - showNativeNotification, - showInAppNotification, - isOnPhishingList, - maybeUpdatePhishingList, - createInterface, - getSnap, - }); - - const content = ( - - Hello, world! - + describe('getValidatedParams', () => { + it('throws an error if the params is not an object', () => { + expect(() => getValidatedParams([])).toThrow( + 'Expected params to be a single object.', ); + }); - await expect( - notificationImplementation({ - context: { - origin: 'extension', - }, - method: 'snap_notify', - params: { - type: 'inApp', - message: 'message', - footerLink: { href: 'https://www.metamask.io', text: 'test' }, - title: 'A title', - content, - }, - }), - ).rejects.toThrow('Invalid URL: The specified URL is not allowed.'); + it('throws an error if the type is missing from params object', () => { + expect(() => + getValidatedParams({ type: undefined, message: 'Something happened.' }), + ).toThrow('Must specify a valid notification "type".'); }); - it('throws an error if a link in the `footerLink` property is invalid', async () => { - const showNativeNotification = jest.fn().mockResolvedValueOnce(true); - const showInAppNotification = jest.fn().mockResolvedValueOnce(true); - const isOnPhishingList = jest.fn().mockResolvedValue(true); - const maybeUpdatePhishingList = jest.fn(); - const createInterface = jest.fn(); - const getSnap = jest.fn(); + it('throws an error if the message is empty', () => { + expect(() => + getValidatedParams({ type: NotificationType.InApp, message: '' }), + ).toThrow( + 'Must specify a non-empty string "message" less than 500 characters long.', + ); + }); - const notificationImplementation = getImplementation({ - showNativeNotification, - showInAppNotification, - isOnPhishingList, - maybeUpdatePhishingList, - createInterface, - getSnap, - }); + it('throws an error if the message is not a string', () => { + expect(() => + getValidatedParams({ type: NotificationType.InApp, message: 123 }), + ).toThrow( + 'Must specify a non-empty string "message" less than 500 characters long.', + ); + }); - const content = ( - - Hello, world! - + it('throws an error if the message is larger than or equal to 50 characters for native notifications', () => { + expect(() => + getValidatedParams({ + type: NotificationType.Native, + message: + 'test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg', + }), + ).toThrow( + 'Must specify a non-empty string "message" less than 50 characters long.', ); + }); - await expect( - notificationImplementation({ - context: { - origin: 'extension', - }, - method: 'snap_notify', - params: { - type: 'inApp', - message: 'message', - footerLink: { href: 'http://foo.bar', text: 'test' }, - title: 'A title', - content, - }, + it('throws an error if the message is larger than or equal to 500 characters for in app notifications', () => { + expect(() => + getValidatedParams({ + type: NotificationType.InApp, + message: + 'test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg', }), - ).rejects.toThrow( - 'Invalid params: Invalid URL: Protocol must be one of: https:, mailto:, metamask:.', + ).toThrow( + 'Must specify a non-empty string "message" less than 500 characters long.', ); }); it('returns valid parameters', () => { - const isNotOnPhishingList = jest.fn().mockResolvedValueOnce(false); - expect( - getValidatedParams(validParams, isNotOnPhishingList, jest.fn()), - ).toStrictEqual(validParams); + expect(getValidatedParams(validParams)).toStrictEqual(validParams); }); }); }); diff --git a/packages/snaps-rpc-methods/src/restricted/notify.ts b/packages/snaps-rpc-methods/src/restricted/notify.ts index 5f5feb36c8..86d551951a 100644 --- a/packages/snaps-rpc-methods/src/restricted/notify.ts +++ b/packages/snaps-rpc-methods/src/restricted/notify.ts @@ -5,67 +5,31 @@ import type { } from '@metamask/permission-controller'; import { PermissionType, SubjectType } from '@metamask/permission-controller'; import { rpcErrors } from '@metamask/rpc-errors'; -import { enumValue, NotificationType, union } from '@metamask/snaps-sdk'; +import { NotificationType } from '@metamask/snaps-sdk'; import type { NotifyParams, NotifyResult, - NotificationComponent, + EnumToUnion, } from '@metamask/snaps-sdk'; -import { NotificationComponentsStruct } from '@metamask/snaps-sdk/jsx'; -import { - createUnion, - validateLink, - validateTextLinks, - type Snap, -} from '@metamask/snaps-utils'; -import type { InferMatching } from '@metamask/snaps-utils'; -import { object, string } from '@metamask/superstruct'; +import { type Snap, validateTextLinks } from '@metamask/snaps-utils'; import type { NonEmptyArray } from '@metamask/utils'; -import { hasProperty, isObject } from '@metamask/utils'; +import { isObject } from '@metamask/utils'; import { type MethodHooksObject } from '../utils'; const methodName = 'snap_notify'; -const NativeNotificationStruct = object({ - type: enumValue(NotificationType.Native), - message: string(), -}); - -const InAppNotificationStruct = object({ - type: enumValue(NotificationType.InApp), - message: string(), -}); - -const InAppNotificationWithDetailsStruct = object({ - type: enumValue(NotificationType.InApp), - message: string(), - content: NotificationComponentsStruct, - title: string(), -}); - -const InAppNotificationWithDetailsAndFooterStruct = object({ - type: enumValue(NotificationType.InApp), - message: string(), - content: NotificationComponentsStruct, - title: string(), - footerLink: object({ - href: string(), - text: string(), - }), -}); - -const NotificationParametersStruct = union([ - InAppNotificationStruct, - InAppNotificationWithDetailsStruct, - InAppNotificationWithDetailsAndFooterStruct, - NativeNotificationStruct, -]); +export type NotificationArgs = { + /** + * Enum type to determine notification type. + */ + type: EnumToUnion; -export type NotificationParameters = InferMatching< - typeof NotificationParametersStruct, - NotifyParams ->; + /** + * A message to show on the notification. + */ + message: string; +}; export type NotifyMethodHooks = { /** @@ -74,7 +38,7 @@ export type NotifyMethodHooks = { */ showNativeNotification: ( snapId: string, - args: NotificationParameters, + args: NotificationArgs, ) => Promise; /** @@ -83,17 +47,13 @@ export type NotifyMethodHooks = { */ showInAppNotification: ( snapId: string, - args: NotificationParameters, + args: NotificationArgs, ) => Promise; isOnPhishingList: (url: string) => boolean; maybeUpdatePhishingList: () => Promise; - createInterface: ( - origin: string, - content: NotificationComponent, - ) => Promise; getSnap: (snapId: string) => Snap | undefined; }; @@ -137,7 +97,6 @@ const methodHooks: MethodHooksObject = { showInAppNotification: true, isOnPhishingList: true, maybeUpdatePhishingList: true, - createInterface: true, getSnap: true, }; @@ -155,7 +114,6 @@ export const notifyBuilder = Object.freeze({ * @param hooks.showInAppNotification - A function that shows a notification in the MetaMask UI. * @param hooks.isOnPhishingList - A function that checks for links against the phishing list. * @param hooks.maybeUpdatePhishingList - A function that updates the phishing list if needed. - * @param hooks.createInterface - A function that creates the interface in SnapInterfaceController. * @param hooks.getSnap - A function that checks if a snap is installed. * @returns The method implementation which returns `null` on success. * @throws If the params are invalid. @@ -165,7 +123,6 @@ export function getImplementation({ showInAppNotification, isOnPhishingList, maybeUpdatePhishingList, - createInterface, getSnap, }: NotifyMethodHooks) { return async function implementation( @@ -176,22 +133,11 @@ export function getImplementation({ context: { origin }, } = args; - await maybeUpdatePhishingList(); + const validatedParams = getValidatedParams(params); - const validatedParams = getValidatedParams( - params, - isOnPhishingList, - getSnap, - ); + await maybeUpdatePhishingList(); - let id; - if (hasProperty(validatedParams, 'content')) { - id = await createInterface( - origin, - validatedParams.content as NotificationComponent, - ); - validatedParams.content = id; - } + validateTextLinks(validatedParams.message, isOnPhishingList, getSnap); switch (validatedParams.type) { case NotificationType.Native: @@ -211,16 +157,9 @@ export function getImplementation({ * type. Throws if validation fails. * * @param params - The unvalidated params object from the method request. - * @param isOnPhishingList - The function that checks for links against the phishing list. - * @param getSnap - A function that checks if a snap is installed. * @returns The validated method parameter object. - * @throws If the params are invalid. */ -export function getValidatedParams( - params: unknown, - isOnPhishingList: NotifyMethodHooks['isOnPhishingList'], - getSnap: NotifyMethodHooks['getSnap'], -): NotifyParams { +export function getValidatedParams(params: unknown): NotifyParams { if (!isObject(params)) { throw rpcErrors.invalidParams({ message: 'Expected params to be a single object.', @@ -261,28 +200,5 @@ export function getValidatedParams( }); } - try { - const validatedParams = createUnion( - params, - NotificationParametersStruct, - 'type', - ); - - validateTextLinks(validatedParams.message, isOnPhishingList, getSnap); - - if (hasProperty(validatedParams, 'footerLink')) { - validateTextLinks( - validatedParams.footerLink.text, - isOnPhishingList, - getSnap, - ); - validateLink(validatedParams.footerLink.href, isOnPhishingList, getSnap); - } - - return validatedParams; - } catch (error) { - throw rpcErrors.invalidParams({ - message: `Invalid params: ${error.message}`, - }); - } + return params as NotificationArgs; } diff --git a/packages/snaps-sdk/src/jsx/index.ts b/packages/snaps-sdk/src/jsx/index.ts index 651d253e37..b5cb2e6a51 100644 --- a/packages/snaps-sdk/src/jsx/index.ts +++ b/packages/snaps-sdk/src/jsx/index.ts @@ -11,5 +11,4 @@ export { BoxChildStruct, FormChildStruct, FieldChildUnionStruct, - NotificationComponentsStruct, } from './validation'; diff --git a/packages/snaps-sdk/src/jsx/validation.test.tsx b/packages/snaps-sdk/src/jsx/validation.test.tsx index 9cf87ac8e3..cb39566aee 100644 --- a/packages/snaps-sdk/src/jsx/validation.test.tsx +++ b/packages/snaps-sdk/src/jsx/validation.test.tsx @@ -69,7 +69,6 @@ import { IconStruct, SelectorStruct, SectionStruct, - NotificationComponentsStruct, AvatarStruct, } from './validation'; @@ -599,87 +598,6 @@ describe('BoxStruct', () => { }); }); -describe('NotificationComponentsStruct', () => { - it.each([ - - foo - , - - foo - bar - , - - foo - - alt - - , - - foo - - alt - - , - - foo - - alt - - , - - Foo - {[1, 2, 3, 4, 5].map((value) => ( - {value.toString()} - ))} - , - foo, - - alt - , - ])( - "validates content returned for a notification's detailed view", - (value) => { - expect(is(value, NotificationComponentsStruct)).toBe(true); - }, - ); - - it.each([ - 'foo', - 42, - null, - undefined, - {}, - [], - // @ts-expect-error - Invalid props. - - foo - - alt - - , - // @ts-expect-error - Invalid props. - - foo - - alt - - , - -
- - - - -
-
, - - - , - ])('does not validate "%p"', (value) => { - expect(is(value, NotificationComponentsStruct)).toBe(false); - }); -}); - describe('FooterStruct', () => { it.each([