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

fix: adjust extension permissions #1613

Closed
wants to merge 5 commits into from
Closed
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
2 changes: 1 addition & 1 deletion src/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
},
"devtools_page": "Devtools/devtools.html",
"web_accessible_resources": ["insights.html", "assessments/*", "injected/*", "background/*", "common/*", "DetailsView/*", "bundle/*"],
"permissions": ["storage", "webNavigation", "tabs", "notifications", "https://*/*", "http://*/*", "file://*/*"],
"permissions": ["storage", "webNavigation", "tabs", "notifications", "activeTab"],
"commands": {
"_execute_browser_action": {
"suggested_key": {
Expand Down
51 changes: 39 additions & 12 deletions src/tests/end-to-end/common/browser-factory.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,54 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { generateUID } from 'common/uid-generator';
import * as fs from 'fs';
import * as path from 'path';
import * as Puppeteer from 'puppeteer';
import * as util from 'util';
import { generateUID } from '../../../common/uid-generator';
import { Browser } from './browser';
import { popupPageElementIdentifiers } from './element-identifiers/popup-page-element-identifiers';
import { DEFAULT_BROWSER_LAUNCH_TIMEOUT_MS } from './timeouts';

export const chromeLogsPath = path.join(__dirname, '../../../../test-results/e2e/chrome-logs/');

export function browserLogPath(browserInstanceId: string): string {
return path.join(chromeLogsPath, browserInstanceId);
}
export const browserLogPath = (browserInstanceId: string): string => path.join(chromeLogsPath, browserInstanceId);

const fileExists = util.promisify(fs.exists);
const writeFile = util.promisify(fs.writeFile);
const readFile = util.promisify(fs.readFile);

export interface ExtensionOptions {
suppressFirstTimeDialog: boolean;
addLocalhostToPermissions?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would probably help prevent bugs for this to be non-optional; if a test wants to not have the extra permissions, it has to choose that explicitly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see it the other way around: we don't give those permissions, unless the test explicitly set this flag to true

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it should default to true, I'm saying I think it should maybe not have any default value (force every test to choose explicitly)

Copy link
Contributor Author

@smoralesd smoralesd Nov 6, 2019

Choose a reason for hiding this comment

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

It defaults to false and having optional make tests that don't need this permissions less verbose about it.

}

export async function launchBrowser(extensionOptions: ExtensionOptions): Promise<Browser> {
const browserInstanceId = generateUID();
const puppeteerBrowser = await launchNewBrowser(browserInstanceId);
const browser = new Browser(browserInstanceId, puppeteerBrowser);

// only unpacked extension paths are supported
const devExtensionPath = `${(global as any).rootDir}/drop/extension/dev/product`;
const manifestPath = getManifestPath(devExtensionPath);

let onClose: () => Promise<void>;

if (extensionOptions.addLocalhostToPermissions) {
const originalManifest = await readFile(manifestPath);

const permissiveManifest = addLocalhostPermissionsToManifest(originalManifest.toString());

await writeFile(manifestPath, permissiveManifest);

onClose = async () => await writeFile(manifestPath, originalManifest.toString());
}

const puppeteerBrowser = await launchNewBrowser(browserInstanceId, devExtensionPath);

const browser = new Browser(browserInstanceId, puppeteerBrowser, onClose);

if (extensionOptions.suppressFirstTimeDialog) {
await suppressFirstTimeUsagePrompt(browser);
}

return browser;
}

Expand All @@ -40,12 +62,12 @@ async function suppressFirstTimeUsagePrompt(browser: Browser): Promise<void> {
await popupPage.close();
}

function fileExists(filePath: string): Promise<boolean> {
return new Promise(resolve => fs.exists(filePath, resolve));
function getManifestPath(extensionPath: string): string {
return `${extensionPath}/manifest.json`;
}

async function verifyExtensionIsBuilt(extensionPath: string): Promise<void> {
const manifestPath = `${extensionPath}/manifest.json`;
const manifestPath = getManifestPath(extensionPath);
if (!(await fileExists(manifestPath))) {
throw new Error(
`Cannot launch extension-enabled browser instance because extension has not been built.\n` +
Expand All @@ -55,10 +77,15 @@ async function verifyExtensionIsBuilt(extensionPath: string): Promise<void> {
}
}

async function launchNewBrowser(browserInstanceId: string): Promise<Puppeteer.Browser> {
// only unpacked extension paths are supported
const extensionPath = `${(global as any).rootDir}/drop/extension/dev/product`;
function addLocalhostPermissionsToManifest(originalManifest: string): string {
const manifest = JSON.parse(originalManifest);

manifest['permissions'].push('http://localhost:9050/*');
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider pulling this number out of testResourceServerConfig.port


return JSON.stringify(manifest, null, 2);
}

async function launchNewBrowser(browserInstanceId: string, extensionPath: string): Promise<Puppeteer.Browser> {
// It's important that we verify this before calling Puppeteer.launch because its behavior if the
// extension can't be loaded is "the Chromium instance hangs with an alert and everything on Puppeteer's
// end shows up as a generic timeout error with no meaningful logging".
Expand Down
10 changes: 9 additions & 1 deletion src/tests/end-to-end/common/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,19 @@ export class Browser {
private memoizedBackgroundPage: BackgroundPage;
private pages: Array<Page> = [];

constructor(private readonly browserInstanceId: string, private readonly underlyingBrowser: Puppeteer.Browser) {
constructor(
private readonly browserInstanceId: string,
private readonly underlyingBrowser: Puppeteer.Browser,
private readonly onClose?: () => Promise<void>,
) {
underlyingBrowser.on('disconnected', onBrowserDisconnected);
}

public async close(): Promise<void> {
if (this.onClose) {
await this.onClose();
}

this.underlyingBrowser.removeListener('disconnected', onBrowserDisconnected);
await this.underlyingBrowser.close();
}
Expand Down
4 changes: 2 additions & 2 deletions src/tests/end-to-end/tests/details-view/headings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('Headings Page', () => {
let headingsPage: DetailsViewPage;

beforeAll(async () => {
browser = await launchBrowser({ suppressFirstTimeDialog: true });
browser = await launchBrowser({ suppressFirstTimeDialog: true, addLocalhostToPermissions: true });
targetPage = await browser.newTargetPage();
headingsPage = await openHeadingsPage(browser, targetPage);
});
Expand All @@ -39,7 +39,7 @@ describe('Headings Page', () => {
let headingsPage: DetailsViewPage;

beforeAll(async () => {
browser = await launchBrowser({ suppressFirstTimeDialog: true });
browser = await launchBrowser({ suppressFirstTimeDialog: true, addLocalhostToPermissions: true });
targetPage = await browser.newTargetPage();
headingsPage = await openHeadingsPage(browser, targetPage);
await headingsPage.enableHighContrast();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Ad hoc tools should display the pinned target page visualizations when enabling the "Automated checks" toggle 1`] = `
exports[`Ad hoc tools ad hoc toggles should display the pinned target page visualizations when enabling the "Automated checks" toggle 1`] = `
<DocumentFragment>
<div
id="insights-shadow-container"
Expand Down Expand Up @@ -59,7 +59,7 @@ exports[`Ad hoc tools should display the pinned target page visualizations when
</DocumentFragment>
`;

exports[`Ad hoc tools should display the pinned target page visualizations when enabling the "Color" toggle 1`] = `
exports[`Ad hoc tools ad hoc toggles should display the pinned target page visualizations when enabling the "Color" toggle 1`] = `
<DocumentFragment>
<div
id="insights-shadow-container"
Expand All @@ -78,7 +78,7 @@ exports[`Ad hoc tools should display the pinned target page visualizations when
</DocumentFragment>
`;

exports[`Ad hoc tools should display the pinned target page visualizations when enabling the "Headings" toggle 1`] = `
exports[`Ad hoc tools ad hoc toggles should display the pinned target page visualizations when enabling the "Headings" toggle 1`] = `
<DocumentFragment>
<div
id="insights-shadow-container"
Expand Down Expand Up @@ -125,7 +125,7 @@ exports[`Ad hoc tools should display the pinned target page visualizations when
</DocumentFragment>
`;

exports[`Ad hoc tools should display the pinned target page visualizations when enabling the "Landmarks" toggle 1`] = `
exports[`Ad hoc tools ad hoc toggles should display the pinned target page visualizations when enabling the "Landmarks" toggle 1`] = `
<DocumentFragment>
<div
id="insights-shadow-container"
Expand Down
104 changes: 62 additions & 42 deletions src/tests/end-to-end/tests/popup/adhoc-panel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,67 +12,87 @@ describe('Ad hoc tools', () => {
let targetPage: TargetPage;
let popupPage: PopupPage;

beforeEach(async () => {
browser = await launchBrowser({ suppressFirstTimeDialog: true });
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
await popupPage.bringToFront();
});

afterEach(async () => {
if (browser) {
await browser.close();
browser = undefined;
}
});

it('should have launchpad link that takes us to adhoc panel & is sticky', async () => {
await popupPage.gotoAdhocPanel();

// verify adhoc panel state is sticky
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
await popupPage.verifyAdhocPanelLoaded();
});
describe('navigation', () => {
beforeEach(async () => {
browser = await launchBrowser({ suppressFirstTimeDialog: true });
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
await popupPage.bringToFront();
});

it('should take back to Launch pad on clicking "Back to Launch pad" link & is sticky', async () => {
await popupPage.clickSelectorXPath(popupPageElementIdentifiers.adhocLaunchPadLinkXPath);
await popupPage.clickSelector(popupPageElementIdentifiers.backToLaunchPadLink);
it('should have launchpad link that takes us to adhoc panel & is sticky', async () => {
await popupPage.gotoAdhocPanel();

await popupPage.verifyLaunchPadLoaded();
// verify ad hoc panel state is sticky
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
await popupPage.verifyAdhocPanelLoaded();
});

// verify adhoc panel state is sticky
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
await popupPage.verifyLaunchPadLoaded();
});
it('should take back to Launch pad on clicking "Back to Launch pad" link & is sticky', async () => {
await popupPage.clickSelectorXPath(popupPageElementIdentifiers.adhocLaunchPadLinkXPath);
await popupPage.clickSelector(popupPageElementIdentifiers.backToLaunchPadLink);

it('should pass accessibility validation', async () => {
await popupPage.gotoAdhocPanel();
await popupPage.verifyLaunchPadLoaded();

const results = await scanForAccessibilityIssues(popupPage, '*');
expect(results).toHaveLength(0);
// verify ad hoc panel state is sticky
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
await popupPage.verifyLaunchPadLoaded();
});
});

it('should pass accessibility validation in high contrast', async () => {
const detailsViewPage = await browser.newDetailsViewPage(targetPage);
await detailsViewPage.enableHighContrast();
describe('ad hoc toggles', () => {
beforeEach(async () => {
browser = await launchBrowser({ suppressFirstTimeDialog: true, addLocalhostToPermissions: true });
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
await popupPage.bringToFront();
});

await popupPage.bringToFront();
await popupPage.gotoAdhocPanel();
it.each(['Automated checks', 'Landmarks', 'Headings', 'Color'])(
'should display the pinned target page visualizations when enabling the "%s" toggle',
async (toggleAriaLabel: string) => {
await popupPage.gotoAdhocPanel();

const results = await scanForAccessibilityIssues(popupPage, '*');
expect(results).toHaveLength(0);
await popupPage.enableToggleByAriaLabel(toggleAriaLabel);

expect(await targetPage.getShadowRootHtmlSnapshot()).toMatchSnapshot();
},
);
});

it.each(['Automated checks', 'Landmarks', 'Headings', 'Color'])(
'should display the pinned target page visualizations when enabling the "%s" toggle',
async (toggleAriaLabel: string) => {
describe('a11y scan', () => {
beforeEach(async () => {
browser = await launchBrowser({ suppressFirstTimeDialog: true, addLocalhostToPermissions: true });
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
await popupPage.bringToFront();
});

it('should pass accessibility validation', async () => {
await popupPage.gotoAdhocPanel();

await popupPage.enableToggleByAriaLabel(toggleAriaLabel);
const results = await scanForAccessibilityIssues(popupPage, '*');
expect(results).toHaveLength(0);
});

it('should pass accessibility validation in high contrast', async () => {
const detailsViewPage = await browser.newDetailsViewPage(targetPage);
await detailsViewPage.enableHighContrast();

expect(await targetPage.getShadowRootHtmlSnapshot()).toMatchSnapshot();
},
);
await popupPage.bringToFront();
await popupPage.gotoAdhocPanel();

const results = await scanForAccessibilityIssues(popupPage, '*');
expect(results).toHaveLength(0);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('Target Page issue dialog', () => {
let popupPage: PopupPage;

beforeAll(async () => {
browser = await launchBrowser({ suppressFirstTimeDialog: true });
browser = await launchBrowser({ suppressFirstTimeDialog: true, addLocalhostToPermissions: true });
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
});
Expand Down
2 changes: 1 addition & 1 deletion src/tests/end-to-end/tests/target-page/tabstop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('tabstop tests', () => {
let popupPage: PopupPage;

beforeEach(async () => {
browser = await launchBrowser({ suppressFirstTimeDialog: true });
browser = await launchBrowser({ suppressFirstTimeDialog: true, addLocalhostToPermissions: true });
targetPage = await browser.newTargetPage({ testResourcePath: 'native-widgets/input-type-radio.html' });
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('Target Page visualization boxes', () => {
let popupPage: PopupPage;

beforeEach(async () => {
browser = await launchBrowser({ suppressFirstTimeDialog: true });
browser = await launchBrowser({ suppressFirstTimeDialog: true, addLocalhostToPermissions: true });
targetPage = await browser.newTargetPage();
popupPage = await browser.newPopupPage(targetPage);
await popupPage.gotoAdhocPanel();
Expand Down