[PATCH 1 of 5] auth: do not redirect to login page on invalid API key

[Thomas De Schampheleire]

Hi Mads,

On Wed, Mar 25, 2015 at 7:54 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 03/25/2015 12:01 PM, Thomas De Schampheleire wrote:
>>
>> # HG changeset patch
>> # User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>> # Date 1427269791 -3600
>> #      Wed Mar 25 08:49:51 2015 +0100
>> # Node ID c5828585502f1a061f162abe8cbd181c17039843
>> # Parent  6017996e4dcfda0f5623498a45c51bb184eb67bb
>> auth: do not redirect to login page on invalid API key
>>
>> When accessing Kallithea through an API call, providing an API key, it
>> doesn't make sense to redirect to a login page on failed authentication.
>> Instead, raise a 401 Unauthorized exception.
>>
>> The WWW-authenticate header is a mandatory element for 401 Unauthorized,
>> as
>> specified by RFC 7235. The exact contents do not seem to be important, so
>> define a custom auth scheme 'APIKEY' with a realm of 'Kallithea'.
>>
>> diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py
>> --- a/kallithea/lib/auth.py
>> +++ b/kallithea/lib/auth.py
>> @@ -58,6 +58,7 @@
>>       get_user_group_slug, conditional_cache
>>   from kallithea.lib.caching_query import FromCache
>>   +from webob.exc import HTTPUnauthorized
>>     log = logging.getLogger(__name__)
>>   @@ -763,6 +764,8 @@
>>                       log.debug("API KEY *NOT* present in request")
>>                   else:
>>                       log.warning("API KEY ****%s *NOT* valid" %
>> _api_key[-4:])
>> +                    headers = [('WWW-Authenticate', 'APIKEY
>> realm="Kallithea"')]
>
>
> The API does not use HTTP authentication. I thus think it is misleading to
> return 401 and WWW-Authenticate.
>
> I think it would be better to just fail with 400 Bad Request.
>
> When we don't add headers, we can just use "return abort(400)" as done
> elsewhere in the same file.

The API does not use the classic 'Basic' nor 'Digest' authentication
methods of HTTP, but IMO passing the API key is a form of
authentication in the spirit of the HTTP RFC 7235:

------------
3.1.  401 Unauthorized

   The 401 (Unauthorized) status code indicates that the request has not
   been applied because it lacks valid authentication credentials for
   the target resource.  The server generating a 401 response MUST send
   a WWW-Authenticate header field (Section 4.1) containing at least one
   challenge applicable to the target resource.

   If the request included authentication credentials, then the 401
   response indicates that authorization has been refused for those
   credentials.  The user agent MAY repeat the request with a new or
   replaced Authorization header field (Section 4.2).  If the 401
   response contains the same challenge as the prior response, and the
   user agent has already attempted authentication at least once, then
   the user agent SHOULD present the enclosed representation to the
   user, since it usually contains relevant diagnostic information.

--------------

except for the fact that we are not using the Authorization header but
a GET value 'api_key'.

Failing with 400 Bad Request is indeed also an option, but it would
not allow clear differentiation between a bad API key, or invalid
parameters.

And what about API requests to methods that are not exposed through
API? In the patch I had used 403 Forbidden. Do you suggest to keep
this or move to 400 Bad Request too?

Best regards,
Thomas