fix(api) Prevent PATCH from overriding plugin's value

Since plugin_configuration's `value` is stored as plain text in
Cassandra, if one made a PATCH request it could potentially override the
stored value because the DAO treated the field just like any other.

Instead, we need to set each property of a `value` that is not updated
by the PATCH before updating the row.

That behaviour doesn't apply to PUT requests.

kong/api/crud_helpers.lua

@@ -106,8 +106,8 @@ function _M.post(params, dao_collection, success)
   end
 end
 
-function _M.patch(new_entity, old_entity, dao_collection)
-  for k, v in pairs(new_entity) do
+function _M.patch(params, old_entity, dao_collection)
+  for k, v in pairs(params) do
     old_entity[k] = v
   end
 

kong/dao/cassandra/base_dao.lua

@@ -411,6 +411,24 @@ local function extract_primary_key(t, primary_key, clustering_key)
   return t_primary_key, t_no_primary_key
 end
 
+-- When updating a row that has a json-as-text column (ex: plugin_configuration.value),
+-- we want to avoid overriding it with a partial value.
+-- Ex: value.key_name + value.hide_credential, if we update only one field,
+-- the other should be preserved. Of course this only applies in partial update.
+local function fix_tables(t, old_t, schema)
+  for k, v in pairs(schema.fields) do
+    if v.schema then
+      local s = type(v.schema) == "function" and v.schema(t) or v.schema
+      for s_k, s_v in pairs(s.fields) do
+        if not t[k][s_k] and old_t[k] then
+          t[k][s_k] = old_t[k][s_k]
+        end
+      end
+      fix_tables(t[k], old_t[k], s)
+    end
+  end
+end
+
 -- Update a row: find the row with the given PRIMARY KEY and update the other values
 -- If `full`, sets to NULL values that are not included in the schema.
 -- Performs schema validation, UNIQUE and FOREIGN checks.
@@ -432,6 +450,10 @@ function BaseDao:update(t, full)
     return false
   end
 
+  if not full then
+    fix_tables(t, res, self._schema)
+  end
+
   -- Validate schema
   errors = validations.validate(t, self, {partial_update = not full, full_update = full})
   if errors then

kong/dao/schemas_validation.lua

@@ -132,8 +132,8 @@ function _M.validate_fields(t, schema, options)
 
       if sub_schema then
         -- Check for sub-schema defaults and required properties in advance
-        for sub_field_k, sub_field in pairs(sub_schema.fields) do
-          if t[column] == nil then
+        if t[column] == nil then
+          for sub_field_k, sub_field in pairs(sub_schema.fields) do
             if sub_field.default ~= nil then -- Sub-value has a default, be polite and pre-assign the sub-value
               t[column] = {}
             elseif sub_field.required then -- Only check required if field doesn't have a default

spec/integration/admin_api/apis_routes_spec.lua

@@ -249,6 +249,7 @@ describe("Admin API", function()
       end)
 
       describe("PUT", function()
+        local plugin_id
 
         it("[FAILURE] should return proper errors", function()
           send_content_types(BASE_URL, "PUT", {},
@@ -268,21 +269,44 @@ describe("Admin API", function()
 
           response, status = http_client.put(BASE_URL, {
             name = "keyauth",
-            value = {key_names={"apikey"}}
-          }, {["content-type"]="application/json"})
+            value = {key_names = {"apikey"}}
+          }, {["content-type"] = "application/json"})
           assert.equal(201, status)
           body = json.decode(response)
 
+          plugin_id = body.id
+
           response, status = http_client.put(BASE_URL, {
-            id = body.id,
+            id = plugin_id,
             name = "keyauth",
-            value = {key_names={"updated_apikey"}}
-          }, {["content-type"]="application/json"})
+            value = {key_names = {"updated_apikey"}}
+          }, {["content-type"] = "application/json"})
           assert.equal(200, status)
           body = json.decode(response)
           assert.equal("updated_apikey", body.value.key_names[1])
         end)
 
+        it("should override a plugin's `value` if partial", function()
+          local response, status = http_client.put(BASE_URL, {
+            id = plugin_id,
+            name = "keyauth",
+            ["value.key_names"] = {"api_key"},
+            ["value.hide_credentials"] = true
+          })
+          assert.equal(200, status)
+          assert.truthy(response)
+
+          response, status = http_client.put(BASE_URL, {
+            id = plugin_id,
+            name = "keyauth",
+            ["value.key_names"] = {"api_key_updated"}
+          })
+          assert.equal(200, status)
+          local body = json.decode(response)
+          assert.same({"api_key_updated"}, body.value.key_names)
+          assert.falsy(body.hide_credentials)
+        end)
+
       end)
 
       describe("GET", function()
@@ -349,6 +373,23 @@ describe("Admin API", function()
             assert.equal(404, status)
           end)
 
+          it("should not override a plugin's `value` if partial", function()
+            -- This is delicate since a plugin's `value` is a text field in a DB like Cassandra
+            local response, status = http_client.patch(BASE_URL..plugin.name, {
+              ["value.key_names"] = {"key_set_null_test"},
+              ["value.hide_credentials"] = true
+            })
+            assert.equal(200, status)
+
+            response, status = http_client.patch(BASE_URL..plugin.name, {
+              ["value.key_names"] = {"key_set_null_test_updated"}
+            })
+            assert.equal(200, status)
+            local body = json.decode(response)
+            assert.same({"key_set_null_test_updated"}, body.value.key_names)
+            assert.equal(true, body.value.hide_credentials)
+          end)
+
         end)
 
         describe("DELETE", function()