Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for metamask: schemed URLs #2719

Merged
merged 29 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7710379
Add navigation to button
hmalik88 Sep 12, 2024
b1c97e9
update shasums
hmalik88 Sep 12, 2024
393094c
Merge branch 'main' into hm/add-navigation-to-button
hmalik88 Sep 12, 2024
cf1b918
recalculate shasums
hmalik88 Sep 12, 2024
38e10b5
Merge branch 'main' into hm/add-navigation-to-button
hmalik88 Sep 12, 2024
8a28736
Merge branch 'main' into hm/add-navigation-to-button
hmalik88 Sep 13, 2024
a531f15
add icon as link child, introduce metamask urls
hmalik88 Sep 13, 2024
432a509
add getSnap function for updated validation function signature
hmalik88 Sep 13, 2024
3b3ebdd
update link validation functions to check for metamask scheme
hmalik88 Sep 13, 2024
dabf345
update validate link call
hmalik88 Sep 13, 2024
b732264
recalculate shasums
hmalik88 Sep 13, 2024
1ef2fc6
update tests
hmalik88 Sep 13, 2024
082292e
fix lint issue
hmalik88 Sep 13, 2024
870f5de
add more tests, fix logic
hmalik88 Sep 13, 2024
c672e2b
made some changes per review
hmalik88 Sep 16, 2024
b211949
Merge branch 'main' into hm/add-navigation-to-button
hmalik88 Sep 16, 2024
eaa563b
make changes per comments
hmalik88 Sep 17, 2024
bb80c95
Merge branch 'main' into hm/add-navigation-to-button
hmalik88 Sep 17, 2024
396bc37
add note to jsdoc
hmalik88 Sep 17, 2024
9b5f00b
make changes per review
hmalik88 Sep 19, 2024
2dcc0fe
Merge branch 'main' into hm/add-navigation-to-button
hmalik88 Sep 19, 2024
34cb66a
update jsdoc
hmalik88 Sep 19, 2024
28089ac
Merge branch 'main' into hm/add-navigation-to-button
hmalik88 Sep 23, 2024
e6009c3
rebuild
hmalik88 Sep 23, 2024
972329b
apply code review
hmalik88 Sep 24, 2024
cc5b1e9
fix order
hmalik88 Sep 24, 2024
f361fd6
fix test
hmalik88 Sep 24, 2024
f92e768
fix coverage
hmalik88 Sep 24, 2024
11bfb5b
fix coverage again
hmalik88 Sep 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snaps.git"
},
"source": {
"shasum": "oqO5wicjNs9i86WgNV2vx79nOYYD/uxnOtAOECtWqzU=",
"shasum": "PHfXj/qdxoIA1JIdIqlLMztPr22RMiGQlKI2RKg2DPE=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/examples/packages/browserify/snap.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snaps.git"
},
"source": {
"shasum": "LQJI6F5ptgr0wY9vQ+58Q8Tv8VyMx/8lvkDSmDgdqVM=",
"shasum": "tYGbR3Ru8MxnikWI/ZwUljGviLhF3P60NpCSII7vnUI=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/examples/packages/dialogs/snap.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snaps.git"
},
"source": {
"shasum": "zuBFS3XWpLP7vKsIJqiecvXakBW9rgzuVToBRS0ItSg=",
"shasum": "v2oIs3UY5j/3dXW5eRh3rg7Tv5qcVzHon7qzPQeRsAE=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/examples/packages/file-upload/snap.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snaps.git"
},
"source": {
"shasum": "exoRIe26MvGPe9XZxn8uq/xBn8uYvzvn0y6L5uTPlKM=",
"shasum": "z4vMdrs40TdVE+vk7sPruIvWi0q669V+p3jc6WQIib4=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/examples/packages/home-page/snap.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snaps.git"
},
"source": {
"shasum": "x94obetFeJZPEwAU/DsNvFj5/eotAwlylCGS7IfM+84=",
"shasum": "emnlXHvrhsGBMi/y1IPhiQPX9ogHmjmOU/iujBu5GB8=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snaps.git"
},
"source": {
"shasum": "iN1fXS+FpkQR21mGkHO0N+xo4nW3HFwsfXOsGTpiM68=",
"shasum": "uOuMArCEwmOmCl0Bl1WnRRm+DKcq0Y+O+5n8Z1KBMr8=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/examples/packages/jsx/snap.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snaps.git"
},
"source": {
"shasum": "Rv/yARDSnZz9twUQ/mUWpNFSjjRkNy68DUy2gTwq+ZA=",
"shasum": "3U9+Lvmmdc9JRmaHLcwGgi9lpnaj25joBF9+zXY4644=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { assert } from '@metamask/utils';
import { castDraft } from 'immer';
import { nanoid } from 'nanoid';

import type { GetSnap } from '../snaps';
import {
constructState,
getJsxInterface,
Expand Down Expand Up @@ -74,7 +75,8 @@ export type SnapInterfaceControllerAllowedActions =
| TestOrigin
| MaybeUpdateState
| HasApprovalRequest
| AcceptRequest;
| AcceptRequest
| GetSnap;

export type SnapInterfaceControllerActions =
| CreateInterface
Expand Down Expand Up @@ -379,6 +381,10 @@ export class SnapInterfaceController extends BaseController<
);

await this.#triggerPhishingListUpdate();
validateJsxLinks(element, this.#checkPhishingList.bind(this));
validateJsxLinks(
element,
this.#checkPhishingList.bind(this),
(id: string) => this.messagingSystem.call('SnapController:get', id),
);
}
}
15 changes: 14 additions & 1 deletion packages/snaps-rpc-methods/src/restricted/notify.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('snap_notify', () => {
showInAppNotification: jest.fn(),
isOnPhishingList: jest.fn(),
maybeUpdatePhishingList: jest.fn(),
getSnap: jest.fn(),
};

expect(
Expand All @@ -41,13 +42,15 @@ describe('snap_notify', () => {
const showNativeNotification = jest.fn().mockResolvedValueOnce(true);
const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
const isOnPhishingList = jest.fn().mockResolvedValueOnce(false);
const getSnap = jest.fn();
const maybeUpdatePhishingList = jest.fn();

const notificationImplementation = getImplementation({
showNativeNotification,
showInAppNotification,
isOnPhishingList,
maybeUpdatePhishingList,
getSnap,
});

await notificationImplementation({
Expand All @@ -72,12 +75,14 @@ describe('snap_notify', () => {
const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
const isOnPhishingList = jest.fn().mockResolvedValueOnce(false);
const maybeUpdatePhishingList = jest.fn();
const getSnap = jest.fn();

const notificationImplementation = getImplementation({
showNativeNotification,
showInAppNotification,
isOnPhishingList,
maybeUpdatePhishingList,
getSnap,
});

await notificationImplementation({
Expand All @@ -102,12 +107,14 @@ describe('snap_notify', () => {
const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
const isOnPhishingList = jest.fn().mockResolvedValueOnce(false);
const maybeUpdatePhishingList = jest.fn();
const getSnap = jest.fn();

const notificationImplementation = getImplementation({
showNativeNotification,
showInAppNotification,
isOnPhishingList,
maybeUpdatePhishingList,
getSnap,
});

await notificationImplementation({
Expand All @@ -132,12 +139,14 @@ describe('snap_notify', () => {
const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
const isOnPhishingList = jest.fn().mockResolvedValueOnce(false);
const maybeUpdatePhishingList = jest.fn();
const getSnap = jest.fn();

const notificationImplementation = getImplementation({
showNativeNotification,
showInAppNotification,
isOnPhishingList,
maybeUpdatePhishingList,
getSnap,
});

await expect(
Expand All @@ -160,12 +169,14 @@ describe('snap_notify', () => {
const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
const isOnPhishingList = jest.fn().mockResolvedValueOnce(true);
const maybeUpdatePhishingList = jest.fn();
const getSnap = jest.fn();

const notificationImplementation = getImplementation({
showNativeNotification,
showInAppNotification,
isOnPhishingList,
maybeUpdatePhishingList,
getSnap,
});

await expect(
Expand All @@ -187,12 +198,14 @@ describe('snap_notify', () => {
const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
const isOnPhishingList = jest.fn().mockResolvedValueOnce(true);
const maybeUpdatePhishingList = jest.fn();
const getSnap = jest.fn();

const notificationImplementation = getImplementation({
showNativeNotification,
showInAppNotification,
isOnPhishingList,
maybeUpdatePhishingList,
getSnap,
});

await expect(
Expand All @@ -207,7 +220,7 @@ describe('snap_notify', () => {
},
}),
).rejects.toThrow(
'Invalid URL: Protocol must be one of: https:, mailto:.',
'Invalid URL: Protocol must be one of: https:, mailto:, metamask:.',
);
});
});
Expand Down
9 changes: 7 additions & 2 deletions packages/snaps-rpc-methods/src/restricted/notify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {
NotifyResult,
EnumToUnion,
} from '@metamask/snaps-sdk';
import { validateTextLinks } from '@metamask/snaps-utils';
import { type Snap, validateTextLinks } from '@metamask/snaps-utils';
import type { NonEmptyArray } from '@metamask/utils';
import { isObject } from '@metamask/utils';

Expand Down Expand Up @@ -53,6 +53,8 @@ export type NotifyMethodHooks = {
isOnPhishingList: (url: string) => boolean;

maybeUpdatePhishingList: () => Promise<void>;

getSnap: (snapId: string) => Snap | undefined;
};

type SpecificationBuilderOptions = {
Expand Down Expand Up @@ -95,6 +97,7 @@ const methodHooks: MethodHooksObject<NotifyMethodHooks> = {
showInAppNotification: true,
isOnPhishingList: true,
maybeUpdatePhishingList: true,
getSnap: true,
};

export const notifyBuilder = Object.freeze({
Expand All @@ -111,6 +114,7 @@ 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.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.
*/
Expand All @@ -119,6 +123,7 @@ export function getImplementation({
showInAppNotification,
isOnPhishingList,
maybeUpdatePhishingList,
getSnap,
}: NotifyMethodHooks) {
return async function implementation(
args: RestrictedMethodOptions<NotifyParams>,
Expand All @@ -132,7 +137,7 @@ export function getImplementation({

await maybeUpdatePhishingList();

validateTextLinks(validatedParams.message, isOnPhishingList);
validateTextLinks(validatedParams.message, isOnPhishingList, getSnap);

switch (validatedParams.type) {
case NotificationType.Native:
Expand Down
22 changes: 22 additions & 0 deletions packages/snaps-sdk/src/jsx/components/Link.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Icon } from './Icon';
import { Link } from './Link';

describe('Link', () => {
Expand Down Expand Up @@ -27,6 +28,27 @@ describe('Link', () => {
});
});

it('renders a link with an icon', () => {
const result = (
<Link href="metamask://client/">
<Icon name="arrow-left" size="md" />
</Link>
);

expect(result).toStrictEqual({
type: 'Link',
key: null,
props: {
href: 'metamask://client/',
children: {
type: 'Icon',
key: null,
props: { name: 'arrow-left', size: 'md' },
},
},
});
});

it('renders a link with a conditional value', () => {
const result = (
<Link href="https://example.com">
Expand Down
5 changes: 4 additions & 1 deletion packages/snaps-sdk/src/jsx/components/Link.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import type { SnapsChildren } from '../component';
import { createSnapComponent } from '../component';
import type { StandardFormattingElement } from './formatting';
import { type IconElement } from './Icon';

/**
* The children of the {@link Link} component.
*/
export type LinkChildren = SnapsChildren<string | StandardFormattingElement>;
export type LinkChildren = SnapsChildren<
string | StandardFormattingElement | IconElement
hmalik88 marked this conversation as resolved.
Show resolved Hide resolved
>;

/**
* The props of the {@link Link} component.
Expand Down
13 changes: 0 additions & 13 deletions packages/snaps-sdk/src/jsx/components/form/Button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,4 @@ describe('Button', () => {
key: null,
});
});

it('returns a button element with navigation props', () => {
const result = <Button navigateTo="home">bar</Button>;

expect(result).toStrictEqual({
type: 'Button',
props: {
children: 'bar',
navigateTo: 'home',
},
key: null,
});
});
});
7 changes: 0 additions & 7 deletions packages/snaps-sdk/src/jsx/components/form/Button.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
import type { EnumToUnion } from 'src/internals';

import type { SnapsChildren, StringElement } from '../../component';
import { createSnapComponent } from '../../component';
import type { IconElement } from '../Icon';
import type { ImageElement } from '../Image';

export enum SnapRoutes {
Home = 'home',
}

// TODO: Add the `onClick` prop to the `ButtonProps` type.

/**
Expand All @@ -31,7 +25,6 @@ export type ButtonProps = {
variant?: 'primary' | 'destructive' | undefined;
disabled?: boolean | undefined;
form?: string | undefined;
navigateTo?: EnumToUnion<SnapRoutes> | undefined;
};

const TYPE = 'Button';
Expand Down
17 changes: 8 additions & 9 deletions packages/snaps-sdk/src/jsx/validation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ describe('ButtonStruct', () => {
<Image src="<svg></svg>" />
</Button>,
<Button form="foo">bar</Button>,
<Button navigateTo="home">Go home</Button>,
])('validates a button element', (value) => {
expect(is(value, ButtonStruct)).toBe(true);
});
Expand All @@ -193,8 +192,6 @@ describe('ButtonStruct', () => {
<Row label="label">
<Image src="<svg />" alt="alt" />
</Row>,
// @ts-expect-error - Invalid props.
<Button navigateTo="foo">Go foo</Button>,
])('does not validate "%p"', (value) => {
expect(is(value, ButtonStruct)).toBe(false);
});
Expand Down Expand Up @@ -994,12 +991,14 @@ describe('ImageStruct', () => {
});

describe('LinkStruct', () => {
it.each([<Link href="https://example.com">foo</Link>])(
'validates a link element',
(value) => {
expect(is(value, LinkStruct)).toBe(true);
},
);
it.each([
<Link href="https://example.com">foo</Link>,
<Link href="metamask://client/">
<Icon name="arrow-left" size="md" />
</Link>,
])('validates a link element', (value) => {
expect(is(value, LinkStruct)).toBe(true);
});

it.each([
'foo',
Expand Down
3 changes: 1 addition & 2 deletions packages/snaps-sdk/src/jsx/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ export const ButtonStruct: Describe<ButtonElement> = element('Button', {
variant: optional(nullUnion([literal('primary'), literal('destructive')])),
disabled: optional(boolean()),
form: optional(string()),
navigateTo: optional(nullUnion([literal('home')])),
});

/**
Expand Down Expand Up @@ -591,7 +590,7 @@ export const HeadingStruct: Describe<HeadingElement> = element('Heading', {
*/
export const LinkStruct: Describe<LinkElement> = element('Link', {
href: string(),
children: children([FormattingStruct, string()]),
children: children([FormattingStruct, string(), IconStruct]),
});

/**
Expand Down
Loading
Loading