firebase / firebase-js-sdk

Firebase Javascript SDK
https://firebase.google.com/docs/web/setup
Other
4.77k stars 874 forks source link

Database and Firestore throw away emulated auth credentials, if initialized first #4110

Open jamesdaniels opened 3 years ago

jamesdaniels commented 3 years ago

If Firestore or Database (w/useEmulator) are initialized before Firebase Auth and an emulated user had previously signed in then a 400 error is logged to the console and the emulated auth credential are disposed of.

This is unexpected as one does not need to load the auth SDK to get correct behavior with production tokens. As such I promote lazy-loading Auth only when it's needed. Also FWIW both AngularFire and ReactFire dynamically import the SDKs & it's leading to reports such as this https://github.com/angular/angularfire/issues/2656

My expectation would be that if useEmulator is called on Firestore or Database then the SDKs would not throw away emulated auth tokens, instead use them to contact the emulator, even if auth hasn't been initialized with useEmulator yet itself.

pechisworks commented 3 years ago

Here's a Stackoverflow Question with the same problem and a workaround: https://stackoverflow.com/questions/65025005/angularfireauth-emulator-login-is-lost-on-page-reload

Feiyang1 commented 3 years ago

What if the intention is to use prod auth with emulated Firestore/Database, then using the emulated token in cache would be wrong. I don't think the SDK should be responsible for guessing what user wants to do, and user should make sure auth.useEmulator() is called before using Firestore.

@samtstern once proposed to have an app level useEmulator() that lets you configure multiple SDK's emulator settings in a single atomic operation which should solve the problem, but we ended up choosing the current API. Should we revisit the decision?

jamesdaniels commented 3 years ago

I'm not suggesting that we only take emulated credentials, but to not throw them away & log the user out if your pointing at the emulator w/o first initializing Auth. FWIW it seems storage and functions don't do this (last I checked) it's only Firestore & database.

Say you (as a developer) have an app that heavily uses Firestore/Database on you main component but you only import Auth in components that require it (sign in, admin section, comments, etc.) To save the bytes on that critical first render (and because Auth is heavy) you code-split/lazy load it... or it's entirely a child component on a nested route (article -> comments) and is chunked automatically by your tooling. This isn't uncommon. Now you have a confusing experience when pointing at the emulator as you log out on every refresh or if using Hot reloading (say angular) everytime you save a file.

It's not always easy to control order of operations when you're trying to be side effect free, limit module scope, & do code splitting.

Further, SDK wise, I won't be able to add a work around for this in Angular/AngularFire without bumping a major. I'll only be able to at all because I'd have the Auth emulator settings in DI (global scope), could check for that, ensure the Auth chunk has been loaded, and then proceed with Firestore initialization. This would require AngularFire to have an entirely async API, which is the direction I'm steering it towards, but slowly. In ReactFire we'd never be able to build in a work around due to its design & would have to rely on docs to explain.

Feiyang1 commented 3 years ago

It's actually Auth throwing out the emulated credentials. Auth is immediately initialized when it's dynamically loaded before AngularFire calls firebase.auth() because Firestore/Database registered a listener on it. At this point before AngularFire has a chance to call auth.useEmulator(), Auth will try to read the cached credential and invalidate the emulated credential because it's operating in prod mode.

Storage and Functions work because they ask for auth token on demand( they don't register a listener), so auth is initialized later and we have the chance to call auth.useEmulator() before the cached credential is read.

I think what we need is a flag in the global scope that sets the Auth emulator state, and is read by the Auth SDK on initialization in order to operate in the correct mode regardless of the timing of the initialization. I can see 2 ways of doing it:

  1. Support useEmulator() on app level. e.g. app.useEmulator(options)
  2. Allow to enable Auth emulator mode using a global variable. e.g. window.FIREBASE_AUTH_EMULATOR = {}

@samtstern Any thoughts?

samtstern commented 3 years ago

I think this recent issue on firebase-tools is related: https://github.com/firebase/firebase-tools/issues/2926

And also this other one: https://github.com/firebase/firebase-tools/issues/2877

samtstern commented 3 years ago

@Feiyang1 I do think that app.useEmulator(options) is the best way to solve this, but it was pretty enthusiastically rejected in API review so maybe we should consider other options?

jamesdaniels commented 3 years ago

I think auth looking for a global for emulator initialization would be a fine solution. If I were making it I would use an array of args, so it wouldn't be a breaking change when auth decides to change their API to allow options like Firestore and others are doing:

globalThis.FIREBASE_AUTH_EMULATOR = ['localhost:9099', { /* some future args */ }];

// in auth/internal-auth initialization
if (globalThis.FIREBASE_AUTH_EMULATOR) {
  this.useEmulator(...globalThis.FIREBASE_AUTH_EMULATOR);
}

Or perhaps a object based on app name, incase they had multiple apps with different settings:

globalThis.FIREBASE_AUTH_EMULATOR = {'[DEFAULT]': ['localhost:9099']};

// in auth/internal-auth initialization
if (globalThis.FIREBASE_AUTH_EMULATOR?.[app.name]) {
  this.useEmulator(...globalThis.FIREBASE_AUTH_EMULATOR[app.name]);
}

Another option would be if firestore/database useEmulator took options for internal-auth. e.g, firestore.useEmulator(['localhost', 8080], { useAuthEmulator: 'localhost:9099' }). It could warn if auth has already been initialized with different options.

Though are there any other non-emulator settings (such as tenancy) that internal-auth will have expected auth to be initialized before-hand and will respond destructively to if not?

athoma13 commented 3 years ago

For those using AngularFire and are struggling with this issue: My work around - in app.module.ts, instead of using InjectionTokens to provide emulator details as recommended in #this link

Initialize your firebase app like so:

import firebase from 'firebase/app';
import { environment } from '../environments/environment';
...
if (!environment.production) {
  const app = firebase.default.initializeApp(environment.firebase, 'myapp');
  // NOTE: Sequence may be important initialize Auth first.
  app.auth().useEmulator('http://localhost:9099');
  app.firestore().settings({ host: 'localhost:8080',  ssl: false  });
  app.functions().useEmulator('localhost', 5001);
}
...

@NgModule({
  declarations: [],
  imports: [
     ...
    AngularFireModule.initializeApp(environment.firebase, 'myapp'),
    ...
 ]

Giving an app name (and I think you can give it '[DEFAULT]' as a name here and skip it in the imports declaration), the FirebaseAppFactory of AngularFire will return the existing instance, rather than creating a new one.

jamesdaniels commented 3 years ago

@athoma13 do note that that your solution loses all the benefits of dynamic imports. Your main bundle will have the entirety of Firebase & reduce your application's performance—even in prod.

A better solution might be:

src/app/firebase-initialization.ts

// Work around for https://github.com/firebase/firebase-js-sdk/issues/4110
import firebase from 'firebase/app';
import 'firebase/firestore';
import 'firebase/auth';
import 'firebase/functions';
import { environment } from '../environments/environment';

const app = firebase.default.initializeApp(environment.firebase, 'myapp');
app.auth().useEmulator('http://localhost:9099');
app.firestore().useEmulator('localhost', 8080);
app.functions().useEmulator('localhost', 5001);

src/app/firebase-initialization.prod.ts

// Don't do anything, lean on AngularFire's DI init

Import in your app module and do a file replacement in your production target in your angular.json.

jessycormier commented 3 years ago

@athoma13 do note that that your solution loses all the benefits of dynamic imports. Your main bundle will have the entirety of Firebase & reduce your application's performance—even in prod.

A better solution might be:

src/app/firebase-initialization.ts

// Work around for https://github.com/firebase/firebase-js-sdk/issues/4110
import firebase from 'firebase/app';
import 'firebase/firestore';
import 'firebase/auth';
import 'firebase/functions';
import { environment } from '../environments/environment';

const app = firebase.default.initializeApp(environment.firebase, 'myapp');
app.auth().useEmulator('http://localhost:9099');
app.firestore().useEmulator('localhost', 8080);
app.functions().useEmulator('localhost', 5001);

src/app/firebase-initialization.prod.ts

// Don't do anything, lean on AngularFire's DI init

Import in your app module and do a file replacement in your production target in your angular.json.

My apologies for pinging everyone on this issue. @jamesdaniels I follow all the concepts and reasoning behind your solution except for how to import this into the app module. Since nothing is exported in this file how or what needs to be done to accomplish this? Thank you for your input!

jamesdaniels commented 3 years ago

@jessycormier Firebase is not a pure library (in the functional programming sense). The Firebase app and it's auth, firestore, functions instances are initialized in the global scope. So if you simply import this as a "side-effect" in your app module import "firebase-initialization"; then it should resolve the emulator initialization issue at the loss of lazy-loading of auth, functions, et al. in development.

jessycormier commented 3 years ago

@jamesdaniels Thanks for this information. I had originally tried this but had seen some interesting side effects. Auth was now using the live env while still showing the banner on the bottom for it being emulated. I assume I have some other setup conflicts that I'll have to figure out. Thanks very much for your time and knowledge!

mattpenner commented 2 years ago

@athoma13 do note that that your solution loses all the benefits of dynamic imports. Your main bundle will have the entirety of Firebase & reduce your application's performance—even in prod. A better solution might be: src/app/firebase-initialization.ts

// Work around for https://github.com/firebase/firebase-js-sdk/issues/4110
import firebase from 'firebase/app';
import 'firebase/firestore';
import 'firebase/auth';
import 'firebase/functions';
import { environment } from '../environments/environment';

const app = firebase.default.initializeApp(environment.firebase, 'myapp');
app.auth().useEmulator('http://localhost:9099');
app.firestore().useEmulator('localhost', 8080);
app.functions().useEmulator('localhost', 5001);

src/app/firebase-initialization.prod.ts

// Don't do anything, lean on AngularFire's DI init

Import in your app module and do a file replacement in your production target in your angular.json.

I tried this work around but it does not seem to be working for me. While functions and firestore are corretly using the emulator auth is still hitting the cloud.

Also, I had two issues with the code throwing compiling errors:

  1. firebase.default.initializeApp gave the error "Property 'default' does not exist on type 'typeof firebase'.ts". I had to use firebase.initializeApp
  2. import 'firebase-initialization'; gave the error: ERROR in ./src/app/app.module.ts Module not found: Error: Can't resolve 'firebase-initialization' in 'my-angular-project-path\src\app' I had to use import './firebase-initialization';

I am using Angular 9.1.13, Firebase 8.9.1 and AngularFire 6.1.5.

Any ideas what I might be doing wrong?

davidecampello commented 2 years ago

Any update on this?

schmidt-sebastian commented 2 years ago

@jamesdaniels - Given the release of v9, do we still have a way forward here? This looks like it need to be addressed in Auth, but I am not sure if there are any implications for Firestore and Database.

athoma13 commented 2 years ago

Agreed, this is a non-issue in v9