New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add set time expiration timeouts for every user session #556

Closed
pdeloof opened this Issue Oct 20, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@pdeloof
Copy link

pdeloof commented Oct 20, 2018

Description of problem

As a new user of kiwitcms, i notice when i'm logged in web application,
my user session is unlimited and is always active.
I think it will be a good way to add a time out session parameters as recommended here : https://www.owasp.org/index.php/Session_Management_Cheat_Sheet

@atodorov

This comment has been minimized.

Copy link
Member

atodorov commented Oct 20, 2018

Sounds like a fair RFE, however I'm not confident we can find default values that will work for everyone. The defaults proposed by OWASP (5 minutes for high risk apps, 15-20 mins for moderate risk) is not convenient IMO. While testing there are often times where I have to wait for an hour before reporting the result. Having to login few times per day is going to be annoying.

OTOH there is this app: https://pypi.org/project/django-session-timeout/ can you confirm that it works (or doesn't work for you) ?

For the record: Django doesn't seem to have a built-in setting by which to control session expiration.

@pdeloof

This comment has been minimized.

Copy link

pdeloof commented Oct 20, 2018

i added in common.py :
MIDDLEWARE = [
'django.contrib.sessions.middleware.SessionMiddleware',
'django_session_timeout.middleware.SessionTimeoutMiddleware', # <-- line added
...
SESSION_EXPIRE_SECONDS = 300 # 5minutes
SESSION_EXPIRE_AFTER_LAST_ACTIVITY = True

and
RUN pip install django-session-timeout
in Dockerfile

Build new docker images run at localhost

so all works like a charm after 5 minutes without activity session is killed

I let you decide if you want add this enhancement for the community...

Thanks a lot

@atodorov

This comment has been minimized.

Copy link
Member

atodorov commented Oct 26, 2018

According to https://docs.djangoproject.com/en/2.1/ref/settings/#session-cookie-age SESSION_COOKIE_AGE is the setting which controls when browser session cookies will expire.

@pdeloof

This comment has been minimized.

Copy link

pdeloof commented Oct 28, 2018

A nice feature will be to add in Site administration page a section about session management.
Advantages will be :

  • no need to edit common.py and build docker image when need change value.
  • plain and easily way to change parameters session via an administration web page.

Note : maybe other parameters could be set in administration settings pages too, as
FILE_UPLOAD_MAX_SIZE, all mail parameters as EMAIL_*, ALLOWED_HOSTS etc...

It will be really more flexible and avoid to build many times the specific kiwitcms docker image.

@atodorov

This comment has been minimized.

Copy link
Member

atodorov commented Oct 31, 2018

Adding for reference: https://github.com/jazzband/django-constance looks like a good app to use for settings configuration in the DB.

For the settings that we use internally that is a viable option. However the settings used by other apps and Django try to read from the settings object and no 3rd party library will change that. We may have to resort to documenting how to mount the settings file from the outside to make it easier to override the settings without rebuilding the container.

atodorov added a commit that referenced this issue Dec 8, 2018

By default expire session cookies after 24hrs. Fixes #556
this is mostly used to document the correct setting so that people
who are concerned about it can find it more easily and override it!

Whatever default value we set as the default will not suit everyone
so I go with a value that I like. In practice you should be serving
Kiwi TCMS over https and in controlled environment so this should
be less of an issue.

@atodorov atodorov closed this in 871852a Dec 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment