kallithea Changeset - 5c5f0eb45681

Changeset - 5c5f0eb45681

Parent rev.

Child rev.

[Not reviewed]

default

0 2 0

Mads Kiilerich - 7 years ago 2019-01-04 03:51:38
mads@kiilerich.com

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.

2 files changed with 16 insertions and 18 deletions:

kallithea/lib/auth.py

kallithea/lib/base.py

0 comments (0 inline, 0 general)

kallithea/lib/auth.py

➞

Show inline comments

@@ @@ -666,36 +666,24 @@ class LoginRequired(object): @@
     def __init__(self, allow_default_user=False):
         self.allow_default_user = allow_default_user
     def __call__(self, func):
         return decorator(self.__wrapper, func)
     def __wrapper(self, func, *fargs, **fkwargs):
         controller = fargs[0]
         user = request.authuser
         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:
                 log.info('default user @ %s', loc)
                 return func(*fargs, **fkwargs)
             log.info('default user is not accepted here @ %s', loc)
         elif user.is_anonymous: # default user is disabled and no proper authentication
             log.warning('user is anonymous and NOT authenticated with regular auth @ %s', loc)
         else: # regular authentication
             log.info('user %s authenticated with regular auth @ %s', user, loc)
             return func(*fargs, **fkwargs)
         raise _redirect_to_login()

kallithea/lib/base.py

➞

Show inline comments

@@ @@ -318,24 +318,36 @@ class BaseVCSController(object): @@
         finally:
             log = logging.getLogger('kallithea.' + self.__class__.__name__)
             log.debug('Request time: %.3fs', time.time() - start)
             meta.Session.remove()
 class BaseController(TGController):
     def _before(self, *args, **kwargs):
         """
         _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()
         # Visual options
         c.visual = AttributeDict({})
         ## DB stored
         c.visual.show_public_icon = str2bool(rc_config.get('show_public_icon'))
         c.visual.show_private_icon = str2bool(rc_config.get('show_private_icon'))
         c.visual.stylify_metalabels = str2bool(rc_config.get('stylify_metalabels'))
         c.visual.page_size = safe_int(rc_config.get('dashboard_items', 100))
         c.visual.admin_grid_items = safe_int(rc_config.get('admin_grid_items', 100))
@@ @@ -458,44 +470,42 @@ class BaseController(TGController): @@
                 type, params = request.authorization
             except (ValueError, TypeError):
                 pass
             else:
                 if type.lower() == 'bearer':
                     api_key = params # bearer token is an api key too
             if api_key is None:
                 authuser = self._determine_auth_user(
                     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')
                 raise webob.exc.HTTPForbidden()
             # 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,
                 safe_unicode(_get_access_path(environ)),
+            )
             return super(BaseController, self).__call__(environ, context)
         except webob.exc.HTTPException as e:
             return e
 class BaseRepoController(BaseController):
     """

0 comments (0 inline, 0 general)