Fix App Check state setting and promise clearing

.changeset/spicy-bags-sparkle.md

@@ -0,0 +1,5 @@
+---
+'@firebase/app-check': patch
+---
+
+Clear App Check exchange promise correctly after request succeeds.

packages/app-check/src/api.test.ts

@@ -16,7 +16,7 @@
  */
 import '../test/setup';
 import { expect } from 'chai';
-import { spy, stub } from 'sinon';
+import { SinonStub, spy, stub } from 'sinon';
 import {
   setTokenAutoRefreshEnabled,
   initializeAppCheck,
@@ -31,7 +31,12 @@ import {
   getFakeAppCheck,
   removegreCAPTCHAScriptsOnPage
 } from '../test/util';
-import { clearState, getState } from './state';
+import {
+  clearState,
+  DEFAULT_STATE,
+  getStateReference,
+  setInitialState
+} from './state';
 import * as reCAPTCHA from './recaptcha';
 import * as util from './util';
 import * as logger from './logger';
@@ -52,15 +57,21 @@ import { getDebugToken } from './debug';
 
 describe('api', () => {
   let app: FirebaseApp;
+  let storageReadStub: SinonStub;
+  let storageWriteStub: SinonStub;
 
   beforeEach(() => {
     app = getFullApp();
+    storageReadStub = stub(storage, 'readTokenFromStorage').resolves(undefined);
+    storageWriteStub = stub(storage, 'writeTokenToStorage');
     stub(util, 'getRecaptcha').returns(getFakeGreCAPTCHA());
   });
 
-  afterEach(() => {
+  afterEach(async () => {
     clearState();
     removegreCAPTCHAScriptsOnPage();
+    storageReadStub.restore();
+    storageWriteStub.restore();
     return deleteApp(app);
   });
 
@@ -216,19 +227,19 @@ describe('api', () => {
     });
 
     it('sets activated to true', () => {
-      expect(getState(app).activated).to.equal(false);
+      expect(getStateReference(app).activated).to.equal(false);
       initializeAppCheck(app, {
         provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
       });
-      expect(getState(app).activated).to.equal(true);
+      expect(getStateReference(app).activated).to.equal(true);
     });
 
     it('isTokenAutoRefreshEnabled value defaults to global setting', () => {
       app.automaticDataCollectionEnabled = false;
       initializeAppCheck(app, {
         provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
       });
-      expect(getState(app).isTokenAutoRefreshEnabled).to.equal(false);
+      expect(getStateReference(app).isTokenAutoRefreshEnabled).to.equal(false);
     });
 
     it('sets isTokenAutoRefreshEnabled correctly, overriding global setting', () => {
@@ -237,15 +248,16 @@ describe('api', () => {
         provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
         isTokenAutoRefreshEnabled: true
       });
-      expect(getState(app).isTokenAutoRefreshEnabled).to.equal(true);
+      expect(getStateReference(app).isTokenAutoRefreshEnabled).to.equal(true);
     });
   });
   describe('setTokenAutoRefreshEnabled()', () => {
     it('sets isTokenAutoRefreshEnabled correctly', () => {
       const app = getFakeApp({ automaticDataCollectionEnabled: false });
       const appCheck = getFakeAppCheck(app);
+      setInitialState(app, { ...DEFAULT_STATE });
       setTokenAutoRefreshEnabled(appCheck, true);
-      expect(getState(app).isTokenAutoRefreshEnabled).to.equal(true);
+      expect(getStateReference(app).isTokenAutoRefreshEnabled).to.equal(true);
     });
   });
   describe('getToken()', () => {
@@ -279,7 +291,7 @@ describe('api', () => {
         isTokenAutoRefreshEnabled: true
       });
 
-      expect(getState(app).tokenObservers.length).to.equal(1);
+      expect(getStateReference(app).tokenObservers.length).to.equal(1);
 
       const fakeRecaptchaToken = 'fake-recaptcha-token';
       const fakeRecaptchaAppCheckToken = {
@@ -291,7 +303,6 @@ describe('api', () => {
       stub(client, 'exchangeToken').returns(
         Promise.resolve(fakeRecaptchaAppCheckToken)
       );
-      stub(storage, 'writeTokenToStorage').returns(Promise.resolve(undefined));
 
       const listener1 = stub().throws(new Error());
       const listener2 = spy();
@@ -302,7 +313,7 @@ describe('api', () => {
       const unsubscribe1 = onTokenChanged(appCheck, listener1, errorFn1);
       const unsubscribe2 = onTokenChanged(appCheck, listener2, errorFn2);
 
-      expect(getState(app).tokenObservers.length).to.equal(3);
+      expect(getStateReference(app).tokenObservers.length).to.equal(3);
 
       await internalApi.getToken(appCheck as AppCheckService);
 
@@ -315,7 +326,7 @@ describe('api', () => {
       expect(errorFn2).to.not.be.called;
       unsubscribe1();
       unsubscribe2();
-      expect(getState(app).tokenObservers.length).to.equal(1);
+      expect(getStateReference(app).tokenObservers.length).to.equal(1);
     });
 
     it('Listeners work when using Observer pattern', async () => {
@@ -324,7 +335,7 @@ describe('api', () => {
         isTokenAutoRefreshEnabled: true
       });
 
-      expect(getState(app).tokenObservers.length).to.equal(1);
+      expect(getStateReference(app).tokenObservers.length).to.equal(1);
 
       const fakeRecaptchaToken = 'fake-recaptcha-token';
       const fakeRecaptchaAppCheckToken = {
@@ -336,7 +347,7 @@ describe('api', () => {
       stub(client, 'exchangeToken').returns(
         Promise.resolve(fakeRecaptchaAppCheckToken)
       );
-      stub(storage, 'writeTokenToStorage').returns(Promise.resolve(undefined));
+      storageWriteStub.returns(Promise.resolve(undefined));
 
       const listener1 = stub().throws(new Error());
       const listener2 = spy();
@@ -357,7 +368,7 @@ describe('api', () => {
         error: errorFn1
       });
 
-      expect(getState(app).tokenObservers.length).to.equal(3);
+      expect(getStateReference(app).tokenObservers.length).to.equal(3);
 
       await internalApi.getToken(appCheck as AppCheckService);
 
@@ -370,7 +381,7 @@ describe('api', () => {
       expect(errorFn2).to.not.be.called;
       unsubscribe1();
       unsubscribe2();
-      expect(getState(app).tokenObservers.length).to.equal(1);
+      expect(getStateReference(app).tokenObservers.length).to.equal(1);
     });
 
     it('onError() catches token errors', async () => {
@@ -380,12 +391,12 @@ describe('api', () => {
         isTokenAutoRefreshEnabled: false
       });
 
-      expect(getState(app).tokenObservers.length).to.equal(0);
+      expect(getStateReference(app).tokenObservers.length).to.equal(0);
 
       const fakeRecaptchaToken = 'fake-recaptcha-token';
       stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken));
       stub(client, 'exchangeToken').rejects('exchange error');
-      stub(storage, 'writeTokenToStorage').returns(Promise.resolve(undefined));
+      storageWriteStub.returns(Promise.resolve(undefined));
 
       const listener1 = spy();
 
@@ -395,13 +406,13 @@ describe('api', () => {
 
       await internalApi.getToken(appCheck as AppCheckService);
 
-      expect(getState(app).tokenObservers.length).to.equal(1);
+      expect(getStateReference(app).tokenObservers.length).to.equal(1);
 
       expect(errorFn1).to.be.calledOnce;
       expect(errorFn1.args[0][0].name).to.include('exchange error');
 
       unsubscribe1();
-      expect(getState(app).tokenObservers.length).to.equal(0);
+      expect(getStateReference(app).tokenObservers.length).to.equal(0);
     });
   });
 });

packages/app-check/src/api.ts

@@ -23,7 +23,12 @@ import {
   PartialObserver
 } from './public-types';
 import { ERROR_FACTORY, AppCheckError } from './errors';
-import { getState, setState, AppCheckState, getDebugState } from './state';
+import {
+  getStateReference,
+  getDebugState,
+  DEFAULT_STATE,
+  setInitialState
+} from './state';
 import { FirebaseApp, getApp, _getProvider } from '@firebase/app';
 import { getModularInstance, ErrorFn, NextFn } from '@firebase/util';
 import { AppCheckService } from './factory';
@@ -101,7 +106,7 @@ export function initializeAppCheck(
   // If isTokenAutoRefreshEnabled is false, do not send any requests to the
   // exchange endpoint without an explicit call from the user either directly
   // or through another Firebase library (storage, functions, etc.)
-  if (getState(app).isTokenAutoRefreshEnabled) {
+  if (getStateReference(app).isTokenAutoRefreshEnabled) {
     // Adding a listener will start the refresher and fetch a token if needed.
     // This gets a token ready and prevents a delay when an internal library
     // requests the token.
@@ -128,13 +133,15 @@ function _activate(
   provider: AppCheckProvider,
   isTokenAutoRefreshEnabled?: boolean
 ): void {
-  const state = getState(app);
+  // Create an entry in the APP_CHECK_STATES map. Further changes should
+  // directly mutate this object.
+  const state = setInitialState(app, { ...DEFAULT_STATE });
 
-  const newState: AppCheckState = { ...state, activated: true };
-  newState.provider = provider; // Read cached token from storage if it exists and store it in memory.
-  newState.cachedTokenPromise = readTokenFromStorage(app).then(cachedToken => {
+  state.activated = true;
+  state.provider = provider; // Read cached token from storage if it exists and store it in memory.
+  state.cachedTokenPromise = readTokenFromStorage(app).then(cachedToken => {
     if (cachedToken && isValid(cachedToken)) {
-      setState(app, { ...getState(app), token: cachedToken });
+      state.token = cachedToken;
       // notify all listeners with the cached token
       notifyTokenListeners(app, { token: cachedToken.token });
     }
@@ -144,14 +151,12 @@ function _activate(
   // Use value of global `automaticDataCollectionEnabled` (which
   // itself defaults to false if not specified in config) if
   // `isTokenAutoRefreshEnabled` param was not provided by user.
-  newState.isTokenAutoRefreshEnabled =
+  state.isTokenAutoRefreshEnabled =
     isTokenAutoRefreshEnabled === undefined
       ? app.automaticDataCollectionEnabled
       : isTokenAutoRefreshEnabled;
 
-  setState(app, newState);
-
-  newState.provider.initialize(app);
+  state.provider.initialize(app);
 }
 
 /**
@@ -168,7 +173,7 @@ export function setTokenAutoRefreshEnabled(
   isTokenAutoRefreshEnabled: boolean
 ): void {
   const app = appCheckInstance.app;
-  const state = getState(app);
+  const state = getStateReference(app);
   // This will exist if any product libraries have called
   // `addTokenListener()`
   if (state.tokenRefresher) {
@@ -178,7 +183,7 @@ export function setTokenAutoRefreshEnabled(
       state.tokenRefresher.stop();
     }
   }
-  setState(app, { ...state, isTokenAutoRefreshEnabled });
+  state.isTokenAutoRefreshEnabled = isTokenAutoRefreshEnabled;
 }
 /**
  * Get the current App Check token. Attaches to the most recent

packages/app-check/src/factory.ts

@@ -24,7 +24,7 @@ import {
   removeTokenListener
 } from './internal-api';
 import { Provider } from '@firebase/component';
-import { getState } from './state';
+import { getStateReference } from './state';
 
 /**
  * AppCheck Service class.
@@ -35,7 +35,7 @@ export class AppCheckService implements AppCheck, _FirebaseService {
     public heartbeatServiceProvider: Provider<'heartbeat'>
   ) {}
   _delete(): Promise<void> {
-    const { tokenObservers } = getState(this.app);
+    const { tokenObservers } = getStateReference(this.app);
     for (const tokenObserver of tokenObservers) {
       removeTokenListener(this.app, tokenObserver.next);
     }