From: Pieter Lexis <pieter@plexis.eu>
Date: Tue, 10 Feb 2015 18:09:51 +0000 (+0100)
Subject: JSON-API: Send 401 on bad API-Key
X-Git-Tag: dnsdist-1.0.0-alpha1~277^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F2215%2Fhead;p=thirdparty%2Fpdns.git

JSON-API: Send 401 on bad API-Key

  * Closes #2179
  * We send an HTTP 401 (Unauthorized) when:
    * The API Key is wrong
    * The API Key is empty or missing
  * Authentication failures are logged as Error (was Debug)
  * Fix the API regression test to accept this 401 as valid
---

diff --git a/pdns/webserver.cc b/pdns/webserver.cc
index 506c06af97..308da1394d 100644
--- a/pdns/webserver.cc
+++ b/pdns/webserver.cc
@@ -118,14 +118,14 @@ static void apiWrapper(WebServer::HandlerFunction handler, HttpRequest* req, Htt
   resp->headers["access-control-allow-origin"] = "*";
 
   if (api_key.empty()) {
-    L<<Logger::Debug<<"HTTP API Request \"" << req->url.path << "\": Authentication failed, API Key missing in config" << endl;
-    throw HttpUnauthorizedException();
+    L<<Logger::Error<<"HTTP API Request \"" << req->url.path << "\": Authentication failed, API Key missing in config" << endl;
+    throw HttpUnauthorizedException("X-API-Key");
   }
   bool auth_ok = req->compareHeader("x-api-key", api_key) || req->getvars["api-key"]==api_key;
   
   if (!auth_ok) {
-    L<<Logger::Debug<<"HTTP Request \"" << req->url.path << "\": Authentication by API Key failed" << endl;
-    throw HttpBadRequestException();
+    L<<Logger::Error<<"HTTP Request \"" << req->url.path << "\": Authentication by API Key failed" << endl;
+    throw HttpUnauthorizedException("X-API-Key");
   }
 
   resp->headers["Content-Type"] = "application/json";
@@ -174,7 +174,7 @@ static void webWrapper(WebServer::HandlerFunction handler, HttpRequest* req, Htt
     bool auth_ok = req->compareAuthorization(web_password);
     if (!auth_ok) {
       L<<Logger::Debug<<"HTTP Request \"" << req->url.path << "\": Web Authentication failed" << endl;
-      throw HttpUnauthorizedException();
+      throw HttpUnauthorizedException("Basic");
     }
   }
 
diff --git a/pdns/webserver.hh b/pdns/webserver.hh
index cbfdc1f8a1..7eee5e2663 100644
--- a/pdns/webserver.hh
+++ b/pdns/webserver.hh
@@ -81,12 +81,17 @@ public:
 
 class HttpUnauthorizedException : public HttpException {
 public:
-  HttpUnauthorizedException() : HttpException(401)
+  HttpUnauthorizedException(string const &scheme) : HttpException(401)
   {
-    d_response.headers["WWW-Authenticate"] = "Basic realm=\"PowerDNS\"";
+    d_response.headers["WWW-Authenticate"] = scheme + " realm=\"PowerDNS\"";
   }
 };
 
+class HttpForbiddenException : public HttpException {
+public:
+  HttpForbiddenException() : HttpException(403) { };
+};
+
 class HttpNotFoundException : public HttpException {
 public:
   HttpNotFoundException() : HttpException(404) { };
diff --git a/regression-tests.api/test_Basics.py b/regression-tests.api/test_Basics.py
index c275018e5e..79c3d9ecfd 100644
--- a/regression-tests.api/test_Basics.py
+++ b/regression-tests.api/test_Basics.py
@@ -8,7 +8,7 @@ class TestBasics(ApiTestCase):
 
     def test_unauth(self):
         r = requests.get(self.url("/servers/localhost"))
-        self.assertEquals(r.status_code, requests.codes.bad_request)
+        self.assertEquals(r.status_code, requests.codes.unauthorized)
 
     def test_split_request(self):
         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)