From 3407e2b46d50d8555965590bb4de5f93184cd165 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 25 Jul 2024 10:18:51 -0400 Subject: [PATCH 01/12] remove referrerPolicy from auth fetch requests --- packages/auth/src/api/index.ts | 1 - packages/auth/test/helpers/mock_fetch.test.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/auth/src/api/index.ts b/packages/auth/src/api/index.ts index 81ecf5a356a..ba764eb7d75 100644 --- a/packages/auth/src/api/index.ts +++ b/packages/auth/src/api/index.ts @@ -153,7 +153,6 @@ export async function _performApiRequest( { method, headers, - referrerPolicy: 'no-referrer', ...body } ); diff --git a/packages/auth/test/helpers/mock_fetch.test.ts b/packages/auth/test/helpers/mock_fetch.test.ts index 3a04b7ce501..5077adfca5c 100644 --- a/packages/auth/test/helpers/mock_fetch.test.ts +++ b/packages/auth/test/helpers/mock_fetch.test.ts @@ -30,7 +30,6 @@ async function fetchJson(path: string, req?: object): Promise { headers: { 'Content-Type': 'application/json' }, - referrerPolicy: 'no-referrer', ...body }; From 936f3f26cf50d6dad66355379c71f8bd402ba218 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 5 Sep 2024 09:15:24 -0400 Subject: [PATCH 02/12] Append referrerPolicy for non cloudflare envs. --- common/api-review/util.api.md | 5 +++++ packages/auth/src/api/index.ts | 20 ++++++++++++++------ packages/util/src/environment.ts | 14 ++++++++++++-- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/common/api-review/util.api.md b/common/api-review/util.api.md index 09558e72ce8..4dfc135e3a0 100644 --- a/common/api-review/util.api.md +++ b/common/api-review/util.api.md @@ -264,6 +264,11 @@ export function isBrowser(): boolean; // @public (undocumented) export function isBrowserExtension(): boolean; +// Warning: (ae-missing-release-tag) "isCloudflareRunner" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public +export function isCloudflareRunner(): boolean; + // Warning: (ae-missing-release-tag) "isElectron" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public diff --git a/packages/auth/src/api/index.ts b/packages/auth/src/api/index.ts index ba764eb7d75..d4a6ffe39bd 100644 --- a/packages/auth/src/api/index.ts +++ b/packages/auth/src/api/index.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { FirebaseError, querystring } from '@firebase/util'; +import { FirebaseError, isCloudflareRunner, querystring } from '@firebase/util'; import { AuthErrorCode, NamedErrorParams } from '../core/errors'; import { @@ -148,13 +148,21 @@ export async function _performApiRequest( headers[HttpHeader.X_FIREBASE_LOCALE] = auth.languageCode; } + var fetchArgs = { + method, + headers, + ...body + }; + + if (!isCloudflareRunner()) { + Object.assign(fetchArgs, { + refererPolicy: 'no-referrer' + }); + } + return FetchProvider.fetch()( _getFinalTarget(auth, auth.config.apiHost, path, query), - { - method, - headers, - ...body - } + fetchArgs ); }); } diff --git a/packages/util/src/environment.ts b/packages/util/src/environment.ts index 998000308c3..37d33add610 100644 --- a/packages/util/src/environment.ts +++ b/packages/util/src/environment.ts @@ -79,7 +79,7 @@ export function isNode(): boolean { } /** - * Detect Browser Environment + * Detect Browser Environment. * Note: This will return true for certain test frameworks that are incompletely * mimicking a browser, and should not lead to assuming all browser APIs are * available. @@ -89,7 +89,7 @@ export function isBrowser(): boolean { } /** - * Detect Web Worker context + * Detect Web Worker context. */ export function isWebWorker(): boolean { return ( @@ -99,6 +99,16 @@ export function isWebWorker(): boolean { ); } +/** + * Detect Cloudflare Runner context. + */ +export function isCloudflareRunner() { + return ( + typeof navigator !== 'undefined' && + navigator.userAgent === 'Cloudflare-Workers' + ); +} + /** * Detect browser extensions (Chrome and Firefox at least). */ From 4b4a66f3b543b3299ff0a54942e827ee8b2a3665 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 5 Sep 2024 09:28:54 -0400 Subject: [PATCH 03/12] lint fixes --- packages/auth/src/api/index.ts | 2 +- packages/util/src/environment.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/auth/src/api/index.ts b/packages/auth/src/api/index.ts index d4a6ffe39bd..a8646a96677 100644 --- a/packages/auth/src/api/index.ts +++ b/packages/auth/src/api/index.ts @@ -148,7 +148,7 @@ export async function _performApiRequest( headers[HttpHeader.X_FIREBASE_LOCALE] = auth.languageCode; } - var fetchArgs = { + const fetchArgs = { method, headers, ...body diff --git a/packages/util/src/environment.ts b/packages/util/src/environment.ts index 37d33add610..21bdd32b9d8 100644 --- a/packages/util/src/environment.ts +++ b/packages/util/src/environment.ts @@ -102,7 +102,7 @@ export function isWebWorker(): boolean { /** * Detect Cloudflare Runner context. */ -export function isCloudflareRunner() { +export function isCloudflareRunner(): boolean { return ( typeof navigator !== 'undefined' && navigator.userAgent === 'Cloudflare-Workers' From d2b604f739d7f69f691bf913c28755e8d8434746 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 5 Sep 2024 10:18:02 -0400 Subject: [PATCH 04/12] changeset --- .changeset/khaki-numbers-nail.md | 7 +++++++ common/api-review/util.api.md | 4 ++-- packages/auth/src/api/index.ts | 4 ++-- packages/util/src/environment.ts | 4 ++-- 4 files changed, 13 insertions(+), 6 deletions(-) create mode 100644 .changeset/khaki-numbers-nail.md diff --git a/.changeset/khaki-numbers-nail.md b/.changeset/khaki-numbers-nail.md new file mode 100644 index 00000000000..70e8b514f50 --- /dev/null +++ b/.changeset/khaki-numbers-nail.md @@ -0,0 +1,7 @@ +--- +'@firebase/auth': minor +'@firebase/util': minor +'firebase': minor +--- + +Suppress the use of the `fetch` parameter `referrPolicy` within Auth for `fetch` requests originating from Cloudflare Workers. Clouldflare Worker environments do not support this parameter and throw when it's used. diff --git a/common/api-review/util.api.md b/common/api-review/util.api.md index 4dfc135e3a0..91d2f04cb40 100644 --- a/common/api-review/util.api.md +++ b/common/api-review/util.api.md @@ -264,10 +264,10 @@ export function isBrowser(): boolean; // @public (undocumented) export function isBrowserExtension(): boolean; -// Warning: (ae-missing-release-tag) "isCloudflareRunner" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// Warning: (ae-missing-release-tag) "isCloudflareWorker" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public -export function isCloudflareRunner(): boolean; +export function isCloudflareWorker(): boolean; // Warning: (ae-missing-release-tag) "isElectron" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // diff --git a/packages/auth/src/api/index.ts b/packages/auth/src/api/index.ts index a8646a96677..4a66ba5f0d1 100644 --- a/packages/auth/src/api/index.ts +++ b/packages/auth/src/api/index.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { FirebaseError, isCloudflareRunner, querystring } from '@firebase/util'; +import { FirebaseError, isCloudflareWorker, querystring } from '@firebase/util'; import { AuthErrorCode, NamedErrorParams } from '../core/errors'; import { @@ -154,7 +154,7 @@ export async function _performApiRequest( ...body }; - if (!isCloudflareRunner()) { + if (!isCloudflareWorker()) { Object.assign(fetchArgs, { refererPolicy: 'no-referrer' }); diff --git a/packages/util/src/environment.ts b/packages/util/src/environment.ts index 21bdd32b9d8..a0467b08c59 100644 --- a/packages/util/src/environment.ts +++ b/packages/util/src/environment.ts @@ -100,9 +100,9 @@ export function isWebWorker(): boolean { } /** - * Detect Cloudflare Runner context. + * Detect Cloudflare Worker context. */ -export function isCloudflareRunner(): boolean { +export function isCloudflareWorker(): boolean { return ( typeof navigator !== 'undefined' && navigator.userAgent === 'Cloudflare-Workers' From c63a5c725cffd05eda2d73e9755b30b5288a8338 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 5 Sep 2024 11:19:35 -0400 Subject: [PATCH 05/12] Use RequestInit type for fetchArgs --- packages/auth/src/api/index.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/auth/src/api/index.ts b/packages/auth/src/api/index.ts index 4a66ba5f0d1..89c89bcdbe1 100644 --- a/packages/auth/src/api/index.ts +++ b/packages/auth/src/api/index.ts @@ -148,16 +148,14 @@ export async function _performApiRequest( headers[HttpHeader.X_FIREBASE_LOCALE] = auth.languageCode; } - const fetchArgs = { + const fetchArgs: RequestInit = { method, headers, ...body }; if (!isCloudflareWorker()) { - Object.assign(fetchArgs, { - refererPolicy: 'no-referrer' - }); + fetchArgs.referrerPolicy = 'no-referrer'; } return FetchProvider.fetch()( From 357fd9e71e1b0558b2a994a0b2ce2c0ffbae76bd Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 5 Sep 2024 20:52:08 -0400 Subject: [PATCH 06/12] Patch bump for Auth instead of a minor bump. --- .changeset/khaki-numbers-nail.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/khaki-numbers-nail.md b/.changeset/khaki-numbers-nail.md index 70e8b514f50..49aa1491337 100644 --- a/.changeset/khaki-numbers-nail.md +++ b/.changeset/khaki-numbers-nail.md @@ -1,5 +1,5 @@ --- -'@firebase/auth': minor +'@firebase/auth': patch '@firebase/util': minor 'firebase': minor --- From c2a52860c8c948e3cedbe05207c2cba248bd9c9b Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 6 Sep 2024 08:06:57 -0400 Subject: [PATCH 07/12] Update khaki-numbers-nail.md --- .changeset/khaki-numbers-nail.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.changeset/khaki-numbers-nail.md b/.changeset/khaki-numbers-nail.md index 49aa1491337..487d6e1b70b 100644 --- a/.changeset/khaki-numbers-nail.md +++ b/.changeset/khaki-numbers-nail.md @@ -1,7 +1,6 @@ --- '@firebase/auth': patch '@firebase/util': minor -'firebase': minor --- Suppress the use of the `fetch` parameter `referrPolicy` within Auth for `fetch` requests originating from Cloudflare Workers. Clouldflare Worker environments do not support this parameter and throw when it's used. From 48c394074f0f0094d4cd77007cf301ee1a3edaa6 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Mon, 9 Sep 2024 13:52:22 -0400 Subject: [PATCH 08/12] Added uTests & isCloudflareWorker usage comments --- .changeset/khaki-numbers-nail.md | 2 +- packages/auth/src/api/index.test.ts | 61 +++++++++++++++++++++++++++++ packages/auth/src/api/index.ts | 4 ++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/.changeset/khaki-numbers-nail.md b/.changeset/khaki-numbers-nail.md index 487d6e1b70b..6580c989a1e 100644 --- a/.changeset/khaki-numbers-nail.md +++ b/.changeset/khaki-numbers-nail.md @@ -3,4 +3,4 @@ '@firebase/util': minor --- -Suppress the use of the `fetch` parameter `referrPolicy` within Auth for `fetch` requests originating from Cloudflare Workers. Clouldflare Worker environments do not support this parameter and throw when it's used. +Suppress the use of the `fetch` parameter `referrerPolicy` within Auth for `fetch` requests originating from Cloudflare Workers. Clouldflare Worker environments do not support this parameter and throw when it's used. diff --git a/packages/auth/src/api/index.test.ts b/packages/auth/src/api/index.test.ts index d95be550867..c721971297e 100644 --- a/packages/auth/src/api/index.test.ts +++ b/packages/auth/src/api/index.test.ts @@ -22,6 +22,7 @@ import { useFakeTimers } from 'sinon'; import sinonChai from 'sinon-chai'; import { FirebaseError, getUA } from '@firebase/util'; +import * as utils from '@firebase/util'; import { mockEndpoint } from '../../test/helpers/api/helper'; import { testAuth, TestAuth } from '../../test/helpers/mock_auth'; @@ -308,6 +309,66 @@ describe('api/_performApiRequest', () => { }); }); + context('referer policy exists on fetch request', () => { + afterEach(mockFetch.tearDown); + + it('should have referrerPolicy set', async () => { + /* eslint-disable no-var */ + var referrerPolicySet: boolean = false; + /* eslint-enable no-var */ + mockFetch.setUpWithOverride( + (input: RequestInfo | URL, request?: RequestInit) => { + if (request !== undefined && request.referrerPolicy !== undefined) { + referrerPolicySet = true; + } + return new Promise((_, reject) => + reject(new Error('network error')) + ); + } + ); + const promise = _performApiRequest( + auth, + HttpMethod.POST, + Endpoint.SIGN_UP, + request + ); + await expect(promise).to.be.rejectedWith( + FirebaseError, + 'auth/network-request-failed' + ); + expect(referrerPolicySet).to.be.true; + }); + + it('should not have referrerPolicy set on Cloudflare workers', async () => { + sinon.stub(utils, 'isCloudflareWorker').returns(true); + /* eslint-disable no-var */ + var referrerPolicySet: boolean = false; + /* eslint-enable no-var */ + mockFetch.setUpWithOverride( + (input: RequestInfo | URL, request?: RequestInit) => { + if (request !== undefined && request.referrerPolicy !== undefined) { + referrerPolicySet = true; + } + return new Promise((_, reject) => + reject(new Error('network error')) + ); + } + ); + const promise = _performApiRequest( + auth, + HttpMethod.POST, + Endpoint.SIGN_UP, + request + ); + await expect(promise).to.be.rejectedWith( + FirebaseError, + 'auth/network-request-failed' + ); + expect(referrerPolicySet).to.be.false; + sinon.restore(); + }); + }); + context('with network issues', () => { afterEach(mockFetch.tearDown); diff --git a/packages/auth/src/api/index.ts b/packages/auth/src/api/index.ts index 89c89bcdbe1..12d89b2bd7d 100644 --- a/packages/auth/src/api/index.ts +++ b/packages/auth/src/api/index.ts @@ -154,6 +154,10 @@ export async function _performApiRequest( ...body }; + /* Security-conscious server-side frameworks tend to have built in mitigations for referrer + problems". See the Cloudflare GitHub issue #487: Error: The 'referrerPolicy' field on + 'RequestInitializerDict' is not implemented." + https://github.com/cloudflare/next-on-pages/issues/487 */ if (!isCloudflareWorker()) { fetchArgs.referrerPolicy = 'no-referrer'; } From 73b6eca9f2fba5890c22005ee83e8fe820e103b1 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Mon, 9 Sep 2024 15:24:45 -0400 Subject: [PATCH 09/12] Revert removal from mock_fetch_tests --- packages/auth/test/helpers/mock_fetch.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/auth/test/helpers/mock_fetch.test.ts b/packages/auth/test/helpers/mock_fetch.test.ts index 5077adfca5c..3a04b7ce501 100644 --- a/packages/auth/test/helpers/mock_fetch.test.ts +++ b/packages/auth/test/helpers/mock_fetch.test.ts @@ -30,6 +30,7 @@ async function fetchJson(path: string, req?: object): Promise { headers: { 'Content-Type': 'application/json' }, + referrerPolicy: 'no-referrer', ...body }; From 9d220d62baa60e89e347364f7cf285ea76857ff3 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 10 Sep 2024 09:38:25 -0400 Subject: [PATCH 10/12] convert referrerPolicy var to let. --- packages/auth/src/api/index.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/auth/src/api/index.test.ts b/packages/auth/src/api/index.test.ts index c721971297e..e827c05cf23 100644 --- a/packages/auth/src/api/index.test.ts +++ b/packages/auth/src/api/index.test.ts @@ -314,8 +314,7 @@ describe('api/_performApiRequest', () => { it('should have referrerPolicy set', async () => { /* eslint-disable no-var */ - var referrerPolicySet: boolean = false; - /* eslint-enable no-var */ + let referrerPolicySet: boolean = false; mockFetch.setUpWithOverride( (input: RequestInfo | URL, request?: RequestInit) => { if (request !== undefined && request.referrerPolicy !== undefined) { @@ -342,8 +341,7 @@ describe('api/_performApiRequest', () => { it('should not have referrerPolicy set on Cloudflare workers', async () => { sinon.stub(utils, 'isCloudflareWorker').returns(true); /* eslint-disable no-var */ - var referrerPolicySet: boolean = false; - /* eslint-enable no-var */ + let referrerPolicySet: boolean = false; mockFetch.setUpWithOverride( (input: RequestInfo | URL, request?: RequestInit) => { if (request !== undefined && request.referrerPolicy !== undefined) { From 1629b935937595883e0714b931db4e58f13d4a75 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 10 Sep 2024 09:39:14 -0400 Subject: [PATCH 11/12] remove comment --- packages/auth/src/api/index.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/auth/src/api/index.test.ts b/packages/auth/src/api/index.test.ts index e827c05cf23..26bc2a5e1d6 100644 --- a/packages/auth/src/api/index.test.ts +++ b/packages/auth/src/api/index.test.ts @@ -313,7 +313,6 @@ describe('api/_performApiRequest', () => { afterEach(mockFetch.tearDown); it('should have referrerPolicy set', async () => { - /* eslint-disable no-var */ let referrerPolicySet: boolean = false; mockFetch.setUpWithOverride( (input: RequestInfo | URL, request?: RequestInit) => { @@ -340,7 +339,6 @@ describe('api/_performApiRequest', () => { it('should not have referrerPolicy set on Cloudflare workers', async () => { sinon.stub(utils, 'isCloudflareWorker').returns(true); - /* eslint-disable no-var */ let referrerPolicySet: boolean = false; mockFetch.setUpWithOverride( (input: RequestInfo | URL, request?: RequestInit) => { From b1f3860a45b06f44b764bedf71ff9ae60c6be0f3 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 10 Sep 2024 16:27:30 -0400 Subject: [PATCH 12/12] Update mocks to resolve not reject --- packages/auth/src/api/index.test.ts | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/packages/auth/src/api/index.test.ts b/packages/auth/src/api/index.test.ts index 26bc2a5e1d6..11070509d73 100644 --- a/packages/auth/src/api/index.test.ts +++ b/packages/auth/src/api/index.test.ts @@ -319,21 +319,16 @@ describe('api/_performApiRequest', () => { if (request !== undefined && request.referrerPolicy !== undefined) { referrerPolicySet = true; } - return new Promise((_, reject) => - reject(new Error('network error')) - ); + return Promise.resolve(new Response(JSON.stringify(serverResponse))); } ); - const promise = _performApiRequest( + const promise = _performApiRequest( auth, HttpMethod.POST, Endpoint.SIGN_UP, request ); - await expect(promise).to.be.rejectedWith( - FirebaseError, - 'auth/network-request-failed' - ); + await expect(promise).to.be.fulfilled; expect(referrerPolicySet).to.be.true; }); @@ -345,21 +340,16 @@ describe('api/_performApiRequest', () => { if (request !== undefined && request.referrerPolicy !== undefined) { referrerPolicySet = true; } - return new Promise((_, reject) => - reject(new Error('network error')) - ); + return Promise.resolve(new Response(JSON.stringify(serverResponse))); } ); - const promise = _performApiRequest( + const promise = _performApiRequest( auth, HttpMethod.POST, Endpoint.SIGN_UP, request ); - await expect(promise).to.be.rejectedWith( - FirebaseError, - 'auth/network-request-failed' - ); + await expect(promise).to.be.fulfilled; expect(referrerPolicySet).to.be.false; sinon.restore(); });