[application/connect] simplify connection logic
``application.connect`` now either sets a full featured ``DBAPISession`` to the
``WebRequest`` object or raises ``AuthenticationError``. The creation and usage
of a fake DBAPISession is now handled by ``main_handle_request`` when needed.
This means that fake DBAPISession are no longer tracked by the session manager
and that user are not given anyway to retrieve them for a later request.
This fake DBAPISession is still passed to ``core_handle`` because multiple cubes
like registration or forgotten password need this behavior. We would like to get
ride of it in the future.
This clarification of the connection API greatly simplifies ``DBAPISession``
retrieval//creation process opening the
way to improvements in this area.
Related to #2503918
--- a/devtools/testlib.py Thu Jun 13 15:36:10 2013 +0200
+++ b/devtools/testlib.py Thu Jun 13 18:50:19 2013 +0200
@@ -39,7 +39,7 @@
from logilab.common.deprecation import deprecated, class_deprecated
from logilab.common.shellutils import getlogin
-from cubicweb import ValidationError, NoSelectableObject
+from cubicweb import ValidationError, NoSelectableObject, AuthenticationError
from cubicweb import cwconfig, dbapi, devtools, web, server
from cubicweb.utils import json
from cubicweb.sobjects import notification
@@ -787,12 +787,10 @@
self.assertEqual(session.anonymous_session, False)
def assertAuthFailure(self, req, nbsessions=0):
- self.app.connect(req)
- self.assertIsInstance(req.session, dbapi.DBAPISession)
- self.assertEqual(req.session.cnx, None)
- self.assertIsInstance(req.cnx, (dbapi._NeedAuthAccessMock, NoneType))
- # + 1 since we should still have session without connection set
- self.assertEqual(len(self.open_sessions), nbsessions + 1)
+ with self.assertRaises(AuthenticationError):
+ self.app.connect(req)
+ # +0 since we do not track the opened session
+ self.assertEqual(len(self.open_sessions), nbsessions)
clear_cache(req, 'get_authorization')
# content validation #######################################################
--- a/web/application.py Thu Jun 13 15:36:10 2013 +0200
+++ b/web/application.py Thu Jun 13 18:50:19 2013 +0200
@@ -212,44 +212,17 @@
sessioncookie = self.session_cookie(req)
try:
sessionid = str(cookie[sessioncookie].value)
- except KeyError: # no session cookie
- session = self.open_session(req)
- else:
- try:
- session = self.get_session(req, sessionid)
- except InvalidSession:
- # try to open a new session, so we get an anonymous session if
- # allowed
- 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)
+ self.get_session(req, sessionid)
+ except (KeyError, InvalidSession): # no valid session cookie
+ self.open_session(req)
def get_session(self, req, sessionid):
session = self.session_manager.get_session(req, sessionid)
session.mtime = time()
return session
- def open_session(self, req, allow_no_cnx=True):
- session = self.session_manager.open_session(req, allow_no_cnx=allow_no_cnx)
+ def open_session(self, req):
+ session = self.session_manager.open_session(req)
sessioncookie = self.session_cookie(req)
secure = req.https and req.base_url().startswith('https://')
req.set_cookie(sessioncookie, session.sessionid,
@@ -362,7 +335,13 @@
req.set_header('WWW-Authenticate', [('Basic', {'realm' : realm })], raw=False)
content = ''
try:
- self.connect(req)
+ try:
+ self.connect(req)
+ except AuthenticationError:
+ # XXX We want to clean up this approach in the future. But
+ # several cubes like registration or forgotten password rely on
+ # this principle.
+ req.set_session(DBAPISession(None))
# DENY https acces for anonymous_user
if (req.https
and req.session.anonymous_session
--- a/web/test/unittest_application.py Thu Jun 13 15:36:10 2013 +0200
+++ b/web/test/unittest_application.py Thu Jun 13 18:50:19 2013 +0200
@@ -267,9 +267,9 @@
def _test_cleaned(self, kwargs, injected, cleaned):
req = self.request(**kwargs)
- page = self.app.handle_request(req, 'view')
- self.assertFalse(injected in page, (kwargs, injected))
- self.assertTrue(cleaned in page, (kwargs, cleaned))
+ page = self.app_handle_request(req, 'view')
+ self.assertNotIn(injected, page)
+ self.assertIn(cleaned, page)
def test_nonregr_script_kiddies(self):
"""test against current script injection"""
@@ -319,8 +319,9 @@
def test_http_auth_no_anon(self):
req, origsession = self.init_authentication('http')
self.assertAuthFailure(req)
- self.assertRaises(AuthenticationError, self.app_handle_request, req, 'login')
- self.assertEqual(req.cnx, None)
+ self.app.handle_request(req, 'login')
+ self.assertEqual(401, req.status_out)
+ clear_cache(req, 'get_authorization')
authstr = base64.encodestring('%s:%s' % (self.admlogin, self.admpassword))
req.set_request_header('Authorization', 'basic %s' % authstr)
self.assertAuthSuccess(req, origsession)
@@ -331,9 +332,10 @@
req, origsession = self.init_authentication('cookie')
self.assertAuthFailure(req)
try:
- form = self.app_handle_request(req, 'login')
+ form = self.app.handle_request(req, 'login')
except Redirect as redir:
self.fail('anonymous user should get login form')
+ clear_cache(req, 'get_authorization')
self.assertTrue('__login' in form)
self.assertTrue('__password' in form)
self.assertEqual(req.cnx, None)
--- a/web/views/sessions.py Thu Jun 13 15:36:10 2013 +0200
+++ b/web/views/sessions.py Thu Jun 13 18:50:19 2013 +0200
@@ -64,22 +64,15 @@
req.set_session(session, user)
return session
- def open_session(self, req, allow_no_cnx=True):
+ def open_session(self, req):
"""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)
"""
- try:
- cnx, login = self.authmanager.authenticate(req)
- except AuthenticationError:
- if allow_no_cnx:
- session = DBAPISession(None)
- else:
- raise
- else:
- session = DBAPISession(cnx, login)
+ cnx, login = self.authmanager.authenticate(req)
+ session = DBAPISession(cnx, login)
self._sessions[session.sessionid] = session
# associate the connection to the current request
req.set_session(session)