[web session] fix web session id bug on automatic reconnection. The web session id should keep the first connection id, then differ of the repo connection id once some reconnection has been done (since the session cookie isn't updated in such cases). Also, use a lock to avoid potential race condition on reconnection.
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Wed, 14 Apr 2010 17:38:24 +0200
changeset 5251 b675edd05c19
parent 5250 1c0eb5f74fd4
child 5270 6297d5265572
[web session] fix web session id bug on automatic reconnection. The web session id should keep the first connection id, then differ of the repo connection id once some reconnection has been done (since the session cookie isn't updated in such cases). Also, use a lock to avoid potential race condition on reconnection.
dbapi.py
web/test/unittest_session.py
web/views/authentication.py
--- a/dbapi.py	Wed Apr 14 17:31:41 2010 +0200
+++ b/dbapi.py	Wed Apr 14 17:38:24 2010 +0200
@@ -168,15 +168,18 @@
         self.data = {}
         self.login = login
         self.authinfo = authinfo
+        # dbapi session identifier is the same as the first connection
+        # identifier, but may later differ in case of auto-reconnection as done
+        # by the web authentication manager (in cw.web.views.authentication)
+        if cnx is not None:
+            self.sessionid = cnx.sessionid
+        else:
+            self.sessionid = None
 
     @property
     def anonymous_session(self):
         return not self.cnx or self.cnx.anonymous_connection
 
-    @property
-    def sessionid(self):
-        return self.cnx.sessionid
-
 
 class DBAPIRequest(RequestSessionBase):
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/web/test/unittest_session.py	Wed Apr 14 17:38:24 2010 +0200
@@ -0,0 +1,33 @@
+# -*- coding: iso-8859-1 -*-
+"""unit tests for cubicweb.web.application
+
+:organization: Logilab
+:copyright: 2001-2010 LOGILAB S.A. (Paris, FRANCE), license is LGPL v2.
+:contact: http://www.logilab.fr/ -- mailto:contact@logilab.fr
+:license: GNU Lesser General Public License, v2.1 - http://www.gnu.org/licenses
+"""
+from cubicweb.devtools.testlib import CubicWebTC
+
+class SessionTC(CubicWebTC):
+
+    def test_auto_reconnection(self):
+        sm = self.app.session_handler.session_manager
+        # make is if the web session has been opened by the session manager
+        sm._sessions[self.cnx.sessionid] = self.websession
+        sessionid = self.websession.sessionid
+        self.assertEquals(len(sm._sessions), 1)
+        self.assertEquals(self.websession.sessionid, self.websession.cnx.sessionid)
+        # fake the repo session is expiring
+        self.repo.close(sessionid)
+        # fake an incoming http query with sessionid in session cookie
+        # don't use self.request() which try to call req.set_session
+        req = self.requestcls(self.vreg)
+        websession = sm.get_session(req, sessionid)
+        self.assertEquals(len(sm._sessions), 1)
+        self.assertIs(websession, self.websession)
+        self.assertEquals(websession.sessionid, sessionid)
+        self.assertNotEquals(websession.sessionid, websession.cnx.sessionid)
+
+if __name__ == '__main__':
+    from logilab.common.testlib import unittest_main
+    unittest_main()
--- a/web/views/authentication.py	Wed Apr 14 17:31:41 2010 +0200
+++ b/web/views/authentication.py	Wed Apr 14 17:38:24 2010 +0200
@@ -5,8 +5,12 @@
 :contact: http://www.logilab.fr/ -- mailto:contact@logilab.fr
 :license: GNU Lesser General Public License, v2.1 - http://www.gnu.org/licenses
 """
+from __future__ import with_statement
+
 __docformat__ = "restructuredtext en"
 
+from threading import Lock
+
 from logilab.common.decorators import clear_cache
 
 from cubicweb import AuthenticationError, BadConnectionId
@@ -76,25 +80,32 @@
         """
         # with this authentication manager, session is actually a dbapi
         # connection
-        cnx = session.cnx
         login = req.get_authorization()[0]
-        # check cnx.login and not user.login, since in case of login by
+        # check session.login and not user.login, since in case of login by
         # email, login and cnx.login are the email while user.login is the
         # actual user login
         if login and session.login != login:
             raise InvalidSession('login mismatch')
         try:
-            # calling cnx.user() check connection validity, raise
-            # BadConnectionId on failure
-            user = cnx.user(req)
-        except BadConnectionId:
-            # check if a connection should be automatically restablished
-            if (login is None or login == session.login):
-                cnx = self._authenticate(session.login, session.authinfo)
+            lock = session.reconnection_lock
+        except AttributeError:
+            lock = session.reconnection_lock = Lock()
+        # need to be locked two avoid duplicated reconnections on concurrent
+        # requests
+        with lock:
+            cnx = session.cnx
+            try:
+                # calling cnx.user() check connection validity, raise
+                # BadConnectionId on failure
                 user = cnx.user(req)
-                session.cnx = cnx
-            else:
-                raise InvalidSession('bad connection id')
+            except BadConnectionId:
+                # check if a connection should be automatically restablished
+                if (login is None or login == session.login):
+                    cnx = self._authenticate(session.login, session.authinfo)
+                    user = cnx.user(req)
+                    session.cnx = cnx
+                else:
+                    raise InvalidSession('bad connection id')
         return user
 
     def authenticate(self, req):