Skip to content

Add Node debug token support #4841

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.

Sign up for GitHub

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 5 commits into from
Apr 30, 2021
Merged

Add Node debug token support #4841

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8dd15b0
Add Node debug token support
hsubox76 Apr 28, 2021
dab0445
Figure out sketchy test
hsubox76 Apr 29, 2021
ab2e8c5
Figure out sketchy test 2
hsubox76 Apr 29, 2021
8e58f85
Revert test debugging changes
hsubox76 Apr 29, 2021
7cbf046
Add explanatory comment to database
hsubox76 Apr 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 10 additions & 14 deletions packages/app-check/src/debug.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,13 @@

import { getDebugState } from './state';
import { readOrCreateDebugTokenFromStorage } from './storage';
import { Deferred } from '@firebase/util';
import { Deferred, getGlobal } from '@firebase/util';

declare global {
interface Window {
/**
* When it is a string, we treat it as the debug token and give it to consumers as the app check token
* When it is `true`, we will try to read the debug token from the indexeddb,
* if it doesn't exist, create one, print it to console, and ask developers to register it in the Firebase console.
* When it is `undefined`, `false` or any unsupported value type, the SDK will operate in production mode
*/
FIREBASE_APPCHECK_DEBUG_TOKEN?: boolean | string;
}
// var must be used for global scopes
// https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html#type-checking-for-globalthis
// eslint-disable-next-line no-var
var FIREBASE_APPCHECK_DEBUG_TOKEN: boolean | string | undefined;
}

export function isDebugMode(): boolean {
Expand All @@ -50,9 +45,10 @@ export async function getDebugToken(): Promise<string> {
}

export function initializeDebugMode(): void {
const globals = getGlobal();
if (
typeof self.FIREBASE_APPCHECK_DEBUG_TOKEN !== 'string' &&
self.FIREBASE_APPCHECK_DEBUG_TOKEN !== true
typeof globals.FIREBASE_APPCHECK_DEBUG_TOKEN !== 'string' &&
globals.FIREBASE_APPCHECK_DEBUG_TOKEN !== true
) {
return;
}
Expand All @@ -62,8 +58,8 @@ export function initializeDebugMode(): void {
const deferredToken = new Deferred<string>();
debugState.token = deferredToken;

if (typeof self.FIREBASE_APPCHECK_DEBUG_TOKEN === 'string') {
deferredToken.resolve(self.FIREBASE_APPCHECK_DEBUG_TOKEN);
if (typeof globals.FIREBASE_APPCHECK_DEBUG_TOKEN === 'string') {
deferredToken.resolve(globals.FIREBASE_APPCHECK_DEBUG_TOKEN);
} else {
deferredToken.resolve(readOrCreateDebugTokenFromStorage());
}
Expand Down
1 change: 1 addition & 0 deletions packages/database/src/realtime/WebSocketConnection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ export class WebSocketConnection implements Transport {
}
};

// Adding a comment to ask questions
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this comment to place a discussion about this section. The next 5 lines were added in https://github.com/FirebasePrivate/firebase-js-sdk/pull/239/files#diff-ddbaa9e762a6010e5a0aa0c8048d48deea15594f14caa3d51e68b283f36e489fR171

I was wondering if I could get some more info about why only the Authorization header is sent if it's admin node, and only the AppCheck header is sent if it's client node? @schmidt-sebastian ?

To help answer the questions raised in the discussion started here: #4833 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

On node, we want to send the Authorization header if the user has admin credentials. I probably wrote this without knowing that there is a node client SDK. Assuming the node client SDK can use admin creds, this should be checking if the authToken looks like an admin credential.

For more context, if a client has admin credentials, we skip any appcheck-related checks on the server side, which is why we send admin creds in the header (and no appcheck token). If the client has any other type of credentials, they will eventually be sent after the connection is established, but they won't effect whether the initial request to establish a connection is allowed (only the appcheck header matters there).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. As far as I know, the Node client SDK should never have admin creds so looks like we'll always want to send the AppCheck token.

(Which would probably only be the dummy token or the debug token because Node can't do reCAPTCHA. Or possibly a token from a custom provider.)

if (this.nodeAdmin) {
options.headers['Authorization'] = this.authToken || '';
} else {
Expand Down
17 changes: 17 additions & 0 deletions packages/util/src/environment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,20 @@ export function areCookiesEnabled(): boolean {
}
return true;
}

/**
* Polyfill for `globalThis` object.
* @returns the `globalThis` object for the given environment.
*/
export function getGlobal(): typeof globalThis {
if (typeof self !== 'undefined') {
return self;
}
if (typeof window !== 'undefined') {
return window;
}
if (typeof global !== 'undefined') {
return global;
}
throw new Error('Unable to locate global object.');
}