Skip to content

Add debug token caching to app-check-exp #5110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 13, 2021
Merged

Add debug token caching to app-check-exp #5110

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a97b231
Add debug token caching to app-check-exp
hsubox76 Jul 2, 2021
9187e31
Address PR comments
hsubox76 Jul 9, 2021
b1e2a67
Update packages-exp/app-check-exp/src/internal-api.ts
hsubox76 Jul 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages-exp/app-check-exp/src/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ describe('client', () => {
});

it('returns a AppCheck token', async () => {
useFakeTimers();
// To get a consistent expireTime/issuedAtTime.
const clock = useFakeTimers();
fetchStub.returns(
Promise.resolve({
status: 200,
Expand All @@ -77,6 +78,7 @@ describe('client', () => {
expireTimeMillis: 3600,
issuedAtTimeMillis: 0
});
clock.restore();
});

it('throws when there is a network error', async () => {
Expand Down
5 changes: 5 additions & 0 deletions packages-exp/app-check-exp/src/factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
removeTokenListener
} from './internal-api';
import { Provider } from '@firebase/component';
import { getState } from './state';

/**
* AppCheck Service class.
Expand All @@ -34,6 +35,10 @@ export class AppCheckService implements AppCheck, _FirebaseService {
public platformLoggerProvider: Provider<'platform-logger'>
) {}
_delete(): Promise<void> {
const { tokenObservers } = getState(this.app);
for (const tokenObserver of tokenObservers) {
removeTokenListener(this.app, tokenObserver.next);
}
return Promise.resolve();
}
}
Expand Down
81 changes: 17 additions & 64 deletions packages-exp/app-check-exp/src/internal-api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,6 @@ describe('internal api', () => {
fakeRecaptchaToken
);
expect(token).to.deep.equal({ token: fakeRecaptchaAppCheckToken.token });
// TODO: Permanently fix.
// Small delay to prevent common test flakiness where this test runs
// into afterEach() sometimes
await new Promise(resolve => setTimeout(resolve, 50));
});

it('resolves with a dummy token and an error if failed to get a token', async () => {
Expand Down Expand Up @@ -148,7 +144,7 @@ describe('internal api', () => {
it('notifies listeners using cached token', async () => {
const appCheck = initializeAppCheck(app, {
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
isTokenAutoRefreshEnabled: true
isTokenAutoRefreshEnabled: false
});

const clock = useFakeTimers();
Expand Down Expand Up @@ -216,6 +212,7 @@ describe('internal api', () => {
});

it('calls 3P error handler if there is an error getting a token', async () => {
stub(console, 'error');
const appCheck = initializeAppCheck(app, {
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
isTokenAutoRefreshEnabled: true
Expand All @@ -239,6 +236,7 @@ describe('internal api', () => {
});

it('ignores listeners that throw', async () => {
stub(console, 'error');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it to just suppress error messages during testing? If so, can you please add comments to clarify?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, done.

const appCheck = initializeAppCheck(app, {
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
isTokenAutoRefreshEnabled: true
Expand All @@ -247,9 +245,7 @@ describe('internal api', () => {
stub(client, 'exchangeToken').returns(
Promise.resolve(fakeRecaptchaAppCheckToken)
);
const listener1 = (): void => {
throw new Error();
};
const listener1 = stub().throws(new Error());
const listener2 = spy();

addTokenListener(
Expand All @@ -265,6 +261,9 @@ describe('internal api', () => {

await getToken(appCheck as AppCheckService);

expect(listener1).to.be.calledWith({
token: fakeRecaptchaAppCheckToken.token
});
expect(listener2).to.be.calledWith({
token: fakeRecaptchaAppCheckToken.token
});
Expand Down Expand Up @@ -344,7 +343,7 @@ describe('internal api', () => {
});
});

it('exchanges debug token if in debug mode', async () => {
it('exchanges debug token if in debug mode and there is no cached token', async () => {
const exchangeTokenStub: SinonStub = stub(
client,
'exchangeToken'
Expand Down Expand Up @@ -393,15 +392,10 @@ describe('internal api', () => {
expect(getState(app).tokenRefresher?.isRunning()).to.be.true;
});

it('notifies the listener with the valid token in memory immediately', done => {
it('notifies the listener with the valid token in memory immediately', async () => {
const clock = useFakeTimers();
const fakeListener: AppCheckTokenListener = token => {
expect(token).to.deep.equal({
token: `fake-memory-app-check-token`
});
clock.restore();
done();
};

const listener = stub();

setState(app, {
...getState(app),
Expand All @@ -415,12 +409,16 @@ describe('internal api', () => {
addTokenListener(
{ app } as AppCheckService,
ListenerType.INTERNAL,
fakeListener
listener
);
await clock.runAllAsync();
expect(listener).to.be.calledWith({
token: 'fake-memory-app-check-token'
});
clock.restore();
});

it('notifies the listener with the valid token in storage', done => {
const clock = useFakeTimers();
const appCheck = initializeAppCheck(app, {
provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
isTokenAutoRefreshEnabled: true
Expand All @@ -437,60 +435,15 @@ describe('internal api', () => {
expect(token).to.deep.equal({
token: `fake-cached-app-check-token`
});
clock.restore();
done();
};

addTokenListener(
appCheck as AppCheckService,
ListenerType.INTERNAL,
fakeListener
);

clock.tick(1);
});

it('notifies the listener with the debug token immediately', done => {
const fakeListener: AppCheckTokenListener = token => {
expect(token).to.deep.equal({
token: `my-debug-token`
});
done();
};

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)
});
addTokenListener(
appCheck as AppCheckService,
ListenerType.INTERNAL,
fakeListener
);
});

it('does NOT start token refresher in debug mode', () => {
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)
});
addTokenListener(
appCheck as AppCheckService,
ListenerType.INTERNAL,
() => {}
);

const state = getState(app);
expect(state.tokenRefresher).is.undefined;
});
});

describe('removeTokenListener', () => {
Expand Down
100 changes: 45 additions & 55 deletions packages-exp/app-check-exp/src/internal-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
ListenerType
} from './types';
import { AppCheckTokenListener } from './public-types';
import { getDebugState, getState, setState } from './state';
import { getState, setState } from './state';
import { TOKEN_REFRESH_TIME } from './constants';
import { Refresher } from './proactive-refresh';
import { ensureActivated } from './util';
Expand Down Expand Up @@ -63,25 +63,17 @@ export async function getToken(
): Promise<AppCheckTokenResult> {
const app = appCheck.app;
ensureActivated(app);
/**
* DEBUG MODE
* return the debug token directly
*/
if (isDebugMode()) {
const tokenFromDebugExchange: AppCheckTokenInternal = await exchangeToken(
getExchangeDebugTokenRequest(app, await getDebugToken()),
appCheck.platformLoggerProvider
);
return { token: tokenFromDebugExchange.token };
}

const state = getState(app);

/**
* 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;

/**
* try to load token from indexedDB if it's the first time this function is called
* If there is no token in memory, try to load token from indexedDB.
*/
if (!token) {
// readTokenFromStorage() always resolves. In case of an error, it resolves with `undefined`.
Expand All @@ -95,13 +87,30 @@ export async function getToken(
}
}

// return the cached token if it's valid
// Return the cached token (from either memory or indexedDB) if it's valid
if (!forceRefresh && token && isValid(token)) {
return {
token: token.token
};
}

/**
* 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 tokenFromDebugExchange: AppCheckTokenInternal = await exchangeToken(
getExchangeDebugTokenRequest(app, await getDebugToken()),
appCheck.platformLoggerProvider
);
// Write debug token to indexedDB.
await writeTokenToStorage(app, tokenFromDebugExchange);
// Write debug token to state.
setState(app, { ...state, token: tokenFromDebugExchange });
return { token: tokenFromDebugExchange.token };
}

/**
* request a new token
*/
Expand All @@ -125,7 +134,7 @@ export async function getToken(
interopTokenResult = {
token: token.token
};
// write the new token to the memory state as well ashe persistent storage.
// write the new token to the memory state as well as the persistent storage.
// Only do it if we got a valid new token
setState(app, { ...state, token });
await writeTokenToStorage(app, token);
Expand All @@ -152,50 +161,31 @@ export function addTokenListener(
...state,
tokenObservers: [...state.tokenObservers, tokenObserver]
};

/**
* DEBUG MODE
*
* invoke the listener once with the debug token.
* Invoke the listener with the valid token, then start the token refresher
*/
if (isDebugMode()) {
const debugState = getDebugState();
if (debugState.enabled && debugState.token) {
debugState.token.promise
.then(token => listener({ token }))
.catch(() => {
/* we don't care about exceptions thrown in listeners */
});
}
} else {
/**
* PROD MODE
*
* invoke the listener with the valid token, then start the token refresher
*/
if (!newState.tokenRefresher) {
const tokenRefresher = createTokenRefresher(appCheck);
newState.tokenRefresher = tokenRefresher;
}
if (!newState.tokenRefresher) {
const tokenRefresher = createTokenRefresher(appCheck);
newState.tokenRefresher = tokenRefresher;
}

// Create the refresher but don't start it if `isTokenAutoRefreshEnabled`
// is not true.
if (
!newState.tokenRefresher.isRunning() &&
state.isTokenAutoRefreshEnabled === true
) {
newState.tokenRefresher.start();
}
// Create the refresher but don't start it if `isTokenAutoRefreshEnabled`
// is not true.
if (
!newState.tokenRefresher.isRunning() &&
state.isTokenAutoRefreshEnabled === true
) {
newState.tokenRefresher.start();
}

// invoke the listener async immediately if there is a valid token
if (state.token && isValid(state.token)) {
const validToken = state.token;
Promise.resolve()
.then(() => listener({ token: validToken.token }))
.catch(() => {
/* we don't care about exceptions thrown in listeners */
});
}
// invoke the listener async immediately if there is a valid token
if (state.token && isValid(state.token)) {
const validToken = state.token;
Promise.resolve()
.then(() => listener({ token: validToken.token }))
.catch(() => {
/* we don't care about exceptions thrown in listeners */
});
}

setState(app, newState);
Expand Down