Skip to content
Merged
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
14 changes: 12 additions & 2 deletions packages/playwright-core/src/server/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,6 @@ export abstract class APIRequestContext extends SdkObject {
headers,
agent: options.agent,
maxRedirects: options.maxRedirects - 1,
...getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, url.origin),
__testHookLookup: options.__testHookLookup,
};
// rejectUnauthorized = undefined is treated as true in node 12.
Expand All @@ -438,6 +437,15 @@ export abstract class APIRequestContext extends SdkObject {
if (headers['host'])
headers['host'] = locationURL.host;

// Drop credentials scoped to the original origin on cross-origin redirects.
if (locationURL.origin !== url.origin)
removeHeader(headers, 'authorization');

// Client certificates are origin-scoped — pick them based on the redirect
// target, not the original URL.
Object.assign(redirectOptions,
getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, locationURL.origin));

notifyRequestFinished();
fulfill(this._sendRequest(progress, locationURL, redirectOptions, postData));
request.destroy();
Expand Down Expand Up @@ -767,7 +775,9 @@ function getHeader(headers: HeadersObject, name: string) {
}

function removeHeader(headers: { [name: string]: string }, name: string) {
delete headers[name];
const existing = Object.entries(headers).find(pair => pair[0].toLowerCase() === name.toLowerCase());
if (existing)
delete headers[existing[0]];
}

function setBasicAuthorizationHeader(headers: { [name: string]: string }, credentials: HTTPCredentials) {
Expand Down
34 changes: 34 additions & 0 deletions tests/library/client-certificates.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,40 @@ test.describe('fetch', () => {
await request.dispose();
});

test('should not leak client certificate to cross-origin redirect target', async ({ playwright, startCCServer, asset }) => {
const targetURL = await startCCServer();

// Standalone HTTPS server (not cert-required) that 302-redirects to the cert-required server.
const redirectServer = createHttpsServer({
key: fs.readFileSync(asset('client-certificates/server/server_key.pem')),
cert: fs.readFileSync(asset('client-certificates/server/server_cert.pem')),
}, (req, res) => {
res.writeHead(302, { Location: targetURL });
res.end();
});
await new Promise<void>(f => redirectServer.listen(0, '127.0.0.1', () => f()));
const redirectAddr = redirectServer.address() as net.AddressInfo;
// Use 'localhost' for the redirect origin so it differs from the target's '127.0.0.1' origin
// even though both resolve to the loopback interface and share the same trusted CA.
const redirectURL = `https://localhost:${redirectAddr.port}/redir`;

const request = await playwright.request.newContext({
ignoreHTTPSErrors: true,
// Cert is scoped to the redirect *origin*, not the target — it must not follow the redirect.
clientCertificates: [{
origin: new URL(redirectURL).origin,
certPath: asset('client-certificates/client/trusted/cert.pem'),
keyPath: asset('client-certificates/client/trusted/key.pem'),
}],
});
const response = await request.get(redirectURL);
expect(response.url()).toBe(targetURL);
expect(response.status()).toBe(401);
expect(await response.text()).toContain('you need to provide a client certificate');
await request.dispose();
await new Promise<void>(f => redirectServer.close(() => f()));
});

test('pass with trusted client certificates in pfx format', async ({ playwright, startCCServer, asset }) => {
const serverURL = await startCCServer();
const request = await playwright.request.newContext({
Expand Down
22 changes: 22 additions & 0 deletions tests/library/global-fetch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,28 @@ it('should propagate extra http headers with redirects', async ({ playwright, se
await request.dispose();
});

it('should preserve authorization on same-origin redirect but strip on cross-origin', async ({ playwright, server }) => {
server.setRedirect('/same/redirect', '/same/dest');
server.setRedirect('/cross/redirect', server.CROSS_PROCESS_PREFIX + '/cross/dest');
const request = await playwright.request.newContext({
extraHTTPHeaders: { 'Authorization': 'Bearer secret' },
});

const [sameDestReq] = await Promise.all([
server.waitForRequest('/same/dest'),
request.get(`${server.PREFIX}/same/redirect`),
]);
expect(sameDestReq.headers['authorization']).toBe('Bearer secret');

const [crossDestReq] = await Promise.all([
server.waitForRequest('/cross/dest'),
request.get(`${server.PREFIX}/cross/redirect`),
]);
expect(crossDestReq.headers['authorization']).toBeUndefined();

await request.dispose();
});

it('should support global httpCredentials option', async ({ playwright, server }) => {
server.setAuth('/empty.html', 'user', 'pass');
const request1 = await playwright.request.newContext();
Expand Down
Loading