Skip to content

Commit

Permalink
validate before calling approve. some test changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@turbocrime
turbocrime committed Jun 22, 2024
1 parent ccdc050 commit d03889a
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 56 deletions.
13 changes: 9 additions & 4 deletions apps/extension/src/listeners/message-prax-request.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,21 @@ import { assertValidSender } from '../origins/valid-sender';
// listen for page connection requests.
// this is the only message we handle from an unapproved content script.
chrome.runtime.onMessage.addListener(
(req, sender, respond: (failure?: PenumbraRequestFailure) => void) => {
(req, unvalidatedSender, respond: (failure?: PenumbraRequestFailure) => void) => {
if (req !== PraxConnection.Request) return false; // instruct chrome we will not respond

void approveOrigin(sender).then(
const validSender = assertValidSender(unvalidatedSender);

void approveOrigin(validSender).then(
status => {
// user interacted with the popup, resulting in a choice.
const { documentId } = assertValidSender(sender);
if (status === UserChoice.Approved) {
// the request was succesful
respond();
void chrome.runtime.sendMessage(PraxConnection.Init, { documentId });
// init happens separately
void chrome.tabs.sendMessage(validSender.tab.id, PraxConnection.Init, {
documentId: validSender.documentId,
});
} else {
respond(PenumbraRequestFailure.Denied);
}
Expand Down
13 changes: 0 additions & 13 deletions apps/extension/src/origins/approve-origin.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ vi.mock('../popup', () => ({
popup: mockPopup,
}));

const mockAssertValidSender = vi.hoisted(() => vi.fn((id: chrome.runtime.MessageSender) => id));
vi.mock('./valid-sender', () => ({ assertValidSender: mockAssertValidSender }));

const mockTab = {
favIconUrl: 'mock://doesntmatter.example.com/mock.ico',
title: 'Mock Tab Title',
Expand All @@ -29,15 +26,6 @@ describe('origin approvals', () => {
});

describe('approveOrigin', () => {
it('throws if validation fails', async () => {
mockAssertValidSender.mockImplementationOnce(() => {
throw new Error('Mock invalid');
});
const messageSender = { origin: 'mock://inputignored.example.invalid', tab: mockTab };
await expect(approveOrigin(messageSender)).rejects.toThrow('Mock invalid');
expect(mockAssertValidSender).toHaveBeenCalled();
});

it('prompts and stores choice for a new origin', async () => {
mockLocalStorage.get.mockReturnValue(Promise.resolve([]));
const messageSender = { origin: 'mock://unknown.example.com', tab: mockTab };
Expand All @@ -53,7 +41,6 @@ describe('origin approvals', () => {
const choice = await approveOrigin(messageSender);
expect(mockLocalStorage.set).toHaveBeenCalledWith('knownSites', [newOriginRecord]);
expect(choice).toBe(UserChoice.Approved);
expect(mockAssertValidSender).toHaveBeenCalled();
});

it('returns stored choice', async () => {
Expand Down
26 changes: 17 additions & 9 deletions apps/extension/src/origins/approve-origin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { localExtStorage } from '../storage/local';
import { OriginApproval, PopupType } from '../message/popup';
import { popup } from '../popup';
import { UserChoice } from '@penumbra-zone/types/user-choice';
import { assertValidSender } from './valid-sender';
import { produce } from 'immer';
import { OriginRecord } from '../storage/types';

const getOriginRecord = async (getOrigin: string) => {
const getOriginRecord = async (getOrigin?: string) => {
if (!getOrigin) return undefined;
const knownSites = await localExtStorage.get('knownSites');
const existingRecords = knownSites.filter(record => record.origin === getOrigin);
if (!existingRecords.length) {
Expand Down Expand Up @@ -36,10 +36,18 @@ const upsertOriginRecord = async (proposal: OriginRecord) => {
export const alreadyApprovedOrigin = async (getOrigin: string): Promise<boolean> =>
getOriginRecord(getOrigin).then(r => r?.choice === UserChoice.Approved);

export const approveOrigin = async (sender: chrome.runtime.MessageSender): Promise<UserChoice> => {
const validSender = assertValidSender(sender);

const existingRecord = await getOriginRecord(validSender.origin);
/**
* Obtain the approval status of an origin, for use by connection request
* handler. Input origins should already be validated.
*
* @param sender A sender that has already been validated
* @returns The user's choice about the origin, from storage or fresh off the popup
*/
export const approveOrigin = async (sender: {
origin: string;
tab: chrome.tabs.Tab;
}): Promise<UserChoice> => {
const existingRecord = await getOriginRecord(sender.origin);

switch (existingRecord?.choice) {
case UserChoice.Approved:
Expand All @@ -51,9 +59,9 @@ export const approveOrigin = async (sender: chrome.runtime.MessageSender): Promi
const popupResponse = await popup<OriginApproval>({
type: PopupType.OriginApproval,
request: {
origin: validSender.origin,
favIconUrl: validSender.tab.favIconUrl,
title: validSender.tab.title,
origin: sender.origin,
favIconUrl: sender.tab.favIconUrl,
title: sender.tab.title,
lastRequest: existingRecord?.date,
},
});
Expand Down
35 changes: 23 additions & 12 deletions apps/extension/src/origins/valid-sender.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ describe('assertValidSender', () => {
expect(() => assertValidSender(unparseableOrigin)).toThrow('Invalid URL');
});

it('throws if sender origin contains unexpanded unicode', () => {
it("throws if sender origin can't roundtrip", () => {
const invalidOrigin: chrome.runtime.MessageSender = {
...mockValid,
// cyrillic 'а' instead of latin 'a' in origin
// cyrillic lookalike for latin 'a' in hostname
origin: 'https://exаmple.com',
};
expect(() => assertValidSender(invalidOrigin)).toThrow('Sender origin is invalid');
Expand All @@ -85,7 +85,7 @@ describe('assertValidSender', () => {
it('throws if sender origin contains path', () => {
const invalidOrigin: chrome.runtime.MessageSender = {
...mockValid,
// trailing slash is path, not origin
// trailing slash is a path, not part of an origin
origin: 'https://example.com/',
};
expect(() => assertValidSender(invalidOrigin)).toThrow('Sender origin is invalid');
Expand All @@ -107,28 +107,39 @@ describe('assertValidSender', () => {
expect(() => assertValidSender(urlless)).toThrow('Sender has no URL');
});

it('throws if sender URL contains unencoded special characters', () => {
it("throws if sender URL can't roundtrip", () => {
const invalidUrl: chrome.runtime.MessageSender = {
...mockValid,
url: 'https://example.com/some/p th',
// unicode RTL override in URL
origin: 'https://example.invalid',
url: 'https://‮sdrawkcab%[email protected]/',
};
expect(() => assertValidSender(invalidUrl)).toThrow('Sender URL is invalid');
});

it('throws if sender URL contains unexpanded unicode', () => {
const invalidUrl: chrome.runtime.MessageSender = {
it('throws if sender URL has unexpected host', () => {
const different: chrome.runtime.MessageSender = {
...mockValid,
// cyrillic 'а' instead of latin 'a' in path
url: 'https://example.com/some/pаth',
origin: 'https://example.com',
url: 'https://example.net/some/path',
};
expect(() => assertValidSender(invalidUrl)).toThrow('Sender URL is invalid');
expect(() => assertValidSender(different)).toThrow('Sender URL has unexpected origin');
});

it('throws if sender URL has unexpected origin', () => {
it('throws if sender URL has unexpected port', () => {
const different: chrome.runtime.MessageSender = {
...mockValid,
origin: 'https://example.com',
url: 'https://example.net/some/path',
url: 'https://example.com:123/some/path',
};
expect(() => assertValidSender(different)).toThrow('Sender URL has unexpected origin');
});

it('throws if sender URL is missing expected port', () => {
const different: chrome.runtime.MessageSender = {
...mockValid,
origin: 'https://example.com:999',
url: 'https://example.com/some/path',
};
expect(() => assertValidSender(different)).toThrow('Sender URL has unexpected origin');
});
Expand Down
32 changes: 14 additions & 18 deletions apps/extension/src/origins/valid-sender.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,18 @@ enum ValidProtocol {
'https:' = 'https:',
}

type ValidOrigin = `${string & ValidProtocol}//${string}`;

type ValidSender<O extends ValidOrigin> = chrome.runtime.MessageSender &
Required<{
frameId: 0;
documentId: string;
tab: chrome.tabs.Tab & { id: number };
origin: O;
url: `${O}/${string}`;
}>;

export const assertValidSender = <O extends ValidOrigin>(
sender?: chrome.runtime.MessageSender & { origin?: O | string },
) => {
type ValidSender = chrome.runtime.MessageSender & {
frameId: 0;
documentId: string;
tab: chrome.tabs.Tab & { id: number };

// the relationship between origin and url is pretty complex.
// just rely on the browser's tools.
origin: `${ValidProtocol}//${string}`;
url: `${ValidProtocol}//${string}/${string}`;
};

export const assertValidSender = (sender?: chrome.runtime.MessageSender) => {
if (!sender) throw new Error('Sender undefined');
if (!sender.tab?.id) throw new Error('Sender is not a tab');
if (sender.frameId !== 0) throw new Error('Sender is not a top-level frame');
Expand All @@ -36,12 +34,10 @@ export const assertValidSender = <O extends ValidOrigin>(
//if (!sender.tlsChannelId) throw new Error('Sender has no tlsChannelId');
//if (!sender.id) throw new Error('Sender has no crx id');

return sender as ValidSender<O>;
return sender as ValidSender;
};

export const isValidSender = <O extends ValidOrigin>(
sender: chrome.runtime.MessageSender & { origin?: string | O },
): sender is ValidSender<O> => {
export const isValidSender = (sender: chrome.runtime.MessageSender): sender is ValidSender => {
try {
return Boolean(assertValidSender(sender));
} catch {
Expand Down

0 comments on commit d03889a

Please sign in to comment.