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

fix: 🎣 remove trailing ? from signIn url #8466

Merged
merged 3 commits into from
Sep 6, 2023
Merged

Conversation

jwyce
Copy link

@jwyce jwyce commented Aug 31, 2023

☕️ Reasoning

Fixing a weird bug where some URL categorizers that VPNs use will falsely categorize one of the sign-in redirects as phishing since the URL ends in a ? when no parameters are provided to next-auth/react's signIn() function.

The fix was simply to not include the ? for query params when none are provided.

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

📌 Resources

Sorry, something went wrong.

@jwyce jwyce requested a review from ThangHuuVu as a code owner August 31, 2023 15:42
@vercel
Copy link

vercel bot commented Aug 31, 2023

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

Name Status Preview Comments Updated (UTC)
next-auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 6, 2023 5:28am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
auth-docs ⬜️ Ignored (Inspect) Visit Preview Sep 6, 2023 5:28am

@vercel
Copy link

vercel bot commented Aug 31, 2023

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

A member of the Team first needs to authorize it.

@github-actions github-actions bot added client Client related code legacy Refers to `next-auth` v4. Minimal maintenance. labels Aug 31, 2023
@jwyce jwyce changed the title fix: 🎣 avoid phishing categorization by VPNs fix: 🎣 remove trailing ? from redirect url Sep 2, 2023
@jwyce jwyce changed the title fix: 🎣 remove trailing ? from redirect url fix: 🎣 remove trailing ? from redirect url Sep 4, 2023
@jwyce
Copy link
Author

jwyce commented Sep 5, 2023

@balazsorban44 / @ThangHuuVu do you see any potential problems with merging this code into the next v4 patch release?

@jwyce jwyce changed the title fix: 🎣 remove trailing ? from redirect url fix: 🎣 remove trailing ? from signIn url Sep 5, 2023
@ThangHuuVu
Copy link
Member

I think we can merge this one! Thanks for the PR, @jwyce

@ThangHuuVu ThangHuuVu merged commit ebfdaec into nextauthjs:v4 Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Client related code legacy Refers to `next-auth` v4. Minimal maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants