changeset 6381:ee6b7e9f34e6

auth: perform basic HTTP security checks already in BaseController There's no reason to postpone these to a LoginRequired decorated controller function. This way, they run unconditionally for all subclasses of BaseController (so everything except JSON-RPC and VCS controllers).
author Søren Løvborg <sorenl@unity3d.com>
date Fri, 23 Sep 2016 00:29:30 +0200
parents c987aa2eb2a8
children 245b4e3abf39
files kallithea/lib/auth.py kallithea/lib/base.py kallithea/tests/functional/test_basecontroller.py
diffstat 3 files changed, 47 insertions(+), 27 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/lib/auth.py	Thu Nov 10 15:09:47 2016 +0100
+++ b/kallithea/lib/auth.py	Fri Sep 23 00:29:30 2016 +0200
@@ -742,26 +742,6 @@
                 log.warning('API access to %s is not allowed', loc)
                 raise HTTPForbidden()
 
-        # Only allow the following HTTP request methods.
-        if request.method not in ['GET', 'HEAD', 'POST']:
-            raise HTTPMethodNotAllowed()
-
-        # Also verify the _method override - no longer allowed
-        _method = request.params.get('_method')
-        if _method is None:
-            pass # no override, no problem
-        else:
-            raise HTTPMethodNotAllowed()
-
-        # Make sure CSRF token never appears in the URL. If so, invalidate it.
-        if secure_form.token_key in request.GET:
-            log.error('CSRF key leak detected')
-            session.pop(secure_form.token_key, None)
-            session.save()
-            from kallithea.lib import helpers as h
-            h.flash(_("CSRF token leak has been detected - all form tokens have been expired"),
-                    category='error')
-
         # CSRF protection: Whenever a request has ambient authority (whether
         # through a session cookie or its origin IP address), it must include
         # the correct token, unless the HTTP method is GET or HEAD (and thus
@@ -774,13 +754,6 @@
                 log.error('CSRF check failed')
                 raise HTTPForbidden()
 
-        # WebOb already ignores request payload parameters for anything other
-        # than POST/PUT, but double-check since other Kallithea code relies on
-        # this assumption.
-        if request.method not in ['POST', 'PUT'] and request.POST:
-            log.error('%r request with payload parameters; WebOb should have stopped this', request.method)
-            raise HTTPBadRequest()
-
         # regular user authentication
         if user.is_authenticated or user.is_default_user:
             log.info('user %s authenticated with regular auth @ %s', user, loc)
--- a/kallithea/lib/base.py	Thu Nov 10 15:09:47 2016 +0100
+++ b/kallithea/lib/base.py	Fri Sep 23 00:29:30 2016 +0200
@@ -37,6 +37,7 @@
 import paste.httpexceptions
 import paste.auth.basic
 import paste.httpheaders
+from webhelpers.pylonslib import secure_form
 
 from pylons import config, tmpl_context as c, request, session
 from pylons.controllers import WSGIController
@@ -412,6 +413,36 @@
         # User is anonymous
         return AuthUser()
 
+    @staticmethod
+    def _basic_security_checks():
+        """Perform basic security/sanity checks before processing the request."""
+
+        # Only allow the following HTTP request methods.
+        if request.method not in ['GET', 'HEAD', 'POST']:
+            raise webob.exc.HTTPMethodNotAllowed()
+
+        # Also verify the _method override - no longer allowed.
+        if request.params.get('_method') is None:
+            pass # no override, no problem
+        else:
+            raise webob.exc.HTTPMethodNotAllowed()
+
+        # Make sure CSRF token never appears in the URL. If so, invalidate it.
+        if secure_form.token_key in request.GET:
+            log.error('CSRF key leak detected')
+            session.pop(secure_form.token_key, None)
+            session.save()
+            from kallithea.lib import helpers as h
+            h.flash(_('CSRF token leak has been detected - all form tokens have been expired'),
+                    category='error')
+
+        # WebOb already ignores request payload parameters for anything other
+        # than POST/PUT, but double-check since other Kallithea code relies on
+        # this assumption.
+        if request.method not in ['POST', 'PUT'] and request.POST:
+            log.error('%r request with payload parameters; WebOb should have stopped this', request.method)
+            raise webob.exc.HTTPBadRequest()
+
     def __call__(self, environ, start_response):
         """Invoke the Controller"""
 
@@ -422,6 +453,8 @@
             self.ip_addr = _get_ip_addr(environ)
             # make sure that we update permissions each time we call controller
 
+            self._basic_security_checks()
+
             #set globals for auth user
             self.authuser = c.authuser = request.user = self._determine_auth_user(
                 request.GET.get('api_key'),
@@ -433,6 +466,8 @@
                 safe_unicode(_get_access_path(environ)),
             )
             return WSGIController.__call__(self, environ, start_response)
+        except webob.exc.HTTPException as e:
+            return e(environ, start_response)
         finally:
             meta.Session.remove()
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/kallithea/tests/functional/test_basecontroller.py	Fri Sep 23 00:29:30 2016 +0200
@@ -0,0 +1,12 @@
+from kallithea.tests.base import TestController, url
+
+
+class TestBaseController(TestController):
+
+    def test_banned_http_methods(self):
+        self.app.request(url(controller='login', action='index'), method='PUT', status=405)
+        self.app.request(url(controller='login', action='index'), method='DELETE', status=405)
+
+    def test_banned_http_method_override(self):
+        self.app.get(url(controller='login', action='index'), {'_method': 'POST'}, status=405)
+        self.app.post(url(controller='login', action='index'), {'_method': 'PUT'}, status=405)