Skip to content

Commit 1496461

Browse files
committed
fix(AppCheck): Fixed getToken promise not being cleared
1 parent d7acc96 commit 1496461

File tree

3 files changed

+115
-13
lines changed

3 files changed

+115
-13
lines changed

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,103 @@ describe('internal api', () => {
368368
});
369369
});
370370

371+
it('no dangling exchangeToken promise internal', async () => {
372+
const appCheck = initializeAppCheck(app, {
373+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
374+
});
375+
376+
setState(app, {
377+
...getState(app),
378+
token: fakeRecaptchaAppCheckToken,
379+
cachedTokenPromise: undefined
380+
});
381+
382+
stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken));
383+
stub(client, 'exchangeToken').returns(
384+
Promise.resolve({
385+
token: 'new-recaptcha-app-check-token',
386+
expireTimeMillis: Date.now() + 60000,
387+
issuedAtTimeMillis: 0
388+
})
389+
);
390+
391+
const getTokenPromise = getToken(appCheck as AppCheckService, true);
392+
393+
expect(getState(app).exchangeTokenFetcher.promise).to.be.instanceOf(
394+
Promise
395+
);
396+
397+
const state = {
398+
...getState(app)
399+
};
400+
401+
await getTokenPromise;
402+
403+
setState(app, state);
404+
405+
expect(getState(app).exchangeTokenFetcher.promise).to.be.equal(undefined);
406+
});
407+
408+
it('no dangling exchangeToken promise', async () => {
409+
const clock = useFakeTimers();
410+
411+
const appCheck = initializeAppCheck(app, {
412+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
413+
});
414+
415+
const soonExpiredToken = {
416+
token: `recaptcha-app-check-token-old`,
417+
expireTimeMillis: Date.now() + 1000,
418+
issuedAtTimeMillis: 0
419+
};
420+
421+
setState(app, {
422+
...getState(app),
423+
token: soonExpiredToken,
424+
cachedTokenPromise: undefined
425+
});
426+
427+
stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken));
428+
let count = 0;
429+
stub(client, 'exchangeToken').callsFake(
430+
() =>
431+
new Promise(res =>
432+
setTimeout(
433+
() =>
434+
res({
435+
token: `recaptcha-app-check-token-new-${count++}`,
436+
expireTimeMillis: Date.now() + 60000,
437+
issuedAtTimeMillis: 0
438+
}),
439+
3000
440+
)
441+
)
442+
);
443+
444+
// start fetch token
445+
void getToken(appCheck as AppCheckService, true);
446+
447+
clock.tick(2000);
448+
449+
// save expired `token-old` with copied state and wait fetch token
450+
void getToken(appCheck as AppCheckService);
451+
452+
// wait fetch token with copied state
453+
void getToken(appCheck as AppCheckService);
454+
455+
// stored copied state with `token-new-0`
456+
await clock.runAllAsync();
457+
458+
// fetch token with copied state
459+
const newToken = getToken(appCheck as AppCheckService, true);
460+
461+
await clock.runAllAsync();
462+
463+
expect(await newToken).to.deep.equal({
464+
token: 'recaptcha-app-check-token-new-1'
465+
});
466+
});
467+
371468
it('ignores in-memory token if it is invalid and continues to exchange request', async () => {
372469
const appCheck = initializeAppCheck(app, {
373470
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -117,18 +117,18 @@ export async function getToken(
117117
*/
118118
if (isDebugMode()) {
119119
// Avoid making another call to the exchange endpoint if one is in flight.
120-
if (!state.exchangeTokenPromise) {
121-
state.exchangeTokenPromise = exchangeToken(
120+
if (!state.exchangeTokenFetcher.promise) {
121+
state.exchangeTokenFetcher.promise = exchangeToken(
122122
getExchangeDebugTokenRequest(app, await getDebugToken()),
123123
appCheck.heartbeatServiceProvider
124124
).finally(() => {
125125
// Clear promise when settled - either resolved or rejected.
126-
state.exchangeTokenPromise = undefined;
126+
state.exchangeTokenFetcher.promise = undefined;
127127
});
128128
shouldCallListeners = true;
129129
}
130-
const tokenFromDebugExchange: AppCheckTokenInternal =
131-
await state.exchangeTokenPromise;
130+
const tokenFromDebugExchange: AppCheckTokenInternal = await state
131+
.exchangeTokenFetcher.promise;
132132
// Write debug token to indexedDB.
133133
await writeTokenToStorage(app, tokenFromDebugExchange);
134134
// Write debug token to state.
@@ -143,17 +143,19 @@ export async function getToken(
143143
*/
144144
try {
145145
// Avoid making another call to the exchange endpoint if one is in flight.
146-
if (!state.exchangeTokenPromise) {
146+
if (!state.exchangeTokenFetcher.promise) {
147147
// state.provider is populated in initializeAppCheck()
148148
// ensureActivated() at the top of this function checks that
149149
// initializeAppCheck() has been called.
150-
state.exchangeTokenPromise = state.provider!.getToken().finally(() => {
151-
// Clear promise when settled - either resolved or rejected.
152-
state.exchangeTokenPromise = undefined;
153-
});
150+
state.exchangeTokenFetcher.promise = state
151+
.provider!.getToken()
152+
.finally(() => {
153+
// Clear promise when settled - either resolved or rejected.
154+
state.exchangeTokenFetcher.promise = undefined;
155+
});
154156
shouldCallListeners = true;
155157
}
156-
token = await state.exchangeTokenPromise;
158+
token = await state.exchangeTokenFetcher.promise;
157159
} catch (e) {
158160
if ((e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}`) {
159161
// Warn if throttled, but do not treat it as an error.

packages/app-check/src/state.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ export interface AppCheckState {
3030
provider?: AppCheckProvider;
3131
token?: AppCheckTokenInternal;
3232
cachedTokenPromise?: Promise<AppCheckTokenInternal | undefined>;
33-
exchangeTokenPromise?: Promise<AppCheckTokenInternal>;
33+
exchangeTokenFetcher: {
34+
promise?: Promise<AppCheckTokenInternal>;
35+
};
3436
tokenRefresher?: Refresher;
3537
reCAPTCHAState?: ReCAPTCHAState;
3638
isTokenAutoRefreshEnabled?: boolean;
@@ -50,7 +52,8 @@ export interface DebugState {
5052
const APP_CHECK_STATES = new Map<FirebaseApp, AppCheckState>();
5153
export const DEFAULT_STATE: AppCheckState = {
5254
activated: false,
53-
tokenObservers: []
55+
tokenObservers: [],
56+
exchangeTokenFetcher: {}
5457
};
5558

5659
const DEBUG_STATE: DebugState = {

0 commit comments

Comments
 (0)