fix(admin): duplicate consumer authentication keys are silently accepted

apisix/admin/consumers.lua

@@ -18,6 +18,7 @@ local core    = require("apisix.core")
 local plugins = require("apisix.admin.plugins")
 local plugins_encrypt_conf = require("apisix.admin.plugins").encrypt_conf
 local resource = require("apisix.admin.resource")
+local apisix_consumer = require("apisix.consumer")
 
 
 local function check_conf(username, conf, need_username, schema, opts)
@@ -36,6 +37,13 @@ local function check_conf(username, conf, need_username, schema, opts)
         if not ok then
             return nil, {error_msg = "invalid plugins configuration: " .. err}
         end
+
+        if not opts.skip_references_check then
+            ok, err = apisix_consumer.check_duplicate_key(conf.plugins, conf.username)
+            if not ok then
+                return nil, {error_msg = err}
+            end
+        end
     end
 
     if conf.group_id and not opts.skip_references_check then

apisix/admin/credentials.lua

@@ -18,10 +18,12 @@ local core     = require("apisix.core")
 local plugins  = require("apisix.admin.plugins")
 local plugin   = require("apisix.plugin")
 local resource = require("apisix.admin.resource")
+local apisix_consumer = require("apisix.consumer")
 local plugins_encrypt_conf = require("apisix.admin.plugins").encrypt_conf
 local pairs    = pairs
 
-local function check_conf(_id, conf, _need_id, schema)
+local function check_conf(id, conf, _need_id, schema, opts)
+    opts = opts or {}
     local ok, err = core.schema.check(schema, conf)
     if not ok then
         return nil, {error_msg = "invalid configuration: " .. err}
@@ -42,6 +44,16 @@ local function check_conf(_id, conf, _need_id, schema)
                 return nil, {error_msg = "only supports auth type plugins in consumer credential"}
             end
         end
+
+        -- opts.sub_path is in the form of {consumer_name}/credentials or
+        -- {consumer_name}/credentials/{credential_id}
+        if not opts.skip_references_check and opts.sub_path then
+            local consumer_name = core.utils.split_uri(opts.sub_path)[1]
+            ok, err = apisix_consumer.check_duplicate_key(conf.plugins, consumer_name, id)
+            if not ok then
+                return nil, {error_msg = err}
+            end
+        end
     end
 
     return true, nil

apisix/admin/resource.lua

@@ -81,7 +81,7 @@ local function check_forbidden_properties(conf, forbidden_properties)
 end
 
 
-function _M:check_conf(id, conf, need_id, typ, allow_time)
+function _M:check_conf(id, conf, need_id, typ, allow_time, sub_path)
     if self.name == "secrets" then
         id = typ .. "/" .. id
     end
@@ -123,7 +123,8 @@ function _M:check_conf(id, conf, need_id, typ, allow_time)
     end
 
     local conf_for_check = tbl_deepcopy(conf)
-    local ok, err = self.checker(id, conf_for_check, need_id, self.schema, {secret_type = typ})
+    local ok, err = self.checker(id, conf_for_check, need_id, self.schema,
+                                 {secret_type = typ, sub_path = sub_path})
 
     if self.encrypt_conf then
         self.encrypt_conf(id, conf)
@@ -253,7 +254,7 @@ function _M:put(id, conf, sub_path, args)
     end
 
     local need_id = not no_id_res[self.name]
-    local ok, err = self:check_conf(id, conf, need_id, typ)
+    local ok, err = self:check_conf(id, conf, need_id, typ, nil, sub_path)
     if not ok then
         return 400, err
     end

apisix/consumer.lua

@@ -24,6 +24,7 @@ local error          = error
 local ipairs         = ipairs
 local pairs          = pairs
 local type           = type
+local tostring       = tostring
 local string_sub     = string.sub
 local consumers
 
@@ -279,6 +280,113 @@ 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 cannot be resolved at write time")
+                else
+                    keys = keys or {}
+                    keys[plugin_name] = key_value
+                end
+            end
+        end
+    end
+
+    return keys
+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.
+--
+-- The lookup goes through find_consumer(), i.e. the consumer data that this
+-- process has already watched and synced from etcd -- the same view the
+-- runtime uses for auth matching -- instead of pulling the full /consumers
+-- range from etcd on every admin write. The tradeoff is that the local view
+-- may lag the latest writes by one sync cycle, so a duplicate written moments
+-- earlier can slip through under rapid successive or concurrent writes. This
+-- is accepted: the check is best-effort protection against misconfiguration,
+-- and the authoritative behavior at runtime remains last-loaded-wins.
+--
+-- When writing a consumer, consumer_name is its username and credential_id is
+-- nil: a key owned by the consumer itself (inline or by its credentials) is
+-- not treated as a duplicate. 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 /consumers watcher may not be initialized yet, e.g. the write
+    -- arrives right after the worker starts: degrade to allow, consistent
+    -- with the best-effort nature of this check
+    if not consumers then
+        return true
+    end
+
+    local in_keys = get_auth_unique_keys(plugins_conf)
+    if not in_keys then
+        return true
+    end
+
+    -- the credential id may come from the request payload as a number, while
+    -- the cached credential_id is taken from the etcd key as a string
+    if credential_id then
+        credential_id = tostring(credential_id)
+    end
+
+    for plugin_name, key_value in pairs(in_keys) do
+        local key_attr = plugin_unique_key_attrs[plugin_name]
+        local owner = _M.find_consumer(plugin_name, key_attr, key_value)
+        if owner then
+            local is_self
+            if credential_id then
+                -- a credential does not conflict with itself
+                is_self = owner.consumer_name == consumer_name
+                          and owner.credential_id == credential_id
+            else
+                -- a consumer does not conflict with itself or its own credentials
+                is_self = owner.consumer_name == consumer_name
+            end
+
+            if not is_self then
+                local owner_desc
+                if owner.credential_id then
+                    owner_desc = "credential: " .. owner.credential_id ..
+                                 " of consumer: " .. owner.consumer_name
+                else
+                    owner_desc = "consumer: " .. owner.consumer_name
+                end
+                return nil, "duplicate " .. key_attr .. " of plugin " .. plugin_name
+                            .. " found with " .. owner_desc
+            end
+        end
+    end
+
+    return true
+end
+
+
 local function check_consumer(consumer, key)
     local data_valid
     local err