Merge #1783

1783: Switch to server-side sessions r=mergify[bot] a=nextgens

## What type of PR?

bug-fix

## What does this PR do?

It simplifies session management.
- it ensures that sessions will eventually expire (*)
- it implements some mitigation against session-fixation attacks
- it switches from client-side to server-side sessions (in Redis)

It doesn't prevent us from (re)-implementing a "remember_me" type of feature if that's considered useful by some.


Co-authored-by: Florent Daigniere <nextgens@freenetproject.org>

core/admin/mailu/__init__.py

@@ -1,5 +1,8 @@
 import flask
 import flask_bootstrap
+import redis
+from flask_kvsession import KVSessionExtension
+from simplekv.memory.redisstore import RedisStore
 
 from mailu import utils, debug, models, manage, configuration
 
@@ -17,6 +20,7 @@ def create_app_from_config(config):
     # Initialize application extensions
     config.init_app(app)
     models.db.init_app(app)
+    KVSessionExtension(RedisStore(redis.StrictRedis().from_url('redis://{0}/3'.format(config['REDIS_ADDRESS']))), app).cleanup_sessions(app)
     utils.limiter.init_app(app)
     utils.babel.init_app(app)
     utils.login.init_app(app)

core/admin/mailu/configuration.py

@@ -1,5 +1,6 @@
 import os
 
+from datetime import timedelta
 from socrate import system
 
 DEFAULT_CONFIG = {
@@ -54,6 +55,7 @@ DEFAULT_CONFIG = {
     'RECAPTCHA_PRIVATE_KEY': '',
     # Advanced settings
     'LOG_LEVEL': 'WARNING',
+    'SESSION_LIFETIME': 24,
     'SESSION_COOKIE_SECURE': True,
     'CREDENTIAL_ROUNDS': 12,
     # Host settings
@@ -136,6 +138,8 @@ class ConfigManager(dict):
         self.config['QUOTA_STORAGE_URL'] = 'redis://{0}/1'.format(self.config['REDIS_ADDRESS'])
         self.config['SESSION_COOKIE_SAMESITE'] = 'Strict'
         self.config['SESSION_COOKIE_HTTPONLY'] = True
+        self.config['SESSION_KEY_BITS'] = 128
+        self.config['PERMANENT_SESSION_LIFETIME'] = timedelta(hours=int(self.config['SESSION_LIFETIME']))
         # update the app config itself
         app.config = self
 

core/admin/mailu/ui/forms.py

@@ -46,6 +46,8 @@ class ConfirmationForm(flask_wtf.FlaskForm):
 
 
 class LoginForm(flask_wtf.FlaskForm):
+    class Meta:
+        csrf = False
     email = fields.StringField(_('E-mail'), [validators.Email()])
     pw = fields.PasswordField(_('Password'), [validators.DataRequired()])
     submit = fields.SubmitField(_('Sign in'))

core/admin/mailu/ui/views/base.py

@@ -17,6 +17,7 @@ def login():
     if form.validate_on_submit():
         user = models.User.login(form.email.data, form.pw.data)
         if user:
+            flask.session.regenerate()
             flask_login.login_user(user)
             endpoint = flask.request.args.get('next', '.index')
             return flask.redirect(flask.url_for(endpoint)
@@ -30,6 +31,7 @@ def login():
 @access.authenticated
 def logout():
     flask_login.logout_user()
+    flask.session.destroy()
     return flask.redirect(flask.url_for('.index'))
 
 

core/admin/mailu/ui/views/users.py

@@ -119,6 +119,7 @@ def user_password(user_email):
         if form.pw.data != form.pw2.data:
             flask.flash('Passwords do not match', 'error')
         else:
+            flask.session.regenerate()
             user.set_password(form.pw.data)
             models.db.session.commit()
             flask.flash('Password updated for %s' % user)
@@ -186,6 +187,7 @@ def user_signup(domain_name=None):
         if domain.has_email(form.localpart.data):
             flask.flash('Email is already used', 'error')
         else:
+            flask.session.regenerate()
             user = models.User(domain=domain)
             form.populate_obj(user)
             user.set_password(form.pw.data)

core/admin/requirements-prod.txt

@@ -13,6 +13,7 @@ Flask==1.0.2
 Flask-Babel==0.12.2
 Flask-Bootstrap==3.3.7.1
 Flask-DebugToolbar==0.10.1
+Flask-KVSession==0.6.2
 Flask-Limiter==1.0.1
 Flask-Login==0.4.1
 Flask-Migrate==2.4.0

core/admin/requirements.txt

@@ -3,6 +3,7 @@ Flask-Login
 Flask-SQLAlchemy
 Flask-bootstrap
 Flask-Babel
+Flask-KVSession
 Flask-migrate
 Flask-script
 Flask-wtf

docs/configuration.rst

@@ -149,6 +149,8 @@ The ``CREDENTIAL_ROUNDS`` (default: 12) setting is the number of rounds used by
 
 The ``SESSION_COOKIE_SECURE`` (default: True) setting controls the secure flag on the cookies of the administrative interface. It should only be turned off if you intend to access it over plain HTTP.
 
+``SESSION_LIFETIME`` (default: 24) is the length in hours a session is valid for on the administrative interface.
+
 The ``LOG_LEVEL`` setting is used by the python start-up scripts as a logging threshold.
 Log messages equal or higher than this priority will be printed.
 Can be one of: CRITICAL, ERROR, WARNING, INFO, DEBUG or NOTSET.

towncrier/newsfragments/1783.misc

@@ -0,0 +1 @@
+Switch from client side sessions (cookies) to server-side sessions (Redis). This simplies the security model a lot and allows for an easier recovery should a cookie ever land in the hands of an attacker.