-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat(adapters): move to firebase-admin
in Firebase Adapter
#6225
feat(adapters): move to firebase-admin
in Firebase Adapter
#6225
Conversation
@wyattades is attempting to deploy a commit to the authjs Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
19c6807
to
3be7bb7
Compare
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.
Thanks for the PR! I am going through it right now!
I think we can just release it as a major bump of the existing adapter, so I made some changes. I'll test it, fix up the docs, and it will probably be good to merge! Nice work!
firebase-admin
in Firebase Adapter
60b4934
to
1cd5cbb
Compare
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.
Nice PR, I suggest one change regarding the NPM publish files 🙌
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.
Let's ship it!
This is now available under Docs at https://authjs.dev/reference/adapter/firebase Thanks for everyone following along! 🎉 Check it out, and give us feedback/reports bugs (in issues) if you find any. 🙏 |
Can't install it with yarn yet! |
This means that you won't need security rules cause the firebase adapter runs on the server and has full access to the DB. |
Not in npm repo yet? |
@Matgsan However the documentation is wrong I think, as it mentions
But in the examples on the same page, |
Migrating to the new adapter is giving me this error:
Firebase-admin: 11.5.0 I followed the documentation instructions and used the provided |
@dbeaudway Yes, I'm facing the same issue. If you import it from node modules using a relative path it should work for now. Adapter works great other than that. |
Have anyone tested this with Nextjs 13? I followed the instructions from https://authjs.dev/reference/adapter/firebase, using the credentials cert method. So when I sign in, it does create sessions, users, accounts, in firestore. but i get this error, and when I call getServerSession(), it is null.
Specifics: |
@RisKakaN Have you added a JWT env variable? |
@darrencarlin no i have not. is that required, if so, how do i add one? if i look in the browser cookies, the session token is a uuid. not sure if that is an issue. do you have a better example/guide i can follow other than what is in authjs. in case i am missing some basic steps. |
Sorry that's what I was referring to, the Here is my simple repo with a working example using google login https://github.com/darrencarlin/auth If that doesn't help feel free to upload your code and I can take a look |
thanks for the example, helps a lot! I tried your repo with my firebase config and app, and it works fine! so then there should be no issues with my google provider or firebase. the only difference i see is that i am using the new nextjs 13 app dir, and you are not. so maybe there is an issue with using the new app dir. i am also using middleware, but i dont see how that would be an issue. still getting error when i remove it. feel free to check out my test branch here. just setup your own env.local file with the necessary variables. https://github.com/RisKakaN/food-recipe-manager/tree/test-1 |
@RisKakaN I was able to solve the issue by following this comment (I didn't add ctx) #4075 (comment) Gives a console warning leading to this page https://next-auth.js.org/configuration/nextjs#getServerSession
But I am able to login without the error previous error. I think it's important to note this issue isn't related to the firebase adapter. |
Huge thanks @darrencarlin ! however, now i am running into the next issue :D in my middleware, i am using withAuth, so that i can print out what is happening in the authorized callback. the token there is null, therefore not allowing me to go to any protected pages. i think i read somewhere that this middleware currently does not support anything other that jwt session strategy, whereas the adapter uses a database session strategy. would you know any workaround/solution to this? otherwise i would have to do manual checks if session exists. the neat thing with middleware is that it handles it all for you. |
i ended up creating my own middleware logic. in the middleware withAuth authorized callback, i call an internal api endpoint, which in turn calls getSession to verify whether we have a session or not. the downside is that there is an extra step with calling an endpoint which calls getSession. |
☕️ Reasoning
This PR adds a new adapter for the Firebase Admin SDK. The current Firebase
next-auth
adapter (@next-auth/firebase-adapter
) uses the frontend Firebase SDK, and should not be used because it requires developers to loosen their Firebase security rules to accommodate the adapter. (For that reason, I would recommend deprecating@next-auth/firebase-adapter
if/when this new adapter is stable).This adapter is inspired by @NorbertHuethmay's PR #5628 (which for some reason was never merged?) but also adds: tests, more use of firestore transactions, and a
preferSnakeCase
option.🧢 Checklist
🎫 Affected issues
Fixes:
fixes #5049, closes #6230, closes #5449, closes #5055, fixes #5049, fixes #4927