fix: Decoupled proactive token refresh from FirebaseApp

[hiranya911]

Our existing code for OAuth2 token management is quite complicated. This is due to FirebaseApp doing too much -- namely handling token caching as well proactive token refreshing. The proactive token refresh was added to make our RTDB connections more stable, but having this logic in FirebaseApp impacts other APIs in the SDK too. For instance this is why the SDK won't let the Node.js process terminate, unless you call app.delete().

In order to simplify our token management, and align more with other GCP libraries that deal with tokens, I'm proposing to separate the proactive token refresh from the FirebaseApp class. To that end this PR implements the following changes:

1. Remove the proactive token refresh logic from FirebaseApp.
2. Simplify the token caching and fetching behavior.
3. Treat any cached token that's at most 5 minutes away from expiration to be already expired.
4. Implement the proactive token refresh as part of the database service, where it will only get initialized if a developer starts an RTDB connection.
5. Remove the existing complicated retry logic in the proactive token refresh.

Change no. 3 is pretty standard across GCP libraries (see here and here). We never implemented this in the Admin Node.js SDK since the token refresher was indirectly handling it. But it appears in certain environments we cannot rely on the token refresher to keep the auth token current. Therefore by eagerly treating cached tokens as expired, we can mitigate issues like #1193 entirely.

As for change no. 5, the existing retry logic in the token refresher is rather error prone and redundant. If something goes wrong with a proactive refresh attempt, the worst that could happen is any active RTDB connections dropping out upon the expiration of the current token. The RTDB SDK automatically reconnects when this happens (provided it can fetch a new token at that point). This is what we do in our Java SDK too, so there's some prior art as well.

RELEASE NOTE: Improved OAuth2 token caching and management. The SDK now treats any OAuth2 token 5 minutes away from expiration as already expired, and proactively refreshes them. This helps avoid certain types of authorization-related race conditions.
RELEASE NOTE: The periodic token refresher background task has been decoupled from the SDK core, and moved into the RTDB module. This task no longer starts automatically, unless the admin.database() API is explicitly invoked.

[inlined]

IIRC there was no other token refresh listener. If the right action is to immediately ask for a new token from app.INTERNAL should we just remove the parameter?

[hiranya911]

I think an even better approach would be to change the signature of the listener to accept a FirebaseAccessToken which contains both token and expiry time. That way we wouldn't have to call getToken() in the listener at all (we currently do it to get hold of the expiry time).

However, this API currently mirrors an API in the JS SDK:

https://github.com/firebase/firebase-js-sdk/blob/ec6ccc7d319732588ceacae0824d6c7551afb026/packages/app-types/private.d.ts#L76

The RTDB implementation expects our App and App.INTERNAL APIs to be compatible with the JS SDK. Therefore we'll need to check with the JS SDK team before making any changes to the listener signature.

[inlined]

Absolutely awesome turnaround time. Thanks Hiranya!

[inlined]

I'm confused. isn't getAccesssToken() already returning a promise? Why do we need to resolve the promise instead of just chaining?

[hiranya911]

I thought so too :)

But when I made the change, it actually caused a test failure here:

firebase-admin-node/test/unit/firebase-app.spec.ts

Lines 677 to 691 in 994fd43

	it('throws a custom credential implementation which returns invalid access tokens', () => {
	const credential = {
	getAccessToken: () => 5,
	};
	const app = utils.createAppWithOptions({
	credential: credential as any,
	});
	return app.INTERNAL.getToken().then(() => {
	throw new Error('Unexpected success');
	}, (err) => {
	expect(err.toString()).to.include('Invalid access token generated');
	});
	});

This is because the credential in this case returns a value which cannot be chained. I'm not sure how much we care about this sort of edge cases, but for now I've opted to keep it as is.

[schmidt-sebastian]

It think the error most likely occurred during getToken() and not during the scheduling step.

[hiranya911]

True. But overall this function is attempting to schedule a new token refresh event. It calls getToken() just to figure out when to schedule that event.

[schmidt-sebastian]

Optional suggestion:

You can make this a little simpler if you combine tokenRefresh and tokenRefreshTimeout. You could initialize tokenRefreshTimeout to undefined initially and then set the schedule right here, which seems more intuitive to me. With the current setup, I have to look at onTokenChange to figure out the scheduling.

I also wonder if we should use setInterval. It seems a little more appropriate, but might make things more complicated in some places.

[hiranya911]

This code is just intended to register the token change listener. There's no guarantee that the listener will fire immediately. So theoretically it is possible for tokenRefresh to be true, while tokenRefreshTimeout is not yet set. What we really need is a way to track whether we have registered the listener or not (so we can avoid duplicate registrations, and also unregister it at delete). So I'm renaming tokenRefresh to tokenListenerRegistered. That should clear up any confusions, and also address the above comment regarding naming.

As for setInterval(), given that we can receive tokens with arbitrary expiration times from auth (we've seen tokens with expiry times like 30min and 45min), an interval scheduler is not very useful. So we use the token listener to watch for token changes, and schedule refresh events according to the expiry time we get.

[schmidt-sebastian]

Can the RTDB component just set forceRefresh to true if the expiration time has been hit? That way, we only need to deal with TOKEN_REFRESH_THRESHOLD_MILLIS in one place.

[hiranya911]

These 2 constants are used for slightly different purposes:

1. In firebase-app it is used to determine how early we should treat a valid token as expired.
2. In database-internal it is used to determine when to schedule the next refresh event.

These don't have to be the same value. For instance, we can have FirebaseApp invalidate a token 5 minutes before it expires, but have RTDB trigger the token refresh 10 minutes before the current token expires.

To make the above distinction clear, I'm renaming the constant in firebase-app to TOKEN_EXPIRY_THRESHOLD_MILLIS. This constant doesn't have anything to do with refreshing tokens. It's simply when we treat the token as expired.

[lahirumaramba]

Thanks, Hiranya! LGTM!
Left a couple comments.

[lahirumaramba]

Does it matter the what order we disarm here? Could this cause a race condition?

[hiranya911]

I felt like we should unregister the listener before clearing out the timer. If we clear the timer first, theoretically it's possible for the listener to fire again and re-initialize the timer. Given both these APIs are synchronous, I don't think it's a real possibility (given how Node.js schedules code), but this seems safer.