diff --git a/packages/firestore-compat/src/index.console.ts b/packages/firestore-compat/src/index.console.ts index 65a42f19acb..2e6ccfd3dcd 100644 --- a/packages/firestore-compat/src/index.console.ts +++ b/packages/firestore-compat/src/index.console.ts @@ -23,6 +23,7 @@ import { FirestoreError } from '@firebase/firestore'; +import { EmptyCredentialsProvider } from './src/api/credentials'; import { Firestore as FirestoreCompat, MemoryPersistenceProvider @@ -55,7 +56,7 @@ export class Firestore extends FirestoreCompat { databaseIdFromFirestoreDatabase(firestoreDatabase), new FirestoreExp( databaseIdFromFirestoreDatabase(firestoreDatabase), - authProvider + new EmptyCredentialsProvider() ), new MemoryPersistenceProvider() ); diff --git a/packages/firestore/lite/register.ts b/packages/firestore/lite/register.ts index 6527d19e678..3a483d84fbd 100644 --- a/packages/firestore/lite/register.ts +++ b/packages/firestore/lite/register.ts @@ -23,6 +23,7 @@ import { import { Component, ComponentType } from '@firebase/component'; import { version } from '../package.json'; +import { LiteCredentialsProvider } from '../src/api/credentials'; import { setSDKVersion } from '../src/core/version'; import { Firestore } from '../src/lite-api/database'; import { FirestoreSettings } from '../src/lite-api/settings'; @@ -42,7 +43,7 @@ export function registerFirestore(): void { const app = container.getProvider('app-exp').getImmediate()!; const firestoreInstance = new Firestore( app, - container.getProvider('auth-internal') + new LiteCredentialsProvider(container.getProvider('auth-internal')) ); if (settings) { firestoreInstance._setSettings(settings); diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index 1218e164364..a5030b39098 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -87,6 +87,15 @@ export type CredentialChangeListener = (user: User) => Promise; * listening for changes. */ export interface CredentialsProvider { + /** + * Starts the credentials provider and specifies a listener to be notified of + * credential changes (sign-in / sign-out, token changes). It is immediately + * called once with the initial user. + * + * The change listener is invoked on the provided AsyncQueue. + */ + start(asyncQueue: AsyncQueue, changeListener: CredentialChangeListener): void; + /** Requests a token for the current user. */ getToken(): Promise; @@ -96,53 +105,26 @@ export interface CredentialsProvider { */ invalidateToken(): void; - /** - * Specifies a listener to be notified of credential changes - * (sign-in / sign-out, token changes). It is immediately called once with the - * initial user. - * - * The change listener is invoked on the provided AsyncQueue. - */ - setChangeListener( - asyncQueue: AsyncQueue, - changeListener: CredentialChangeListener - ): void; - - /** Removes the previously-set change listener. */ - removeChangeListener(): void; + shutdown(): void; } /** A CredentialsProvider that always yields an empty token. */ export class EmptyCredentialsProvider implements CredentialsProvider { - /** - * Stores the listener registered with setChangeListener() - * This isn't actually necessary since the UID never changes, but we use this - * to verify the listen contract is adhered to in tests. - */ - private changeListener: CredentialChangeListener | null = null; - getToken(): Promise { return Promise.resolve(null); } invalidateToken(): void {} - setChangeListener( + start( asyncQueue: AsyncQueue, changeListener: CredentialChangeListener ): void { - debugAssert( - !this.changeListener, - 'Can only call setChangeListener() once.' - ); - this.changeListener = changeListener; // Fire with initial user. asyncQueue.enqueueRetryable(() => changeListener(User.UNAUTHENTICATED)); } - removeChangeListener(): void { - this.changeListener = null; - } + shutdown(): void {} } /** @@ -165,7 +147,7 @@ export class EmulatorCredentialsProvider implements CredentialsProvider { invalidateToken(): void {} - setChangeListener( + start( asyncQueue: AsyncQueue, changeListener: CredentialChangeListener ): void { @@ -178,80 +160,148 @@ export class EmulatorCredentialsProvider implements CredentialsProvider { asyncQueue.enqueueRetryable(() => changeListener(this.token.user)); } - removeChangeListener(): void { + shutdown(): void { this.changeListener = null; } } +/** Credential provider for the Lite SDK. */ +export class LiteCredentialsProvider implements CredentialsProvider { + private auth: FirebaseAuthInternal | null = null; + + constructor(authProvider: Provider) { + authProvider.onInit(auth => { + this.auth = auth; + }); + } + + getToken(): Promise { + if (!this.auth) { + return Promise.resolve(null); + } + + return this.auth.getToken().then(tokenData => { + if (tokenData) { + hardAssert( + typeof tokenData.accessToken === 'string', + 'Invalid tokenData returned from getToken():' + tokenData + ); + return new OAuthToken( + tokenData.accessToken, + new User(this.auth!.getUid()) + ); + } else { + return null; + } + }); + } + + invalidateToken(): void {} + + start( + asyncQueue: AsyncQueue, + changeListener: CredentialChangeListener + ): void {} + + shutdown(): void {} +} + export class FirebaseCredentialsProvider implements CredentialsProvider { /** * The auth token listener registered with FirebaseApp, retained here so we * can unregister it. */ - private tokenListener: () => void; + private tokenListener!: () => void; /** Tracks the current User. */ private currentUser: User = User.UNAUTHENTICATED; - /** Promise that allows blocking on the initialization of Firebase Auth. */ - private authDeferred = new Deferred(); - /** * Counter used to detect if the token changed while a getToken request was * outstanding. */ private tokenCounter = 0; - /** The listener registered with setChangeListener(). */ - private changeListener?: CredentialChangeListener; - private forceRefresh = false; private auth: FirebaseAuthInternal | null = null; - private asyncQueue: AsyncQueue | null = null; + constructor(private authProvider: Provider) {} + + start( + asyncQueue: AsyncQueue, + changeListener: CredentialChangeListener + ): void { + let lastTokenId = this.tokenCounter; + + // A change listener that prevents double-firing for the same token change. + const guardedChangeListener: (user: User) => Promise = user => { + if (this.tokenCounter !== lastTokenId) { + lastTokenId = this.tokenCounter; + return changeListener(user); + } else { + return Promise.resolve(); + } + }; + + // A promise that can be waited on to block on the next token change. + // This promise is re-created after each change. + let nextToken = new Deferred(); - constructor(authProvider: Provider) { this.tokenListener = () => { this.tokenCounter++; this.currentUser = this.getUser(); - this.authDeferred.resolve(); - if (this.changeListener) { - this.asyncQueue!.enqueueRetryable(() => - this.changeListener!(this.currentUser) - ); - } + nextToken.resolve(); + nextToken = new Deferred(); + asyncQueue.enqueueRetryable(() => + guardedChangeListener(this.currentUser) + ); }; const registerAuth = (auth: FirebaseAuthInternal): void => { - logDebug('FirebaseCredentialsProvider', 'Auth detected'); - this.auth = auth; - this.auth.addAuthTokenListener(this.tokenListener); + asyncQueue.enqueueRetryable(async () => { + logDebug('FirebaseCredentialsProvider', 'Auth detected'); + this.auth = auth; + this.auth.addAuthTokenListener(this.tokenListener); + + // Call the change listener inline to block on the user change. + await nextToken.promise; + await guardedChangeListener(this.currentUser); + }); }; - authProvider.onInit(auth => registerAuth(auth)); + this.authProvider.onInit(auth => registerAuth(auth)); // Our users can initialize Auth right after Firestore, so we give it // a chance to register itself with the component framework before we // determine whether to start up in unauthenticated mode. setTimeout(() => { if (!this.auth) { - const auth = authProvider.getImmediate({ optional: true }); + const auth = this.authProvider.getImmediate({ optional: true }); if (auth) { registerAuth(auth); } else { // If auth is still not available, proceed with `null` user logDebug('FirebaseCredentialsProvider', 'Auth not yet detected'); - this.authDeferred.resolve(); + nextToken.resolve(); + nextToken = new Deferred(); } } }, 0); + + asyncQueue.enqueueRetryable(async () => { + // If we have not received a token, wait for the first one. + if (this.tokenCounter === 0) { + await nextToken.promise; + await guardedChangeListener(this.currentUser); + } + }); } getToken(): Promise { debugAssert( this.tokenListener != null, - 'getToken cannot be called after listener removed.' + 'FirebaseCredentialsProvider not started.' ); // Take note of the current value of the tokenCounter so that this method @@ -293,26 +343,10 @@ export class FirebaseCredentialsProvider implements CredentialsProvider { this.forceRefresh = true; } - setChangeListener( - asyncQueue: AsyncQueue, - changeListener: CredentialChangeListener - ): void { - debugAssert(!this.asyncQueue, 'Can only call setChangeListener() once.'); - this.asyncQueue = asyncQueue; - - // Blocks the AsyncQueue until the next user is available. - this.asyncQueue!.enqueueRetryable(async () => { - await this.authDeferred.promise; - await changeListener(this.currentUser); - this.changeListener = changeListener; - }); - } - - removeChangeListener(): void { + shutdown(): void { if (this.auth) { this.auth.removeAuthTokenListener(this.tokenListener!); } - this.changeListener = () => Promise.resolve(); } // Auth.getUid() can return null even with a user logged in. It is because @@ -389,7 +423,7 @@ export class FirstPartyCredentialsProvider implements CredentialsProvider { ); } - setChangeListener( + start( asyncQueue: AsyncQueue, changeListener: CredentialChangeListener ): void { @@ -397,7 +431,7 @@ export class FirstPartyCredentialsProvider implements CredentialsProvider { asyncQueue.enqueueRetryable(() => changeListener(User.FIRST_PARTY)); } - removeChangeListener(): void {} + shutdown(): void {} invalidateToken(): void {} } diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index ac36ed00159..922ec7c5a17 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -22,8 +22,6 @@ import { FirebaseApp, getApp } from '@firebase/app-exp'; -import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; -import { Provider } from '@firebase/component'; import { deepEqual } from '@firebase/util'; import { @@ -60,6 +58,7 @@ import { cast } from '../util/input_validation'; import { Deferred } from '../util/promise'; import { LoadBundleTask } from './bundle'; +import { CredentialsProvider } from './credentials'; import { PersistenceSettings, FirestoreSettings } from './settings'; export { connectFirestoreEmulator, @@ -103,9 +102,9 @@ export class Firestore extends LiteFirestore { /** @hideconstructor */ constructor( databaseIdOrApp: DatabaseId | FirebaseApp, - authProvider: Provider + credentialsProvider: CredentialsProvider ) { - super(databaseIdOrApp, authProvider); + super(databaseIdOrApp, credentialsProvider); this._persistenceKey = 'name' in databaseIdOrApp ? databaseIdOrApp.name : '[DEFAULT]'; } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index fc73d49d02f..d8e5b04840a 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -116,7 +116,7 @@ export class FirestoreClient { public asyncQueue: AsyncQueue, private databaseInfo: DatabaseInfo ) { - this.credentials.setChangeListener(asyncQueue, async user => { + this.credentials.start(asyncQueue, async user => { logDebug(LOG_TAG, 'Received user=', user.uid); await this.credentialListener(user); this.user = user; @@ -163,10 +163,10 @@ export class FirestoreClient { await this.offlineComponents.terminate(); } - // `removeChangeListener` must be called after shutting down the - // RemoteStore as it will prevent the RemoteStore from retrieving - // auth tokens. - this.credentials.removeChangeListener(); + // The credentials provider must be terminated after shutting down the + // RemoteStore as it will prevent the RemoteStore from retrieving auth + // tokens. + this.credentials.shutdown(); deferred.resolve(); } catch (e) { const firestoreError = wrapInUserErrorIfRecoverable( diff --git a/packages/firestore/src/lite-api/database.ts b/packages/firestore/src/lite-api/database.ts index 9ab5e9a9f3f..97dd9db739b 100644 --- a/packages/firestore/src/lite-api/database.ts +++ b/packages/firestore/src/lite-api/database.ts @@ -22,15 +22,11 @@ import { FirebaseApp, getApp } from '@firebase/app-exp'; -import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; -import { Provider } from '@firebase/component'; import { createMockUserToken, EmulatorMockTokenOptions } from '@firebase/util'; import { CredentialsProvider, - EmptyCredentialsProvider, EmulatorCredentialsProvider, - FirebaseCredentialsProvider, makeCredentialsProvider, OAuthToken } from '../api/credentials'; @@ -67,7 +63,6 @@ export class Firestore implements FirestoreService { readonly _databaseId: DatabaseId; readonly _persistenceKey: string = '(lite)'; - _credentials: CredentialsProvider; private _settings = new FirestoreSettingsImpl({}); private _settingsFrozen = false; @@ -81,15 +76,13 @@ export class Firestore implements FirestoreService { /** @hideconstructor */ constructor( databaseIdOrApp: DatabaseId | FirebaseApp, - authProvider: Provider + public _credentials: CredentialsProvider ) { if (databaseIdOrApp instanceof DatabaseId) { this._databaseId = databaseIdOrApp; - this._credentials = new EmptyCredentialsProvider(); } else { this._app = databaseIdOrApp as FirebaseApp; this._databaseId = databaseIdFromApp(databaseIdOrApp as FirebaseApp); - this._credentials = new FirebaseCredentialsProvider(authProvider); } } diff --git a/packages/firestore/src/register.ts b/packages/firestore/src/register.ts index baeff5f4a1b..1faf12813c4 100644 --- a/packages/firestore/src/register.ts +++ b/packages/firestore/src/register.ts @@ -23,6 +23,7 @@ import { import { Component, ComponentType } from '@firebase/component'; import { name, version } from '../package.json'; +import { FirebaseCredentialsProvider } from '../src/api/credentials'; import { setSDKVersion } from '../src/core/version'; import { Firestore } from './api/database'; @@ -37,7 +38,9 @@ export function registerFirestore(variant?: string): void { const app = container.getProvider('app-exp').getImmediate()!; const firestoreInstance = new Firestore( app, - container.getProvider('auth-internal') + new FirebaseCredentialsProvider( + container.getProvider('auth-internal') + ) ); settings = { useFetchStreams: true, ...settings }; firestoreInstance._setSettings(settings); diff --git a/packages/firestore/test/integration/util/internal_helpers.ts b/packages/firestore/test/integration/util/internal_helpers.ts index e5b294a3a7c..5c2962dec1f 100644 --- a/packages/firestore/test/integration/util/internal_helpers.ts +++ b/packages/firestore/test/integration/util/internal_helpers.ts @@ -73,11 +73,8 @@ export class MockCredentialsProvider extends EmptyCredentialsProvider { this.asyncQueue!.enqueueRetryable(async () => this.listener!(newUser)); } - setChangeListener( - asyncQueue: AsyncQueue, - listener: CredentialChangeListener - ): void { - super.setChangeListener(asyncQueue, listener); + start(asyncQueue: AsyncQueue, listener: CredentialChangeListener): void { + super.start(asyncQueue, listener); this.asyncQueue = asyncQueue; this.listener = listener; } diff --git a/packages/firestore/test/util/api_helpers.ts b/packages/firestore/test/util/api_helpers.ts index 3aa85845504..432f3fe4666 100644 --- a/packages/firestore/test/util/api_helpers.ts +++ b/packages/firestore/test/util/api_helpers.ts @@ -18,8 +18,6 @@ // Helpers here mock Firestore in order to unit-test API types. Do NOT use // these in any integration test, where we expect working Firestore object. -import { Provider, ComponentContainer } from '@firebase/component'; - import { CollectionReference, DocumentReference, @@ -30,6 +28,7 @@ import { QuerySnapshot, UserDataWriter } from '../../compat/api/database'; +import { EmptyCredentialsProvider } from '../../src/api/credentials'; import { ensureFirestoreConfigured, Firestore as ExpFirestore @@ -70,10 +69,7 @@ export function firestore(): Firestore { export function newTestFirestore(projectId = 'new-project'): Firestore { return new Firestore( new DatabaseId(projectId), - new ExpFirestore( - new DatabaseId(projectId), - new Provider('auth-internal', new ComponentContainer('default')) - ), + new ExpFirestore(new DatabaseId(projectId), new EmptyCredentialsProvider()), new IndexedDbPersistenceProvider() ); }