Skip to content

Commit

Permalink
fix(permissions): browserContext.grantPermissions to respect the orig…
Browse files Browse the repository at this point in the history
…in (#3542)

Due to wrong type usage, we ignored the origin while granting permissions.
Switching to generated types revealed this issue. We should follow up
with switching all dispatchers to the generated types.
  • Loading branch information
dgozman authored Aug 20, 2020
1 parent 9f3a1b5 commit 4c56354
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 28 deletions.
16 changes: 8 additions & 8 deletions src/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,17 +123,17 @@ export abstract class BrowserContext extends EventEmitter {
this._doExposeBinding(binding);
}

async grantPermissions(permissions: string[], options?: { origin?: string }) {
let origin = '*';
if (options && options.origin) {
const url = new URL(options.origin);
origin = url.origin;
async grantPermissions(permissions: string[], origin?: string) {
let resolvedOrigin = '*';
if (origin) {
const url = new URL(origin);
resolvedOrigin = url.origin;
}
const existing = new Set(this._permissions.get(origin) || []);
const existing = new Set(this._permissions.get(resolvedOrigin) || []);
permissions.forEach(p => existing.add(p));
const list = [...existing.values()];
this._permissions.set(origin, list);
await this._doGrantPermissions(origin, list);
this._permissions.set(resolvedOrigin, list);
await this._doGrantPermissions(resolvedOrigin, list);
}

async clearPermissions() {
Expand Down
37 changes: 18 additions & 19 deletions src/rpc/server/browserContextDispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,17 @@
* limitations under the License.
*/

import * as types from '../../types';
import { BrowserContext } from '../../browserContext';
import { Events } from '../../events';
import { Dispatcher, DispatcherScope, lookupDispatcher } from './dispatcher';
import { PageDispatcher, BindingCallDispatcher, WorkerDispatcher } from './pageDispatcher';
import { PageChannel, BrowserContextChannel, BrowserContextInitializer, CDPSessionChannel, BrowserContextSetGeolocationParams, BrowserContextSetHTTPCredentialsParams } from '../channels';
import * as channels from '../channels';
import { RouteDispatcher, RequestDispatcher } from './networkDispatchers';
import { CRBrowserContext } from '../../chromium/crBrowser';
import { CDPSessionDispatcher } from './cdpSessionDispatcher';
import { Events as ChromiumEvents } from '../../chromium/events';

export class BrowserContextDispatcher extends Dispatcher<BrowserContext, BrowserContextInitializer> implements BrowserContextChannel {
export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channels.BrowserContextInitializer> implements channels.BrowserContextChannel {
private _context: BrowserContext;

constructor(scope: DispatcherScope, context: BrowserContext) {
Expand All @@ -50,67 +49,67 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, Browser
}
}

async setDefaultNavigationTimeoutNoReply(params: { timeout: number }) {
async setDefaultNavigationTimeoutNoReply(params: channels.BrowserContextSetDefaultNavigationTimeoutNoReplyParams) {
this._context.setDefaultNavigationTimeout(params.timeout);
}

async setDefaultTimeoutNoReply(params: { timeout: number }) {
async setDefaultTimeoutNoReply(params: channels.BrowserContextSetDefaultTimeoutNoReplyParams) {
this._context.setDefaultTimeout(params.timeout);
}

async exposeBinding(params: { name: string }): Promise<void> {
async exposeBinding(params: channels.BrowserContextExposeBindingParams): Promise<void> {
await this._context.exposeBinding(params.name, (source, ...args) => {
const binding = new BindingCallDispatcher(this._scope, params.name, source, args);
this._dispatchEvent('bindingCall', { binding });
return binding.promise();
});
}

async newPage(): Promise<{ page: PageChannel }> {
async newPage(): Promise<channels.BrowserContextNewPageResult> {
return { page: lookupDispatcher<PageDispatcher>(await this._context.newPage()) };
}

async cookies(params: { urls: string[] }): Promise<{ cookies: types.NetworkCookie[] }> {
async cookies(params: channels.BrowserContextCookiesParams): Promise<channels.BrowserContextCookiesResult> {
return { cookies: await this._context.cookies(params.urls) };
}

async addCookies(params: { cookies: types.SetNetworkCookieParam[] }): Promise<void> {
async addCookies(params: channels.BrowserContextAddCookiesParams): Promise<void> {
await this._context.addCookies(params.cookies);
}

async clearCookies(): Promise<void> {
await this._context.clearCookies();
}

async grantPermissions(params: { permissions: string[], options: { origin?: string } }): Promise<void> {
await this._context.grantPermissions(params.permissions, params.options);
async grantPermissions(params: channels.BrowserContextGrantPermissionsParams): Promise<void> {
await this._context.grantPermissions(params.permissions, params.origin);
}

async clearPermissions(): Promise<void> {
await this._context.clearPermissions();
}

async setGeolocation(params: BrowserContextSetGeolocationParams): Promise<void> {
async setGeolocation(params: channels.BrowserContextSetGeolocationParams): Promise<void> {
await this._context.setGeolocation(params.geolocation);
}

async setExtraHTTPHeaders(params: { headers: types.HeadersArray }): Promise<void> {
async setExtraHTTPHeaders(params: channels.BrowserContextSetExtraHTTPHeadersParams): Promise<void> {
await this._context.setExtraHTTPHeaders(params.headers);
}

async setOffline(params: { offline: boolean }): Promise<void> {
async setOffline(params: channels.BrowserContextSetOfflineParams): Promise<void> {
await this._context.setOffline(params.offline);
}

async setHTTPCredentials(params: BrowserContextSetHTTPCredentialsParams): Promise<void> {
async setHTTPCredentials(params: channels.BrowserContextSetHTTPCredentialsParams): Promise<void> {
await this._context.setHTTPCredentials(params.httpCredentials);
}

async addInitScript(params: { source: string }): Promise<void> {
async addInitScript(params: channels.BrowserContextAddInitScriptParams): Promise<void> {
await this._context._doAddInitScript(params.source);
}

async setNetworkInterceptionEnabled(params: { enabled: boolean }): Promise<void> {
async setNetworkInterceptionEnabled(params: channels.BrowserContextSetNetworkInterceptionEnabledParams): Promise<void> {
if (!params.enabled) {
await this._context._setRequestInterceptor(undefined);
return;
Expand All @@ -124,8 +123,8 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, Browser
await this._context.close();
}

async crNewCDPSession(params: { page: PageDispatcher }): Promise<{ session: CDPSessionChannel }> {
async crNewCDPSession(params: channels.BrowserContextCrNewCDPSessionParams): Promise<channels.BrowserContextCrNewCDPSessionResult> {
const crBrowserContext = this._object as CRBrowserContext;
return { session: new CDPSessionDispatcher(this._scope, await crBrowserContext.newCDPSession(params.page._object)) };
return { session: new CDPSessionDispatcher(this._scope, await crBrowserContext.newCDPSession((params.page as PageDispatcher)._object)) };
}
}
9 changes: 8 additions & 1 deletion test/permissions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,19 @@ describe.skip(options.WEBKIT)('permissions', () => {
expect(error.message).toContain('Unknown permission: foo');
});

it('should grant geolocation permission when listed', async({page, server, context}) => {
it('should grant geolocation permission when origin is listed', async({page, server, context}) => {
await page.goto(server.EMPTY_PAGE);
await context.grantPermissions(['geolocation'], { origin: server.EMPTY_PAGE });
expect(await getPermission(page, 'geolocation')).toBe('granted');
});

it('should prompt for geolocation permission when origin is not listed', async({page, server, context}) => {
await page.goto(server.EMPTY_PAGE);
await context.grantPermissions(['geolocation'], { origin: server.EMPTY_PAGE });
await page.goto(server.EMPTY_PAGE.replace('localhost', '127.0.0.1'));
expect(await getPermission(page, 'geolocation')).toBe('prompt');
});

it('should grant notifications permission when listed', async({page, server, context}) => {
await page.goto(server.EMPTY_PAGE);
await context.grantPermissions(['notifications'], { origin: server.EMPTY_PAGE });
Expand Down

0 comments on commit 4c56354

Please sign in to comment.