From 797a2e9446118e31eb6e85bcd9b56e354b211e39 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Wed, 29 Mar 2023 15:25:28 -0700 Subject: [PATCH 01/11] FAC scoped token support --- common/api-review/app-check.api.md | 3 + packages/app-check/src/api.test.ts | 30 +++- packages/app-check/src/api.ts | 23 +++ packages/app-check/src/internal-api.test.ts | 177 +++++++++++++++++++- packages/app-check/src/internal-api.ts | 83 +++++++++ 5 files changed, 314 insertions(+), 2 deletions(-) diff --git a/common/api-review/app-check.api.md b/common/api-review/app-check.api.md index 9623a51c9ad..a7917973db2 100644 --- a/common/api-review/app-check.api.md +++ b/common/api-review/app-check.api.md @@ -60,6 +60,9 @@ export interface CustomProviderOptions { getToken: () => Promise; } +// @public +export function getScopedToken(appCheckInstance: AppCheck): Promise; + // @public export function getToken(appCheckInstance: AppCheck, forceRefresh?: boolean): Promise; diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index 5f6cf1082a3..8c56d119716 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -21,7 +21,8 @@ import { setTokenAutoRefreshEnabled, initializeAppCheck, getToken, - onTokenChanged + onTokenChanged, + getScopedToken } from './api'; import { FAKE_SITE_KEY, @@ -288,6 +289,33 @@ describe('api', () => { ); }); }); + describe('getScopedToken()', () => { + it('getScopedToken() calls the internal getScopedToken() function', async () => { + const app = getFakeApp({ automaticDataCollectionEnabled: true }); + const appCheck = getFakeAppCheck(app); + const internalGetScopedToken = stub( + internalApi, + 'getScopedToken' + ).resolves({ + token: 'a-token-string' + }); + await getScopedToken(appCheck); + expect(internalGetScopedToken).to.be.calledWith(appCheck); + }); + it('getScopedToken() throws errors returned with token', async () => { + const app = getFakeApp({ automaticDataCollectionEnabled: true }); + const appCheck = getFakeAppCheck(app); + // If getScopedToken() errors, it returns a dummy token with an error field + // instead of throwing. + stub(internalApi, 'getScopedToken').resolves({ + token: 'a-dummy-token', + error: Error('there was an error') + }); + await expect(getScopedToken(appCheck)).to.be.rejectedWith( + 'there was an error' + ); + }); + }); describe('onTokenChanged()', () => { it('Listeners work when using top-level parameters pattern', async () => { const appCheck = initializeAppCheck(app, { diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index 1b7e29e008d..23b2462a0e5 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -35,6 +35,7 @@ import { AppCheckService } from './factory'; import { AppCheckProvider, ListenerType } from './types'; import { getToken as getTokenInternal, + getScopedToken as getScopedTokenInternal, addTokenListener, removeTokenListener, isValid, @@ -209,6 +210,28 @@ export async function getToken( return { token: result.token }; } +/** + * Requests a Firebase App Check token. This method should be used ONLY if you + * need to authorize requests to a non-Firebase backend. Tokens from this + * method are intended for use with the Admin SDKs when `consume` is set to + * true. Tokens from this method do not interact with FirebaseAppCheck’s token + * cache. + * + * @param appCheckInstance - The App Check service instance. + * @public + */ +export async function getScopedToken( + appCheckInstance: AppCheck +): Promise { + const result = await getScopedTokenInternal( + appCheckInstance as AppCheckService + ); + if (result.error) { + throw result.error; + } + return { token: result.token }; +} + /** * Registers a listener to changes in the token state. There can be more * than one listener registered at the same time for one or more diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index 81a0bdb2f99..cee5a6985af 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -32,7 +32,8 @@ import { addTokenListener, removeTokenListener, formatDummyToken, - defaultTokenErrorData + defaultTokenErrorData, + getScopedToken } from './internal-api'; import * as reCAPTCHA from './recaptcha'; import * as client from './client'; @@ -663,6 +664,180 @@ describe('internal api', () => { }); }); + describe('getScopedToken()', () => { + it('uses customTokenProvider to get an AppCheck token', async () => { + const customTokenProvider = getFakeCustomTokenProvider(); + const customProviderSpy = spy(customTokenProvider, 'getToken'); + + const appCheck = initializeAppCheck(app, { + provider: customTokenProvider + }); + const token = await getScopedToken(appCheck as AppCheckService); + + expect(customProviderSpy).to.be.called; + expect(token).to.deep.equal({ + token: 'fake-custom-app-check-token' + }); + }); + + it('does not interact with state', async () => { + const customTokenProvider = getFakeCustomTokenProvider(); + spy(customTokenProvider, 'getToken'); + + const appCheck = initializeAppCheck(app, { + provider: customTokenProvider + }); + await getScopedToken(appCheck as AppCheckService); + + expect(getStateReference(app).token).to.be.undefined; + expect(getStateReference(app).isTokenAutoRefreshEnabled).to.be.false; + }); + + it('uses reCAPTCHA (V3) token to exchange for AppCheck token', async () => { + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + + const reCAPTCHASpy = stub(reCAPTCHA, 'getToken').returns( + Promise.resolve(fakeRecaptchaToken) + ); + const exchangeTokenStub: SinonStub = stub( + client, + 'exchangeToken' + ).returns(Promise.resolve(fakeRecaptchaAppCheckToken)); + + const token = await getScopedToken(appCheck as AppCheckService); + + expect(reCAPTCHASpy).to.be.called; + + expect(exchangeTokenStub.args[0][0].body['recaptcha_v3_token']).to.equal( + fakeRecaptchaToken + ); + expect(token).to.deep.equal({ token: fakeRecaptchaAppCheckToken.token }); + }); + + it('uses reCAPTCHA (Enterprise) token to exchange for AppCheck token', async () => { + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaEnterpriseProvider(FAKE_SITE_KEY) + }); + + const reCAPTCHASpy = stub(reCAPTCHA, 'getToken').returns( + Promise.resolve(fakeRecaptchaToken) + ); + const exchangeTokenStub: SinonStub = stub( + client, + 'exchangeToken' + ).returns(Promise.resolve(fakeRecaptchaAppCheckToken)); + + const token = await getScopedToken(appCheck as AppCheckService); + + expect(reCAPTCHASpy).to.be.called; + + expect( + exchangeTokenStub.args[0][0].body['recaptcha_enterprise_token'] + ).to.equal(fakeRecaptchaToken); + expect(token).to.deep.equal({ token: fakeRecaptchaAppCheckToken.token }); + }); + + it('resolves with a dummy token and an error if failed to get a token', async () => { + const errorStub = stub(console, 'error'); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + + const reCAPTCHASpy = stub(reCAPTCHA, 'getToken').returns( + Promise.resolve(fakeRecaptchaToken) + ); + + const error = new Error('oops, something went wrong'); + stub(client, 'exchangeToken').returns(Promise.reject(error)); + + const token = await getScopedToken(appCheck as AppCheckService); + + expect(reCAPTCHASpy).to.be.called; + expect(token).to.deep.equal({ + token: formatDummyToken(defaultTokenErrorData), + error + }); + expect(errorStub.args[0][1].message).to.include( + 'oops, something went wrong' + ); + errorStub.restore(); + }); + + it('exchanges debug token if in debug mode', async () => { + const exchangeTokenStub: SinonStub = stub( + client, + 'exchangeToken' + ).returns(Promise.resolve(fakeRecaptchaAppCheckToken)); + const debugState = getDebugState(); + debugState.enabled = true; + debugState.token = new Deferred(); + debugState.token.resolve('my-debug-token'); + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + + const token = await getScopedToken(appCheck as AppCheckService); + expect(exchangeTokenStub.args[0][0].body['debug_token']).to.equal( + 'my-debug-token' + ); + expect(token).to.deep.equal({ token: fakeRecaptchaAppCheckToken.token }); + }); + + it('throttles for a period less than 1d on 503', async () => { + // More detailed check of exponential backoff in providers.test.ts + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + const warnStub = stub(logger, 'warn'); + stub(client, 'exchangeToken').returns( + Promise.reject( + ERROR_FACTORY.create(AppCheckError.FETCH_STATUS_ERROR, { + httpStatus: 503 + }) + ) + ); + + const token = await getScopedToken(appCheck as AppCheckService); + + // ReCaptchaV3Provider's _throttleData is private so checking + // the resulting error message to be sure it has roughly the + // correct throttle time. This also tests the time formatter. + // Check both the error itself and that it makes it through to + // console.warn + expect(token.error?.message).to.include('503'); + expect(token.error?.message).to.include('00m'); + expect(token.error?.message).to.not.include('1d'); + expect(warnStub.args[0][0]).to.include('503'); + }); + + it('throttles 1d on 403', async () => { + const appCheck = initializeAppCheck(app, { + provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) + }); + const warnStub = stub(logger, 'warn'); + stub(client, 'exchangeToken').returns( + Promise.reject( + ERROR_FACTORY.create(AppCheckError.FETCH_STATUS_ERROR, { + httpStatus: 403 + }) + ) + ); + + const token = await getScopedToken(appCheck as AppCheckService); + + // ReCaptchaV3Provider's _throttleData is private so checking + // the resulting error message to be sure it has roughly the + // correct throttle time. This also tests the time formatter. + // Check both the error itself and that it makes it through to + // console.warn + expect(token.error?.message).to.include('403'); + expect(token.error?.message).to.include('1d'); + expect(warnStub.args[0][0]).to.include('403'); + }); + }); + describe('addTokenListener', () => { afterEach(async () => { clearState(); diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index f07b044be8a..7a18b1f627e 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -205,6 +205,89 @@ export async function getToken( return interopTokenResult; } +/** + * This function always resolves. + * The result will contain an error field if there is any error. + * In case there is an error, the token field in the result will be populated with a dummy value + */ +export async function getScopedToken( + appCheck: AppCheckService +): Promise { + const app = appCheck.app; + ensureActivated(app); + + const { provider } = getStateReference(app); + + /** + * First check if there is a token in memory from a previous `getToken()` call. + */ + let token: AppCheckTokenInternal | undefined = undefined; + let error: Error | undefined = undefined; + + /** + * DEBUG MODE + * If debug mode is set, and there is no cached token, fetch a new App + * Check token using the debug token, and return it directly. + */ + if (isDebugMode()) { + const debugToken = await getDebugToken(); + const tokenFromDebugExchange: AppCheckTokenInternal = await exchangeToken( + getExchangeDebugTokenRequest(app, debugToken), + appCheck.heartbeatServiceProvider + ); + return { token: tokenFromDebugExchange.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 { + token = await provider!.getToken(); + } catch (e) { + if ((e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}`) { + // Warn if throttled, but do not treat it as an error. + logger.warn((e as FirebaseError).message); + } else { + // `getToken()` should never throw, but logging error text to console will aid debugging. + logger.error(e); + } + // Always save error to be added to dummy token. + error = e as FirebaseError; + } + + let interopTokenResult: AppCheckTokenResult | undefined; + if (!token) { + // If token is undefined, there must be an error. + // Return a dummy token along with the error. + interopTokenResult = makeDummyTokenResult(error!); + } 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 + // 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 + }; + } + return interopTokenResult; +} + export function addTokenListener( appCheck: AppCheckService, type: ListenerType, From eacc19d25b58572db274a90a0c554ce5c68af60d Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Wed, 5 Apr 2023 15:09:44 -0700 Subject: [PATCH 02/11] Rename --- packages/app-check/src/api.test.ts | 22 ++++++++++----------- packages/app-check/src/api.ts | 20 +++++++++++-------- packages/app-check/src/internal-api.test.ts | 20 +++++++++---------- packages/app-check/src/internal-api.ts | 2 +- 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index 8c56d119716..cda3bb99891 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -22,7 +22,7 @@ import { initializeAppCheck, getToken, onTokenChanged, - getScopedToken + getLimitedUseToken } from './api'; import { FAKE_SITE_KEY, @@ -289,29 +289,29 @@ describe('api', () => { ); }); }); - describe('getScopedToken()', () => { - it('getScopedToken() calls the internal getScopedToken() function', async () => { + describe('getLimitedUseToken()', () => { + it('getLimitedUseToken() calls the internal getLimitedUseToken() function', async () => { const app = getFakeApp({ automaticDataCollectionEnabled: true }); const appCheck = getFakeAppCheck(app); - const internalGetScopedToken = stub( + const internalgetLimitedUseToken = stub( internalApi, - 'getScopedToken' + 'getLimitedUseToken' ).resolves({ token: 'a-token-string' }); - await getScopedToken(appCheck); - expect(internalGetScopedToken).to.be.calledWith(appCheck); + await getLimitedUseToken(appCheck); + expect(internalgetLimitedUseToken).to.be.calledWith(appCheck); }); - it('getScopedToken() throws errors returned with token', async () => { + it('getLimitedUseToken() throws errors returned with token', async () => { const app = getFakeApp({ automaticDataCollectionEnabled: true }); const appCheck = getFakeAppCheck(app); - // If getScopedToken() errors, it returns a dummy token with an error field + // If getLimitedUseToken() errors, it returns a dummy token with an error field // instead of throwing. - stub(internalApi, 'getScopedToken').resolves({ + stub(internalApi, 'getLimitedUseToken').resolves({ token: 'a-dummy-token', error: Error('there was an error') }); - await expect(getScopedToken(appCheck)).to.be.rejectedWith( + await expect(getLimitedUseToken(appCheck)).to.be.rejectedWith( 'there was an error' ); }); diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index 23b2462a0e5..a821a6d1611 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -35,7 +35,7 @@ import { AppCheckService } from './factory'; import { AppCheckProvider, ListenerType } from './types'; import { getToken as getTokenInternal, - getScopedToken as getScopedTokenInternal, + getLimitedUseToken as getLimitedUseTokenInternal, addTokenListener, removeTokenListener, isValid, @@ -211,19 +211,23 @@ export async function getToken( } /** - * Requests a Firebase App Check token. This method should be used ONLY if you - * need to authorize requests to a non-Firebase backend. Tokens from this - * method are intended for use with the Admin SDKs when `consume` is set to - * true. Tokens from this method do not interact with FirebaseAppCheck’s token - * cache. + * Requests a Firebase App Check token. This method should be used + * *only* if you need to authorize requests to a non-Firebase backend. * + * Returns limited-use tokens that are intended for use with your + * non-Firebase backend endpoints that are protected with + * Replay Protection. This method + * does not affect the token generation behavior of the + * #getAppCheckToken() method. + * * @param appCheckInstance - The App Check service instance. + * @returns The limited use token. * @public */ -export async function getScopedToken( +export async function getLimitedUseToken( appCheckInstance: AppCheck ): Promise { - const result = await getScopedTokenInternal( + const result = await getLimitedUseTokenInternal( appCheckInstance as AppCheckService ); if (result.error) { diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index cee5a6985af..6ced448d382 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -33,7 +33,7 @@ import { removeTokenListener, formatDummyToken, defaultTokenErrorData, - getScopedToken + getLimitedUseToken } from './internal-api'; import * as reCAPTCHA from './recaptcha'; import * as client from './client'; @@ -664,7 +664,7 @@ describe('internal api', () => { }); }); - describe('getScopedToken()', () => { + describe('getLimitedUseToken()', () => { it('uses customTokenProvider to get an AppCheck token', async () => { const customTokenProvider = getFakeCustomTokenProvider(); const customProviderSpy = spy(customTokenProvider, 'getToken'); @@ -672,7 +672,7 @@ describe('internal api', () => { const appCheck = initializeAppCheck(app, { provider: customTokenProvider }); - const token = await getScopedToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck as AppCheckService); expect(customProviderSpy).to.be.called; expect(token).to.deep.equal({ @@ -687,7 +687,7 @@ describe('internal api', () => { const appCheck = initializeAppCheck(app, { provider: customTokenProvider }); - await getScopedToken(appCheck as AppCheckService); + await getLimitedUseToken(appCheck as AppCheckService); expect(getStateReference(app).token).to.be.undefined; expect(getStateReference(app).isTokenAutoRefreshEnabled).to.be.false; @@ -706,7 +706,7 @@ describe('internal api', () => { 'exchangeToken' ).returns(Promise.resolve(fakeRecaptchaAppCheckToken)); - const token = await getScopedToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck as AppCheckService); expect(reCAPTCHASpy).to.be.called; @@ -729,7 +729,7 @@ describe('internal api', () => { 'exchangeToken' ).returns(Promise.resolve(fakeRecaptchaAppCheckToken)); - const token = await getScopedToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck as AppCheckService); expect(reCAPTCHASpy).to.be.called; @@ -752,7 +752,7 @@ describe('internal api', () => { const error = new Error('oops, something went wrong'); stub(client, 'exchangeToken').returns(Promise.reject(error)); - const token = await getScopedToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck as AppCheckService); expect(reCAPTCHASpy).to.be.called; expect(token).to.deep.equal({ @@ -778,7 +778,7 @@ describe('internal api', () => { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) }); - const token = await getScopedToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck as AppCheckService); expect(exchangeTokenStub.args[0][0].body['debug_token']).to.equal( 'my-debug-token' ); @@ -799,7 +799,7 @@ describe('internal api', () => { ) ); - const token = await getScopedToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck as AppCheckService); // ReCaptchaV3Provider's _throttleData is private so checking // the resulting error message to be sure it has roughly the @@ -825,7 +825,7 @@ describe('internal api', () => { ) ); - const token = await getScopedToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck as AppCheckService); // ReCaptchaV3Provider's _throttleData is private so checking // the resulting error message to be sure it has roughly the diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 7a18b1f627e..1ccc00d3420 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -210,7 +210,7 @@ export async function getToken( * The result will contain an error field if there is any error. * In case there is an error, the token field in the result will be populated with a dummy value */ -export async function getScopedToken( +export async function getLimitedUseToken( appCheck: AppCheckService ): Promise { const app = appCheck.app; From 2ecb2236e3cfeda49c9ce3fe1686a2a0d6995bb9 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Thu, 6 Apr 2023 11:33:35 -0700 Subject: [PATCH 03/11] PR feedback --- packages/app-check/src/api.test.ts | 4 +- packages/app-check/src/api.ts | 7 +- packages/app-check/src/internal-api.test.ts | 30 +-- packages/app-check/src/internal-api.ts | 227 ++++++++------------ 4 files changed, 108 insertions(+), 160 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index cda3bb99891..f9aa20d022b 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -295,7 +295,7 @@ describe('api', () => { const appCheck = getFakeAppCheck(app); const internalgetLimitedUseToken = stub( internalApi, - 'getLimitedUseToken' + 'getToken' ).resolves({ token: 'a-token-string' }); @@ -307,7 +307,7 @@ describe('api', () => { const appCheck = getFakeAppCheck(app); // If getLimitedUseToken() errors, it returns a dummy token with an error field // instead of throwing. - stub(internalApi, 'getLimitedUseToken').resolves({ + stub(internalApi, 'getToken').resolves({ token: 'a-dummy-token', error: Error('there was an error') }); diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index a821a6d1611..021da593687 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -35,7 +35,6 @@ import { AppCheckService } from './factory'; import { AppCheckProvider, ListenerType } from './types'; import { getToken as getTokenInternal, - getLimitedUseToken as getLimitedUseTokenInternal, addTokenListener, removeTokenListener, isValid, @@ -227,8 +226,10 @@ export async function getToken( export async function getLimitedUseToken( appCheckInstance: AppCheck ): Promise { - const result = await getLimitedUseTokenInternal( - appCheckInstance as AppCheckService + const result = await getTokenInternal( + appCheckInstance as AppCheckService, + /* forceRefresh */ true, + /* isLimitedUse */ true ); if (result.error) { throw result.error; diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index 6ced448d382..3dc89553575 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -33,7 +33,6 @@ import { removeTokenListener, formatDummyToken, defaultTokenErrorData, - getLimitedUseToken } from './internal-api'; import * as reCAPTCHA from './recaptcha'; import * as client from './client'; @@ -46,11 +45,11 @@ import { setInitialState, getDebugState } from './state'; -import { AppCheckTokenListener } from './public-types'; +import { AppCheck, AppCheckTokenListener } from './public-types'; import { Deferred } from '@firebase/util'; import { ReCaptchaEnterpriseProvider, ReCaptchaV3Provider } from './providers'; import { AppCheckService } from './factory'; -import { ListenerType } from './types'; +import { AppCheckTokenResult, ListenerType } from './types'; import { AppCheckError, ERROR_FACTORY } from './errors'; const fakeRecaptchaToken = 'fake-recaptcha-token'; @@ -664,7 +663,14 @@ describe('internal api', () => { }); }); - describe('getLimitedUseToken()', () => { + describe('getToken() for limited use', () => { + function getLimitedUseToken(appCheck: AppCheck): Promise { + return getToken( + appCheck as AppCheckService, + /*forceRefresh*/ true, + /* isLimitedUse */ true); + } + it('uses customTokenProvider to get an AppCheck token', async () => { const customTokenProvider = getFakeCustomTokenProvider(); const customProviderSpy = spy(customTokenProvider, 'getToken'); @@ -672,7 +678,7 @@ describe('internal api', () => { const appCheck = initializeAppCheck(app, { provider: customTokenProvider }); - const token = await getLimitedUseToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck); expect(customProviderSpy).to.be.called; expect(token).to.deep.equal({ @@ -687,7 +693,7 @@ describe('internal api', () => { const appCheck = initializeAppCheck(app, { provider: customTokenProvider }); - await getLimitedUseToken(appCheck as AppCheckService); + await getLimitedUseToken(appCheck); expect(getStateReference(app).token).to.be.undefined; expect(getStateReference(app).isTokenAutoRefreshEnabled).to.be.false; @@ -706,7 +712,7 @@ describe('internal api', () => { 'exchangeToken' ).returns(Promise.resolve(fakeRecaptchaAppCheckToken)); - const token = await getLimitedUseToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck); expect(reCAPTCHASpy).to.be.called; @@ -729,7 +735,7 @@ describe('internal api', () => { 'exchangeToken' ).returns(Promise.resolve(fakeRecaptchaAppCheckToken)); - const token = await getLimitedUseToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck); expect(reCAPTCHASpy).to.be.called; @@ -752,7 +758,7 @@ describe('internal api', () => { const error = new Error('oops, something went wrong'); stub(client, 'exchangeToken').returns(Promise.reject(error)); - const token = await getLimitedUseToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck); expect(reCAPTCHASpy).to.be.called; expect(token).to.deep.equal({ @@ -778,7 +784,7 @@ describe('internal api', () => { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) }); - const token = await getLimitedUseToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck); expect(exchangeTokenStub.args[0][0].body['debug_token']).to.equal( 'my-debug-token' ); @@ -799,7 +805,7 @@ describe('internal api', () => { ) ); - const token = await getLimitedUseToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck); // ReCaptchaV3Provider's _throttleData is private so checking // the resulting error message to be sure it has roughly the @@ -825,7 +831,7 @@ describe('internal api', () => { ) ); - const token = await getLimitedUseToken(appCheck as AppCheckService); + const token = await getLimitedUseToken(appCheck); // ReCaptchaV3Provider's _throttleData is private so checking // the resulting error message to be sure it has roughly the diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 1ccc00d3420..5f46e67ebce 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -60,54 +60,60 @@ export function formatDummyToken( */ export async function getToken( appCheck: AppCheckService, - forceRefresh = false + forceRefresh = false, + isLimitedUse = false, ): Promise { const app = appCheck.app; ensureActivated(app); const state = getStateReference(app); - /** - * First check if there is a token in memory from a previous `getToken()` call. - */ - let token: AppCheckTokenInternal | undefined = state.token; + let token: AppCheckTokenInternal | undefined = undefined; let error: Error | undefined = undefined; - /** - * If an invalid token was found in memory, clear token from - * memory and unset the local variable `token`. - */ - if (token && !isValid(token)) { - state.token = undefined; - token = undefined; - } + if (!isLimitedUse) { + /** + * First check if there is a token in memory from a previous `getToken()` call. + */ + token = state.token; + + /** + * If an invalid token was found in memory, clear token from + * memory and unset the local variable `token`. + */ + if (token && !isValid(token)) { + 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) { - if (isValid(cachedToken)) { - token = cachedToken; - } else { - // If there was an invalid token in the indexedDB cache, clear it. - await writeTokenToStorage(app, 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) { + if (isValid(cachedToken)) { + token = cachedToken; + } else { + // If there was an invalid token in the indexedDB cache, clear it. + await writeTokenToStorage(app, undefined); + } } } - } - // Return the cached token (from either memory or indexedDB) if it's valid - if (!forceRefresh && token && isValid(token)) { - return { - token: token.token - }; + // Return the cached token (from either memory or indexedDB) if it's valid + if (!forceRefresh && token && isValid(token)) { + return { + token: token.token + }; + } } // Only set to true if this `getToken()` call is making the actual // REST call to the exchange endpoint, versus waiting for an already - // in-flight call (see debug and regular exchange endpoint paths below) + // in-flight call (see debug and regular exchange endpoint paths below). + // This will never be true if isLimitedUseToken is true. let shouldCallListeners = false; /** @@ -116,44 +122,58 @@ export async function getToken( * Check token using the debug token, and return it directly. */ if (isDebugMode()) { - // Avoid making another call to the exchange endpoint if one is in flight. - if (!state.exchangeTokenPromise) { - state.exchangeTokenPromise = exchangeToken( + // Avoid making another call to the exchange endpoint if one is in flight + // (unless requesting a limited use token) + let promise = isLimitedUse ? null : state.exchangeTokenPromise; + if (!promise) { + promise = exchangeToken( getExchangeDebugTokenRequest(app, await getDebugToken()), appCheck.heartbeatServiceProvider - ).finally(() => { - // Clear promise when settled - either resolved or rejected. - state.exchangeTokenPromise = undefined; - }); - shouldCallListeners = true; + ); + + if (!isLimitedUse) { + promise = promise.finally(() => { + // Clear promise when settled - either resolved or rejected. + state.exchangeTokenPromise = undefined; + }); + shouldCallListeners = true; + state.exchangeTokenPromise = promise; + } + } + const tokenFromDebugExchange: AppCheckTokenInternal = await promise; + + if (!isLimitedUse) { + await writeTokenToStorage(app, tokenFromDebugExchange); + // Write debug token to state if this isn't a limited use token. + state.token = tokenFromDebugExchange; } - const tokenFromDebugExchange: AppCheckTokenInternal = - await state.exchangeTokenPromise; - // Write debug token to indexedDB. - await writeTokenToStorage(app, tokenFromDebugExchange); - // Write debug token to state. - state.token = tokenFromDebugExchange; return { token: tokenFromDebugExchange.token }; } /** * There are no valid tokens in memory or indexedDB and we are not in - * debug mode. + * debug mode, or we are requesting a limited use token. * Request a new token from the exchange endpoint. */ try { - // Avoid making another call to the exchange endpoint if one is in flight. - if (!state.exchangeTokenPromise) { + // Avoid making another call to the exchange endpoint if one is in flight + // (unless requesting a limited use token) + let promise = isLimitedUse ? null : state.exchangeTokenPromise; + if (!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; - }); - shouldCallListeners = true; + promise = state.provider!.getToken(); + if (!isLimitedUse) { + promise = promise.finally(() => { + // Clear promise when settled - either resolved or rejected. + state.exchangeTokenPromise = undefined; + }); + state.exchangeTokenPromise = promise; + shouldCallListeners = true; + } } - token = await getStateReference(app).exchangeTokenPromise; + token = await promise; } catch (e) { if ((e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}`) { // Warn if throttled, but do not treat it as an error. @@ -193,97 +213,18 @@ export async function getToken( interopTokenResult = { token: token.token }; - // write the new token to the memory state as well as the persistent storage. - // Only do it if we got a valid new token - state.token = token; - await writeTokenToStorage(app, token); - } - - if (shouldCallListeners) { - notifyTokenListeners(app, interopTokenResult); - } - return interopTokenResult; -} - -/** - * This function always resolves. - * The result will contain an error field if there is any error. - * In case there is an error, the token field in the result will be populated with a dummy value - */ -export async function getLimitedUseToken( - appCheck: AppCheckService -): Promise { - const app = appCheck.app; - ensureActivated(app); - - const { provider } = getStateReference(app); - - /** - * First check if there is a token in memory from a previous `getToken()` call. - */ - let token: AppCheckTokenInternal | undefined = undefined; - let error: Error | undefined = undefined; - - /** - * DEBUG MODE - * If debug mode is set, and there is no cached token, fetch a new App - * Check token using the debug token, and return it directly. - */ - if (isDebugMode()) { - const debugToken = await getDebugToken(); - const tokenFromDebugExchange: AppCheckTokenInternal = await exchangeToken( - getExchangeDebugTokenRequest(app, debugToken), - appCheck.heartbeatServiceProvider - ); - return { token: tokenFromDebugExchange.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 { - token = await provider!.getToken(); - } catch (e) { - if ((e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}`) { - // Warn if throttled, but do not treat it as an error. - logger.warn((e as FirebaseError).message); - } else { - // `getToken()` should never throw, but logging error text to console will aid debugging. - logger.error(e); + if (!isLimitedUse) { + // write the new token to the memory state as well as the persistent storage. + // Only do it if we got a valid new token and this isn't a limited use token. + state.token = token; + await writeTokenToStorage(app, token); } - // Always save error to be added to dummy token. - error = e as FirebaseError; } - let interopTokenResult: AppCheckTokenResult | undefined; - if (!token) { - // If token is undefined, there must be an error. - // Return a dummy token along with the error. - interopTokenResult = makeDummyTokenResult(error!); - } 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 - // 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 - }; + if (shouldCallListeners && !isLimitedUse) { + // If we're here, isLimitedUse should always be false -- but put it in + // the 'if' clause just in case. + notifyTokenListeners(app, interopTokenResult); } return interopTokenResult; } From cf5c5683a0820de2b2c94261f73ad1e6be830d53 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 6 Apr 2023 11:34:31 -0700 Subject: [PATCH 04/11] Changeset --- .changeset/wicked-tomatoes-smoke.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/wicked-tomatoes-smoke.md diff --git a/.changeset/wicked-tomatoes-smoke.md b/.changeset/wicked-tomatoes-smoke.md new file mode 100644 index 00000000000..37510cbdaa1 --- /dev/null +++ b/.changeset/wicked-tomatoes-smoke.md @@ -0,0 +1,6 @@ +--- +"@firebase/app-check": minor +"@firebase/app": minor +--- + +Add new limited use token method to App Check From 2adf7c9af42ebf01f0e7e77d81ebcc7824e4524c Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Wed, 12 Apr 2023 14:20:30 -0700 Subject: [PATCH 05/11] PR feedback, docs --- .changeset/wicked-tomatoes-smoke.md | 2 +- common/api-review/app-check.api.md | 2 +- docs-devsite/app-check.md | 25 +++++++++++++++++++++++++ packages/app-check/src/api.test.ts | 4 ++-- packages/app-check/src/api.ts | 3 ++- 5 files changed, 31 insertions(+), 5 deletions(-) diff --git a/.changeset/wicked-tomatoes-smoke.md b/.changeset/wicked-tomatoes-smoke.md index 37510cbdaa1..7da13ffc526 100644 --- a/.changeset/wicked-tomatoes-smoke.md +++ b/.changeset/wicked-tomatoes-smoke.md @@ -1,6 +1,6 @@ --- "@firebase/app-check": minor -"@firebase/app": minor +"firebase": minor --- Add new limited use token method to App Check diff --git a/common/api-review/app-check.api.md b/common/api-review/app-check.api.md index a7917973db2..65cfb36aa90 100644 --- a/common/api-review/app-check.api.md +++ b/common/api-review/app-check.api.md @@ -61,7 +61,7 @@ export interface CustomProviderOptions { } // @public -export function getScopedToken(appCheckInstance: AppCheck): Promise; +export function getLimitedUseToken(appCheckInstance: AppCheck): Promise; // @public export function getToken(appCheckInstance: AppCheck, forceRefresh?: boolean): Promise; diff --git a/docs-devsite/app-check.md b/docs-devsite/app-check.md index b7895a05c96..0ed1dd178a6 100644 --- a/docs-devsite/app-check.md +++ b/docs-devsite/app-check.md @@ -19,6 +19,7 @@ Firebase App Check | function(app...) | | [initializeAppCheck(app, options)](./app-check.md#initializeappcheck) | Activate App Check for the given app. Can be called only once per app. | | function(appCheckInstance...) | +| [getLimitedUseToken(appCheckInstance)](./app-check.md#getlimitedusetoken) | Requests a Firebase App Check token. This method should be used \*only\* if you need to authorize requests to a non-Firebase backend.Returns limited-use tokens that are intended for use with your non-Firebase backend endpoints that are protected with Replay Protection. This method does not affect the token generation behavior of the \#getAppCheckToken() method. | | [getToken(appCheckInstance, forceRefresh)](./app-check.md#gettoken) | Get the current App Check token. Attaches to the most recent in-flight request if one is present. Returns null if no token is present and no token requests are in-flight. | | [onTokenChanged(appCheckInstance, observer)](./app-check.md#ontokenchanged) | Registers a listener to changes in the token state. There can be more than one listener registered at the same time for one or more App Check instances. The listeners call back on the UI thread whenever the current token associated with this App Check instance changes. | | [onTokenChanged(appCheckInstance, onNext, onError, onCompletion)](./app-check.md#ontokenchanged) | Registers a listener to changes in the token state. There can be more than one listener registered at the same time for one or more App Check instances. The listeners call back on the UI thread whenever the current token associated with this App Check instance changes. | @@ -69,6 +70,30 @@ export declare function initializeAppCheck(app: FirebaseApp | undefined, options [AppCheck](./app-check.appcheck.md#appcheck_interface) +## getLimitedUseToken() + +Requests a Firebase App Check token. This method should be used \*only\* if you need to authorize requests to a non-Firebase backend. + +Returns limited-use tokens that are intended for use with your non-Firebase backend endpoints that are protected with Replay Protection. This method does not affect the token generation behavior of the \#getAppCheckToken() method. + +Signature: + +```typescript +export declare function getLimitedUseToken(appCheckInstance: AppCheck): Promise; +``` + +### Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| appCheckInstance | [AppCheck](./app-check.appcheck.md#appcheck_interface) | The App Check service instance. | + +Returns: + +Promise<[AppCheckTokenResult](./app-check.appchecktokenresult.md#appchecktokenresult_interface)> + +The limited use token. + ## getToken() Get the current App Check token. Attaches to the most recent in-flight request if one is present. Returns null if no token is present and no token requests are in-flight. diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index f9aa20d022b..1ae20f3427d 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -305,8 +305,8 @@ describe('api', () => { it('getLimitedUseToken() throws errors returned with token', async () => { const app = getFakeApp({ automaticDataCollectionEnabled: true }); const appCheck = getFakeAppCheck(app); - // If getLimitedUseToken() errors, it returns a dummy token with an error field - // instead of throwing. + // If the internal getToken() errors, it returns a dummy token + // with an error field instead of throwing. stub(internalApi, 'getToken').resolves({ token: 'a-dummy-token', error: Error('there was an error') diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index 021da593687..acf7eaa0ccf 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -215,7 +215,8 @@ export async function getToken( * * Returns limited-use tokens that are intended for use with your * non-Firebase backend endpoints that are protected with - * Replay Protection. This method + * + * Replay Protection. This method * does not affect the token generation behavior of the * #getAppCheckToken() method. * From 5c0cd1271e4472d49dbb5ebc1da727c1353c1584 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Wed, 12 Apr 2023 14:22:36 -0700 Subject: [PATCH 06/11] Cleanup --- docs-devsite/app-check.md | 2 +- packages/app-check/src/api.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs-devsite/app-check.md b/docs-devsite/app-check.md index 0ed1dd178a6..725bd5c6075 100644 --- a/docs-devsite/app-check.md +++ b/docs-devsite/app-check.md @@ -19,7 +19,7 @@ Firebase App Check | function(app...) | | [initializeAppCheck(app, options)](./app-check.md#initializeappcheck) | Activate App Check for the given app. Can be called only once per app. | | function(appCheckInstance...) | -| [getLimitedUseToken(appCheckInstance)](./app-check.md#getlimitedusetoken) | Requests a Firebase App Check token. This method should be used \*only\* if you need to authorize requests to a non-Firebase backend.Returns limited-use tokens that are intended for use with your non-Firebase backend endpoints that are protected with Replay Protection. This method does not affect the token generation behavior of the \#getAppCheckToken() method. | +| [getLimitedUseToken(appCheckInstance)](./app-check.md#getlimitedusetoken) | Requests a Firebase App Check token. This method should be used only if you need to authorize requests to a non-Firebase backend.Returns limited-use tokens that are intended for use with your non-Firebase backend endpoints that are protected with Replay Protection. This method does not affect the token generation behavior of the \#getAppCheckToken() method. | | [getToken(appCheckInstance, forceRefresh)](./app-check.md#gettoken) | Get the current App Check token. Attaches to the most recent in-flight request if one is present. Returns null if no token is present and no token requests are in-flight. | | [onTokenChanged(appCheckInstance, observer)](./app-check.md#ontokenchanged) | Registers a listener to changes in the token state. There can be more than one listener registered at the same time for one or more App Check instances. The listeners call back on the UI thread whenever the current token associated with this App Check instance changes. | | [onTokenChanged(appCheckInstance, onNext, onError, onCompletion)](./app-check.md#ontokenchanged) | Registers a listener to changes in the token state. There can be more than one listener registered at the same time for one or more App Check instances. The listeners call back on the UI thread whenever the current token associated with this App Check instance changes. | diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index acf7eaa0ccf..19ffa8c356e 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -211,7 +211,7 @@ export async function getToken( /** * Requests a Firebase App Check token. This method should be used - * *only* if you need to authorize requests to a non-Firebase backend. + * only if you need to authorize requests to a non-Firebase backend. * * Returns limited-use tokens that are intended for use with your * non-Firebase backend endpoints that are protected with From 9b57283700049af5ea4d909efd3f882e070eab94 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Wed, 12 Apr 2023 14:34:20 -0700 Subject: [PATCH 07/11] Formatting --- packages/app-check/src/api.test.ts | 11 +++++------ packages/app-check/src/api.ts | 2 +- packages/app-check/src/internal-api.test.ts | 9 ++++++--- packages/app-check/src/internal-api.ts | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index 1ae20f3427d..e0881dbca26 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -293,12 +293,11 @@ describe('api', () => { it('getLimitedUseToken() calls the internal getLimitedUseToken() function', async () => { const app = getFakeApp({ automaticDataCollectionEnabled: true }); const appCheck = getFakeAppCheck(app); - const internalgetLimitedUseToken = stub( - internalApi, - 'getToken' - ).resolves({ - token: 'a-token-string' - }); + const internalgetLimitedUseToken = stub(internalApi, 'getToken').resolves( + { + token: 'a-token-string' + } + ); await getLimitedUseToken(appCheck); expect(internalgetLimitedUseToken).to.be.calledWith(appCheck); }); diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index 19ffa8c356e..96c95b047ec 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -219,7 +219,7 @@ export async function getToken( * Replay Protection. This method * does not affect the token generation behavior of the * #getAppCheckToken() method. - * + * * @param appCheckInstance - The App Check service instance. * @returns The limited use token. * @public diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index 3dc89553575..4c23176be13 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -32,7 +32,7 @@ import { addTokenListener, removeTokenListener, formatDummyToken, - defaultTokenErrorData, + defaultTokenErrorData } from './internal-api'; import * as reCAPTCHA from './recaptcha'; import * as client from './client'; @@ -664,11 +664,14 @@ describe('internal api', () => { }); describe('getToken() for limited use', () => { - function getLimitedUseToken(appCheck: AppCheck): Promise { + function getLimitedUseToken( + appCheck: AppCheck + ): Promise { return getToken( appCheck as AppCheckService, /*forceRefresh*/ true, - /* isLimitedUse */ true); + /* isLimitedUse */ true + ); } it('uses customTokenProvider to get an AppCheck token', async () => { diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 5f46e67ebce..28c43ba67a9 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -61,7 +61,7 @@ export function formatDummyToken( export async function getToken( appCheck: AppCheckService, forceRefresh = false, - isLimitedUse = false, + isLimitedUse = false ): Promise { const app = appCheck.app; ensureActivated(app); From 7a553782406da1ced7afab23564a4b04a0e770e5 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Wed, 12 Apr 2023 14:47:37 -0700 Subject: [PATCH 08/11] sinon.restore --- packages/app-check/src/internal-api.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index 4c23176be13..a943b27ce87 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -18,6 +18,7 @@ import '../test/setup'; import { expect } from 'chai'; import { SinonStub, spy, stub, useFakeTimers } from 'sinon'; +import * as sinon from 'sinon'; import { deleteApp, FirebaseApp } from '@firebase/app'; import { FAKE_SITE_KEY, @@ -90,6 +91,7 @@ describe('internal api', () => { clearState(); removegreCAPTCHAScriptsOnPage(); return deleteApp(app); + sinon.restore(); }); // TODO: test error conditions describe('getToken()', () => { From bb54aab94f98319eab566b2050c35dc8298b2017 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Tue, 18 Apr 2023 09:19:59 -0700 Subject: [PATCH 09/11] PR feedback --- packages/app-check/src/api.test.ts | 28 ++-- packages/app-check/src/api.ts | 13 +- packages/app-check/src/internal-api.test.ts | 113 ++----------- packages/app-check/src/internal-api.ts | 170 ++++++++++---------- 4 files changed, 109 insertions(+), 215 deletions(-) diff --git a/packages/app-check/src/api.test.ts b/packages/app-check/src/api.test.ts index e0881dbca26..c01cc5f4029 100644 --- a/packages/app-check/src/api.test.ts +++ b/packages/app-check/src/api.test.ts @@ -293,26 +293,16 @@ describe('api', () => { it('getLimitedUseToken() calls the internal getLimitedUseToken() function', async () => { const app = getFakeApp({ automaticDataCollectionEnabled: true }); const appCheck = getFakeAppCheck(app); - const internalgetLimitedUseToken = stub(internalApi, 'getToken').resolves( - { - token: 'a-token-string' - } - ); - await getLimitedUseToken(appCheck); - expect(internalgetLimitedUseToken).to.be.calledWith(appCheck); - }); - it('getLimitedUseToken() throws errors returned with token', async () => { - const app = getFakeApp({ automaticDataCollectionEnabled: true }); - const appCheck = getFakeAppCheck(app); - // If the internal getToken() errors, it returns a dummy token - // with an error field instead of throwing. - stub(internalApi, 'getToken').resolves({ - token: 'a-dummy-token', - error: Error('there was an error') + const internalgetLimitedUseToken = stub( + internalApi, + 'getLimitedUseToken' + ).resolves({ + token: 'a-token-string' }); - await expect(getLimitedUseToken(appCheck)).to.be.rejectedWith( - 'there was an error' - ); + expect(await getLimitedUseToken(appCheck)).to.eql({ + token: 'a-token-string' + }); + expect(internalgetLimitedUseToken).to.be.calledWith(appCheck); }); }); describe('onTokenChanged()', () => { diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts index 96c95b047ec..f738b21fce2 100644 --- a/packages/app-check/src/api.ts +++ b/packages/app-check/src/api.ts @@ -35,6 +35,7 @@ import { AppCheckService } from './factory'; import { AppCheckProvider, ListenerType } from './types'; import { getToken as getTokenInternal, + getLimitedUseToken as getLimitedUseTokenInternal, addTokenListener, removeTokenListener, isValid, @@ -224,18 +225,10 @@ export async function getToken( * @returns The limited use token. * @public */ -export async function getLimitedUseToken( +export function getLimitedUseToken( appCheckInstance: AppCheck ): Promise { - const result = await getTokenInternal( - appCheckInstance as AppCheckService, - /* forceRefresh */ true, - /* isLimitedUse */ true - ); - if (result.error) { - throw result.error; - } - return { token: result.token }; + return getLimitedUseTokenInternal(appCheckInstance as AppCheckService); } /** diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index a943b27ce87..f99eeff430d 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -33,7 +33,8 @@ import { addTokenListener, removeTokenListener, formatDummyToken, - defaultTokenErrorData + defaultTokenErrorData, + getLimitedUseToken } from './internal-api'; import * as reCAPTCHA from './recaptcha'; import * as client from './client'; @@ -46,11 +47,11 @@ import { setInitialState, getDebugState } from './state'; -import { AppCheck, AppCheckTokenListener } from './public-types'; +import { AppCheckTokenListener } from './public-types'; import { Deferred } from '@firebase/util'; import { ReCaptchaEnterpriseProvider, ReCaptchaV3Provider } from './providers'; import { AppCheckService } from './factory'; -import { AppCheckTokenResult, ListenerType } from './types'; +import { ListenerType } from './types'; import { AppCheckError, ERROR_FACTORY } from './errors'; const fakeRecaptchaToken = 'fake-recaptcha-token'; @@ -666,16 +667,6 @@ describe('internal api', () => { }); describe('getToken() for limited use', () => { - function getLimitedUseToken( - appCheck: AppCheck - ): Promise { - return getToken( - appCheck as AppCheckService, - /*forceRefresh*/ true, - /* isLimitedUse */ true - ); - } - it('uses customTokenProvider to get an AppCheck token', async () => { const customTokenProvider = getFakeCustomTokenProvider(); const customProviderSpy = spy(customTokenProvider, 'getToken'); @@ -683,7 +674,7 @@ describe('internal api', () => { const appCheck = initializeAppCheck(app, { provider: customTokenProvider }); - const token = await getLimitedUseToken(appCheck); + const token = await getLimitedUseToken(appCheck as AppCheckService); expect(customProviderSpy).to.be.called; expect(token).to.deep.equal({ @@ -698,7 +689,7 @@ describe('internal api', () => { const appCheck = initializeAppCheck(app, { provider: customTokenProvider }); - await getLimitedUseToken(appCheck); + await getLimitedUseToken(appCheck as AppCheckService); expect(getStateReference(app).token).to.be.undefined; expect(getStateReference(app).isTokenAutoRefreshEnabled).to.be.false; @@ -709,15 +700,13 @@ describe('internal api', () => { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) }); - const reCAPTCHASpy = stub(reCAPTCHA, 'getToken').returns( - Promise.resolve(fakeRecaptchaToken) - ); + const reCAPTCHASpy = stubGetRecaptchaToken(); const exchangeTokenStub: SinonStub = stub( client, 'exchangeToken' ).returns(Promise.resolve(fakeRecaptchaAppCheckToken)); - const token = await getLimitedUseToken(appCheck); + const token = await getLimitedUseToken(appCheck as AppCheckService); expect(reCAPTCHASpy).to.be.called; @@ -732,15 +721,13 @@ describe('internal api', () => { provider: new ReCaptchaEnterpriseProvider(FAKE_SITE_KEY) }); - const reCAPTCHASpy = stub(reCAPTCHA, 'getToken').returns( - Promise.resolve(fakeRecaptchaToken) - ); + const reCAPTCHASpy = stubGetRecaptchaToken(); const exchangeTokenStub: SinonStub = stub( client, 'exchangeToken' ).returns(Promise.resolve(fakeRecaptchaAppCheckToken)); - const token = await getLimitedUseToken(appCheck); + const token = await getLimitedUseToken(appCheck as AppCheckService); expect(reCAPTCHASpy).to.be.called; @@ -750,32 +737,6 @@ describe('internal api', () => { expect(token).to.deep.equal({ token: fakeRecaptchaAppCheckToken.token }); }); - it('resolves with a dummy token and an error if failed to get a token', async () => { - const errorStub = stub(console, 'error'); - const appCheck = initializeAppCheck(app, { - provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) - }); - - const reCAPTCHASpy = stub(reCAPTCHA, 'getToken').returns( - Promise.resolve(fakeRecaptchaToken) - ); - - const error = new Error('oops, something went wrong'); - stub(client, 'exchangeToken').returns(Promise.reject(error)); - - const token = await getLimitedUseToken(appCheck); - - expect(reCAPTCHASpy).to.be.called; - expect(token).to.deep.equal({ - token: formatDummyToken(defaultTokenErrorData), - error - }); - expect(errorStub.args[0][1].message).to.include( - 'oops, something went wrong' - ); - errorStub.restore(); - }); - it('exchanges debug token if in debug mode', async () => { const exchangeTokenStub: SinonStub = stub( client, @@ -789,64 +750,12 @@ describe('internal api', () => { provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) }); - const token = await getLimitedUseToken(appCheck); + const token = await getLimitedUseToken(appCheck as AppCheckService); expect(exchangeTokenStub.args[0][0].body['debug_token']).to.equal( 'my-debug-token' ); expect(token).to.deep.equal({ token: fakeRecaptchaAppCheckToken.token }); }); - - it('throttles for a period less than 1d on 503', async () => { - // More detailed check of exponential backoff in providers.test.ts - const appCheck = initializeAppCheck(app, { - provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) - }); - const warnStub = stub(logger, 'warn'); - stub(client, 'exchangeToken').returns( - Promise.reject( - ERROR_FACTORY.create(AppCheckError.FETCH_STATUS_ERROR, { - httpStatus: 503 - }) - ) - ); - - const token = await getLimitedUseToken(appCheck); - - // ReCaptchaV3Provider's _throttleData is private so checking - // the resulting error message to be sure it has roughly the - // correct throttle time. This also tests the time formatter. - // Check both the error itself and that it makes it through to - // console.warn - expect(token.error?.message).to.include('503'); - expect(token.error?.message).to.include('00m'); - expect(token.error?.message).to.not.include('1d'); - expect(warnStub.args[0][0]).to.include('503'); - }); - - it('throttles 1d on 403', async () => { - const appCheck = initializeAppCheck(app, { - provider: new ReCaptchaV3Provider(FAKE_SITE_KEY) - }); - const warnStub = stub(logger, 'warn'); - stub(client, 'exchangeToken').returns( - Promise.reject( - ERROR_FACTORY.create(AppCheckError.FETCH_STATUS_ERROR, { - httpStatus: 403 - }) - ) - ); - - const token = await getLimitedUseToken(appCheck); - - // ReCaptchaV3Provider's _throttleData is private so checking - // the resulting error message to be sure it has roughly the - // correct throttle time. This also tests the time formatter. - // Check both the error itself and that it makes it through to - // console.warn - expect(token.error?.message).to.include('403'); - expect(token.error?.message).to.include('1d'); - expect(warnStub.args[0][0]).to.include('403'); - }); }); describe('addTokenListener', () => { diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts index 28c43ba67a9..728f2ca5e68 100644 --- a/packages/app-check/src/internal-api.ts +++ b/packages/app-check/src/internal-api.ts @@ -60,60 +60,54 @@ export function formatDummyToken( */ export async function getToken( appCheck: AppCheckService, - forceRefresh = false, - isLimitedUse = false + forceRefresh = false ): Promise { const app = appCheck.app; ensureActivated(app); const state = getStateReference(app); - let token: AppCheckTokenInternal | undefined = undefined; + /** + * First check if there is a token in memory from a previous `getToken()` call. + */ + let token: AppCheckTokenInternal | undefined = state.token; let error: Error | undefined = undefined; - if (!isLimitedUse) { - /** - * First check if there is a token in memory from a previous `getToken()` call. - */ - token = state.token; - - /** - * If an invalid token was found in memory, clear token from - * memory and unset the local variable `token`. - */ - if (token && !isValid(token)) { - state.token = undefined; - token = undefined; - } + /** + * If an invalid token was found in memory, clear token from + * memory and unset the local variable `token`. + */ + if (token && !isValid(token)) { + 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) { - if (isValid(cachedToken)) { - token = cachedToken; - } else { - // If there was an invalid token in the indexedDB cache, clear it. - await writeTokenToStorage(app, 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) { + if (isValid(cachedToken)) { + token = cachedToken; + } else { + // If there was an invalid token in the indexedDB cache, clear it. + await writeTokenToStorage(app, undefined); } } + } - // Return the cached token (from either memory or indexedDB) if it's valid - if (!forceRefresh && token && isValid(token)) { - return { - token: token.token - }; - } + // Return the cached token (from either memory or indexedDB) if it's valid + if (!forceRefresh && token && isValid(token)) { + return { + token: token.token + }; } // Only set to true if this `getToken()` call is making the actual // REST call to the exchange endpoint, versus waiting for an already - // in-flight call (see debug and regular exchange endpoint paths below). - // This will never be true if isLimitedUseToken is true. + // in-flight call (see debug and regular exchange endpoint paths below) let shouldCallListeners = false; /** @@ -122,58 +116,44 @@ export async function getToken( * Check token using the debug token, and return it directly. */ if (isDebugMode()) { - // Avoid making another call to the exchange endpoint if one is in flight - // (unless requesting a limited use token) - let promise = isLimitedUse ? null : state.exchangeTokenPromise; - if (!promise) { - promise = exchangeToken( + // Avoid making another call to the exchange endpoint if one is in flight. + if (!state.exchangeTokenPromise) { + state.exchangeTokenPromise = exchangeToken( getExchangeDebugTokenRequest(app, await getDebugToken()), appCheck.heartbeatServiceProvider - ); - - if (!isLimitedUse) { - promise = promise.finally(() => { - // Clear promise when settled - either resolved or rejected. - state.exchangeTokenPromise = undefined; - }); - shouldCallListeners = true; - state.exchangeTokenPromise = promise; - } - } - const tokenFromDebugExchange: AppCheckTokenInternal = await promise; - - if (!isLimitedUse) { - await writeTokenToStorage(app, tokenFromDebugExchange); - // Write debug token to state if this isn't a limited use token. - state.token = tokenFromDebugExchange; + ).finally(() => { + // Clear promise when settled - either resolved or rejected. + state.exchangeTokenPromise = undefined; + }); + shouldCallListeners = true; } + const tokenFromDebugExchange: AppCheckTokenInternal = + await state.exchangeTokenPromise; + // Write debug token to indexedDB. + await writeTokenToStorage(app, tokenFromDebugExchange); + // Write debug token to state. + state.token = tokenFromDebugExchange; return { token: tokenFromDebugExchange.token }; } /** * There are no valid tokens in memory or indexedDB and we are not in - * debug mode, or we are requesting a limited use token. + * debug mode. * Request a new token from the exchange endpoint. */ try { - // Avoid making another call to the exchange endpoint if one is in flight - // (unless requesting a limited use token) - let promise = isLimitedUse ? null : state.exchangeTokenPromise; - if (!promise) { + // Avoid making another call to the exchange endpoint if one is in flight. + if (!state.exchangeTokenPromise) { // state.provider is populated in initializeAppCheck() // ensureActivated() at the top of this function checks that // initializeAppCheck() has been called. - promise = state.provider!.getToken(); - if (!isLimitedUse) { - promise = promise.finally(() => { - // Clear promise when settled - either resolved or rejected. - state.exchangeTokenPromise = undefined; - }); - state.exchangeTokenPromise = promise; - shouldCallListeners = true; - } + state.exchangeTokenPromise = state.provider!.getToken().finally(() => { + // Clear promise when settled - either resolved or rejected. + state.exchangeTokenPromise = undefined; + }); + shouldCallListeners = true; } - token = await promise; + token = await getStateReference(app).exchangeTokenPromise; } catch (e) { if ((e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}`) { // Warn if throttled, but do not treat it as an error. @@ -213,22 +193,44 @@ export async function getToken( interopTokenResult = { token: token.token }; - if (!isLimitedUse) { - // write the new token to the memory state as well as the persistent storage. - // Only do it if we got a valid new token and this isn't a limited use token. - state.token = token; - await writeTokenToStorage(app, token); - } + // write the new token to the memory state as well as the persistent storage. + // Only do it if we got a valid new token + state.token = token; + await writeTokenToStorage(app, token); } - if (shouldCallListeners && !isLimitedUse) { - // If we're here, isLimitedUse should always be false -- but put it in - // the 'if' clause just in case. + if (shouldCallListeners) { notifyTokenListeners(app, interopTokenResult); } return interopTokenResult; } +/** + * Internal API for limited use tokens. Skips all FAC state and simply calls + * the underlying provider. + */ +export async function getLimitedUseToken( + appCheck: AppCheckService +): Promise { + const app = appCheck.app; + ensureActivated(app); + + const { provider } = getStateReference(app); + + if (isDebugMode()) { + const debugToken = await getDebugToken(); + const { token } = await exchangeToken( + getExchangeDebugTokenRequest(app, debugToken), + appCheck.heartbeatServiceProvider + ); + return { token }; + } else { + // provider is definitely valid since we ensure AppCheck was activated + const { token } = await provider!.getToken(); + return { token }; + } +} + export function addTokenListener( appCheck: AppCheckService, type: ListenerType, From 3bb9629f7dae7864b3e5a054655235209eb497f8 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Tue, 18 Apr 2023 09:46:48 -0700 Subject: [PATCH 10/11] Devsite --- docs-devsite/app-check.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs-devsite/app-check.md b/docs-devsite/app-check.md index 725bd5c6075..da1b06b12fa 100644 --- a/docs-devsite/app-check.md +++ b/docs-devsite/app-check.md @@ -72,7 +72,7 @@ export declare function initializeAppCheck(app: FirebaseApp | undefined, options ## getLimitedUseToken() -Requests a Firebase App Check token. This method should be used \*only\* if you need to authorize requests to a non-Firebase backend. +Requests a Firebase App Check token. This method should be used only if you need to authorize requests to a non-Firebase backend. Returns limited-use tokens that are intended for use with your non-Firebase backend endpoints that are protected with Replay Protection. This method does not affect the token generation behavior of the \#getAppCheckToken() method. From 38efaa65229eb1530dcbaa485f640a8d782221b0 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Tue, 18 Apr 2023 12:33:51 -0700 Subject: [PATCH 11/11] PR feedback --- packages/app-check/src/internal-api.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts index f99eeff430d..360ec3a026a 100644 --- a/packages/app-check/src/internal-api.test.ts +++ b/packages/app-check/src/internal-api.test.ts @@ -18,7 +18,6 @@ import '../test/setup'; import { expect } from 'chai'; import { SinonStub, spy, stub, useFakeTimers } from 'sinon'; -import * as sinon from 'sinon'; import { deleteApp, FirebaseApp } from '@firebase/app'; import { FAKE_SITE_KEY, @@ -92,7 +91,6 @@ describe('internal api', () => { clearState(); removegreCAPTCHAScriptsOnPage(); return deleteApp(app); - sinon.restore(); }); // TODO: test error conditions describe('getToken()', () => { @@ -666,7 +664,7 @@ describe('internal api', () => { }); }); - describe('getToken() for limited use', () => { + describe('getLimitedUseToken()', () => { it('uses customTokenProvider to get an AppCheck token', async () => { const customTokenProvider = getFakeCustomTokenProvider(); const customProviderSpy = spy(customTokenProvider, 'getToken');