[IMP] auth_session_timeout: Deprecate backwards compat + improve

* Deprecate backwards compatibility methods that were retained during v9 rework
* Upgrade API and rename a few things for PEP-8
* Switch to HttpCase for tests
* Switch to isolated build

.travis.yml

@@ -24,10 +24,12 @@ env:
   matrix:
   - LINT_CHECK="1"
   - TRANSIFEX="1"
-  - TESTS="1" ODOO_REPO="odoo/odoo" EXCLUDE="database_cleanup"
-  - TESTS="1" ODOO_REPO="OCA/OCB" EXCLUDE="database_cleanup"
+  - TESTS="1" ODOO_REPO="odoo/odoo" EXCLUDE="database_cleanup,auth_session_timeout"
+  - TESTS="1" ODOO_REPO="OCA/OCB" EXCLUDE="database_cleanup,auth_session_timeout"
   - TESTS="1" ODOO_REPO="odoo/odoo" INCLUDE="database_cleanup"
   - TESTS="1" ODOO_REPO="OCA/OCB" INCLUDE="database_cleanup"
+  - TESTS="1" ODOO_REPO="odoo/odoo" INCLUDE="auth_session_timeout"
+  - TESTS="1" ODOO_REPO="OCA/OCB" INCLUDE="auth_session_timeout"
 
 virtualenv:
   system_site_packages: true

auth_session_timeout/models/ir_config_parameter.py

@@ -1,9 +1,8 @@
 # -*- 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, SUPERUSER_ID
+from odoo import api, models, tools
 
 DELAY_KEY = 'inactive_session_time_out_delay'
 IGNORED_PATH_KEY = 'inactive_session_time_out_ignored_url'
@@ -12,34 +11,30 @@ IGNORED_PATH_KEY = 'inactive_session_time_out_ignored_url'
 class IrConfigParameter(models.Model):
     _inherit = 'ir.config_parameter'
 
-    @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
-
+    @api.model
+    @tools.ormcache('self.env.cr.dbname')
     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
+        return int(
+            self.env['ir.config_parameter'].sudo().get_param(
+                DELAY_KEY, 7200,
+            )
+        )
+
+    @api.model
+    @tools.ormcache('self.env.cr.dbname')
+    def _auth_timeout_get_parameter_ignored_urls(self):
+        urls = self.env['ir.config_parameter'].sudo().get_param(
+            IGNORED_PATH_KEY, '',
+        )
+        return urls.split(',')
 
     @api.multi
-    def write(self, vals, context=None):
+    def write(self, vals):
         res = super(IrConfigParameter, self).write(vals)
-        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)
+        self._auth_timeout_get_parameter_delay.clear_cache(
+            self.filtered(lambda r: r.key == DELAY_KEY),
+        )
+        self._auth_timeout_get_parameter_ignored_urls.clear_cache(
+            self.filtered(lambda r: r.key == IGNORED_PATH_KEY),
+        )
         return res

auth_session_timeout/models/res_users.py

@@ -5,14 +5,11 @@
 
 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 os import utime
+
+from odoo import api, http, models
 
 _logger = logging.getLogger(__name__)
 
@@ -20,23 +17,27 @@ _logger = logging.getLogger(__name__)
 class ResUsers(models.Model):
     _inherit = 'res.users'
 
-    def _auth_timeout_ignoredurls_get(self):
+    @api.model_cr_context
+    def _auth_timeout_get_ignored_urls(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()
+        params = self.env['ir.config_parameter']
+        return params._auth_timeout_get_parameter_ignored_urls()
 
+    @api.model_cr_context
     def _auth_timeout_deadline_calculate(self):
         """Pluggable method for calculating timeout deadline
-        Defaults to current time minus delay using delay stored as config param
+        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:
+        params = self.env['ir.config_parameter']
+        delay = params._auth_timeout_get_parameter_delay()
+        if delay <= 0:
             return False
         return time() - delay
 
+    @api.model_cr_context
     def _auth_timeout_session_terminate(self, session):
         """Pluggable method for terminating a timed-out session
 
@@ -52,11 +53,14 @@ class ResUsers(models.Model):
             session.logout(keep_db=True)
         return True
 
+    @api.model_cr_context
     def _auth_timeout_check(self):
-        if not request:
+        """Perform session timeout validation and expire if needed."""
+
+        if not http.request:
             return
 
-        session = request.session
+        session = http.request.session
 
         # Calculate deadline
         deadline = self._auth_timeout_deadline_calculate()
@@ -64,15 +68,15 @@ class ResUsers(models.Model):
         # Check if past deadline
         expired = False
         if deadline is not False:
-            path = root.session_store.get_session_filename(session.sid)
+            path = http.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
+                _logger.exception(
+                    'Exception reading session file modified time.',
                 )
-                pass
+                # Force expire the session. Will be resolved with new session.
+                expired = True
 
         # Try to terminate the session
         terminated = False
@@ -84,27 +88,22 @@ class ResUsers(models.Model):
             return
 
         # Else, conditionally update session modified and access times
-        ignoredurls = self._auth_timeout_ignoredurls_get()
+        ignored_urls = self._auth_timeout_get_ignored_urls()
 
-        if request.httprequest.path not in ignoredurls:
+        if http.request.http.request.path not in ignored_urls:
             if 'path' not in locals():
-                path = root.session_store.get_session_filename(session.sid)
+                path = http.root.session_store.get_session_filename(
+                    session.sid,
+                )
             try:
                 utime(path, None)
             except OSError as e:
-                _logger.warning(
-                    'Exception updating session file access/modified times: %s'
-                    % e
+                _logger.exception(
+                    'Exception updating session file access/modified times.',
                 )
-                pass
-
-        return
-
-    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)
+    @classmethod
+    def check(cls, *args, **kwargs):
+        res = super(ResUsers, cls).check(*args, **kwargs)
+        http.request.env.user._auth_timeout_check()
         return res

auth_session_timeout/tests/test_ir_config_parameter.py

@@ -16,19 +16,13 @@ class TestIrConfigParameter(common.TransactionCase):
         self.delay = self.env.ref(
             '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()
+        urls = self.param_obj._auth_timeout_get_parameter_ignored_urls()
         self.assertIsInstance(urls, list)
 
 
@@ -43,7 +37,8 @@ class TestIrConfigParameterCaching(common.TransactionCase):
 
         def get_param(*args, **kwargs):
             test.get_param_called = True
-            return orig_get_param(args[3], args[4])
+            return orig_get_param(*args[1:], **kwargs)
+
         orig_get_param = self.param_obj.get_param
         self.param_obj._patch_method(
             'get_param',
@@ -53,19 +48,29 @@ class TestIrConfigParameterCaching(common.TransactionCase):
         super(TestIrConfigParameterCaching, self).tearDown()
         self.param_obj._revert_method('get_param')
 
-    def test_check_param_cache_working(self):
+    def test_auth_timeout_get_parameter_delay_cache(self):
+        """It should cache the parameter call."""
         self.get_param_called = False
-        delay, urls = self.param_obj.get_session_parameters(self.db)
+        self.param_obj._auth_timeout_get_parameter_delay()
         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_check_param_writes_clear_cache(self):
+    def test_auth_timeout_get_parameter_ignored_urls_cache(self):
+        """It should cache the parameter call."""
         self.get_param_called = False
-        delay, urls = self.param_obj.get_session_parameters(self.db)
+        self.param_obj._auth_timeout_get_parameter_ignored_urls()
         self.assertTrue(self.get_param_called)
+
+    def test_check_param_writes_clear_delay_cache(self):
+        self.param_obj._auth_timeout_get_parameter_delay()
         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.param_obj._auth_timeout_get_parameter_delay()
+        self.assertTrue(self.get_param_called)
+
+    def test_check_param_writes_clear_ignore_url_cache(self):
+        self.param_obj._auth_timeout_get_parameter_ignored_urls()
+        self.get_param_called = False
+        self.param_obj.set_param('inactive_session_time_out_ignored_url',
+                                 'example.com')
+        self.param_obj._auth_timeout_get_parameter_ignored_urls()
         self.assertTrue(self.get_param_called)

auth_session_timeout/tests/test_res_users.py

@@ -1,17 +1,14 @@
 # -*- coding: utf-8 -*-
-# Copyright 2016 LasLabs Inc.
+# Copyright 2016-2017 LasLabs Inc.
 # 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.tools.misc import mute_logger
 from odoo.tests.common import TransactionCase
 
-_package_path = 'odoo.addons.auth_session_timeout'
-
 
 class EndTestException(Exception):
     """ It stops tests from continuing """
@@ -38,31 +35,23 @@ class TestResUsers(TransactionCase):
         ) as mocks:
             yield mocks
 
-    def _check_session_validity(self):
-        """ It wraps ``_check_session_validity`` for easier calling """
+    def _auth_timeout_check(self, http_mock):
+        """ It wraps ``_auth_timeout_check`` for easier calling """
         self.db = mock.MagicMock()
         self.uid = mock.MagicMock()
         self.passwd = mock.MagicMock()
-        return self.ResUsers._check_session_validity(
-            self.db, self.uid, self.passwd,
-        )
+        self.path = '/this/is/a/test/path'
+        get_filename = http_mock.root.session_store.get_session_filename
+        get_filename.return_value = self.path
+        return self.ResUsers._auth_timeout_check()
 
     def test_session_validity_no_request(self):
         """ It should return immediately if no request """
         with self._mock_assets() as assets:
             assets['http'].request = False
-            res = self._check_session_validity()
+            res = self._auth_timeout_check(assets['http'])
             self.assertFalse(res)
 
-    def test_session_validity_gets_params(self):
-        """ It should call ``get_session_parameters`` with db """
-        with self._mock_assets() as assets:
-            get_params = assets['http'].request.env[''].get_session_parameters
-            get_params.side_effect = EndTestException
-            with self.assertRaises(EndTestException):
-                self._check_session_validity()
-            get_params.assert_called_once_with()
-
     def test_session_validity_gets_session_file(self):
         """ It should call get the session file for the session id """
         with self._mock_assets() as assets:
@@ -71,7 +60,7 @@ class TestResUsers(TransactionCase):
             store = assets['http'].root.session_store
             store.get_session_filename.side_effect = EndTestException
             with self.assertRaises(EndTestException):
-                self._check_session_validity()
+                self._auth_timeout_check(assets['http'])
             store.get_session_filename.assert_called_once_with(
                 assets['http'].request.session.sid,
             )
@@ -82,7 +71,7 @@ class TestResUsers(TransactionCase):
             get_params = assets['http'].request.env[''].get_session_parameters
             get_params.return_value = -9999, []
             assets['getmtime'].return_value = 0
-            self._check_session_validity()
+            self._auth_timeout_check(assets['http'])
             assets['http'].request.session.logout.assert_called_once_with(
                 keep_db=True,
             )
@@ -92,44 +81,28 @@ class TestResUsers(TransactionCase):
         with self._mock_assets(['http', 'getmtime', 'utime']) as assets:
             get_params = assets['http'].request.env[''].get_session_parameters
             get_params.return_value = 9999, []
-            self._check_session_validity()
+            self._auth_timeout_check(assets['http'])
             assets['utime'].assert_called_once_with(
                 assets['http'].root.session_store.get_session_filename(),
                 None,
             )
 
+    @mute_logger('odoo.addons.auth_session_timeout.models.res_users')
     def test_session_validity_os_error_guard(self):
         """ It should properly guard from OSError & return """
         with self._mock_assets(['http', 'utime', 'getmtime']) as assets:
             get_params = assets['http'].request.env[''].get_session_parameters
             get_params.return_value = 0, []
             assets['getmtime'].side_effect = OSError
-            res = self._check_session_validity()
+            res = self._auth_timeout_check(assets['http'])
             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()
-
+    def test_on_timeout_session_loggedout(self):
+        with self._mock_assets(['http', 'getmtime']) as assets:
+            assets['getmtime'].return_value = 0
+            assets['http'].request.session.uid = self.env.uid
+            assets['http'].request.session.dbname = self.env.cr.dbname
+            assets['http'].request.session.sid = 123
+            assets['http'].request.session.logout = mock.Mock()
+            self.ResUsers._auth_timeout_check()
+            self.assertTrue(assets['http'].request.session.logout.called)

base_external_system/tests/common.py

@@ -5,10 +5,10 @@
 from contextlib import contextmanager
 from mock import MagicMock
 
-from odoo.tests.common import TransactionCase
+from odoo.tests.common import HttpCase
 
 
-class Common(TransactionCase):
+class Common(HttpCase):
 
     @contextmanager
     def _mock_method(self, method_name, method_obj=None):