changeset 6813:6b636fb29740

Refactor client.py session cookie code. Remove session db access. The original code did a session_db.exists test followed by a session_db.getall. Refactor does a getall and if a KeyError is thrown, handles the error. Most likely the session key will be found so exception handling won't be triggered. Added test case to test the exception code path and minor rearrangement of setup code.
author John Rouillard <rouilj@ieee.org>
date Wed, 03 Aug 2022 17:34:58 -0400
parents d7905a78ab8a
children 3f60a71b0812
files roundup/cgi/client.py test/test_liveserver.py
diffstat 2 files changed, 37 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- a/roundup/cgi/client.py	Tue Aug 02 23:32:26 2022 -0400
+++ b/roundup/cgi/client.py	Wed Aug 03 17:34:58 2022 -0400
@@ -174,13 +174,13 @@
             re.sub('[^a-zA-Z]', '', client.instance.config.TRACKER_NAME)
         cookies = LiberalCookie(client.env.get('HTTP_COOKIE', ''))
         if self.cookie_name in cookies:
-            if not self.session_db.exists(cookies[self.cookie_name].value):
+            try:
+                self._sid = cookies[self.cookie_name].value
+                self._data = self.session_db.getall(self._sid)
+            except KeyError:
                 self._sid = None
                 # remove old cookie
                 self.client.add_cookie(self.cookie_name, None)
-            else:
-                self._sid = cookies[self.cookie_name].value
-                self._data = self.session_db.getall(self._sid)
 
     def _gen_sid(self):
         """ generate a unique session key """
--- a/test/test_liveserver.py	Tue Aug 02 23:32:26 2022 -0400
+++ b/test/test_liveserver.py	Wed Aug 03 17:34:58 2022 -0400
@@ -73,15 +73,16 @@
         cls.db.config.MAILHOST = "localhost"
         cls.db.config.MAIL_HOST = "localhost"
         cls.db.config.MAIL_DEBUG = "../_test_tracker_mail.log"
+
+        # added to enable csrf forgeries/CORS to be tested
         cls.db.config.WEB_CSRF_ENFORCE_HEADER_ORIGIN = "required"
         cls.db.config.WEB_ALLOWED_API_ORIGINS = "https://client.com"
+        cls.db.config['WEB_CSRF_ENFORCE_HEADER_X-REQUESTED-WITH'] = "required"
 
         # disable web login rate limiting. The fast rate of tests
         # causes them to trip the rate limit and fail.
         cls.db.config.WEB_LOGIN_ATTEMPTS_MIN = 0
         
-        cls.db.config['WEB_CSRF_ENFORCE_HEADER_X-REQUESTED-WITH'] = "required"
-
         # enable static precompressed files
         cls.db.config.WEB_USE_PRECOMPRESSED_FILES = 1
 
@@ -903,6 +904,36 @@
         self.assertEqual(f.status_code, 200)
         self.assertEqual(f.headers['Cache-Control'], 'public, max-age=1209600')
 
+    def test_missing_session_key(self):
+        '''Test case where we have an outdated session cookie. Make
+           sure cookie is removed.
+        '''
+        session = requests.Session()
+        session.headers.update({'Origin': 'http://localhost:9001'})
+
+        # login using form to get cookie
+        login = {"__login_name": 'admin', '__login_password': 'sekrit',
+                 "@action": "login"}
+        f = session.post(self.url_base()+'/', data=login)
+        
+        # verify cookie is present and we are logged in
+        self.assertIn('<b>Hello, admin</b>', f.text)
+        self.assertIn('roundup_session_Roundupissuetracker',
+                         session.cookies)
+
+        f = session.get(self.url_base()+'/')
+        self.assertIn('<b>Hello, admin</b>', f.text)
+
+        for cookie in session.cookies:
+            if cookie.name == 'roundup_session_Roundupissuetracker':
+                cookie.value = 'bad_cookie_no_chocolate'
+                break
+
+        f = session.get(self.url_base()+'/')
+
+        self.assertNotIn('<b>Hello, admin</b>', f.text)
+        self.assertNotIn('roundup_session_Roundupissuetracker', session.cookies)
+
     def test_login_fail_then_succeed(self):
         # Set up session to manage cookies <insert blue monster here>
         session = requests.Session()

Roundup Issue Tracker: http://roundup-tracker.org/