On 03/26/2015 08:57 PM, Thomas De Schampheleire wrote:
Hi Mads,
On Wed, Mar 25, 2015 at 7:54 PM, Mads Kiilerich <[email protected]> wrote:
On 03/25/2015 12:01 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire <[email protected]>
# 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'.
Hard question ... no easy answer ... and I changed my mind several times ...
Some random related but (IMO) non-conclusive links:
http://brockallen.com/2013/10/27/using-cookie-authentication-middleware-with-web-api-and-401-response-codes/
http://www.w3.org/html/wg/tracker/issues/13
https://github.com/tbroyer/http-cookie-auth/blob/master/spec/draft-broyer-http-cookie-auth.rst
http://webmasters.stackexchange.com/questions/24443/should-i-return-a-http-401-status-code-on-an-html-based-login-form
I think I will maintain that even though it might work and not is
explicitly wrong, HTTP authorization mechanisms are currently only
intended and supported for HTTP level authentication. It would be
unfortunate to respond with non-standard mechanism in Authorization
header - that will make it impossible to use the API with "clever"
standards compliant client software / middleware.
Also, a random link for some inspiration:
https://groups.google.com/forum/#!topic/json-rpc/PN462g49yL8 where
Kallithea pretty much is option b (and I don't want to mix it up with c).
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?
I think it is the nature of (our?) json-rpc to be transport independant.
All response have an error field that is used for reporting errors. Our
documentation says: "All responses from API will be ``HTTP/1.0 200 OK``.
If there is an error, the reponse will have a failure description in
*error* and *result* will be null."
Some examples of error reporting:
{"error": "Incorrect JSON query missing 'api_key'", "id": null,
"result": null}
{"error": "Invalid API KEY", "id": 1, "result": null}
{"error": "No such method: foo", "id": 1, "result": null}
{"error": "Missing non optional `repoid` arg in JSON DATA", "id": 1,
"result": null}
{"error": "repository `foo` does not exist", "id": 1, "result": null}
I think missing / bad authentication or requests to a disabled method
fits perfectly into that scheme.
Do you see problems with that?
I guess one problem is that it would be nice to have some stable
identifier of the kind of error instead of just having the human
readable error message. We could perhaps redefine the error field or add
another field, depending on what is most 'standard json-rpc'. That is a
general existing problem that I guess would solve this problem too.
/Mads
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
