[PATCH 3 of 5] auth: return early in LoginRequired on API key validation

Thomas De Schampheleire patrickdepinguin at gmail.com
Wed Mar 25 07:01:31 EDT 2015


# HG changeset patch
# User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
# Date 1427274589 -3600
#      Wed Mar 25 10:09:49 2015 +0100
# Node ID 32ced9e32b506c7441339f67da3c4237de0e6fa3
# Parent  43b0bc9088d3eedacf7610ffbf2f4b429ae98c45
auth: return early in LoginRequired on API key validation

Simplify the logic in the LoginRequired decorator when Kallithea is accessed
using an API key. Either:
- the key is valid and API access is allowed for the accessed method
  (continue), or
- the key is invalid (401 Unauthorized), or
- the accessed method does not allow API access (403 Forbidden)

In none of these cases does it make sense to continue checking for user
authentication, or redirecting to a login page, so return early.

diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py
--- a/kallithea/lib/auth.py
+++ b/kallithea/lib/auth.py
@@ -58,7 +58,7 @@
     get_user_group_slug, conditional_cache
 from kallithea.lib.caching_query import FromCache
 
-from webob.exc import HTTPUnauthorized
+from webob.exc import HTTPUnauthorized, HTTPForbidden
 
 log = logging.getLogger(__name__)
 
@@ -738,6 +738,7 @@
         cls = fargs[0]
         user = cls.authuser
         loc = "%s:%s" % (cls.__class__.__name__, func.__name__)
+        log.debug('Checking access for user %s @ %s' % (user, loc))
 
         def redirect_to_login():
             p = url.current()
@@ -754,37 +755,34 @@
             return redirect_to_login()
 
         # check if we used an APIKEY and it's a valid one
-        # defined whitelist of controllers which API access will be enabled
         _api_key = request.GET.get('api_key', '')
-        api_access_valid = allowed_api_access(loc, api_key=_api_key)
-
-        # explicit controller is enabled or API is in our whitelist
-        if self.api_access or api_access_valid:
-            log.debug('Checking API KEY access for %s' % cls)
-            if _api_key and _api_key in user.api_keys:
-                api_access_valid = True
-                log.debug('API KEY ****%s is VALID' % _api_key[-4:])
-            else:
-                api_access_valid = False
-                if not _api_key:
-                    log.debug("API KEY *NOT* present in request")
+        if _api_key:
+            # explicit controller is enabled or API is in our whitelist
+            if self.api_access or allowed_api_access(loc, api_key=_api_key):
+                if _api_key in user.api_keys:
+                    log.info('user %s authenticated with API key ****%s @ %s'
+                             % (user, _api_key[-4:], loc))
+                    return func(*fargs, **fkwargs)
                 else:
-                    log.warning("API KEY ****%s *NOT* valid" % _api_key[-4:])
+                    log.warning('API key ****%s is NOT valid' % _api_key[-4:])
                     headers = [('WWW-Authenticate', 'APIKEY realm="Kallithea"')]
                     raise HTTPUnauthorized(headers=headers)
+            else:
+                # controller does not allow API access
+                log.warning('API access to %s is not allowed' % loc)
+                raise HTTPForbidden()
 
         log.debug('Checking if %s is authenticated @ %s' % (user.username, loc))
         reason = 'RegularAuth' if user.is_authenticated else 'APIAuth'
 
-        if user.is_authenticated or api_access_valid:
+        if user.is_authenticated:
             log.info('user %s authenticating with:%s IS authenticated on func %s '
                      % (user, reason, loc)
             )
             return func(*fargs, **fkwargs)
         else:
             log.warning('user %s authenticating with:%s NOT authenticated on func: %s: '
-                     'API_ACCESS:%s'
-                     % (user, reason, loc, api_access_valid)
+                     % (user, reason, loc)
             )
             return redirect_to_login()
 


More information about the kallithea-general mailing list