Skip to content

Commit

Permalink
Merge 2e00374 into e542f1d
Browse files Browse the repository at this point in the history
  • Loading branch information
dlarocque authored Jul 25, 2024
2 parents e542f1d + 2e00374 commit d717de4
Show file tree
Hide file tree
Showing 18 changed files with 143 additions and 89 deletions.
6 changes: 6 additions & 0 deletions .changeset/tender-apes-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/analytics': patch
'@firebase/app-check': patch
---

Revert introduction of safevalues to prevent issues from arising in Browser CommonJS environments due to ES5 incompatibility. For more information, see [GitHub PR #8395](https://github.com/firebase/firebase-js-sdk/pull/8395)
5 changes: 2 additions & 3 deletions packages/analytics/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,15 @@
"@firebase/logger": "0.4.2",
"@firebase/util": "1.9.7",
"@firebase/component": "0.6.8",
"tslib": "^2.1.0",
"safevalues": "0.6.0"
"tslib": "^2.1.0"
},
"license": "Apache-2.0",
"devDependencies": {
"@firebase/app": "0.10.7",
"rollup": "2.79.1",
"@rollup/plugin-commonjs": "21.1.0",
"@rollup/plugin-json": "4.1.0",
"@rollup/plugin-node-resolve": "13.3.0",
"rollup": "2.79.1",
"rollup-plugin-typescript2": "0.31.2",
"typescript": "4.7.4"
},
Expand Down
78 changes: 73 additions & 5 deletions packages/analytics/src/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,16 @@ import {
insertScriptTag,
wrapOrCreateGtag,
findGtagScriptOnPage,
promiseAllSettled
promiseAllSettled,
createGtagTrustedTypesScriptURL,
createTrustedTypesPolicy
} from './helpers';
import { GtagCommand } from './constants';
import { GtagCommand, GTAG_URL } from './constants';
import { Deferred } from '@firebase/util';
import { ConsentSettings } from './public-types';
import { removeGtagScripts } from '../testing/gtag-script-util';
import { logger } from './logger';
import { AnalyticsError, ERROR_FACTORY } from './errors';

const fakeMeasurementId = 'abcd-efgh-ijkl';
const fakeAppId = 'my-test-app-1234';
Expand All @@ -46,6 +50,71 @@ const fakeDynamicConfig: DynamicConfig = {
};
const fakeDynamicConfigPromises = [Promise.resolve(fakeDynamicConfig)];

describe('Trusted Types policies and functions', () => {
if (window.trustedTypes) {
describe('Trusted types exists', () => {
let ttStub: SinonStub;

beforeEach(() => {
ttStub = stub(
window.trustedTypes as TrustedTypePolicyFactory,
'createPolicy'
).returns({
createScriptURL: (s: string) => s
} as any);
});

afterEach(() => {
removeGtagScripts();
ttStub.restore();
});

it('Verify trustedTypes is called if the API is available', () => {
const trustedTypesPolicy = createTrustedTypesPolicy(
'firebase-js-sdk-policy',
{
createScriptURL: createGtagTrustedTypesScriptURL
}
);

expect(ttStub).to.be.called;
expect(trustedTypesPolicy).not.to.be.undefined;
});

it('createGtagTrustedTypesScriptURL verifies gtag URL base exists when a URL is provided', () => {
expect(createGtagTrustedTypesScriptURL(GTAG_URL)).to.equal(GTAG_URL);
});

it('createGtagTrustedTypesScriptURL rejects URLs with non-gtag base', () => {
const NON_GTAG_URL = 'http://iamnotgtag.com';
const loggerWarnStub = stub(logger, 'warn');
const errorMessage = ERROR_FACTORY.create(
AnalyticsError.INVALID_GTAG_RESOURCE,
{
gtagURL: NON_GTAG_URL
}
).message;

expect(createGtagTrustedTypesScriptURL(NON_GTAG_URL)).to.equal('');
expect(loggerWarnStub).to.be.calledWith(errorMessage);
});
});
}
describe('Trusted types does not exist', () => {
it('Verify trustedTypes functions are not called if the API is not available', () => {
delete window.trustedTypes;
const trustedTypesPolicy = createTrustedTypesPolicy(
'firebase-js-sdk-policy',
{
createScriptURL: createGtagTrustedTypesScriptURL
}
);

expect(trustedTypesPolicy).to.be.undefined;
});
});
});

describe('Gtag wrapping functions', () => {
afterEach(() => {
removeGtagScripts();
Expand All @@ -67,9 +136,8 @@ describe('Gtag wrapping functions', () => {
insertScriptTag(customDataLayerName, fakeMeasurementId);
const scriptTag = findGtagScriptOnPage(customDataLayerName);
expect(scriptTag).to.not.be.null;
expect(scriptTag!.src).to.equal(
`https://www.googletagmanager.com/gtag/js?l=${customDataLayerName}&id=${fakeMeasurementId}`
);
expect(scriptTag!.src).to.contain(`l=customDataLayerName`);
expect(scriptTag!.src).to.contain(`id=${fakeMeasurementId}`);
});

// The test above essentially already touches this functionality but it is still valuable
Expand Down
60 changes: 50 additions & 10 deletions packages/analytics/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,25 @@ import {
import { DynamicConfig, DataLayer, Gtag, MinimalDynamicConfig } from './types';
import { GtagCommand, GTAG_URL } from './constants';
import { logger } from './logger';
import { trustedResourceUrl } from 'safevalues';
import { safeScriptEl } from 'safevalues/dom';
import { AnalyticsError, ERROR_FACTORY } from './errors';

// Possible parameter types for gtag 'event' and 'config' commands
type GtagConfigOrEventParams = ControlParams & EventParams & CustomParams;

/**
* Verifies and creates a TrustedScriptURL.
*/
export function createGtagTrustedTypesScriptURL(url: string): string {
if (!url.startsWith(GTAG_URL)) {
const err = ERROR_FACTORY.create(AnalyticsError.INVALID_GTAG_RESOURCE, {
gtagURL: url
});
logger.warn(err.message);
return '';
}
return url;
}

/**
* Makeshift polyfill for Promise.allSettled(). Resolves when all promises
* have either resolved or rejected.
Expand All @@ -42,6 +55,29 @@ export function promiseAllSettled<T>(
return Promise.all(promises.map(promise => promise.catch(e => e)));
}

/**
* Creates a TrustedTypePolicy object that implements the rules passed as policyOptions.
*
* @param policyName A string containing the name of the policy
* @param policyOptions Object containing implementations of instance methods for TrustedTypesPolicy, see {@link https://developer.mozilla.org/en-US/docs/Web/API/TrustedTypePolicy#instance_methods
* | the TrustedTypePolicy reference documentation}.
*/
export function createTrustedTypesPolicy(
policyName: string,
policyOptions: Partial<TrustedTypePolicyOptions>
): Partial<TrustedTypePolicy> | undefined {
// Create a TrustedTypes policy that we can use for updating src
// properties
let trustedTypesPolicy: Partial<TrustedTypePolicy> | undefined;
if (window.trustedTypes) {
trustedTypesPolicy = window.trustedTypes.createPolicy(
policyName,
policyOptions
);
}
return trustedTypesPolicy;
}

/**
* Inserts gtag script tag into the page to asynchronously download gtag.
* @param dataLayerName Name of datalayer (most often the default, "_dataLayer").
Expand All @@ -50,17 +86,21 @@ export function insertScriptTag(
dataLayerName: string,
measurementId: string
): void {
const script = document.createElement('script');
const trustedTypesPolicy = createTrustedTypesPolicy(
'firebase-js-sdk-policy',
{
createScriptURL: createGtagTrustedTypesScriptURL
}
);

const script = document.createElement('script');
// We are not providing an analyticsId in the URL because it would trigger a `page_view`
// without fid. We will initialize ga-id using gtag (config) command together with fid.
//
// We also have to ensure the template string before the first expression constitutes a valid URL
// start, as this is what the initial validation focuses on. If the template literal begins
// directly with an expression (e.g. `${GTAG_SCRIPT_URL}`), the validation fails due to an
// empty initial string.
const url = trustedResourceUrl`https://www.googletagmanager.com/gtag/js?l=${dataLayerName}&id=${measurementId}`;
safeScriptEl.setSrc(script, url);

const gtagScriptURL = `${GTAG_URL}?l=${dataLayerName}&id=${measurementId}`;
(script.src as string | TrustedScriptURL) = trustedTypesPolicy
? (trustedTypesPolicy as TrustedTypePolicy)?.createScriptURL(gtagScriptURL)
: gtagScriptURL;

script.async = true;
document.head.appendChild(script);
Expand Down
1 change: 0 additions & 1 deletion packages/app-check/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
"@firebase/util": "1.9.7",
"@firebase/component": "0.6.8",
"@firebase/logger": "0.4.2",
"safevalues": "0.6.0",
"tslib": "^2.1.0"
},
"license": "Apache-2.0",
Expand Down
12 changes: 3 additions & 9 deletions packages/app-check/src/recaptcha.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ import {
initializeV3,
initializeEnterprise,
getToken,
GreCAPTCHATopLevel,
RECAPTCHA_ENTERPRISE_URL,
RECAPTCHA_URL
GreCAPTCHATopLevel
} from './recaptcha';
import * as utils from './util';
import {
Expand Down Expand Up @@ -83,9 +81,7 @@ describe('recaptcha', () => {

expect(findgreCAPTCHAScriptsOnPage().length).to.equal(0);
await initializeV3(app, FAKE_SITE_KEY);
const greCATPCHAScripts = findgreCAPTCHAScriptsOnPage();
expect(greCATPCHAScripts.length).to.equal(1);
expect(greCATPCHAScripts[0].src).to.equal(RECAPTCHA_URL);
expect(findgreCAPTCHAScriptsOnPage().length).to.equal(1);
});

it('creates invisible widget', async () => {
Expand Down Expand Up @@ -132,9 +128,7 @@ describe('recaptcha', () => {

expect(findgreCAPTCHAScriptsOnPage().length).to.equal(0);
await initializeEnterprise(app, FAKE_SITE_KEY);
const greCAPTCHAScripts = findgreCAPTCHAScriptsOnPage();
expect(greCAPTCHAScripts.length).to.equal(1);
expect(greCAPTCHAScripts[0].src).to.equal(RECAPTCHA_ENTERPRISE_URL);
expect(findgreCAPTCHAScriptsOnPage().length).to.equal(1);
});

it('creates invisible widget', async () => {
Expand Down
15 changes: 2 additions & 13 deletions packages/app-check/src/recaptcha.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,7 @@ import { FirebaseApp } from '@firebase/app';
import { getStateReference } from './state';
import { Deferred } from '@firebase/util';
import { getRecaptcha, ensureActivated } from './util';
import { trustedResourceUrl } from 'safevalues';
import { safeScriptEl } from 'safevalues/dom';

// Note that these are used for testing. If they are changed, the URLs used in loadReCAPTCHAV3Script
// and loadReCAPTCHAEnterpriseScript must also be changed. They aren't used to create the URLs
// since trusted resource URLs must be created using template string literals.
export const RECAPTCHA_URL = 'https://www.google.com/recaptcha/api.js';
export const RECAPTCHA_ENTERPRISE_URL =
'https://www.google.com/recaptcha/enterprise.js';
Expand Down Expand Up @@ -171,20 +166,14 @@ function renderInvisibleWidget(

function loadReCAPTCHAV3Script(onload: () => void): void {
const script = document.createElement('script');
safeScriptEl.setSrc(
script,
trustedResourceUrl`https://www.google.com/recaptcha/api.js`
);
script.src = RECAPTCHA_URL;
script.onload = onload;
document.head.appendChild(script);
}

function loadReCAPTCHAEnterpriseScript(onload: () => void): void {
const script = document.createElement('script');
safeScriptEl.setSrc(
script,
trustedResourceUrl`https://www.google.com/recaptcha/enterprise.js`
);
script.src = RECAPTCHA_ENTERPRISE_URL;
script.onload = onload;
document.head.appendChild(script);
}
Expand Down
3 changes: 1 addition & 2 deletions packages/auth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@
"@firebase/logger": "0.4.2",
"@firebase/util": "1.9.7",
"undici": "5.28.4",
"tslib": "^2.1.0",
"safevalues": "0.6.0"
"tslib": "^2.1.0"
},
"license": "Apache-2.0",
"devDependencies": {
Expand Down
4 changes: 0 additions & 4 deletions packages/auth/src/platform_browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,6 @@ _setExternalJSProvider({
// TODO: consider adding timeout support & cancellation
return new Promise((resolve, reject) => {
const el = document.createElement('script');
// TODO: Do not use setAttribute, since it can lead to XSS. Instead, use the safevalues library to
// safely set an attribute for a sanitized trustedResourceUrl. Since the trustedResourceUrl
// must be initialized from a template string literal, this could involve some heavy
// refactoring.
el.setAttribute('src', url);
el.onload = resolve;
el.onerror = e => {
Expand Down
4 changes: 0 additions & 4 deletions packages/auth/src/platform_browser/load_js.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ describe('platform-browser/load_js', () => {
loadJS(url: string): Promise<Event> {
return new Promise((resolve, reject) => {
const el = document.createElement('script');
// TODO: Do not use setAttribute, as this can lead to XSS. Instead, use the safevalues
// library, or get an exception for tests.
el.setAttribute('src', url);
el.onload = resolve;
el.onerror = e => {
Expand All @@ -67,8 +65,6 @@ describe('platform-browser/load_js', () => {

// eslint-disable-next-line @typescript-eslint/no-floating-promises
_loadJS('http://localhost/url');
// TODO: Do not use setAttribute, as this can lead to XSS. Instead, use the safevalues
// library, or get an exception for tests.
expect(el.setAttribute).to.have.been.calledWith(
'src',
'http://localhost/url'
Expand Down
10 changes: 2 additions & 8 deletions packages/auth/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
{
"extends": "../../config/tsconfig.base.json",
"compilerOptions": {
"outDir": "dist",
"plugins": [
{
"name": "tsec",
"reportTsecDiagnosticsOnly": true,
}
]
"outDir": "dist"
},
"exclude": [
"dist/**/*",
"demo/**/*"
]
}
}
10 changes: 2 additions & 8 deletions packages/database-compat/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,9 @@
"compilerOptions": {
"outDir": "dist",
"strict": false,
"downlevelIteration": true,
"plugins": [
{
"name": "tsec",
"reportTsecDiagnosticsOnly": true,
}
]
"downlevelIteration": true
},
"exclude": [
"dist/**/*"
]
}
}
1 change: 0 additions & 1 deletion packages/database/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
"@firebase/app-check-interop-types": "0.3.2",
"@firebase/auth-interop-types": "0.2.3",
"faye-websocket": "0.11.4",
"safevalues": "0.6.0",
"tslib": "^2.1.0"
},
"devDependencies": {
Expand Down
6 changes: 0 additions & 6 deletions packages/database/src/realtime/BrowserPollConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,6 @@ export class FirebaseIFrameScriptHolder {
const iframeContents = '<html><body>' + script + '</body></html>';
try {
this.myIFrame.doc.open();
// TODO: Do not use document.write, since it can lead to XSS. Instead, use the safevalues
// library to sanitize the HTML in the iframeContents.
this.myIFrame.doc.write(iframeContents);
this.myIFrame.doc.close();
} catch (e) {
Expand Down Expand Up @@ -719,10 +717,6 @@ export class FirebaseIFrameScriptHolder {
const newScript = this.myIFrame.doc.createElement('script');
newScript.type = 'text/javascript';
newScript.async = true;
// TODO: We cannot assign an arbitrary URL to a script attached to the DOM, since it is
// at risk of XSS. We should use the safevalues library to create a safeScriptEl, and
// assign a sanitized trustedResourceURL to it. Since the URL must be a template string
// literal, this could require some heavy refactoring.
newScript.src = url;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
newScript.onload = (newScript as any).onreadystatechange =
Expand Down
Loading

0 comments on commit d717de4

Please sign in to comment.