From 71de13d7c5b7923b9d8848ea71a3793ea5d368ee Mon Sep 17 00:00:00 2001
From: Pieter Lexis <pieter.lexis@powerdns.com>
Date: Wed, 23 May 2018 18:18:36 +0200
Subject: [PATCH] API: 404 on non-existing CryptoKey keyid

---
 pdns/ws-auth.cc                         | 28 ++++++++++++++-----------
 regression-tests.api/test_cryptokeys.py |  3 +--
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index 5a38d0f9d..ce66ceae2 100644
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -906,6 +906,21 @@ static void apiZoneMetadataKind(HttpRequest* req, HttpResponse* resp) {
     throw HttpMethodNotAllowedException();
 }
 
+// Throws 404 if the key with inquireKeyId does not exist
+static void apiZoneCryptoKeysCheckKeyExists(DNSName zonename, int inquireKeyId, 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();
+  }
+}
+
 static void apiZoneCryptokeysGET(DNSName zonename, int inquireKeyId, HttpResponse *resp, DNSSECKeeper *dk) {
   DNSSECKeeper::keyset_t keyset=dk->getKeys(zonename, false);
 
@@ -972,18 +987,6 @@ static void apiZoneCryptokeysGET(DNSName zonename, int inquireKeyId, HttpRespons
  *      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 = 204;
@@ -1167,6 +1170,7 @@ static void apiZoneCryptokeys(HttpRequest *req, HttpResponse *resp) {
   int inquireKeyId = -1;
   if (req->parameters.count("key_id")) {
     inquireKeyId = std::stoi(req->parameters["key_id"]);
+    apiZoneCryptoKeysCheckKeyExists(zonename, inquireKeyId, &dk);
   }
 
   if (req->method == "GET") {
diff --git a/regression-tests.api/test_cryptokeys.py b/regression-tests.api/test_cryptokeys.py
index a3469ca9a..8a4c874d6 100644
--- a/regression-tests.api/test_cryptokeys.py
+++ b/regression-tests.api/test_cryptokeys.py
@@ -71,8 +71,7 @@ class Cryptokeys(ApiTestCase):
         self.remove_zone_key(self.keyid)
         #checks for key is gone. Its ok even if no key had to be deleted. Or something went wrong with the backend.
         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.content, b"")
+        self.assertEquals(r.status_code, 404)
 
     # Prepares the json object for Post and sends it to the server
     def add_key(self, content='', type='ksk', active='true', algo='', bits=None):
-- 
2.40.0