Module auth_session_timeout: Pluggability (#887)

* Module auth_session_timeout:
---------------------------

* Refactor to allow other modules to inherit and augment or override the following:
** Session expiry time (deadline) calculation
** Ignored URLs
** Final session expiry (with possibility to late-abort)
* Re-ordered functionality to remove unnecessary work, as this code is called very often.
* Do not expire a session if delay gets set to zero (or unset / false)

* WIP

* Fixed flake8 lint errors

* Fixed flake8 lint errors

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* Module: auth-session-timeout: Refactor ResUser tests to use `unittest.mock` patching

* Module: auth_session_timeout: Fixed flake8 lint errors

* Module: auth_session_timeout: Fixed flake8 lint errors

auth_session_timeout/README.rst

@@ -50,6 +50,7 @@ Contributors
 
 * Cédric Pigeon <cedric.pigeon@acsone.eu>
 * Dhinesh D <dvdhinesh.mail@gmail.com>
+* Jesse Morgan <jmorgan.nz@gmail.com>
 * Dave Lasley <dave@laslabs.com>
 
 Maintainer

auth_session_timeout/__manifest__.py

@@ -8,6 +8,7 @@
         This module disable all inactive sessions since a given delay""",
     'author': "ACSONE SA/NV, "
               "Dhinesh D, "
+              "Jesse Morgan, "
               "LasLabs, "
               "Odoo Community Association (OCA)",
     'maintainer': 'Odoo Community Association (OCA)',

auth_session_timeout/models/ir_config_parameter.py

@@ -1,9 +1,9 @@
 # -*- coding: utf-8 -*-
 # (c) 2015 ACSONE SA/NV, Dhinesh D
-# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
-from odoo import models, api, tools
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
+from odoo import models, api, tools, SUPERUSER_ID
 
 DELAY_KEY = 'inactive_session_time_out_delay'
 IGNORED_PATH_KEY = 'inactive_session_time_out_ignored_url'
@@ -12,18 +12,34 @@ IGNORED_PATH_KEY = 'inactive_session_time_out_ignored_url'
 class IrConfigParameter(models.Model):
     _inherit = 'ir.config_parameter'
 
-    @api.model
-    @tools.ormcache('self.env.cr.dbname')
-    def get_session_parameters(self):
-        ConfigParam = self.env['ir.config_parameter']
-        delay = ConfigParam.get_param(DELAY_KEY, 7200)
-        urls = ConfigParam.get_param(IGNORED_PATH_KEY, '').split(',')
-        return int(delay), urls
+    @tools.ormcache('db')
+    def get_session_parameters(self, db):
+        param_model = self.pool['ir.config_parameter']
+        cr = self.pool.cursor()
+        delay = False
+        urls = []
+        try:
+            delay = int(param_model.get_param(
+                cr, SUPERUSER_ID, DELAY_KEY, 7200))
+            urls = param_model.get_param(
+                cr, SUPERUSER_ID, IGNORED_PATH_KEY, '').split(',')
+        finally:
+            cr.close()
+        return delay, urls
+
+    def _auth_timeout_get_parameter_delay(self):
+        delay, urls = self.get_session_parameters(self.pool.db_name)
+        return delay
+
+    def _auth_timeout_get_parameter_ignoredurls(self):
+        delay, urls = self.get_session_parameters(self.pool.db_name)
+        return urls
 
     @api.multi
-    def write(self, vals):
+    def write(self, vals, context=None):
         res = super(IrConfigParameter, self).write(vals)
-        for rec_id in self:
-            if rec_id.key in (DELAY_KEY, IGNORED_PATH_KEY):
-                self.get_session_parameters.clear_cache(self)
+        if self.key == DELAY_KEY:
+            self.get_session_parameters.clear_cache(self)
+        elif self.key == IGNORED_PATH_KEY:
+            self.get_session_parameters.clear_cache(self)
         return res

auth_session_timeout/models/res_users.py

@@ -1,41 +1,110 @@
 # -*- coding: utf-8 -*-
 # (c) 2015 ACSONE SA/NV, Dhinesh D
+
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
+import logging
+
+from odoo import models
+
+from odoo.http import root
+from odoo.http import request
+
 from os import utime
 from os.path import getmtime
 from time import time
 
-from odoo import models, http
+_logger = logging.getLogger(__name__)
 
 
 class ResUsers(models.Model):
     _inherit = 'res.users'
 
-    @classmethod
-    def _check_session_validity(cls, db, uid, passwd):
-        if not http.request:
+    def _auth_timeout_ignoredurls_get(self):
+        """Pluggable method for calculating ignored urls
+        Defaults to stored config param
+        """
+        param_model = self.pool['ir.config_parameter']
+        return param_model._auth_timeout_get_parameter_ignoredurls()
+
+    def _auth_timeout_deadline_calculate(self):
+        """Pluggable method for calculating timeout deadline
+        Defaults to current time minus delay using delay stored as config param
+        """
+        param_model = self.pool['ir.config_parameter']
+        delay = param_model._auth_timeout_get_parameter_delay()
+        if delay is False or delay <= 0:
+            return False
+        return time() - delay
+
+    def _auth_timeout_session_terminate(self, session):
+        """Pluggable method for terminating a timed-out session
+
+        This is a late stage where a session timeout can be aborted.
+        Useful if you want to do some heavy checking, as it won't be
+        called unless the session inactivity deadline has been reached.
+
+        Return:
+            True: session terminated
+            False: session timeout cancelled
+        """
+        if session.db and session.uid:
+            session.logout(keep_db=True)
+        return True
+
+    def _auth_timeout_check(self):
+        if not request:
             return
-        session = http.request.session
-        session_store = http.root.session_store
-        ConfigParam = http.request.env['ir.config_parameter']
-        delay, urls = ConfigParam.get_session_parameters()
-        deadline = time() - delay
-        path = session_store.get_session_filename(session.sid)
-        try:
-            if getmtime(path) < deadline:
-                if session.db and session.uid:
-                    session.logout(keep_db=True)
-            elif http.request.httprequest.path not in urls:
-                # the session is not expired, update the last modification
-                # and access time.
+
+        session = request.session
+
+        # Calculate deadline
+        deadline = self._auth_timeout_deadline_calculate()
+
+        # Check if past deadline
+        expired = False
+        if deadline is not False:
+            path = root.session_store.get_session_filename(session.sid)
+            try:
+                expired = getmtime(path) < deadline
+            except OSError as e:
+                _logger.warning(
+                    'Exception reading session file modified time: %s'
+                    % e
+                )
+                pass
+
+        # Try to terminate the session
+        terminated = False
+        if expired:
+            terminated = self._auth_timeout_session_terminate(session)
+
+        # If session terminated, all done
+        if terminated:
+            return
+
+        # Else, conditionally update session modified and access times
+        ignoredurls = self._auth_timeout_ignoredurls_get()
+
+        if request.httprequest.path not in ignoredurls:
+            if 'path' not in locals():
+                path = root.session_store.get_session_filename(session.sid)
+            try:
                 utime(path, None)
-        except OSError:
-            pass
+            except OSError as e:
+                _logger.warning(
+                    'Exception updating session file access/modified times: %s'
+                    % e
+                )
+                pass
+
         return
 
-    @classmethod
-    def check(cls, db, uid, passwd):
-        res = super(ResUsers, cls).check(db, uid, passwd)
-        cls._check_session_validity(db, uid, passwd)
+    def _check_session_validity(self, db, uid, passwd):
+        """Adaptor method for backward compatibility"""
+        return self._auth_timeout_check()
+
+    def check(self, db, uid, passwd):
+        res = super(ResUsers, self).check(db, uid, passwd)
+        self._check_session_validity(db, uid, passwd)
         return res

auth_session_timeout/tests/test_ir_config_parameter.py

@@ -1,32 +1,71 @@
 # -*- coding: utf-8 -*-
 # (c) 2015 ACSONE SA/NV, Dhinesh D
-# Copyright 2016 LasLabs Inc.
+
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
-from odoo.tests.common import TransactionCase
+from odoo.tests import common
 
 
-class TestIrConfigParameter(TransactionCase):
+class TestIrConfigParameter(common.TransactionCase):
 
     def setUp(self):
         super(TestIrConfigParameter, self).setUp()
+        self.db = self.env.cr.dbname
         self.param_obj = self.env['ir.config_parameter']
         self.data_obj = self.env['ir.model.data']
         self.delay = self.env.ref(
-            'auth_session_timeout.inactive_session_time_out_delay'
-        )
-        self.url = self.env.ref(
-            'auth_session_timeout.inactive_session_time_out_ignored_url'
-        )
-        self.urls = ['url1', 'url2']
-        self.url.value = ','.join(self.urls)
-
-    def test_get_session_parameters_delay(self):
-        """ It should return the proper delay """
-        delay, _ = self.param_obj.get_session_parameters()
+            'auth_session_timeout.inactive_session_time_out_delay')
+
+    def test_check_session_params(self):
+        delay, urls = self.param_obj.get_session_parameters(self.db)
+        self.assertEqual(delay, int(self.delay.value))
+        self.assertIsInstance(delay, int)
+        self.assertIsInstance(urls, list)
+
+    def test_check_session_param_delay(self):
+        delay = self.param_obj._auth_timeout_get_parameter_delay()
         self.assertEqual(delay, int(self.delay.value))
+        self.assertIsInstance(delay, int)
+
+    def test_check_session_param_urls(self):
+        urls = self.param_obj._auth_timeout_get_parameter_ignoredurls()
+        self.assertIsInstance(urls, list)
+
+
+class TestIrConfigParameterCaching(common.TransactionCase):
+
+    def setUp(self):
+        super(TestIrConfigParameterCaching, self).setUp()
+        self.db = self.env.cr.dbname
+        self.param_obj = self.env['ir.config_parameter']
+        self.get_param_called = False
+        test = self
+
+        def get_param(*args, **kwargs):
+            test.get_param_called = True
+            return orig_get_param(args[3], args[4])
+        orig_get_param = self.param_obj.get_param
+        self.param_obj._patch_method(
+            'get_param',
+            get_param)
+
+    def tearDown(self):
+        super(TestIrConfigParameterCaching, self).tearDown()
+        self.param_obj._revert_method('get_param')
+
+    def test_check_param_cache_working(self):
+        self.get_param_called = False
+        delay, urls = self.param_obj.get_session_parameters(self.db)
+        self.assertTrue(self.get_param_called)
+        self.get_param_called = False
+        delay, urls = self.param_obj.get_session_parameters(self.db)
+        self.assertFalse(self.get_param_called)
 
-    def test_get_session_parameters_url(self):
-        """ It should return URIs split by comma """
-        _, urls = self.param_obj.get_session_parameters()
-        self.assertEqual(urls, self.urls)
+    def test_check_param_writes_clear_cache(self):
+        self.get_param_called = False
+        delay, urls = self.param_obj.get_session_parameters(self.db)
+        self.assertTrue(self.get_param_called)
+        self.get_param_called = False
+        self.param_obj.set_param('inactive_session_time_out_delay', 7201)
+        delay, urls = self.param_obj.get_session_parameters(self.db)
+        self.assertTrue(self.get_param_called)

auth_session_timeout/tests/test_res_users.py

@@ -3,11 +3,15 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
 import mock
+from os import strerror
+from errno import ENOENT
 
 from contextlib import contextmanager
 
 from odoo.tests.common import TransactionCase
 
+_package_path = 'odoo.addons.auth_session_timeout'
+
 
 class EndTestException(Exception):
     """ It stops tests from continuing """
@@ -102,3 +106,30 @@ class TestResUsers(TransactionCase):
             assets['getmtime'].side_effect = OSError
             res = self._check_session_validity()
             self.assertFalse(res)
+
+    @mock.patch(_package_path + '.models.res_users.request')
+    @mock.patch(_package_path + '.models.res_users.root')
+    @mock.patch(_package_path + '.models.res_users.getmtime')
+    def test_on_timeout_session_loggedout(self, mock_getmtime,
+                                          mock_root, mock_request):
+        mock_getmtime.return_value = 0
+        mock_request.session.uid = self.env.uid
+        mock_request.session.dbname = self.env.cr.dbname
+        mock_request.session.sid = 123
+        mock_request.session.logout = mock.Mock()
+        self.resUsers._auth_timeout_check()
+        self.assertTrue(mock_request.session.logout.called)
+
+    @mock.patch(_package_path + '.models.res_users.request')
+    @mock.patch(_package_path + '.models.res_users.root')
+    @mock.patch(_package_path + '.models.res_users.getmtime')
+    @mock.patch(_package_path + '.models.res_users.utime')
+    def test_sessionfile_io_exceptions_managed(self, mock_utime, mock_getmtime,
+                                               mock_root, mock_request):
+        mock_getmtime.side_effect = OSError(
+            ENOENT, strerror(ENOENT), 'non-existent-filename')
+        mock_request.session.uid = self.env.uid
+        mock_request.session.dbname = self.env.cr.dbname
+        mock_request.session.sid = 123
+        self.resUsers._auth_timeout_check()
+