From ac4301bb0182ca3ac564b0d831dabf53d54504ae Mon Sep 17 00:00:00 2001 From: Dave Lasley Date: Thu, 20 Oct 2016 18:37:10 -0700 Subject: [PATCH 1/3] [MIG] auth_session_timeout: Migrate to v10 * Bump versions * Installable to True * Add Usage section to ReadMe w/ Runbot link * `_crypt_context` now directly exposes the `CryptContext` * Change all instances of openerp to odoo * Add test coverage to IrConfigParameter * Add test coverage for res.users * Remove db from `get_session_parameters` method call * Remove deprecated skiparg for ormcache * Fix tests & lint * Switch cache to use self.cr.dbname * Fix ormcache --- auth_session_timeout/README.rst | 43 ++++++-- auth_session_timeout/__init__.py | 1 - auth_session_timeout/__manifest__.py | 18 +-- .../data/ir_config_parameter_data.xml | 23 ++-- auth_session_timeout/models/__init__.py | 1 - .../models/ir_config_parameter.py | 31 ++---- auth_session_timeout/models/res_users.py | 29 +++-- auth_session_timeout/tests/__init__.py | 4 +- .../tests/test_ir_config_parameter.py | 32 +++--- auth_session_timeout/tests/test_res_users.py | 104 ++++++++++++++++++ 10 files changed, 195 insertions(+), 91 deletions(-) create mode 100644 auth_session_timeout/tests/test_res_users.py diff --git a/auth_session_timeout/README.rst b/auth_session_timeout/README.rst index 63325502c..8b0f2c282 100644 --- a/auth_session_timeout/README.rst +++ b/auth_session_timeout/README.rst @@ -1,6 +1,8 @@ .. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg - :alt: License: AGPL-3 + :target: http://www.gnu.org/licenses/agpl-3.0-standalone.html + :alt: License: AGPL-3 +========================= Inactive Sessions Timeout ========================= @@ -13,16 +15,32 @@ Configuration Two system parameters are available: -* inactive_session_time_out_delay: validity of a session in seconds (default = 2 Hours) -* inactive_session_time_out_ignored_url: technical urls where the check does not occur +* ``inactive_session_time_out_delay``: validity of a session in seconds + (default = 2 Hours) +* ``inactive_session_time_out_ignored_url``: technical urls where the check + does not occur + +Usage +===== + +Setup the session parameters as described above. + +.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas + :alt: Try me on Runbot + :target: https://runbot.odoo-community.org/runbot/149/9.0 + +Known issues / Roadmap +====================== + Bug Tracker =========== -Bugs are tracked on `GitHub Issues `_. -In case of trouble, please check there if your issue has already been reported. -If you spotted it first, help us smashing it by providing a detailed and welcomed feedback -`here `_. +Bugs are tracked on `GitHub Issues +`_. In case of trouble, please +check there if your issue has already been reported. If you spotted it first, +help us smashing it by providing a detailed and welcomed feedback. + Credits ======= @@ -32,16 +50,19 @@ Contributors * Cédric Pigeon * Dhinesh D +* Dave Lasley Maintainer ---------- -.. image:: http://odoo-community.org/logo.png +.. image:: https://odoo-community.org/logo.png :alt: Odoo Community Association - :target: http://odoo-community.org + :target: https://odoo-community.org This module is maintained by the OCA. -OCA, or the Odoo Community Association, is a nonprofit organization whose mission is to support the collaborative development of Odoo features and promote its widespread use. +OCA, or the Odoo Community Association, is a nonprofit organization whose +mission is to support the collaborative development of Odoo features and +promote its widespread use. -To contribute to this module, please visit http://odoo-community.org. +To contribute to this module, please visit https://odoo-community.org. diff --git a/auth_session_timeout/__init__.py b/auth_session_timeout/__init__.py index 9b1fb35b0..852bfeac2 100644 --- a/auth_session_timeout/__init__.py +++ b/auth_session_timeout/__init__.py @@ -1,6 +1,5 @@ # -*- coding: utf-8 -*- # (c) 2015 ACSONE SA/NV, Dhinesh D - # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). from . import models diff --git a/auth_session_timeout/__manifest__.py b/auth_session_timeout/__manifest__.py index 5b1452402..837ae425b 100644 --- a/auth_session_timeout/__manifest__.py +++ b/auth_session_timeout/__manifest__.py @@ -1,28 +1,22 @@ # -*- coding: utf-8 -*- # (c) 2015 ACSONE SA/NV, Dhinesh D - # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). { 'name': "Inactive Sessions Timeout", - 'summary': """ This module disable all inactive sessions since a given delay""", - - 'author': "ACSONE SA/NV, Dhinesh D, Odoo Community Association (OCA)", + 'author': "ACSONE SA/NV, " + "Dhinesh D, " + "LasLabs, " + "Odoo Community Association (OCA)", 'maintainer': 'Odoo Community Association (OCA)', 'website': "http://acsone.eu", - 'category': 'Tools', - 'version': '9.0.1.0.0', + 'version': '10.0.1.0.0', 'license': 'AGPL-3', - - 'depends': [ - 'base', - ], - 'data': [ 'data/ir_config_parameter_data.xml' ], - 'installable': False, + 'installable': True, } diff --git a/auth_session_timeout/data/ir_config_parameter_data.xml b/auth_session_timeout/data/ir_config_parameter_data.xml index 96b0194bf..fb0b1709e 100644 --- a/auth_session_timeout/data/ir_config_parameter_data.xml +++ b/auth_session_timeout/data/ir_config_parameter_data.xml @@ -4,18 +4,13 @@ License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). --> - - - - inactive_session_time_out_delay - 7200 - - - - - - inactive_session_time_out_ignored_url - /calendar/notify,/longpolling/poll - - + + + inactive_session_time_out_delay + 7200 + + + inactive_session_time_out_ignored_url + /calendar/notify,/longpolling/poll + diff --git a/auth_session_timeout/models/__init__.py b/auth_session_timeout/models/__init__.py index 0c6063031..4c14e36fd 100644 --- a/auth_session_timeout/models/__init__.py +++ b/auth_session_timeout/models/__init__.py @@ -1,6 +1,5 @@ # -*- coding: utf-8 -*- # (c) 2015 ACSONE SA/NV, Dhinesh D - # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). from . import res_users diff --git a/auth_session_timeout/models/ir_config_parameter.py b/auth_session_timeout/models/ir_config_parameter.py index 69a7003f0..8e2e87e36 100644 --- a/auth_session_timeout/models/ir_config_parameter.py +++ b/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 openerp import models, api, tools, SUPERUSER_ID +from odoo import models, api, tools DELAY_KEY = 'inactive_session_time_out_delay' @@ -13,24 +12,18 @@ IGNORED_PATH_KEY = 'inactive_session_time_out_ignored_url' class IrConfigParameter(models.Model): _inherit = 'ir.config_parameter' - @tools.ormcache(skiparg=0) - 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 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 @api.multi - def write(self, vals, context=None): + def write(self, vals): res = super(IrConfigParameter, self).write(vals) - if self.key in [DELAY_KEY, IGNORED_PATH_KEY]: - self.get_session_parameters.clear_cache(self) + for rec_id in self: + if rec_id.key in (DELAY_KEY, IGNORED_PATH_KEY): + self.get_session_parameters.clear_cache(self) return res diff --git a/auth_session_timeout/models/res_users.py b/auth_session_timeout/models/res_users.py index 95a137df9..db9b5dc6e 100644 --- a/auth_session_timeout/models/res_users.py +++ b/auth_session_timeout/models/res_users.py @@ -1,29 +1,25 @@ # -*- coding: utf-8 -*- # (c) 2015 ACSONE SA/NV, Dhinesh D - # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -from openerp import models -from openerp import http - -from openerp.http import root -from openerp.http import request - from os import utime from os.path import getmtime from time import time +from odoo import models, http + class ResUsers(models.Model): _inherit = 'res.users' - def _check_session_validity(self, db, uid, passwd): - if not request: + @classmethod + def _check_session_validity(cls, db, uid, passwd): + if not http.request: return - session = request.session - session_store = root.session_store - param_obj = self.pool['ir.config_parameter'] - delay, urls = param_obj.get_session_parameters(db) + 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: @@ -38,7 +34,8 @@ class ResUsers(models.Model): pass return - 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, db, uid, passwd): + res = super(ResUsers, cls).check(db, uid, passwd) + cls._check_session_validity(db, uid, passwd) return res diff --git a/auth_session_timeout/tests/__init__.py b/auth_session_timeout/tests/__init__.py index 7ae3d0692..a4a711662 100644 --- a/auth_session_timeout/tests/__init__.py +++ b/auth_session_timeout/tests/__init__.py @@ -1,6 +1,4 @@ # -*- coding: utf-8 -*- -# (c) 2015 ACSONE SA/NV, Dhinesh D - -# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). from . import test_ir_config_parameter +from . import test_res_users diff --git a/auth_session_timeout/tests/test_ir_config_parameter.py b/auth_session_timeout/tests/test_ir_config_parameter.py index 6a7249f8e..b5cc8d2de 100644 --- a/auth_session_timeout/tests/test_ir_config_parameter.py +++ b/auth_session_timeout/tests/test_ir_config_parameter.py @@ -1,28 +1,32 @@ # -*- 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). -import threading - -from openerp.tests import common -import openerp +from odoo.tests.common import TransactionCase -class TestIrConfigParameter(common.TransactionCase): +class TestIrConfigParameter(TransactionCase): def setUp(self): super(TestIrConfigParameter, self).setUp() - self.db = openerp.tools.config['db_name'] - if not self.db and hasattr(threading.current_thread(), 'dbname'): - self.db = threading.current_thread().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') + '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_check_delay(self): - delay, urls = self.param_obj.get_session_parameters(self.db) + def test_get_session_parameters_delay(self): + """ It should return the proper delay """ + delay, _ = self.param_obj.get_session_parameters() self.assertEqual(delay, int(self.delay.value)) - self.assertIsInstance(delay, int) - self.assertIsInstance(urls, list) + + 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) diff --git a/auth_session_timeout/tests/test_res_users.py b/auth_session_timeout/tests/test_res_users.py new file mode 100644 index 000000000..eab8c802d --- /dev/null +++ b/auth_session_timeout/tests/test_res_users.py @@ -0,0 +1,104 @@ +# -*- coding: utf-8 -*- +# Copyright 2016 LasLabs Inc. +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). + +import mock + +from contextlib import contextmanager + +from odoo.tests.common import TransactionCase + + +class EndTestException(Exception): + """ It stops tests from continuing """ + pass + + +class TestResUsers(TransactionCase): + + def setUp(self): + super(TestResUsers, self).setUp() + self.ResUsers = self.env['res.users'] + + @contextmanager + def _mock_assets(self, assets=None): + """ It provides mocked imports from res_users.py + :param assets: (list) Name of imports to mock. Mocks `http` if None + :return: (dict) Dictionary of mocks, keyed by module name + """ + if assets is None: + assets = ['http'] + patches = {name: mock.DEFAULT for name in assets} + with mock.patch.multiple( + 'odoo.addons.auth_session_timeout.models.res_users', **patches + ) as mocks: + yield mocks + + def _check_session_validity(self): + """ It wraps ``_check_session_validity`` 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, + ) + + 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() + 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: + get_params = assets['http'].request.env[''].get_session_parameters + get_params.return_value = 0, [] + store = assets['http'].root.session_store + store.get_session_filename.side_effect = EndTestException + with self.assertRaises(EndTestException): + self._check_session_validity() + store.get_session_filename.assert_called_once_with( + assets['http'].request.session.sid, + ) + + def test_session_validity_logout(self): + """ It should log out of session if past deadline """ + with self._mock_assets(['http', 'getmtime', 'utime']) as assets: + get_params = assets['http'].request.env[''].get_session_parameters + get_params.return_value = -9999, [] + assets['getmtime'].return_value = 0 + self._check_session_validity() + assets['http'].request.session.logout.assert_called_once_with( + keep_db=True, + ) + + def test_session_validity_updates_utime(self): + """ It should update utime of session file if not expired """ + 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() + assets['utime'].assert_called_once_with( + assets['http'].root.session_store.get_session_filename(), + None, + ) + + 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() + self.assertFalse(res) From 2fda45fdfd6bf6a037aef6bbe0e04db85aacb123 Mon Sep 17 00:00:00 2001 From: jmorgannz Date: Thu, 7 Sep 2017 03:53:23 +1200 Subject: [PATCH 2/3] 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 | 1 + auth_session_timeout/__manifest__.py | 1 + .../models/ir_config_parameter.py | 42 +++++-- auth_session_timeout/models/res_users.py | 115 ++++++++++++++---- .../tests/test_ir_config_parameter.py | 75 +++++++++--- auth_session_timeout/tests/test_res_users.py | 31 +++++ 6 files changed, 211 insertions(+), 54 deletions(-) diff --git a/auth_session_timeout/README.rst b/auth_session_timeout/README.rst index 8b0f2c282..19397eaac 100644 --- a/auth_session_timeout/README.rst +++ b/auth_session_timeout/README.rst @@ -50,6 +50,7 @@ Contributors * Cédric Pigeon * Dhinesh D +* Jesse Morgan * Dave Lasley Maintainer diff --git a/auth_session_timeout/__manifest__.py b/auth_session_timeout/__manifest__.py index 837ae425b..031ed28b9 100644 --- a/auth_session_timeout/__manifest__.py +++ b/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)', diff --git a/auth_session_timeout/models/ir_config_parameter.py b/auth_session_timeout/models/ir_config_parameter.py index 8e2e87e36..bcc8022a3 100644 --- a/auth_session_timeout/models/ir_config_parameter.py +++ b/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 diff --git a/auth_session_timeout/models/res_users.py b/auth_session_timeout/models/res_users.py index db9b5dc6e..c916a6d1f 100644 --- a/auth_session_timeout/models/res_users.py +++ b/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 diff --git a/auth_session_timeout/tests/test_ir_config_parameter.py b/auth_session_timeout/tests/test_ir_config_parameter.py index b5cc8d2de..2b15d2b0d 100644 --- a/auth_session_timeout/tests/test_ir_config_parameter.py +++ b/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) diff --git a/auth_session_timeout/tests/test_res_users.py b/auth_session_timeout/tests/test_res_users.py index eab8c802d..f824dcdce 100644 --- a/auth_session_timeout/tests/test_res_users.py +++ b/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() + From 00e79300e30a01a7bd9c1a9bc34f825a1821fe5f Mon Sep 17 00:00:00 2001 From: Dave Lasley Date: Mon, 6 Nov 2017 08:39:31 -0800 Subject: [PATCH 3/3] [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 | 6 +- .../models/ir_config_parameter.py | 51 ++++++------- auth_session_timeout/models/res_users.py | 71 +++++++++--------- .../tests/test_ir_config_parameter.py | 37 ++++++---- auth_session_timeout/tests/test_res_users.py | 73 ++++++------------- base_external_system/tests/common.py | 4 +- 6 files changed, 108 insertions(+), 134 deletions(-) diff --git a/.travis.yml b/.travis.yml index ce704dbad..19a5a9f9e 100644 --- a/.travis.yml +++ b/.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 diff --git a/auth_session_timeout/models/ir_config_parameter.py b/auth_session_timeout/models/ir_config_parameter.py index bcc8022a3..ea6854942 100644 --- a/auth_session_timeout/models/ir_config_parameter.py +++ b/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 diff --git a/auth_session_timeout/models/res_users.py b/auth_session_timeout/models/res_users.py index c916a6d1f..00b0833cc 100644 --- a/auth_session_timeout/models/res_users.py +++ b/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 diff --git a/auth_session_timeout/tests/test_ir_config_parameter.py b/auth_session_timeout/tests/test_ir_config_parameter.py index 2b15d2b0d..e10e938d8 100644 --- a/auth_session_timeout/tests/test_ir_config_parameter.py +++ b/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) diff --git a/auth_session_timeout/tests/test_res_users.py b/auth_session_timeout/tests/test_res_users.py index f824dcdce..ad601202e 100644 --- a/auth_session_timeout/tests/test_res_users.py +++ b/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) diff --git a/base_external_system/tests/common.py b/base_external_system/tests/common.py index 0c212359e..17cfebc6d 100644 --- a/base_external_system/tests/common.py +++ b/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):