-
Notifications
You must be signed in to change notification settings - Fork 205
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
Add debug mode #992
Add debug mode #992
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks cool 😎
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks cool 😎
src/debug.ts
Outdated
// SOFTWARE. | ||
|
||
// Do NOT turn on a debug feature in production. | ||
const debugMode = process.env.FIREBASE_DEBUG_MODE === 'true'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also check that FIREBASE_DEBUG_FEATURES exists? Is having two env vars just used for some security?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I thought extra, repeated step makes it slightly harder to accidentally invoke a debug mode.
Another thing I was trying to do here is to make sure that we minimize the number of environment variable read. Apparently, reading env var is not the cheapest thing in Nodejs, so this guard ensures that we issue env var per request only when the debug mode is turned on.
src/debug.ts
Outdated
const debugMode = process.env.FIREBASE_DEBUG_MODE === 'true'; | ||
|
||
interface DebugFeatures { | ||
skipCallableTokenVerification?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we also will have a disableTimeouts, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, but I plan on adding it as needed.
src/debug.ts
Outdated
@@ -0,0 +1,52 @@ | |||
// The MIT License (MIT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this should go into src/common since most of the stuff under src/ will go into v1 in the next major change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great call. Moving.
To replace monkey-patching of the Firebase Functions SDK in the Functions Emulator ([code](https://github.com/firebase/firebase-tools/blob/c2feb0836f6f64236e117f2906ef6083840e212b/src/emulator/functionsEmulatorRuntime.ts#L401-L496)), we provide native support for bypassing token verification for `onCall` handlers. Using the new debug mode introduced in #992, Auth/App Check token included in the request will be decoded but no verified.
Debug mode is intended to be used by the Functions Emulator to enable/disable certain aspect of the Functions SDK. Debug mode allows the Functions SDK to operate in development friendly way without the need to monkey-patch the Functions SDK. For example, auth or App Check token verification can be disabled during development to make Functions SDK compatible with the Auth Emulator.
This feature is intended for internal use-cases and won't expose any public interfaces. No one outside the Firebase team should use it, and if they do, we can't promise any stability or feature support.