[ServerApp] Firebase Server App feature branch #8005
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Baseline addition of the FirebaseServerApp object.
…IdToken (#7944) Add support for logging-in users with the FirebaseServerApp's authIdToken. ### Testing Local project testing client-side created users, passing idTokens to serverApps, and logging in the user. Tested with multiple users and multiple instances of FirebaseServerApps w/ Auth. CI tests (added integration tests). ### API Changes N/A
…ck later (#7989) Removed the appCheck and installations token parameters in FirebaseServerAppSettings as they won't be part of our initial launch. Additionally, update the the doxgen comments to no longer refer to these parameters, and to inform users that the User refreshToken will not work for User objects signed in with the authIdToken.
Mangle the name of the ServerAap based on the hash of the parameters passed in. In addition, return the same instance of app when the same parameters are used.
Update the `FirebaseServerApp` creation to return the same object if an existing object exists with the same configuration. However, the `deleteOnDeref` field is ignored when detecting duplicate apps, since that object reference could vary across multiple SSR rendering passes. The hope is that a `FirebaseServerApp` instance awaiting deletion from a the `deleteOnDeref` feature maybe be reused if another SSR pass occurs in rapid succession, there by speeding up the SSR code.
🦋 Changeset detectedLatest commit: db24403 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Changeset File Check ✅
|
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1This report is too large (448,338 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.Test Logs |
renkelvin
reviewed
Feb 7, 2024
Updates FirebaseServerApp implementation in Auth to prevent operations that would change the currently logged in user. The user should be that of the authIdToken provided to FirebaseServerApp only. Note: some of the method implementations currently reside in browser-only files. I added safe guards to these methods even though FirebaseServerApp is not supported in browser enviornments. These guards protect us in case the methods are later adapted to other environments and/or migrated to other files that are not browser-only. The changes to the browser implementations produce little overhead, so I thought that safety first was the correct call here.
egilmorez
reviewed
Feb 14, 2024
egilmorez
reviewed
Feb 26, 2024
egilmorez
approved these changes
Feb 28, 2024
hsubox76
approved these changes
Mar 6, 2024
| let auth: Auth; | ||
|
|
||
| beforeEach(() => { | ||
| auth = getTestInstance(); |
There was a problem hiding this comment.
Is this default instance ever used or are all the tests run on a getTestInstanceForServerApp() instance?
There was a problem hiding this comment.
Yes, the tests use this auth instance to create a new user (through varying means) and then uses that newly created user's idToken to initialize FirebaseServerApp.
DellaBitta
requested review from
a team,
markarndt,
kevinthecheung,
lisajian,
Xiaoshouzi-gh and
sam-gc
as code owners
4 tasks
jamesdaniels
approved these changes
Mar 14, 2024
It turns out the FirebaseServerApp implementation in #8005 was not blocking auth initialization, so when testing end-to-end we were seeing race conditions with auth state. In this PR I address by awaiting the user fetch and moving the implementation into AuthImpl#initializeCurrentUser for cleanliness.
We're rethinking how authIDTokenVerified should be used. It might reside in its own function. Remove the method for now.
Add a new serverapp string so that register version to track FirebaseServerApp usage.
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Discussion
Feature branch for the addition of FirebaseServerApp - a tool to ease the use of working with Firebase in SSR frameworks.
Testing
New CI tests in Auth and App, including unit and integration tests.
API Changes
API proposal approved.