Skip to content

Remove referrerPolicy from Cloudflare Worker fetch requests #8393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Sep 11, 2024
Merged
6 changes: 6 additions & 0 deletions .changeset/khaki-numbers-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/auth': patch
'@firebase/util': minor
---

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.
5 changes: 5 additions & 0 deletions common/api-review/util.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ export function isBrowser(): boolean;
// @public (undocumented)
export function isBrowserExtension(): boolean;

// 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 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)
//
// @public
Expand Down
47 changes: 47 additions & 0 deletions packages/auth/src/api/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -308,6 +309,52 @@ describe('api/_performApiRequest', () => {
});
});

context('referer policy exists on fetch request', () => {
afterEach(mockFetch.tearDown);

it('should have referrerPolicy set', async () => {
let referrerPolicySet: boolean = false;
mockFetch.setUpWithOverride(
(input: RequestInfo | URL, request?: RequestInit) => {
if (request !== undefined && request.referrerPolicy !== undefined) {
referrerPolicySet = true;
}
return Promise.resolve(new Response(JSON.stringify(serverResponse)));
}
);
const promise = _performApiRequest<typeof request, typeof serverResponse>(
auth,
HttpMethod.POST,
Endpoint.SIGN_UP,
request
);
await expect(promise).to.be.fulfilled;
expect(referrerPolicySet).to.be.true;
});

it('should not have referrerPolicy set on Cloudflare workers', async () => {
sinon.stub(utils, 'isCloudflareWorker').returns(true);
let referrerPolicySet: boolean = false;
mockFetch.setUpWithOverride(
(input: RequestInfo | URL, request?: RequestInit) => {
if (request !== undefined && request.referrerPolicy !== undefined) {
referrerPolicySet = true;
}
return Promise.resolve(new Response(JSON.stringify(serverResponse)));
}
);
const promise = _performApiRequest<typeof request, typeof serverResponse>(
auth,
HttpMethod.POST,
Endpoint.SIGN_UP,
request
);
await expect(promise).to.be.fulfilled;
expect(referrerPolicySet).to.be.false;
sinon.restore();
});
});

context('with network issues', () => {
afterEach(mockFetch.tearDown);

Expand Down
23 changes: 16 additions & 7 deletions packages/auth/src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import { FirebaseError, querystring } from '@firebase/util';
import { FirebaseError, isCloudflareWorker, querystring } from '@firebase/util';

import { AuthErrorCode, NamedErrorParams } from '../core/errors';
import {
Expand Down Expand Up @@ -148,14 +148,23 @@ export async function _performApiRequest<T, V>(
headers[HttpHeader.X_FIREBASE_LOCALE] = auth.languageCode;
}

const fetchArgs: RequestInit = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should consider adding some unit tests on _performApiRequest since we introduced new logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

method,
headers,
...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';
}

return FetchProvider.fetch()(
_getFinalTarget(auth, auth.config.apiHost, path, query),
{
method,
headers,
referrerPolicy: 'no-referrer',
...body
}
fetchArgs
);
});
}
Expand Down
14 changes: 12 additions & 2 deletions packages/util/src/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -89,7 +89,7 @@ export function isBrowser(): boolean {
}

/**
* Detect Web Worker context
* Detect Web Worker context.
*/
export function isWebWorker(): boolean {
return (
Expand All @@ -99,6 +99,16 @@ export function isWebWorker(): boolean {
);
}

/**
* Detect Cloudflare Worker context.
*/
export function isCloudflareWorker(): boolean {
return (
typeof navigator !== 'undefined' &&
navigator.userAgent === 'Cloudflare-Workers'
);
}

/**
* Detect browser extensions (Chrome and Firefox at least).
*/
Expand Down
Loading