Skip to content
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

Merged
merged 27 commits into from
Feb 5, 2023

Conversation

wyattades
Copy link
Contributor

@wyattades wyattades commented Dec 30, 2022

☕️ 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

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

Fixes:
fixes #5049, closes #6230, closes #5449, closes #5055, fixes #5049, fixes #4927

@vercel
Copy link

vercel bot commented Dec 30, 2022

@wyattades 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

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

Name Status Preview Comments Updated
auth-docs ❌ Failed (Inspect) Feb 5, 2023 at 0:59AM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
next-auth-docs ⬜️ Ignored (Inspect) Feb 5, 2023 at 0:59AM (UTC)

@github-actions github-actions bot added the adapters Changes related to the core code concerning database adapters label Dec 30, 2022
Copy link
Member

@balazsorban44 balazsorban44 left a 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!

@balazsorban44 balazsorban44 changed the title add firebase-admin adapter feat(adapters): move to firebase-admin in Firebase Adapter Jan 19, 2023
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.

Nice PR, I suggest one change regarding the NPM publish files 🙌

packages/adapter-firebase/package.json Show resolved Hide resolved
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.

Let's ship it!

@balazsorban44 balazsorban44 merged commit 6445eb6 into nextauthjs:main Feb 5, 2023
balazsorban44 pushed a commit that referenced this pull request Feb 5, 2023
Co-authored-by: Balázs Orbán <info@balazsorban.com>
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
Copy link
Member

This is now available under @next-auth/firebase-adapter@2.0.0

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. 🙏

@Furkan-rgb
Copy link

Can't install it with yarn yet!

@nhuethmayr
Copy link

If this gets merged, does that mean the next auth Firebase adapter uses the admin SDK, meaning we can implement Firestore security rules?

This means that you won't need security rules cause the firebase adapter runs on the server and has full access to the DB.

@Matgsan
Copy link

Matgsan commented Feb 6, 2023

Not in npm repo yet?

@frederikatgcompany
Copy link

@Matgsan
It is available on NPM: https://www.npmjs.com/package/@next-auth/firebase-adapter (Look at the version, and the last publish date).

However the documentation is wrong I think, as it mentions @next-auth/firebase-admin-adapter instead of @next-auth/firebase-adapter.

npm install next-auth @next-auth/firebase-admin-adapter firebase-admin

But in the examples on the same page, @next-auth/firebase-adapter is used:
import { FirestoreAdapter } from "@next-auth/firebase-adapter".

@dbeaudway
Copy link

Migrating to the new adapter is giving me this error:

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '.../node_modules/@next-auth/firebase-adapter/utils' imported from .../node_modules/@next-auth/firebase-adapter/index.js

Firebase-admin: 11.5.0
Next-auth: 4.19.2
@next-auth/firebase-adapter: 2.0.0

I followed the documentation instructions and used the provided initFirestore function. Anyone else experiencing this?

@darrencarlin
Copy link

@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.

@martinso95
Copy link

Have anyone tested this with Nextjs 13? I followed the instructions from https://authjs.dev/reference/adapter/firebase, using the credentials cert method.
But I am getting Invalid Compact JWE error. See logs.

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.

[next-auth][error][JWT_SESSION_ERROR] https://next-auth.js.org/errors#jwt_session_error Invalid Compact JWE { message: 'Invalid Compact JWE', stack: 'JWEInvalid: Invalid Compact JWE\n' + ' at compactDecrypt (webpack-internal:///(sc_server)/./node_modules/jose/dist/node/esm/jwe/compact/decrypt.js:20:15)\n' + ' at jwtDecrypt (webpack-internal:///(sc_server)/./node_modules/jose/dist/node/esm/jwt/decrypt.js:12:100)\n' + ' at Object.decode (webpack-internal:///(sc_server)/./node_modules/next-auth/jwt/index.js:44:53)\n' + ' at async Object.session (webpack-internal:///(sc_server)/./node_modules/next-auth/core/routes/session.js:25:34)\n' + ' at async AuthHandler (webpack-internal:///(sc_server)/./node_modules/next-auth/core/index.js:157:37)\n' + ' at async getServerSession (webpack-internal:///(sc_server)/./node_modules/next-auth/next/index.js:86:21)\n' + ' at async RootLayout (webpack-internal:///(sc_server)/./src/app/layout.tsx:16:21)', name: 'JWEInvalid' }

Specifics:
next: 13.1.6
next-auth: 4.19.2
firebase-admin: 11.5.0
@next-auth/firebase-adapter: 2.0.0
Provider: Google

@darrencarlin
Copy link

@RisKakaN Have you added a JWT env variable?

@martinso95
Copy link

@darrencarlin no i have not. is that required, if so, how do i add one?
I have only added the config from the instructions.
then i also have NEXTAUTH_SECRET, generated with: openssl rand -base64 32.

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.

@darrencarlin
Copy link

Sorry that's what I was referring to, the NEXTAUTH_SECRET

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

@martinso95
Copy link

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.
i am also not getting the ERR_MODULE_NOT_FOUND mentioned above, even though i am on exact same versions.

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.
my expected scenario is to not get the jwe error, as well as being able to be directed to a logged in state in my app. right now, im just stuck in the login page.

https://github.com/RisKakaN/food-recipe-manager/tree/test-1

@darrencarlin
Copy link

@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

[next-auth][warn][EXPERIMENTAL_API] 
`getServerSession` is used in a React Server Component. 
https://next-auth.js.org/configuration/nextjs#getServerSession} 
https://next-auth.js.org/warnings#EXPERIMENTAL_API

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.

@martinso95
Copy link

Huge thanks @darrencarlin !
I totally missed that you had to pass in the auth config to getServerSession. Because it was not needed when not using the adapter.

however, now i am running into the next issue :D
have you tried this with a middleware, which blocks pages if you are not authenticated?
the issue is, even if i am logged in, and have a session, the middleware says that i am not authenticated. so i cant navigate to my protected pages.

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.

@martinso95
Copy link

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.
and then in my middleware, i define some protected routes.

the downside is that there is an extra step with calling an endpoint which calls getSession.
but the use of getSession here should be alright. and i am not aware of any other downsides or risks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters Changes related to the core code concerning database adapters core Refers to `@auth/core` firebase @auth/firebase-adapter
Projects
None yet