Skip to content

Always block on Auth #5340

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.

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 2 commits into from
Aug 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion packages/firestore-compat/src/index.console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
FirestoreError
} from '@firebase/firestore';

import { EmptyCredentialsProvider } from './src/api/credentials';
import {
Firestore as FirestoreCompat,
MemoryPersistenceProvider
Expand Down Expand Up @@ -55,7 +56,7 @@ export class Firestore extends FirestoreCompat {
databaseIdFromFirestoreDatabase(firestoreDatabase),
new FirestoreExp(
databaseIdFromFirestoreDatabase(firestoreDatabase),
authProvider
new EmptyCredentialsProvider()
),
new MemoryPersistenceProvider()
);
Expand Down
3 changes: 2 additions & 1 deletion packages/firestore/lite/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
Expand Down
180 changes: 107 additions & 73 deletions packages/firestore/src/api/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,15 @@ export type CredentialChangeListener = (user: User) => Promise<void>;
* 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<Token | null>;

Expand All @@ -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<Token | null> {
return Promise.resolve<Token | null>(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 {}
}

/**
Expand All @@ -165,7 +147,7 @@ export class EmulatorCredentialsProvider implements CredentialsProvider {

invalidateToken(): void {}

setChangeListener(
start(
asyncQueue: AsyncQueue,
changeListener: CredentialChangeListener
): void {
Expand All @@ -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<FirebaseAuthInternalName>) {
authProvider.onInit(auth => {
this.auth = auth;
});
}

getToken(): Promise<Token | null> {
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<FirebaseAuthInternalName>) {}

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<void> = 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<void>();

constructor(authProvider: Provider<FirebaseAuthInternalName>) {
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<void>();
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<void>();
}
}
}, 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<Token | null> {
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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -389,15 +423,15 @@ export class FirstPartyCredentialsProvider implements CredentialsProvider {
);
}

setChangeListener(
start(
asyncQueue: AsyncQueue,
changeListener: CredentialChangeListener
): void {
// Fire with initial uid.
asyncQueue.enqueueRetryable(() => changeListener(User.FIRST_PARTY));
}

removeChangeListener(): void {}
shutdown(): void {}

invalidateToken(): void {}
}
Expand Down
7 changes: 3 additions & 4 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -103,9 +102,9 @@ export class Firestore extends LiteFirestore {
/** @hideconstructor */
constructor(
databaseIdOrApp: DatabaseId | FirebaseApp,
authProvider: Provider<FirebaseAuthInternalName>
credentialsProvider: CredentialsProvider
) {
super(databaseIdOrApp, authProvider);
super(databaseIdOrApp, credentialsProvider);
this._persistenceKey =
'name' in databaseIdOrApp ? databaseIdOrApp.name : '[DEFAULT]';
}
Expand Down
10 changes: 5 additions & 5 deletions packages/firestore/src/core/firestore_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Loading