Skip to content

feat: add isNewUser arg on the signIn callback#8478

Closed
kahveciderin wants to merge 12 commits intonextauthjs:mainfrom
kahveciderin:main
Closed

feat: add isNewUser arg on the signIn callback#8478
kahveciderin wants to merge 12 commits intonextauthjs:mainfrom
kahveciderin:main

Conversation

@kahveciderin
Copy link
Copy Markdown

@kahveciderin kahveciderin commented Sep 3, 2023 •
edited
Loading

This PR implements the isNewUser field on the signIn callback, to allow for the rejection of a potential sign up.

☕️ Reasoning

Sometimes we might want to reject the signIn event if the user doesn't already have an account.

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

Fixes #8272

@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 3, 2023 •
edited
Loading

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

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

@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 3, 2023

@kahveciderin 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 the legacy Refers to `next-auth` v4. Minimal maintenance. label Sep 3, 2023
@kahveciderin kahveciderin marked this pull request as draft September 3, 2023 00:27
@kahveciderin
Copy link
Copy Markdown
Author

kahveciderin commented Sep 3, 2023 •
edited
Loading

As far as I understand, the "Credentials" provider is intended to support use cases where one has an existing system one needs to authenticate users against, so a user cannot be a new user when using the Credentials provider, thus I've now added isNewUser: false to the signIn callback when signing in with the credentials provider to reflect that.

@kahveciderin
Export input validation on the account parameter on callbackHandler t…
fecd450 
…o reduce code duplication between callbackHandler and checkIfUserIsNew
@kahveciderin
Copy link
Copy Markdown
Author

kahveciderin commented Sep 3, 2023

I have exported the "input validation" lines to an external helper function, because the same thing with the callbackHandler was being validated in the new checkIfUserIsNew function. Also, I simplified the checkIfUserIsNew function's logic by first checking the email no matter what provider is used, and it checks if the account exists only after establishing that the email doesn't exist and the user is using oauth.

@kahveciderin kahveciderin marked this pull request as ready for review September 3, 2023 09:50
@kahveciderin
Copy link
Copy Markdown
Author

kahveciderin commented Sep 3, 2023

I think I forgot to stage my last changes, will be pushing them now.

@kahveciderin
Copy link
Copy Markdown
Author

kahveciderin commented Sep 3, 2023

This PR should now be ready to merge, would be happy to get some reviews.

tilgovi
tilgovi reviewed Oct 3, 2023
View reviewed changes
Comment thread packages/next-auth/src/core/routes/callback.ts
}

try {
const isNewUser = await checkIfUserIsNew({
Copy link
Copy Markdown

@tilgovi tilgovi Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be stored as let isNewUser = true on line 81 and set to false on line 89, couldn't it?

@balazsorban44 balazsorban44 force-pushed the main branch 3 times, most recently from fa96b45 to 65aa467 Compare October 24, 2023 00:35
@ndom91 ndom91 changed the title Add support for isNewUser on the signIn callback feat: add isNewUser arg on the signIn callback Dec 24, 2023
@april83c
Copy link
Copy Markdown
Contributor

april83c commented Mar 17, 2024

What's going on with this? Will it be merged?

@bilalmohib
Copy link
Copy Markdown

bilalmohib commented Apr 6, 2024

Please merge this work

@ehenoma
Copy link
Copy Markdown

ehenoma commented Jun 5, 2024

push

@siddharthd0
Copy link
Copy Markdown

siddharthd0 commented Jul 7, 2024

merge when

mcnaveen
mcnaveen approved these changes Sep 19, 2024
View reviewed changes
Copy link
Copy Markdown

@mcnaveen mcnaveen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🫡

@garshythoel
Copy link
Copy Markdown
Contributor

garshythoel commented Nov 1, 2024

Another bump here! Thanks

@stale
Copy link
Copy Markdown

stale Bot commented Jan 21, 2025

It looks like this issue did not receive any activity for 60 days. It will be closed in 7 days if no further activity occurs. If you think your issue is still relevant, commenting will keep it open. Thanks!

@stale stale Bot added the stale Did not receive any activity for 60 days label Jan 21, 2025
@stale
Copy link
Copy Markdown

stale Bot commented Feb 1, 2025

To keep things tidy, we are closing this issue for now. If you think your issue is still relevant, leave a comment and we might reopen it. Thanks!

@stale stale Bot closed this Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0ubbe 0ubbe Awaiting requested review from 0ubbe

@ThangHuuVu ThangHuuVu Awaiting requested review from ThangHuuVu ThangHuuVu is a code owner

3 more reviewers

@tilgovi tilgovi tilgovi left review comments

@mcnaveen mcnaveen mcnaveen approved these changes

@siddharthd0 siddharthd0 siddharthd0 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

legacy Refers to `next-auth` v4. Minimal maintenance. stale Did not receive any activity for 60 days

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow to block authorization depending on whether it is a new user or not

8 participants

@kahveciderin @april83c @bilalmohib @ehenoma @siddharthd0 @garshythoel @tilgovi @mcnaveen