Merge pull request #1495 from daramousk/10.0-fix-auth_session_timeout

[FIX] Use _compute_session_token to invalidate user sessions on auth_session_timeout

auth_session_timeout/models/res_users.py

@@ -102,9 +102,8 @@ class ResUsers(models.Model):
                     'Exception updating session file access/modified times.',
                 )
 
-    @classmethod
-    def check(cls, *args, **kwargs):
-        res = super(ResUsers, cls).check(*args, **kwargs)
+    def _compute_session_token(self, sid):
+        res = super(ResUsers, self)._compute_session_token(sid)
         if http.request:
             http.request.env.user._auth_timeout_check()
         return res

auth_session_timeout/tests/test_res_users.py

@@ -17,15 +17,23 @@ class EndTestException(Exception):
 
 class TestResUsers(TransactionCase):
 
+    post_install = True
+    at_install = False
+
     def setUp(self):
         super(TestResUsers, self).setUp()
-        self.ResUsers = self.env['res.users']
+        self.TestUser = self.env['res.users'].create({
+            'login': 'test_user',
+            'name': 'test_user',
+        })
 
     @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
+        """ Multi patch names in `odoo.addons.auth_session_timeout.models.
+        res_users` for mocking them.
+
+        :param assets: The symbols in res_users that will be patched with
+        MagicMock objects.
         """
         if assets is None:
             assets = ['http']
@@ -43,20 +51,25 @@ class TestResUsers(TransactionCase):
         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()
+        return self.TestUser._auth_timeout_check()
 
     def test_session_validity_no_request(self):
-        """ It should return immediately if no request """
+        """ Tests what happens when the user being tested has not made any
+        requests.
+        """
         with self._mock_assets() as assets:
             assets['http'].request = False
             res = self._auth_timeout_check(assets['http'])
             self.assertFalse(res)
 
     def test_session_validity_gets_session_file(self):
-        """ It should call get the session file for the session id """
+        """ All the sessions a user generates are saved as a file in the
+        filesystem by Werkzeug.
+
+        This function makes sure that our `_auth_timeout_check` makes an
+        attempt in fetching that file by the correct 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):
@@ -66,10 +79,10 @@ class TestResUsers(TransactionCase):
             )
 
     def test_session_validity_logout(self):
-        """ It should log out of session if past deadline """
+        """ Forcefully expire an already existing session and see if the user
+        is actually logged out.
+        """
         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._auth_timeout_check(assets['http'])
             assets['http'].request.session.logout.assert_called_once_with(
@@ -77,10 +90,14 @@ class TestResUsers(TransactionCase):
             )
 
     def test_session_validity_updates_utime(self):
-        """ It should update utime of session file if not expired """
+        """ When a user makes a request, `_auth_timeout_check` is keeping the
+        user's time of request by setting the access time of the session file
+        using utime.
+
+        This function asserts that the access time of the session file is set
+        correctly.
+        """
         with self._mock_assets(['http', 'getmtime', 'utime']) as assets:
-            get_params = assets['http'].request.env[''].get_session_parameters
-            get_params.return_value = 9999, []
             self._auth_timeout_check(assets['http'])
             assets['utime'].assert_called_once_with(
                 assets['http'].root.session_store.get_session_filename(),
@@ -89,20 +106,24 @@ class TestResUsers(TransactionCase):
 
     @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 """
+        """ Make sure that when we get an OSError while trying to set up an
+        access time the session is terminated immediately.
+        """
         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._auth_timeout_check(assets['http'])
             self.assertFalse(res)
 
     def test_on_timeout_session_loggedout(self):
+        """ Make sure that when the timeout has come, the user is actually
+        logged out.
+        """
         with self._mock_assets(['http', 'getmtime']) as assets:
             assets['getmtime'].return_value = 0
-            assets['http'].request.session.uid = self.env.uid
+            assets['http'].request.env.user = self.TestUser
+            assets['http'].request.session.uid = self.TestUser.id
             assets['http'].request.session.dbname = self.env.cr.dbname
-            assets['http'].request.session.sid = 123
+            assets['http'].request.session.sid = '123'
             assets['http'].request.session.logout = mock.Mock()
-            self.ResUsers._auth_timeout_check()
+            self.TestUser._compute_session_token('123')
             self.assertTrue(assets['http'].request.session.logout.called)