Skip to content

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

Merged
nic-6443 merged 6 commits into
apache:masterfrom
nic-6443:fix/consumer-key-duplication-11197
Jul 17, 2026
Merged

fix(admin): duplicate consumer authentication keys are silently accepted#13529
nic-6443 merged 6 commits into
apache:masterfrom
nic-6443:fix/consumer-key-duplication-11197

Conversation

@nic-6443

@nic-6443 nic-6443 commented Jun 11, 2026

Copy link
Copy Markdown
Member

Description

The Admin API accepts two different consumers (or credentials) configured with the same authentication key, e.g. two consumers with an identical key-auth key. At runtime, consumer matching builds a key -> consumer map, so the last loaded consumer silently wins and traffic is attributed to an arbitrary one of them — identity, quotas and ACLs all follow the wrong consumer. The root cause is that there is no write-time validation of key uniqueness.

This PR revives the approach from #12040 (write-time checking, as endorsed in the issue discussion): on consumer and credential create/update, the Admin API rejects the write with 400 when the same unique key of an auth plugin (key-auth key, basic-auth username, jwt-auth key, hmac-auth key_id, ldap-auth user_dn) is already used by another owner. Re-PUT of the same owner with an unchanged key stays allowed.

The check runs against the locally watched /consumers data via consumer.find_consumer() — the same view the runtime uses for auth matching — so it answers exactly the question "would this key collide at runtime", and it costs no etcd read on the write path. Since the watcher decrypts plugin conf during config sync, the comparison also works when data_encryption is enabled.

Notes on scope:

  • The locally synced 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.
  • Key values that are secret references ($secret://, $env://) in the incoming conf are skipped, since they cannot be resolved at write time.
  • A credential duplicating a key that the consumer itself already uses in its inline plugin conf is also rejected (same as fix: consumer key duplication check #12040), because the two entries may carry different secrets/passwords under the same lookup key. One pre-existing test case in t/admin/credentials.t relied on such a duplicate and was adjusted accordingly.

Which issue(s) this PR fixes:

Fixes #11197

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@nic-6443
nic-6443 marked this pull request as ready for review June 11, 2026 13:21
Copilot AI review requested due to automatic review settings June 11, 2026 13:21
@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. bug Something isn't working labels Jun 11, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds Admin API write-time validation to prevent duplicate unique-key attributes used by auth plugins (e.g., key-auth.key, basic-auth.username) across consumers and credentials, avoiding ambiguous runtime consumer matching where the “last loaded wins”.

Changes:

  • Add etcd-backed duplicate-key detection (with data_encryption decrypt support and secret-ref skip behavior) for consumer/credential writes.
  • Pass sub_path through the admin resource checker pipeline so credential writes can determine owning consumer.
  • Extend Admin API tests to cover duplicate rejection cases (including encrypted stored configs).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
apisix/consumer.lua Implements duplicate-key scan by reading /consumers from etcd and comparing decrypted stored plugin conf values.
apisix/admin/resource.lua Threads sub_path into checker options so sub-resources (credentials) can validate with owner context.
apisix/admin/credentials.lua Runs duplicate-key validation on credential PUT using sub_path to extract consumer name.
apisix/admin/consumers.lua Runs duplicate-key validation on consumer PUT.
t/admin/credentials.t Adjusts an existing basic-auth username and adds credential-focused duplicate-key test coverage.
t/admin/consumers.t Adds consumer-focused duplicate-key tests, including encrypted storage scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apisix/consumer.lua Outdated
Comment thread apisix/consumer.lua
Comment thread apisix/consumer.lua
@nic-6443 nic-6443 assigned nic-6443 and unassigned nic-6443 Jul 13, 2026
nic-6443 added 3 commits July 16, 2026 10:32
skip_references_check guards the checks that require the referenced
resource to exist (consumer group, upstream, etc.), which must be
skipped when validating a config bundle whose references may live in
the same payload. The duplicate key check is not a reference check,
so it should not be gated by this flag.
ldap-auth also matches the consumer by a unique key attribute at
runtime (user_dn), so duplicating it across consumers has the same
last-loaded-wins ambiguity.

Also make the duplicate-check tests wait deterministically until the
local consumer watcher has synced the previous write (polling
find_consumer instead of relying on restart timing or a fixed sleep),
since the check runs against the locally synced consumer data.
@nic-6443
nic-6443 merged commit 72b2475 into apache:master Jul 17, 2026
20 checks passed
@nic-6443
nic-6443 deleted the fix/consumer-key-duplication-11197 branch July 17, 2026 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API KEY Unique

5 participants