feedback 5

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.

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

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);
   }

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 <your-playwright-app>\' 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<Browser> {
     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;
   }
 

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();
     });
   }

packages/playwright-core/types/types.d.ts

@@ -13224,7 +13224,6 @@ export interface BrowserType<Unused = {}> {
     /**
      * 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.
    *

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.
    *

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();
+    });
+  });
 });