nic-6443 commented on code in PR #13529:
URL: https://github.com/apache/apisix/pull/13529#discussion_r3397106716
##########
apisix/consumer.lua:
##########
@@ -279,6 +280,154 @@ function _M.find_consumer(plugin_name, key, key_value)
end
+-- The auth plugins below match the consumer by a unique key attribute, which
is
+-- the attribute that find_consumer() is called with at runtime. Duplicating
such
+-- keys across consumers/credentials makes the runtime matching ambiguous: the
+-- last loaded consumer silently wins.
+local plugin_unique_key_attrs = {
+ ["key-auth"] = "key",
+ ["basic-auth"] = "username",
+ ["jwt-auth"] = "key",
+ ["hmac-auth"] = "key_id",
+}
+
+
+local function get_auth_unique_keys(plugins_conf)
+ local keys
+ for plugin_name, plugin_conf in pairs(plugins_conf) do
+ local key_attr = plugin_unique_key_attrs[plugin_name]
+ if key_attr and type(plugin_conf) == "table" then
+ local key_value = plugin_conf[key_attr]
+ if type(key_value) == "string" then
+ if secret.is_secret_ref(key_value) then
+ core.log.info("skip duplicate check for the ", key_attr,
+ " of plugin ", plugin_name,
+ ": secret reference can not be resolved at
write time")
+ else
+ keys = keys or {}
+ keys[plugin_name] = key_value
+ end
+ end
+ end
+ end
+
+ return keys
+end
+
+
+local function get_stored_unique_key(stored_plugins, plugin_name, key_attr)
+ local stored_conf = stored_plugins[plugin_name]
+ if type(stored_conf) ~= "table" then
+ return nil
+ end
+
+ -- the stored conf may contain encrypted fields (data_encryption),
+ -- decrypt a copy of it like the Admin API GET handler does
+ local conf = core.table.deepcopy(stored_conf)
+ plugin.decrypt_conf(plugin_name, conf, core.schema.TYPE_CONSUMER)
+
+ local key_value = conf[key_attr]
+ if type(key_value) ~= "string" or secret.is_secret_ref(key_value) then
+ return nil
+ end
+
+ return key_value
+end
+
+
+-- Reject the write when an auth plugin key in plugins_conf is already used by
+-- another consumer or credential, since the runtime consumer matching can not
+-- distinguish them.
+-- When writing a consumer, consumer_name is its username and credential_id is
+-- nil: entries owned by the consumer itself are not treated as duplicates.
+-- When writing a credential, both consumer_name and credential_id are set:
+-- only the credential itself is skipped.
+function _M.check_duplicate_key(plugins_conf, consumer_name, credential_id)
+ if not plugins_conf then
+ return true
+ end
+
+ -- the credential id may come from the request payload as a number
+ if credential_id then
+ credential_id = tostring(credential_id)
+ end
+
+ local in_keys = get_auth_unique_keys(plugins_conf)
+ if not in_keys then
+ return true
+ end
+
+ local res, err = core.etcd.get("/consumers", true)
+ if not res then
+ return nil, "failed to list consumers: " .. err
+ end
+
+ if res.status == 404 then
+ return true
+ end
+
+ if res.status ~= 200 then
+ return nil, "failed to list consumers, response code: " .. res.status
+ end
+
+ -- admin_api_version v3 puts the entries in body.list,
+ -- while v2 keeps them in body.node.nodes
+ local items = res.body.list
+ if not items and res.body.node then
+ items = res.body.node.nodes
+ end
+
+ for _, item in ipairs(items or {}) do
+ if type(item) ~= "table" or type(item.value) ~= "table"
+ or type(item.value.plugins) ~= "table" then
+ goto CONTINUE
+ end
+
+ local item_consumer_name, item_credential_id
+ if is_credential_etcd_key(item.key) then
+ item_consumer_name =
get_consumer_name_from_credential_etcd_key(item.key)
+ item_credential_id = get_credential_id_from_etcd_key(item.key)
+ else
+ item_consumer_name = item.value.username
+ end
+
+ if credential_id then
+ -- a credential does not conflict with itself
+ if item_consumer_name == consumer_name
+ and item_credential_id == credential_id then
+ goto CONTINUE
+ end
+ else
+ -- a consumer does not conflict with itself or its own credentials
+ if item_consumer_name == consumer_name then
+ goto CONTINUE
+ end
+ end
Review Comment:
This exemption is deliberate (see the comment above check_duplicate_key): a
consumer PUT must resubmit its inline plugins, so rejecting collisions with its
own credentials would make any consumer that already carries both (e.g. data
created before this check) impossible to update at all, even for unrelated
fields. Both entries resolve to the same consumer identity at runtime, so the
blast radius is limited to which auth_conf copy wins — unlike cross-consumer
duplicates, where the identity itself becomes ambiguous. The reverse direction
(a new credential duplicating its own consumer's inline key) is rejected
because that write introduces the ambiguity and can simply omit the duplicate.
##########
apisix/consumer.lua:
##########
@@ -279,6 +280,154 @@ function _M.find_consumer(plugin_name, key, key_value)
end
+-- The auth plugins below match the consumer by a unique key attribute, which
is
+-- the attribute that find_consumer() is called with at runtime. Duplicating
such
+-- keys across consumers/credentials makes the runtime matching ambiguous: the
+-- last loaded consumer silently wins.
+local plugin_unique_key_attrs = {
+ ["key-auth"] = "key",
+ ["basic-auth"] = "username",
+ ["jwt-auth"] = "key",
+ ["hmac-auth"] = "key_id",
+}
+
+
+local function get_auth_unique_keys(plugins_conf)
+ local keys
+ for plugin_name, plugin_conf in pairs(plugins_conf) do
+ local key_attr = plugin_unique_key_attrs[plugin_name]
+ if key_attr and type(plugin_conf) == "table" then
+ local key_value = plugin_conf[key_attr]
+ if type(key_value) == "string" then
+ if secret.is_secret_ref(key_value) then
+ core.log.info("skip duplicate check for the ", key_attr,
+ " of plugin ", plugin_name,
+ ": secret reference can not be resolved at
write time")
+ else
Review Comment:
Fixed in 414b66988.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
