[web session] fix session handling so we get a chance to have for instance the 'forgotpwd' feature working on a site where anonymous are not allowed stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Wed, 25 May 2011 10:58:43 +0200
branchstable
changeset 7428 5338d895b891
parent 7426 254bc099db1a
child 7429 20ef21926774
child 7430 ef5165fa99e0
[web session] fix session handling so we get a chance to have for instance the 'forgotpwd' feature working on a site where anonymous are not allowed fix several pbs: * we need a session id and a session cookie anyway, else subsequent http queries are unrelated * this imply some changes in the session attribution workflow for session without a cnx * some views/selectors must be fixed for cases where session has no cnx On the way, avoid unnecessary Redirect on successful login. closes #750543
dbapi.py
selectors.py
web/application.py
web/views/basecomponents.py
web/views/basetemplates.py
web/views/debug.py
web/views/sessions.py
--- a/dbapi.py	Wed May 25 08:51:45 2011 +0200
+++ b/dbapi.py	Wed May 25 10:58:43 2011 +0200
@@ -30,6 +30,7 @@
 from itertools import count
 from warnings import warn
 from os.path import join
+from uuid import uuid4
 
 from logilab.common.logging_ext import set_log_methods
 from logilab.common.decorators import monkeypatch
@@ -246,7 +247,7 @@
         if cnx is not None:
             self.sessionid = cnx.sessionid
         else:
-            self.sessionid = None
+            self.sessionid = uuid4().hex
 
     @property
     def anonymous_session(self):
--- a/selectors.py	Wed May 25 08:51:45 2011 +0200
+++ b/selectors.py	Wed May 25 10:58:43 2011 +0200
@@ -1342,6 +1342,8 @@
 
     @lltrace
     def __call__(self, cls, req, rset=None, row=None, col=0, **kwargs):
+        if not req.cnx:
+            return 0
         user = req.user
         if user is None:
             return int('guests' in self.expected)
--- a/web/application.py	Wed May 25 08:51:45 2011 +0200
+++ b/web/application.py	Wed May 25 10:58:43 2011 +0200
@@ -1,4 +1,4 @@
-# copyright 2003-2010 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2011 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -204,17 +204,34 @@
             except InvalidSession:
                 # try to open a new session, so we get an anonymous session if
                 # allowed
-                try:
-                    session = self.open_session(req)
-                except AuthenticationError:
-                    req.remove_cookie(cookie, sessioncookie)
-                    raise
+                session = self.open_session(req)
+            else:
+                if not session.cnx:
+                    # session exists but is not bound to a connection. We should
+                    # try to authenticate
+                    loginsucceed = False
+                    try:
+                        if self.open_session(req, allow_no_cnx=False):
+                            loginsucceed = True
+                    except Redirect:
+                        # may be raised in open_session (by postlogin mechanism)
+                        # on successful connection
+                        loginsucceed = True
+                        raise
+                    except AuthenticationError:
+                        # authentication failed, continue to use this session
+                        req.set_session(session)
+                    finally:
+                        if loginsucceed:
+                            # session should be replaced by new session created
+                            # in open_session
+                            self.session_manager.close_session(session)
 
     def get_session(self, req, sessionid):
         return self.session_manager.get_session(req, sessionid)
 
-    def open_session(self, req):
-        session = self.session_manager.open_session(req)
+    def open_session(self, req, allow_no_cnx=True):
+        session = self.session_manager.open_session(req, allow_no_cnx=allow_no_cnx)
         cookie = req.get_cookie()
         sessioncookie = self.session_cookie(req)
         cookie[sessioncookie] = session.sessionid
@@ -279,10 +296,7 @@
         sessions (i.e. a new connection may be created or an already existing
         one may be reused
         """
-        try:
-            self.session_handler.set_session(req)
-        except AuthenticationError:
-            req.set_session(DBAPISession(None))
+        self.session_handler.set_session(req)
 
     # publish methods #########################################################
 
@@ -365,11 +379,12 @@
                 # redirect is raised by edit controller when everything went fine,
                 # so try to commit
                 try:
-                    txuuid = req.cnx.commit()
-                    if txuuid is not None:
-                        msg = u'<span class="undo">[<a href="%s">%s</a>]</span>' %(
-                            req.build_url('undo', txuuid=txuuid), req._('undo'))
-                        req.append_to_redirect_message(msg)
+                    if req.cnx:
+                        txuuid = req.cnx.commit()
+                        if txuuid is not None:
+                            msg = u'<span class="undo">[<a href="%s">%s</a>]</span>' %(
+                                req.build_url('undo', txuuid=txuuid), req._('undo'))
+                            req.append_to_redirect_message(msg)
                 except ValidationError, ex:
                     self.validation_error_handler(req, ex)
                 except Unauthorized, ex:
--- a/web/views/basecomponents.py	Wed May 25 08:51:45 2011 +0200
+++ b/web/views/basecomponents.py	Wed May 25 10:58:43 2011 +0200
@@ -29,8 +29,8 @@
 from logilab.common.deprecation import class_renamed
 from rql import parse
 
-from cubicweb.selectors import (yes, multi_etypes_rset, match_form_params,
-                                match_context, configuration_values,
+from cubicweb.selectors import (yes, no_cnx, match_form_params, match_context,
+                                multi_etypes_rset, configuration_values,
                                 anonymous_user, authenticated_user)
 from cubicweb.schema import display_name
 from cubicweb.utils import wrap_on_write
@@ -88,6 +88,7 @@
 class ApplLogo(HeaderComponent):
     """build the instance logo, usually displayed in the header"""
     __regid__ = 'logo'
+    __select__ = yes() # no need for a cnx
     order = -1
 
     def render(self, w):
@@ -150,7 +151,7 @@
 
 class AnonUserStatusLink(HeaderComponent):
     __regid__ = 'userstatus'
-    __select__ = HeaderComponent.__select__ & anonymous_user()
+    __select__ = anonymous_user()
     context = _('header-right')
     order = HeaderComponent.order - 10
 
@@ -159,7 +160,7 @@
 
 
 class AuthenticatedUserStatus(AnonUserStatusLink):
-    __select__ = HeaderComponent.__select__ & authenticated_user()
+    __select__ = authenticated_user()
 
     def render(self, w):
         # display useractions and siteactions
@@ -180,7 +181,7 @@
     """display messages given using the __message parameter into a special div
     section
     """
-    __select__ = yes()
+    __select__ = ~no_cnx()
     __regid__ = 'applmessages'
     # don't want user to hide this component using an cwproperty
     cw_property_defs = {}
--- a/web/views/basetemplates.py	Wed May 25 08:51:45 2011 +0200
+++ b/web/views/basetemplates.py	Wed May 25 10:58:43 2011 +0200
@@ -1,4 +1,4 @@
-# copyright 2003-2010 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2011 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
--- a/web/views/debug.py	Wed May 25 08:51:45 2011 +0200
+++ b/web/views/debug.py	Wed May 25 10:58:43 2011 +0200
@@ -1,4 +1,4 @@
-# copyright 2003-2010 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2011 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -133,6 +133,9 @@
             if sessions:
                 w(u'<ul>')
                 for session in sessions:
+                    if not session.cnx:
+                        w(u'<li>%s (NO CNX)</li>' % session.sessionid)
+                        continue
                     try:
                         last_usage_time = session.cnx.check()
                     except BadConnectionId:
--- a/web/views/sessions.py	Wed May 25 08:51:45 2011 +0200
+++ b/web/views/sessions.py	Wed May 25 10:58:43 2011 +0200
@@ -1,4 +1,4 @@
-# copyright 2003-2010 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2011 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -21,7 +21,7 @@
 
 __docformat__ = "restructuredtext en"
 
-from cubicweb import RepositoryError, Unauthorized
+from cubicweb import RepositoryError, Unauthorized, AuthenticationError
 from cubicweb.web import InvalidSession, Redirect
 from cubicweb.web.application import AbstractSessionManager
 from cubicweb.dbapi import DBAPISession
@@ -49,28 +49,36 @@
 
     def get_session(self, req, sessionid):
         """return existing session for the given session identifier"""
-        if not sessionid in self._sessions:
+        if sessionid not in self._sessions:
             raise InvalidSession()
         session = self._sessions[sessionid]
-        try:
-            user = self.authmanager.validate_session(req, session)
-        except InvalidSession:
-            # invalid session
-            self.close_session(session)
-            raise
-        # associate the connection to the current request
-        req.set_session(session, user)
+        if session.cnx:
+            try:
+                user = self.authmanager.validate_session(req, session)
+            except InvalidSession:
+                # invalid session
+                self.close_session(session)
+                raise
+            # associate the connection to the current request
+            req.set_session(session, user)
         return session
 
-    def open_session(self, req):
+    def open_session(self, req, allow_no_cnx=True):
         """open and return a new session for the given request. The session is
         also bound to the request.
 
         raise :exc:`cubicweb.AuthenticationError` if authentication failed
         (no authentication info found or wrong user/password)
         """
-        cnx, login = self.authmanager.authenticate(req)
-        session = DBAPISession(cnx, login)
+        try:
+            cnx, login = self.authmanager.authenticate(req)
+        except AuthenticationError:
+            if allow_no_cnx:
+                session = DBAPISession(None)
+            else:
+                raise
+        else:
+            session = DBAPISession(cnx, login)
         self._sessions[session.sessionid] = session
         # associate the connection to the current request
         req.set_session(session)
@@ -89,15 +97,16 @@
         args = req.form
         for forminternal_key in ('__form_id', '__domid', '__errorurl'):
             args.pop(forminternal_key, None)
-        args['__message'] = req._('welcome %s !') % req.user.login
-        if 'vid' in req.form:
-            args['vid'] = req.form['vid']
-        if 'rql' in req.form:
-            args['rql'] = req.form['rql']
         path = req.relative_path(False)
         if path == 'login':
             path = 'view'
-        raise Redirect(req.build_url(path, **args))
+            args['__message'] = req._('welcome %s !') % req.user.login
+            if 'vid' in req.form:
+                args['vid'] = req.form['vid']
+            if 'rql' in req.form:
+                args['rql'] = req.form['rql']
+            raise Redirect(req.build_url(path, **args))
+        req.set_message(req._('welcome %s !') % req.user.login)
 
     def _update_last_login_time(self, req):
         # XXX should properly detect missing permission / non writeable source
@@ -120,10 +129,11 @@
         """
         self.info('closing http session %s' % session.sessionid)
         del self._sessions[session.sessionid]
-        try:
-            session.cnx.close()
-        except:
-            # already closed, may occurs if the repository session expired but
-            # not the web session
-            pass
-        session.cnx = None
+        if session.cnx:
+            try:
+                session.cnx.close()
+            except:
+                # already closed, may occur if the repository session expired
+                # but not the web session
+                pass
+            session.cnx = None