From c3995b51e22065929e8a8850a9d8b59b8057b363 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Mon, 19 Sep 2022 17:21:23 -0700 Subject: [PATCH 1/6] First pass --- packages/app-check/src/internal-api.ts | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 6b086c675cd..3f657fb9959 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -84,6 +84,13 @@ export async function getToken( } } + if (token && !isValid(token)) { + // If an invalid token was found in memory or indexedDB, clear token from + // memory and the local variable. + setState(app, { ...state, token: undefined }); + token = undefined; + } + // Return the cached token (from either memory or indexedDB) if it's valid if (!forceRefresh && token && isValid(token)) { return { @@ -107,14 +114,14 @@ export async function getToken( state.exchangeTokenPromise = exchangeToken( getExchangeDebugTokenRequest(app, await getDebugToken()), appCheck.heartbeatServiceProvider - ).then(token => { + ).finally(() => { + // Clear promise when settled - either resolved or rejected. state.exchangeTokenPromise = undefined; - return token; }); shouldCallListeners = true; } const tokenFromDebugExchange: AppCheckTokenInternal = - await state.exchangeTokenPromise; + await state.exchangeTokenPromise!; // Write debug token to indexedDB. await writeTokenToStorage(app, tokenFromDebugExchange); // Write debug token to state. @@ -131,9 +138,9 @@ export async function getToken( // state.provider is populated in initializeAppCheck() // ensureActivated() at the top of this function checks that // initializeAppCheck() has been called. - state.exchangeTokenPromise = state.provider!.getToken().then(token => { + state.exchangeTokenPromise = state.provider!.getToken().finally(() => { + // Clear promise when settled - either resolved or rejected. state.exchangeTokenPromise = undefined; - return token; }); shouldCallListeners = true; } @@ -151,7 +158,7 @@ export async function getToken( } let interopTokenResult: AppCheckTokenResult | undefined; - if (!token) { + if (!token || error) { // if token is undefined, there must be an error. // we return a dummy token along with the error interopTokenResult = makeDummyTokenResult(error!); @@ -290,7 +297,7 @@ function createTokenRefresher(appCheck: AppCheckService): Refresher { let nextRefreshTimeMillis = state.token.issuedAtTimeMillis + (state.token.expireTimeMillis - state.token.issuedAtTimeMillis) * - 0.5 + + 0.5 + 5 * 60 * 1000; // Do not allow refresh time to be past (expireTime - 5 minutes) const latestAllowableRefresh = From 15f518bb6bf16f645203fb59553af0437e3482ae Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 20 Sep 2022 08:54:14 -0700 Subject: [PATCH 2/6] Update comments. --- packages/app-check/src/internal-api.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 3f657fb9959..7597a50a66d 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -84,9 +84,9 @@ export async function getToken( } } + // If an invalid token was found in memory or indexedDB, clear token from + // memory and the local variable. if (token && !isValid(token)) { - // If an invalid token was found in memory or indexedDB, clear token from - // memory and the local variable. setState(app, { ...state, token: undefined }); token = undefined; } @@ -159,8 +159,11 @@ export async function getToken( let interopTokenResult: AppCheckTokenResult | undefined; if (!token || error) { - // if token is undefined, there must be an error. - // we return a dummy token along with the error + // If token is undefined, there must be an error. + // It's also possible a token exists, but there's also an error. (Such as + // if the token is almost expired, tries to refresh, and fails the + // exchange request.) + // In either case, return a dummy token along with the error. interopTokenResult = makeDummyTokenResult(error!); } else { interopTokenResult = { @@ -297,7 +300,7 @@ function createTokenRefresher(appCheck: AppCheckService): Refresher { let nextRefreshTimeMillis = state.token.issuedAtTimeMillis + (state.token.expireTimeMillis - state.token.issuedAtTimeMillis) * - 0.5 + + 0.5 + 5 * 60 * 1000; // Do not allow refresh time to be past (expireTime - 5 minutes) const latestAllowableRefresh = From 6a16eb7443bc4510d5f79af880319fc8daed21b2 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 20 Sep 2022 09:03:02 -0700 Subject: [PATCH 3/6] Add changeset --- .changeset/selfish-worms-glow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/selfish-worms-glow.md diff --git a/.changeset/selfish-worms-glow.md b/.changeset/selfish-worms-glow.md new file mode 100644 index 00000000000..af25eed8fbb --- /dev/null +++ b/.changeset/selfish-worms-glow.md @@ -0,0 +1,5 @@ +--- +'@firebase/app-check': patch +--- + +Fix timer issues in App Check that caused the token to fail to refresh after the token expired, or caused rapid repeated requests attempting to do so. From 6d453578e6026600b391cd3c1edac610a2bdef04 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 20 Sep 2022 12:56:37 -0700 Subject: [PATCH 4/6] Add tests --- packages/app-check/src/indexeddb.ts | 2 +- packages/app-check/src/internal-api.test.ts | 292 +++++++++++++++++++- packages/app-check/src/internal-api.ts | 74 +++-- packages/app-check/src/storage.ts | 2 +- packages/app-check/src/types.ts | 4 + 5 files changed, 350 insertions(+), 24 deletions(-) diff --git a/packages/app-check/src/indexeddb.ts b/packages/app-check/src/indexeddb.ts index 4041a535904..4da7165c8fa 100644 --- a/packages/app-check/src/indexeddb.ts +++ b/packages/app-check/src/indexeddb.ts @@ -80,7 +80,7 @@ export function readTokenFromIndexedDB( export function writeTokenToIndexedDB( app: FirebaseApp, - token: AppCheckTokenInternal + token?: AppCheckTokenInternal ): Promise { return write(computeKey(app), token); } diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index 09b81f5ee24..9c082bcbe73 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -368,6 +368,102 @@ describe('internal api', () => { }); }); + 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) + }); + setState(app, { + ...getState(app), token: { + token: 'something', + expireTimeMillis: Date.now() - 1000, + issuedAtTimeMillis: 0 + } + }); + + 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 + }) + ); + + expect(await getToken(appCheck as AppCheckService)).to.deep.equal({ + token: 'new-recaptcha-app-check-token' + }); + }); + + it('returns the valid token in storage without making a network request', async () => { + const clock = useFakeTimers(); + + storageReadStub.resolves(fakeCachedAppCheckToken); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + + const clientStub = stub(client, 'exchangeToken'); + expect(await getToken(appCheck as AppCheckService)).to.deep.equal({ + token: fakeCachedAppCheckToken.token + }); + expect(clientStub).to.not.have.been.called; + + clock.restore(); + }); + + it('deletes cached token if it is invalid and continues to exchange request', async () => { + storageReadStub.resolves({ + token: 'something', + expireTimeMillis: Date.now() - 1000, + issuedAtTimeMillis: 0 + }); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + + const freshToken = { + token: 'new-recaptcha-app-check-token', + expireTimeMillis: Date.now() + 60000, + issuedAtTimeMillis: 0 + }; + + stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken)); + stub(client, 'exchangeToken').returns( + Promise.resolve(freshToken) + ); + + expect(await getToken(appCheck as AppCheckService)).to.deep.equal({ + token: 'new-recaptcha-app-check-token' + }); + + // When it wiped the invalid token. + expect(storageWriteStub).has.been.calledWith( + app, + undefined + ); + + // When it wrote the new token fetched from the exchange endpoint. + expect(storageWriteStub).has.been.calledWith( + app, + freshToken + ); + }); + + it('returns the actual token and an internalError if a token is valid but the request fails', async () => { + stub(logger, 'error'); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + setState(app, { ...getState(app), token: fakeRecaptchaAppCheckToken }); + + stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken)); + stub(client, 'exchangeToken').returns(Promise.reject(new Error('blah'))); + + const tokenResult = await getToken(appCheck as AppCheckService, true); + expect(tokenResult.internalError?.message).to.equal('blah'); + expect(tokenResult.token).to.equal('fake-recaptcha-app-check-token'); + }); + it('exchanges debug token if in debug mode and there is no cached token', async () => { const exchangeTokenStub: SinonStub = stub( client, @@ -443,7 +539,7 @@ describe('internal api', () => { describe('addTokenListener', () => { it('adds token listeners', () => { - const listener = (): void => {}; + const listener = (): void => { }; setState(app, { ...getState(app), cachedTokenPromise: Promise.resolve(undefined) @@ -459,7 +555,7 @@ describe('internal api', () => { }); it('starts proactively refreshing token after adding the first listener', async () => { - const listener = (): void => {}; + const listener = (): void => { }; setState(app, { ...getState(app), isTokenAutoRefreshEnabled: true, @@ -534,11 +630,199 @@ describe('internal api', () => { fakeListener ); }); + + it('does not make rapid requests within proactive refresh window', async () => { + const clock = useFakeTimers(); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY), + isTokenAutoRefreshEnabled: true + }); + setState(app, { + ...getState(app), token: { + token: `fake-cached-app-check-token`, + // within refresh window + expireTimeMillis: 10000, + issuedAtTimeMillis: 0 + } + }); + + const fakeListener: AppCheckTokenListener = stub(); + + const fakeExchange = stub(client, 'exchangeToken').returns( + Promise.resolve({ + token: 'new-recaptcha-app-check-token', + expireTimeMillis: 10 * 60 * 1000, + issuedAtTimeMillis: 0 + }) + ); + + addTokenListener( + appCheck as AppCheckService, + ListenerType.INTERNAL, + fakeListener + ); + // Tick 10s, make sure nothing is called repeatedly in that time. + await clock.tickAsync(10000); + expect(fakeListener).to.be.calledWith({ token: 'fake-cached-app-check-token' }); + expect(fakeListener).to.be.calledWith({ token: 'new-recaptcha-app-check-token' }); + expect(fakeExchange).to.be.calledOnce; + clock.restore(); + }); + + it('proactive refresh window test - exchange request fails - wait 10s', async () => { + stub(logger, 'error'); + const clock = useFakeTimers(); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY), + isTokenAutoRefreshEnabled: true + }); + setState(app, { + ...getState(app), token: { + token: `fake-cached-app-check-token`, + // not expired but within refresh window + expireTimeMillis: 10000, + issuedAtTimeMillis: 0 + } + }); + + const fakeListener: AppCheckTokenListener = stub(); + + const fakeExchange = stub(client, 'exchangeToken').returns( + Promise.reject(new Error('fetch failed or something')) + ); + + addTokenListener( + appCheck as AppCheckService, + ListenerType.EXTERNAL, + fakeListener + ); + // Tick 10s, make sure nothing is called repeatedly in that time. + await clock.tickAsync(10000); + expect(fakeListener).to.be.calledWith({ token: 'fake-cached-app-check-token' }); + // once on init and once invoked directly in this test + expect(fakeListener).to.be.calledTwice; + expect(fakeExchange).to.be.calledOnce; + clock.restore(); + }); + + it('proactive refresh window test - exchange request fails - wait 40s', async () => { + stub(logger, 'error'); + const clock = useFakeTimers(); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY), + isTokenAutoRefreshEnabled: true + }); + setState(app, { + ...getState(app), token: { + token: `fake-cached-app-check-token`, + // not expired but within refresh window + expireTimeMillis: 10000, + issuedAtTimeMillis: 0 + } + }); + + const fakeListener: AppCheckTokenListener = stub(); + + const fakeExchange = stub(client, 'exchangeToken').returns( + Promise.reject(new Error('fetch failed or something')) + ); + + addTokenListener( + appCheck as AppCheckService, + ListenerType.EXTERNAL, + fakeListener + ); + // Tick 40s, expect one initial exchange request and one retry. + // (First backoff is 30s). + await clock.tickAsync(40000); + expect(fakeListener).to.be.calledTwice; + expect(fakeExchange).to.be.calledTwice; + clock.restore(); + }); + + it('expired token - exchange request fails - wait 10s', async () => { + stub(logger, 'error'); + const clock = useFakeTimers(); + clock.tick(1); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY), + isTokenAutoRefreshEnabled: true + }); + setState(app, { + ...getState(app), token: { + token: `fake-cached-app-check-token`, + // expired + expireTimeMillis: 0, + issuedAtTimeMillis: 0 + } + }); + + const fakeListener = stub(); + const errorHandler = stub(); + const fakeNetworkError = new Error('fetch failed or something'); + + const fakeExchange = stub(client, 'exchangeToken').returns( + Promise.reject(fakeNetworkError) + ); + + addTokenListener( + appCheck as AppCheckService, + ListenerType.EXTERNAL, + fakeListener, + errorHandler + ); + // Tick 10s, make sure nothing is called repeatedly in that time. + await clock.tickAsync(10000); + expect(fakeListener).not.to.be.called; + expect(fakeExchange).to.be.calledOnce; + expect(errorHandler).to.be.calledWith(fakeNetworkError); + clock.restore(); + }); + + it('expired token - exchange request fails - wait 40s', async () => { + stub(logger, 'error'); + const clock = useFakeTimers(); + clock.tick(1); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY), + isTokenAutoRefreshEnabled: true + }); + setState(app, { + ...getState(app), token: { + token: `fake-cached-app-check-token`, + // expired + expireTimeMillis: 0, + issuedAtTimeMillis: 0 + } + }); + + const fakeListener = stub(); + const errorHandler = stub(); + const fakeNetworkError = new Error('fetch failed or something'); + + const fakeExchange = stub(client, 'exchangeToken').returns( + Promise.reject(fakeNetworkError) + ); + + addTokenListener( + appCheck as AppCheckService, + ListenerType.EXTERNAL, + fakeListener, + errorHandler + ); + // Tick 40s, expect one initial exchange request and one retry. + // (First backoff is 30s). + await clock.tickAsync(40000); + expect(fakeListener).not.to.be.called; + expect(fakeExchange).to.be.calledTwice; + expect(errorHandler).to.be.calledTwice; + clock.restore(); + }); }); describe('removeTokenListener', () => { it('should remove token listeners', () => { - const listener = (): void => {}; + const listener = (): void => { }; setState(app, { ...getState(app), cachedTokenPromise: Promise.resolve(undefined) @@ -555,7 +839,7 @@ describe('internal api', () => { }); it('should stop proactively refreshing token after deleting the last listener', async () => { - const listener = (): void => {}; + const listener = (): void => { }; setState(app, { ...getState(app), isTokenAutoRefreshEnabled: true }); setState(app, { ...getState(app), diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 7597a50a66d..81ff6af741c 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -74,23 +74,30 @@ export async function getToken( let error: Error | undefined = undefined; /** - * If there is no token in memory, try to load token from indexedDB. + * If an invalid token was found in memory, clear token from + * memory and unset the local variable `token`. + */ + if (token && !isValid(token)) { + setState(app, { ...state, token: undefined }); + token = undefined; + } + + /** + * If there is no valid token in memory, try to load token from indexedDB. */ if (!token) { // cachedTokenPromise contains the token found in IndexedDB or undefined if not found. const cachedToken = await state.cachedTokenPromise; - if (cachedToken && isValid(cachedToken)) { - token = cachedToken; + if (cachedToken) { + if (isValid(cachedToken)) { + token = cachedToken; + } else { + // If there was an invalid token in the indexedDB cache, clear it. + await writeTokenToStorage(app, undefined); + } } } - // If an invalid token was found in memory or indexedDB, clear token from - // memory and the local variable. - if (token && !isValid(token)) { - setState(app, { ...state, token: undefined }); - token = undefined; - } - // Return the cached token (from either memory or indexedDB) if it's valid if (!forceRefresh && token && isValid(token)) { return { @@ -130,7 +137,9 @@ export async function getToken( } /** - * request a new token + * There are no valid tokens in memory or indexedDB and we are not in + * debug mode. + * Request a new token from the exchange endpoint. */ try { // Avoid making another call to the exchange endpoint if one is in flight. @@ -158,13 +167,28 @@ export async function getToken( } let interopTokenResult: AppCheckTokenResult | undefined; - if (!token || error) { + if (!token) { // If token is undefined, there must be an error. - // It's also possible a token exists, but there's also an error. (Such as - // if the token is almost expired, tries to refresh, and fails the - // exchange request.) - // In either case, return a dummy token along with the error. + // Return a dummy token along with the error. interopTokenResult = makeDummyTokenResult(error!); + } else if (error && isValid(token)) { + if (isValid(token)) { + // It's also possible a valid token exists, but there's also an error. + // (Such as if the token is almost expired, tries to refresh, and + // the exchange request fails.) + // We add a special error property here so that the refresher will + // count this as a failed attempt and use the backoff instead of + // retrying repeatedly with no delay, but any 3P listeners will not + // be hindered in getting the still-valid token. + interopTokenResult = { + token: token.token, + internalError: error + }; + } else { + // No invalid tokens should make it to this step. Memory and cached tokens + // are checked. Other tokens are from fresh exchanges. But just in case. + interopTokenResult = makeDummyTokenResult(error!); + } } else { interopTokenResult = { token: token.token @@ -284,10 +308,24 @@ function createTokenRefresher(appCheck: AppCheckService): Refresher { result = await getToken(appCheck, true); } - // getToken() always resolves. In case the result has an error field defined, it means the operation failed, and we should retry. + /** + * getToken() always resolves. In case the result has an error field defined, it means + * the operation failed, and we should retry. + */ if (result.error) { throw result.error; } + /** + * A special `internalError` field reflects that there was an error + * getting a new token from the exchange endpoint, but there's still a + * previous token that's valid for now and this should be passed to 2P/3P + * requests for a token. But we want this callback (`this.operation` in + * `Refresher`) to throw in order to kick off the Refresher's retry + * backoff. (Setting `hasSucceeded` to false.) + */ + if (result.internalError) { + throw result.internalError; + } }, () => { return true; @@ -300,7 +338,7 @@ function createTokenRefresher(appCheck: AppCheckService): Refresher { let nextRefreshTimeMillis = state.token.issuedAtTimeMillis + (state.token.expireTimeMillis - state.token.issuedAtTimeMillis) * - 0.5 + + 0.5 + 5 * 60 * 1000; // Do not allow refresh time to be past (expireTime - 5 minutes) const latestAllowableRefresh = diff --git a/packages/app-check/src/storage.ts b/packages/app-check/src/storage.ts index 0acc5ddc218..5c680a08393 100644 --- a/packages/app-check/src/storage.ts +++ b/packages/app-check/src/storage.ts @@ -51,7 +51,7 @@ export async function readTokenFromStorage( */ export function writeTokenToStorage( app: FirebaseApp, - token: AppCheckTokenInternal + token?: AppCheckTokenInternal ): Promise { if (isIndexedDBAvailable()) { return writeTokenToIndexedDB(app, token).catch(e => { diff --git a/packages/app-check/src/types.ts b/packages/app-check/src/types.ts index 534509d04d4..e4292f2d51a 100644 --- a/packages/app-check/src/types.ts +++ b/packages/app-check/src/types.ts @@ -52,6 +52,10 @@ export const enum ListenerType { export interface AppCheckTokenResult { readonly token: string; readonly error?: Error; + // Error that should not be propagated to 3P listeners, e.g. exchange + // request errors during proactive refresh period, while the token + // is still valid. + readonly internalError?: Error; } export interface AppCheckTokenInternal extends AppCheckToken { From dd0ecc700e3c668fde765657ddfa9c570c2d153c Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 20 Sep 2022 12:56:51 -0700 Subject: [PATCH 5/6] Formatting pass. --- packages/app-check/src/internal-api.test.ts | 52 +++++++++++---------- packages/app-check/src/internal-api.ts | 2 +- 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index 9c082bcbe73..03c690fb42e 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -373,7 +373,8 @@ describe('internal api', () => { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) }); setState(app, { - ...getState(app), token: { + ...getState(app), + token: { token: 'something', expireTimeMillis: Date.now() - 1000, issuedAtTimeMillis: 0 @@ -428,25 +429,17 @@ describe('internal api', () => { }; stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken)); - stub(client, 'exchangeToken').returns( - Promise.resolve(freshToken) - ); + stub(client, 'exchangeToken').returns(Promise.resolve(freshToken)); expect(await getToken(appCheck as AppCheckService)).to.deep.equal({ token: 'new-recaptcha-app-check-token' }); // When it wiped the invalid token. - expect(storageWriteStub).has.been.calledWith( - app, - undefined - ); + expect(storageWriteStub).has.been.calledWith(app, undefined); // When it wrote the new token fetched from the exchange endpoint. - expect(storageWriteStub).has.been.calledWith( - app, - freshToken - ); + expect(storageWriteStub).has.been.calledWith(app, freshToken); }); it('returns the actual token and an internalError if a token is valid but the request fails', async () => { @@ -539,7 +532,7 @@ describe('internal api', () => { describe('addTokenListener', () => { it('adds token listeners', () => { - const listener = (): void => { }; + const listener = (): void => {}; setState(app, { ...getState(app), cachedTokenPromise: Promise.resolve(undefined) @@ -555,7 +548,7 @@ describe('internal api', () => { }); it('starts proactively refreshing token after adding the first listener', async () => { - const listener = (): void => { }; + const listener = (): void => {}; setState(app, { ...getState(app), isTokenAutoRefreshEnabled: true, @@ -638,7 +631,8 @@ describe('internal api', () => { isTokenAutoRefreshEnabled: true }); setState(app, { - ...getState(app), token: { + ...getState(app), + token: { token: `fake-cached-app-check-token`, // within refresh window expireTimeMillis: 10000, @@ -663,8 +657,12 @@ describe('internal api', () => { ); // Tick 10s, make sure nothing is called repeatedly in that time. await clock.tickAsync(10000); - expect(fakeListener).to.be.calledWith({ token: 'fake-cached-app-check-token' }); - expect(fakeListener).to.be.calledWith({ token: 'new-recaptcha-app-check-token' }); + expect(fakeListener).to.be.calledWith({ + token: 'fake-cached-app-check-token' + }); + expect(fakeListener).to.be.calledWith({ + token: 'new-recaptcha-app-check-token' + }); expect(fakeExchange).to.be.calledOnce; clock.restore(); }); @@ -677,7 +675,8 @@ describe('internal api', () => { isTokenAutoRefreshEnabled: true }); setState(app, { - ...getState(app), token: { + ...getState(app), + token: { token: `fake-cached-app-check-token`, // not expired but within refresh window expireTimeMillis: 10000, @@ -698,7 +697,9 @@ describe('internal api', () => { ); // Tick 10s, make sure nothing is called repeatedly in that time. await clock.tickAsync(10000); - expect(fakeListener).to.be.calledWith({ token: 'fake-cached-app-check-token' }); + expect(fakeListener).to.be.calledWith({ + token: 'fake-cached-app-check-token' + }); // once on init and once invoked directly in this test expect(fakeListener).to.be.calledTwice; expect(fakeExchange).to.be.calledOnce; @@ -713,7 +714,8 @@ describe('internal api', () => { isTokenAutoRefreshEnabled: true }); setState(app, { - ...getState(app), token: { + ...getState(app), + token: { token: `fake-cached-app-check-token`, // not expired but within refresh window expireTimeMillis: 10000, @@ -749,7 +751,8 @@ describe('internal api', () => { isTokenAutoRefreshEnabled: true }); setState(app, { - ...getState(app), token: { + ...getState(app), + token: { token: `fake-cached-app-check-token`, // expired expireTimeMillis: 0, @@ -788,7 +791,8 @@ describe('internal api', () => { isTokenAutoRefreshEnabled: true }); setState(app, { - ...getState(app), token: { + ...getState(app), + token: { token: `fake-cached-app-check-token`, // expired expireTimeMillis: 0, @@ -822,7 +826,7 @@ describe('internal api', () => { describe('removeTokenListener', () => { it('should remove token listeners', () => { - const listener = (): void => { }; + const listener = (): void => {}; setState(app, { ...getState(app), cachedTokenPromise: Promise.resolve(undefined) @@ -839,7 +843,7 @@ describe('internal api', () => { }); it('should stop proactively refreshing token after deleting the last listener', async () => { - const listener = (): void => { }; + const listener = (): void => {}; setState(app, { ...getState(app), isTokenAutoRefreshEnabled: true }); setState(app, { ...getState(app), diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 81ff6af741c..2260ff05067 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -338,7 +338,7 @@ function createTokenRefresher(appCheck: AppCheckService): Refresher { let nextRefreshTimeMillis = state.token.issuedAtTimeMillis + (state.token.expireTimeMillis - state.token.issuedAtTimeMillis) * - 0.5 + + 0.5 + 5 * 60 * 1000; // Do not allow refresh time to be past (expireTime - 5 minutes) const latestAllowableRefresh = From 15a052442ffd1756f5f17e31b8fcd344647b8954 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Wed, 21 Sep 2022 09:09:15 -0700 Subject: [PATCH 6/6] Fix 2 small errors --- packages/app-check/src/internal-api.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 2260ff05067..99c0f5d2c00 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -128,7 +128,7 @@ export async function getToken( shouldCallListeners = true; } const tokenFromDebugExchange: AppCheckTokenInternal = - await state.exchangeTokenPromise!; + await state.exchangeTokenPromise; // Write debug token to indexedDB. await writeTokenToStorage(app, tokenFromDebugExchange); // Write debug token to state. @@ -171,7 +171,7 @@ export async function getToken( // If token is undefined, there must be an error. // Return a dummy token along with the error. interopTokenResult = makeDummyTokenResult(error!); - } else if (error && isValid(token)) { + } else if (error) { if (isValid(token)) { // It's also possible a valid token exists, but there's also an error. // (Such as if the token is almost expired, tries to refresh, and