On 03/25/2015 12:01 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire <thomas.de.schamphele...@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', '')
I think we should use request.GET.get('api_key') and thus get None if no
key is provided (empty or not). (And let's get rid of the leading _
while touching it.)
- 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:
- and here we should use: if _api_key is not None
That would make it more obvious that an empty key is an (invalid) key.
/Mads
+ # 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()
_______________________________________________
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
_______________________________________________
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
