From bda577ea469b41947697d4fa05eabbc554c2f846 Mon Sep 17 00:00:00 2001 From: Stefan Pfaffel Date: Fri, 12 Aug 2022 08:29:00 +0200 Subject: [PATCH 1/5] fix: prioritize boolean cors value over debug flags As discussed in https://github.com/firebase/firebase-tools/issues/4862 it's not possible to disable cors in v2 functions, because the emulator enables the debug feature for cors overriding `opts.cors`. If we could prioritize the boolean flag over the debug feature, we could have both: The ability to disable cors and also prioritize the debug flag over user provided configuration. --- spec/v2/providers/https.spec.ts | 32 +++++++++++++++++++++++++++++++- src/v2/providers/https.ts | 2 +- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/spec/v2/providers/https.spec.ts b/spec/v2/providers/https.spec.ts index 65e573849..9f2c876b8 100644 --- a/spec/v2/providers/https.spec.ts +++ b/spec/v2/providers/https.spec.ts @@ -28,7 +28,7 @@ import * as options from '../../../src/v2/options'; import * as https from '../../../src/v2/providers/https'; import { expectedResponseHeaders, - MockRequest, + MockRequest } from '../../fixtures/mockrequest'; import { runHandler } from '../../helper'; import { FULL_ENDPOINT, FULL_OPTIONS, FULL_TRIGGER } from './fixtures'; @@ -218,6 +218,36 @@ describe('onRequest', () => { sinon.restore(); }); + + it('should NOT add CORS headers if debug feature is enabled and cors has value false', async () => { + sinon + .stub(debug, 'isDebugFeatureEnabled') + .withArgs('enableCors') + .returns(true); + + const func = https.onRequest({ cors: false }, (req, res) => { + res.status(200).send('Good') + }); + + const req = new MockRequest( + { + data: {}, + }, + { + 'Access-Control-Request-Method': 'POST', + 'Access-Control-Request-Headers': 'origin', + origin: 'example.com', + } + ); + req.method = 'OPTIONS'; + + const resp = await runHandler(func, req as any); + expect(resp.status).to.equal(200); + expect(resp.body).to.be.equal('Good'); + expect(resp.headers).to.deep.equal({}); + + sinon.restore(); + }); }); describe('onCall', () => { diff --git a/src/v2/providers/https.ts b/src/v2/providers/https.ts index 0ab9c531c..6cb847053 100644 --- a/src/v2/providers/https.ts +++ b/src/v2/providers/https.ts @@ -220,7 +220,7 @@ export function onRequest( opts = optsOrHandler as HttpsOptions; } - if (isDebugFeatureEnabled('enableCors') || 'cors' in opts) { + if (opts.cors !== false && (isDebugFeatureEnabled('enableCors') || 'cors' in opts)) { const origin = isDebugFeatureEnabled('enableCors') ? true : opts.cors; const userProvidedHandler = handler; handler = (req: Request, res: express.Response): void | Promise => { From 6fdd887ef2ac49fc560099f098b5ae8085625563 Mon Sep 17 00:00:00 2001 From: Stefan Pfaffel Date: Thu, 25 Aug 2022 21:33:04 +0200 Subject: [PATCH 2/5] chore: use less conditions to check desired cors state Co-authored-by: Daniel Lee --- src/v2/providers/https.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/v2/providers/https.ts b/src/v2/providers/https.ts index 6cb847053..e5c30e00b 100644 --- a/src/v2/providers/https.ts +++ b/src/v2/providers/https.ts @@ -35,7 +35,7 @@ import { FunctionsErrorCode, HttpsError, onCallHandler, - Request, + Request } from '../../common/providers/https'; import { ManifestEndpoint } from '../../runtime/manifest'; import * as options from '../options'; @@ -220,8 +220,13 @@ export function onRequest( opts = optsOrHandler as HttpsOptions; } - if (opts.cors !== false && (isDebugFeatureEnabled('enableCors') || 'cors' in opts)) { - const origin = isDebugFeatureEnabled('enableCors') ? true : opts.cors; + if (isDebugFeatureEnabled('enableCors') || 'cors' in opts) { + let origin = opts.cors; + if (isDebugFeatureEnabled('enableCors')) { + // Respect `cors: false` to turn off cors even if debug feature is enabled. + origin = opts.cors === false ? false : true + } + const userProvidedHandler = handler; handler = (req: Request, res: express.Response): void | Promise => { return new Promise((resolve) => { From 055c289c03f0de0f6ad9d9c8276c8d83a4566c57 Mon Sep 17 00:00:00 2001 From: Stefan Pfaffel Date: Fri, 26 Aug 2022 08:06:04 +0200 Subject: [PATCH 3/5] style: format --- src/v2/providers/https.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/v2/providers/https.ts b/src/v2/providers/https.ts index e5c30e00b..32df1d204 100644 --- a/src/v2/providers/https.ts +++ b/src/v2/providers/https.ts @@ -224,7 +224,7 @@ export function onRequest( let origin = opts.cors; if (isDebugFeatureEnabled('enableCors')) { // Respect `cors: false` to turn off cors even if debug feature is enabled. - origin = opts.cors === false ? false : true + origin = opts.cors === false ? false : true } const userProvidedHandler = handler; @@ -330,8 +330,8 @@ export function onCall>( const origin = isDebugFeatureEnabled('enableCors') ? true : 'cors' in opts - ? opts.cors - : true; + ? opts.cors + : true; // onCallHandler sniffs the function length to determine which API to present. // fix the length to prevent api versions from being mismatched. From a49bf3067a5e1276060903ae4ea5ef0a81f5d842 Mon Sep 17 00:00:00 2001 From: Stefan Pfaffel Date: Sun, 11 Sep 2022 21:36:03 +0200 Subject: [PATCH 4/5] chore: run prettier to follow formatting rules --- src/v2/providers/https.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/v2/providers/https.ts b/src/v2/providers/https.ts index 32df1d204..1c4cdf7ce 100644 --- a/src/v2/providers/https.ts +++ b/src/v2/providers/https.ts @@ -35,7 +35,7 @@ import { FunctionsErrorCode, HttpsError, onCallHandler, - Request + Request, } from '../../common/providers/https'; import { ManifestEndpoint } from '../../runtime/manifest'; import * as options from '../options'; @@ -224,7 +224,7 @@ export function onRequest( let origin = opts.cors; if (isDebugFeatureEnabled('enableCors')) { // Respect `cors: false` to turn off cors even if debug feature is enabled. - origin = opts.cors === false ? false : true + origin = opts.cors === false ? false : true; } const userProvidedHandler = handler; @@ -330,8 +330,8 @@ export function onCall>( const origin = isDebugFeatureEnabled('enableCors') ? true : 'cors' in opts - ? opts.cors - : true; + ? opts.cors + : true; // onCallHandler sniffs the function length to determine which API to present. // fix the length to prevent api versions from being mismatched. From ba3ffc67cbdc2304c6906e8d2ee8a16503a799c3 Mon Sep 17 00:00:00 2001 From: Stefan Pfaffel Date: Thu, 15 Dec 2022 18:33:16 +0100 Subject: [PATCH 5/5] style: format https spec file --- spec/v2/providers/https.spec.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/spec/v2/providers/https.spec.ts b/spec/v2/providers/https.spec.ts index 844de018d..1d7d30ae3 100644 --- a/spec/v2/providers/https.spec.ts +++ b/spec/v2/providers/https.spec.ts @@ -216,14 +216,11 @@ describe("onRequest", () => { sinon.restore(); }); - it('should NOT add CORS headers if debug feature is enabled and cors has value false', async () => { - sinon - .stub(debug, 'isDebugFeatureEnabled') - .withArgs('enableCors') - .returns(true); + it("should NOT add CORS headers if debug feature is enabled and cors has value false", async () => { + sinon.stub(debug, "isDebugFeatureEnabled").withArgs("enableCors").returns(true); const func = https.onRequest({ cors: false }, (req, res) => { - res.status(200).send('Good') + res.status(200).send("Good"); }); const req = new MockRequest( @@ -231,16 +228,16 @@ describe("onRequest", () => { data: {}, }, { - 'Access-Control-Request-Method': 'POST', - 'Access-Control-Request-Headers': 'origin', - origin: 'example.com', + "Access-Control-Request-Method": "POST", + "Access-Control-Request-Headers": "origin", + origin: "example.com", } ); - req.method = 'OPTIONS'; + req.method = "OPTIONS"; const resp = await runHandler(func, req as any); expect(resp.status).to.equal(200); - expect(resp.body).to.be.equal('Good'); + expect(resp.body).to.be.equal("Good"); expect(resp.headers).to.deep.equal({}); sinon.restore();