kleydon / prisma-session-store

Express session store for Prisma
MIT License
116 stars 18 forks source link

feat: support for logging out of all sessions #70

Closed kleydon closed 2 years ago

kleydon commented 2 years ago

Adds a function, destroyUsersSessions(), to enable deleting all sessions for a given user, given the session id for one of the given user's sessions.

Note: Use of this functionality requires that all sessions have been added to the store using the store's set() function with the set() function's session argument containing a (new) uid (user id / user name) property.

Question: This feature necessitate addition of a new uid (user id / name) field to schema.prisma. Could this create any backward-compatability issues? Can/should they be mitigated by this library?

kleydon commented 2 years ago

@wSedlacek, if you feel at all like giving this PR a review, I'd love to get your feedback.

kleydon commented 2 years ago

@wSedlacek - Appreciate you having having a quick look.

My initial thought was if this feature would be within scope of this project, perhaps this is something that express-session needs to offer and then we could simply provide the proper interface for that feature

I'd been back-and-forth on this as well; going to mull it over, and see if a request for this functionality has come up in the express-session project before.

At some point we should switch over to eslint, and update our tooling. I have been working on a similar high quality eslint config, but haven't had the time to get it published in any major way.

Good point! Well, something for the future...

zaniluca commented 2 years ago

Waiting for the merge! this would be very useful

kleydon commented 2 years ago

Hi @ZaniLuca, thanks for the feedback. Temporarily holding off, to verify that the express session team doesn't have an equivalent of this feature on their roadmap (which might result in future backward compatibility issues). Hoping to hear back from them this week.

HartS commented 2 years ago

This looks great so far, other than a couple of things I pointed out. Removing Promise.all and just making one database request seems like the biggest improvement to me, as it'll ensure the session deletions either all fail or succeed, and perhaps this could be an issue when there's a low connection limit setting for the prisma client?

Tomorrow I should have time to pull up the changes and give it a try in our project

kleydon commented 2 years ago

@HartS - Just merged the refinement you suggested.

Tomorrow I should have time to pull up the changes and give it a try in our project

Great - thanks!

Do post if this PR causes any trouble, or doesn't behave as you'd expect.

zaniluca commented 2 years ago

Hi, so you're not going to merge this pr? Some major conflicts from the express-session team?

kleydon commented 2 years ago

Hi @ZaniLuca - sorry I neglected to include the rationale in the comments for this PR.

Here's the issue; (quoting @ultimate-tester from a related discussion in the express-session repo):

The solution provided by revington [prefixing session ids with the user's username to facilitate the capability of deleting all of a user's sessions] is definitely a good one, it will do what you expect. Though, I am not a fan of solutions that involve loops. Especially when you have a high-traffic site, you will notice problems. On top of that, having the list of sessions per user is probably not the complete functionality requested. Usually sites that offer the functionality you describe also show what kind of device the session is associated with, the login time, the IP, and other metadata.

>Therefore, my suggestion is to leave the requested functionality out of session management but persist this information in the database. Upon login, the user session ID should be stored into the database together with all the metadata you want. You might think "But then you are storing data redundantly as my session ID is already in the database", but I disagree with that. You are merely storing a reference just as you would with other relations between tables.