Mercurial > kallithea

changeset 6383:06398585de03

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
auth: track API key used for authentication in AuthUser This allows us to define only once how an API key is passed to the app. We might e.g. allow API keys to be passed in an HTTP header; with this change, we only need to update the code in one place. Also change the code to verify up front that the API key resolved to a valid and active user, so LoginRequired doesn't need to do that. Also return plain 403 Forbidden for bad API keys instead of redirecting to the login form, which makes more sense for non-interactive clients (the typical users of API keys).
author Søren Løvborg <sorenl@unity3d.com>
date Thu, 10 Nov 2016 20:38:40 +0100
parents 245b4e3abf39
children 9cf90371d0f1
files kallithea/lib/auth.py kallithea/lib/base.py kallithea/tests/functional/test_login.py
diffstat 3 files changed, 22 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/lib/auth.py	Mon Jan 02 20:35:25 2017 +0100
+++ b/kallithea/lib/auth.py	Thu Nov 10 20:38:40 2016 +0100
@@ -473,11 +473,12 @@
     "default". Use `is_anonymous` to check for both "default" and "no user".
     """
 
-    def __init__(self, user_id=None, dbuser=None,
+    def __init__(self, user_id=None, dbuser=None, authenticating_api_key=None,
             is_external_auth=False):
 
         self.is_authenticated = False
         self.is_external_auth = is_external_auth
+        self.authenticating_api_key = authenticating_api_key
 
         user_model = UserModel()
         self._default_user = User.get_default_user(cache=True)
@@ -738,23 +739,19 @@
         if not AuthUser.check_ip_allowed(user, controller.ip_addr):
             raise _redirect_to_login(_('IP %s not allowed') % controller.ip_addr)
 
-        # check if we used an API key and it's a valid one
-        api_key = request.GET.get('api_key')
+        # Check if we used an API key to authenticate.
+        api_key = user.authenticating_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:])
-                    raise _redirect_to_login(_('Invalid API key'))
-            else:
+            # Check that controller is enabled for API key usage.
+            if not self.api_access and not allowed_api_access(loc, api_key=api_key):
                 # controller does not allow API access
                 log.warning('API access to %s is not allowed', loc)
                 raise HTTPForbidden()
 
+            log.info('user %s authenticated with API key ****%s @ %s',
+                     user, api_key[-4:], loc)
+            return func(*fargs, **fkwargs)
+
         # 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
--- a/kallithea/lib/base.py	Mon Jan 02 20:35:25 2017 +0100
+++ b/kallithea/lib/base.py	Thu Nov 10 20:38:40 2016 +0100
@@ -372,10 +372,13 @@
         """
 
         # Authenticate by API key
-        if api_key:
-            # when using API_KEY we are sure user exists.
-            return AuthUser(dbuser=User.get_by_api_key(api_key),
-                            is_external_auth=True)
+        if api_key is not None:
+            au = AuthUser(dbuser=User.get_by_api_key(api_key),
+                authenticating_api_key=api_key, is_external_auth=True)
+            if au.is_anonymous:
+                log.warning('API key ****%s is NOT valid', api_key[-4:])
+                raise webob.exc.HTTPForbidden(_('Invalid API key'))
+            return au
 
         # Authenticate by session cookie
         # In ancient login sessions, 'authuser' may not be a dict.
--- a/kallithea/tests/functional/test_login.py	Mon Jan 02 20:35:25 2017 +0100
+++ b/kallithea/tests/functional/test_login.py	Thu Nov 10 20:38:40 2016 +0100
@@ -465,10 +465,10 @@
 
     @parametrize('test_name,api_key,code', [
         ('none', None, 302),
-        ('empty_string', '', 302),
-        ('fake_number', '123456', 302),
-        ('fake_not_alnum', 'a-z', 302),
-        ('fake_api_key', '0123456789abcdef0123456789ABCDEF01234567', 302),
+        ('empty_string', '', 403),
+        ('fake_number', '123456', 403),
+        ('fake_not_alnum', 'a-z', 403),
+        ('fake_api_key', '0123456789abcdef0123456789ABCDEF01234567', 403),
         ('proper_api_key', True, 200)
     ])
     def test_access_whitelisted_page_via_api_key(self, test_name, api_key, code):
@@ -496,4 +496,4 @@
             #patch the API key and make it expired
             new_api_key.expires = 0
             Session().commit()
-            self._api_key_test(new_api_key.api_key, status=302)
+            self._api_key_test(new_api_key.api_key, status=403)