-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat(core): allow clearing cookies from jwt()
#6337
Conversation
@iMrDJAi is attempting to deploy a commit to the authjs Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few comments:
- This should be done against https://github.com/nextauthjs/next-auth/tree/main/packages/core as
next-auth
only receives critical fixes at this point - Let's allow returning
null
injwt
which indicates that we should callsessionStore.clean()
. - Looks like you did changes to the
session
callback too, but I don't understand the reason. Could we revert that?
@balazsorban44 Hello, sorry for the delay, working on the changes that you have requested. Regarding the session callback, clearing the cookies could be done when fetching the session, not sure what else to return from it when the token is Does setting the Regarding the new option Then passing it to In this way, we could for example make a provider to sign out users, as we have their pervious state. |
So basically passing the previous token if I understand correctly. That should be a different PR. Could we split it please and simplify this PR? |
@balazsorban44 Alright! one thing left:
Working on it... |
@balazsorban44 Before creating a new PR, what should we call that option? |
don't think it should be an option, just give it as BTW this will resolve #3946 |
Just curious regarding ...
What exactly does "at this point" mean? Is next-auth end-of-life? |
@rolanday Auth.js will replace NextAuth.js. @balazsorban44 Actually that's a very good idea, we can pass both pervious and new tokens! I'll play with it a little bit then I'll create a PR. Edit: Here is the new PR link: #6357. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
☕️ Reasoning
Currently there is no way to invalidate session from the
jwt()
callback. This change allows clearing the user cookies whenjwt()
returnsa falsy valuenull.Also, a new provider option has been added,decodeJWT
, which allows passing the existing token of a user (if any) instead of the default one to thejwt()
callback after provider'sauthorize()
. This allows creating providers for the purpose of checking and invalidating existing user sessions.🧢 Checklist
🎫 Affected issues
Fixes: #6325