pallets-eco / flask-session

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

SqlAlchemy: TypeError if SESSION_PERMANENT=False #67

Closed Ardius closed 8 months ago

Ardius commented 7 years ago

Using SqlAlchemy (both with mysql and sqlite) if I set SESSION_PERMANENT to False the system return a TypeError

File "/home/draio/project/venv/lib/python2.7/site-packages/flask_session/sessions.py", line 516, in open_session
     if saved_session and saved_session.expiry <= datetime.utcnow():
 TypeError: can't compare datetime.datetime to NoneType

Changing the SESSION_TYPE in filesystem the system works and the cookie will be deleted when the browser is closed.

On the DB table the session row has the expiry row empty. Expiry comes from expires variable tha is set on row 547: expires = self.get_expiration_time(app, session)

get_expiration_time is a flask fuction that returns the expiration date for the session:

 if session.permanent:
            return datetime.utcnow() + app.permanent_session_lifetime

That's why the expiry field on sessions table is empty, a cookie set as non-permanent doesn't have a expiration date. So it's not possible for mysql and sqlite to check a NULL date.

To populate the expiry field in the DB table I changed: expires = self.get_expiration_time(app, session) into

expires = self.get_expiration_time(app, session) \
           or datetime.utcnow() + app.permanent_session_lifetime

No more error but if i close the browser the ,session cookie is still there. That's because the set_cookie function must have expires empty to delete the cookie when browser is closed.

So I changed: expires = self.get_expiration_time(app, session) into

expires = self.get_expiration_time(app, session) \
           or datetime.utcnow() + app.permanent_session_lifetime
expires_cookie=expires if self.permanent else ''

And on line 561 I changed:

response.set_cookie(app.session_cookie_name, session_id,
                    expires=expires, httponly=httponly,
                    domain=domain, path=path, secure=secure)

into

response.set_cookie(app.session_cookie_name, session_id,
                    expires=expires_cookie, httponly=httponly,
                    domain=domain, path=path, secure=secure)

I hope that this could by helpful and clear.


UPDATE

Another way, more elegant, to solve the problem is to check self.permanent before checking if saved_session.expire is outdated.

On line 516 change if saved_session and saved_session.expiry <= datetime.utcnow(): into something like if self.permanent and saved_session and saved_session.expiry <= datetime.utcnow():

Also this solution works but, if I'm not wrong, in this way the session will never be deleted from the DB table and, more important, it will never be tested. That's could be a security problem.

In the other solution the expiration date is written to the DB. In that case the problem may be that for a time a cookie remains active even if on the browser has already been deleted. I do not know if this could be considered a security problem.


UPDATE IE11 issue

On IE11 you cannot have a Expires=;, otherwise the cookie will never be stored. For now I solved brutally with an if on

if not saved_session:
     if expires_cookie:
         response.set_cookie(app.session_cookie_name, session_id,
                                          expires=expires_cookie, httponly=httponly,
                                          domain=domain, path=path, secure=secure)
     else:
         response.set_cookie(app.session_cookie_name, session_id,
                                           httponly=httponly,
                                           domain=domain, path=path, secure=secure)
NightBlues commented 6 years ago

Same bug for mongodb backend.

nagappan commented 6 years ago

I made this fix for Mongo when testing with curl

    if document:
        expiration = document.get('expiration')
    else:
        expiration = None
    if document and expiration and expiration <= datetime.utcnow():

Worked for me

zardilior commented 5 years ago

@negappan could you pull request?

macTracyHuang commented 4 years ago

Same issue in PostgreSql

DLAcoding commented 1 year ago

Same here, solved with:

if saved_session and saved_session.expiry:
    if saved_session.expiry<= datetime.utcnow():
manugarri commented 1 year ago

is there any way to fix this without changing the app code? We are experiencing this bug on AWS Airflow 2.4.3 and we cant really change airflow's version there

Danipiario commented 1 year ago

I have the same problem with a fresh installation on AWS Airflow 2.4.3 I've substituted in mwwa requirements flask-session lib with Flask-Session2 (https://github.com/christopherpickering/flask-session2), the code was changed in release 1.2.0 (https://github.com/christopherpickering/flask-session2/pull/4) due to this bug

manugarri commented 1 year ago

For what is worth, clearing browser data seems to 'fix' the issue

Danipiario commented 1 year ago

Yes, clearing the session data fix the access but temporarily until the session expires again. I've experienced the issue more times

giom-l commented 1 year ago

Hi @Danipiario , Did the update of flask-session lib in mwaa requirements fix the issue ? Or are you still encountering it in mwaa 2.4.3 despite the update ?

Danipiario commented 1 year ago

So far the workaround works

wilminator commented 1 year ago

FYI I have found a fork of this project on Pypi flask-session2 that is actively maintained and does not have this problem. I was able to use it in my project as a drop-in replacement.

Lxstr commented 8 months ago

Fixed since 0.6.0. Please report any further issues

HammadAwan424 commented 8 months ago

It doesn't give error but now nothing interesting happens. After setting up session, it returns None if you try to retrieve session data.

Lxstr commented 8 months ago

@HammadAwan424 can you show me a minimum example? Also can you try 0.7.0rc2?

If you are upgrading from 0.5.0 while there are existing sessions, it will remove those sessions that had expiry None.