search

nasonfish / derp

Development Experience Reporting Platform
0 stars 3 forks source link

Are we doing sessions correctly? Did I ruin it? #4

Open nasonfish opened 5 years ago

nasonfish commented 5 years ago

I seem to have implemented a system which keeps track of sessions nicely. However, I have no idea what line 18 of src/derp/__init__.py is or if that has anything to do with it.

app.session_interface = PickleSessionInterface(path)

This should ~probably~ be investigated

nasonfish commented 5 years ago

I feel dumb because I think I may have sort of reinvented the wheel here with my session system.

From my understanding, a pickle session is some sort of system for storing data in a client's session without actually having that data stored in the browser.

When I was looking at the code for the first time, I saw something along the lines of:

session['duck_id'] = request.form['duck_id']
session['logged_in'] = True
...
if(session['logged_in']):
    # they successfully logged in

My immediate thought was "no no no no no! this means that the user can change the session data in their browser and impersonate any other user, easily! how could this have been written!"

Now I'm realizing it's a little more complex than setting a plaintext session cookie. I still don't quite understand the session system but it's looking to me like the user gets some sort of token which refers to the session data which is stored on the server, and we reference that data, meaning the user can't change it. That must be what the SECRET_KEY option in the config is-- it's some sort of key which encrypts/retrieves the data we're looking for.

So anyway, that's why I feel dumb. I did what I thought was "fixing" this issue, and made it so all the important session information is kept in the session table in the database. Then, the user's session array contains two items-- their user id and a randomly generated challenge token. When they send us a request, we check the user_id against the challenge (and their IP address) to verify if they are logged in or not, and who they are. Even if these attributes are kept in plaintext, a user will not be able to spoof another's login token.

Now I worry that we're going through two layers, by keeping the session challenge on the server and the user has a second challenge from the pickle session. I'm going to have to look into this more and figure out what exactly I did and how to slim it down a little bit.

Oops?