django-user-accounts
django-user-accounts copied to clipboard
Session expiry is incorrectly set to expire at browser close
While debugging a full-screen app (created from mobile Safari's feature of adding a site to the home screen), I discovered the undesirable behavior of being logged out when "closing" the app.
After digging into this issue I discovered that the login_user
method of both SignupView
and LoginView
is calling request.session.set_expiry(0)
. LoginView
is conditional on whether the user asked for the site to remember them in which cases uses the value in ACCOUNT_REMEMBER_ME_EXPIRY
.
I tried to figure out if there is any reason for this behavior. The best answer I could come up with takes us back to Django pre-1.0. The remember me behavior was added here:
https://github.com/pinax/pinax/commit/caec0ffd166b0b770f18aab878da2072d3efa7ed
During this time it was pretty common for folks to run off of Django trunk. This means that reading code was typical for the early Pinax team as a replacement for documentation. The code at the time read like this for the getter methods around session expiry:
https://github.com/django/django/blob/stable/1.0.x/django/contrib/sessions/backends/base.py#L180 https://github.com/django/django/blob/stable/1.0.x/django/contrib/sessions/backends/base.py#L192
However, the setter read correctly:
https://github.com/django/django/blob/stable/1.0.x/django/contrib/sessions/backends/base.py#L202
That's the best I can do to explain it. It was a fun little rabbit hole, but let's get on to figuring out what to do moving forward.
I think the behavior of falling back to Django's SESSION_COOKIE_AGE
is the right way to go, but it raises some questions. It seems to be slightly at odds with ACCOUNT_REMEMBER_ME_EXPIRY
. I am inclined to go the direction of correctly falling back by using None
instead of 0
and documenting this behavior. A site developer will want to be aware of SESSION_COOKIE_AGE
allowing them make the right decision on what it's value should be.
With all that said, this is a backward incompatible change IMO, but the current behavior just seems wrong to me. I'd love to know if anyone is aware of the current behavior and thinks it is good? Any other thoughts on this are most welcome!
After setting the expire session property to 0, the current session should be closed immediately after closing the browser. But in reality, the session time is set to the value of the SESSION_COOKIE_AGE variable set in Django global settings. The situation will be the same if you set expire to None. This is most likely due to an error in Django. When you set the expire property to 0, this property really becomes zero. But, when it comes time to calculate the lifetime of the session, before the calculation is checked, whether the value is set to expire. If this value is None, the default value must be taken. It happens like this:
expiry = expiry or settings.SESSION_COOKIE_AGE # Checks both None and 0 cases
This line is taken from a file django\contrib\sessions\backends\base.py, Django version 2.1.1, line # 243. As can be seen, even in comments it is specified that is checked as None and 0. And really logical operator "0 or 100" will give the value 100. Therefore, as a solution to this problem, I recommend to set expire in 1, that is, in 1 second.