Skip to content

fix(adapters): allow already initialized firebase app 🐛 #6230

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.

Sign up for GitHub

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

Jump to bottom
Merged
ThangHuuVu merged 3 commits into from
Jan 25, 2023
Merged

fix(adapters): allow already initialized firebase app 🐛 #6230

ThangHuuVu merged 3 commits into from
Jan 25, 2023

Conversation

@jsellam
Copy link
Contributor

@jsellam jsellam commented Dec 30, 2022 •
edited
Loading

NOTE:

  • It's a good idea to open an issue first to discuss potential changes.
  • Please make sure that you are NOT opening a PR to fix a potential security vulnerability. Instead, please follow the Security guidelines to disclose the issue to us confidentially.

☕️ Reasoning

Firebase adapter can't be used with an already initialized firebase app.

Before initializing a new app, we search if an app with the same name has been initialized

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

[[Please scout and link issues that might be solved by this PR.]

#5029
#4927

Fixes: INSERT_ISSUE_LINK_HERE

📌 Resources

@jsellam jsellam requested a review from ndom91 as a code owner December 30, 2022 10:30
@vercel
Copy link

vercel bot commented Dec 30, 2022

Someone is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Dec 30, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 25, 2023 at 0:38AM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
next-auth-docs ⬜️ Ignored (Inspect) Jan 25, 2023 at 0:38AM (UTC)

@github-actions github-actions bot added adapters Changes related to the core code concerning database adapters firebase @auth/firebase-adapter labels Dec 30, 2022
@ndom91 ndom91 changed the title 🐛 Fix already initialized firebase app fix(adapters): allow already initialized firebase app 🐛 Dec 30, 2022
@ndom91 ndom91 changed the title fix(adapters): allow already initialized firebase app 🐛 fix(adapters): allow already initialized firebase app 🐛 Dec 30, 2022
@ndom91
Copy link
Member

ndom91 commented Dec 30, 2022

LGTM, thanks 🥳

Can we merge this? @ThangHuuVu @balazsorban44

@jsellam jsellam force-pushed the fix/multi-firebase-init branch from 4f46db8 to 829baf5 Compare January 5, 2023 07:21
@balazsorban44 balazsorban44 force-pushed the main branch 6 times, most recently from 19c6807 to 3be7bb7 Compare January 7, 2023 08:50
@jsellam
Copy link
Contributor Author

jsellam commented Jan 9, 2023

Someone is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

Does I need to do something ?

@balazsorban44 balazsorban44 mentioned this pull request Jan 19, 2023
Merged
3 tasks
ThangHuuVu
ThangHuuVu approved these changes Jan 25, 2023
View reviewed changes
Copy link
Member

@ThangHuuVu ThangHuuVu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ThangHuuVu ThangHuuVu merged commit a336ba7 into nextauthjs:main Jan 25, 2023
@balazsorban44
Copy link
Member

balazsorban44 commented Jan 27, 2023

Heads up, this will be superseded by #6225 (incorporating this change too!)

balazsorban44 pushed a commit that referenced this pull request Feb 5, 2023
@wyattades
feat(adapters): move to firebase-admin in Firebase Adapter (#6225)
6445eb6 
Co-authored-by: Balázs Orbán <[email protected]>
fixes #5049
closes #6230
closes #5449
closes #5055
fixes #4927

BREAKING CHANGE:
The adapter now expects `firebase-admin` instead of the `firebase` package and also supports either passing a config object or a firestore instance.
balazsorban44 pushed a commit that referenced this pull request Feb 5, 2023
@wyattades @balazsorban44
feat(adapters): move to firebase-admin in Firebase Adapter (#6225)
4258857 
Co-authored-by: Balázs Orbán <[email protected]>
fixes #5049
closes #6230
closes #5449
closes #5055
fixes #4927

BREAKING CHANGE:
The adapter now expects `firebase-admin` instead of the `firebase` package and also supports either passing a config object or a firestore instance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ThangHuuVu ThangHuuVu ThangHuuVu approved these changes

@ndom91 ndom91 Awaiting requested review from ndom91 ndom91 is a code owner

Assignees

No one assigned

Labels

adapters Changes related to the core code concerning database adapters firebase @auth/firebase-adapter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jsellam @ndom91 @balazsorban44 @ThangHuuVu