From 14964614dd695084f78d012783a989e427687ad6 Mon Sep 17 00:00:00 2001 From: ishowta Date: Fri, 28 Oct 2022 22:12:21 +0900 Subject: [PATCH] fix(AppCheck): Fixed getToken promise not being cleared --- packages/app-check/src/internal-api.test.ts | 97 +++++++++++++++++++++ packages/app-check/src/internal-api.ts | 24 ++--- packages/app-check/src/state.ts | 7 +- 3 files changed, 115 insertions(+), 13 deletions(-) diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index 03c690fb42e..ae2787d4d56 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -368,6 +368,103 @@ describe('internal api', () => { }); }); + it('no dangling exchangeToken promise internal', async () => { + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + + setState(app, { + ...getState(app), + token: fakeRecaptchaAppCheckToken, + cachedTokenPromise: undefined + }); + + stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken)); + stub(client, 'exchangeToken').returns( + Promise.resolve({ + token: 'new-recaptcha-app-check-token', + expireTimeMillis: Date.now() + 60000, + issuedAtTimeMillis: 0 + }) + ); + + const getTokenPromise = getToken(appCheck as AppCheckService, true); + + expect(getState(app).exchangeTokenFetcher.promise).to.be.instanceOf( + Promise + ); + + const state = { + ...getState(app) + }; + + await getTokenPromise; + + setState(app, state); + + expect(getState(app).exchangeTokenFetcher.promise).to.be.equal(undefined); + }); + + it('no dangling exchangeToken promise', async () => { + const clock = useFakeTimers(); + + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + + const soonExpiredToken = { + token: `recaptcha-app-check-token-old`, + expireTimeMillis: Date.now() + 1000, + issuedAtTimeMillis: 0 + }; + + setState(app, { + ...getState(app), + token: soonExpiredToken, + cachedTokenPromise: undefined + }); + + stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken)); + let count = 0; + stub(client, 'exchangeToken').callsFake( + () => + new Promise(res => + setTimeout( + () => + res({ + token: `recaptcha-app-check-token-new-${count++}`, + expireTimeMillis: Date.now() + 60000, + issuedAtTimeMillis: 0 + }), + 3000 + ) + ) + ); + + // start fetch token + void getToken(appCheck as AppCheckService, true); + + clock.tick(2000); + + // save expired `token-old` with copied state and wait fetch token + void getToken(appCheck as AppCheckService); + + // wait fetch token with copied state + void getToken(appCheck as AppCheckService); + + // stored copied state with `token-new-0` + await clock.runAllAsync(); + + // fetch token with copied state + const newToken = getToken(appCheck as AppCheckService, true); + + await clock.runAllAsync(); + + expect(await newToken).to.deep.equal({ + token: 'recaptcha-app-check-token-new-1' + }); + }); + it('ignores in-memory token if it is invalid and continues to exchange request', async () => { const appCheck = initializeAppCheck(app, { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 99c0f5d2c00..bafc474b63c 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -117,18 +117,18 @@ export async function getToken( */ if (isDebugMode()) { // Avoid making another call to the exchange endpoint if one is in flight. - if (!state.exchangeTokenPromise) { - state.exchangeTokenPromise = exchangeToken( + if (!state.exchangeTokenFetcher.promise) { + state.exchangeTokenFetcher.promise = exchangeToken( getExchangeDebugTokenRequest(app, await getDebugToken()), appCheck.heartbeatServiceProvider ).finally(() => { // Clear promise when settled - either resolved or rejected. - state.exchangeTokenPromise = undefined; + state.exchangeTokenFetcher.promise = undefined; }); shouldCallListeners = true; } - const tokenFromDebugExchange: AppCheckTokenInternal = - await state.exchangeTokenPromise; + const tokenFromDebugExchange: AppCheckTokenInternal = await state + .exchangeTokenFetcher.promise; // Write debug token to indexedDB. await writeTokenToStorage(app, tokenFromDebugExchange); // Write debug token to state. @@ -143,17 +143,19 @@ export async function getToken( */ try { // Avoid making another call to the exchange endpoint if one is in flight. - if (!state.exchangeTokenPromise) { + if (!state.exchangeTokenFetcher.promise) { // state.provider is populated in initializeAppCheck() // ensureActivated() at the top of this function checks that // initializeAppCheck() has been called. - state.exchangeTokenPromise = state.provider!.getToken().finally(() => { - // Clear promise when settled - either resolved or rejected. - state.exchangeTokenPromise = undefined; - }); + state.exchangeTokenFetcher.promise = state + .provider!.getToken() + .finally(() => { + // Clear promise when settled - either resolved or rejected. + state.exchangeTokenFetcher.promise = undefined; + }); shouldCallListeners = true; } - token = await state.exchangeTokenPromise; + token = await state.exchangeTokenFetcher.promise; } catch (e) { if ((e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}`) { // Warn if throttled, but do not treat it as an error. diff --git a/packages/app-check/src/state.ts b/packages/app-check/src/state.ts index 0a81ba81636..c8eefb4cec3 100644 --- a/packages/app-check/src/state.ts +++ b/packages/app-check/src/state.ts @@ -30,7 +30,9 @@ export interface AppCheckState { provider?: AppCheckProvider; token?: AppCheckTokenInternal; cachedTokenPromise?: Promise; - exchangeTokenPromise?: Promise; + exchangeTokenFetcher: { + promise?: Promise; + }; tokenRefresher?: Refresher; reCAPTCHAState?: ReCAPTCHAState; isTokenAutoRefreshEnabled?: boolean; @@ -50,7 +52,8 @@ export interface DebugState { const APP_CHECK_STATES = new Map(); export const DEFAULT_STATE: AppCheckState = { activated: false, - tokenObservers: [] + tokenObservers: [], + exchangeTokenFetcher: {} }; const DEBUG_STATE: DebugState = {