From 0e756857bc5c8499396e3455a5145c3caeea84a0 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Sun, 17 Oct 2021 00:12:11 -0500 Subject: [PATCH 01/25] AppCheck integration for Firestore. --- packages/firestore/src/api/credentials.ts | 177 ++++++++++++++++-- packages/firestore/src/api/database.ts | 13 +- .../firestore/src/core/component_provider.ts | 10 +- .../firestore/src/core/firestore_client.ts | 22 ++- packages/firestore/src/lite-api/components.ts | 7 +- packages/firestore/src/lite-api/database.ts | 8 +- packages/firestore/src/register.ts | 8 +- packages/firestore/src/remote/datastore.ts | 28 ++- .../firestore/src/remote/persistent_stream.ts | 21 ++- .../test/integration/util/internal_helpers.ts | 18 +- .../firestore/test/unit/api/database.test.ts | 4 +- .../test/unit/remote/datastore.test.ts | 81 ++++++-- .../test/unit/specs/spec_test_components.ts | 7 +- .../test/unit/specs/spec_test_runner.ts | 5 +- packages/firestore/test/util/api_helpers.ts | 11 +- 15 files changed, 339 insertions(+), 81 deletions(-) diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index 12352cae958..e2e35484d17 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -19,6 +19,13 @@ import { FirebaseAuthInternal, FirebaseAuthInternalName } from '@firebase/auth-interop-types'; + +import { + AppCheckInternalComponentName, + AppCheckTokenListener, + AppCheckTokenResult, + FirebaseAppCheckInternal +} from '@firebase/app-check-interop-types'; import { Provider } from '@firebase/component'; import { User } from '../auth/user'; @@ -42,7 +49,7 @@ export interface FirstPartyCredentialsSettings { export interface ProviderCredentialsSettings { // These are external types. Prevent minification. ['type']: 'provider'; - ['client']: CredentialsProvider; + ['client']: CredentialsProvider; } /** Settings for private credentials */ @@ -50,7 +57,7 @@ export type CredentialsSettings = | FirstPartyCredentialsSettings | ProviderCredentialsSettings; -export type TokenType = 'OAuth' | 'FirstParty'; +export type TokenType = 'OAuth' | 'FirstParty' | 'AppCheck'; export interface Token { /** Type of token. */ type: TokenType; @@ -58,6 +65,7 @@ export interface Token { /** * The user with which the token is associated (used for persisting user * state on disk, etc.). + * This will be null for Tokens of the type 'AppCheck'. */ user: User; @@ -80,13 +88,13 @@ export class OAuthToken implements Token { * token and may need to invalidate other state if the current user has also * changed. */ -export type CredentialChangeListener = (user: User) => Promise; +export type CredentialChangeListener = (credential: T) => Promise; /** * Provides methods for getting the uid and token for the current user and * listening for changes. */ -export interface CredentialsProvider { +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 @@ -94,7 +102,10 @@ export interface CredentialsProvider { * * The change listener is invoked on the provided AsyncQueue. */ - start(asyncQueue: AsyncQueue, changeListener: CredentialChangeListener): void; + start( + asyncQueue: AsyncQueue, + changeListener: CredentialChangeListener + ): void; /** Requests a token for the current user. */ getToken(): Promise; @@ -112,7 +123,7 @@ export interface CredentialsProvider { * A CredentialsProvider that always yields an empty token. * @internal */ -export class EmptyCredentialsProvider implements CredentialsProvider { +export class EmptyCredentialsProvider implements CredentialsProvider { getToken(): Promise { return Promise.resolve(null); } @@ -121,7 +132,7 @@ export class EmptyCredentialsProvider implements CredentialsProvider { start( asyncQueue: AsyncQueue, - changeListener: CredentialChangeListener + changeListener: CredentialChangeListener ): void { // Fire with initial user. asyncQueue.enqueueRetryable(() => changeListener(User.UNAUTHENTICATED)); @@ -134,7 +145,7 @@ export class EmptyCredentialsProvider implements CredentialsProvider { * A CredentialsProvider that always returns a constant token. Used for * emulator token mocking. */ -export class EmulatorCredentialsProvider implements CredentialsProvider { +export class EmulatorCredentialsProvider implements CredentialsProvider { constructor(private token: Token) {} /** @@ -142,7 +153,7 @@ export class EmulatorCredentialsProvider implements CredentialsProvider { * 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; + private changeListener: CredentialChangeListener | null = null; getToken(): Promise { return Promise.resolve(this.token); @@ -152,7 +163,7 @@ export class EmulatorCredentialsProvider implements CredentialsProvider { start( asyncQueue: AsyncQueue, - changeListener: CredentialChangeListener + changeListener: CredentialChangeListener ): void { debugAssert( !this.changeListener, @@ -169,7 +180,7 @@ export class EmulatorCredentialsProvider implements CredentialsProvider { } /** Credential provider for the Lite SDK. */ -export class LiteCredentialsProvider implements CredentialsProvider { +export class LiteCredentialsProvider implements CredentialsProvider { private auth: FirebaseAuthInternal | null = null; constructor(authProvider: Provider) { @@ -203,13 +214,13 @@ export class LiteCredentialsProvider implements CredentialsProvider { start( asyncQueue: AsyncQueue, - changeListener: CredentialChangeListener + changeListener: CredentialChangeListener ): void {} shutdown(): void {} } -export class FirebaseCredentialsProvider implements CredentialsProvider { +export class FirebaseCredentialsProvider implements CredentialsProvider { /** * The auth token listener registered with FirebaseApp, retained here so we * can unregister it. @@ -233,7 +244,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider { start( asyncQueue: AsyncQueue, - changeListener: CredentialChangeListener + changeListener: CredentialChangeListener ): void { let lastTokenId = this.tokenCounter; @@ -410,7 +421,9 @@ export class FirstPartyToken implements Token { * to authenticate the user, using technique that is only available * to applications hosted by Google. */ -export class FirstPartyCredentialsProvider implements CredentialsProvider { +export class FirstPartyCredentialsProvider + implements CredentialsProvider +{ constructor( private gapi: Gapi, private sessionIndex: string, @@ -425,7 +438,7 @@ export class FirstPartyCredentialsProvider implements CredentialsProvider { start( asyncQueue: AsyncQueue, - changeListener: CredentialChangeListener + changeListener: CredentialChangeListener ): void { // Fire with initial uid. asyncQueue.enqueueRetryable(() => changeListener(User.FIRST_PARTY)); @@ -436,13 +449,143 @@ export class FirstPartyCredentialsProvider implements CredentialsProvider { invalidateToken(): void {} } +export class AppCheckToken implements Token { + type = 'AppCheck' as TokenType; + user = null as any; + authHeaders: { [header: string]: string }; + + constructor(value: string) { + this.authHeaders = {}; + // Set the headers using Object Literal notation to avoid minification + this.authHeaders['x-firebase-appcheck'] = value; + } +} + +export class FirebaseAppCheckTokenProvider + implements CredentialsProvider +{ + /** + * The AppCheck token listener registered with FirebaseApp, retained here so + * we can unregister it. + */ + private tokenListener!: AppCheckTokenListener; + + private forceRefresh = false; + + private appCheck: FirebaseAppCheckInternal | null = null; + + constructor( + private appCheckProvider: Provider + ) {} + + start( + asyncQueue: AsyncQueue, + changeListener: CredentialChangeListener + ): void { + const onTokenChanged: (tokenResult: AppCheckTokenResult) => Promise = + tokenResult => { + if (tokenResult.error != null) { + logDebug( + 'FirebaseAppCheckTokenProvider', + `Error getting App Check token; using placeholder token instead. Error: ${tokenResult.error.message}` + ); + } + return changeListener(tokenResult.token); + }; + + this.tokenListener = (tokenResult: AppCheckTokenResult) => { + asyncQueue.enqueueRetryable(() => onTokenChanged(tokenResult)); + }; + + const registerAppCheck = (appCheck: FirebaseAppCheckInternal): void => { + logDebug('FirebaseAppCheckTokenProvider', 'AppCheck detected'); + this.appCheck = appCheck; + this.appCheck.addTokenListener(this.tokenListener); + }; + + this.appCheckProvider.onInit(appCheck => registerAppCheck(appCheck)); + + // Our users can initialize AppCheck after Firestore, so we give it + // a chance to register itself with the component framework. + setTimeout(() => { + if (!this.appCheck) { + const appCheck = this.appCheckProvider.getImmediate({ optional: true }); + if (appCheck) { + registerAppCheck(appCheck); + } else { + // If AppCheck is still not available, proceed without it. + logDebug( + 'FirebaseAppCheckTokenProvider', + 'AppCheck not yet detected' + ); + } + } + }, 0); + } + + getToken(): Promise { + debugAssert( + this.tokenListener != null, + 'FirebaseAppCheckTokenProvider not started.' + ); + + const forceRefresh = this.forceRefresh; + this.forceRefresh = false; + + if (!this.appCheck) { + return Promise.resolve(null); + } + + return this.appCheck.getToken(forceRefresh).then(tokenResult => { + if (tokenResult) { + hardAssert( + typeof tokenResult.token === 'string', + 'Invalid tokenResult returned from getToken():' + tokenResult + ); + return new AppCheckToken(tokenResult.token); + } else { + return null; + } + }); + } + + invalidateToken(): void { + this.forceRefresh = true; + } + + shutdown(): void { + if (this.appCheck) { + this.appCheck.removeTokenListener(this.tokenListener!); + } + } +} + +/** + * An AppCheck token provider that always yields an empty token. + * @internal + */ +export class EmptyAppCheckTokenProvider implements CredentialsProvider { + getToken(): Promise { + return Promise.resolve(new AppCheckToken('')); + } + + invalidateToken(): void {} + + start( + asyncQueue: AsyncQueue, + changeListener: CredentialChangeListener + ): void {} + + shutdown(): void {} +} + /** * Builds a CredentialsProvider depending on the type of * the credentials passed in. */ export function makeCredentialsProvider( credentials?: CredentialsSettings -): CredentialsProvider { +): CredentialsProvider { if (!credentials) { return new EmptyCredentialsProvider(); } diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index c4a74e4e541..36225eebc8d 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -60,6 +60,7 @@ import { Deferred } from '../util/promise'; import { LoadBundleTask } from './bundle'; import { CredentialsProvider } from './credentials'; import { PersistenceSettings, FirestoreSettings } from './settings'; +import { User } from '../auth/user'; export { connectFirestoreEmulator, EmulatorMockTokenOptions @@ -102,9 +103,14 @@ export class Firestore extends LiteFirestore { /** @hideconstructor */ constructor( databaseIdOrApp: DatabaseId | FirebaseApp, - credentialsProvider: CredentialsProvider + authCredentialsProvider: CredentialsProvider, + appCheckCredentialsProvider: CredentialsProvider ) { - super(databaseIdOrApp, credentialsProvider); + super( + databaseIdOrApp, + authCredentialsProvider, + appCheckCredentialsProvider + ); this._persistenceKey = 'name' in databaseIdOrApp ? databaseIdOrApp.name : '[DEFAULT]'; } @@ -207,7 +213,8 @@ export function configureFirestore(firestore: Firestore): void { settings ); firestore._firestoreClient = new FirestoreClient( - firestore._credentials, + firestore._authCredentials, + firestore._appCheckCredentials, firestore._queue, databaseInfo ); diff --git a/packages/firestore/src/core/component_provider.ts b/packages/firestore/src/core/component_provider.ts index 9c847ffd34c..87ffaac3710 100644 --- a/packages/firestore/src/core/component_provider.ts +++ b/packages/firestore/src/core/component_provider.ts @@ -75,7 +75,8 @@ import { OnlineStateSource } from './types'; export interface ComponentConfiguration { asyncQueue: AsyncQueue; databaseInfo: DatabaseInfo; - credentials: CredentialsProvider; + authCredentials: CredentialsProvider; + appCheckCredentials: CredentialsProvider; clientId: ClientId; initialUser: User; maxConcurrentLimboResolutions: number; @@ -371,7 +372,12 @@ export class OnlineComponentProvider { createDatastore(cfg: ComponentConfiguration): Datastore { const serializer = newSerializer(cfg.databaseInfo.databaseId); const connection = newConnection(cfg.databaseInfo); - return newDatastore(cfg.credentials, connection, serializer); + return newDatastore( + cfg.authCredentials, + cfg.appCheckCredentials, + connection, + serializer + ); } createRemoteStore(cfg: ComponentConfiguration): RemoteStore { diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index d8e5b04840a..c333744e3a7 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -97,14 +97,15 @@ export const MAX_CONCURRENT_LIMBO_RESOLUTIONS = 100; export class FirestoreClient { private user = User.UNAUTHENTICATED; private readonly clientId = AutoId.newId(); - private credentialListener: CredentialChangeListener = () => + private authCredentialListener: CredentialChangeListener = () => Promise.resolve(); offlineComponents?: OfflineComponentProvider; onlineComponents?: OnlineComponentProvider; constructor( - private credentials: CredentialsProvider, + private authCredentials: CredentialsProvider, + private appCheckCredentials: CredentialsProvider, /** * Asynchronous queue responsible for all of our internal processing. When * we get incoming work from the user (via public API) or the network @@ -116,11 +117,16 @@ export class FirestoreClient { public asyncQueue: AsyncQueue, private databaseInfo: DatabaseInfo ) { - this.credentials.start(asyncQueue, async user => { + this.authCredentials.start(asyncQueue, async user => { logDebug(LOG_TAG, 'Received user=', user.uid); - await this.credentialListener(user); + await this.authCredentialListener(user); this.user = user; }); + this.appCheckCredentials.start(asyncQueue, async token => { + logDebug(LOG_TAG, 'Received new AppCheck token'); + // Register an empty credentials change listener to activate token + // refresh. + }); } async getConfiguration(): Promise { @@ -128,14 +134,15 @@ export class FirestoreClient { asyncQueue: this.asyncQueue, databaseInfo: this.databaseInfo, clientId: this.clientId, - credentials: this.credentials, + authCredentials: this.authCredentials, + appCheckCredentials: this.appCheckCredentials, initialUser: this.user, maxConcurrentLimboResolutions: MAX_CONCURRENT_LIMBO_RESOLUTIONS }; } setCredentialChangeListener(listener: (user: User) => Promise): void { - this.credentialListener = listener; + this.authCredentialListener = listener; } /** @@ -166,7 +173,8 @@ export class FirestoreClient { // The credentials provider must be terminated after shutting down the // RemoteStore as it will prevent the RemoteStore from retrieving auth // tokens. - this.credentials.shutdown(); + this.authCredentials.shutdown(); + this.appCheckCredentials.shutdown(); deferred.resolve(); } catch (e) { const firestoreError = wrapInUserErrorIfRecoverable( diff --git a/packages/firestore/src/lite-api/components.ts b/packages/firestore/src/lite-api/components.ts index 8dfaa32d1b9..e20ca5e0ff7 100644 --- a/packages/firestore/src/lite-api/components.ts +++ b/packages/firestore/src/lite-api/components.ts @@ -27,6 +27,7 @@ import { Code, FirestoreError } from '../util/error'; import { logDebug } from '../util/log'; import { FirestoreSettingsImpl } from './settings'; +import { User } from '../auth/user'; export const LOG_TAG = 'ComponentProvider'; @@ -41,7 +42,8 @@ export const LOG_TAG = 'ComponentProvider'; * This interface mainly exists to remove a cyclic dependency. */ export interface FirestoreService extends _FirebaseService { - _credentials: CredentialsProvider; + _authCredentials: CredentialsProvider; + _appCheckCredentials: CredentialsProvider; _persistenceKey: string; _databaseId: DatabaseId; _terminated: boolean; @@ -77,7 +79,8 @@ export function getDatastore(firestore: FirestoreService): Datastore { const connection = newConnection(databaseInfo); const serializer = newSerializer(firestore._databaseId); const datastore = newDatastore( - firestore._credentials, + firestore._authCredentials, + firestore._appCheckCredentials, connection, serializer ); diff --git a/packages/firestore/src/lite-api/database.ts b/packages/firestore/src/lite-api/database.ts index a082af975d3..4784bca5b20 100644 --- a/packages/firestore/src/lite-api/database.ts +++ b/packages/firestore/src/lite-api/database.ts @@ -27,6 +27,7 @@ import { createMockUserToken, EmulatorMockTokenOptions } from '@firebase/util'; import { CredentialsProvider, EmulatorCredentialsProvider, + FirebaseAppCheckTokenProvider, makeCredentialsProvider, OAuthToken } from '../api/credentials'; @@ -76,7 +77,8 @@ export class Firestore implements FirestoreService { /** @hideconstructor */ constructor( databaseIdOrApp: DatabaseId | FirebaseApp, - public _credentials: CredentialsProvider + public _authCredentials: CredentialsProvider, + public _appCheckCredentials: CredentialsProvider ) { if (databaseIdOrApp instanceof DatabaseId) { this._databaseId = databaseIdOrApp; @@ -120,7 +122,7 @@ export class Firestore implements FirestoreService { } this._settings = new FirestoreSettingsImpl(settings); if (settings.credentials !== undefined) { - this._credentials = makeCredentialsProvider(settings.credentials); + this._authCredentials = makeCredentialsProvider(settings.credentials); } } @@ -274,7 +276,7 @@ export function connectFirestoreEmulator( user = new User(uid); } - firestore._credentials = new EmulatorCredentialsProvider( + firestore._authCredentials = new EmulatorCredentialsProvider( new OAuthToken(token, user) ); } diff --git a/packages/firestore/src/register.ts b/packages/firestore/src/register.ts index 58343e8a470..77e6b698ab2 100644 --- a/packages/firestore/src/register.ts +++ b/packages/firestore/src/register.ts @@ -23,7 +23,10 @@ import { import { Component, ComponentType } from '@firebase/component'; import { name, version } from '../package.json'; -import { FirebaseCredentialsProvider } from '../src/api/credentials'; +import { + FirebaseAppCheckTokenProvider, + FirebaseCredentialsProvider +} from '../src/api/credentials'; import { setSDKVersion } from '../src/core/version'; import { Firestore } from './api/database'; @@ -43,6 +46,9 @@ export function registerFirestore( app, new FirebaseCredentialsProvider( container.getProvider('auth-internal') + ), + new FirebaseAppCheckTokenProvider( + container.getProvider('app-check-internal') ) ); settings = { useFetchStreams, ...settings }; diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index 944a59e5b4c..0e3dd302354 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -46,6 +46,7 @@ import { toName, toQueryTarget } from './serializer'; +import { User } from '../auth/user'; /** * Datastore and its related methods are a wrapper around the external Google @@ -64,7 +65,8 @@ class DatastoreImpl extends Datastore { terminated = false; constructor( - readonly credentials: CredentialsProvider, + readonly authCredentials: CredentialsProvider, + readonly appCheckCredentials: CredentialsProvider, readonly connection: Connection, readonly serializer: JsonProtoSerializer ) { @@ -88,7 +90,7 @@ class DatastoreImpl extends Datastore { request: Req ): Promise { this.verifyInitialized(); - return this.credentials + return this.authCredentials .getToken() .then(token => { return this.connection.invokeRPC( @@ -101,7 +103,7 @@ class DatastoreImpl extends Datastore { .catch((error: FirestoreError) => { if (error.name === 'FirebaseError') { if (error.code === Code.UNAUTHENTICATED) { - this.credentials.invalidateToken(); + this.authCredentials.invalidateToken(); } throw error; } else { @@ -117,7 +119,7 @@ class DatastoreImpl extends Datastore { request: Req ): Promise { this.verifyInitialized(); - return this.credentials + return this.authCredentials .getToken() .then(token => { return this.connection.invokeStreamingRPC( @@ -130,7 +132,7 @@ class DatastoreImpl extends Datastore { .catch((error: FirestoreError) => { if (error.name === 'FirebaseError') { if (error.code === Code.UNAUTHENTICATED) { - this.credentials.invalidateToken(); + this.authCredentials.invalidateToken(); } throw error; } else { @@ -147,11 +149,17 @@ class DatastoreImpl extends Datastore { // TODO(firestorexp): Make sure there is only one Datastore instance per // firestore-exp client. export function newDatastore( - credentials: CredentialsProvider, + authCredentials: CredentialsProvider, + appCheckCredentials: CredentialsProvider, connection: Connection, serializer: JsonProtoSerializer ): Datastore { - return new DatastoreImpl(credentials, connection, serializer); + return new DatastoreImpl( + authCredentials, + appCheckCredentials, + connection, + serializer + ); } export async function invokeCommitRpc( @@ -224,7 +232,8 @@ export function newPersistentWriteStream( return new PersistentWriteStream( queue, datastoreImpl.connection, - datastoreImpl.credentials, + datastoreImpl.authCredentials, + datastoreImpl.appCheckCredentials, datastoreImpl.serializer, listener ); @@ -240,7 +249,8 @@ export function newPersistentWatchStream( return new PersistentListenStream( queue, datastoreImpl.connection, - datastoreImpl.credentials, + datastoreImpl.authCredentials, + datastoreImpl.appCheckCredentials, datastoreImpl.serializer, listener ); diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index 3a684e8c543..c2716f6f40d 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -46,6 +46,7 @@ import { versionFromListenResponse } from './serializer'; import { WatchChange } from './watch_change'; +import { User } from '../auth/user'; const LOG_TAG = 'PersistentStream'; @@ -199,7 +200,8 @@ export abstract class PersistentStream< private idleTimerId: TimerId, private healthTimerId: TimerId, protected connection: Connection, - private credentialsProvider: CredentialsProvider, + private authCredentialsProvider: CredentialsProvider, + private appCheckCredentialsProvider: CredentialsProvider, protected listener: ListenerType ) { this.backoff = new ExponentialBackoff(queue, connectionTimerId); @@ -387,7 +389,8 @@ export abstract class PersistentStream< // fail, however. In this case, we should get a Code.UNAUTHENTICATED error // before we received the first message and we need to invalidate the token // to ensure that we fetch a new token. - this.credentialsProvider.invalidateToken(); + this.authCredentialsProvider.invalidateToken(); + this.appCheckCredentialsProvider.invalidateToken(); } // Clean up the underlying stream because we are no longer interested in events. @@ -439,7 +442,7 @@ export abstract class PersistentStream< // TODO(mikelehen): Just use dispatchIfNotClosed, but see TODO below. const closeCount = this.closeCount; - this.credentialsProvider.getToken().then( + this.authCredentialsProvider.getToken().then( token => { // Stream can be stopped while waiting for authentication. // TODO(mikelehen): We really should just use dispatchIfNotClosed @@ -597,7 +600,8 @@ export class PersistentListenStream extends PersistentStream< constructor( queue: AsyncQueue, connection: Connection, - credentials: CredentialsProvider, + authCredentials: CredentialsProvider, + appCheckCredentials: CredentialsProvider, private serializer: JsonProtoSerializer, listener: WatchStreamListener ) { @@ -607,7 +611,8 @@ export class PersistentListenStream extends PersistentStream< TimerId.ListenStreamIdle, TimerId.HealthCheckTimeout, connection, - credentials, + authCredentials, + appCheckCredentials, listener ); } @@ -706,7 +711,8 @@ export class PersistentWriteStream extends PersistentStream< constructor( queue: AsyncQueue, connection: Connection, - credentials: CredentialsProvider, + authCredentials: CredentialsProvider, + appCheckCredentials: CredentialsProvider, private serializer: JsonProtoSerializer, listener: WriteStreamListener ) { @@ -716,7 +722,8 @@ export class PersistentWriteStream extends PersistentStream< TimerId.WriteStreamIdle, TimerId.HealthCheckTimeout, connection, - credentials, + authCredentials, + appCheckCredentials, listener ); } diff --git a/packages/firestore/test/integration/util/internal_helpers.ts b/packages/firestore/test/integration/util/internal_helpers.ts index 5c2962dec1f..e5da7fb58e6 100644 --- a/packages/firestore/test/integration/util/internal_helpers.ts +++ b/packages/firestore/test/integration/util/internal_helpers.ts @@ -21,6 +21,7 @@ import { Firestore } from '../../../compat/api/database'; import { CredentialChangeListener, CredentialsProvider, + EmptyAppCheckTokenProvider, EmptyCredentialsProvider } from '../../../src/api/credentials'; import { User } from '../../../src/auth/user'; @@ -56,24 +57,33 @@ export function getDefaultDatabaseInfo(): DatabaseInfo { export function withTestDatastore( fn: (datastore: Datastore) => Promise, - credentialsProvider: CredentialsProvider = new EmptyCredentialsProvider() + authCredentialsProvider: CredentialsProvider = new EmptyCredentialsProvider(), + appCheckTokenProvider: CredentialsProvider = new EmptyAppCheckTokenProvider() ): Promise { const databaseInfo = getDefaultDatabaseInfo(); const connection = newConnection(databaseInfo); const serializer = newSerializer(databaseInfo.databaseId); - const datastore = newDatastore(credentialsProvider, connection, serializer); + const datastore = newDatastore( + authCredentialsProvider, + appCheckTokenProvider, + connection, + serializer + ); return fn(datastore); } export class MockCredentialsProvider extends EmptyCredentialsProvider { - private listener: CredentialChangeListener | null = null; + private listener: CredentialChangeListener | null = null; private asyncQueue: AsyncQueue | null = null; triggerUserChange(newUser: User): void { this.asyncQueue!.enqueueRetryable(async () => this.listener!(newUser)); } - start(asyncQueue: AsyncQueue, listener: CredentialChangeListener): void { + start( + asyncQueue: AsyncQueue, + listener: CredentialChangeListener + ): void { super.start(asyncQueue, listener); this.asyncQueue = asyncQueue; this.listener = listener; diff --git a/packages/firestore/test/unit/api/database.test.ts b/packages/firestore/test/unit/api/database.test.ts index 8ffec48399a..a6ea0ce1257 100644 --- a/packages/firestore/test/unit/api/database.test.ts +++ b/packages/firestore/test/unit/api/database.test.ts @@ -259,7 +259,7 @@ describe('Settings', () => { const mockUserToken = { sub: 'foobar' }; db.useEmulator('localhost', 9000, { mockUserToken }); - const credentials = db._delegate._credentials; + const credentials = db._delegate._authCredentials; expect(credentials).to.be.instanceOf(EmulatorCredentialsProvider); const token = await credentials.getToken(); expect(token!.type).to.eql('OAuth'); @@ -273,7 +273,7 @@ describe('Settings', () => { mockUserToken: 'my-custom-mock-user-token' }); - const credentials = db._delegate._credentials; + const credentials = db._delegate._authCredentials; expect(credentials).to.be.instanceOf(EmulatorCredentialsProvider); const token = await credentials.getToken(); expect(token!.type).to.eql('OAuth'); diff --git a/packages/firestore/test/unit/remote/datastore.test.ts b/packages/firestore/test/unit/remote/datastore.test.ts index 45e0dbf8843..fef65d2421a 100644 --- a/packages/firestore/test/unit/remote/datastore.test.ts +++ b/packages/firestore/test/unit/remote/datastore.test.ts @@ -18,7 +18,11 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; -import { EmptyCredentialsProvider, Token } from '../../../src/api/credentials'; +import { + EmptyAppCheckTokenProvider, + EmptyCredentialsProvider, + Token +} from '../../../src/api/credentials'; import { DatabaseId } from '../../../src/core/database_info'; import { Connection, Stream } from '../../../src/remote/connection'; import { @@ -95,6 +99,7 @@ describe('Datastore', () => { it('newDatastore() returns an an instance of Datastore', () => { const datastore = newDatastore( new EmptyCredentialsProvider(), + new EmptyAppCheckTokenProvider(), new MockConnection(), serializer ); @@ -104,6 +109,7 @@ describe('Datastore', () => { it('DatastoreImpl.invokeRPC() fails if terminated', async () => { const datastore = newDatastore( new EmptyCredentialsProvider(), + new EmptyAppCheckTokenProvider(), new MockConnection(), serializer ); @@ -120,49 +126,68 @@ describe('Datastore', () => { const connection = new MockConnection(); connection.invokeRPC = () => Promise.reject(new FirestoreError(Code.ABORTED, 'zzyzx')); - const credentials = new MockCredentialsProvider(); - const datastore = newDatastore(credentials, connection, serializer); + const authCredentials = new MockCredentialsProvider(); + const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const datastore = newDatastore( + authCredentials, + appCheckCredentials, + connection, + serializer + ); await expect(invokeDatastoreImplInvokeRpc(datastore)) .to.eventually.be.rejectedWith('zzyzx') .and.include({ 'name': 'FirebaseError', 'code': Code.ABORTED }); - expect(credentials.invalidateTokenInvoked).to.be.false; + expect(authCredentials.invalidateTokenInvoked).to.be.false; }); it('DatastoreImpl.invokeRPC() wraps unknown exceptions in a FirestoreError', async () => { const connection = new MockConnection(); connection.invokeRPC = () => Promise.reject('zzyzx'); - const credentials = new MockCredentialsProvider(); - const datastore = newDatastore(credentials, connection, serializer); + const authCredentials = new MockCredentialsProvider(); + const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const datastore = newDatastore( + authCredentials, + appCheckCredentials, + connection, + serializer + ); await expect(invokeDatastoreImplInvokeRpc(datastore)) .to.eventually.be.rejectedWith('zzyzx') .and.include({ 'name': 'FirebaseError', 'code': Code.UNKNOWN }); - expect(credentials.invalidateTokenInvoked).to.be.false; + expect(authCredentials.invalidateTokenInvoked).to.be.false; }); it('DatastoreImpl.invokeRPC() invalidates the token if unauthenticated', async () => { const connection = new MockConnection(); connection.invokeRPC = () => Promise.reject(new FirestoreError(Code.UNAUTHENTICATED, 'zzyzx')); - const credentials = new MockCredentialsProvider(); - const datastore = newDatastore(credentials, connection, serializer); + const authCredentials = new MockCredentialsProvider(); + const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const datastore = newDatastore( + authCredentials, + appCheckCredentials, + connection, + serializer + ); await expect(invokeDatastoreImplInvokeRpc(datastore)) .to.eventually.be.rejectedWith('zzyzx') .and.include({ 'name': 'FirebaseError', 'code': Code.UNAUTHENTICATED }); - expect(credentials.invalidateTokenInvoked).to.be.true; + expect(authCredentials.invalidateTokenInvoked).to.be.true; }); it('DatastoreImpl.invokeStreamingRPC() fails if terminated', async () => { const datastore = newDatastore( new EmptyCredentialsProvider(), + new EmptyAppCheckTokenProvider(), new MockConnection(), serializer ); @@ -179,43 +204,61 @@ describe('Datastore', () => { const connection = new MockConnection(); connection.invokeStreamingRPC = () => Promise.reject(new FirestoreError(Code.ABORTED, 'zzyzx')); - const credentials = new MockCredentialsProvider(); - const datastore = newDatastore(credentials, connection, serializer); + const authCredentials = new MockCredentialsProvider(); + const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const datastore = newDatastore( + authCredentials, + appCheckCredentials, + connection, + serializer + ); await expect(invokeDatastoreImplInvokeStreamingRPC(datastore)) .to.eventually.be.rejectedWith('zzyzx') .and.include({ 'name': 'FirebaseError', 'code': Code.ABORTED }); - expect(credentials.invalidateTokenInvoked).to.be.false; + expect(authCredentials.invalidateTokenInvoked).to.be.false; }); it('DatastoreImpl.invokeStreamingRPC() wraps unknown exceptions in a FirestoreError', async () => { const connection = new MockConnection(); connection.invokeStreamingRPC = () => Promise.reject('zzyzx'); - const credentials = new MockCredentialsProvider(); - const datastore = newDatastore(credentials, connection, serializer); + const authCredentials = new MockCredentialsProvider(); + const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const datastore = newDatastore( + authCredentials, + appCheckCredentials, + connection, + serializer + ); await expect(invokeDatastoreImplInvokeStreamingRPC(datastore)) .to.eventually.be.rejectedWith('zzyzx') .and.include({ 'name': 'FirebaseError', 'code': Code.UNKNOWN }); - expect(credentials.invalidateTokenInvoked).to.be.false; + expect(authCredentials.invalidateTokenInvoked).to.be.false; }); it('DatastoreImpl.invokeStreamingRPC() invalidates the token if unauthenticated', async () => { const connection = new MockConnection(); connection.invokeStreamingRPC = () => Promise.reject(new FirestoreError(Code.UNAUTHENTICATED, 'zzyzx')); - const credentials = new MockCredentialsProvider(); - const datastore = newDatastore(credentials, connection, serializer); + const authCredentials = new MockCredentialsProvider(); + const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const datastore = newDatastore( + authCredentials, + appCheckCredentials, + connection, + serializer + ); await expect(invokeDatastoreImplInvokeStreamingRPC(datastore)) .to.eventually.be.rejectedWith('zzyzx') .and.include({ 'name': 'FirebaseError', 'code': Code.UNAUTHENTICATED }); - expect(credentials.invalidateTokenInvoked).to.be.true; + expect(authCredentials.invalidateTokenInvoked).to.be.true; }); }); diff --git a/packages/firestore/test/unit/specs/spec_test_components.ts b/packages/firestore/test/unit/specs/spec_test_components.ts index e5fe4fa3743..d37407a5850 100644 --- a/packages/firestore/test/unit/specs/spec_test_components.ts +++ b/packages/firestore/test/unit/specs/spec_test_components.ts @@ -135,7 +135,12 @@ export class MockOnlineComponentProvider extends OnlineComponentProvider { cfg.databaseInfo.databaseId, /* useProto3Json= */ true ); - return newDatastore(cfg.credentials, this.connection, serializer); + return newDatastore( + cfg.authCredentials, + cfg.appCheckCredentials, + this.connection, + serializer + ); } } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 3ba157871dc..431e8a88f6f 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -18,7 +18,7 @@ import { expect } from 'chai'; import { LoadBundleTask } from '../../../src/api/bundle'; -import { EmptyCredentialsProvider } from '../../../src/api/credentials'; +import {EmptyAppCheckTokenProvider, EmptyCredentialsProvider} from '../../../src/api/credentials'; import { User } from '../../../src/auth/user'; import { ComponentConfiguration } from '../../../src/core/component_provider'; import { DatabaseInfo } from '../../../src/core/database_info'; @@ -286,7 +286,8 @@ abstract class TestRunner { const configuration = { asyncQueue: this.queue, databaseInfo: this.databaseInfo, - credentials: new EmptyCredentialsProvider(), + authCredentials: new EmptyCredentialsProvider(), + appCheckCredentials: new EmptyAppCheckTokenProvider(), clientId: this.clientId, initialUser: this.user, maxConcurrentLimboResolutions: diff --git a/packages/firestore/test/util/api_helpers.ts b/packages/firestore/test/util/api_helpers.ts index 432f3fe4666..b44c7d5c142 100644 --- a/packages/firestore/test/util/api_helpers.ts +++ b/packages/firestore/test/util/api_helpers.ts @@ -28,7 +28,10 @@ import { QuerySnapshot, UserDataWriter } from '../../compat/api/database'; -import { EmptyCredentialsProvider } from '../../src/api/credentials'; +import { + EmptyAppCheckTokenProvider, + EmptyCredentialsProvider +} from '../../src/api/credentials'; import { ensureFirestoreConfigured, Firestore as ExpFirestore @@ -69,7 +72,11 @@ export function firestore(): Firestore { export function newTestFirestore(projectId = 'new-project'): Firestore { return new Firestore( new DatabaseId(projectId), - new ExpFirestore(new DatabaseId(projectId), new EmptyCredentialsProvider()), + new ExpFirestore( + new DatabaseId(projectId), + new EmptyCredentialsProvider(), + new EmptyAppCheckTokenProvider() + ), new IndexedDbPersistenceProvider() ); } From 601136cbfbdc55a1975407093e49dd6710d6b356 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Sun, 17 Oct 2021 21:36:26 -0500 Subject: [PATCH 02/25] Update datastore and connection. --- .../src/platform/node/grpc_connection.ts | 33 +++-- packages/firestore/src/remote/connection.ts | 9 +- packages/firestore/src/remote/datastore.ts | 128 +++++++++++++----- .../firestore/src/remote/persistent_stream.ts | 31 +++-- .../firestore/src/remote/rest_connection.ts | 32 +++-- .../test/unit/specs/spec_test_runner.ts | 5 +- 6 files changed, 171 insertions(+), 67 deletions(-) diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index 613d4e3f8cb..cd22358b8bf 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/packages/firestore/src/platform/node/grpc_connection.ts @@ -49,19 +49,22 @@ const X_GOOG_API_CLIENT_VALUE = `gl-node/${process.versions.node} fire/${SDK_VER function createMetadata( databasePath: string, - token: Token | null, + authToken: Token | null, + appCheckToken: Token | null, appId: string ): Metadata { hardAssert( - token === null || token.type === 'OAuth', + authToken === null || authToken.type === 'OAuth', 'If provided, token must be OAuth' ); const metadata = new Metadata(); - if (token) { - for (const header in token.authHeaders) { - if (token.authHeaders.hasOwnProperty(header)) { - metadata.set(header, token.authHeaders[header]); + for (let token of [authToken, appCheckToken]) { + if (token) { + for (const header in token.authHeaders) { + if (token.authHeaders.hasOwnProperty(header)) { + metadata.set(header, token.authHeaders[header]); + } } } } @@ -115,12 +118,14 @@ export class GrpcConnection implements Connection { rpcName: string, path: string, request: Req, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Promise { const stub = this.ensureActiveStub(); const metadata = createMetadata( this.databasePath, - token, + authToken, + appCheckToken, this.databaseInfo.appId ); const jsonRequest = { database: this.databasePath, ...request }; @@ -156,7 +161,8 @@ export class GrpcConnection implements Connection { rpcName: string, path: string, request: Req, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Promise { const results: Resp[] = []; const responseDeferred = new Deferred(); @@ -169,7 +175,8 @@ export class GrpcConnection implements Connection { const stub = this.ensureActiveStub(); const metadata = createMetadata( this.databasePath, - token, + authToken, + appCheckToken, this.databaseInfo.appId ); const jsonRequest = { ...request, database: this.databasePath }; @@ -194,12 +201,14 @@ export class GrpcConnection implements Connection { // TODO(mikelehen): This "method" is a monster. Should be refactored. openStream( rpcName: string, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Stream { const stub = this.ensureActiveStub(); const metadata = createMetadata( this.databasePath, - token, + authToken, + appCheckToken, this.databaseInfo.appId ); const grpcStream = stub[rpcName](metadata); diff --git a/packages/firestore/src/remote/connection.ts b/packages/firestore/src/remote/connection.ts index 2aa981c0a5d..1a88c5c48c8 100644 --- a/packages/firestore/src/remote/connection.ts +++ b/packages/firestore/src/remote/connection.ts @@ -47,7 +47,8 @@ export interface Connection { rpcName: string, path: string, request: Req, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Promise; /** @@ -66,7 +67,8 @@ export interface Connection { rpcName: string, path: string, request: Req, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Promise; /** @@ -77,7 +79,8 @@ export interface Connection { */ openStream( rpcName: string, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Stream; // TODO(mcg): subscribe to connection state changes. diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index 0e3dd302354..ded45667847 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { CredentialsProvider } from '../api/credentials'; +import { CredentialsProvider, Token } from '../api/credentials'; import { Query, queryToTarget } from '../core/query'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; @@ -83,27 +83,40 @@ class DatastoreImpl extends Datastore { } } - /** Gets an auth token and invokes the provided RPC. */ - invokeRPC( + /** Gets an auth token and AppCheck token and invokes the provided RPC. */ + invokeWithTokens( rpcName: string, path: string, - request: Req - ): Promise { + request: Req, + func: ( + rpcName: string, + path: string, + request: Req, + authToken: Token | null, + appCheckToken: Token | null + ) => Promise + ): Promise { this.verifyInitialized(); - return this.authCredentials - .getToken() - .then(token => { - return this.connection.invokeRPC( + return Promise.all([ + this.authCredentials.getToken(), + this.appCheckCredentials.getToken() + ]) + .then(values => { + const authToken = values[0]; + const appCheckToken = values[1]; + return func( rpcName, path, request, - token + authToken, + appCheckToken ); }) .catch((error: FirestoreError) => { if (error.name === 'FirebaseError') { if (error.code === Code.UNAUTHENTICATED) { this.authCredentials.invalidateToken(); + this.appCheckCredentials.invalidateToken(); } throw error; } else { @@ -112,33 +125,86 @@ class DatastoreImpl extends Datastore { }); } + /** Gets an auth token and invokes the provided RPC. */ + invokeRPC( + rpcName: string, + path: string, + request: Req + ): Promise { + return this.invokeWithTokens( + rpcName, + path, + request, + this.connection.invokeRPC + ); + // this.verifyInitialized(); + // return Promise.all([ + // this.authCredentials.getToken(), + // this.appCheckCredentials.getToken() + // ]) + // .then(values => { + // const authToken = values[0]; + // const appCheckToken = values[1]; + // return this.connection.invokeRPC( + // rpcName, + // path, + // request, + // authToken, + // appCheckToken + // ); + // }) + // .catch((error: FirestoreError) => { + // if (error.name === 'FirebaseError') { + // if (error.code === Code.UNAUTHENTICATED) { + // this.authCredentials.invalidateToken(); + // this.appCheckCredentials.invalidateToken(); + // } + // throw error; + // } else { + // throw new FirestoreError(Code.UNKNOWN, error.toString()); + // } + // }); + } + /** Gets an auth token and invokes the provided RPC with streamed results. */ invokeStreamingRPC( rpcName: string, path: string, request: Req ): Promise { - this.verifyInitialized(); - return this.authCredentials - .getToken() - .then(token => { - return this.connection.invokeStreamingRPC( - rpcName, - path, - request, - token - ); - }) - .catch((error: FirestoreError) => { - if (error.name === 'FirebaseError') { - if (error.code === Code.UNAUTHENTICATED) { - this.authCredentials.invalidateToken(); - } - throw error; - } else { - throw new FirestoreError(Code.UNKNOWN, error.toString()); - } - }); + return this.invokeWithTokens( + rpcName, + path, + request, + this.connection.invokeStreamingRPC + ); + // this.verifyInitialized(); + // return Promise.all([ + // this.authCredentials.getToken(), + // this.appCheckCredentials.getToken() + // ]) + // .then(values => { + // const authToken = values[0]; + // const appCheckToken = values[1]; + // return this.connection.invokeStreamingRPC( + // rpcName, + // path, + // request, + // authToken, + // appCheckToken + // ); + // }) + // .catch((error: FirestoreError) => { + // if (error.name === 'FirebaseError') { + // if (error.code === Code.UNAUTHENTICATED) { + // this.authCredentials.invalidateToken(); + // this.appCheckCredentials.invalidateToken(); + // } + // throw error; + // } else { + // throw new FirestoreError(Code.UNKNOWN, error.toString()); + // } + // }); } terminate(): void { diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index c2716f6f40d..6b2e3a68522 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -419,7 +419,8 @@ export abstract class PersistentStream< * connection stream. */ protected abstract startRpc( - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Stream; /** @@ -442,8 +443,11 @@ export abstract class PersistentStream< // TODO(mikelehen): Just use dispatchIfNotClosed, but see TODO below. const closeCount = this.closeCount; - this.authCredentialsProvider.getToken().then( - token => { + Promise.all([ + this.authCredentialsProvider.getToken(), + this.appCheckCredentialsProvider.getToken() + ]).then( + tokens => { // Stream can be stopped while waiting for authentication. // TODO(mikelehen): We really should just use dispatchIfNotClosed // and let this dispatch onto the queue, but that opened a spec test can @@ -452,7 +456,7 @@ export abstract class PersistentStream< // Normally we'd have to schedule the callback on the AsyncQueue. // However, the following calls are safe to be called outside the // AsyncQueue since they don't chain asynchronous calls - this.startStream(token); + this.startStream(tokens[0], tokens[1]); } }, (error: Error) => { @@ -467,7 +471,10 @@ export abstract class PersistentStream< ); } - private startStream(token: Token | null): void { + private startStream( + authToken: Token | null, + appCheckToken: Token | null + ): void { debugAssert( this.state === PersistentStreamState.Starting, 'Trying to start stream in a non-starting state' @@ -475,7 +482,7 @@ export abstract class PersistentStream< const dispatchIfNotClosed = this.getCloseGuardedDispatcher(this.closeCount); - this.stream = this.startRpc(token); + this.stream = this.startRpc(authToken, appCheckToken); this.stream.onOpen(() => { dispatchIfNotClosed(() => { debugAssert( @@ -618,11 +625,13 @@ export class PersistentListenStream extends PersistentStream< } protected startRpc( - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Stream { return this.connection.openStream( 'Listen', - token + authToken, + appCheckToken ); } @@ -760,11 +769,13 @@ export class PersistentWriteStream extends PersistentStream< } protected startRpc( - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Stream { return this.connection.openStream( 'Write', - token + authToken, + appCheckToken ); } diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index 2f4b75e192b..2c8723d8c71 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/packages/firestore/src/remote/rest_connection.ts @@ -70,13 +70,14 @@ export abstract class RestConnection implements Connection { rpcName: string, path: string, req: Req, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Promise { const url = this.makeUrl(rpcName, path); logDebug(LOG_TAG, 'Sending: ', url, req); const headers = {}; - this.modifyHeadersForRequest(headers, token); + this.modifyHeadersForRequest(headers, authToken, appCheckToken); return this.performRPCRequest(rpcName, url, headers, req).then( response => { @@ -102,16 +103,24 @@ export abstract class RestConnection implements Connection { rpcName: string, path: string, request: Req, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Promise { // The REST API automatically aggregates all of the streamed results, so we // can just use the normal invoke() method. - return this.invokeRPC(rpcName, path, request, token); + return this.invokeRPC( + rpcName, + path, + request, + authToken, + appCheckToken + ); } abstract openStream( rpcName: string, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Stream; /** @@ -120,7 +129,8 @@ export abstract class RestConnection implements Connection { */ protected modifyHeadersForRequest( headers: StringMap, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): void { headers['X-Goog-Api-Client'] = getGoogApiClientValue(); @@ -133,10 +143,12 @@ export abstract class RestConnection implements Connection { if (this.databaseInfo.appId) { headers['X-Firebase-GMPID'] = this.databaseInfo.appId; } - if (token) { - for (const header in token.authHeaders) { - if (token.authHeaders.hasOwnProperty(header)) { - headers[header] = token.authHeaders[header]; + for (let token of [authToken, appCheckToken]) { + if (token) { + for (const header in token.authHeaders) { + if (token.authHeaders.hasOwnProperty(header)) { + headers[header] = token.authHeaders[header]; + } } } } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 431e8a88f6f..fdb8d88b55c 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -18,7 +18,10 @@ import { expect } from 'chai'; import { LoadBundleTask } from '../../../src/api/bundle'; -import {EmptyAppCheckTokenProvider, EmptyCredentialsProvider} from '../../../src/api/credentials'; +import { + EmptyAppCheckTokenProvider, + EmptyCredentialsProvider +} from '../../../src/api/credentials'; import { User } from '../../../src/auth/user'; import { ComponentConfiguration } from '../../../src/core/component_provider'; import { DatabaseInfo } from '../../../src/core/database_info'; From 52a753a09e99be50d56454ecf480458f6299ae85 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 18 Oct 2021 14:50:43 -0500 Subject: [PATCH 03/25] Better invokeWithTokens function. --- packages/firestore/src/remote/datastore.ts | 103 ++++----------------- 1 file changed, 20 insertions(+), 83 deletions(-) diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index ded45667847..9975b4f7ea9 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -83,18 +83,9 @@ class DatastoreImpl extends Datastore { } } - /** Gets an auth token and AppCheck token and invokes the provided RPC. */ - invokeWithTokens( - rpcName: string, - path: string, - request: Req, - func: ( - rpcName: string, - path: string, - request: Req, - authToken: Token | null, - appCheckToken: Token | null - ) => Promise + /** Gets an auth token and AppCheck token and invokes the provided callback. */ + invokeWithTokens( + func: (authToken: Token | null, appCheckToken: Token | null) => Promise ): Promise { this.verifyInitialized(); return Promise.all([ @@ -104,13 +95,7 @@ class DatastoreImpl extends Datastore { .then(values => { const authToken = values[0]; const appCheckToken = values[1]; - return func( - rpcName, - path, - request, - authToken, - appCheckToken - ); + return func(authToken, appCheckToken); }) .catch((error: FirestoreError) => { if (error.name === 'FirebaseError') { @@ -131,39 +116,15 @@ class DatastoreImpl extends Datastore { path: string, request: Req ): Promise { - return this.invokeWithTokens( - rpcName, - path, - request, - this.connection.invokeRPC + return this.invokeWithTokens((authToken, appCheckToken) => + this.connection.invokeRPC( + rpcName, + path, + request, + authToken, + appCheckToken + ) ); - // this.verifyInitialized(); - // return Promise.all([ - // this.authCredentials.getToken(), - // this.appCheckCredentials.getToken() - // ]) - // .then(values => { - // const authToken = values[0]; - // const appCheckToken = values[1]; - // return this.connection.invokeRPC( - // rpcName, - // path, - // request, - // authToken, - // appCheckToken - // ); - // }) - // .catch((error: FirestoreError) => { - // if (error.name === 'FirebaseError') { - // if (error.code === Code.UNAUTHENTICATED) { - // this.authCredentials.invalidateToken(); - // this.appCheckCredentials.invalidateToken(); - // } - // throw error; - // } else { - // throw new FirestoreError(Code.UNKNOWN, error.toString()); - // } - // }); } /** Gets an auth token and invokes the provided RPC with streamed results. */ @@ -172,39 +133,15 @@ class DatastoreImpl extends Datastore { path: string, request: Req ): Promise { - return this.invokeWithTokens( - rpcName, - path, - request, - this.connection.invokeStreamingRPC + return this.invokeWithTokens((authToken, appCheckToken) => + this.connection.invokeStreamingRPC( + rpcName, + path, + request, + authToken, + appCheckToken + ) ); - // this.verifyInitialized(); - // return Promise.all([ - // this.authCredentials.getToken(), - // this.appCheckCredentials.getToken() - // ]) - // .then(values => { - // const authToken = values[0]; - // const appCheckToken = values[1]; - // return this.connection.invokeStreamingRPC( - // rpcName, - // path, - // request, - // authToken, - // appCheckToken - // ); - // }) - // .catch((error: FirestoreError) => { - // if (error.name === 'FirebaseError') { - // if (error.code === Code.UNAUTHENTICATED) { - // this.authCredentials.invalidateToken(); - // this.appCheckCredentials.invalidateToken(); - // } - // throw error; - // } else { - // throw new FirestoreError(Code.UNKNOWN, error.toString()); - // } - // }); } terminate(): void { From 4d576e875ef26a16c8676151eb83b0f2f50fd3a9 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 18 Oct 2021 16:00:38 -0500 Subject: [PATCH 04/25] Update/add unit tests. --- .../test/unit/remote/datastore.test.ts | 25 ++++++++++++++----- .../test/unit/remote/rest_connection.test.ts | 14 +++++++++-- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/firestore/test/unit/remote/datastore.test.ts b/packages/firestore/test/unit/remote/datastore.test.ts index fef65d2421a..40215099a0c 100644 --- a/packages/firestore/test/unit/remote/datastore.test.ts +++ b/packages/firestore/test/unit/remote/datastore.test.ts @@ -74,6 +74,13 @@ describe('Datastore', () => { } } + class MockAppCheckTokenProvider extends EmptyAppCheckTokenProvider { + invalidateTokenInvoked = false; + invalidateToken(): void { + this.invalidateTokenInvoked = true; + } + } + const serializer = new JsonProtoSerializer( new DatabaseId('test-project'), /* useProto3Json= */ false @@ -127,7 +134,7 @@ describe('Datastore', () => { connection.invokeRPC = () => Promise.reject(new FirestoreError(Code.ABORTED, 'zzyzx')); const authCredentials = new MockCredentialsProvider(); - const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, appCheckCredentials, @@ -141,13 +148,14 @@ describe('Datastore', () => { 'code': Code.ABORTED }); expect(authCredentials.invalidateTokenInvoked).to.be.false; + expect(appCheckCredentials.invalidateTokenInvoked).to.be.false; }); it('DatastoreImpl.invokeRPC() wraps unknown exceptions in a FirestoreError', async () => { const connection = new MockConnection(); connection.invokeRPC = () => Promise.reject('zzyzx'); const authCredentials = new MockCredentialsProvider(); - const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, appCheckCredentials, @@ -161,6 +169,7 @@ describe('Datastore', () => { 'code': Code.UNKNOWN }); expect(authCredentials.invalidateTokenInvoked).to.be.false; + expect(appCheckCredentials.invalidateTokenInvoked).to.be.false; }); it('DatastoreImpl.invokeRPC() invalidates the token if unauthenticated', async () => { @@ -168,7 +177,7 @@ describe('Datastore', () => { connection.invokeRPC = () => Promise.reject(new FirestoreError(Code.UNAUTHENTICATED, 'zzyzx')); const authCredentials = new MockCredentialsProvider(); - const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, appCheckCredentials, @@ -182,6 +191,7 @@ describe('Datastore', () => { 'code': Code.UNAUTHENTICATED }); expect(authCredentials.invalidateTokenInvoked).to.be.true; + expect(appCheckCredentials.invalidateTokenInvoked).to.be.true; }); it('DatastoreImpl.invokeStreamingRPC() fails if terminated', async () => { @@ -205,7 +215,7 @@ describe('Datastore', () => { connection.invokeStreamingRPC = () => Promise.reject(new FirestoreError(Code.ABORTED, 'zzyzx')); const authCredentials = new MockCredentialsProvider(); - const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, appCheckCredentials, @@ -219,13 +229,14 @@ describe('Datastore', () => { 'code': Code.ABORTED }); expect(authCredentials.invalidateTokenInvoked).to.be.false; + expect(appCheckCredentials.invalidateTokenInvoked).to.be.false; }); it('DatastoreImpl.invokeStreamingRPC() wraps unknown exceptions in a FirestoreError', async () => { const connection = new MockConnection(); connection.invokeStreamingRPC = () => Promise.reject('zzyzx'); const authCredentials = new MockCredentialsProvider(); - const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, appCheckCredentials, @@ -239,6 +250,7 @@ describe('Datastore', () => { 'code': Code.UNKNOWN }); expect(authCredentials.invalidateTokenInvoked).to.be.false; + expect(appCheckCredentials.invalidateTokenInvoked).to.be.false; }); it('DatastoreImpl.invokeStreamingRPC() invalidates the token if unauthenticated', async () => { @@ -246,7 +258,7 @@ describe('Datastore', () => { connection.invokeStreamingRPC = () => Promise.reject(new FirestoreError(Code.UNAUTHENTICATED, 'zzyzx')); const authCredentials = new MockCredentialsProvider(); - const appCheckCredentials = new EmptyAppCheckTokenProvider(); + const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, appCheckCredentials, @@ -260,5 +272,6 @@ describe('Datastore', () => { 'code': Code.UNAUTHENTICATED }); expect(authCredentials.invalidateTokenInvoked).to.be.true; + expect(appCheckCredentials.invalidateTokenInvoked).to.be.true; }); }); diff --git a/packages/firestore/test/unit/remote/rest_connection.test.ts b/packages/firestore/test/unit/remote/rest_connection.test.ts index 626ac35ac1e..ee8229bcb7d 100644 --- a/packages/firestore/test/unit/remote/rest_connection.test.ts +++ b/packages/firestore/test/unit/remote/rest_connection.test.ts @@ -35,7 +35,8 @@ export class TestRestConnection extends RestConnection { openStream( rpcName: string, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Stream { throw new Error('Not Implemented'); } @@ -73,6 +74,7 @@ describe('RestConnection', () => { 'Commit', 'projects/testproject/databases/(default)/documents', {}, + null, null ); expect(connection.lastUrl).to.equal( @@ -89,13 +91,19 @@ describe('RestConnection', () => { user: User.UNAUTHENTICATED, type: 'OAuth', authHeaders: { 'Authorization': 'Bearer owner' } + }, + { + user: null as any, + type: 'AppCheck', + authHeaders: { 'x-firebase-appcheck': 'some-app-check-token' } } ); expect(connection.lastHeaders).to.deep.equal({ 'Authorization': 'Bearer owner', 'Content-Type': 'text/plain', 'X-Firebase-GMPID': 'test-app-id', - 'X-Goog-Api-Client': `gl-js/ fire/${SDK_VERSION}` + 'X-Goog-Api-Client': `gl-js/ fire/${SDK_VERSION}`, + 'x-firebase-appcheck': 'some-app-check-token' }); }); @@ -105,6 +113,7 @@ describe('RestConnection', () => { 'RunQuery', 'projects/testproject/databases/(default)/documents/coll', {}, + null, null ); expect(response).to.deep.equal({ response: true }); @@ -118,6 +127,7 @@ describe('RestConnection', () => { 'RunQuery', 'projects/testproject/databases/(default)/documents/coll', {}, + null, null ) ).to.be.eventually.rejectedWith(error); From 090304dfdb8a6a242b49dd79fa0ffc1c88a0a639 Mon Sep 17 00:00:00 2001 From: Ehsan Date: Mon, 18 Oct 2021 16:14:09 -0500 Subject: [PATCH 05/25] Create mean-elephants-rush.md --- .changeset/mean-elephants-rush.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/mean-elephants-rush.md diff --git a/.changeset/mean-elephants-rush.md b/.changeset/mean-elephants-rush.md new file mode 100644 index 00000000000..807256ba377 --- /dev/null +++ b/.changeset/mean-elephants-rush.md @@ -0,0 +1,5 @@ +--- +"@firebase/firestore": patch +--- + +AppCheck integration for Firestore From 8e54e9510fdde2d7e67a117173d1ac4b083fb05b Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 18 Oct 2021 16:30:20 -0500 Subject: [PATCH 06/25] Fix lint issues. --- packages/firestore/src/api/credentials.ts | 19 +++++++++---------- packages/firestore/src/api/database.ts | 4 ++-- .../firestore/src/core/component_provider.ts | 2 +- .../firestore/src/core/firestore_client.ts | 2 +- packages/firestore/src/lite-api/components.ts | 4 ++-- packages/firestore/src/lite-api/database.ts | 3 +-- .../src/platform/node/grpc_connection.ts | 2 +- packages/firestore/src/remote/datastore.ts | 6 +++--- .../firestore/src/remote/persistent_stream.ts | 8 ++++---- .../firestore/src/remote/rest_connection.ts | 2 +- .../test/integration/util/internal_helpers.ts | 2 +- .../test/unit/remote/rest_connection.test.ts | 2 +- 12 files changed, 27 insertions(+), 29 deletions(-) diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index e2e35484d17..0bd19e967f6 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -15,17 +15,16 @@ * limitations under the License. */ -import { - FirebaseAuthInternal, - FirebaseAuthInternalName -} from '@firebase/auth-interop-types'; - import { AppCheckInternalComponentName, AppCheckTokenListener, AppCheckTokenResult, FirebaseAppCheckInternal } from '@firebase/app-check-interop-types'; +import { + FirebaseAuthInternal, + FirebaseAuthInternalName +} from '@firebase/auth-interop-types'; import { Provider } from '@firebase/component'; import { User } from '../auth/user'; @@ -451,7 +450,7 @@ export class FirstPartyCredentialsProvider export class AppCheckToken implements Token { type = 'AppCheck' as TokenType; - user = null as any; + user = new User(null); authHeaders: { [header: string]: string }; constructor(value: string) { @@ -462,7 +461,7 @@ export class AppCheckToken implements Token { } export class FirebaseAppCheckTokenProvider - implements CredentialsProvider + implements CredentialsProvider { /** * The AppCheck token listener registered with FirebaseApp, retained here so @@ -480,7 +479,7 @@ export class FirebaseAppCheckTokenProvider start( asyncQueue: AsyncQueue, - changeListener: CredentialChangeListener + changeListener: CredentialChangeListener ): void { const onTokenChanged: (tokenResult: AppCheckTokenResult) => Promise = tokenResult => { @@ -564,7 +563,7 @@ export class FirebaseAppCheckTokenProvider * An AppCheck token provider that always yields an empty token. * @internal */ -export class EmptyAppCheckTokenProvider implements CredentialsProvider { +export class EmptyAppCheckTokenProvider implements CredentialsProvider { getToken(): Promise { return Promise.resolve(new AppCheckToken('')); } @@ -573,7 +572,7 @@ export class EmptyAppCheckTokenProvider implements CredentialsProvider { start( asyncQueue: AsyncQueue, - changeListener: CredentialChangeListener + changeListener: CredentialChangeListener ): void {} shutdown(): void {} diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 36225eebc8d..0dee34d1c2c 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -24,6 +24,7 @@ import { } from '@firebase/app'; import { deepEqual } from '@firebase/util'; +import { User } from '../auth/user'; import { IndexedDbOfflineComponentProvider, MultiTabOfflineComponentProvider, @@ -60,7 +61,6 @@ import { Deferred } from '../util/promise'; import { LoadBundleTask } from './bundle'; import { CredentialsProvider } from './credentials'; import { PersistenceSettings, FirestoreSettings } from './settings'; -import { User } from '../auth/user'; export { connectFirestoreEmulator, EmulatorMockTokenOptions @@ -104,7 +104,7 @@ export class Firestore extends LiteFirestore { constructor( databaseIdOrApp: DatabaseId | FirebaseApp, authCredentialsProvider: CredentialsProvider, - appCheckCredentialsProvider: CredentialsProvider + appCheckCredentialsProvider: CredentialsProvider ) { super( databaseIdOrApp, diff --git a/packages/firestore/src/core/component_provider.ts b/packages/firestore/src/core/component_provider.ts index 87ffaac3710..1cf6a51db2b 100644 --- a/packages/firestore/src/core/component_provider.ts +++ b/packages/firestore/src/core/component_provider.ts @@ -76,7 +76,7 @@ export interface ComponentConfiguration { asyncQueue: AsyncQueue; databaseInfo: DatabaseInfo; authCredentials: CredentialsProvider; - appCheckCredentials: CredentialsProvider; + appCheckCredentials: CredentialsProvider; clientId: ClientId; initialUser: User; maxConcurrentLimboResolutions: number; diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index c333744e3a7..827fe65e538 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -105,7 +105,7 @@ export class FirestoreClient { constructor( private authCredentials: CredentialsProvider, - private appCheckCredentials: CredentialsProvider, + private appCheckCredentials: CredentialsProvider, /** * Asynchronous queue responsible for all of our internal processing. When * we get incoming work from the user (via public API) or the network diff --git a/packages/firestore/src/lite-api/components.ts b/packages/firestore/src/lite-api/components.ts index e20ca5e0ff7..8280d396b0b 100644 --- a/packages/firestore/src/lite-api/components.ts +++ b/packages/firestore/src/lite-api/components.ts @@ -19,6 +19,7 @@ import { _FirebaseService } from '@firebase/app'; import { CredentialsProvider } from '../api/credentials'; +import { User } from '../auth/user'; import { DatabaseId, DatabaseInfo } from '../core/database_info'; import { newConnection } from '../platform/connection'; import { newSerializer } from '../platform/serializer'; @@ -27,7 +28,6 @@ import { Code, FirestoreError } from '../util/error'; import { logDebug } from '../util/log'; import { FirestoreSettingsImpl } from './settings'; -import { User } from '../auth/user'; export const LOG_TAG = 'ComponentProvider'; @@ -43,7 +43,7 @@ export const LOG_TAG = 'ComponentProvider'; */ export interface FirestoreService extends _FirebaseService { _authCredentials: CredentialsProvider; - _appCheckCredentials: CredentialsProvider; + _appCheckCredentials: CredentialsProvider; _persistenceKey: string; _databaseId: DatabaseId; _terminated: boolean; diff --git a/packages/firestore/src/lite-api/database.ts b/packages/firestore/src/lite-api/database.ts index 4784bca5b20..6d6fbdbad05 100644 --- a/packages/firestore/src/lite-api/database.ts +++ b/packages/firestore/src/lite-api/database.ts @@ -27,7 +27,6 @@ import { createMockUserToken, EmulatorMockTokenOptions } from '@firebase/util'; import { CredentialsProvider, EmulatorCredentialsProvider, - FirebaseAppCheckTokenProvider, makeCredentialsProvider, OAuthToken } from '../api/credentials'; @@ -78,7 +77,7 @@ export class Firestore implements FirestoreService { constructor( databaseIdOrApp: DatabaseId | FirebaseApp, public _authCredentials: CredentialsProvider, - public _appCheckCredentials: CredentialsProvider + public _appCheckCredentials: CredentialsProvider ) { if (databaseIdOrApp instanceof DatabaseId) { this._databaseId = databaseIdOrApp; diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index cd22358b8bf..9a8b62dba7c 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/packages/firestore/src/platform/node/grpc_connection.ts @@ -59,7 +59,7 @@ function createMetadata( ); const metadata = new Metadata(); - for (let token of [authToken, appCheckToken]) { + for (const token of [authToken, appCheckToken]) { if (token) { for (const header in token.authHeaders) { if (token.authHeaders.hasOwnProperty(header)) { diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index 9975b4f7ea9..8e781f0c4f8 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -16,6 +16,7 @@ */ import { CredentialsProvider, Token } from '../api/credentials'; +import { User } from '../auth/user'; import { Query, queryToTarget } from '../core/query'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; @@ -46,7 +47,6 @@ import { toName, toQueryTarget } from './serializer'; -import { User } from '../auth/user'; /** * Datastore and its related methods are a wrapper around the external Google @@ -66,7 +66,7 @@ class DatastoreImpl extends Datastore { constructor( readonly authCredentials: CredentialsProvider, - readonly appCheckCredentials: CredentialsProvider, + readonly appCheckCredentials: CredentialsProvider, readonly connection: Connection, readonly serializer: JsonProtoSerializer ) { @@ -153,7 +153,7 @@ class DatastoreImpl extends Datastore { // firestore-exp client. export function newDatastore( authCredentials: CredentialsProvider, - appCheckCredentials: CredentialsProvider, + appCheckCredentials: CredentialsProvider, connection: Connection, serializer: JsonProtoSerializer ): Datastore { diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index 6b2e3a68522..1a3f216bdb7 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -16,6 +16,7 @@ */ import { CredentialsProvider, Token } from '../api/credentials'; +import { User } from '../auth/user'; import { SnapshotVersion } from '../core/snapshot_version'; import { TargetId } from '../core/types'; import { TargetData } from '../local/target_data'; @@ -46,7 +47,6 @@ import { versionFromListenResponse } from './serializer'; import { WatchChange } from './watch_change'; -import { User } from '../auth/user'; const LOG_TAG = 'PersistentStream'; @@ -201,7 +201,7 @@ export abstract class PersistentStream< private healthTimerId: TimerId, protected connection: Connection, private authCredentialsProvider: CredentialsProvider, - private appCheckCredentialsProvider: CredentialsProvider, + private appCheckCredentialsProvider: CredentialsProvider, protected listener: ListenerType ) { this.backoff = new ExponentialBackoff(queue, connectionTimerId); @@ -608,7 +608,7 @@ export class PersistentListenStream extends PersistentStream< queue: AsyncQueue, connection: Connection, authCredentials: CredentialsProvider, - appCheckCredentials: CredentialsProvider, + appCheckCredentials: CredentialsProvider, private serializer: JsonProtoSerializer, listener: WatchStreamListener ) { @@ -721,7 +721,7 @@ export class PersistentWriteStream extends PersistentStream< queue: AsyncQueue, connection: Connection, authCredentials: CredentialsProvider, - appCheckCredentials: CredentialsProvider, + appCheckCredentials: CredentialsProvider, private serializer: JsonProtoSerializer, listener: WriteStreamListener ) { diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index 2c8723d8c71..7cdec73cb78 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/packages/firestore/src/remote/rest_connection.ts @@ -143,7 +143,7 @@ export abstract class RestConnection implements Connection { if (this.databaseInfo.appId) { headers['X-Firebase-GMPID'] = this.databaseInfo.appId; } - for (let token of [authToken, appCheckToken]) { + for (const token of [authToken, appCheckToken]) { if (token) { for (const header in token.authHeaders) { if (token.authHeaders.hasOwnProperty(header)) { diff --git a/packages/firestore/test/integration/util/internal_helpers.ts b/packages/firestore/test/integration/util/internal_helpers.ts index e5da7fb58e6..dbfa206a5b5 100644 --- a/packages/firestore/test/integration/util/internal_helpers.ts +++ b/packages/firestore/test/integration/util/internal_helpers.ts @@ -58,7 +58,7 @@ export function getDefaultDatabaseInfo(): DatabaseInfo { export function withTestDatastore( fn: (datastore: Datastore) => Promise, authCredentialsProvider: CredentialsProvider = new EmptyCredentialsProvider(), - appCheckTokenProvider: CredentialsProvider = new EmptyAppCheckTokenProvider() + appCheckTokenProvider: CredentialsProvider = new EmptyAppCheckTokenProvider() ): Promise { const databaseInfo = getDefaultDatabaseInfo(); const connection = newConnection(databaseInfo); diff --git a/packages/firestore/test/unit/remote/rest_connection.test.ts b/packages/firestore/test/unit/remote/rest_connection.test.ts index ee8229bcb7d..f81c6f02a13 100644 --- a/packages/firestore/test/unit/remote/rest_connection.test.ts +++ b/packages/firestore/test/unit/remote/rest_connection.test.ts @@ -93,7 +93,7 @@ describe('RestConnection', () => { authHeaders: { 'Authorization': 'Bearer owner' } }, { - user: null as any, + user: new User(null), type: 'AppCheck', authHeaders: { 'x-firebase-appcheck': 'some-app-check-token' } } From 75ed9ae6c0d7599905ccb93ad34446b5596fe8ce Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 18 Oct 2021 22:09:32 -0500 Subject: [PATCH 07/25] Update webchannel_connection too. --- .../firestore/src/platform/browser/webchannel_connection.ts | 5 +++-- .../firestore/test/integration/browser/webchannel.test.ts | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/platform/browser/webchannel_connection.ts b/packages/firestore/src/platform/browser/webchannel_connection.ts index 239ba6a44de..a6272c716f3 100644 --- a/packages/firestore/src/platform/browser/webchannel_connection.ts +++ b/packages/firestore/src/platform/browser/webchannel_connection.ts @@ -160,7 +160,8 @@ export class WebChannelConnection extends RestConnection { openStream( rpcName: string, - token: Token | null + authToken: Token | null, + appCheckToken: Token | null ): Stream { const urlParts = [ this.baseUrl, @@ -201,7 +202,7 @@ export class WebChannelConnection extends RestConnection { request.xmlHttpFactory = new FetchXmlHttpFactory({}); } - this.modifyHeadersForRequest(request.initMessageHeaders!, token); + this.modifyHeadersForRequest(request.initMessageHeaders!, authToken, appCheckToken); // Sending the custom headers we just added to request.initMessageHeaders // (Authorization, etc.) will trigger the browser to make a CORS preflight diff --git a/packages/firestore/test/integration/browser/webchannel.test.ts b/packages/firestore/test/integration/browser/webchannel.test.ts index 67656d4d34f..be181bf8dc9 100644 --- a/packages/firestore/test/integration/browser/webchannel.test.ts +++ b/packages/firestore/test/integration/browser/webchannel.test.ts @@ -39,6 +39,7 @@ describeFn('WebChannel', () => { const conn = new WebChannelConnection(info); const stream = conn.openStream( 'Listen', + null, null ); From 13976908056227c19b9eeb7c332f62a8368f479a Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 18 Oct 2021 22:32:12 -0500 Subject: [PATCH 08/25] Make 'user' optional. And better function name. --- packages/firestore/src/api/credentials.ts | 9 ++++----- packages/firestore/src/lite-api/database.ts | 4 ++-- packages/firestore/test/unit/api/database.test.ts | 2 +- .../firestore/test/unit/remote/rest_connection.test.ts | 1 - 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index 0bd19e967f6..e7d7416f76e 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -66,7 +66,7 @@ export interface Token { * state on disk, etc.). * This will be null for Tokens of the type 'AppCheck'. */ - user: User; + user?: User; /** Extra header values to be passed along with a request */ authHeaders: { [header: string]: string }; @@ -170,7 +170,7 @@ export class EmulatorCredentialsProvider implements CredentialsProvider { ); this.changeListener = changeListener; // Fire with initial user. - asyncQueue.enqueueRetryable(() => changeListener(this.token.user)); + asyncQueue.enqueueRetryable(() => changeListener(this.token.user!)); } shutdown(): void { @@ -450,7 +450,6 @@ export class FirstPartyCredentialsProvider export class AppCheckToken implements Token { type = 'AppCheck' as TokenType; - user = new User(null); authHeaders: { [header: string]: string }; constructor(value: string) { @@ -582,7 +581,7 @@ export class EmptyAppCheckTokenProvider implements CredentialsProvider { * Builds a CredentialsProvider depending on the type of * the credentials passed in. */ -export function makeCredentialsProvider( +export function makeAuthCredentialsProvider( credentials?: CredentialsSettings ): CredentialsProvider { if (!credentials) { @@ -614,7 +613,7 @@ export function makeCredentialsProvider( default: throw new FirestoreError( Code.INVALID_ARGUMENT, - 'makeCredentialsProvider failed due to invalid credential type' + 'makeAuthCredentialsProvider failed due to invalid credential type' ); } } diff --git a/packages/firestore/src/lite-api/database.ts b/packages/firestore/src/lite-api/database.ts index 6d6fbdbad05..cc3dd14263a 100644 --- a/packages/firestore/src/lite-api/database.ts +++ b/packages/firestore/src/lite-api/database.ts @@ -27,7 +27,7 @@ import { createMockUserToken, EmulatorMockTokenOptions } from '@firebase/util'; import { CredentialsProvider, EmulatorCredentialsProvider, - makeCredentialsProvider, + makeAuthCredentialsProvider, OAuthToken } from '../api/credentials'; import { User } from '../auth/user'; @@ -121,7 +121,7 @@ export class Firestore implements FirestoreService { } this._settings = new FirestoreSettingsImpl(settings); if (settings.credentials !== undefined) { - this._authCredentials = makeCredentialsProvider(settings.credentials); + this._authCredentials = makeAuthCredentialsProvider(settings.credentials); } } diff --git a/packages/firestore/test/unit/api/database.test.ts b/packages/firestore/test/unit/api/database.test.ts index a6ea0ce1257..db0d496ad6f 100644 --- a/packages/firestore/test/unit/api/database.test.ts +++ b/packages/firestore/test/unit/api/database.test.ts @@ -263,7 +263,7 @@ describe('Settings', () => { expect(credentials).to.be.instanceOf(EmulatorCredentialsProvider); const token = await credentials.getToken(); expect(token!.type).to.eql('OAuth'); - expect(token!.user.uid).to.eql(mockUserToken.sub); + expect(token!.user!.uid).to.eql(mockUserToken.sub); }); it('sets credentials based on mockUserToken string', async () => { diff --git a/packages/firestore/test/unit/remote/rest_connection.test.ts b/packages/firestore/test/unit/remote/rest_connection.test.ts index f81c6f02a13..32ef94e9b86 100644 --- a/packages/firestore/test/unit/remote/rest_connection.test.ts +++ b/packages/firestore/test/unit/remote/rest_connection.test.ts @@ -93,7 +93,6 @@ describe('RestConnection', () => { authHeaders: { 'Authorization': 'Bearer owner' } }, { - user: new User(null), type: 'AppCheck', authHeaders: { 'x-firebase-appcheck': 'some-app-check-token' } } From b5ceec59a9e10d6f8bbf247a5a9208ecab2aa234 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 18 Oct 2021 22:58:11 -0500 Subject: [PATCH 09/25] Address code review comments. --- packages/firestore/src/core/firestore_client.ts | 7 ++----- packages/firestore/src/remote/datastore.ts | 14 +++++--------- packages/firestore/src/remote/persistent_stream.ts | 4 ++-- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 827fe65e538..dabdc35edf3 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -122,11 +122,8 @@ export class FirestoreClient { await this.authCredentialListener(user); this.user = user; }); - this.appCheckCredentials.start(asyncQueue, async token => { - logDebug(LOG_TAG, 'Received new AppCheck token'); - // Register an empty credentials change listener to activate token - // refresh. - }); + // Register an empty credentials change listener to activate token refresh. + this.appCheckCredentials.start(asyncQueue, () => Promise.resolve()); } async getConfiguration(): Promise { diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index 8e781f0c4f8..51b8e254036 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -84,19 +84,15 @@ class DatastoreImpl extends Datastore { } /** Gets an auth token and AppCheck token and invokes the provided callback. */ - invokeWithTokens( - func: (authToken: Token | null, appCheckToken: Token | null) => Promise + private invokeWithTokens( + callback: (authToken: Token | null, appCheckToken: Token | null) => Promise ): Promise { this.verifyInitialized(); return Promise.all([ this.authCredentials.getToken(), this.appCheckCredentials.getToken() ]) - .then(values => { - const authToken = values[0]; - const appCheckToken = values[1]; - return func(authToken, appCheckToken); - }) + .then(([authToken, appCheckToken]) => callback(authToken, appCheckToken)) .catch((error: FirestoreError) => { if (error.name === 'FirebaseError') { if (error.code === Code.UNAUTHENTICATED) { @@ -110,7 +106,7 @@ class DatastoreImpl extends Datastore { }); } - /** Gets an auth token and invokes the provided RPC. */ + /** Invokes the provided RPC with auth and AppCheck tokens. */ invokeRPC( rpcName: string, path: string, @@ -127,7 +123,7 @@ class DatastoreImpl extends Datastore { ); } - /** Gets an auth token and invokes the provided RPC with streamed results. */ + /** Invokes the provided RPC with streamed results with auth and AppCheck tokens. */ invokeStreamingRPC( rpcName: string, path: string, diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index 1a3f216bdb7..b7f657876b4 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -447,7 +447,7 @@ export abstract class PersistentStream< this.authCredentialsProvider.getToken(), this.appCheckCredentialsProvider.getToken() ]).then( - tokens => { + ([authToken, appCheckToken]) => { // Stream can be stopped while waiting for authentication. // TODO(mikelehen): We really should just use dispatchIfNotClosed // and let this dispatch onto the queue, but that opened a spec test can @@ -456,7 +456,7 @@ export abstract class PersistentStream< // Normally we'd have to schedule the callback on the AsyncQueue. // However, the following calls are safe to be called outside the // AsyncQueue since they don't chain asynchronous calls - this.startStream(tokens[0], tokens[1]); + this.startStream(authToken, appCheckToken); } }, (error: Error) => { From 659552709264deff096cbb8410bd95449dcf9d51 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 18 Oct 2021 23:02:38 -0500 Subject: [PATCH 10/25] Fix formatting. --- .../firestore/src/platform/browser/webchannel_connection.ts | 6 +++++- packages/firestore/src/remote/datastore.ts | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/platform/browser/webchannel_connection.ts b/packages/firestore/src/platform/browser/webchannel_connection.ts index a6272c716f3..1f22f025607 100644 --- a/packages/firestore/src/platform/browser/webchannel_connection.ts +++ b/packages/firestore/src/platform/browser/webchannel_connection.ts @@ -202,7 +202,11 @@ export class WebChannelConnection extends RestConnection { request.xmlHttpFactory = new FetchXmlHttpFactory({}); } - this.modifyHeadersForRequest(request.initMessageHeaders!, authToken, appCheckToken); + this.modifyHeadersForRequest( + request.initMessageHeaders!, + authToken, + appCheckToken + ); // Sending the custom headers we just added to request.initMessageHeaders // (Authorization, etc.) will trigger the browser to make a CORS preflight diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index 51b8e254036..a9737b2eded 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -85,7 +85,10 @@ class DatastoreImpl extends Datastore { /** Gets an auth token and AppCheck token and invokes the provided callback. */ private invokeWithTokens( - callback: (authToken: Token | null, appCheckToken: Token | null) => Promise + callback: ( + authToken: Token | null, + appCheckToken: Token | null + ) => Promise ): Promise { this.verifyInitialized(); return Promise.all([ From 333d9a185dc1c8676f225fa14feaa23afdf8d31d Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 18 Oct 2021 23:20:55 -0500 Subject: [PATCH 11/25] update changeset. --- .changeset/mean-elephants-rush.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/mean-elephants-rush.md b/.changeset/mean-elephants-rush.md index 807256ba377..28535a0ad64 100644 --- a/.changeset/mean-elephants-rush.md +++ b/.changeset/mean-elephants-rush.md @@ -1,5 +1,5 @@ --- -"@firebase/firestore": patch +"@firebase/firestore": minor --- AppCheck integration for Firestore From 840d4ac9966ddd9613e6f921e56c5c426894e654 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Tue, 19 Oct 2021 10:57:38 -0500 Subject: [PATCH 12/25] try without invokeWithTokens. --- packages/firestore/src/remote/datastore.ts | 76 ++++++++++++---------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index a9737b2eded..e6ee7710a04 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { CredentialsProvider, Token } from '../api/credentials'; +import { CredentialsProvider } from '../api/credentials'; import { User } from '../auth/user'; import { Query, queryToTarget } from '../core/query'; import { Document } from '../model/document'; @@ -83,19 +83,26 @@ class DatastoreImpl extends Datastore { } } - /** Gets an auth token and AppCheck token and invokes the provided callback. */ - private invokeWithTokens( - callback: ( - authToken: Token | null, - appCheckToken: Token | null - ) => Promise - ): Promise { + /** Invokes the provided RPC with auth and AppCheck tokens. */ + invokeRPC( + rpcName: string, + path: string, + request: Req + ): Promise { this.verifyInitialized(); return Promise.all([ this.authCredentials.getToken(), this.appCheckCredentials.getToken() ]) - .then(([authToken, appCheckToken]) => callback(authToken, appCheckToken)) + .then(([authToken, appCheckToken]) => { + return this.connection.invokeRPC( + rpcName, + path, + request, + authToken, + appCheckToken + ); + }) .catch((error: FirestoreError) => { if (error.name === 'FirebaseError') { if (error.code === Code.UNAUTHENTICATED) { @@ -109,38 +116,37 @@ class DatastoreImpl extends Datastore { }); } - /** Invokes the provided RPC with auth and AppCheck tokens. */ - invokeRPC( - rpcName: string, - path: string, - request: Req - ): Promise { - return this.invokeWithTokens((authToken, appCheckToken) => - this.connection.invokeRPC( - rpcName, - path, - request, - authToken, - appCheckToken - ) - ); - } - /** Invokes the provided RPC with streamed results with auth and AppCheck tokens. */ invokeStreamingRPC( rpcName: string, path: string, request: Req ): Promise { - return this.invokeWithTokens((authToken, appCheckToken) => - this.connection.invokeStreamingRPC( - rpcName, - path, - request, - authToken, - appCheckToken - ) - ); + this.verifyInitialized(); + return Promise.all([ + this.authCredentials.getToken(), + this.appCheckCredentials.getToken() + ]) + .then(([authToken, appCheckToken]) => { + return this.connection.invokeStreamingRPC( + rpcName, + path, + request, + authToken, + appCheckToken + ); + }) + .catch((error: FirestoreError) => { + if (error.name === 'FirebaseError') { + if (error.code === Code.UNAUTHENTICATED) { + this.authCredentials.invalidateToken(); + this.appCheckCredentials.invalidateToken(); + } + throw error; + } else { + throw new FirestoreError(Code.UNKNOWN, error.toString()); + } + }); } terminate(): void { From dc9505f9f26b3d6e6ccbd230d3439ce2bf31d3d8 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Tue, 19 Oct 2021 12:04:16 -0500 Subject: [PATCH 13/25] Update lite/register.ts. --- packages/firestore/lite/register.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/firestore/lite/register.ts b/packages/firestore/lite/register.ts index 867a55707f6..c58ad030ad8 100644 --- a/packages/firestore/lite/register.ts +++ b/packages/firestore/lite/register.ts @@ -23,7 +23,10 @@ import { import { Component, ComponentType } from '@firebase/component'; import { version } from '../package.json'; -import { LiteCredentialsProvider } from '../src/api/credentials'; +import { + FirebaseAppCheckTokenProvider, + 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'; @@ -43,7 +46,10 @@ export function registerFirestore(): void { const app = container.getProvider('app').getImmediate()!; const firestoreInstance = new Firestore( app, - new LiteCredentialsProvider(container.getProvider('auth-internal')) + new LiteCredentialsProvider(container.getProvider('auth-internal')), + new FirebaseAppCheckTokenProvider( + container.getProvider('app-check-internal') + ) ); if (settings) { firestoreInstance._setSettings(settings); From b8c19a569c1bf7a10971472947ef83a953a37a78 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Tue, 19 Oct 2021 16:51:33 -0500 Subject: [PATCH 14/25] Don't add AppCheck token if it's an empty string. --- packages/firestore/src/api/credentials.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index e7d7416f76e..bffd664d760 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -454,8 +454,10 @@ export class AppCheckToken implements Token { constructor(value: string) { this.authHeaders = {}; - // Set the headers using Object Literal notation to avoid minification - this.authHeaders['x-firebase-appcheck'] = value; + if(value && value.length > 0) { + // Set the headers using Object Literal notation to avoid minification + this.authHeaders['x-firebase-appcheck'] = value; + } } } From 74c041c99dfd55363e36ff8e6a45e74725c1912f Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Tue, 19 Oct 2021 22:13:29 -0500 Subject: [PATCH 15/25] Prettier. --- packages/firestore/src/api/credentials.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index bffd664d760..e68528a1875 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -454,7 +454,7 @@ export class AppCheckToken implements Token { constructor(value: string) { this.authHeaders = {}; - if(value && value.length > 0) { + if (value && value.length > 0) { // Set the headers using Object Literal notation to avoid minification this.authHeaders['x-firebase-appcheck'] = value; } From 537a5fec5bcf58904627e9caca2938a3732e45cc Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Tue, 19 Oct 2021 22:27:31 -0500 Subject: [PATCH 16/25] Fix firestore-compat. --- packages/firestore-compat/src/index.console.ts | 6 ++++-- packages/firestore/src/api.ts | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/firestore-compat/src/index.console.ts b/packages/firestore-compat/src/index.console.ts index 0dc7105d400..1cfc6d97536 100644 --- a/packages/firestore-compat/src/index.console.ts +++ b/packages/firestore-compat/src/index.console.ts @@ -21,7 +21,8 @@ import { _DatabaseId, Firestore as FirestoreExp, FirestoreError, - _EmptyCredentialsProvider + _EmptyCredentialsProvider, + _EmptyAppCheckTokenProvider } from '@firebase/firestore'; import { @@ -91,7 +92,8 @@ export class Firestore extends FirestoreCompat { databaseIdFromFirestoreDatabase(firestoreDatabase), new FirestoreExp( databaseIdFromFirestoreDatabase(firestoreDatabase), - new _EmptyCredentialsProvider() + new _EmptyCredentialsProvider(), + new _EmptyAppCheckTokenProvider(), ), new MemoryPersistenceProvider() ); diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index e66fda26582..4aedaaaacc4 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -157,3 +157,4 @@ export type { ResourcePath as _ResourcePath } from './model/path'; export type { ByteString as _ByteString } from './util/byte_string'; export { logWarn as _logWarn } from './util/log'; export { EmptyCredentialsProvider as _EmptyCredentialsProvider } from './api/credentials'; +export { EmptyAppCheckTokenProvider as _EmptyAppCheckTokenProvider } from './api/credentials'; From d5462fee8617b9bd89f6228c1377fbc7ff2e90ae Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Tue, 19 Oct 2021 22:37:24 -0500 Subject: [PATCH 17/25] Prettier. --- packages/firestore-compat/src/index.console.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore-compat/src/index.console.ts b/packages/firestore-compat/src/index.console.ts index 1cfc6d97536..a832b534b9c 100644 --- a/packages/firestore-compat/src/index.console.ts +++ b/packages/firestore-compat/src/index.console.ts @@ -93,7 +93,7 @@ export class Firestore extends FirestoreCompat { new FirestoreExp( databaseIdFromFirestoreDatabase(firestoreDatabase), new _EmptyCredentialsProvider(), - new _EmptyAppCheckTokenProvider(), + new _EmptyAppCheckTokenProvider() ), new MemoryPersistenceProvider() ); From 4fbe5f06349d2de71d440f0a28b406b9ea3b888f Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Tue, 19 Oct 2021 22:57:06 -0500 Subject: [PATCH 18/25] Add LiteAppCheckTokenProvider. --- packages/firestore/lite/register.ts | 4 +-- packages/firestore/src/api/credentials.ts | 40 +++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/firestore/lite/register.ts b/packages/firestore/lite/register.ts index c58ad030ad8..5f78a195a7b 100644 --- a/packages/firestore/lite/register.ts +++ b/packages/firestore/lite/register.ts @@ -24,7 +24,7 @@ import { Component, ComponentType } from '@firebase/component'; import { version } from '../package.json'; import { - FirebaseAppCheckTokenProvider, + LiteAppCheckTokenProvider, LiteCredentialsProvider } from '../src/api/credentials'; import { setSDKVersion } from '../src/core/version'; @@ -47,7 +47,7 @@ export function registerFirestore(): void { const firestoreInstance = new Firestore( app, new LiteCredentialsProvider(container.getProvider('auth-internal')), - new FirebaseAppCheckTokenProvider( + new LiteAppCheckTokenProvider( container.getProvider('app-check-internal') ) ); diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index e68528a1875..1727d2c271a 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -579,6 +579,46 @@ export class EmptyAppCheckTokenProvider implements CredentialsProvider { shutdown(): void {} } +/** AppCheck token provider for the Lite SDK. */ +export class LiteAppCheckTokenProvider implements CredentialsProvider { + private appCheck: FirebaseAppCheckInternal | null = null; + + constructor( + private appCheckProvider: Provider + ) { + appCheckProvider.onInit(appCheck => { + this.appCheck = appCheck; + }); + } + + getToken(): Promise { + if (!this.appCheck) { + return Promise.resolve(null); + } + + return this.appCheck.getToken().then(tokenResult => { + if (tokenResult) { + hardAssert( + typeof tokenResult.token === 'string', + 'Invalid tokenResult returned from getToken():' + tokenResult + ); + return new AppCheckToken(tokenResult.token); + } else { + return null; + } + }); + } + + invalidateToken(): void {} + + start( + asyncQueue: AsyncQueue, + changeListener: CredentialChangeListener + ): void {} + + shutdown(): void {} +} + /** * Builds a CredentialsProvider depending on the type of * the credentials passed in. From 098bdd4a6729bb77c89d13678096fee672c93469 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Tue, 19 Oct 2021 22:58:50 -0500 Subject: [PATCH 19/25] string, not String. --- packages/firestore/src/api/credentials.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index 1727d2c271a..75c6f4894cf 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -580,7 +580,7 @@ export class EmptyAppCheckTokenProvider implements CredentialsProvider { } /** AppCheck token provider for the Lite SDK. */ -export class LiteAppCheckTokenProvider implements CredentialsProvider { +export class LiteAppCheckTokenProvider implements CredentialsProvider { private appCheck: FirebaseAppCheckInternal | null = null; constructor( From 30624ae11430c034756e58464040a776781f022c Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Wed, 20 Oct 2021 14:21:23 -0500 Subject: [PATCH 20/25] Add "Auth" to class names where applicable. --- .../firestore-compat/src/index.console.ts | 4 ++-- packages/firestore/lite/register.ts | 4 ++-- packages/firestore/src/api.ts | 2 +- packages/firestore/src/api/credentials.ts | 22 +++++++++---------- packages/firestore/src/lite-api/database.ts | 4 ++-- packages/firestore/src/register.ts | 4 ++-- .../test/integration/remote/stream.test.ts | 10 ++++----- .../test/integration/util/internal_helpers.ts | 10 ++++----- .../firestore/test/unit/api/database.test.ts | 6 ++--- .../test/unit/remote/datastore.test.ts | 22 +++++++++---------- .../test/unit/specs/spec_test_runner.ts | 4 ++-- packages/firestore/test/util/api_helpers.ts | 4 ++-- 12 files changed, 48 insertions(+), 48 deletions(-) diff --git a/packages/firestore-compat/src/index.console.ts b/packages/firestore-compat/src/index.console.ts index a832b534b9c..7c9a7fc63a2 100644 --- a/packages/firestore-compat/src/index.console.ts +++ b/packages/firestore-compat/src/index.console.ts @@ -21,7 +21,7 @@ import { _DatabaseId, Firestore as FirestoreExp, FirestoreError, - _EmptyCredentialsProvider, + _EmptyAuthCredentialsProvider, _EmptyAppCheckTokenProvider } from '@firebase/firestore'; @@ -92,7 +92,7 @@ export class Firestore extends FirestoreCompat { databaseIdFromFirestoreDatabase(firestoreDatabase), new FirestoreExp( databaseIdFromFirestoreDatabase(firestoreDatabase), - new _EmptyCredentialsProvider(), + new _EmptyAuthCredentialsProvider(), new _EmptyAppCheckTokenProvider() ), new MemoryPersistenceProvider() diff --git a/packages/firestore/lite/register.ts b/packages/firestore/lite/register.ts index 5f78a195a7b..0b5c73e409b 100644 --- a/packages/firestore/lite/register.ts +++ b/packages/firestore/lite/register.ts @@ -25,7 +25,7 @@ import { Component, ComponentType } from '@firebase/component'; import { version } from '../package.json'; import { LiteAppCheckTokenProvider, - LiteCredentialsProvider + LiteAuthCredentialsProvider } from '../src/api/credentials'; import { setSDKVersion } from '../src/core/version'; import { Firestore } from '../src/lite-api/database'; @@ -46,7 +46,7 @@ export function registerFirestore(): void { const app = container.getProvider('app').getImmediate()!; const firestoreInstance = new Firestore( app, - new LiteCredentialsProvider(container.getProvider('auth-internal')), + new LiteAuthCredentialsProvider(container.getProvider('auth-internal')), new LiteAppCheckTokenProvider( container.getProvider('app-check-internal') ) diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 4aedaaaacc4..aab20b52e8e 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -156,5 +156,5 @@ export { FieldPath as _FieldPath } from './model/path'; export type { ResourcePath as _ResourcePath } from './model/path'; export type { ByteString as _ByteString } from './util/byte_string'; export { logWarn as _logWarn } from './util/log'; -export { EmptyCredentialsProvider as _EmptyCredentialsProvider } from './api/credentials'; +export { EmptyAuthCredentialsProvider as _EmptyAuthCredentialsProvider } from './api/credentials'; export { EmptyAppCheckTokenProvider as _EmptyAppCheckTokenProvider } from './api/credentials'; diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index 75c6f4894cf..fca16616f56 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -122,7 +122,7 @@ export interface CredentialsProvider { * A CredentialsProvider that always yields an empty token. * @internal */ -export class EmptyCredentialsProvider implements CredentialsProvider { +export class EmptyAuthCredentialsProvider implements CredentialsProvider { getToken(): Promise { return Promise.resolve(null); } @@ -144,7 +144,7 @@ export class EmptyCredentialsProvider implements CredentialsProvider { * A CredentialsProvider that always returns a constant token. Used for * emulator token mocking. */ -export class EmulatorCredentialsProvider implements CredentialsProvider { +export class EmulatorAuthCredentialsProvider implements CredentialsProvider { constructor(private token: Token) {} /** @@ -179,7 +179,7 @@ export class EmulatorCredentialsProvider implements CredentialsProvider { } /** Credential provider for the Lite SDK. */ -export class LiteCredentialsProvider implements CredentialsProvider { +export class LiteAuthCredentialsProvider implements CredentialsProvider { private auth: FirebaseAuthInternal | null = null; constructor(authProvider: Provider) { @@ -219,7 +219,7 @@ export class LiteCredentialsProvider implements CredentialsProvider { shutdown(): void {} } -export class FirebaseCredentialsProvider implements CredentialsProvider { +export class FirebaseAuthCredentialsProvider implements CredentialsProvider { /** * The auth token listener registered with FirebaseApp, retained here so we * can unregister it. @@ -280,7 +280,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider { }; const registerAuth = (auth: FirebaseAuthInternal): void => { - logDebug('FirebaseCredentialsProvider', 'Auth detected'); + logDebug('FirebaseAuthCredentialsProvider', 'Auth detected'); this.auth = auth; this.auth.addAuthTokenListener(this.tokenListener); awaitNextToken(); @@ -298,7 +298,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider { registerAuth(auth); } else { // If auth is still not available, proceed with `null` user - logDebug('FirebaseCredentialsProvider', 'Auth not yet detected'); + logDebug('FirebaseAuthCredentialsProvider', 'Auth not yet detected'); nextToken.resolve(); nextToken = new Deferred(); } @@ -311,7 +311,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider { getToken(): Promise { debugAssert( this.tokenListener != null, - 'FirebaseCredentialsProvider not started.' + 'FirebaseAuthCredentialsProvider not started.' ); // Take note of the current value of the tokenCounter so that this method @@ -331,7 +331,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider { // user, we can't be sure). if (this.tokenCounter !== initialTokenCounter) { logDebug( - 'FirebaseCredentialsProvider', + 'FirebaseAuthCredentialsProvider', 'getToken aborted due to token change.' ); return this.getToken(); @@ -420,7 +420,7 @@ export class FirstPartyToken implements Token { * to authenticate the user, using technique that is only available * to applications hosted by Google. */ -export class FirstPartyCredentialsProvider +export class FirstPartyAuthCredentialsProvider implements CredentialsProvider { constructor( @@ -627,7 +627,7 @@ export function makeAuthCredentialsProvider( credentials?: CredentialsSettings ): CredentialsProvider { if (!credentials) { - return new EmptyCredentialsProvider(); + return new EmptyAuthCredentialsProvider(); } switch (credentials['type']) { @@ -643,7 +643,7 @@ export function makeAuthCredentialsProvider( ), 'unexpected gapi interface' ); - return new FirstPartyCredentialsProvider( + return new FirstPartyAuthCredentialsProvider( client, credentials['sessionIndex'] || '0', credentials['iamToken'] || null diff --git a/packages/firestore/src/lite-api/database.ts b/packages/firestore/src/lite-api/database.ts index cc3dd14263a..07db0a3c868 100644 --- a/packages/firestore/src/lite-api/database.ts +++ b/packages/firestore/src/lite-api/database.ts @@ -26,7 +26,7 @@ import { createMockUserToken, EmulatorMockTokenOptions } from '@firebase/util'; import { CredentialsProvider, - EmulatorCredentialsProvider, + EmulatorAuthCredentialsProvider, makeAuthCredentialsProvider, OAuthToken } from '../api/credentials'; @@ -275,7 +275,7 @@ export function connectFirestoreEmulator( user = new User(uid); } - firestore._authCredentials = new EmulatorCredentialsProvider( + firestore._authCredentials = new EmulatorAuthCredentialsProvider( new OAuthToken(token, user) ); } diff --git a/packages/firestore/src/register.ts b/packages/firestore/src/register.ts index 77e6b698ab2..35646080a47 100644 --- a/packages/firestore/src/register.ts +++ b/packages/firestore/src/register.ts @@ -25,7 +25,7 @@ import { Component, ComponentType } from '@firebase/component'; import { name, version } from '../package.json'; import { FirebaseAppCheckTokenProvider, - FirebaseCredentialsProvider + FirebaseAuthCredentialsProvider } from '../src/api/credentials'; import { setSDKVersion } from '../src/core/version'; @@ -44,7 +44,7 @@ export function registerFirestore( const app = container.getProvider('app').getImmediate()!; const firestoreInstance = new Firestore( app, - new FirebaseCredentialsProvider( + new FirebaseAuthCredentialsProvider( container.getProvider('auth-internal') ), new FirebaseAppCheckTokenProvider( diff --git a/packages/firestore/test/integration/remote/stream.test.ts b/packages/firestore/test/integration/remote/stream.test.ts index 2793872287b..f493e2c0ef7 100644 --- a/packages/firestore/test/integration/remote/stream.test.ts +++ b/packages/firestore/test/integration/remote/stream.test.ts @@ -17,7 +17,7 @@ import { expect } from 'chai'; -import { EmptyCredentialsProvider, Token } from '../../../src/api/credentials'; +import { EmptyAuthCredentialsProvider, Token } from '../../../src/api/credentials'; import { SnapshotVersion } from '../../../src/core/snapshot_version'; import { MutationResult } from '../../../src/model/mutation'; import { @@ -147,7 +147,7 @@ describe('Watch Stream', () => { }); }); -class MockCredentialsProvider extends EmptyCredentialsProvider { +class MockAuthCredentialsProvider extends EmptyAuthCredentialsProvider { private states: string[] = []; get observedStates(): string[] { @@ -240,7 +240,7 @@ describe('Write Stream', () => { }); it('force refreshes auth token on receiving unauthenticated error', () => { - const credentials = new MockCredentialsProvider(); + const credentials = new MockAuthCredentialsProvider(); return withTestWriteStream(async (writeStream, streamListener) => { await streamListener.awaitCallback('open'); @@ -275,7 +275,7 @@ describe('Write Stream', () => { }); it('token is not invalidated once the stream is healthy', () => { - const credentials = new MockCredentialsProvider(); + const credentials = new MockAuthCredentialsProvider(); return withTestWriteStream(async (writeStream, streamListener, queue) => { await streamListener.awaitCallback('open'); @@ -298,7 +298,7 @@ export async function withTestWriteStream( streamListener: StreamStatusListener, queue: AsyncQueueImpl ) => Promise, - credentialsProvider = new EmptyCredentialsProvider() + credentialsProvider = new EmptyAuthCredentialsProvider() ): Promise { await withTestDatastore(async datastore => { const queue = newAsyncQueue() as AsyncQueueImpl; diff --git a/packages/firestore/test/integration/util/internal_helpers.ts b/packages/firestore/test/integration/util/internal_helpers.ts index dbfa206a5b5..8548241b7bf 100644 --- a/packages/firestore/test/integration/util/internal_helpers.ts +++ b/packages/firestore/test/integration/util/internal_helpers.ts @@ -22,7 +22,7 @@ import { CredentialChangeListener, CredentialsProvider, EmptyAppCheckTokenProvider, - EmptyCredentialsProvider + EmptyAuthCredentialsProvider } from '../../../src/api/credentials'; import { User } from '../../../src/auth/user'; import { DatabaseId, DatabaseInfo } from '../../../src/core/database_info'; @@ -57,7 +57,7 @@ export function getDefaultDatabaseInfo(): DatabaseInfo { export function withTestDatastore( fn: (datastore: Datastore) => Promise, - authCredentialsProvider: CredentialsProvider = new EmptyCredentialsProvider(), + authCredentialsProvider: CredentialsProvider = new EmptyAuthCredentialsProvider(), appCheckTokenProvider: CredentialsProvider = new EmptyAppCheckTokenProvider() ): Promise { const databaseInfo = getDefaultDatabaseInfo(); @@ -72,7 +72,7 @@ export function withTestDatastore( return fn(datastore); } -export class MockCredentialsProvider extends EmptyCredentialsProvider { +export class MockAuthCredentialsProvider extends EmptyAuthCredentialsProvider { private listener: CredentialChangeListener | null = null; private asyncQueue: AsyncQueue | null = null; @@ -94,10 +94,10 @@ export function withMockCredentialProviderTestDb( persistence: boolean, fn: ( db: firestore.FirebaseFirestore, - mockCredential: MockCredentialsProvider + mockCredential: MockAuthCredentialsProvider ) => Promise ): Promise { - const mockCredentialsProvider = new MockCredentialsProvider(); + const mockCredentialsProvider = new MockAuthCredentialsProvider(); const settings = { ...DEFAULT_SETTINGS, credentials: { client: mockCredentialsProvider, type: 'provider' } diff --git a/packages/firestore/test/unit/api/database.test.ts b/packages/firestore/test/unit/api/database.test.ts index db0d496ad6f..6e14c588f74 100644 --- a/packages/firestore/test/unit/api/database.test.ts +++ b/packages/firestore/test/unit/api/database.test.ts @@ -17,7 +17,7 @@ import { expect } from 'chai'; -import { EmulatorCredentialsProvider } from '../../../src/api/credentials'; +import { EmulatorAuthCredentialsProvider } from '../../../src/api/credentials'; import { User } from '../../../src/auth/user'; import { collectionReference, @@ -260,7 +260,7 @@ describe('Settings', () => { db.useEmulator('localhost', 9000, { mockUserToken }); const credentials = db._delegate._authCredentials; - expect(credentials).to.be.instanceOf(EmulatorCredentialsProvider); + expect(credentials).to.be.instanceOf(EmulatorAuthCredentialsProvider); const token = await credentials.getToken(); expect(token!.type).to.eql('OAuth'); expect(token!.user!.uid).to.eql(mockUserToken.sub); @@ -274,7 +274,7 @@ describe('Settings', () => { }); const credentials = db._delegate._authCredentials; - expect(credentials).to.be.instanceOf(EmulatorCredentialsProvider); + expect(credentials).to.be.instanceOf(EmulatorAuthCredentialsProvider); const token = await credentials.getToken(); expect(token!.type).to.eql('OAuth'); expect(token!.user).to.eql(User.MOCK_USER); diff --git a/packages/firestore/test/unit/remote/datastore.test.ts b/packages/firestore/test/unit/remote/datastore.test.ts index 40215099a0c..46729f1c5a8 100644 --- a/packages/firestore/test/unit/remote/datastore.test.ts +++ b/packages/firestore/test/unit/remote/datastore.test.ts @@ -20,7 +20,7 @@ import * as chaiAsPromised from 'chai-as-promised'; import { EmptyAppCheckTokenProvider, - EmptyCredentialsProvider, + EmptyAuthCredentialsProvider, Token } from '../../../src/api/credentials'; import { DatabaseId } from '../../../src/core/database_info'; @@ -67,7 +67,7 @@ describe('Datastore', () => { } } - class MockCredentialsProvider extends EmptyCredentialsProvider { + class MockAuthCredentialsProvider extends EmptyAuthCredentialsProvider { invalidateTokenInvoked = false; invalidateToken(): void { this.invalidateTokenInvoked = true; @@ -105,7 +105,7 @@ describe('Datastore', () => { it('newDatastore() returns an an instance of Datastore', () => { const datastore = newDatastore( - new EmptyCredentialsProvider(), + new EmptyAuthCredentialsProvider(), new EmptyAppCheckTokenProvider(), new MockConnection(), serializer @@ -115,7 +115,7 @@ describe('Datastore', () => { it('DatastoreImpl.invokeRPC() fails if terminated', async () => { const datastore = newDatastore( - new EmptyCredentialsProvider(), + new EmptyAuthCredentialsProvider(), new EmptyAppCheckTokenProvider(), new MockConnection(), serializer @@ -133,7 +133,7 @@ describe('Datastore', () => { const connection = new MockConnection(); connection.invokeRPC = () => Promise.reject(new FirestoreError(Code.ABORTED, 'zzyzx')); - const authCredentials = new MockCredentialsProvider(); + const authCredentials = new MockAuthCredentialsProvider(); const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, @@ -154,7 +154,7 @@ describe('Datastore', () => { it('DatastoreImpl.invokeRPC() wraps unknown exceptions in a FirestoreError', async () => { const connection = new MockConnection(); connection.invokeRPC = () => Promise.reject('zzyzx'); - const authCredentials = new MockCredentialsProvider(); + const authCredentials = new MockAuthCredentialsProvider(); const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, @@ -176,7 +176,7 @@ describe('Datastore', () => { const connection = new MockConnection(); connection.invokeRPC = () => Promise.reject(new FirestoreError(Code.UNAUTHENTICATED, 'zzyzx')); - const authCredentials = new MockCredentialsProvider(); + const authCredentials = new MockAuthCredentialsProvider(); const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, @@ -196,7 +196,7 @@ describe('Datastore', () => { it('DatastoreImpl.invokeStreamingRPC() fails if terminated', async () => { const datastore = newDatastore( - new EmptyCredentialsProvider(), + new EmptyAuthCredentialsProvider(), new EmptyAppCheckTokenProvider(), new MockConnection(), serializer @@ -214,7 +214,7 @@ describe('Datastore', () => { const connection = new MockConnection(); connection.invokeStreamingRPC = () => Promise.reject(new FirestoreError(Code.ABORTED, 'zzyzx')); - const authCredentials = new MockCredentialsProvider(); + const authCredentials = new MockAuthCredentialsProvider(); const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, @@ -235,7 +235,7 @@ describe('Datastore', () => { it('DatastoreImpl.invokeStreamingRPC() wraps unknown exceptions in a FirestoreError', async () => { const connection = new MockConnection(); connection.invokeStreamingRPC = () => Promise.reject('zzyzx'); - const authCredentials = new MockCredentialsProvider(); + const authCredentials = new MockAuthCredentialsProvider(); const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, @@ -257,7 +257,7 @@ describe('Datastore', () => { const connection = new MockConnection(); connection.invokeStreamingRPC = () => Promise.reject(new FirestoreError(Code.UNAUTHENTICATED, 'zzyzx')); - const authCredentials = new MockCredentialsProvider(); + const authCredentials = new MockAuthCredentialsProvider(); const appCheckCredentials = new MockAppCheckTokenProvider(); const datastore = newDatastore( authCredentials, diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index fdb8d88b55c..8626250046d 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -20,7 +20,7 @@ import { expect } from 'chai'; import { LoadBundleTask } from '../../../src/api/bundle'; import { EmptyAppCheckTokenProvider, - EmptyCredentialsProvider + EmptyAuthCredentialsProvider } from '../../../src/api/credentials'; import { User } from '../../../src/auth/user'; import { ComponentConfiguration } from '../../../src/core/component_provider'; @@ -289,7 +289,7 @@ abstract class TestRunner { const configuration = { asyncQueue: this.queue, databaseInfo: this.databaseInfo, - authCredentials: new EmptyCredentialsProvider(), + authCredentials: new EmptyAuthCredentialsProvider(), appCheckCredentials: new EmptyAppCheckTokenProvider(), clientId: this.clientId, initialUser: this.user, diff --git a/packages/firestore/test/util/api_helpers.ts b/packages/firestore/test/util/api_helpers.ts index b44c7d5c142..374c58abeb2 100644 --- a/packages/firestore/test/util/api_helpers.ts +++ b/packages/firestore/test/util/api_helpers.ts @@ -30,7 +30,7 @@ import { } from '../../compat/api/database'; import { EmptyAppCheckTokenProvider, - EmptyCredentialsProvider + EmptyAuthCredentialsProvider } from '../../src/api/credentials'; import { ensureFirestoreConfigured, @@ -74,7 +74,7 @@ export function newTestFirestore(projectId = 'new-project'): Firestore { new DatabaseId(projectId), new ExpFirestore( new DatabaseId(projectId), - new EmptyCredentialsProvider(), + new EmptyAuthCredentialsProvider(), new EmptyAppCheckTokenProvider() ), new IndexedDbPersistenceProvider() From 56f121cc054bb9f3aaadc429299f6e58ce8fbaa5 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Wed, 20 Oct 2021 15:02:21 -0500 Subject: [PATCH 21/25] Rename authHeaders->headers. Add a test for empty AppCheck token. --- packages/firestore/lite/register.ts | 4 ++- packages/firestore/src/api/credentials.ts | 32 +++++++++++-------- .../src/platform/node/grpc_connection.ts | 6 ++-- .../firestore/src/remote/rest_connection.ts | 6 ++-- .../test/integration/remote/stream.test.ts | 5 ++- .../test/unit/remote/rest_connection.test.ts | 22 +++++++++++-- 6 files changed, 50 insertions(+), 25 deletions(-) diff --git a/packages/firestore/lite/register.ts b/packages/firestore/lite/register.ts index 0b5c73e409b..1200b68ad73 100644 --- a/packages/firestore/lite/register.ts +++ b/packages/firestore/lite/register.ts @@ -46,7 +46,9 @@ export function registerFirestore(): void { const app = container.getProvider('app').getImmediate()!; const firestoreInstance = new Firestore( app, - new LiteAuthCredentialsProvider(container.getProvider('auth-internal')), + new LiteAuthCredentialsProvider( + container.getProvider('auth-internal') + ), new LiteAppCheckTokenProvider( container.getProvider('app-check-internal') ) diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index fca16616f56..5f80378c863 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -69,16 +69,16 @@ export interface Token { user?: User; /** Extra header values to be passed along with a request */ - authHeaders: { [header: string]: string }; + headers: { [header: string]: string }; } export class OAuthToken implements Token { type = 'OAuth' as TokenType; - authHeaders: { [header: string]: string }; + headers: { [header: string]: string }; constructor(value: string, public user: User) { - this.authHeaders = {}; + this.headers = {}; // Set the headers using Object Literal notation to avoid minification - this.authHeaders['Authorization'] = `Bearer ${value}`; + this.headers['Authorization'] = `Bearer ${value}`; } } @@ -144,7 +144,9 @@ export class EmptyAuthCredentialsProvider implements CredentialsProvider { * A CredentialsProvider that always returns a constant token. Used for * emulator token mocking. */ -export class EmulatorAuthCredentialsProvider implements CredentialsProvider { +export class EmulatorAuthCredentialsProvider + implements CredentialsProvider +{ constructor(private token: Token) {} /** @@ -219,7 +221,9 @@ export class LiteAuthCredentialsProvider implements CredentialsProvider { shutdown(): void {} } -export class FirebaseAuthCredentialsProvider implements CredentialsProvider { +export class FirebaseAuthCredentialsProvider + implements CredentialsProvider +{ /** * The auth token listener registered with FirebaseApp, retained here so we * can unregister it. @@ -399,19 +403,19 @@ export class FirstPartyToken implements Token { private iamToken: string | null ) {} - get authHeaders(): { [header: string]: string } { - const headers: { [header: string]: string } = { + get headers(): { [header: string]: string } { + const result: { [header: string]: string } = { 'X-Goog-AuthUser': this.sessionIndex }; // Use array notation to prevent minification const authHeader = this.gapi['auth']['getAuthHeaderValueForFirstParty']([]); if (authHeader) { - headers['Authorization'] = authHeader; + result['Authorization'] = authHeader; } if (this.iamToken) { - headers['X-Goog-Iam-Authorization-Token'] = this.iamToken; + result['X-Goog-Iam-Authorization-Token'] = this.iamToken; } - return headers; + return result; } } @@ -450,13 +454,13 @@ export class FirstPartyAuthCredentialsProvider export class AppCheckToken implements Token { type = 'AppCheck' as TokenType; - authHeaders: { [header: string]: string }; + headers: { [header: string]: string }; constructor(value: string) { - this.authHeaders = {}; + this.headers = {}; if (value && value.length > 0) { // Set the headers using Object Literal notation to avoid minification - this.authHeaders['x-firebase-appcheck'] = value; + this.headers['x-firebase-appcheck'] = value; } } } diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index 9a8b62dba7c..0e588003ed5 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/packages/firestore/src/platform/node/grpc_connection.ts @@ -61,9 +61,9 @@ function createMetadata( const metadata = new Metadata(); for (const token of [authToken, appCheckToken]) { if (token) { - for (const header in token.authHeaders) { - if (token.authHeaders.hasOwnProperty(header)) { - metadata.set(header, token.authHeaders[header]); + for (const header in token.headers) { + if (token.headers.hasOwnProperty(header)) { + metadata.set(header, token.headers[header]); } } } diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index 7cdec73cb78..a0653b82ccf 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/packages/firestore/src/remote/rest_connection.ts @@ -145,9 +145,9 @@ export abstract class RestConnection implements Connection { } for (const token of [authToken, appCheckToken]) { if (token) { - for (const header in token.authHeaders) { - if (token.authHeaders.hasOwnProperty(header)) { - headers[header] = token.authHeaders[header]; + for (const header in token.headers) { + if (token.headers.hasOwnProperty(header)) { + headers[header] = token.headers[header]; } } } diff --git a/packages/firestore/test/integration/remote/stream.test.ts b/packages/firestore/test/integration/remote/stream.test.ts index f493e2c0ef7..6726f92a772 100644 --- a/packages/firestore/test/integration/remote/stream.test.ts +++ b/packages/firestore/test/integration/remote/stream.test.ts @@ -17,7 +17,10 @@ import { expect } from 'chai'; -import { EmptyAuthCredentialsProvider, Token } from '../../../src/api/credentials'; +import { + EmptyAuthCredentialsProvider, + Token +} from '../../../src/api/credentials'; import { SnapshotVersion } from '../../../src/core/snapshot_version'; import { MutationResult } from '../../../src/model/mutation'; import { diff --git a/packages/firestore/test/unit/remote/rest_connection.test.ts b/packages/firestore/test/unit/remote/rest_connection.test.ts index 32ef94e9b86..ca4bf6783de 100644 --- a/packages/firestore/test/unit/remote/rest_connection.test.ts +++ b/packages/firestore/test/unit/remote/rest_connection.test.ts @@ -17,7 +17,7 @@ import { expect } from 'chai'; -import { Token } from '../../../src/api/credentials'; +import { AppCheckToken, Token } from '../../../src/api/credentials'; import { User } from '../../../src/auth/user'; import { DatabaseId, DatabaseInfo } from '../../../src/core/database_info'; import { SDK_VERSION } from '../../../src/core/version'; @@ -90,11 +90,11 @@ describe('RestConnection', () => { { user: User.UNAUTHENTICATED, type: 'OAuth', - authHeaders: { 'Authorization': 'Bearer owner' } + headers: { 'Authorization': 'Bearer owner' } }, { type: 'AppCheck', - authHeaders: { 'x-firebase-appcheck': 'some-app-check-token' } + headers: { 'x-firebase-appcheck': 'some-app-check-token' } } ); expect(connection.lastHeaders).to.deep.equal({ @@ -106,6 +106,22 @@ describe('RestConnection', () => { }); }); + it('empty app check token is not added to headers', async () => { + await connection.invokeRPC( + 'RunQuery', + 'projects/testproject/databases/(default)/documents/foo', + {}, + null, + new AppCheckToken('') + ); + expect(connection.lastHeaders).to.deep.equal({ + 'Content-Type': 'text/plain', + 'X-Firebase-GMPID': 'test-app-id', + 'X-Goog-Api-Client': `gl-js/ fire/${SDK_VERSION}` + // Note: AppCheck token should not exist here. + }); + }); + it('returns success', async () => { connection.nextResponse = Promise.resolve({ response: true }); const response = await connection.invokeRPC( From 0cab3936c2bed848b0c4f13e4cef1e5042c9dcfc Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Wed, 20 Oct 2021 22:23:01 -0500 Subject: [PATCH 22/25] Better way to apply token headers. --- packages/firestore/src/api/credentials.ts | 35 ++++++++----------- .../src/platform/node/grpc_connection.ts | 18 +++++----- .../firestore/src/remote/rest_connection.ts | 18 +++++----- .../test/unit/remote/rest_connection.test.ts | 13 ++----- 4 files changed, 36 insertions(+), 48 deletions(-) diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index 5f80378c863..8da8bb1876b 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -68,17 +68,16 @@ export interface Token { */ user?: User; - /** Extra header values to be passed along with a request */ - headers: { [header: string]: string }; + /** Invokes the given callback with the token's key and value. */ + applyHeaders(callback: (tokenKey: string, tokenValue: string) => void): void; } export class OAuthToken implements Token { type = 'OAuth' as TokenType; - headers: { [header: string]: string }; - constructor(value: string, public user: User) { - this.headers = {}; - // Set the headers using Object Literal notation to avoid minification - this.headers['Authorization'] = `Bearer ${value}`; + constructor(private value: string, public user: User) {} + + applyHeaders(callback: (tokenKey: string, tokenValue: string) => void): void { + callback('Authorization', `Bearer ${this.value}`); } } @@ -403,19 +402,15 @@ export class FirstPartyToken implements Token { private iamToken: string | null ) {} - get headers(): { [header: string]: string } { - const result: { [header: string]: string } = { - 'X-Goog-AuthUser': this.sessionIndex - }; - // Use array notation to prevent minification + applyHeaders(callback: (tokenKey: string, tokenValue: string) => void): void { + callback('X-Goog-AuthUser', this.sessionIndex); const authHeader = this.gapi['auth']['getAuthHeaderValueForFirstParty']([]); if (authHeader) { - result['Authorization'] = authHeader; + callback('Authorization', authHeader); } if (this.iamToken) { - result['X-Goog-Iam-Authorization-Token'] = this.iamToken; + callback('X-Goog-Iam-Authorization-Token', this.iamToken); } - return result; } } @@ -454,13 +449,11 @@ export class FirstPartyAuthCredentialsProvider export class AppCheckToken implements Token { type = 'AppCheck' as TokenType; - headers: { [header: string]: string }; + constructor(private value: string) {} - constructor(value: string) { - this.headers = {}; - if (value && value.length > 0) { - // Set the headers using Object Literal notation to avoid minification - this.headers['x-firebase-appcheck'] = value; + applyHeaders(callback: (tokenKey: string, tokenValue: string) => void): void { + if (this.value && this.value.length > 0) { + callback('x-firebase-appcheck', this.value); } } } diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index 0e588003ed5..4d447d8b21b 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/packages/firestore/src/platform/node/grpc_connection.ts @@ -57,16 +57,16 @@ function createMetadata( authToken === null || authToken.type === 'OAuth', 'If provided, token must be OAuth' ); - const metadata = new Metadata(); - for (const token of [authToken, appCheckToken]) { - if (token) { - for (const header in token.headers) { - if (token.headers.hasOwnProperty(header)) { - metadata.set(header, token.headers[header]); - } - } - } + if (authToken) { + authToken.applyHeaders((tokenKey, tokenValue) => + metadata.set(tokenKey, tokenValue) + ); + } + if (appCheckToken) { + appCheckToken.applyHeaders((tokenKey, tokenValue) => + metadata.set(tokenKey, tokenValue) + ); } if (appId) { metadata.set('X-Firebase-GMPID', appId); diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index a0653b82ccf..ebaeb074d1a 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/packages/firestore/src/remote/rest_connection.ts @@ -143,14 +143,16 @@ export abstract class RestConnection implements Connection { if (this.databaseInfo.appId) { headers['X-Firebase-GMPID'] = this.databaseInfo.appId; } - for (const token of [authToken, appCheckToken]) { - if (token) { - for (const header in token.headers) { - if (token.headers.hasOwnProperty(header)) { - headers[header] = token.headers[header]; - } - } - } + + if (authToken) { + authToken.applyHeaders( + (tokenKey, tokenValue) => (headers[tokenKey] = tokenValue) + ); + } + if (appCheckToken) { + appCheckToken.applyHeaders( + (tokenKey, tokenValue) => (headers[tokenKey] = tokenValue) + ); } } diff --git a/packages/firestore/test/unit/remote/rest_connection.test.ts b/packages/firestore/test/unit/remote/rest_connection.test.ts index ca4bf6783de..8266ce74cf2 100644 --- a/packages/firestore/test/unit/remote/rest_connection.test.ts +++ b/packages/firestore/test/unit/remote/rest_connection.test.ts @@ -17,7 +17,7 @@ import { expect } from 'chai'; -import { AppCheckToken, Token } from '../../../src/api/credentials'; +import { AppCheckToken, OAuthToken, Token } from '../../../src/api/credentials'; import { User } from '../../../src/auth/user'; import { DatabaseId, DatabaseInfo } from '../../../src/core/database_info'; import { SDK_VERSION } from '../../../src/core/version'; @@ -87,15 +87,8 @@ describe('RestConnection', () => { 'RunQuery', 'projects/testproject/databases/(default)/documents/foo', {}, - { - user: User.UNAUTHENTICATED, - type: 'OAuth', - headers: { 'Authorization': 'Bearer owner' } - }, - { - type: 'AppCheck', - headers: { 'x-firebase-appcheck': 'some-app-check-token' } - } + new OAuthToken('owner', User.UNAUTHENTICATED), + new AppCheckToken('some-app-check-token') ); expect(connection.lastHeaders).to.deep.equal({ 'Authorization': 'Bearer owner', From 8f8a79389a7270e2fca00a3a32fa501d34590638 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Thu, 4 Nov 2021 10:52:31 -0500 Subject: [PATCH 23/25] Improve code readability for applying tokens. --- packages/firestore/src/api/credentials.ts | 37 ++++++++----------- .../src/platform/node/grpc_connection.ts | 8 +--- .../firestore/src/remote/rest_connection.ts | 8 +--- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index 8da8bb1876b..b8277beb3ec 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/packages/firestore/src/api/credentials.ts @@ -68,16 +68,16 @@ export interface Token { */ user?: User; - /** Invokes the given callback with the token's key and value. */ - applyHeaders(callback: (tokenKey: string, tokenValue: string) => void): void; + /** Header values to set for this token */ + headers: Map; } export class OAuthToken implements Token { type = 'OAuth' as TokenType; - constructor(private value: string, public user: User) {} + headers = new Map(); - applyHeaders(callback: (tokenKey: string, tokenValue: string) => void): void { - callback('Authorization', `Bearer ${this.value}`); + constructor(value: string, public user: User) { + this.headers.set('Authorization', `Bearer ${value}`); } } @@ -395,21 +395,16 @@ interface Gapi { export class FirstPartyToken implements Token { type = 'FirstParty' as TokenType; user = User.FIRST_PARTY; + headers = new Map(); - constructor( - private gapi: Gapi, - private sessionIndex: string, - private iamToken: string | null - ) {} - - applyHeaders(callback: (tokenKey: string, tokenValue: string) => void): void { - callback('X-Goog-AuthUser', this.sessionIndex); - const authHeader = this.gapi['auth']['getAuthHeaderValueForFirstParty']([]); + constructor(gapi: Gapi, sessionIndex: string, iamToken: string | null) { + this.headers.set('X-Goog-AuthUser', sessionIndex); + const authHeader = gapi['auth']['getAuthHeaderValueForFirstParty']([]); if (authHeader) { - callback('Authorization', authHeader); + this.headers.set('Authorization', authHeader); } - if (this.iamToken) { - callback('X-Goog-Iam-Authorization-Token', this.iamToken); + if (iamToken) { + this.headers.set('X-Goog-Iam-Authorization-Token', iamToken); } } } @@ -449,11 +444,11 @@ export class FirstPartyAuthCredentialsProvider export class AppCheckToken implements Token { type = 'AppCheck' as TokenType; - constructor(private value: string) {} + headers = new Map(); - applyHeaders(callback: (tokenKey: string, tokenValue: string) => void): void { - if (this.value && this.value.length > 0) { - callback('x-firebase-appcheck', this.value); + constructor(private value: string) { + if (value && value.length > 0) { + this.headers.set('x-firebase-appcheck', this.value); } } } diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index 4d447d8b21b..78c08869b18 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/packages/firestore/src/platform/node/grpc_connection.ts @@ -59,14 +59,10 @@ function createMetadata( ); const metadata = new Metadata(); if (authToken) { - authToken.applyHeaders((tokenKey, tokenValue) => - metadata.set(tokenKey, tokenValue) - ); + authToken.headers.forEach((value, key) => metadata.set(key, value)); } if (appCheckToken) { - appCheckToken.applyHeaders((tokenKey, tokenValue) => - metadata.set(tokenKey, tokenValue) - ); + appCheckToken.headers.forEach((value, key) => metadata.set(key, value)); } if (appId) { metadata.set('X-Firebase-GMPID', appId); diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index ebaeb074d1a..524e52c385a 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/packages/firestore/src/remote/rest_connection.ts @@ -145,14 +145,10 @@ export abstract class RestConnection implements Connection { } if (authToken) { - authToken.applyHeaders( - (tokenKey, tokenValue) => (headers[tokenKey] = tokenValue) - ); + authToken.headers.forEach((value, key) => (headers[key] = value)); } if (appCheckToken) { - appCheckToken.applyHeaders( - (tokenKey, tokenValue) => (headers[tokenKey] = tokenValue) - ); + appCheckToken.headers.forEach((value, key) => (headers[key] = value)); } } From f46d21ef64d0bcfaa7070a0dd4e1270e5978af88 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Fri, 5 Nov 2021 14:33:41 -0500 Subject: [PATCH 24/25] Add app-check-interop-types to externs.json. --- packages/firestore/externs.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/firestore/externs.json b/packages/firestore/externs.json index 7b60d0bbecf..003eed2d069 100644 --- a/packages/firestore/externs.json +++ b/packages/firestore/externs.json @@ -14,6 +14,7 @@ "packages/app-types/index.d.ts", "packages/app-types/private.d.ts", "packages/app/dist/app.d.ts", + "packages/app-check-interop-types/index.d.ts", "packages/auth-interop-types/index.d.ts", "packages/firestore/dist/lite/internal.d.ts", "packages/firestore/dist/internal.d.ts", From 9e6f82dba52a3c27b17d22b7d5757ebf38d672b8 Mon Sep 17 00:00:00 2001 From: Ehsan Date: Mon, 15 Nov 2021 16:02:40 -0600 Subject: [PATCH 25/25] Update .changeset/mean-elephants-rush.md Co-authored-by: Sebastian Schmidt --- .changeset/mean-elephants-rush.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/mean-elephants-rush.md b/.changeset/mean-elephants-rush.md index 28535a0ad64..8c06e72a276 100644 --- a/.changeset/mean-elephants-rush.md +++ b/.changeset/mean-elephants-rush.md @@ -1,5 +1,6 @@ --- "@firebase/firestore": minor +firebase: minor --- AppCheck integration for Firestore