Skip to content

Commit 703f0cb

Browse files
authored
fix(fetch): drop authorization and recompute client cert on cross-ori… (#40547)
1 parent 4a80eed commit 703f0cb

3 files changed

Lines changed: 68 additions & 2 deletions

File tree

packages/playwright-core/src/server/fetch.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,6 @@ export abstract class APIRequestContext extends SdkObject {
414414
headers,
415415
agent: options.agent,
416416
maxRedirects: options.maxRedirects - 1,
417-
...getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, url.origin),
418417
__testHookLookup: options.__testHookLookup,
419418
};
420419
// rejectUnauthorized = undefined is treated as true in node 12.
@@ -438,6 +437,15 @@ export abstract class APIRequestContext extends SdkObject {
438437
if (headers['host'])
439438
headers['host'] = locationURL.host;
440439

440+
// Drop credentials scoped to the original origin on cross-origin redirects.
441+
if (locationURL.origin !== url.origin)
442+
removeHeader(headers, 'authorization');
443+
444+
// Client certificates are origin-scoped — pick them based on the redirect
445+
// target, not the original URL.
446+
Object.assign(redirectOptions,
447+
getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, locationURL.origin));
448+
441449
notifyRequestFinished();
442450
fulfill(this._sendRequest(progress, locationURL, redirectOptions, postData));
443451
request.destroy();
@@ -767,7 +775,9 @@ function getHeader(headers: HeadersObject, name: string) {
767775
}
768776

769777
function removeHeader(headers: { [name: string]: string }, name: string) {
770-
delete headers[name];
778+
const existing = Object.entries(headers).find(pair => pair[0].toLowerCase() === name.toLowerCase());
779+
if (existing)
780+
delete headers[existing[0]];
771781
}
772782

773783
function setBasicAuthorizationHeader(headers: { [name: string]: string }, credentials: HTTPCredentials) {

tests/library/client-certificates.spec.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,40 @@ test.describe('fetch', () => {
156156
await request.dispose();
157157
});
158158

159+
test('should not leak client certificate to cross-origin redirect target', async ({ playwright, startCCServer, asset }) => {
160+
const targetURL = await startCCServer();
161+
162+
// Standalone HTTPS server (not cert-required) that 302-redirects to the cert-required server.
163+
const redirectServer = createHttpsServer({
164+
key: fs.readFileSync(asset('client-certificates/server/server_key.pem')),
165+
cert: fs.readFileSync(asset('client-certificates/server/server_cert.pem')),
166+
}, (req, res) => {
167+
res.writeHead(302, { Location: targetURL });
168+
res.end();
169+
});
170+
await new Promise<void>(f => redirectServer.listen(0, '127.0.0.1', () => f()));
171+
const redirectAddr = redirectServer.address() as net.AddressInfo;
172+
// Use 'localhost' for the redirect origin so it differs from the target's '127.0.0.1' origin
173+
// even though both resolve to the loopback interface and share the same trusted CA.
174+
const redirectURL = `https://localhost:${redirectAddr.port}/redir`;
175+
176+
const request = await playwright.request.newContext({
177+
ignoreHTTPSErrors: true,
178+
// Cert is scoped to the redirect *origin*, not the target — it must not follow the redirect.
179+
clientCertificates: [{
180+
origin: new URL(redirectURL).origin,
181+
certPath: asset('client-certificates/client/trusted/cert.pem'),
182+
keyPath: asset('client-certificates/client/trusted/key.pem'),
183+
}],
184+
});
185+
const response = await request.get(redirectURL);
186+
expect(response.url()).toBe(targetURL);
187+
expect(response.status()).toBe(401);
188+
expect(await response.text()).toContain('you need to provide a client certificate');
189+
await request.dispose();
190+
await new Promise<void>(f => redirectServer.close(() => f()));
191+
});
192+
159193
test('pass with trusted client certificates in pfx format', async ({ playwright, startCCServer, asset }) => {
160194
const serverURL = await startCCServer();
161195
const request = await playwright.request.newContext({

tests/library/global-fetch.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,28 @@ it('should propagate extra http headers with redirects', async ({ playwright, se
9090
await request.dispose();
9191
});
9292

93+
it('should preserve authorization on same-origin redirect but strip on cross-origin', async ({ playwright, server }) => {
94+
server.setRedirect('/same/redirect', '/same/dest');
95+
server.setRedirect('/cross/redirect', server.CROSS_PROCESS_PREFIX + '/cross/dest');
96+
const request = await playwright.request.newContext({
97+
extraHTTPHeaders: { 'Authorization': 'Bearer secret' },
98+
});
99+
100+
const [sameDestReq] = await Promise.all([
101+
server.waitForRequest('/same/dest'),
102+
request.get(`${server.PREFIX}/same/redirect`),
103+
]);
104+
expect(sameDestReq.headers['authorization']).toBe('Bearer secret');
105+
106+
const [crossDestReq] = await Promise.all([
107+
server.waitForRequest('/cross/dest'),
108+
request.get(`${server.PREFIX}/cross/redirect`),
109+
]);
110+
expect(crossDestReq.headers['authorization']).toBeUndefined();
111+
112+
await request.dispose();
113+
});
114+
93115
it('should support global httpCredentials option', async ({ playwright, server }) => {
94116
server.setAuth('/empty.html', 'user', 'pass');
95117
const request1 = await playwright.request.newContext();

0 commit comments

Comments
 (0)