From 4c5635434a2a80642766b1a2723e9b450d9abb1e Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 20 Aug 2020 14:19:27 -0700 Subject: [PATCH] fix(permissions): browserContext.grantPermissions to respect the origin (#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. --- src/browserContext.ts | 16 +++++----- src/rpc/server/browserContextDispatcher.ts | 37 +++++++++++----------- test/permissions.spec.ts | 9 +++++- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/browserContext.ts b/src/browserContext.ts index 314cb6db49..46a0633f68 100644 --- a/src/browserContext.ts +++ b/src/browserContext.ts @@ -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() { diff --git a/src/rpc/server/browserContextDispatcher.ts b/src/rpc/server/browserContextDispatcher.ts index 12a464c35f..7a3529ad60 100644 --- a/src/rpc/server/browserContextDispatcher.ts +++ b/src/rpc/server/browserContextDispatcher.ts @@ -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 implements BrowserContextChannel { +export class BrowserContextDispatcher extends Dispatcher implements channels.BrowserContextChannel { private _context: BrowserContext; constructor(scope: DispatcherScope, context: BrowserContext) { @@ -50,15 +49,15 @@ export class BrowserContextDispatcher extends Dispatcher { + async exposeBinding(params: channels.BrowserContextExposeBindingParams): Promise { await this._context.exposeBinding(params.name, (source, ...args) => { const binding = new BindingCallDispatcher(this._scope, params.name, source, args); this._dispatchEvent('bindingCall', { binding }); @@ -66,15 +65,15 @@ export class BrowserContextDispatcher extends Dispatcher { + async newPage(): Promise { return { page: lookupDispatcher(await this._context.newPage()) }; } - async cookies(params: { urls: string[] }): Promise<{ cookies: types.NetworkCookie[] }> { + async cookies(params: channels.BrowserContextCookiesParams): Promise { return { cookies: await this._context.cookies(params.urls) }; } - async addCookies(params: { cookies: types.SetNetworkCookieParam[] }): Promise { + async addCookies(params: channels.BrowserContextAddCookiesParams): Promise { await this._context.addCookies(params.cookies); } @@ -82,35 +81,35 @@ export class BrowserContextDispatcher extends Dispatcher { - await this._context.grantPermissions(params.permissions, params.options); + async grantPermissions(params: channels.BrowserContextGrantPermissionsParams): Promise { + await this._context.grantPermissions(params.permissions, params.origin); } async clearPermissions(): Promise { await this._context.clearPermissions(); } - async setGeolocation(params: BrowserContextSetGeolocationParams): Promise { + async setGeolocation(params: channels.BrowserContextSetGeolocationParams): Promise { await this._context.setGeolocation(params.geolocation); } - async setExtraHTTPHeaders(params: { headers: types.HeadersArray }): Promise { + async setExtraHTTPHeaders(params: channels.BrowserContextSetExtraHTTPHeadersParams): Promise { await this._context.setExtraHTTPHeaders(params.headers); } - async setOffline(params: { offline: boolean }): Promise { + async setOffline(params: channels.BrowserContextSetOfflineParams): Promise { await this._context.setOffline(params.offline); } - async setHTTPCredentials(params: BrowserContextSetHTTPCredentialsParams): Promise { + async setHTTPCredentials(params: channels.BrowserContextSetHTTPCredentialsParams): Promise { await this._context.setHTTPCredentials(params.httpCredentials); } - async addInitScript(params: { source: string }): Promise { + async addInitScript(params: channels.BrowserContextAddInitScriptParams): Promise { await this._context._doAddInitScript(params.source); } - async setNetworkInterceptionEnabled(params: { enabled: boolean }): Promise { + async setNetworkInterceptionEnabled(params: channels.BrowserContextSetNetworkInterceptionEnabledParams): Promise { if (!params.enabled) { await this._context._setRequestInterceptor(undefined); return; @@ -124,8 +123,8 @@ export class BrowserContextDispatcher extends Dispatcher { + async crNewCDPSession(params: channels.BrowserContextCrNewCDPSessionParams): Promise { 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)) }; } } diff --git a/test/permissions.spec.ts b/test/permissions.spec.ts index 702e9a7343..f7948a6fc7 100644 --- a/test/permissions.spec.ts +++ b/test/permissions.spec.ts @@ -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 });