gvwilson / web-tutorial

Web Programming for Data Scientists
https://gvwilson.github.io/web-tutorial/
Other
9 stars 0 forks source link

Implement persistent user sessions to survive server restarts #66

Open juananpe opened 2 months ago

juananpe commented 2 months ago

The current implementation has a significant issue with session persistence. When the server restarts, the sessions dictionary is reset, which invalidates all existing user sessions. This means that even if a user is still logged in on their client side (i.e., their browser still has a valid cookie), they won't be able to access their experiments after a server restart. Let's address this issue:

I have implemented a possible solution to this issue, but it requires persisting the sessions in a new table of the SQLite database. Maybe it is overkill... /cc @gvwilson

https://github.com/gvwilson/wp4ds/compare/main...juananpe:wp4ds:sessions?expand=1

gvwilson commented 2 months ago

I like this - do you think it belongs here, or should there be an entirely new lesson on session management?

juananpe commented 2 months ago

I would like to explore session management and JWT usage. Perhaps we could create a new lesson to cover these topics. I have also added a new commit to change the migrate_sessions target in the Makefile to not run the migrations again if temp.db is already there (otherwise, the 02_fwd_add_plaintext_passwords.sql would fail).

gvwilson commented 2 months ago

:+1: to adding a lesson on session management and JWT usage - 21_session would make a lot of sense. Regarding database migration, I've been copying data/lab.db to temp.db and re-running all migrations each time I restart the server because it only takes a second and because one of the next lessons (19_workflow) is going to start modifying the DB, so doing a reset each time the server restarts seemed safe. Happy to be convinced otherwise…

juananpe commented 2 months ago

so doing a reset each time the server restarts seemed safe

But if you reset the db each time the server restarts, then all the sessions persisted in the database will be lost (...like tears in rain... sorry, couldn't resist :)

In fact, this is already an issue: log in as a staff member, restart the server, the user seems logged in, but the session has been reset on the server so the cookie is invalid for the current user (but they don't know; from the user's point of view, it seems like a backend error).

gvwilson commented 2 months ago

Yup, that's definitely a bug - I tried to add an "erase cookie" button into the web page but couldn't get it working, then decided that explaining what's going wrong here would be useful. I'll file an issue saying that we need to delete cookies on startup in the browser in the early lessons, and another to say that it's OK for early lessons to re-set the database every time but it's not OK for later lessons (after session persistence) to do that. Good catch...

gvwilson commented 2 months ago

see #67 and the rename of this issue.

gvwilson commented 2 months ago

b355bad adds 21_sessions directory for this lesson.