# HG changeset patch
# User Søren Løvborg <sorenl@unity3d.com>
# Date 1478806720 -3600
# Node ID 06398585de039e3767d19bf51c3de5e69b762dd3
# Parent  245b4e3abf399fb6770afb9b4916baab51bff15c
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).

diff -r 245b4e3abf39 -r 06398585de03 kallithea/lib/auth.py
--- 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
diff -r 245b4e3abf39 -r 06398585de03 kallithea/lib/base.py
--- 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.
diff -r 245b4e3abf39 -r 06398585de03 kallithea/tests/functional/test_login.py
--- 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)