Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix/api] Prevent PATCH from overriding plugin's value #363

Merged
merged 1 commit into from
Jun 26, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions kong/api/crud_helpers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 22 additions & 0 deletions kong/dao/cassandra/base_dao.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions kong/dao/schemas_validation.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 46 additions & 5 deletions spec/integration/admin_api/apis_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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", {},
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down