-
Notifications
You must be signed in to change notification settings - Fork 915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ServerApp] Update feature branch Auth implementation to use the authIdToken #7944
Merged
DellaBitta
merged 47 commits into
feature-firebaseserverapp
from
ddb-serverapp-use-auth-token
Jan 26, 2024
Merged
Changes from 42 commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
1216622
initial pass of additions
DellaBitta 2efe043
another pass, optional callback failures
DellaBitta fda3060
compilation fixes
DellaBitta 5fa9c21
FirebaseServerAppImpl now extends FirebaseAppImpl
DellaBitta 0ef3bb2
removed Headers object, added three callbacks instead
DellaBitta 8cd1072
added deleteOnDeref support
DellaBitta d42f676
lint fixes
DellaBitta 612f990
deleteOnDeref type converted from WeakRef to object.
DellaBitta 76f2fe1
update cookie func signatures. Check for existence of FinalizationReg…
DellaBitta d68f986
added baseline firebaseServerApp.test.ts
DellaBitta 09e3157
update callback method siganatures with shorter names and alt undef p…
DellaBitta 3750aa4
added some firebaseServerApp tests
DellaBitta 316309c
Merge branch 'master' into ddb-server-app
DellaBitta 6cc428b
format
DellaBitta 53c15e2
Merge branch 'master' into ddb-server-app
DellaBitta 22b9bd9
rework builds, tests need updating
DellaBitta 3c39866
another API update and doc generation
DellaBitta d5e461e
Merge branch 'master' into ddb-server-app
DellaBitta a7d111a
reflect new API proposal in source
DellaBitta 0427934
FirbaseServerAppSettings name?: undefined
DellaBitta db9b941
merge master
DellaBitta e146815
remove firebaseServerApp name
DellaBitta e9f28ed
remove engines from package.json
DellaBitta a13f418
FirebaseApp / Options typeguard. More tests.
DellaBitta 8a745d1
Update FirebaseServerAppConfig name to be undefined.
DellaBitta 7b02583
throw if firebaseServerApp constructed in browser. Remove lib ESNext …
DellaBitta d86eeb7
throw in initializeServerApp if invoked in a browser env.
DellaBitta b34b878
Update index.d.ts
DellaBitta 4aeb9a6
Update index.d.ts
DellaBitta 04af931
Remove the new FirebaseServerApp from the app-types.
DellaBitta 968e176
Leverage the authIdToken to login users in FirebaseServerApp instances
DellaBitta b5a84b0
merged in feature branch
DellaBitta 826a3c4
void promise
DellaBitta e2a0f9a
const providerData
DellaBitta a6bdf3e
provider data initialized inline
DellaBitta c323811
format and getToken test update
DellaBitta e8151d6
more stsmanager tests
DellaBitta 8e6ab2d
fix test deep-equal
DellaBitta 1980c7b
[ServerApp] Add integration tests for FirebaseServerApp (#7960)
DellaBitta 6543206
add comment about authIdToken string member of FirebaseServerAppImpl
DellaBitta 29fa942
remove duplicate assertions in getToken.
DellaBitta bb9ba8a
Merge branch 'feature-firebaseserverapp' into ddb-serverapp-use-auth-…
DellaBitta 940ec40
test for invalid token
DellaBitta 8bf5286
Fix tests for emulators
DellaBitta 3b066fa
Fix isFirebaseSeverAppImpl export problem
DellaBitta 020687b
formatter.
DellaBitta 63cf1fb
Skip loading users from persistence with FirebaseServerApps.
DellaBitta File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're exporting this out of
@firebase/app
you should also exportFirebaseServerAppImpl
, as its typing depends on that type, otherwise you could run into some TS errors. The other alternative is to be a little looser with the typings and not do theobj is FirebaseServerAppImpl
and just doboolean
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, I was exporting
FirebaseServerAppImpl
so that Auth could get the authIdToken from it. I didn't realize that it would then be part of the public API, too, but that makes total sense that it is.I'll work to make it
FirebaseServerAppImpl
to say withinapp
only, and figure out something else forauth
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, to be clear,
FirebaseServerAppImpl
isn't being exported at the moment, as far as I can tell, which is what api-extractor is warning you about._isFirebaseServerAppImpl
is being exported. But since the typing of_isFirebaseServerAppImpl
depends onFirebaseServerAppImpl
, api-extractor is complaining that you have to export that too. The way to get around it is to not use theobj is
return type and return a boolean instead.The internal.ts file isn't "public" in that it's not meant to be used be users of the SDK, it's meant to export "internal" methods used by other packages, like auth, so I think this is fine to export this function in that file (if you check
dist/
after building you should see it show up in the "internal" bundle and not in the main bundle). The only problem is that there may possibly be some TS issue at some point if you don't resolve this warning, so I think the simplest way is to change the return type to boolean.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I was saying to fix the error I would have needed to export
FirebaseServerAppImpl
, and I didn't want to do that.This should be fixed now. I've updated the function to simple by
isFirebaseServerApp
and notisFirebaseServerAppImpl
.FirebasServerApp
is already a public interface so we should be good to go here.