nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
24.23k stars 3.37k forks source link

TypeORM + Postgress: Unable to sign in The sign in link is no longer valid. #1968

Closed ghost closed 3 years ago

ghost commented 3 years ago

Question 💬

Hey! I faced problem with Email provider. I cannot verify Email request, it says:

Unable to sign in The sign in link is no longer valid. image

How to reproduce ☕️

Video example: https://i.imgur.com/n7TWTSZ.gif


Sometimes it's throwing (even on Email sending request) (node:24911) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'isConnected' of null

image

balazsorban44 commented 3 years ago

I think I found the problem. I'll try to fix it in the following days and let you know here.

ghost commented 3 years ago

I think I found the problem. I'll try to fix it in the following days and let you know here.

Thank you!

balazsorban44 commented 3 years ago

I pushed a fix here: https://github.com/nextauthjs/adapters/commit/ff3f3e0f402f769677fcca831473c2a9c24ad646

If you re-run npm install, it should update the @next-auth/typeorm-legacy-adapter version in your package-lock.json or yarn.lock (to @next-auth/typeorm-legacy-adapter@0.0.2-canary.128), and hopefully, the problem will be resolved. Could you please verify?

I also added some error handling, so hopefully even if something still wrong, it won't report an UnhandledRejectedPromise, but rather something like ADAPTER_CONNECTION_ERROR

By the way, thank you for the detailed bug report, it greatly helped me debugging this! :heart:

investorhubcom commented 3 years ago

Hi, I updated to 0.0.2-canary.128 but I am still getting an error. However I am using MongoDB and not Postgress image

balazsorban44 commented 3 years ago

Unfortunate. I'll have another look soon! 😵 So sorry for the inconvenience.

balazsorban44 commented 3 years ago

Update: I reverted my attempts of a better connection handling in https://github.com/nextauthjs/adapters/commit/0b5d8adb191e0929f9322a084ba74badd1643d43 (Link to the original implementation: https://github.com/nextauthjs/adapters/commit/3104f331af511ab17d1b1c6ee2e820876e8b28c4)

This means that the code should be back to how it was pre-moving the adapters out of the core. If you still experience issues, it indicates that the original implementation had this kind of problem as well!

Could you please run the commands I suggested above and see if it helps this time? (The adapter version this time is0.0.2-canary.129) If not, we will have to take a closer look at the adapter implementation.

We should definitely add some tests to the adapter to be able to quickly and confidently iterate on the code.

ghost commented 3 years ago

@balazsorban44 I updated @next-auth/typeorm-legacy-adapter to 129 and seems there's no problem with connection! Great job!

But.. unfortunately the issue with request verification is still relevant. Maybe im doing something wrong but I can't verify my E-mail. Please check this video: https://i.imgur.com/SIezEX3.mp4

I'm getting this error in debug mode: [next-auth][debug][typeorm_legacy__get_verification_request] undefined 3Dbc53738ad00defa9de29585071cb60066346fc8ad02=ab55a1a094aeb37a3a060 5afbf720da22a25d3e304de22b1b2f6ffad56fc501ae2488f2cd6f341d52590d { id: 'email', type: 'email', name: 'Email', server: 'smtp://127.0.0.1:2525', from: 'noone@lol.com', maxAge: 86400, sendVerificationRequest: [Function: sendVerificationRequest], signinUrl: 'http://localhost:4600/api/auth/signin/email', callbackUrl: 'http://localhost:4600/api/auth/callback/email' }

image

I received that link in my video: http://localhost:4600/api/auth/callback/email?= email=3Dtest%40test.com&token=3Dbc53738ad00defa9de29585071cb60066346fc8ad02= ab55a1a094aeb37a3a060

Is that correct link? Because if i delete = in debug console i can see E-mail. And those 3D begins are suspicious to me

Marked these places in picture: image

After deleting = in GET params: image

lcdproductions commented 3 years ago

sorry if this comes off the wrong way - but is there not test coverage covering this adapter?

Magic sign-in is one of the core reasons we went with next-auth and magic sign-in broke as soon as I fired up the app with the update.

balazsorban44 commented 3 years ago

And I'll be straight about it. Unfortunately, no the adapter is not tested at the moment, which is terrible, and we are aware of it. The previous main maintainer had plans adding good tests, but unfortunately being busy with work has never come to it. I kind of got handed the main responsibility over, and it is absolutely on our radar to test the adapters, and generally the source code of next-auth. Sine parts would be more straightforward, than others, but we are starting with the client side code testing. See #1937.

The only tested adapter at this moment is the new Prisma one, you can see its tests here:

https://github.com/nextauthjs/adapters/blob/canary/packages/prisma/tests/index.test.ts

I'm pretty sure even these could be improved to give more confidence. If you have suggestions, we are open!

investorhubcom commented 3 years ago

Hi, running into some next-auth issue here as well, (maybe it's related) let me know if it's not related to this one and I'll open another issue image

followbl commented 3 years ago

@balazsorban44 if you or powers to be broke down https://github.com/nextauthjs/next-auth/issues/1937 it to some bite sized tasks - I would be more than willing to help. This library is awesome and would love to help y'all ship with more confidence with some great tests.

balazsorban44 commented 3 years ago

@investorhubcom It seems unrelated, as it uses JWT (judging from getToken), which does not touch on the adapters code. You seem to have a custom API handler at /api/account/getAccount, so adding it to your reproduction of the new issue would be great. It seems like something is not set correctly. If I had to guess, you either set https://next-auth.js.org/configuration/options#secret or https://next-auth.js.org/configuration/options#jwt > secret wrong. But please open a separate issue with a reproduction, thanks!

@followbl that is nice to hear! Do comment on that issue, and we will get back when we have a rough plan of what and how we want to test, or give your suggestion there!

balazsorban44 commented 3 years ago

@Santokua to understand your issue, could you please provide a reproduction? We have CodeSandbox templates:

ghost commented 3 years ago

@balazsorban44 it's quite hard to make it work on sandbox. SMTP doesn't work there at all, no idea why. I can provide access to my repository for you, so you only will need to specify your database name and SMTP.

ghost commented 3 years ago

@balazsorban44 I sent invite to you

Just change .env after creating new DB and do yarn start

For SMTP dev use python: sudo python -m smtpd -n -c DebuggingServer localhost:2525


UPD: Tested it with https://github.com/nextauthjs/next-auth-typescript-example/tree/main/ locally, doesn't work for me neither. Even with default settings but

EMAIL_SERVER=smtp://127.0.0.1:2525

balazsorban44 commented 3 years ago

I'll have a look, thanks for the invite

ghost commented 3 years ago

@balazsorban44 I found out the problem. Problem is SMTP client or something like this.

This line http://localhost:4600/api/auth/callback/email?= email=3Dtest@test.com&token=3D43efc1e2ceddf0601ef6325276d1552= a028be8a460a4df3ee149edbef980a70d

Should be like: http://localhost:4600/api/auth/callback/email?email=test@test.com&token=43efc1e2ceddf0601ef6325276d1552a028be8a460a4df3ee149edbef980a70d

So: Every 3D and = symbols should be removed.

balazsorban44 commented 3 years ago

I seemed to have lost access to your repository, so I cannot see your code anymore. Also, I am not really sure what is happening with the encoding, but here is the relevant source code, it looks good to me: https://github.com/nextauthjs/next-auth/blob/0fae0c7a8eb224964e38da409ecb9ba0497e8c50/src/server/lib/signin/email.js#L29-L31

Could you please re-share your source code so I can have a look?

ghost commented 3 years ago

@balazsorban44 That's not my code problem but SMTP fake clients. This can issue is completed and if i will face something similar in production i will re-open and inform you! Thanks!

Asywalkers commented 2 years ago

Hi guys I have the same problem? Have you any idea?