Skip to content

Commit b3951c6

Browse files
hsubox76hsubox76
authored
Fix App Check timer issues (#6617)
1 parent e35db6f commit b3951c6

6 files changed

Lines changed: 358 additions & 13 deletions

File tree

‎.changeset/selfish-worms-glow.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@firebase/app-check': patch
3+
---
4+
5+
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.

‎packages/app-check/src/indexeddb.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export function readTokenFromIndexedDB(
8080

8181
export function writeTokenToIndexedDB(
8282
app: FirebaseApp,
83-
token: AppCheckTokenInternal
83+
token?: AppCheckTokenInternal
8484
): Promise<void> {
8585
return write(computeKey(app), token);
8686
}

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

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

371+
it('ignores in-memory token if it is invalid and continues to exchange request', async () => {
372+
const appCheck = initializeAppCheck(app, {
373+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
374+
});
375+
setState(app, {
376+
...getState(app),
377+
token: {
378+
token: 'something',
379+
expireTimeMillis: Date.now() - 1000,
380+
issuedAtTimeMillis: 0
381+
}
382+
});
383+
384+
stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken));
385+
stub(client, 'exchangeToken').returns(
386+
Promise.resolve({
387+
token: 'new-recaptcha-app-check-token',
388+
expireTimeMillis: Date.now() + 60000,
389+
issuedAtTimeMillis: 0
390+
})
391+
);
392+
393+
expect(await getToken(appCheck as AppCheckService)).to.deep.equal({
394+
token: 'new-recaptcha-app-check-token'
395+
});
396+
});
397+
398+
it('returns the valid token in storage without making a network request', async () => {
399+
const clock = useFakeTimers();
400+
401+
storageReadStub.resolves(fakeCachedAppCheckToken);
402+
const appCheck = initializeAppCheck(app, {
403+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
404+
});
405+
406+
const clientStub = stub(client, 'exchangeToken');
407+
expect(await getToken(appCheck as AppCheckService)).to.deep.equal({
408+
token: fakeCachedAppCheckToken.token
409+
});
410+
expect(clientStub).to.not.have.been.called;
411+
412+
clock.restore();
413+
});
414+
415+
it('deletes cached token if it is invalid and continues to exchange request', async () => {
416+
storageReadStub.resolves({
417+
token: 'something',
418+
expireTimeMillis: Date.now() - 1000,
419+
issuedAtTimeMillis: 0
420+
});
421+
const appCheck = initializeAppCheck(app, {
422+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
423+
});
424+
425+
const freshToken = {
426+
token: 'new-recaptcha-app-check-token',
427+
expireTimeMillis: Date.now() + 60000,
428+
issuedAtTimeMillis: 0
429+
};
430+
431+
stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken));
432+
stub(client, 'exchangeToken').returns(Promise.resolve(freshToken));
433+
434+
expect(await getToken(appCheck as AppCheckService)).to.deep.equal({
435+
token: 'new-recaptcha-app-check-token'
436+
});
437+
438+
// When it wiped the invalid token.
439+
expect(storageWriteStub).has.been.calledWith(app, undefined);
440+
441+
// When it wrote the new token fetched from the exchange endpoint.
442+
expect(storageWriteStub).has.been.calledWith(app, freshToken);
443+
});
444+
445+
it('returns the actual token and an internalError if a token is valid but the request fails', async () => {
446+
stub(logger, 'error');
447+
const appCheck = initializeAppCheck(app, {
448+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
449+
});
450+
setState(app, { ...getState(app), token: fakeRecaptchaAppCheckToken });
451+
452+
stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken));
453+
stub(client, 'exchangeToken').returns(Promise.reject(new Error('blah')));
454+
455+
const tokenResult = await getToken(appCheck as AppCheckService, true);
456+
expect(tokenResult.internalError?.message).to.equal('blah');
457+
expect(tokenResult.token).to.equal('fake-recaptcha-app-check-token');
458+
});
459+
371460
it('exchanges debug token if in debug mode and there is no cached token', async () => {
372461
const exchangeTokenStub: SinonStub = stub(
373462
client,
@@ -534,6 +623,205 @@ describe('internal api', () => {
534623
fakeListener
535624
);
536625
});
626+
627+
it('does not make rapid requests within proactive refresh window', async () => {
628+
const clock = useFakeTimers();
629+
const appCheck = initializeAppCheck(app, {
630+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
631+
isTokenAutoRefreshEnabled: true
632+
});
633+
setState(app, {
634+
...getState(app),
635+
token: {
636+
token: `fake-cached-app-check-token`,
637+
// within refresh window
638+
expireTimeMillis: 10000,
639+
issuedAtTimeMillis: 0
640+
}
641+
});
642+
643+
const fakeListener: AppCheckTokenListener = stub();
644+
645+
const fakeExchange = stub(client, 'exchangeToken').returns(
646+
Promise.resolve({
647+
token: 'new-recaptcha-app-check-token',
648+
expireTimeMillis: 10 * 60 * 1000,
649+
issuedAtTimeMillis: 0
650+
})
651+
);
652+
653+
addTokenListener(
654+
appCheck as AppCheckService,
655+
ListenerType.INTERNAL,
656+
fakeListener
657+
);
658+
// Tick 10s, make sure nothing is called repeatedly in that time.
659+
await clock.tickAsync(10000);
660+
expect(fakeListener).to.be.calledWith({
661+
token: 'fake-cached-app-check-token'
662+
});
663+
expect(fakeListener).to.be.calledWith({
664+
token: 'new-recaptcha-app-check-token'
665+
});
666+
expect(fakeExchange).to.be.calledOnce;
667+
clock.restore();
668+
});
669+
670+
it('proactive refresh window test - exchange request fails - wait 10s', async () => {
671+
stub(logger, 'error');
672+
const clock = useFakeTimers();
673+
const appCheck = initializeAppCheck(app, {
674+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
675+
isTokenAutoRefreshEnabled: true
676+
});
677+
setState(app, {
678+
...getState(app),
679+
token: {
680+
token: `fake-cached-app-check-token`,
681+
// not expired but within refresh window
682+
expireTimeMillis: 10000,
683+
issuedAtTimeMillis: 0
684+
}
685+
});
686+
687+
const fakeListener: AppCheckTokenListener = stub();
688+
689+
const fakeExchange = stub(client, 'exchangeToken').returns(
690+
Promise.reject(new Error('fetch failed or something'))
691+
);
692+
693+
addTokenListener(
694+
appCheck as AppCheckService,
695+
ListenerType.EXTERNAL,
696+
fakeListener
697+
);
698+
// Tick 10s, make sure nothing is called repeatedly in that time.
699+
await clock.tickAsync(10000);
700+
expect(fakeListener).to.be.calledWith({
701+
token: 'fake-cached-app-check-token'
702+
});
703+
// once on init and once invoked directly in this test
704+
expect(fakeListener).to.be.calledTwice;
705+
expect(fakeExchange).to.be.calledOnce;
706+
clock.restore();
707+
});
708+
709+
it('proactive refresh window test - exchange request fails - wait 40s', async () => {
710+
stub(logger, 'error');
711+
const clock = useFakeTimers();
712+
const appCheck = initializeAppCheck(app, {
713+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
714+
isTokenAutoRefreshEnabled: true
715+
});
716+
setState(app, {
717+
...getState(app),
718+
token: {
719+
token: `fake-cached-app-check-token`,
720+
// not expired but within refresh window
721+
expireTimeMillis: 10000,
722+
issuedAtTimeMillis: 0
723+
}
724+
});
725+
726+
const fakeListener: AppCheckTokenListener = stub();
727+
728+
const fakeExchange = stub(client, 'exchangeToken').returns(
729+
Promise.reject(new Error('fetch failed or something'))
730+
);
731+
732+
addTokenListener(
733+
appCheck as AppCheckService,
734+
ListenerType.EXTERNAL,
735+
fakeListener
736+
);
737+
// Tick 40s, expect one initial exchange request and one retry.
738+
// (First backoff is 30s).
739+
await clock.tickAsync(40000);
740+
expect(fakeListener).to.be.calledTwice;
741+
expect(fakeExchange).to.be.calledTwice;
742+
clock.restore();
743+
});
744+
745+
it('expired token - exchange request fails - wait 10s', async () => {
746+
stub(logger, 'error');
747+
const clock = useFakeTimers();
748+
clock.tick(1);
749+
const appCheck = initializeAppCheck(app, {
750+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
751+
isTokenAutoRefreshEnabled: true
752+
});
753+
setState(app, {
754+
...getState(app),
755+
token: {
756+
token: `fake-cached-app-check-token`,
757+
// expired
758+
expireTimeMillis: 0,
759+
issuedAtTimeMillis: 0
760+
}
761+
});
762+
763+
const fakeListener = stub();
764+
const errorHandler = stub();
765+
const fakeNetworkError = new Error('fetch failed or something');
766+
767+
const fakeExchange = stub(client, 'exchangeToken').returns(
768+
Promise.reject(fakeNetworkError)
769+
);
770+
771+
addTokenListener(
772+
appCheck as AppCheckService,
773+
ListenerType.EXTERNAL,
774+
fakeListener,
775+
errorHandler
776+
);
777+
// Tick 10s, make sure nothing is called repeatedly in that time.
778+
await clock.tickAsync(10000);
779+
expect(fakeListener).not.to.be.called;
780+
expect(fakeExchange).to.be.calledOnce;
781+
expect(errorHandler).to.be.calledWith(fakeNetworkError);
782+
clock.restore();
783+
});
784+
785+
it('expired token - exchange request fails - wait 40s', async () => {
786+
stub(logger, 'error');
787+
const clock = useFakeTimers();
788+
clock.tick(1);
789+
const appCheck = initializeAppCheck(app, {
790+
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
791+
isTokenAutoRefreshEnabled: true
792+
});
793+
setState(app, {
794+
...getState(app),
795+
token: {
796+
token: `fake-cached-app-check-token`,
797+
// expired
798+
expireTimeMillis: 0,
799+
issuedAtTimeMillis: 0
800+
}
801+
});
802+
803+
const fakeListener = stub();
804+
const errorHandler = stub();
805+
const fakeNetworkError = new Error('fetch failed or something');
806+
807+
const fakeExchange = stub(client, 'exchangeToken').returns(
808+
Promise.reject(fakeNetworkError)
809+
);
810+
811+
addTokenListener(
812+
appCheck as AppCheckService,
813+
ListenerType.EXTERNAL,
814+
fakeListener,
815+
errorHandler
816+
);
817+
// Tick 40s, expect one initial exchange request and one retry.
818+
// (First backoff is 30s).
819+
await clock.tickAsync(40000);
820+
expect(fakeListener).not.to.be.called;
821+
expect(fakeExchange).to.be.calledTwice;
822+
expect(errorHandler).to.be.calledTwice;
823+
clock.restore();
824+
});
537825
});
538826

539827
describe('removeTokenListener', () => {

0 commit comments

Comments
 (0)