Support getAuth() after getDatabase() #4889
Conversation
|
There was a problem hiding this comment.
Thanks for making the PR. Database now works well with synchronous Auth initialization regardless if it happens before or after getDatabase(), but it still has problem with asyn Auth initialization. Considering the following example:
const database = getDatabase();
const docRef = ref(database, 'Foo');
setTimeout(() => {
getAuth();
// access denied
get(docRef).then(d => {
console.log('database get async', d.val());
});
onAuthStateChanged(getAuth(), (user) => {
// access denied even in the onAuthStateChanged callback
get(docRef).then(d => {
console.log('database get async', d.val());
});
});
}, 1000)
The behavior is quite unexpected - both get() should send authenticated requests, but they did not. And I was really surprised that the get() in onAuthStateChanged() is also unauthenticated.
IIUC, AngularFire loads Auth asynchronously, so it's important to make Database work correctly with asyn auth initialization. @jamesdaniels, anything to add?
| @@ -36,13 +36,25 @@ export class AppCheckTokenProvider { | |||
| ) { | |||
| this.appCheck = appCheckProvider?.getImmediate({ optional: true }); | |||
| if (!this.appCheck) { | |||
| appCheckProvider?.get().then(appCheck => (this.appCheck = appCheck)); | |||
| appCheckProvider?.onInit(appCheck => (this.appCheck = appCheck)); | |||
There was a problem hiding this comment.
AppCheck does not use Provider.initialize() yet, so we need to use get() instead of onInit() for now. We do need to change AppCheck to use Provider.initialize() and change here accordingly, but let's do that in a separate PR.
There was a problem hiding this comment.
Reverted this part for now.
There was a problem hiding this comment.
Supporting synchronous loading of the RTDB SDK with asynchronous loading of Auth will be quite tricky and we should probably attempt to do this in a separate PR if @jamesdaniels deems it necessary. Lazy-loading Auth after getDatabase() should already work as long as the RTDB has not been used by the time getAuth() is called.
| @@ -36,13 +36,25 @@ export class AppCheckTokenProvider { | |||
| ) { | |||
| this.appCheck = appCheckProvider?.getImmediate({ optional: true }); | |||
| if (!this.appCheck) { | |||
| appCheckProvider?.get().then(appCheck => (this.appCheck = appCheck)); | |||
| appCheckProvider?.onInit(appCheck => (this.appCheck = appCheck)); | |||
There was a problem hiding this comment.
Reverted this part for now.
Binary Size ReportAffected SDKs
Test Logs |
This changes the RTDB SDK to allow getAuth() to be called after getDatabase(). Unlike Firestore, it does not go back into blocking mode if Auth is initialized after startup. This would require us to rewrite the entire networking layer to be asynchronous and I suspect that this simple change is enough to cover most use cases.
The simple fix here works because the initial connection attempt in the networking layer is asynchronous. Only was
this.connectedis set to true do we flip to a synchronous behavior and send all requests immediately.