From 1d98b4b67ba65ead409a36c55a60db8bce02087d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus456@gmail.com>
Date: Fri, 5 May 2017 05:39:23 +0300
Subject: [PATCH] MXS-1220: Return 403 Forbidden for invalid requests

The JSON API specification suggests that the API returns the 403 Forbidden
error when the user does an invalid request. The 400 Bad Request isn't the
ideal error for cases where the syntax is correct but the action being
performed is wrong.
---
 include/maxscale/json_api.h                   |  4 ++++
 server/core/resource.cc                       | 23 ++++++++++---------
 .../test/rest-api/test/schema_validation.js   |  5 ++++
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/include/maxscale/json_api.h b/include/maxscale/json_api.h
index d3d15ad31..35c4fcc6f 100644
--- a/include/maxscale/json_api.h
+++ b/include/maxscale/json_api.h
@@ -28,6 +28,10 @@ MXS_BEGIN_DECLS
 #define MXS_JSON_API_MONITORS  "/monitors/"
 #define MXS_JSON_API_SESSIONS  "/sessions/"
 #define MXS_JSON_API_MAXSCALE  "/maxscale/"
+#define MXS_JSON_API_THREADS   "/maxscale/threads/"
+#define MXS_JSON_API_LOGS      "/maxscale/logs/"
+#define MXS_JSON_API_TASKS     "/maxscale/tasks/"
+#define MXS_JSON_API_MODULES   "/maxscale/modules/"
 
 /**
  * @brief Create a JSON object
diff --git a/server/core/resource.cc b/server/core/resource.cc
index 701388668..9f3299fe8 100644
--- a/server/core/resource.cc
+++ b/server/core/resource.cc
@@ -18,6 +18,7 @@
 #include <maxscale/alloc.h>
 #include <maxscale/jansson.hh>
 #include <maxscale/spinlock.hh>
+#include <maxscale/json_api.h>
 
 #include "maxscale/httprequest.hh"
 #include "maxscale/httpresponse.hh"
@@ -150,7 +151,7 @@ HttpResponse cb_create_server(const HttpRequest& request)
         }
     }
 
-    return HttpResponse(MHD_HTTP_BAD_REQUEST);
+    return HttpResponse(MHD_HTTP_FORBIDDEN);
 }
 
 HttpResponse cb_alter_server(const HttpRequest& request)
@@ -167,7 +168,7 @@ HttpResponse cb_alter_server(const HttpRequest& request)
         }
     }
 
-    return HttpResponse(MHD_HTTP_BAD_REQUEST);
+    return HttpResponse(MHD_HTTP_FORBIDDEN);
 }
 
 HttpResponse cb_create_monitor(const HttpRequest& request)
@@ -184,7 +185,7 @@ HttpResponse cb_create_monitor(const HttpRequest& request)
         }
     }
 
-    return HttpResponse(MHD_HTTP_BAD_REQUEST);
+    return HttpResponse(MHD_HTTP_FORBIDDEN);
 }
 
 HttpResponse cb_alter_monitor(const HttpRequest& request)
@@ -201,7 +202,7 @@ HttpResponse cb_alter_monitor(const HttpRequest& request)
         }
     }
 
-    return HttpResponse(MHD_HTTP_BAD_REQUEST);
+    return HttpResponse(MHD_HTTP_FORBIDDEN);
 }
 
 HttpResponse cb_alter_service(const HttpRequest& request)
@@ -218,7 +219,7 @@ HttpResponse cb_alter_service(const HttpRequest& request)
         }
     }
 
-    return HttpResponse(MHD_HTTP_BAD_REQUEST);
+    return HttpResponse(MHD_HTTP_FORBIDDEN);
 }
 
 HttpResponse cb_delete_server(const HttpRequest& request)
@@ -230,7 +231,7 @@ HttpResponse cb_delete_server(const HttpRequest& request)
         return HttpResponse(MHD_HTTP_NO_CONTENT);
     }
 
-    return HttpResponse(MHD_HTTP_BAD_REQUEST);
+    return HttpResponse(MHD_HTTP_FORBIDDEN);
 }
 
 HttpResponse cb_delete_monitor(const HttpRequest& request)
@@ -242,7 +243,7 @@ HttpResponse cb_delete_monitor(const HttpRequest& request)
         return HttpResponse(MHD_HTTP_NO_CONTENT);
     }
 
-    return HttpResponse(MHD_HTTP_BAD_REQUEST);
+    return HttpResponse(MHD_HTTP_FORBIDDEN);
 }
 
 HttpResponse cb_all_servers(const HttpRequest& request)
@@ -355,7 +356,7 @@ HttpResponse cb_maxscale(const HttpRequest& request)
 HttpResponse cb_logs(const HttpRequest& request)
 {
     // TODO: Show logs
-    return HttpResponse(MHD_HTTP_OK);
+    return HttpResponse(MHD_HTTP_OK, mxs_json_resource(request.host(), MXS_JSON_API_LOGS, json_null()));
 }
 
 HttpResponse cb_flush(const HttpRequest& request)
@@ -363,7 +364,7 @@ HttpResponse cb_flush(const HttpRequest& request)
     // Flush logs
     if (mxs_log_rotate() == 0)
     {
-        return HttpResponse(MHD_HTTP_OK);
+        return HttpResponse(MHD_HTTP_NO_CONTENT);
     }
     else
     {
@@ -374,13 +375,13 @@ HttpResponse cb_flush(const HttpRequest& request)
 HttpResponse cb_threads(const HttpRequest& request)
 {
     // TODO: Show thread status
-    return HttpResponse(MHD_HTTP_OK);
+    return HttpResponse(MHD_HTTP_OK, mxs_json_resource(request.host(), MXS_JSON_API_THREADS, json_null()));
 }
 
 HttpResponse cb_tasks(const HttpRequest& request)
 {
     // TODO: Show housekeeper tasks
-    return HttpResponse(MHD_HTTP_OK);
+    return HttpResponse(MHD_HTTP_OK, mxs_json_resource(request.host(), MXS_JSON_API_TASKS, json_null()));
 }
 
 HttpResponse cb_all_modules(const HttpRequest& request)
diff --git a/server/core/test/rest-api/test/schema_validation.js b/server/core/test/rest-api/test/schema_validation.js
index 8035d24f2..2131a35c5 100644
--- a/server/core/test/rest-api/test/schema_validation.js
+++ b/server/core/test/rest-api/test/schema_validation.js
@@ -35,10 +35,15 @@ describe("Individual Resources", function() {
         "/servers/server1",
         "/servers/server2",
         "/services/RW-Split-Router",
+        "/services/RW-Split-Router/listeners",
         "/monitors/MySQL-Monitor",
         "/filters/Hint",
         "/sessions/1",
         "/maxscale/",
+        "maxscale/threads",
+        "maxscale/logs",
+        "maxscale/tasks",
+        "maxscale/modules",
     ]
 
     tests.forEach(function(endpoint) {