Fix app-check-compat initialization (#5089)

Author: hsubox76

packages-exp/app-check-compat/package.json

@@ -57,4 +57,4 @@
     "reportDir": "./coverage/node"
   },
   "esm5": "dist/index.esm.js"
-}
+}

packages-exp/app-check-compat/src/errors.ts

@@ -0,0 +1,38 @@
+/**
+ * @license
+ * Copyright 2021 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { ErrorFactory, ErrorMap } from '@firebase/util';
+
+export const enum AppCheckError {
+  USE_BEFORE_ACTIVATION = 'use-before-activation'
+}
+
+const ERRORS: ErrorMap<AppCheckError> = {
+  [AppCheckError.USE_BEFORE_ACTIVATION]:
+    'App Check is being used before activate() is called for FirebaseApp {$appName}. ' +
+    'Call activate() before instantiating other Firebase services.'
+};
+
+interface ErrorParams {
+  [AppCheckError.USE_BEFORE_ACTIVATION]: { appName: string };
+}
+
+export const ERROR_FACTORY = new ErrorFactory<AppCheckError, ErrorParams>(
+  'appCheck',
+  'AppCheck',
+  ERRORS
+);

packages-exp/app-check-compat/src/index.ts

@@ -27,7 +27,7 @@ import {
   InstanceFactory
 } from '@firebase/component';
 import { AppCheckService } from './service';
-import { FirebaseAppCheck } from '../../../packages/app-check-types';
+import { FirebaseAppCheck } from '@firebase/app-check-types';
 
 declare module '@firebase/component' {
   interface NameServiceMapping {
@@ -40,11 +40,8 @@ const factory: InstanceFactory<'appCheck-compat'> = (
 ) => {
   // Dependencies
   const app = container.getProvider('app-compat').getImmediate();
-  const appCheckServiceExp = container
-    .getProvider('app-check-exp')
-    .getImmediate();
 
-  return new AppCheckService(app as FirebaseApp, appCheckServiceExp);
+  return new AppCheckService(app as FirebaseApp);
 };
 
 export function registerAppCheck(): void {

packages-exp/app-check-compat/src/service.test.ts

@@ -20,19 +20,30 @@ import { firebase, FirebaseApp } from '@firebase/app-compat';
 import * as appCheckExp from '@firebase/app-check-exp';
 import { stub, match, SinonStub } from 'sinon';
 import * as sinonChai from 'sinon-chai';
-import { CustomProvider, ReCaptchaV3Provider } from '@firebase/app-check-exp';
-import { AppCheckTokenResult } from '../../../packages/app-check-types';
-import { PartialObserver } from '../../../packages/util/dist';
+import {
+  AppCheck,
+  CustomProvider,
+  ReCaptchaV3Provider
+} from '@firebase/app-check-exp';
+import { AppCheckTokenResult } from '@firebase/app-check-types';
+import { PartialObserver } from '@firebase/util';
+import { AppCheckError } from './errors';
 
 use(sinonChai);
 
 function createTestService(app: FirebaseApp): AppCheckService {
-  return new AppCheckService(
-    app,
-    appCheckExp.initializeAppCheck(app, {
-      provider: new ReCaptchaV3Provider('fake-site-key')
-    })
-  );
+  return new AppCheckService(app);
+}
+
+function createActivatedTestService(app: FirebaseApp): AppCheckService {
+  const service = new AppCheckService(app);
+  const initializeAppCheckStub = stub(
+    appCheckExp,
+    'initializeAppCheck'
+  ).returns({} as AppCheck);
+  service.activate('a-site-key');
+  initializeAppCheckStub.restore();
+  return service;
 }
 
 describe('Firebase App Check > Service', () => {
@@ -57,7 +68,7 @@ describe('Firebase App Check > Service', () => {
       'ReCaptchaV3Provider',
     () => {
       const initializeAppCheckStub = stub(appCheckExp, 'initializeAppCheck');
-      service = new AppCheckService(app, {} as appCheckExp.AppCheck);
+      service = new AppCheckService(app);
       service.activate('my_site_key');
       expect(initializeAppCheckStub).to.be.calledWith(app, {
         provider: match.instanceOf(ReCaptchaV3Provider),
@@ -72,7 +83,7 @@ describe('Firebase App Check > Service', () => {
       ' a CustomProvider',
     () => {
       const initializeAppCheckStub = stub(appCheckExp, 'initializeAppCheck');
-      service = new AppCheckService(app, {} as appCheckExp.AppCheck);
+      service = new AppCheckService(app);
       const customGetTokenStub = stub();
       service.activate({
         getToken: customGetTokenStub
@@ -97,7 +108,7 @@ describe('Firebase App Check > Service', () => {
       appCheckExp,
       'setTokenAutoRefreshEnabled'
     );
-    service = createTestService(app);
+    service = createActivatedTestService(app);
     service.setTokenAutoRefreshEnabled(true);
     expect(setTokenAutoRefreshEnabledStub).to.be.calledWith(
       service._delegate,
@@ -107,7 +118,7 @@ describe('Firebase App Check > Service', () => {
   });
 
   it('getToken() calls modular getToken()', async () => {
-    service = createTestService(app);
+    service = createActivatedTestService(app);
     const getTokenStub = stub(appCheckExp, 'getToken');
     await service.getToken(true);
     expect(getTokenStub).to.be.calledWith(service._delegate, true);
@@ -116,7 +127,7 @@ describe('Firebase App Check > Service', () => {
 
   it('onTokenChanged() calls modular onTokenChanged() with observer', () => {
     const onTokenChangedStub = stub(appCheckExp, 'onTokenChanged');
-    service = createTestService(app);
+    service = createActivatedTestService(app);
     const observer: PartialObserver<AppCheckTokenResult> = {
       next: stub(),
       error: stub()
@@ -128,7 +139,7 @@ describe('Firebase App Check > Service', () => {
 
   it('onTokenChanged() calls modular onTokenChanged() with next/error fns', () => {
     const onTokenChangedStub = stub(appCheckExp, 'onTokenChanged');
-    service = createTestService(app);
+    service = createActivatedTestService(app);
     const nextFn = stub();
     const errorFn = stub();
     service.onTokenChanged(nextFn, errorFn);
@@ -139,4 +150,25 @@ describe('Firebase App Check > Service', () => {
     );
     onTokenChangedStub.restore();
   });
+
+  it('setTokenAutoRefreshEnabled() throws if activate() has not been called', async () => {
+    service = createTestService(app);
+    expect(() => service.setTokenAutoRefreshEnabled(true)).to.throw(
+      AppCheckError.USE_BEFORE_ACTIVATION
+    );
+  });
+
+  it('getToken() throws if activate() has not been called', async () => {
+    service = createTestService(app);
+    expect(() => service.getToken(true)).to.throw(
+      AppCheckError.USE_BEFORE_ACTIVATION
+    );
+  });
+
+  it('onTokenChanged() throws if activate() has not been called', async () => {
+    service = createTestService(app);
+    expect(() => service.onTokenChanged(() => {})).to.throw(
+      AppCheckError.USE_BEFORE_ACTIVATION
+    );
+  });
 });

packages-exp/app-check-compat/src/service.ts

@@ -31,12 +31,14 @@ import {
   onTokenChanged as onTokenChangedExp
 } from '@firebase/app-check-exp';
 import { PartialObserver, Unsubscribe } from '@firebase/util';
+import { ERROR_FACTORY, AppCheckError } from './errors';
+
+export class AppCheckService
+  implements FirebaseAppCheck, Omit<_FirebaseService, '_delegate'>
+{
+  _delegate?: AppCheckServiceExp;
+  constructor(public app: FirebaseApp) {}
 
-export class AppCheckService implements FirebaseAppCheck, _FirebaseService {
-  constructor(
-    public app: FirebaseApp,
-    readonly _delegate: AppCheckServiceExp
-  ) {}
   activate(
     siteKeyOrProvider: string | AppCheckProvider,
     isTokenAutoRefreshEnabled?: boolean
@@ -47,24 +49,42 @@ export class AppCheckService implements FirebaseAppCheck, _FirebaseService {
     } else {
       provider = new CustomProvider({ getToken: siteKeyOrProvider.getToken });
     }
-    initializeAppCheck(this.app, {
+    this._delegate = initializeAppCheck(this.app, {
       provider,
       isTokenAutoRefreshEnabled
     });
   }
+
   setTokenAutoRefreshEnabled(isTokenAutoRefreshEnabled: boolean): void {
+    if (!this._delegate) {
+      throw ERROR_FACTORY.create(AppCheckError.USE_BEFORE_ACTIVATION, {
+        appName: this.app.name
+      });
+    }
     setTokenAutoRefreshEnabledExp(this._delegate, isTokenAutoRefreshEnabled);
   }
+
   getToken(forceRefresh?: boolean): Promise<AppCheckTokenResult> {
+    if (!this._delegate) {
+      throw ERROR_FACTORY.create(AppCheckError.USE_BEFORE_ACTIVATION, {
+        appName: this.app.name
+      });
+    }
     return getTokenExp(this._delegate, forceRefresh);
   }
+
   onTokenChanged(
     onNextOrObserver:
       | PartialObserver<AppCheckTokenResult>
       | ((tokenResult: AppCheckTokenResult) => void),
     onError?: (error: Error) => void,
     onCompletion?: () => void
   ): Unsubscribe {
+    if (!this._delegate) {
+      throw ERROR_FACTORY.create(AppCheckError.USE_BEFORE_ACTIVATION, {
+        appName: this.app.name
+      });
+    }
     return onTokenChangedExp(
       this._delegate,
       /**