From 83324faa743ea6a9c0286fa214cab95008e82d47 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Wed, 10 Jul 2024 22:00:20 +0200 Subject: [PATCH] feedback 5 --- docs/src/api/params.md | 2 +- .../playwright-core/bin/socks-certs/README.md | 5 ++-- .../src/server/browserContext.ts | 2 +- .../playwright-core/src/server/browserType.ts | 11 +++++++ .../socksClientCertificatesInterceptor.ts | 29 ++++++++++--------- packages/playwright-core/types/types.d.ts | 4 --- packages/playwright/types/test.d.ts | 1 - tests/library/client-certificates.spec.ts | 23 +++++++++++++++ 8 files changed, 53 insertions(+), 24 deletions(-) diff --git a/docs/src/api/params.md b/docs/src/api/params.md index 52e640bae1..0a1e178559 100644 --- a/docs/src/api/params.md +++ b/docs/src/api/params.md @@ -523,7 +523,7 @@ Does not enforce fixed viewport, allows resizing window in the headed mode. - `passphrase` ?<[string]> Passphrase for the private key (PEM or PFX). - `pfx` ?<[string]> Path to the PFX or PKCS12 encoded private key and certificate chain. -An array of client certificates to be used. Each certificate object must have `cert` and `key` or `pfx` to load the client certificate. Optionally, `passphrase` property should be provided if the private key is encrypted. If the certificate is issued by a custom certificate authority, the `ignoreHTTPSErrors` needs to be set. If the certificate is valid only for specific URLs, the `url` property should be provided with a glob pattern to match the URLs that the certificate is valid for. +An array of client certificates to be used. Each certificate object must have `cert` and `key` or `pfx` to load the client certificate. Optionally, `passphrase` property should be provided if the private key is encrypted. If the certificate is valid only for specific URLs, the `url` property should be provided with a glob pattern to match the URLs that the certificate is valid for. :::note Using Client Certificates in combination with Proxy Servers is not supported. diff --git a/packages/playwright-core/bin/socks-certs/README.md b/packages/playwright-core/bin/socks-certs/README.md index 458d849d02..4950ef1f3c 100644 --- a/packages/playwright-core/bin/socks-certs/README.md +++ b/packages/playwright-core/bin/socks-certs/README.md @@ -2,9 +2,8 @@ These certificates are used when client certificates are used with Playwright. Playwright then creates a Socks proxy, which sits between -the browser and the actual target server. The Socks proxy then does the -TLS upgrade using the certficiates in this directory (locally) while -maintaining the client certificates to the target server. +the browser and the actual target server. The Socks proxy uses this certificiate +to talk to the browser and establishes its own secure TLS connection to the server. The certificates are generated via: ```bash diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index f553532790..388a1027bb 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -689,7 +689,7 @@ export function validateBrowserContextOptions(options: channels.BrowserNewContex if (options.proxy) { if (!browserOptions.proxy && browserOptions.isChromium && os.platform() === 'win32') throw new Error(`Browser needs to be launched with the global proxy. If all contexts override the proxy, global proxy will be never used and can be any string, for example "launch({ proxy: { server: 'http://per-context' } })"`); - if (options.clientCertificates) + if (!browserOptions.persistent && options.clientCertificates) throw new Error('Cannot specify both proxy and clientCertificates'); options.proxy = normalizeProxySettings(options.proxy); } diff --git a/packages/playwright-core/src/server/browserType.ts b/packages/playwright-core/src/server/browserType.ts index cfa8862b84..e13ce20508 100644 --- a/packages/playwright-core/src/server/browserType.ts +++ b/packages/playwright-core/src/server/browserType.ts @@ -40,6 +40,7 @@ import type { CallMetadata } from './instrumentation'; import { SdkObject } from './instrumentation'; import { ManualPromise } from '../utils/manualPromise'; import { type ProtocolError, isProtocolError } from './protocolError'; +import { ClientCertificatesProxy } from './socksClientCertificatesInterceptor'; export const kNoXServerRunningError = 'Looks like you launched a headed browser without having a XServer running.\n' + 'Set either \'headless: true\' or use \'xvfb-run \' before running Playwright.\n\n<3 Playwright Team'; @@ -101,6 +102,14 @@ export abstract class BrowserType extends SdkObject { async _innerLaunch(progress: Progress, options: types.LaunchOptions, persistent: channels.BrowserNewContextParams | undefined, protocolLogger: types.ProtocolLogger, maybeUserDataDir?: string): Promise { options.proxy = options.proxy ? normalizeProxySettings(options.proxy) : undefined; + let clientCertificatesProxy: ClientCertificatesProxy | undefined; + if (persistent?.clientCertificates?.length) { + if (persistent.proxy?.server) + throw new Error('Cannot specify both proxy and clientCertificates'); + clientCertificatesProxy = new ClientCertificatesProxy(persistent); + options.proxy = { server: await clientCertificatesProxy.listen() }; + persistent.ignoreHTTPSErrors = true; + } const browserLogsCollector = new RecentLogsCollector(); const { browserProcess, userDataDir, artifactsDir, transport } = await this._launchProcess(progress, options, !!persistent, browserLogsCollector, maybeUserDataDir); if ((options as any).__testHookBeforeCreateBrowser) @@ -131,6 +140,8 @@ export abstract class BrowserType extends SdkObject { // We assume no control when using custom arguments, and do not prepare the default context in that case. if (persistent && !options.ignoreAllDefaultArgs) await browser._defaultContext!._loadDefaultContext(progress); + if (persistent) + browser._defaultContext!._clientCertificatesProxy = clientCertificatesProxy; return browser; } diff --git a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts index cc21795ae7..d4913ba02c 100644 --- a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts +++ b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts @@ -90,13 +90,14 @@ class SocksProxyConnection { private _attachTLSListeners() { this.internal = new SocksConnectionDuplex(data => this.socksProxy._socksProxy.sendSocketData({ uid: this.uid, data })); - this.internalTLS = new tls.TLSSocket(this.internal, { + const internalTLS = new tls.TLSSocket(this.internal, { isServer: true, // openssl req -new -newkey rsa:2048 -days 3650 -nodes -x509 -keyout key.pem -out cert.pem -subj "/CN=localhost" key: fs.readFileSync(path.join(__dirname, '../../bin/socks-certs/key.pem')), cert: fs.readFileSync(path.join(__dirname, '../../bin/socks-certs/cert.pem')), }); - this.internalTLS.on('close', () => this.socksProxy._socksProxy.sendSocketEnd({ uid: this.uid })); + this.internalTLS = internalTLS; + internalTLS.on('close', () => this.socksProxy._socksProxy.sendSocketEnd({ uid: this.uid })); const targetTLS = tls.connect({ socket: this.target, @@ -104,27 +105,27 @@ class SocksProxyConnection { ...clientCertificatesToTLSOptions(this.socksProxy.contextOptions.clientCertificates, `https://${this.host}:${this.port}/`), }); - this.internalTLS.pipe(targetTLS); - targetTLS.pipe(this.internalTLS); + internalTLS.pipe(targetTLS); + targetTLS.pipe(internalTLS); // Handle close and errors const closeBothSockets = () => { - this.internalTLS?.end(); + internalTLS.end(); targetTLS.end(); }; - this.internalTLS.on('end', () => closeBothSockets()); + internalTLS.on('end', () => closeBothSockets()); targetTLS.on('end', () => closeBothSockets()); - this.internalTLS.on('error', () => closeBothSockets()); + internalTLS.on('error', () => closeBothSockets()); targetTLS.on('error', error => { - this.internalTLS!.write('HTTP/1.1 503 Internal Server Error\r\n'); - this.internalTLS!.write('Content-Type: text/html; charset=utf-8\r\n'); - const responseBody = 'Self-signed certificate error: ' + error.message; - this.internalTLS!.write('Content-Length: ' + Buffer.byteLength(responseBody) + '\r\n'); - this.internalTLS!.write('\r\n'); - this.internalTLS!.write(responseBody); - this.internalTLS!.end(); + internalTLS.write('HTTP/1.1 503 Internal Server Error\r\n'); + internalTLS.write('Content-Type: text/html; charset=utf-8\r\n'); + const responseBody = 'Playwright client-certificate error: ' + error.message; + internalTLS.write('Content-Length: ' + Buffer.byteLength(responseBody) + '\r\n'); + internalTLS.write('\r\n'); + internalTLS.write(responseBody); + internalTLS.end(); closeBothSockets(); }); } diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index c12f3f13ea..c214ec3501 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -13224,7 +13224,6 @@ export interface BrowserType { /** * An array of client certificates to be used. Each certificate object must have `cert` and `key` or `pfx` to load the * client certificate. Optionally, `passphrase` property should be provided if the private key is encrypted. If the - * certificate is issued by a custom certificate authority, the `ignoreHTTPSErrors` needs to be set. If the * certificate is valid only for specific URLs, the `url` property should be provided with a glob pattern to match the * URLs that the certificate is valid for. * @@ -15637,7 +15636,6 @@ export interface APIRequest { /** * An array of client certificates to be used. Each certificate object must have `cert` and `key` or `pfx` to load the * client certificate. Optionally, `passphrase` property should be provided if the private key is encrypted. If the - * certificate is issued by a custom certificate authority, the `ignoreHTTPSErrors` needs to be set. If the * certificate is valid only for specific URLs, the `url` property should be provided with a glob pattern to match the * URLs that the certificate is valid for. * @@ -16832,7 +16830,6 @@ export interface Browser extends EventEmitter { /** * An array of client certificates to be used. Each certificate object must have `cert` and `key` or `pfx` to load the * client certificate. Optionally, `passphrase` property should be provided if the private key is encrypted. If the - * certificate is issued by a custom certificate authority, the `ignoreHTTPSErrors` needs to be set. If the * certificate is valid only for specific URLs, the `url` property should be provided with a glob pattern to match the * URLs that the certificate is valid for. * @@ -20308,7 +20305,6 @@ export interface BrowserContextOptions { /** * An array of client certificates to be used. Each certificate object must have `cert` and `key` or `pfx` to load the * client certificate. Optionally, `passphrase` property should be provided if the private key is encrypted. If the - * certificate is issued by a custom certificate authority, the `ignoreHTTPSErrors` needs to be set. If the * certificate is valid only for specific URLs, the `url` property should be provided with a glob pattern to match the * URLs that the certificate is valid for. * diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts index 76dd66f33b..7650946b3a 100644 --- a/packages/playwright/types/test.d.ts +++ b/packages/playwright/types/test.d.ts @@ -5204,7 +5204,6 @@ export interface PlaywrightTestOptions { /** * An array of client certificates to be used. Each certificate object must have `cert` and `key` or `pfx` to load the * client certificate. Optionally, `passphrase` property should be provided if the private key is encrypted. If the - * certificate is issued by a custom certificate authority, the `ignoreHTTPSErrors` needs to be set. If the * certificate is valid only for specific URLs, the `url` property should be provided with a glob pattern to match the * URLs that the certificate is valid for. * diff --git a/tests/library/client-certificates.spec.ts b/tests/library/client-certificates.spec.ts index fe6aa295f7..94a321ee16 100644 --- a/tests/library/client-certificates.spec.ts +++ b/tests/library/client-certificates.spec.ts @@ -258,4 +258,27 @@ test.describe('browser', () => { await expect(page.getByText('Hello Alice, your certificate was issued by localhost!')).toBeVisible(); await page.close(); }); + + test.describe('persistentContext', () => { + + test('validate input', async ({ launchPersistent }) => { + for (const [contextOptions, expected] of kValidationSubTests) + await expect(launchPersistent(contextOptions)).rejects.toThrow(expected); + }); + + test('should pass with matching certificates', async ({ launchPersistent, serverURLRewrittenToLocalhost, asset }) => { + const { page } = await launchPersistent({ + ignoreHTTPSErrors: true, + clientCertificates: [{ + url: serverURLRewrittenToLocalhost, + certs: [{ + cert: asset('client-certificates/client/alice_cert.pem'), + key: asset('client-certificates/client/alice_key.pem'), + }], + }], + }); + await page.goto(serverURLRewrittenToLocalhost); + await expect(page.getByText('Hello Alice, your certificate was issued by localhost!')).toBeVisible(); + }); + }); });