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: rouilj

roundup/cgi/client.py

@@ -174,13 +174,13 @@ def __init__(self, client):
             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 """

test/test_liveserver.py

@@ -73,15 +73,16 @@ def setup_class(cls):
         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 @@ def test_cache_control_js(self):
         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()