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

signIn callback does nothing when returning an object #751

Closed
1 of 5 tasks
eak12913 opened this issue Oct 7, 2020 · 6 comments
Closed
1 of 5 tasks

signIn callback does nothing when returning an object #751

eak12913 opened this issue Oct 7, 2020 · 6 comments
Labels
bug Something isn't working documentation Relates to documentation enhancement New feature or request help needed The maintainer needs help due to time constraint/missing knowledge

Comments

@eak12913
Copy link

eak12913 commented Oct 7, 2020

Describe the bug
In the documentation for signIn callback (https://next-auth.js.org/configuration/callbacks#sign-in-callback) I see that it should be possible to return

@return {boolean} Return true (or a modified JWT) to allow sign in

This leads me to believe that if I return some kind of object:

return { foo: "bar" }

Then this would get propagated the the jwt. This does not appear to be the case if you look at the code:
https://github.com/nextauthjs/next-auth/blob/main/src/server/routes/callback.js#L71

The only thing that happens with the result is that it's checked to see if it's false

Steps to reproduce

  1. sign in a user
  2. return an object with some sample property (foo: "bar") from the signIn callback
  3. place a log method in the jwt callback method

Expected behavior
foo: "bar" should appear as one of the properties in the jwt callback

Feedback
Documentation refers to searching through online documentation, code comments and issue history. The example project refers to next-auth-example.

  • Found the documentation helpful
  • Found documentation but was incomplete
  • Could not find relevant documentation
  • Found the example project helpful
  • Did not find the example project helpful
@eak12913 eak12913 added the bug Something isn't working label Oct 7, 2020
@eak12913
Copy link
Author

@iaincollins If you have a moment - could you please comment here. Is this an issue, an issue with docs or just an issue with me doing something wrong. Thanks!

@stale
Copy link

stale bot commented Dec 11, 2020

Hi there! It looks like this issue hasn't had any activity for a while. It will be closed if no further activity occurs. If you think your issue is still relevant, feel free to comment on it to keep it open. (Read more at #912) Thanks!

@stale stale bot added the stale Did not receive any activity for 60 days label Dec 11, 2020
@balazsorban44
Copy link
Member

I believe this is an issue with the documentation. We should probably update it to reflect the real behavior.

Or do the opposite and make the code do what the docs says.

@stale stale bot removed the stale Did not receive any activity for 60 days label Dec 11, 2020
@balazsorban44 balazsorban44 added documentation Relates to documentation enhancement New feature or request help needed The maintainer needs help due to time constraint/missing knowledge labels Dec 11, 2020
@eak12913
Copy link
Author

@balazsorban44, it would be convenient to be able to return a set of properties that get appended to the JWT. Right now, I get around by appending a new property to the req object in my signIn callback and then in the JWT callback, I check the req object and, if present, copy that property to the JWT.

@balazsorban44
Copy link
Member

balazsorban44 commented Dec 14, 2020

All right, I guess it should not be hard to forward the response from line 71 to the jwt callback. We only check if the response is explicitly false or an error anyway. I'm going to make a PR for this, and discuss with the others. I'll keep you updated.

UPDATE:
So @eak12913, I just remembered that I actually asked this exact thing before, and got this response: #728 (comment)

Meaning the error is in the docs, and the intended place to modify the jwt is only the jwt callback.

When that said, looking at the code below:

try {
const signInCallbackResponse = await callbacks.signIn(userOrProfile, account, OAuthProfile)
if (signInCallbackResponse === false) {
return redirect(`${baseUrl}${basePath}/error?error=AccessDenied`)
}
} catch (error) {
if (error instanceof Error) {
return redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`)
} else {
return redirect(error)
}
}
// Sign user in
const { user, session, isNewUser } = await callbackHandler(sessionToken, profile, account, options)
if (useJwtSession) {
const defaultJwtPayload = {
name: user.name,
email: user.email,
picture: user.image
}
const jwtPayload = await callbacks.jwt(defaultJwtPayload, user, account, OAuthProfile, isNewUser)

it doesn't look like that we can actually return a string url at the moment. Only throw "/some/url" would work, not return "/some/url".

Either way, there is something wrong with either the docs or the code, but for you the important part is that you should only modify the jwt in the jwt callback

@balazsorban44
Copy link
Member

Updated the code to handle returned strings correctly, and changed the docs that wrongly suggested that you can return a modify JWT. To reiterate, all JWT modifications should be done in the jwt callback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Relates to documentation enhancement New feature or request help needed The maintainer needs help due to time constraint/missing knowledge
Projects
None yet
Development

No branches or pull requests

2 participants