-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
fix(next-auth): redirecting to /undefined
when signIn method throws an error
#11010
fix(next-auth): redirecting to /undefined
when signIn method throws an error
#11010
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Ignored Deployments
|
@andresgutgon is attempting to deploy a commit to the authjs Team on Vercel. A member of the Team first needs to authorize it. |
thanks @andresgutgon , could you help to add a test for this so that we don't regress? the change LGTM otherwise. |
No dependency changes detected. Learn more about Socket for GitHub ↗︎ 👍 No dependency changes detected in pull request |
What did I do to get mad at the bot? 😂 . Also why tests are not running? |
fb1b9f9
to
d74e166
Compare
d74e166
to
f9b1a41
Compare
f9b1a41
to
744ad4f
Compare
744ad4f
to
cef0f19
Compare
error @auth/core `Auth` function can return an internalResponse or a Standard Response. Internal response has a `redirect` attribute. The problem is that the logic inside `Auth` method when there is an error is returning a Standard web Response which doesn't have `redirect` attribute but we can get redirecting url by using `headers.get('Origin')`. I think this fix this issue: nextauthjs#11008
cef0f19
to
5cebb45
Compare
@ThangHuuVu I think I did tests and docs. I can see docs in my machine working and the tests I did are passing too. Thanks for this software and glad to help! |
@ThangHuuVu I think I reviewed all the comments. Let me know anything I need to change |
61abbe5
to
997aa02
Compare
997aa02
to
0d804ea
Compare
config | ||
) | ||
expect(redirectTo).toEqual( | ||
"http://localhost/api/auth/signin/nodemailer?" |
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.
@ThangHuuVu I made a test for that case. Is this what you meant?
…th-server-actions
/undefined
when signIn method throws an error
@ThangHuuVu please let me know if there's anything else I can do to move forward with these changes? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11010 +/- ##
==========================================
+ Coverage 38.74% 40.85% +2.11%
==========================================
Files 176 176
Lines 27887 27893 +6
Branches 1217 1242 +25
==========================================
+ Hits 10804 11397 +593
+ Misses 17083 16496 -587 ☔ View full report in Codecov by Sentry. |
…th-server-actions
thanks @andresgutgon , sorry I was absent for the week. I pushed a commit to simplify the tests. |
…rows an error (nextauthjs#11010) * Fix NextAuth redirecting to /undefined when signIn method throws an error @auth/core `Auth` function can return an internalResponse or a Standard Response. Internal response has a `redirect` attribute. The problem is that the logic inside `Auth` method when there is an error is returning a Standard web Response which doesn't have `redirect` attribute but we can get redirecting url by using `headers.get('Origin')`. I think this fix this issue: nextauthjs#11008 * Move action test to tests folder * Remove test-adapter and add reference to docs * Defend from undefined responseUrl * Fix linter * chore(test): simplify tests --------- Co-authored-by: Thang Vu <hi@thvu.dev>
…rows an error (nextauthjs#11010) * Fix NextAuth redirecting to /undefined when signIn method throws an error @auth/core `Auth` function can return an internalResponse or a Standard Response. Internal response has a `redirect` attribute. The problem is that the logic inside `Auth` method when there is an error is returning a Standard web Response which doesn't have `redirect` attribute but we can get redirecting url by using `headers.get('Origin')`. I think this fix this issue: nextauthjs#11008 * Move action test to tests folder * Remove test-adapter and add reference to docs * Defend from undefined responseUrl * Fix linter * chore(test): simplify tests --------- Co-authored-by: Thang Vu <hi@thvu.dev>
☕️ Reasoning
When using a custom signin page in a NextJS app and using a server action as explained in the docs:
https://authjs.dev/guides/pages/signin
When the auth fails. For example
error=ConfigurationError
it doesn't redirect to error page. It redirects to/undefined
The problem
I think that
@auth/core
Auth
function can return anInternalResponse
or a standard webResponse
. Internal response has aredirect
attribute. The problem is that the logic insideAuth
method when there is an error returns a Standard web Response that doesn't haveredirect
attribute but we can get a redirecting url by usingheaders.get('Location')
.What changes are being made? What feature/bug is being fixed here?
We check if the returned response is a next auth
InternalResponse
or a standard webResponse
. This way we always get redirecting URL.🧢 Checklist
🎫 Affected issues
Fixes: #11008