From bcaf8b2e4add27bedaabb4e45c84b0bfbf34fa0b Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Tue, 20 Aug 2019 19:42:32 +0200
Subject: [PATCH] Move clean_session with the other session management method +
 add a check for session.hashed for additional security ?

---
 moulinette/authenticators/__init__.py | 20 +++++++++++++++++++-
 moulinette/core.py                    | 22 ----------------------
 moulinette/interfaces/api.py          | 20 ++++++++++++--------
 3 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/moulinette/authenticators/__init__.py b/moulinette/authenticators/__init__.py
index afbdc87e..aa6840ac 100644
--- a/moulinette/authenticators/__init__.py
+++ b/moulinette/authenticators/__init__.py
@@ -1,9 +1,10 @@
 # -*- coding: utf-8 -*-
 
+import os
 import gnupg
 import logging
 
-from moulinette.cache import open_cachefile
+from moulinette.cache import open_cachefile, get_cachedir
 from moulinette.core import MoulinetteError
 
 logger = logging.getLogger('moulinette.authenticator')
@@ -165,3 +166,20 @@ class BaseAuthenticator(object):
                 logger.error(error_message)
                 raise MoulinetteError('unable_retrieve_session', exception=error_message)
             return decrypted.data
+
+    def _clean_session(self, session_id):
+        """Clean a session cache
+
+        Remove cache for the session 'session_id' and for this authenticator profile
+
+        Keyword arguments:
+            - session_id -- The session id to clean
+        """
+        sessiondir = get_cachedir('session')
+
+        try:
+            os.remove(os.path.join(sessiondir, self.name, '%s.asc' % session_id))
+        except OSError:
+            pass
+
+
diff --git a/moulinette/core.py b/moulinette/core.py
index 05c8599b..6ccb6d0a 100644
--- a/moulinette/core.py
+++ b/moulinette/core.py
@@ -396,28 +396,6 @@ def init_interface(name, kwargs={}, actionsmap={}):
     return interface(amap, **kwargs)
 
 
-def clean_session(session_id, profiles=[]):
-    """Clean a session cache
-
-    Remove cache for the session 'session_id' and for profiles in
-    'profiles' or for all of them if the list is empty.
-
-    Keyword arguments:
-        - session_id -- The session id to clean
-        - profiles -- A list of profiles to clean
-
-    """
-    sessiondir = get_cachedir('session')
-    if not profiles:
-        profiles = os.listdir(sessiondir)
-
-    for p in profiles:
-        try:
-            os.unlink(os.path.join(sessiondir, p, '%s.asc' % session_id))
-        except OSError:
-            pass
-
-
 # Moulinette core classes ----------------------------------------------
 
 class MoulinetteError(Exception):
diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py
index ebe36f9a..2b41dfa7 100644
--- a/moulinette/interfaces/api.py
+++ b/moulinette/interfaces/api.py
@@ -14,7 +14,7 @@ from bottle import run, request, response, Bottle, HTTPResponse
 from bottle import abort
 
 from moulinette import msignals, m18n, env
-from moulinette.core import MoulinetteError, clean_session
+from moulinette.core import MoulinetteError
 from moulinette.interfaces import (
     BaseActionsMapParser, BaseInterface, ExtendedArgumentParser,
 )
@@ -251,10 +251,7 @@ class _ActionsMapPlugin(object):
         def _logout(callback):
             def wrapper():
                 kwargs = {}
-                try:
-                    kwargs['profile'] = request.POST.get('profile')
-                except KeyError:
-                    pass
+                kwargs['profile'] = request.POST.get('profile', "default")
                 return callback(**kwargs)
             return wrapper
 
@@ -362,7 +359,7 @@ class _ActionsMapPlugin(object):
                                 secret=s_secret)
             return m18n.g('logged_in')
 
-    def logout(self, profile=None):
+    def logout(self, profile):
         """Log out from an authenticator profile
 
         Attempt to unregister a given profile - or all by default - from
@@ -374,14 +371,21 @@ class _ActionsMapPlugin(object):
         """
         s_id = request.get_cookie('session.id')
         try:
-            del self.secrets[s_id]
+            # We check that there's a (signed) session.hash available
+            # for additional security ?
+            # (An attacker could not craft such signed hashed ? (FIXME : need to make sure of this))
+            s_secret = self.secrets[s_id]
+            s_hash = request.get_cookie('session.hashes',
+                                        secret=s_secret, default={})[profile]
         except KeyError:
             raise HTTPUnauthorizedResponse(m18n.g('not_logged_in'))
         else:
+            del self.secrets[s_id]
+            authenticator = self.actionsmap.get_authenticator_for_profile(profile)
+            authenticator._clean_session(s_id)
             # TODO: Clean the session for profile only
             # Delete cookie and clean the session
             response.set_cookie('session.hashes', '', max_age=-1)
-            clean_session(s_id)
         return m18n.g('logged_out')
 
     def messages(self):