search

multinet-app / multinet-server

Multinet server application
https://multinet-app.readthedocs.io
Apache License 2.0
4 stars 2 forks source link

Consider updating use of browser cookies #387

Closed jjnesbitt closed 4 years ago

jjnesbitt commented 4 years ago

Currently, we use the flask session object to store cookies. This gives us the advantage of automatically having a cryptographically signed dictionary that stores any cookies we'd like to store. Because this cookie is signed and not encrypted, it means that the contents of the cookies can't be tampered with, but the value of the cookie is still be visible to the user who holds it (all they have to do is decode the cookie, which is a base64 encoded string).

This isn't really an issue, as we're using https, and once https://github.com/multinet-app/multinet-server/pull/380 is merged, only trusted domains can make requests to the server, and the cookie is HTTP only anyway, meaning the browser shields its value from javascript. However, if for some reason we find it necessary to encrypt this cookie, we could do that as well. I recently found a library called itsdangerous, which provides an interface to encrypt dictionaries.

This means that only the server can actually read the data stored in the browser cookie (for which we'd probably use FLASK_SECRET_KEY as a key), and the user is just carrying around a cookie with useless data.

I'm not saying we should do this, but I found this library interesting and just wanted to put the idea out there.

waxlamp commented 4 years ago

This looks interesting. Can you point me to where it lets you do encryption? It seems like it's specifically meant for signing data, rather than encrypting it.

More than anything else, I found this bit interesting:

Signed objects can be stored in cookies or other untrusted sources which means you don’t need to have sessions stored on the server, which reduces the number of necessary database queries.

That relates to the conversation we had yesterday--if this is true and easy to do, then we don't have to come up with a database schema for storing multiple sessions per user on the serverside.

jjnesbitt commented 4 years ago

This looks interesting. Can you point me to where it lets you do encryption? It seems like it's specifically meant for signing data, rather than encrypting it.

You're right, I was actually mistaken. Flask session already does this, so I suppose this issue is sort of moot.

Signed objects can be stored in cookies or other untrusted sources which means you don’t need to have sessions stored on the server, which reduces the number of necessary database queries.

That relates to the conversation we had yesterday--if this is true and easy to do, then we don't have to come up with a database schema for storing multiple sessions per user on the serverside.

Personally I think the logic of a users login state should still reside on the server (i.e. never trust the client). It also means that the multiple logins would be sharing the same token in their cookie, which also gives off a smell for me.

waxlamp commented 4 years ago

Seems like it's moot then--Flask is already providing what itsdangerous does, and we want to build our own logic for multiple sessions ourselves? If that's true and you're satisfied, go ahead and close this issue as wontfix.

Thanks for the discussion!