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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return new token from signIn callback #728

Closed
1 of 5 tasks
balazsorban44 opened this issue Sep 30, 2020 · 4 comments
Closed
1 of 5 tasks

Return new token from signIn callback #728

balazsorban44 opened this issue Sep 30, 2020 · 4 comments

Comments

@balazsorban44
Copy link
Member

balazsorban44 commented Sep 30, 2020

UPDATE:
I eagerly created both a PR and updated the type definitions to mitigate this. 馃

Describe the bug
I am trying to implement #583 in user land to create a Proof of Concept.

According to the docs, it should be possible to return a modified JWT token in the signIn callback, which I was assuming I could get in the jwt callback.

Steps to reproduce

async signIn(...args) {
  const account = args[1]
  const MAX_CONCURRENT_LOGIN = 1
  const numberOfSessions = await sessionsDB.getNumberOfSessions(account.id)
  if (numberOfSessions - 1 < MAX_CONCURRENT_LOGIN) {
    const sessionId = await sessionsDB.addSession(account.id)
    return {
      ...args[0],
      sessionId,
    }
  }
  return false
},

Expected behavior
I would like to pass a sessionId to the jwt callback.
Screenshots or error logs
If applicable add screenshots or error logs to help explain the problem.

Additional context
On line 71, the signIn callback response is saved, but the value is never used at line 92.

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)

Wouldn't it be enough to resue signInCallbackResponse at line 87 to create a defaultJwtPayload

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
@balazsorban44 balazsorban44 added the bug Something isn't working label Sep 30, 2020
balazsorban44 added a commit to balazsorban44/DefinitelyTyped that referenced this issue Sep 30, 2020
The [docs states](https://next-auth.js.org/configuration/callbacks#sign-in-callback)
>  Return `true` (or a modified JWT) to allow sign in
   *                           Return `false` to deny access

This change is also related to
nextauthjs/next-auth#728
nextauthjs/next-auth#730
@iaincollins iaincollins removed the bug Something isn't working label Oct 1, 2020
@iaincollins
Copy link
Member

Thanks for the PR. It sounds like there is some confusing stemming from a typo in comment in example code in the docs.

Including the response I've also left from the PR for completeness for anyone reading this:

The return type [for the signIn callback] was boolean, but now can be an Error object or a string (in which case a URL to redirect to is assumed) but cannot be a JWT. The intended use for this function is only to determine if a user should be allowed to sign in or not (and optionally to control what specific error message they see if they cannot; e.g. account suspended).

The only intended way to customise the the JWT response is to use the jwt callback, as in the example.

With NextAuth.js you can either use JSON Web Tokens for sessions or use a database for Sessions but you'd need to pick which. Tracking JWTs have been issued in a database is not a flow that is supported in NextAuth.js, it's very much either JWT or database for session access.

If you want to prevent additional sign in attempts, it should be pretty straightforward - you can use database sessions and use the signIn method to either return false (or an error / URL) if a session already exists to prevent them being able to sign in or to call the database an delete any existing sessions that exist for that user and then return true to allow a new one to be created (depending on which behaviour you want).

If you want to use a JWT, in the jwt callback you can detect when it is called at sign in for the first time, and could then add anything to the token you like at that point. The call is async so you can also (conditionally) do things like record in a database that a session was created for that user (you could also use the event callbacks for this).

In theory you could combine logic in the JWT callback with a check in the signIn callback to prevent additional sign in attempts by the same user, but it's probably easier just to store users session tokens in a database as is less logic (unless you really want to use a JWT to optimise to avoid database read calls or for interoperability with another webservice on the same domain).

@balazsorban44
Copy link
Member Author

balazsorban44 commented Oct 1, 2020

Yeah, I aim to support allowing multiple concurrent logins, so the database will/should hold a list of sessions. My initial thoughts are to create a sessionId in the db, and send it back to the user at signin. When a new user is trying to sign in, the DB will also check first if the max no. of allowed concurrent logins is reached, in that case, it will drop the oldest login sessionId, and add a new one for the user currently signing in. After a while, the access token for the old user will expire, and in that case, I check if their sessionId is still present in the session DB. if not, the user should not be authorized anymore and the jwt token returned will be empty. Although, I am not entirely sure if it is the "proper way" to "log the user out". Here is my code at the moment (moving the code I intended to have in the signIn callback here as well):

// [...nextauth.ts]
async jwt(token: JWTTokenWithUser, _, account: IDSAccount, profile: IDSUser) {
  // Signin in
  if (account && profile) {
    const numberOfActiveSessions = await sessionsDB.getNumberOfSessions(account.id)
    if (MAX_CONCURRENT_LOGINS <= numberOfActiveSessions ) {
      return {}
    }
    const accessTokenExpires = new Date(account.accessTokenExpires ?? addSeconds(new Date(), DEFAULT_EXPIRES_IN )).toISOString()
    const jwtToken: JWTTokenWithUser = {
      accessToken: account.accessToken,
      accessTokenExpires,
      id: account.id,
      refreshToken: account.refreshToken,
      sessionId: await sessionsDB.addSession(account.id),
      user: mapIDSProfileToUser(profile),
    }
    return jwtToken
  }
  // Subsequent use of JWT, the user has been logged in before
  // access token did has not expired yet
  if (new Date().toISOString() < token.accessTokenExpires) {
    return token
  }
  // access token has expired, check if the session is still valid.
  if (await sessionsDB.isActiveSession(token.id, token.sessionId)) {
    return refreshAccessToken(token)
  }
  // the session is not valid anymore, annul the jwt token
  return {}
},
//...

UPDATE:

I just tested it, and it kind of works, but returning an empty object throws the following error:

[next-auth][error][jwt_session_error] JWEInvalid: JWE malformed or invalid serialization

And sends me to the following documentation page https://next-auth.js.org/errors#jwt_session_error, which is empty. I solved it by adding some kind of an error in the JWT eg.:

// the session is invalid, annul the jwt token
return {
  error: {
    message: "This user session has expired.",
    type: "SessionExpired",
  },
}

Which is actually fine.

@iaincollins
Copy link
Member

If you move a check like this into the signIn() callback - and leave the rest of the logic in the jwt() callback -that would allow you to control the specific error displayed to the user, by returning an error code or error page URL from the signIn method:

const numberOfActiveSessions = await sessionsDB.getNumberOfSessions(account.id)
if (MAX_CONCURRENT_LOGINS <= numberOfActiveSessions ) {
  return false // or return URL to redirect to
}

@balazsorban44
Copy link
Member Author

I got it working! I will close this issue now, and open a new one if I have further questions. Thanks for the help and explanation!

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 a pull request may close this issue.

2 participants