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

Replace MemoryStore with CookieStore #50

Closed
hoangvvo opened this issue Oct 19, 2019 · 5 comments
Closed

Replace MemoryStore with CookieStore #50

hoangvvo opened this issue Oct 19, 2019 · 5 comments
Labels

Comments

@hoangvvo
Copy link
Owner

MemoryStore is currently the default store in next-session. It saves all the data in the memory, which is obviously not suited for Production.

CookieStore stores everything in a signed cookie. The cookie is readable by the user but will not be tamperable.

Like MemoryStore, CookieStore will still be noted as not recommended and should only be used in development.

Any opinion?

@hoangvvo hoangvvo added the RFC label Oct 19, 2019
@hoangvvo
Copy link
Owner Author

The data is also irrevocable...

@louisgv
Copy link

louisgv commented Nov 2, 2019

Session-id in cookie seems totally fine to me in case of SPA-SSR: https://auth0.com/docs/login/spa/authenticate-with-cookies

@ealtunyay
Copy link
Contributor

Exposing Store and promisifyStore offers compatibility with express-session compatible (server-side) stores, and I'd like to see better support in this direction instead. I believe there is more value, users, and support there.

For example, I found this issue today because I tried to see if a nextjs server using next-session (using applySession inside getServerSideProps) and an express server using express-session and passport would share a session using connect-redis. They will use the same store, but express-session expects a signed cookie and will never pick up a session started by next-session. I did not look into why exactly next-session doesn't pick up a session created by express-session.

Perhaps not entirely relevant, but: CookieStore would align more with cookie-session which is a different thing and seems to have issues when using passportjs

@hoangvvo
Copy link
Owner Author

hoangvvo commented Apr 7, 2020

Exposing Store and promisifyStore offers compatibility with express-session compatible (server-side) stores, and I'd like to see better support in this direction instead. I believe there is more value, users, and support there.

For example, I found this issue today because I tried to see if a nextjs server using next-session (using applySession inside getServerSideProps) and an express server using express-session and passport would share a session using connect-redis. They will use the same store, but express-session expects a signed cookie and will never pick up a session started by next-session. I did not look into why exactly next-session doesn't pick up a session created by express-session.

Perhaps not entirely relevant, but: CookieStore would align more with cookie-session which is a different thing and seems to have issues when using passportjs

next-session does not sign the cookie like express-session like you mentioned. I don't like signing a session id because it does not really help if the session id is really securely generated/hard to guess, while costing a little bit performance. A (heated) discussion here

Security-wise though, it obviously is better to sign the cookie. A future version of next-session would allow us to define a custom sign/verify function to achieve this. Thanks for trying out the experiment.

@andreisena
Copy link

We're having this problem here. Our main application uses express-session and we are creating a new one with Next.js and next-session using RedisStore that needs to share the same session.

Express-session adds a 's:' and signs the cookie with cookie-signature using a secret defined on options as you may see here and here.

I was wondering if next-session could suport this or at least expose a method to parse the session id.

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

No branches or pull requests

4 participants