From 2952e2f2e4634d08da5ddd112d137652b46ff3ce Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 14 Sep 2021 12:34:52 -0700 Subject: [PATCH 1/6] Initialize debug mode on initializeAppCheck --- packages/app-check/src/api.test.ts | 40 +++++++++++++++++++++++++++++- packages/app-check/src/api.ts | 20 +++++++++++++++ packages/app-check/src/index.ts | 2 -- 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index 3e435f5b0cb..0b55e98d791 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -16,7 +16,7 @@ */ import '../test/setup'; import { expect } from 'chai'; -import { spy, stub } from 'sinon'; +import { match, spy, stub } from 'sinon'; import { setTokenAutoRefreshEnabled, initializeAppCheck, @@ -38,10 +38,12 @@ import * as logger from './logger'; import * as client from './client'; import * as storage from './storage'; import * as internalApi from './internal-api'; +import * as indexeddb from './indexeddb'; import { deleteApp, FirebaseApp } from '@firebase/app'; import { CustomProvider, ReCaptchaV3Provider } from './providers'; import { AppCheckService } from './factory'; import { AppCheckToken } from './public-types'; +import { getDebugToken } from './debug'; describe('api', () => { let app: FirebaseApp; @@ -118,6 +120,42 @@ describe('api', () => { }) ).to.equal(appCheckInstance); }); + it('starts debug mode on first call', async () => { + const fakeWrite = ():Promise => Promise.resolve(); + const writeToIndexedDBStub = stub( + indexeddb, + 'writeDebugTokenToIndexedDB' + ).callsFake(fakeWrite); + stub( + indexeddb, + 'readDebugTokenFromIndexedDB' + ).callsFake(() => Promise.resolve(undefined)); + const logStub = stub(logger.logger, 'warn'); + const consoleStub = stub(console, 'log'); + self.FIREBASE_APPCHECK_DEBUG_TOKEN = true; + initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + await fakeWrite(); + const token = writeToIndexedDBStub.args[0]; + expect(logStub).to.not.be.called; + expect(consoleStub.args[0][0]).to.include(token); + self.FIREBASE_APPCHECK_DEBUG_TOKEN = undefined; + }); + it('warns about debug mode on second call', async () => { + const logStub = stub(logger.logger, 'warn'); + self.FIREBASE_APPCHECK_DEBUG_TOKEN = 'abcdefg'; + initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + const token = await getDebugToken(); + expect(token).to.equal('abcdefg'); + expect(logStub).to.be.calledWith(match('abcdefg')); + self.FIREBASE_APPCHECK_DEBUG_TOKEN = undefined; + }); it('initialize reCAPTCHA when a ReCaptchaV3Provider is provided', () => { const initReCAPTCHAStub = stub(reCAPTCHA, 'initialize').returns( diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index 48ed326018c..b1a7b750b32 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -35,6 +35,8 @@ import { isValid } from './internal-api'; import { readTokenFromStorage } from './storage'; +import { getDebugToken, initializeDebugMode, isDebugMode } from './debug'; +import { logger } from './logger'; declare module '@firebase/component' { interface NameServiceMapping { @@ -65,6 +67,21 @@ export function initializeAppCheck( options.isTokenAutoRefreshEnabled && initialOptions.provider.isEqual(options.provider) ) { + // Log a warning if `initializeAppCheck()` is called after the first time + // if this app is still in debug mode, and show the token. + // If it's the first time, `initializeDebugMode()` already logs a message. + if (isDebugMode()) { + logger.warn( + `App Check is in debug mode. To turn off debug mode, unset ` + + `the global variable FIREBASE_APPCHECK_DEBUG_TOKEN and ` + + `restart the app.` + ); + // Make this a separate console statement so user will at least have the + // first message if the token promise doesn't resolve in time. + void getDebugToken().then(token => + logger.warn(`Debug token is ${token}.`) + ); + } return existingInstance; } else { throw ERROR_FACTORY.create(AppCheckError.ALREADY_INITIALIZED, { @@ -73,6 +90,9 @@ export function initializeAppCheck( } } + // Only read global variable on first call to `initializeAppCheck()`. + initializeDebugMode(); + const appCheck = provider.initialize({ options }); _activate(app, options.provider, options.isTokenAutoRefreshEnabled); diff --git a/packages/app-check/src/index.ts b/packages/app-check/src/index.ts index 8272f8fbece..96e7e82eb01 100644 --- a/packages/app-check/src/index.ts +++ b/packages/app-check/src/index.ts @@ -28,7 +28,6 @@ import { } from '@firebase/component'; import { _AppCheckComponentName } from './public-types'; import { factory, internalFactory } from './factory'; -import { initializeDebugMode } from './debug'; import { _AppCheckInternalComponentName } from './types'; import { name, version } from '../package.json'; @@ -82,4 +81,3 @@ function registerAppCheck(): void { } registerAppCheck(); -initializeDebugMode(); From 86331a515d2a73991920f686be5d3734eac5d8d7 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 14 Sep 2021 12:35:03 -0700 Subject: [PATCH 2/6] prettier --- packages/app-check/src/api.test.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index 0b55e98d791..d407d7dd687 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -121,15 +121,14 @@ describe('api', () => { ).to.equal(appCheckInstance); }); it('starts debug mode on first call', async () => { - const fakeWrite = ():Promise => Promise.resolve(); + const fakeWrite = (): Promise => Promise.resolve(); const writeToIndexedDBStub = stub( indexeddb, 'writeDebugTokenToIndexedDB' ).callsFake(fakeWrite); - stub( - indexeddb, - 'readDebugTokenFromIndexedDB' - ).callsFake(() => Promise.resolve(undefined)); + stub(indexeddb, 'readDebugTokenFromIndexedDB').callsFake(() => + Promise.resolve(undefined) + ); const logStub = stub(logger.logger, 'warn'); const consoleStub = stub(console, 'log'); self.FIREBASE_APPCHECK_DEBUG_TOKEN = true; From c5a90de49e31de5529de08757498261992a57414 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Fri, 17 Sep 2021 16:16:30 -0700 Subject: [PATCH 3/6] Address PR comments --- packages/app-check/src/api.test.ts | 21 ++++++++++------ packages/app-check/src/api.ts | 40 ++++++++++++++++-------------- packages/app-check/src/state.ts | 2 ++ 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index d407d7dd687..bc829abc154 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -121,24 +121,29 @@ describe('api', () => { ).to.equal(appCheckInstance); }); it('starts debug mode on first call', async () => { - const fakeWrite = (): Promise => Promise.resolve(); - const writeToIndexedDBStub = stub( + let token: string = ''; + const fakeWrite = (tokenToWrite: string): Promise => { + token = tokenToWrite; + return Promise.resolve(); + }; + stub( indexeddb, 'writeDebugTokenToIndexedDB' ).callsFake(fakeWrite); - stub(indexeddb, 'readDebugTokenFromIndexedDB').callsFake(() => - Promise.resolve(undefined) - ); + stub(indexeddb, 'readDebugTokenFromIndexedDB').resolves(token); const logStub = stub(logger.logger, 'warn'); const consoleStub = stub(console, 'log'); self.FIREBASE_APPCHECK_DEBUG_TOKEN = true; initializeAppCheck(app, { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) }); - await fakeWrite(); - const token = writeToIndexedDBStub.args[0]; - expect(logStub).to.not.be.called; + // Ensure getDebugToken() call inside `initializeAppCheck()` + // has time to resolve, and double check its value matches that + // written to indexedDB. + expect(await getDebugToken()).to.equal(token); expect(consoleStub.args[0][0]).to.include(token); + expect(logStub).to.be.calledWith(match('is in debug mode')); + expect(logStub).to.be.calledWith(match(token)); self.FIREBASE_APPCHECK_DEBUG_TOKEN = undefined; }); it('warns about debug mode on second call', async () => { diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index b1a7b750b32..f779f484f04 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -23,7 +23,7 @@ import { PartialObserver } from './public-types'; import { ERROR_FACTORY, AppCheckError } from './errors'; -import { getState, setState, AppCheckState } from './state'; +import { getState, setState, AppCheckState, getDebugState } from './state'; import { FirebaseApp, getApp, _getProvider } from '@firebase/app'; import { getModularInstance, ErrorFn, NextFn } from '@firebase/util'; import { AppCheckService } from './factory'; @@ -59,6 +59,26 @@ export function initializeAppCheck( app = getModularInstance(app); const provider = _getProvider(app, 'app-check'); + // Ensure initializeDebugMode() is only called once. + if (!getDebugState().initialized) { + initializeDebugMode(); + } + + // Log a warning when `initializeAppCheck()` is called in debug mode, + // and show the token. + if (isDebugMode()) { + logger.warn( + `App Check is in debug mode. To turn off debug mode, unset ` + + `the global variable FIREBASE_APPCHECK_DEBUG_TOKEN and ` + + `restart the app.` + ); + // Make this a separate console statement so user will at least have the + // first message if the token promise doesn't resolve in time. + void getDebugToken().then(token => + logger.warn(`Debug token is ${token}.`) + ); + } + if (provider.isInitialized()) { const existingInstance = provider.getImmediate(); const initialOptions = provider.getOptions() as unknown as AppCheckOptions; @@ -67,21 +87,6 @@ export function initializeAppCheck( options.isTokenAutoRefreshEnabled && initialOptions.provider.isEqual(options.provider) ) { - // Log a warning if `initializeAppCheck()` is called after the first time - // if this app is still in debug mode, and show the token. - // If it's the first time, `initializeDebugMode()` already logs a message. - if (isDebugMode()) { - logger.warn( - `App Check is in debug mode. To turn off debug mode, unset ` + - `the global variable FIREBASE_APPCHECK_DEBUG_TOKEN and ` + - `restart the app.` - ); - // Make this a separate console statement so user will at least have the - // first message if the token promise doesn't resolve in time. - void getDebugToken().then(token => - logger.warn(`Debug token is ${token}.`) - ); - } return existingInstance; } else { throw ERROR_FACTORY.create(AppCheckError.ALREADY_INITIALIZED, { @@ -90,9 +95,6 @@ export function initializeAppCheck( } } - // Only read global variable on first call to `initializeAppCheck()`. - initializeDebugMode(); - const appCheck = provider.initialize({ options }); _activate(app, options.provider, options.isTokenAutoRefreshEnabled); diff --git a/packages/app-check/src/state.ts b/packages/app-check/src/state.ts index 36788d37cc2..671100cd8e0 100644 --- a/packages/app-check/src/state.ts +++ b/packages/app-check/src/state.ts @@ -41,6 +41,7 @@ export interface ReCAPTCHAState { } export interface DebugState { + initialized: boolean; enabled: boolean; token?: Deferred; } @@ -52,6 +53,7 @@ export const DEFAULT_STATE: AppCheckState = { }; const DEBUG_STATE: DebugState = { + initialized: false, enabled: false }; From 76186e8e5058aa89ae10275bc38a954c417259e4 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Fri, 17 Sep 2021 16:16:39 -0700 Subject: [PATCH 4/6] prettier --- packages/app-check/src/api.test.ts | 5 +---- packages/app-check/src/api.ts | 4 +--- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index bc829abc154..f2471452936 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -126,10 +126,7 @@ describe('api', () => { token = tokenToWrite; return Promise.resolve(); }; - stub( - indexeddb, - 'writeDebugTokenToIndexedDB' - ).callsFake(fakeWrite); + stub(indexeddb, 'writeDebugTokenToIndexedDB').callsFake(fakeWrite); stub(indexeddb, 'readDebugTokenFromIndexedDB').resolves(token); const logStub = stub(logger.logger, 'warn'); const consoleStub = stub(console, 'log'); diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index f779f484f04..10c6dd5161d 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -74,9 +74,7 @@ export function initializeAppCheck( ); // Make this a separate console statement so user will at least have the // first message if the token promise doesn't resolve in time. - void getDebugToken().then(token => - logger.warn(`Debug token is ${token}.`) - ); + void getDebugToken().then(token => logger.warn(`Debug token is ${token}.`)); } if (provider.isInitialized()) { From 0c329f76a7beb165752c740b08821a5abfa2d2df Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Mon, 20 Sep 2021 16:25:06 -0700 Subject: [PATCH 5/6] Address PR comments --- packages/app-check/src/api.test.ts | 9 +++------ packages/app-check/src/api.ts | 14 ++++++-------- packages/app-check/src/storage.ts | 4 ---- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index f2471452936..62602660b3c 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -16,7 +16,7 @@ */ import '../test/setup'; import { expect } from 'chai'; -import { match, spy, stub } from 'sinon'; +import { spy, stub } from 'sinon'; import { setTokenAutoRefreshEnabled, initializeAppCheck, @@ -128,7 +128,6 @@ describe('api', () => { }; stub(indexeddb, 'writeDebugTokenToIndexedDB').callsFake(fakeWrite); stub(indexeddb, 'readDebugTokenFromIndexedDB').resolves(token); - const logStub = stub(logger.logger, 'warn'); const consoleStub = stub(console, 'log'); self.FIREBASE_APPCHECK_DEBUG_TOKEN = true; initializeAppCheck(app, { @@ -139,13 +138,11 @@ describe('api', () => { // written to indexedDB. expect(await getDebugToken()).to.equal(token); expect(consoleStub.args[0][0]).to.include(token); - expect(logStub).to.be.calledWith(match('is in debug mode')); - expect(logStub).to.be.calledWith(match(token)); self.FIREBASE_APPCHECK_DEBUG_TOKEN = undefined; }); it('warns about debug mode on second call', async () => { - const logStub = stub(logger.logger, 'warn'); self.FIREBASE_APPCHECK_DEBUG_TOKEN = 'abcdefg'; + const consoleStub = stub(console, 'log'); initializeAppCheck(app, { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) }); @@ -154,7 +151,7 @@ describe('api', () => { }); const token = await getDebugToken(); expect(token).to.equal('abcdefg'); - expect(logStub).to.be.calledWith(match('abcdefg')); + expect(consoleStub.args[0][0]).to.include(token); self.FIREBASE_APPCHECK_DEBUG_TOKEN = undefined; }); diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index 10c6dd5161d..1107800460c 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -36,7 +36,6 @@ import { } from './internal-api'; import { readTokenFromStorage } from './storage'; import { getDebugToken, initializeDebugMode, isDebugMode } from './debug'; -import { logger } from './logger'; declare module '@firebase/component' { interface NameServiceMapping { @@ -67,14 +66,13 @@ export function initializeAppCheck( // Log a warning when `initializeAppCheck()` is called in debug mode, // and show the token. if (isDebugMode()) { - logger.warn( - `App Check is in debug mode. To turn off debug mode, unset ` + - `the global variable FIREBASE_APPCHECK_DEBUG_TOKEN and ` + - `restart the app.` + // Do not block initialization to get the token for the message. + void getDebugToken().then(token => + // Not using logger because I don't think we ever want this accidentally hidden. + console.log( + `App Check debug token: ${token}. You will need to add it to your app's App Check settings in the Firebase console for it to work.` + ) ); - // Make this a separate console statement so user will at least have the - // first message if the token promise doesn't resolve in time. - void getDebugToken().then(token => logger.warn(`Debug token is ${token}.`)); } if (provider.isInitialized()) { diff --git a/packages/app-check/src/storage.ts b/packages/app-check/src/storage.ts index c83a32fbb43..5dfd854e339 100644 --- a/packages/app-check/src/storage.ts +++ b/packages/app-check/src/storage.ts @@ -87,10 +87,6 @@ export async function readOrCreateDebugTokenFromStorage(): Promise { writeDebugTokenToIndexedDB(newToken).catch(e => logger.warn(`Failed to persist debug token to IndexedDB. Error: ${e}`) ); - // Not using logger because I don't think we ever want this accidentally hidden? - console.log( - `App Check debug token: ${newToken}. You will need to add it to your app's App Check settings in the Firebase console for it to work` - ); return newToken; } else { return existingDebugToken; From 675fb02b60a5f02e9657ff486ce5687205f7c695 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Wed, 22 Sep 2021 11:07:09 -0700 Subject: [PATCH 6/6] Fix debug initialize flag --- packages/app-check/src/api.test.ts | 12 +++++++++++- packages/app-check/src/api.ts | 4 ++-- packages/app-check/src/debug.ts | 6 +++++- packages/app-check/src/state.ts | 1 + 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index 62602660b3c..8f30d4b7b91 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -39,6 +39,7 @@ import * as client from './client'; import * as storage from './storage'; import * as internalApi from './internal-api'; import * as indexeddb from './indexeddb'; +import * as debug from './debug'; import { deleteApp, FirebaseApp } from '@firebase/app'; import { CustomProvider, ReCaptchaV3Provider } from './providers'; import { AppCheckService } from './factory'; @@ -140,18 +141,27 @@ describe('api', () => { expect(consoleStub.args[0][0]).to.include(token); self.FIREBASE_APPCHECK_DEBUG_TOKEN = undefined; }); - it('warns about debug mode on second call', async () => { + it('does not call initializeDebugMode on second call', async () => { self.FIREBASE_APPCHECK_DEBUG_TOKEN = 'abcdefg'; const consoleStub = stub(console, 'log'); + const initializeDebugModeSpy = spy(debug, 'initializeDebugMode'); + // First call, should call initializeDebugMode() initializeAppCheck(app, { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) }); + expect(initializeDebugModeSpy).to.be.called; + initializeDebugModeSpy.resetHistory(); + // Second call, should not call initializeDebugMode() initializeAppCheck(app, { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) }); const token = await getDebugToken(); expect(token).to.equal('abcdefg'); + // Two console logs of the token, for each initializeAppCheck call. expect(consoleStub.args[0][0]).to.include(token); + expect(consoleStub.args[1][0]).to.include(token); + expect(consoleStub.args[1][0]).to.equal(consoleStub.args[0][0]); + expect(initializeDebugModeSpy).to.not.be.called; self.FIREBASE_APPCHECK_DEBUG_TOKEN = undefined; }); diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index 1107800460c..b962f756ddf 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -63,8 +63,8 @@ export function initializeAppCheck( initializeDebugMode(); } - // Log a warning when `initializeAppCheck()` is called in debug mode, - // and show the token. + // Log a message containing the debug token when `initializeAppCheck()` + // is called in debug mode. if (isDebugMode()) { // Do not block initialization to get the token for the message. void getDebugToken().then(token => diff --git a/packages/app-check/src/debug.ts b/packages/app-check/src/debug.ts index da6cffbfcc4..864e21a169b 100644 --- a/packages/app-check/src/debug.ts +++ b/packages/app-check/src/debug.ts @@ -46,6 +46,11 @@ export async function getDebugToken(): Promise { export function initializeDebugMode(): void { const globals = getGlobal(); + const debugState = getDebugState(); + // Set to true if this function has been called, whether or not + // it enabled debug mode. + debugState.initialized = true; + if ( typeof globals.FIREBASE_APPCHECK_DEBUG_TOKEN !== 'string' && globals.FIREBASE_APPCHECK_DEBUG_TOKEN !== true @@ -53,7 +58,6 @@ export function initializeDebugMode(): void { return; } - const debugState = getDebugState(); debugState.enabled = true; const deferredToken = new Deferred(); debugState.token = deferredToken; diff --git a/packages/app-check/src/state.ts b/packages/app-check/src/state.ts index 671100cd8e0..27d9de08816 100644 --- a/packages/app-check/src/state.ts +++ b/packages/app-check/src/state.ts @@ -70,6 +70,7 @@ export function clearState(): void { APP_CHECK_STATES.clear(); DEBUG_STATE.enabled = false; DEBUG_STATE.token = undefined; + DEBUG_STATE.initialized = false; } export function getDebugState(): DebugState {