From 6faaabb879822495d1b74db7e15781636ac9d8a6 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Thu, 12 Dec 2024 21:12:34 -0800 Subject: [PATCH 1/6] Use await import instead of require for functions modules that contain top-level awaits --- src/runtime/loader.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/runtime/loader.ts b/src/runtime/loader.ts index 953444e35..5c7af9553 100644 --- a/src/runtime/loader.ts +++ b/src/runtime/loader.ts @@ -48,8 +48,8 @@ async function loadModule(functionsDir: string) { try { return require(path.resolve(absolutePath)); } catch (e) { - if (e.code === "ERR_REQUIRE_ESM") { - // This is an ESM package! + if (e.code === "ERR_REQUIRE_ESM" || e.code === "ERR_REQUIRE_ASYNC_MODULE") { + // This is an ESM package, or one containing top-level awaits! const modulePath = require.resolve(absolutePath); // Resolve module path to file:// URL. Required for windows support. const moduleURL = url.pathToFileURL(modulePath).href; From 574af995fe5d3783796c1f4a8edddc738405bd33 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Fri, 13 Dec 2024 13:22:47 -0800 Subject: [PATCH 2/6] tests ensuring that es modules with top level awaits can be loaded --- scripts/bin-test/sources/esm-top-level-await/exports.js | 3 +++ scripts/bin-test/sources/esm-top-level-await/index.js | 8 ++++++++ scripts/bin-test/sources/esm-top-level-await/package.json | 4 ++++ 3 files changed, 15 insertions(+) create mode 100644 scripts/bin-test/sources/esm-top-level-await/exports.js create mode 100644 scripts/bin-test/sources/esm-top-level-await/index.js create mode 100644 scripts/bin-test/sources/esm-top-level-await/package.json diff --git a/scripts/bin-test/sources/esm-top-level-await/exports.js b/scripts/bin-test/sources/esm-top-level-await/exports.js new file mode 100644 index 000000000..50ece433e --- /dev/null +++ b/scripts/bin-test/sources/esm-top-level-await/exports.js @@ -0,0 +1,3 @@ +export const fn = () => { + return null; +} \ No newline at end of file diff --git a/scripts/bin-test/sources/esm-top-level-await/index.js b/scripts/bin-test/sources/esm-top-level-await/index.js new file mode 100644 index 000000000..05d2e5eca --- /dev/null +++ b/scripts/bin-test/sources/esm-top-level-await/index.js @@ -0,0 +1,8 @@ +import * as functionsv2 from "firebase-functions/v2"; + +const { fn } = await import('./exports.js'); + +export const v2http = functionsv2.https.onRequest((req, resp) => { + fn() + resp.status(200).send("PASS"); +}); diff --git a/scripts/bin-test/sources/esm-top-level-await/package.json b/scripts/bin-test/sources/esm-top-level-await/package.json new file mode 100644 index 000000000..9cb65cb9f --- /dev/null +++ b/scripts/bin-test/sources/esm-top-level-await/package.json @@ -0,0 +1,4 @@ +{ + "name": "esm", + "type": "module" +} From d93a613532e8d263e92d4f9bd293cb9812ea21ba Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Fri, 13 Dec 2024 13:26:27 -0800 Subject: [PATCH 3/6] adds node 22 to the CI matrix on github --- .github/workflows/test.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 686953002..036f93631 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -32,6 +32,7 @@ jobs: node-version: - 18.x - 20.x + - 22.x steps: - uses: actions/checkout@v1 - uses: actions/setup-node@v1 @@ -52,6 +53,7 @@ jobs: node-version: - 18.x - 20.x + - 22.x steps: - uses: actions/checkout@v1 - uses: actions/setup-node@v1 From ef07da592398d689abcb2c21e6b204bdeab9b014 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Fri, 13 Dec 2024 14:24:10 -0800 Subject: [PATCH 4/6] Add support for an authPolicy that returns Permission Denied when failed (#1650) * Add support for an authPolicy that returns Permission Denied when failed * Formatter * Changelog * remove ignorant comment --- CHANGELOG.md | 1 + package-lock.json | 2 +- spec/v2/providers/https.spec.ts | 85 +++++++++++++++++++++++++++++++++ src/common/providers/https.ts | 13 +++-- src/v2/providers/https.ts | 43 +++++++++++++++-- 5 files changed, 135 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb..605aabbf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Add an authPolicy callback to CallableOptions for reusable auth middleware as well as helper auth policies (#1650) diff --git a/package-lock.json b/package-lock.json index 1cb495cce..115a93eca 100644 --- a/package-lock.json +++ b/package-lock.json @@ -65,7 +65,7 @@ "node": ">=14.10.0" }, "peerDependencies": { - "firebase-admin": ">=11.0.0" + "firebase-admin": "^11.10.0 || ^12.0.0 || ^13.0.0" } }, "node_modules/@babel/parser": { diff --git a/spec/v2/providers/https.spec.ts b/spec/v2/providers/https.spec.ts index 77d69bfcc..8fabdf554 100644 --- a/spec/v2/providers/https.spec.ts +++ b/spec/v2/providers/https.spec.ts @@ -30,6 +30,7 @@ import { expectedResponseHeaders, MockRequest } from "../../fixtures/mockrequest import { runHandler } from "../../helper"; import { FULL_ENDPOINT, MINIMAL_V2_ENDPOINT, FULL_OPTIONS, FULL_TRIGGER } from "./fixtures"; import { onInit } from "../../../src/v2/core"; +import { Handler } from "express"; describe("onRequest", () => { beforeEach(() => { @@ -531,4 +532,88 @@ describe("onCall", () => { await runHandler(func, req as any); expect(hello).to.equal("world"); }); + + describe("authPolicy", () => { + function req(data: any, auth?: Record): any { + const headers = { + "content-type": "application/json", + }; + if (auth) { + headers["authorization"] = `bearer ignored.${Buffer.from( + JSON.stringify(auth), + "utf-8" + ).toString("base64")}.ignored`; + } + const ret = new MockRequest({ data }, headers); + ret.method = "POST"; + return ret; + } + + before(() => { + sinon.stub(debug, "isDebugFeatureEnabled").withArgs("skipTokenVerification").returns(true); + }); + + after(() => { + sinon.restore(); + }); + + it("should check isSignedIn", async () => { + const func = https.onCall( + { + authPolicy: https.isSignedIn(), + }, + () => 42 + ); + + const authResp = await runHandler(func, req(null, { sub: "inlined" })); + expect(authResp.status).to.equal(200); + + const anonResp = await runHandler(func, req(null, null)); + expect(anonResp.status).to.equal(403); + }); + + it("should check hasClaim", async () => { + const anyValue = https.onCall( + { + authPolicy: https.hasClaim("meaning"), + }, + () => "HHGTTG" + ); + const specificValue = https.onCall( + { + authPolicy: https.hasClaim("meaning", "42"), + }, + () => "HHGTG" + ); + + const cases: Array<{ fn: Handler; auth: null | Record; status: number }> = [ + { fn: anyValue, auth: { meaning: "42" }, status: 200 }, + { fn: anyValue, auth: { meaning: "43" }, status: 200 }, + { fn: anyValue, auth: { order: "66" }, status: 403 }, + { fn: anyValue, auth: null, status: 403 }, + { fn: specificValue, auth: { meaning: "42" }, status: 200 }, + { fn: specificValue, auth: { meaning: "43" }, status: 403 }, + { fn: specificValue, auth: { order: "66" }, status: 403 }, + { fn: specificValue, auth: null, status: 403 }, + ]; + for (const test of cases) { + const resp = await runHandler(test.fn, req(null, test.auth)); + expect(resp.status).to.equal(test.status); + } + }); + + it("can be any callback", async () => { + const divTwo = https.onCall( + { + authPolicy: (auth, data) => data % 2 === 0, + }, + (req) => req.data / 2 + ); + + const authorized = await runHandler(divTwo, req(2)); + expect(authorized.status).to.equal(200); + const accessDenied = await runHandler(divTwo, req(1)); + expect(accessDenied.status).to.equal(403); + }); + }); }); diff --git a/src/common/providers/https.ts b/src/common/providers/https.ts index 83eaba433..24300cf9d 100644 --- a/src/common/providers/https.ts +++ b/src/common/providers/https.ts @@ -703,10 +703,11 @@ type v2CallableHandler = ( ) => Res; /** @internal **/ -export interface CallableOptions { +export interface CallableOptions { cors: cors.CorsOptions; enforceAppCheck?: boolean; consumeAppCheckToken?: boolean; + authPolicy?: (token: AuthData | null, data: T) => boolean | Promise; /** * Time in seconds between sending heartbeat messages to keep the connection * alive. Set to `null` to disable heartbeats. @@ -718,7 +719,7 @@ export interface CallableOptions { /** @internal */ export function onCallHandler( - options: CallableOptions, + options: CallableOptions, handler: v1CallableHandler | v2CallableHandler, version: "gcfv1" | "gcfv2" ): (req: Request, res: express.Response) => Promise { @@ -739,7 +740,7 @@ function encodeSSE(data: unknown): string { /** @internal */ function wrapOnCallHandler( - options: CallableOptions, + options: CallableOptions, handler: v1CallableHandler | v2CallableHandler, version: "gcfv1" | "gcfv2" ): (req: Request, res: express.Response) => Promise { @@ -841,6 +842,12 @@ function wrapOnCallHandler( } const data: Req = decode(req.body.data); + if (options.authPolicy) { + const authorized = await options.authPolicy(context.auth ?? null, data); + if (!authorized) { + throw new HttpsError("permission-denied", "Permission Denied"); + } + } let result: Res; if (version === "gcfv1") { result = await (handler as v1CallableHandler)(data, context); diff --git a/src/v2/providers/https.ts b/src/v2/providers/https.ts index 321c31765..0a5b3e8c3 100644 --- a/src/v2/providers/https.ts +++ b/src/v2/providers/https.ts @@ -38,6 +38,7 @@ import { HttpsError, onCallHandler, Request, + AuthData, } from "../../common/providers/https"; import { initV2Endpoint, ManifestEndpoint } from "../../runtime/manifest"; import { GlobalOptions, SupportedRegion } from "../options"; @@ -166,7 +167,7 @@ export interface HttpsOptions extends Omit extends HttpsOptions { /** * Determines whether Firebase AppCheck is enforced. * When true, requests with invalid tokens autorespond with a 401 @@ -206,8 +207,39 @@ export interface CallableOptions extends HttpsOptions { * Defaults to 30 seconds. */ heartbeatSeconds?: number | null; + + /** + * Callback for whether a request is authorized. + * + * Designed to allow reusable auth policies to be passed as an options object. Two built-in reusable policies exist: + * isSignedIn and hasClaim. + */ + authPolicy?: (auth: AuthData | null, data: T) => boolean | Promise; } +/** + * An auth policy that requires a user to be signed in. + */ +export const isSignedIn = + () => + (auth: AuthData | null): boolean => + !!auth; + +/** + * An auth policy that requires a user to be both signed in and have a specific claim (optionally with a specific value) + */ +export const hasClaim = + (claim: string, value?: string) => + (auth: AuthData | null): boolean => { + if (!auth) { + return false; + } + if (!(claim in auth.token)) { + return false; + } + return !value || auth.token[claim] === value; + }; + /** * Handles HTTPS requests. */ @@ -233,6 +265,7 @@ export interface CallableFunction extends HttpsFunction { */ run(data: CallableRequest): Return; } + /** * Handles HTTPS requests. * @param opts - Options to set on this function @@ -355,7 +388,7 @@ export function onRequest( * @returns A function that you can export and deploy. */ export function onCall>( - opts: CallableOptions, + opts: CallableOptions, handler: (request: CallableRequest, response?: CallableProxyResponse) => Return ): CallableFunction ? Return : Promise>; @@ -368,7 +401,7 @@ export function onCall>( handler: (request: CallableRequest, response?: CallableProxyResponse) => Return ): CallableFunction ? Return : Promise>; export function onCall>( - optsOrHandler: CallableOptions | ((request: CallableRequest) => Return), + optsOrHandler: CallableOptions | ((request: CallableRequest) => Return), handler?: (request: CallableRequest, response?: CallableProxyResponse) => Return ): CallableFunction ? Return : Promise> { let opts: CallableOptions; @@ -388,14 +421,14 @@ export function onCall>( } // fix the length of handler to make the call to handler consistent - const fixedLen = (req: CallableRequest, resp?: CallableProxyResponse) => - withInit(handler)(req, resp); + const fixedLen = (req: CallableRequest, resp?: CallableProxyResponse) => handler(req, resp); let func: any = onCallHandler( { cors: { origin, methods: "POST" }, enforceAppCheck: opts.enforceAppCheck ?? options.getGlobalOptions().enforceAppCheck, consumeAppCheckToken: opts.consumeAppCheckToken, heartbeatSeconds: opts.heartbeatSeconds, + authPolicy: opts.authPolicy, }, fixedLen, "gcfv2" From 94d04a87cb3c6970a4cfed52e08d5b456c5730e6 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Sat, 14 Dec 2024 10:46:12 -0800 Subject: [PATCH 5/6] Update streaming callable API (#1652) * Update streaming callable API * Fix linter error * Stream type defaults to unknown * Changelog * Format fix --- CHANGELOG.md | 1 + spec/common/providers/https.spec.ts | 12 ++--- spec/helper.ts | 5 +- src/common/providers/https.ts | 75 ++++++++++++++++++----------- src/v2/providers/https.ts | 38 ++++++++++----- 5 files changed, 84 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 605aabbf3..2446e5f49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1 +1,2 @@ - Add an authPolicy callback to CallableOptions for reusable auth middleware as well as helper auth policies (#1650) +- Multiple breaking changes to the not-yet-announced streaming feature for Callable Functions (#1652) diff --git a/spec/common/providers/https.spec.ts b/spec/common/providers/https.spec.ts index ef77a6fa6..86e1b8696 100644 --- a/spec/common/providers/https.spec.ts +++ b/spec/common/providers/https.spec.ts @@ -770,7 +770,7 @@ describe("onCallHandler", () => { cors: { origin: true, methods: "POST" }, }, (req, resp) => { - resp.write("hello"); + resp.sendChunk("hello"); return "world"; }, "gcfv2" @@ -840,10 +840,10 @@ describe("onCallHandler", () => { { cors: { origin: true, methods: "POST" }, }, - (req, resp) => { - resp.write("initial message"); - mockReq.emit("close"); - resp.write("should not be sent"); + async (req, resp) => { + await resp.sendChunk("initial message"); + await mockReq.emit("close"); + await resp.sendChunk("should not be sent"); return "done"; }, "gcfv2" @@ -908,7 +908,7 @@ describe("onCallHandler", () => { }, async (resp, res) => { await new Promise((resolve) => setTimeout(resolve, 3_000)); - res.write("hello"); + res.sendChunk("hello"); await new Promise((resolve) => setTimeout(resolve, 3_000)); return "done"; }, diff --git a/spec/helper.ts b/spec/helper.ts index 5c8ca0c76..04829be38 100644 --- a/spec/helper.ts +++ b/spec/helper.ts @@ -84,8 +84,11 @@ export function runHandler( } } - public write(writeBody: any) { + public write(writeBody: any, cb?: () => void) { this.sentBody += typeof writeBody === "object" ? JSON.stringify(writeBody) : writeBody; + if (cb) { + setImmediate(cb); + } return true; } diff --git a/src/common/providers/https.ts b/src/common/providers/https.ts index 24300cf9d..a6989f130 100644 --- a/src/common/providers/https.ts +++ b/src/common/providers/https.ts @@ -141,23 +141,30 @@ export interface CallableRequest { * The raw request handled by the callable. */ rawRequest: Request; + + /** + * Whether this is a streaming request. + * Code can be optimized by not trying to generate a stream of chunks to + * call response.sendChunk on if request.acceptsStreaming is false. + * It is always safe, however, to call response.sendChunk as this will + * noop if acceptsStreaming is false. + */ + acceptsStreaming: boolean; } /** - * CallableProxyResponse exposes subset of express.Response object - * to allow writing partial, streaming responses back to the client. + * CallableProxyResponse allows streaming response chunks and listening to signals + * triggered in events such as a disconnect. */ -export interface CallableProxyResponse { +export interface CallableResponse { /** * Writes a chunk of the response body to the client. This method can be called * multiple times to stream data progressively. + * Returns a promise of whether the data was written. This can be false, for example, + * if the request was not a streaming request. Rejects if there is a network error. */ - write: express.Response["write"]; - /** - * Indicates whether the client has requested and can handle streaming responses. - * This should be checked before attempting to stream data to avoid compatibility issues. - */ - acceptsStreaming: boolean; + sendChunk: (chunk: T) => Promise; + /** * An AbortSignal that is triggered when the client disconnects or the * request is terminated prematurely. @@ -586,13 +593,9 @@ async function checkTokens( auth: "INVALID", }; - await Promise.all([ - Promise.resolve().then(async () => { - verifications.auth = await checkAuthToken(req, ctx); - }), - Promise.resolve().then(async () => { - verifications.app = await checkAppCheckToken(req, ctx, options); - }), + [verifications.auth, verifications.app] = await Promise.all([ + checkAuthToken(req, ctx), + checkAppCheckToken(req, ctx, options), ]); const logPayload = { @@ -697,9 +700,9 @@ async function checkAppCheckToken( } type v1CallableHandler = (data: any, context: CallableContext) => any | Promise; -type v2CallableHandler = ( +type v2CallableHandler = ( request: CallableRequest, - response?: CallableProxyResponse + response?: CallableResponse ) => Res; /** @internal **/ @@ -718,9 +721,9 @@ export interface CallableOptions { } /** @internal */ -export function onCallHandler( +export function onCallHandler( options: CallableOptions, - handler: v1CallableHandler | v2CallableHandler, + handler: v1CallableHandler | v2CallableHandler, version: "gcfv1" | "gcfv2" ): (req: Request, res: express.Response) => Promise { const wrapped = wrapOnCallHandler(options, handler, version); @@ -739,9 +742,9 @@ function encodeSSE(data: unknown): string { } /** @internal */ -function wrapOnCallHandler( +function wrapOnCallHandler( options: CallableOptions, - handler: v1CallableHandler | v2CallableHandler, + handler: v1CallableHandler | v2CallableHandler, version: "gcfv1" | "gcfv2" ): (req: Request, res: express.Response) => Promise { return async (req: Request, res: express.Response): Promise => { @@ -855,27 +858,41 @@ function wrapOnCallHandler( const arg: CallableRequest = { ...context, data, + acceptsStreaming, }; - const responseProxy: CallableProxyResponse = { - write(chunk): boolean { + const responseProxy: CallableResponse = { + sendChunk(chunk: Stream): Promise { // if client doesn't accept sse-protocol, response.write() is no-op. if (!acceptsStreaming) { - return false; + return Promise.resolve(false); } // if connection is already closed, response.write() is no-op. if (abortController.signal.aborted) { - return false; + return Promise.resolve(false); } const formattedData = encodeSSE({ message: chunk }); - const wrote = res.write(formattedData); + let resolve: (wrote: boolean) => void; + let reject: (err: Error) => void; + const p = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + const wrote = res.write(formattedData, (error) => { + if (error) { + reject(error); + return; + } + resolve(wrote); + }); + // Reset heartbeat timer after successful write if (wrote && heartbeatInterval !== null && heartbeatSeconds > 0) { scheduleHeartbeat(); } - return wrote; + + return p; }, - acceptsStreaming, signal: abortController.signal, }; if (acceptsStreaming) { diff --git a/src/v2/providers/https.ts b/src/v2/providers/https.ts index 0a5b3e8c3..e59ce9704 100644 --- a/src/v2/providers/https.ts +++ b/src/v2/providers/https.ts @@ -33,7 +33,7 @@ import { isDebugFeatureEnabled } from "../../common/debug"; import { ResetValue } from "../../common/options"; import { CallableRequest, - CallableProxyResponse, + CallableResponse, FunctionsErrorCode, HttpsError, onCallHandler, @@ -258,12 +258,17 @@ export type HttpsFunction = (( /** * Creates a callable method for clients to call using a Firebase SDK. */ -export interface CallableFunction extends HttpsFunction { +export interface CallableFunction extends HttpsFunction { /** Executes the handler function with the provided data as input. Used for unit testing. * @param data - An input for the handler function. * @returns The output of the handler function. */ - run(data: CallableRequest): Return; + run(request: CallableRequest): Return; + + stream( + request: CallableRequest, + response: CallableResponse + ): { stream: AsyncIterator; output: Return }; } /** @@ -387,22 +392,22 @@ export function onRequest( * @param handler - A function that takes a {@link https.CallableRequest}. * @returns A function that you can export and deploy. */ -export function onCall>( +export function onCall, Stream = unknown>( opts: CallableOptions, - handler: (request: CallableRequest, response?: CallableProxyResponse) => Return -): CallableFunction ? Return : Promise>; + handler: (request: CallableRequest, response?: CallableResponse) => Return +): CallableFunction ? Return : Promise, Stream>; /** * Declares a callable method for clients to call using a Firebase SDK. * @param handler - A function that takes a {@link https.CallableRequest}. * @returns A function that you can export and deploy. */ -export function onCall>( - handler: (request: CallableRequest, response?: CallableProxyResponse) => Return +export function onCall, Stream = unknown>( + handler: (request: CallableRequest, response?: CallableResponse) => Return ): CallableFunction ? Return : Promise>; -export function onCall>( +export function onCall, Stream = unknown>( optsOrHandler: CallableOptions | ((request: CallableRequest) => Return), - handler?: (request: CallableRequest, response?: CallableProxyResponse) => Return + handler?: (request: CallableRequest, response?: CallableResponse) => Return ): CallableFunction ? Return : Promise> { let opts: CallableOptions; if (arguments.length === 1) { @@ -421,7 +426,7 @@ export function onCall>( } // fix the length of handler to make the call to handler consistent - const fixedLen = (req: CallableRequest, resp?: CallableProxyResponse) => handler(req, resp); + const fixedLen = (req: CallableRequest, resp?: CallableResponse) => handler(req, resp); let func: any = onCallHandler( { cors: { origin, methods: "POST" }, @@ -474,6 +479,17 @@ export function onCall>( callableTrigger: {}, }; + // TODO: in the next major version, do auth/appcheck in these helper methods too. func.run = withInit(handler); + func.stream = () => { + return { + stream: { + next(): Promise> { + return Promise.reject("Coming soon"); + }, + }, + output: Promise.reject("Coming soon"), + }; + }; return func; } From c8429576bc5c023d528fd0fc2210d28a56e4d2e6 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 18 Dec 2024 16:25:49 -0800 Subject: [PATCH 6/6] update changelog.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2446e5f49..26846cc7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,2 +1,3 @@ - Add an authPolicy callback to CallableOptions for reusable auth middleware as well as helper auth policies (#1650) +- Handle ESM functions codebases containing top-level awaits, which would break in node 22.12+ (#1651) - Multiple breaking changes to the not-yet-announced streaming feature for Callable Functions (#1652)