fix(admin) disable retrieval of plugins by name (#2726)

In the `/apis/:api/plugins/:plugin` endpoint, #2252 recently introduced
the possibility to pass a plugin name as `:plugin`, instead of an id
only. Like: `/apis/:api/plugins/basic-auth`.

However, the following minimally reproducible example exposes the flaw
with such an endpoint given the current model:

```
$ http POST :8001/apis/ name=api1 hosts=example.com upstream_url=http://httpbin.org
$ http POST :8001/consumers name=peter
$ http POST :8001/consumers name=mary
$ http POST :8001/plugins name=request-size-limiting api_id=$API1_UUID consumer_id=$PETER_UUID
$ http POST :8001/plugins name=request-size-limiting api_id=$API1_UUID consumer_id=$MARY_UUID
$ http :8001/apis/$API1_UUID/plugins
$ http :8001/apis/$API1_UUID/plugins/request-size-limiting
```

Reverts the main part of #2252

Signed-off-by: Thibault Charbonnier <[email protected]>

CHANGELOG.md

@@ -41,9 +41,6 @@
   - rate-limiting/response-ratelimiting: Optionally hide informative response
     headers.
     [#2087](https://github.com/Mashape/kong/pull/2087)
-  - The endpoint `/apis/:api_name_or_id/plugins/:plugin_name_or_id` now accepts
-    the plugin name as well for the last parameter.
-    [#2252](https://github.com/Mashape/kong/pull/2252)
 
 ## [0.10.3] - 2017/05/24
 

kong/api/crud_helpers.lua

@@ -52,20 +52,13 @@ function _M.find_api_by_name_or_id(self, dao_factory, helpers)
   end
 end
 
--- this function will lookup a plugin by name or id, but REQUIRES
--- also the api to be specified by name or id
-function _M.find_plugin_by_name_or_id(self, dao_factory, helpers)
-  _M.find_api_by_name_or_id(self, dao_factory, helpers)
-
-  local rows, err = _M.find_by_id_or_field(dao_factory.plugins, { api_id = self.api.id },
-                                           self.params.plugin_name_or_id, "name")
-
+function _M.find_plugin_by_filter(self, dao_factory, filter, helpers)
+  local rows, err = dao_factory.plugins:find_all(filter)
   if err then
     return helpers.yield_error(err)
   end
-  self.params.plugin_name_or_id = nil
 
-  -- We know combi of api+plugin is unique for plugins, hence if we have a row, it must be the only one
+  -- We know the id is unique, so if we have a row, it must be the only one
   self.plugin = rows[1]
   if not self.plugin then
     return helpers.responses.send_HTTP_NOT_FOUND()

kong/api/routes/apis.lua

@@ -55,9 +55,13 @@ return {
     end
   },
 
-  ["/apis/:api_name_or_id/plugins/:plugin_name_or_id"] = {
+  ["/apis/:api_name_or_id/plugins/:id"] = {
     before = function(self, dao_factory, helpers)
-      crud.find_plugin_by_name_or_id(self, dao_factory, helpers)
+      crud.find_api_by_name_or_id(self, dao_factory, helpers)
+      crud.find_plugin_by_filter(self, dao_factory, {
+        api_id = self.api.id,
+        id     = self.params.id,
+      }, helpers)
     end,
 
     GET = function(self, dao_factory, helpers)

kong/api/routes/plugins.lua

@@ -49,26 +49,21 @@ return {
 
   ["/plugins/:id"] = {
     before = function(self, dao_factory, helpers)
-      local rows, err = dao_factory.plugins:find_all {id = self.params.id}
-      if err then
-        return helpers.yield_error(err)
-      elseif #rows == 0 then
-        return helpers.responses.send_HTTP_NOT_FOUND()
-      end
-
-      self.plugin_conf = rows[1]
+      crud.find_plugin_by_filter(self, dao_factory, {
+        id = self.params.id
+      }, helpers)
     end,
 
     GET = function(self, dao_factory, helpers)
-      return helpers.responses.send_HTTP_OK(self.plugin_conf)
+      return helpers.responses.send_HTTP_OK(self.plugin)
     end,
 
     PATCH = function(self, dao_factory)
-      crud.patch(self.params, dao_factory.plugins, self.plugin_conf)
+      crud.patch(self.params, dao_factory.plugins, self.plugin)
     end,
 
     DELETE = function(self, dao_factory)
-      crud.delete(self.plugin_conf, dao_factory.plugins)
+      crud.delete(self.plugin, dao_factory.plugins)
     end
   },
 

spec/02-integration/04-admin_api/02-apis_routes_spec.lua

@@ -983,15 +983,6 @@ describe("Admin API " .. kong_config.database, function()
           local json = cjson.decode(body)
           assert.same(plugin, json)
         end)
-        it("retrieves by plugin name", function()
-          local res = assert(client:send {
-            method = "GET",
-            path = "/apis/"..api.name.."/plugins/"..plugin.name
-          })
-          local body = assert.res_status(200, res)
-          local json = cjson.decode(body)
-          assert.same(plugin, json)
-        end)
         it("only retrieves if associated to the correct API", function()
           -- Create an API and try to query our plugin through it
           local w_api = assert(dao.apis:insert {

spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua

@@ -417,6 +417,7 @@ describe("core entities are invalidated with db: " .. kong_conf.database, functi
   ----------
 
   describe("plugins (per API)", function()
+    local api_plugin_id
 
     it("on create", function()
       -- create API
@@ -475,7 +476,9 @@ describe("core entities are invalidated with db: " .. kong_conf.database, functi
           ["Content-Type"] = "application/json",
         },
       })
-      assert.res_status(201, admin_res_plugin)
+      local body = assert.res_status(201, admin_res_plugin)
+      local plugin = cjson.decode(body)
+      api_plugin_id = assert(plugin.id, "could not get plugin id from " .. body)
 
       -- no need to wait for workers propagation (lua-resty-worker-events)
       -- because our test instance only has 1 worker
@@ -506,7 +509,7 @@ describe("core entities are invalidated with db: " .. kong_conf.database, functi
     it("on update", function()
       local admin_res_plugin = assert(admin_client_1:send {
         method = "PATCH",
-        path   = "/apis/dummy-api/plugins/dummy",
+        path   = "/apis/dummy-api/plugins/" .. api_plugin_id,
         body   = {
           ["config.resp_header_value"] = "2",
         },
@@ -545,7 +548,7 @@ describe("core entities are invalidated with db: " .. kong_conf.database, functi
     it("on delete", function()
       local admin_res_plugin = assert(admin_client_1:send {
         method = "DELETE",
-        path   = "/apis/dummy-api/plugins/dummy",
+        path   = "/apis/dummy-api/plugins/" .. api_plugin_id,
       })
       assert.res_status(204, admin_res_plugin)