search

pallets-eco / flask-session

Server side session extension for Flask
https://flask-session.readthedocs.io
BSD 3-Clause "New" or "Revised" License
501 stars 239 forks source link

Sessions are being saved to the datastore even if they are not permanant #79

Closed marcuspen closed 6 months ago

marcuspen commented 6 years ago

It seems that non-permanent sessions are being saved in the datastore.

For example, if SESSIONS_PERMANENT config option is set to False, Flask states that they should end when the browser is closed. However, in this plugin, the session is being saved to Redis (or whatever datastore you are using) with the default permanent session lifetime, whether the session is permanent or not. See: https://github.com/fengsp/flask-session/blob/master/flask_session/sessions.py#L166

Is this intended?

If not, I suggest that we add an if statement to check if the session is permanent before we save anything to a datastore.

We have come across an issue with this at my company, we will fork and raise a PR with our intended fix shortly...

halcyon370 commented 6 years ago

I have the same problem. It seems that adding 'session.permanent = False' before setting sessions will work.

wavesailor commented 5 years ago

I also have this issue.

Is the Flash_Session Extension still actively maintained?

Lxstr commented 7 months ago

It is now been transferred to pallets and maintained. @marcuspen I know this is old but can you confirm with me if your intention was to use permanent client-side and non-permanent server-sessions in the same application? That is not previously something I had considered nor thought possible.

I have so far intended to make all permanent and non-permanent sessions save to datastore AND use the expiry of PERMANENT_SESSION_LIFETIME. This was previously inconsistently implemented by the original author and I had taken that to be the intent.

Without saving to datastore the only place you could keep session data is client side, which Flask-Session cannot do and so makes me wonder about what the use case is. A reproduction would be great.

In the PR for #212 I have also included documentation on this. However, before I merge I would like to know more

Lxstr commented 7 months ago

Perhaps the intent was to simply prevent the empty sessions from being saved to storage, that would make sense to me given this was considered a previous bug and fixed.

Lxstr commented 6 months ago

Closing due to lack of activity and high likelihood this is already fixed. There is a section on 0.7.0 docs relating to permanent sessions.