Mercurial > kallithea

changeset 7703:5c5f0eb45681

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
auth: move CSRF checks from the optional LoginRequired to the more basic BaseController._before _before is not called for the CSRF-immune JSON-API controller and is thus a good place to check CSRF. This also apply CSRF protection to the login controller. The flag for needing CSRF checking is stored in the thread global request object when passed from __call__ to _before for regular controllers. It is thus also set for requests to the JSON-RPC controller, but not used.
author Mads Kiilerich <mads@kiilerich.com>
date Fri, 04 Jan 2019 03:51:38 +0100
parents 797883404f17
children 6104f9106a5a
files kallithea/lib/auth.py kallithea/lib/base.py
diffstat 2 files changed, 16 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/lib/auth.py	Fri Jan 04 03:42:23 2019 +0100
+++ b/kallithea/lib/auth.py	Fri Jan 04 03:51:38 2019 +0100
@@ -675,18 +675,6 @@
         loc = "%s:%s" % (controller.__class__.__name__, func.__name__)
         log.debug('Checking access for user %s @ %s', user, loc)
 
-        # 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
-        # guaranteed to be side effect free. In practice, the only situation
-        # where we allow side effects without ambient authority is when the
-        # authority comes from an API key; and that is handled above.
-        if user.authenticating_api_key is None and request.method not in ['GET', 'HEAD']:
-            token = request.POST.get(secure_form.token_key)
-            if not token or token != secure_form.authentication_token():
-                log.error('CSRF check failed')
-                raise HTTPForbidden()
-
         # regular user authentication
         if user.is_default_user:
             if self.allow_default_user:
--- a/kallithea/lib/base.py	Fri Jan 04 03:42:23 2019 +0100
+++ b/kallithea/lib/base.py	Fri Jan 04 03:51:38 2019 +0100
@@ -327,6 +327,18 @@
         """
         _before is called before controller methods and after __call__
         """
+        if request.needs_csrf_check:
+            # 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
+            # guaranteed to be side effect free. In practice, the only situation
+            # where we allow side effects without ambient authority is when the
+            # authority comes from an API key; and that is handled above.
+            token = request.POST.get(secure_form.token_key)
+            if not token or token != secure_form.authentication_token():
+                log.error('CSRF check failed')
+                raise webob.exc.HTTPForbidden()
+
         c.kallithea_version = __version__
         rc_config = Setting.get_app_settings()
 
@@ -467,18 +479,15 @@
                     session.get('authuser'),
                     ip_addr=ip_addr,
                 )
+                needs_csrf_check = request.method not in ['GET', 'HEAD']
 
             else:
                 dbuser = User.get_by_api_key(api_key)
                 if dbuser is None:
                     log.info('No db user found for authentication with API key ****%s from %s',
                              api_key[-4:], ip_addr)
-                authuser = AuthUser.make(
-                    dbuser=dbuser,
-                    authenticating_api_key=api_key,
-                    is_external_auth=True,
-                    ip_addr=ip_addr,
-                )
+                authuser = AuthUser.make(dbuser=dbuser, authenticating_api_key=api_key, is_external_auth=True, ip_addr=ip_addr)
+                needs_csrf_check = False # API key provides CSRF protection
 
             if authuser is None:
                 log.info('No valid user found')
@@ -487,6 +496,7 @@
             # set globals for auth user
             request.authuser = authuser
             request.ip_addr = ip_addr
+            request.needs_csrf_check = needs_csrf_check
 
             log.info('IP: %s User: %s accessed %s',
                 request.ip_addr, request.authuser,