On 05/10/2015 08:22 PM, Thomas De Schampheleire wrote:
# HG changeset patch # User Thomas De Schampheleire <[email protected]> # Date 1427743622 -7200 # Mon Mar 30 21:27:02 2015 +0200 # Node ID b8ff1ec9f8e70a4540ab03db822367cde8ea1df2 # Parent 126d600ac54455fc07d40b65f511b73577090757 auth: return early in LoginRequired on API key validationSimplify 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 (redirect to login page), 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, 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 @@ -59,6 +59,7 @@ from kallithea.lib.utils import get_repo get_user_group_slug, conditional_cache from kallithea.lib.caching_query import FromCache+from webob.exc import HTTPForbidden log = logging.getLogger(__name__) @@ -746,31 +747,31 @@ class LoginRequired(object):cls = fargs[0] user = cls.authuser loc = "%s:%s" % (cls.__class__.__name__, func.__name__) + log.debug('Checking access for user %s @ %s' % (user, loc))# check if our IP is allowedif not user.ip_allowed: return redirect_to_login(_('IP %s not allowed' % (user.ip_addr)))- # 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:]) + # check if we used an API key and it's a valid one + api_key = request.GET.get('api_key') + if api_key is not None: + # 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 is NOT valid' % api_key[-4:]) + return redirect_to_login(_('Invalid API key')) else: - api_access_valid = False - if not _api_key: - log.debug("API KEY *NOT* present in request") - else: - log.warning("API KEY ****%s *NOT* valid" % _api_key[-4:]) + # controller does not allow API access + log.warning('API access to %s is not allowed' % loc) + raise HTTPForbidden()
I pushed these two, thanks .. but used abort(403) to be consistent with the other 403 in the same function.
/Mads _______________________________________________ kallithea-general mailing list [email protected] http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
