From: Pieter Lexis <pieter.lexis@powerdns.com>
Date: Wed, 23 May 2018 16:09:05 +0000 (+0200)
Subject: API: correct HTTP errors for cryptokey deletions
X-Git-Tag: dnsdist-1.3.1~78^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=75191dc47fe97e78532e70dbcfecab95e5a9132a;p=thirdparty%2Fpdns.git

API: correct HTTP errors for cryptokey deletions
---

diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index 63214bddb7..5a38d0f9d8 100644
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -965,14 +965,28 @@ static void apiZoneCryptokeysGET(DNSName zonename, int inquireKeyId, HttpRespons
  * It deletes a key from :zone_name specified by :cryptokey_id.
  * Server Answers:
  * Case 1: the backend returns true on removal. This means the key is gone.
- *      The server returns 200 OK, no body.
+ *      The server returns 204 No Content, no body.
  * Case 2: the backend returns false on removal. An error occurred.
- *      The sever returns 422 Unprocessable Entity with message "Could not DELETE :cryptokey_id".
+ *      The server returns 422 Unprocessable Entity with message "Could not DELETE :cryptokey_id".
+ * Case 3: the key or zone does not exist.
+ *      The server returns 404 Not Found
  * */
 static void apiZoneCryptokeysDELETE(DNSName zonename, int inquireKeyId, HttpRequest *req, HttpResponse *resp, DNSSECKeeper *dk) {
+  DNSSECKeeper::keyset_t keyset=dk->getKeys(zonename, false);
+  bool found = false;
+  for(const auto& value : keyset) {
+    if (value.second.id == (unsigned) inquireKeyId) {
+      found = true;
+      break;
+    }
+  }
+  if (!found) {
+    throw HttpNotFoundException();
+  }
+
   if (dk->removeKey(zonename, inquireKeyId)) {
     resp->body = "";
-    resp->status = 200;
+    resp->status = 204;
   } else {
     resp->setErrorResult("Could not DELETE " + req->parameters["key_id"], 422);
   }
diff --git a/regression-tests.api/test_cryptokeys.py b/regression-tests.api/test_cryptokeys.py
index e2ce4de8bd..a3469ca9a3 100644
--- a/regression-tests.api/test_cryptokeys.py
+++ b/regression-tests.api/test_cryptokeys.py
@@ -43,7 +43,7 @@ class Cryptokeys(ApiTestCase):
 
         #checks the status code. I don't know how to test explicit that the backend fail removing a key.
         r = self.session.delete(self.url("/api/v1/servers/localhost/zones/"+self.zone+"/cryptokeys/"+self.keyid))
-        self.assertEquals(r.status_code, 200)
+        self.assertEquals(r.status_code, 204)
         self.assertEquals(r.content, b"")
 
         # Check that the key is actually deleted
@@ -55,6 +55,11 @@ class Cryptokeys(ApiTestCase):
         r = self.session.get(self.url("/api/v1/servers/localhost/zones/"+self.zone+"fail/cryptokeys/"+self.keyid))
         self.assertEquals(r.status_code, 404)
 
+    def test_delete_wrong_id(self):
+        self.keyid = self.add_zone_key()
+        r = self.session.delete(self.url("/api/v1/servers/localhost/zones/"+self.zone+"/cryptokeys/1234567"))
+        self.assertEquals(r.status_code, 404)
+
     def test_delete_wrong_zone(self):
         self.keyid = self.add_zone_key()
         #checks for not covered zonename