Skip to content

Commit bb54aab

Browse files
sam-gcsam-gc
committed
PR feedback
1 parent 7a55378 commit bb54aab

File tree

4 files changed

+109
-215
lines changed

4 files changed

+109
-215
lines changed

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

+9-19
Original file line numberDiff line numberDiff line change
@@ -293,26 +293,16 @@ describe('api', () => {
293293
it('getLimitedUseToken() calls the internal getLimitedUseToken() function', async () => {
294294
const app = getFakeApp({ automaticDataCollectionEnabled: true });
295295
const appCheck = getFakeAppCheck(app);
296-
const internalgetLimitedUseToken = stub(internalApi, 'getToken').resolves(
297-
{
298-
token: 'a-token-string'
299-
}
300-
);
301-
await getLimitedUseToken(appCheck);
302-
expect(internalgetLimitedUseToken).to.be.calledWith(appCheck);
303-
});
304-
it('getLimitedUseToken() throws errors returned with token', async () => {
305-
const app = getFakeApp({ automaticDataCollectionEnabled: true });
306-
const appCheck = getFakeAppCheck(app);
307-
// If the internal getToken() errors, it returns a dummy token
308-
// with an error field instead of throwing.
309-
stub(internalApi, 'getToken').resolves({
310-
token: 'a-dummy-token',
311-
error: Error('there was an error')
296+
const internalgetLimitedUseToken = stub(
297+
internalApi,
298+
'getLimitedUseToken'
299+
).resolves({
300+
token: 'a-token-string'
312301
});
313-
await expect(getLimitedUseToken(appCheck)).to.be.rejectedWith(
314-
'there was an error'
315-
);
302+
expect(await getLimitedUseToken(appCheck)).to.eql({
303+
token: 'a-token-string'
304+
});
305+
expect(internalgetLimitedUseToken).to.be.calledWith(appCheck);
316306
});
317307
});
318308
describe('onTokenChanged()', () => {

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

+3-10
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { AppCheckService } from './factory';
3535
import { AppCheckProvider, ListenerType } from './types';
3636
import {
3737
getToken as getTokenInternal,
38+
getLimitedUseToken as getLimitedUseTokenInternal,
3839
addTokenListener,
3940
removeTokenListener,
4041
isValid,
@@ -224,18 +225,10 @@ export async function getToken(
224225
* @returns The limited use token.
225226
* @public
226227
*/
227-
export async function getLimitedUseToken(
228+
export function getLimitedUseToken(
228229
appCheckInstance: AppCheck
229230
): Promise<AppCheckTokenResult> {
230-
const result = await getTokenInternal(
231-
appCheckInstance as AppCheckService,
232-
/* forceRefresh */ true,
233-
/* isLimitedUse */ true
234-
);
235-
if (result.error) {
236-
throw result.error;
237-
}
238-
return { token: result.token };
231+
return getLimitedUseTokenInternal(appCheckInstance as AppCheckService);
239232
}
240233

241234
/**

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

+11-102
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ import {
3333
addTokenListener,
3434
removeTokenListener,
3535
formatDummyToken,
36-
defaultTokenErrorData
36+
defaultTokenErrorData,
37+
getLimitedUseToken
3738
} from './internal-api';
3839
import * as reCAPTCHA from './recaptcha';
3940
import * as client from './client';
@@ -46,11 +47,11 @@ import {
4647
setInitialState,
4748
getDebugState
4849
} from './state';
49-
import { AppCheck, AppCheckTokenListener } from './public-types';
50+
import { AppCheckTokenListener } from './public-types';
5051
import { Deferred } from '@firebase/util';
5152
import { ReCaptchaEnterpriseProvider, ReCaptchaV3Provider } from './providers';
5253
import { AppCheckService } from './factory';
53-
import { AppCheckTokenResult, ListenerType } from './types';
54+
import { ListenerType } from './types';
5455
import { AppCheckError, ERROR_FACTORY } from './errors';
5556

5657
const fakeRecaptchaToken = 'fake-recaptcha-token';
@@ -666,24 +667,14 @@ describe('internal api', () => {
666667
});
667668

668669
describe('getToken() for limited use', () => {
669-
function getLimitedUseToken(
670-
appCheck: AppCheck
671-
): Promise<AppCheckTokenResult> {
672-
return getToken(
673-
appCheck as AppCheckService,
674-
/*forceRefresh*/ true,
675-
/* isLimitedUse */ true
676-
);
677-
}
678-
679670
it('uses customTokenProvider to get an AppCheck token', async () => {
680671
const customTokenProvider = getFakeCustomTokenProvider();
681672
const customProviderSpy = spy(customTokenProvider, 'getToken');
682673

683674
const appCheck = initializeAppCheck(app, {
684675
provider: customTokenProvider
685676
});
686-
const token = await getLimitedUseToken(appCheck);
677+
const token = await getLimitedUseToken(appCheck as AppCheckService);
687678

688679
expect(customProviderSpy).to.be.called;
689680
expect(token).to.deep.equal({
@@ -698,7 +689,7 @@ describe('internal api', () => {
698689
const appCheck = initializeAppCheck(app, {
699690
provider: customTokenProvider
700691
});
701-
await getLimitedUseToken(appCheck);
692+
await getLimitedUseToken(appCheck as AppCheckService);
702693

703694
expect(getStateReference(app).token).to.be.undefined;
704695
expect(getStateReference(app).isTokenAutoRefreshEnabled).to.be.false;
@@ -709,15 +700,13 @@ describe('internal api', () => {
709700
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
710701
});
711702

712-
const reCAPTCHASpy = stub(reCAPTCHA, 'getToken').returns(
713-
Promise.resolve(fakeRecaptchaToken)
714-
);
703+
const reCAPTCHASpy = stubGetRecaptchaToken();
715704
const exchangeTokenStub: SinonStub = stub(
716705
client,
717706
'exchangeToken'
718707
).returns(Promise.resolve(fakeRecaptchaAppCheckToken));
719708

720-
const token = await getLimitedUseToken(appCheck);
709+
const token = await getLimitedUseToken(appCheck as AppCheckService);
721710

722711
expect(reCAPTCHASpy).to.be.called;
723712

@@ -732,15 +721,13 @@ describe('internal api', () => {
732721
provider: new ReCaptchaEnterpriseProvider(FAKE_SITE_KEY)
733722
});
734723

735-
const reCAPTCHASpy = stub(reCAPTCHA, 'getToken').returns(
736-
Promise.resolve(fakeRecaptchaToken)
737-
);
724+
const reCAPTCHASpy = stubGetRecaptchaToken();
738725
const exchangeTokenStub: SinonStub = stub(
739726
client,
740727
'exchangeToken'
741728
).returns(Promise.resolve(fakeRecaptchaAppCheckToken));
742729

743-
const token = await getLimitedUseToken(appCheck);
730+
const token = await getLimitedUseToken(appCheck as AppCheckService);
744731

745732
expect(reCAPTCHASpy).to.be.called;
746733

@@ -750,32 +737,6 @@ describe('internal api', () => {
750737
expect(token).to.deep.equal({ token: fakeRecaptchaAppCheckToken.token });
751738
});
752739

753-
it('resolves with a dummy token and an error if failed to get a token', async () => {
754-
const errorStub = stub(console, 'error');
755-
const appCheck = initializeAppCheck(app, {
756-
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
757-
});
758-
759-
const reCAPTCHASpy = stub(reCAPTCHA, 'getToken').returns(
760-
Promise.resolve(fakeRecaptchaToken)
761-
);
762-
763-
const error = new Error('oops, something went wrong');
764-
stub(client, 'exchangeToken').returns(Promise.reject(error));
765-
766-
const token = await getLimitedUseToken(appCheck);
767-
768-
expect(reCAPTCHASpy).to.be.called;
769-
expect(token).to.deep.equal({
770-
token: formatDummyToken(defaultTokenErrorData),
771-
error
772-
});
773-
expect(errorStub.args[0][1].message).to.include(
774-
'oops, something went wrong'
775-
);
776-
errorStub.restore();
777-
});
778-
779740
it('exchanges debug token if in debug mode', async () => {
780741
const exchangeTokenStub: SinonStub = stub(
781742
client,
@@ -789,64 +750,12 @@ describe('internal api', () => {
789750
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
790751
});
791752

792-
const token = await getLimitedUseToken(appCheck);
753+
const token = await getLimitedUseToken(appCheck as AppCheckService);
793754
expect(exchangeTokenStub.args[0][0].body['debug_token']).to.equal(
794755
'my-debug-token'
795756
);
796757
expect(token).to.deep.equal({ token: fakeRecaptchaAppCheckToken.token });
797758
});
798-
799-
it('throttles for a period less than 1d on 503', async () => {
800-
// More detailed check of exponential backoff in providers.test.ts
801-
const appCheck = initializeAppCheck(app, {
802-
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
803-
});
804-
const warnStub = stub(logger, 'warn');
805-
stub(client, 'exchangeToken').returns(
806-
Promise.reject(
807-
ERROR_FACTORY.create(AppCheckError.FETCH_STATUS_ERROR, {
808-
httpStatus: 503
809-
})
810-
)
811-
);
812-
813-
const token = await getLimitedUseToken(appCheck);
814-
815-
// ReCaptchaV3Provider's _throttleData is private so checking
816-
// the resulting error message to be sure it has roughly the
817-
// correct throttle time. This also tests the time formatter.
818-
// Check both the error itself and that it makes it through to
819-
// console.warn
820-
expect(token.error?.message).to.include('503');
821-
expect(token.error?.message).to.include('00m');
822-
expect(token.error?.message).to.not.include('1d');
823-
expect(warnStub.args[0][0]).to.include('503');
824-
});
825-
826-
it('throttles 1d on 403', async () => {
827-
const appCheck = initializeAppCheck(app, {
828-
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY)
829-
});
830-
const warnStub = stub(logger, 'warn');
831-
stub(client, 'exchangeToken').returns(
832-
Promise.reject(
833-
ERROR_FACTORY.create(AppCheckError.FETCH_STATUS_ERROR, {
834-
httpStatus: 403
835-
})
836-
)
837-
);
838-
839-
const token = await getLimitedUseToken(appCheck);
840-
841-
// ReCaptchaV3Provider's _throttleData is private so checking
842-
// the resulting error message to be sure it has roughly the
843-
// correct throttle time. This also tests the time formatter.
844-
// Check both the error itself and that it makes it through to
845-
// console.warn
846-
expect(token.error?.message).to.include('403');
847-
expect(token.error?.message).to.include('1d');
848-
expect(warnStub.args[0][0]).to.include('403');
849-
});
850759
});
851760

852761
describe('addTokenListener', () => {

0 commit comments

Comments
 (0)