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

feat: allow wildcards in allowedOrigins #2458

Merged
merged 6 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions packages/snaps-utils/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 96.7,
"functions": 98.72,
"lines": 98.81,
"statements": 94.79
"branches": 96.73,
"functions": 98.75,
"lines": 98.83,
"statements": 94.85
}
83 changes: 83 additions & 0 deletions packages/snaps-utils/src/json-rpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ describe('assertIsRpcOrigins', () => {
'Invalid JSON-RPC origins: Must specify at least one JSON-RPC origin.',
);
});

it('throws if allowedOrigins contains too many wildcards', () => {
expect(() =>
assertIsRpcOrigins({ allowedOrigins: ['*.*.metamask.***'] }),
).toThrow(
'Invalid JSON-RPC origins: At path: allowedOrigins.0 -- No more than two wildcards ("*") are allowed in an origin specifier.',
);
});
});

describe('assertIsKeyringOrigin', () => {
Expand Down Expand Up @@ -152,6 +160,81 @@ describe('isOriginAllowed', () => {
expect(isOriginAllowed(origins, SubjectType.Snap, 'foo')).toBe(false);
expect(isOriginAllowed(origins, SubjectType.Website, 'bar')).toBe(false);
});

it('supports wildcards', () => {
const origins: RpcOrigins = {
allowedOrigins: ['*'],
};

expect(isOriginAllowed(origins, SubjectType.Snap, 'foo')).toBe(true);
expect(isOriginAllowed(origins, SubjectType.Website, 'bar')).toBe(true);
});

it('supports prefixes with wildcards', () => {
const origins: RpcOrigins = {
allowedOrigins: ['https://*', 'npm:*'],
};

expect(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are maybe a bit more readable with it.each.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't find it.each that readable personally tbh 😅

isOriginAllowed(
origins,
SubjectType.Website,
'https://snaps.metamask.io',
),
).toBe(true);
expect(isOriginAllowed(origins, SubjectType.Snap, 'npm:filsnap')).toBe(
true,
);
expect(
isOriginAllowed(origins, SubjectType.Website, 'http://snaps.metamask.io'),
).toBe(false);
expect(
isOriginAllowed(origins, SubjectType.Snap, 'local:http://localhost:8080'),
).toBe(false);
});

it('supports partial strings with wildcards', () => {
const origins: RpcOrigins = {
allowedOrigins: ['*.metamask.io'],
};

expect(
isOriginAllowed(
origins,
SubjectType.Website,
'https://snaps.metamask.io',
),
).toBe(true);
expect(
isOriginAllowed(origins, SubjectType.Website, 'https://foo.metamask.io'),
).toBe(true);
});

it('supports multiple wildcards', () => {
const origins: RpcOrigins = {
allowedOrigins: ['*.metamask.*'],
};

expect(
isOriginAllowed(
origins,
SubjectType.Website,
'https://snaps.metamask.io',
),
).toBe(true);
expect(
isOriginAllowed(origins, SubjectType.Website, 'https://foo.metamask.io'),
).toBe(true);
expect(
isOriginAllowed(origins, SubjectType.Website, 'https://foo.metamask.dk'),
).toBe(true);
expect(
isOriginAllowed(origins, SubjectType.Website, 'https://foo.metamask2.io'),
).toBe(false);
expect(
isOriginAllowed(origins, SubjectType.Website, 'https://ametamask2.io'),
).toBe(false);
});
});

describe('assertIsJsonRpcSuccess', () => {
Expand Down
59 changes: 56 additions & 3 deletions packages/snaps-utils/src/json-rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,22 @@ import {
import type { Infer } from 'superstruct';
import { array, boolean, object, optional, refine, string } from 'superstruct';

const AllowedOriginsStruct = array(
refine(string(), 'Allowed origin', (value) => {
const wildcards = value.split('*').length - 1;
if (wildcards > 2) {
return 'No more than two wildcards ("*") are allowed in an origin specifier.';
}

return true;
}),
);

export const RpcOriginsStruct = refine(
object({
dapps: optional(boolean()),
snaps: optional(boolean()),
allowedOrigins: optional(array(string())),
allowedOrigins: optional(AllowedOriginsStruct),
}),
'RPC origins',
(value) => {
Expand Down Expand Up @@ -58,7 +69,7 @@ export function assertIsRpcOrigins(
}

export const KeyringOriginsStruct = object({
allowedOrigins: optional(array(string())),
allowedOrigins: optional(AllowedOriginsStruct),
});

export type KeyringOrigins = Infer<typeof KeyringOriginsStruct>;
Expand All @@ -84,6 +95,44 @@ export function assertIsKeyringOrigins(
);
}

/**
* Create regular expression for matching against an origin while allowing wildcards.
*
* The "*" symbol is treated as a wildcard and will match 0 or more characters.
*
* @param matcher - The string to create the regular expression with.
* @returns The regular expression.
*/
function createOriginRegExp(matcher: string) {
// Escape potential Regex characters
const escaped = matcher.replace(/[.*+?^${}()|[\]\\]/gu, '\\$&');
// Support wildcards
const regex = escaped.replace(/\*/gu, '.*');
return RegExp(regex, 'u');
}

/**
* Check whether an origin is allowed or not using a matcher string.
*
* The matcher string may be a specific origin to match or include wildcards.
* The "*" symbol is treated as a wildcard and will match 0 or more characters.
* Note: this means that https://*metamask.io matches both https://metamask.io
* and https://snaps.metamask.io.
*
* @param matcher - The matcher string.
* @param origin - The origin.
* @returns Whether the origin is allowed.
*/
function checkAllowedOrigin(matcher: string, origin: string) {
// If the matcher is a single wildcard or identical to the origin we can return true immediately.
if (matcher === '*' || matcher === origin) {
return true;
}

const regex = createOriginRegExp(matcher);
return regex.test(origin);
}

/**
* Check if the given origin is allowed by the given JSON-RPC origins object.
*
Expand All @@ -103,7 +152,11 @@ export function isOriginAllowed(
}

// If the origin is in the `allowedOrigins` list, it is allowed.
if (origins.allowedOrigins?.includes(origin)) {
if (
origins.allowedOrigins?.some((matcher) =>
checkAllowedOrigin(matcher, origin),
)
) {
return true;
}

Expand Down
Loading