Skip to content

Commit d65a5f4

Browse files
SkyZeroZxatscott
SkyZeroZx
authored and
atscott
committed
fix(service-worker): Strips sensitive headers on cross-origin redirects
Removes `Authorization`, `Cookie`, and `Proxy-Authorization` headers when a request is redirected to a different origin. This aligns with the Fetch API's redirect algorithm to prevent sensitive information from being sent to third-party origins. (cherry picked from commit 423a109)
1 parent 1a62130 commit d65a5f4

5 files changed

Lines changed: 71 additions & 29 deletions

File tree

‎packages/service-worker/worker/src/adapter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export class Adapter<T extends CacheStorage = CacheStorage> {
5151
/**
5252
* Wrapper around the `Headers` constructor.
5353
*/
54-
newHeaders(headers: {[name: string]: string}): Headers {
54+
newHeaders(headers: HeadersInit): Headers {
5555
return new Headers(headers);
5656
}
5757

‎packages/service-worker/worker/src/assets.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,9 @@ export abstract class AssetGroup {
502502
* metadata that are known to be safe.
503503
*
504504
* Currently, headers, redirect policy, an explicit `credentials: 'omit'`, and the HTTP cache
505-
* mode are preserved.
505+
* mode are preserved. On cross-origin redirects, sensitive headers are removed. This includes
506+
* `Authorization`, as required by the Fetch redirect algorithm, and forbidden request headers
507+
* that could contain credentials.
506508
*
507509
* NOTE:
508510
* `credentials: 'same-origin'` and `credentials: 'include'` are intentionally not preserved.
@@ -515,9 +517,21 @@ export abstract class AssetGroup {
515517
* Investigate preserving more metadata. See, also, discussion on preserving `mode`:
516518
* https://github.com/angular/angular/issues/41931#issuecomment-1227601347.
517519
*/
518-
private newRequestWithMetadata(url: string, options: RequestInit): Request {
520+
private newRequestWithMetadata(url: string, options: Request): Request {
521+
let headers = options.headers;
522+
const parsedUrl = this.adapter.parseUrl(url, this.adapter.origin);
523+
524+
const hasHeaders = headers.keys().next().done !== true;
525+
526+
if (hasHeaders && parsedUrl.origin !== this.adapter.origin) {
527+
headers = this.adapter.newHeaders(options.headers);
528+
headers.delete('Authorization');
529+
headers.delete('Proxy-Authorization');
530+
headers.delete('Cookie');
531+
}
532+
519533
const init: RequestInit = {
520-
headers: options.headers,
534+
headers,
521535
redirect: options.redirect,
522536
};
523537

‎packages/service-worker/worker/test/happy_spec.ts‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ import {envIsSupported} from '../testing/utils';
135135
name: 'other',
136136
installMode: 'lazy',
137137
updateMode: 'lazy',
138-
urls: ['/baz.txt', '/qux.txt', '/lazy/redirected.txt'],
138+
urls: ['/baz.txt', '/qux.txt', '/lazy/redirected.txt', '/lazy/cross-origin-redirected.txt'],
139139
patterns: [],
140140
cacheQueryOptions: {ignoreVary: true},
141141
},
@@ -220,6 +220,11 @@ import {envIsSupported} from '../testing/utils';
220220
.withStaticFiles(dist)
221221
.withRedirect('/redirected.txt', '/redirect-target.txt')
222222
.withRedirect('/lazy/redirected.txt', '/lazy/redirect-target.txt')
223+
.withRedirect(
224+
'/lazy/cross-origin-redirected.txt',
225+
'https://example.com/lazy/redirect-target.txt',
226+
)
227+
.withRedirect('https://example.com/lazy/redirect-target.txt', '/lazy/redirect-target.txt')
223228
.withError('/error.txt');
224229

225230
const server = serverBuilderBase.withManifest(manifest).build();
@@ -1681,14 +1686,40 @@ import {envIsSupported} from '../testing/utils';
16811686
// Request a redirected, lazy-cached asset (so that it is fetched from the network) and
16821687
// provide headers.
16831688
const reqInit = {
1684-
headers: {SomeHeader: 'SomeValue'},
1689+
headers: {
1690+
Authorization: 'Bearer secret',
1691+
SomeHeader: 'SomeValue',
1692+
},
16851693
};
16861694
expect(await makeRequest(scope, '/lazy/redirected.txt', undefined, reqInit)).toBe(
16871695
'this was a redirect too',
16881696
);
16891697

16901698
// Verify that the headers were passed through to the network.
16911699
const [redirectReq] = server.getRequestsFor('/lazy/redirect-target.txt');
1700+
expect(redirectReq.headers.get('Authorization')).toBe('Bearer secret');
1701+
expect(redirectReq.headers.get('SomeHeader')).toBe('SomeValue');
1702+
});
1703+
1704+
it('does not pass sensitive headers through to a different origin', async () => {
1705+
const reqInit = {
1706+
headers: {
1707+
Authorization: 'Bearer secret',
1708+
Cookie: 'session=secret',
1709+
'Proxy-Authorization': 'Basic secret',
1710+
SomeHeader: 'SomeValue',
1711+
},
1712+
};
1713+
expect(
1714+
await makeRequest(scope, '/lazy/cross-origin-redirected.txt', undefined, reqInit),
1715+
).toBe('this was a redirect too');
1716+
1717+
const [redirectReq] = server.getRequestsFor(
1718+
'https://example.com/lazy/redirect-target.txt',
1719+
);
1720+
expect(redirectReq.headers.get('Authorization')).toBeNull();
1721+
expect(redirectReq.headers.get('Cookie')).toBeNull();
1722+
expect(redirectReq.headers.get('Proxy-Authorization')).toBeNull();
16921723
expect(redirectReq.headers.get('SomeHeader')).toBe('SomeValue');
16931724
});
16941725

‎packages/service-worker/worker/testing/fetch.ts‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,20 @@ export class MockBody implements Body {
5959
export class MockHeaders implements Headers {
6060
map = new Map<string, string>();
6161

62+
constructor(headers?: HeadersInit) {
63+
if (headers === undefined) {
64+
return;
65+
}
66+
67+
if (Array.isArray(headers)) {
68+
headers.forEach(([name, value]) => this.set(name, value));
69+
} else if (headers instanceof MockHeaders || headers instanceof Headers) {
70+
headers.forEach((value, name) => this.set(name, value));
71+
} else {
72+
Object.entries(headers).forEach(([name, value]) => this.set(name, value));
73+
}
74+
}
75+
6276
[Symbol.iterator]() {
6377
return this.map[Symbol.iterator]();
6478
}
@@ -131,15 +145,8 @@ export class MockRequest extends MockBody implements Request {
131145
throw 'Not implemented';
132146
}
133147
this.url = input;
134-
const headers = init.headers as {[key: string]: string};
135-
if (headers !== undefined) {
136-
if (headers instanceof MockHeaders) {
137-
this.headers = headers;
138-
} else {
139-
Object.keys(headers).forEach((header) => {
140-
this.headers.set(header, headers[header]);
141-
});
142-
}
148+
if (init.headers !== undefined) {
149+
this.headers = new MockHeaders(init.headers);
143150
}
144151
if (init.cache !== undefined) {
145152
this.cache = init.cache;
@@ -195,15 +202,8 @@ export class MockResponse extends MockBody implements Response {
195202
super(typeof body === 'string' ? body : null);
196203
this.status = init.status !== undefined ? init.status : 200;
197204
this.statusText = init.statusText !== undefined ? init.statusText : 'OK';
198-
const headers = init.headers as {[key: string]: string};
199-
if (headers !== undefined) {
200-
if (headers instanceof MockHeaders) {
201-
this.headers = headers;
202-
} else {
203-
Object.keys(headers).forEach((header) => {
204-
this.headers.set(header, headers[header]);
205-
});
206-
}
205+
if (init.headers !== undefined) {
206+
this.headers = new MockHeaders(init.headers);
207207
}
208208
if (init.type !== undefined) {
209209
this.type = init.type;

‎packages/service-worker/worker/testing/scope.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,8 @@ export class SwTestHarnessImpl
181181
return new MockResponse(body, init);
182182
}
183183

184-
override newHeaders(headers: {[name: string]: string}): Headers {
185-
return Object.keys(headers).reduce((mock, name) => {
186-
mock.set(name, headers[name]);
187-
return mock;
188-
}, new MockHeaders());
184+
override newHeaders(headers: HeadersInit): Headers {
185+
return new MockHeaders(headers);
189186
}
190187

191188
async skipWaiting(): Promise<void> {

0 commit comments

Comments
 (0)