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

Destructuring profile to allow override default fields #961

Closed
wants to merge 11 commits into from

Conversation

vtcaregorodtcev
Copy link

Feature: This change will allow adjusting provider options to get more information about accounts.

What and Why:

For example: By default, Facebook provider configured only to accept clientId and clientSecret, but if we want to fetch, link to the profile we need to adjust those options with the following scope: 'email user_link', and profileUrl: 'https://graph.facebook.com/me?fields=email,name,picture,link',. And then after the user is going to log in, we miss additional info from options because of hardcoded part at https://github.com/nextauthjs/next-auth/blob/canary/src/server/lib/oauth/callback.js#L172

I suggest a destructuring of profile object to override the default fields.

Checklist:

This change will not introduce new behaviour or functionality except for extended possibilities.

  • Documentation
  • Tests
  • Ready to be merged

@vercel
Copy link

vercel bot commented Dec 16, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/4ht3b45zc
✅ Preview: https://next-auth-git-patch-1.nextauthjs.vercel.app

@vercel vercel bot temporarily deployed to Preview December 16, 2020 10:21 Inactive
@vercel vercel bot temporarily deployed to Preview December 16, 2020 10:23 Inactive
@balazsorban44
Copy link
Member

Good point, I think I came to the same conclusion in #959, I didn't really understand why we don't propagate everything, but @iaincollins may have a reasoning here.

Although in the same file, on line 185 you can see we return the whole profile as well. Did you check if the info you need is available there in the jwt callback?

@vtcaregorodtcev
Copy link
Author

Good point, I think I came to the same conclusion in #959, I didn't really understand why we don't propagate everything, but @iaincollins may have a reasoning here.

Although in the same file, on line 185 you can see we return the whole profile as well. Did you check if the info you need is available there in the jwt callback?

Yup, I've checked signIn callback and data come as 3rd argument, but this looks not naturally. In this case, we need additional action to assign data to a user in a place where we only check the access of the user.

Anyway, looks like my PR will not solve this problem, bc I see there are several places where this behaviour with predefined fields is hardcoded, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants