# HG changeset patch # User Pierre-Yves David # Date 1371142219 -7200 # Node ID aa709bc6b6c14f21dc9e2763c5f9fb7505da9fd5 # Parent 0368b94921ed29841a4b5e47edd2a9b3ea190fae [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 diff -r 0368b94921ed -r aa709bc6b6c1 devtools/testlib.py --- 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 ####################################################### diff -r 0368b94921ed -r aa709bc6b6c1 web/application.py --- 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 diff -r 0368b94921ed -r aa709bc6b6c1 web/test/unittest_application.py --- 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) diff -r 0368b94921ed -r aa709bc6b6c1 web/views/sessions.py --- 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)