feat: add ldap-auth-advanced plugin (core authentication) - #13762
feat: add ldap-auth-advanced plugin (core authentication)#13762janiussyafiq wants to merge 3 commits into
Conversation
Add the ldap-auth-advanced plugin (priority 2541), Kong ldap-auth-advanced parity, core authentication: search-then-bind (AD sAMAccountName shape), service-account or anonymous search bind, user_dn Consumer association, LDAPS/StartTLS, LDAP filter-injection defense, strict 401-vs-500 auth/transport error split, multi-auth compatibility. Bump lua-resty-ldap 0.1.0 -> 0.3.0 (pure-Lua client/protocol/filter; the released client is lazy-connect, so the plugin binds first and classifies a (nil, err) return as a transport failure). Note: under 0.1.0 the ldap-auth plugin's tls_verify was a silent no-op (the library read verify_ldap_host); 0.3.0 fixes the field name, so tls_verify:true configs begin real certificate verification. Also regenerate t/certs/localhost_slapd_cert.pem: the old cert has CN=test.com but no subjectAltName, and nginx's hostname verification does not fall back to CN, so any real tls_verify handshake against it fails with "certificate host mismatch". The new cert (same key, same test CA) adds SAN DNS:test.com, DNS:localhost, IP:127.0.0.1. Group collection/authorization and credential caching/anonymous-consumer follow in separate PRs; docs follow up separately.
dosubot
Bot
added
size:XXL
| -- Proxy-Authorization is checked before Authorization | ||
| local header_name = "Proxy-Authorization" | ||
| local auth_header = core.request.header(ctx, header_name) | ||
| if not auth_header then |
There was a problem hiding this comment.
P2: Please fall back to Authorization when Proxy-Authorization is present but cannot produce credentials for conf.header_type. A forward proxy can send its own Proxy-Authorization: Basic ... while forwarding a valid end-user Authorization: ldap ...; currently the mere presence of the proxy header makes the valid Authorization unreachable and returns 401. Proxy credentials should take priority only after scheme, base64, and separator parsing succeeds.
There was a problem hiding this comment.
Thanks, scheme mismatches and decode failures now fall back correctly. One unusable case still bypasses the fallback: Lua treats "" as truthy, so Proxy-Authorization: ldap OnBhc3M= (:pass) or ldap dXNlcjo= (user:) enters this branch; the later empty-field check returns 401 without trying a valid Authorization. Please reject empty usernames/passwords before granting Proxy priority and cover both cases.
- strip client-supplied X-Consumer-Username/X-Consumer-Custom-ID/ X-Credential-Identifier (with X-Authenticated-Groups) before any auth work, so consumer_required=false cannot pass spoofed identity upstream - accept RFC 4512 numeric-OID attribute types and tighten ;option validation in the attribute schema pattern - fall back to Authorization when Proxy-Authorization is present but does not parse into credentials for header_type - classify directory failures by operation and result code: 401 only for invalidCredentials on the user bind plus the not-found/ambiguous cases (including sizeLimitExceeded, the >size_limit ambiguity); service-bind rejections and search operational failures are 500 - look up Consumers via consumer.find_consumer() so a secret-ref user_dn resolves (and unresolved references fail closed) - register ldap-auth-advanced user_dn in plugin_unique_key_attrs so the Admin API rejects duplicate user_dn values
| title = "work with route or service object", | ||
| properties = { | ||
| -- connection | ||
| ldap_uri = { type = "string" }, -- "host[:port]" |
There was a problem hiding this comment.
can we add min and max lengths here? (and other relevant fields)?
There was a problem hiding this comment.
Pull request overview
This PR introduces a new core authentication plugin, ldap-auth-advanced, implementing search-then-bind LDAP authentication (including LDAPS/StartTLS, Consumer user_dn mapping, and filter-injection defenses) and updates the CI LDAP fixtures and plugin registration so it’s available and testable within APISIX.
Changes:
- Added
apisix/plugins/ldap-auth-advanced.luaplus a comprehensive LDAP integration test suite (t/plugin/ldap-auth-advanced.t). - Registered the plugin across admin/plugin listing and default config examples, and enforced Consumer unique-key duplication checks for
user_dn. - Updated the OpenLDAP CI container setup (bootstrap LDIF + ACL hook) and bumped
lua-resty-ldapto0.3.0.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
apisix/plugins/ldap-auth-advanced.lua |
New advanced LDAP auth plugin implementation (search-then-bind, TLS options, Consumer association). |
t/plugin/ldap-auth-advanced.t |
New end-to-end test coverage for the plugin, including injection defenses and multi-auth behavior. |
apisix/consumer.lua |
Adds ldap-auth-advanced to the auth-plugin unique key map for Consumer duplicate checks. |
apisix/cli/config.lua |
Registers ldap-auth-advanced in the built-in plugin list. |
t/admin/plugins.t |
Extends admin plugin list/priority assertions to include the new plugin. |
t/admin/consumers.t |
Adds Consumer duplicate user_dn coverage for ldap-auth-advanced. |
conf/config.yaml.example |
Documents the plugin in the example plugin list with correct priority ordering. |
apisix-master-0.rockspec |
Bumps lua-resty-ldap dependency to 0.3.0-0. |
ci/pod/docker-compose.plugin.yml |
Updates OpenLDAP service config and mounts new bootstrap fixtures/hooks. |
ci/pod/openldap/ad.ldif |
Provides a deterministic LDAP directory tree for CI (including advanced-plugin fixtures). |
ci/pod/openldap/enable-anon-bind.sh |
Configures OpenLDAP to allow unauthenticated bind and installs ACLs used by tests. |
t/certs/localhost_slapd_cert.pem |
Updates test cert material (notably SANs) to support hostname verification paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -- the filter so any reject is a clean 401. | ||
| local search_filter = "(" .. conf.attribute .. "=" | ||
| .. ldap_filter.escape(username) .. ")" | ||
| if not ldap_filter.compile(search_filter) then |
There was a problem hiding this comment.
P2: This preflight rejects a valid RFC 4515 assertion value. ~ (0x7e) belongs to UTF1SUBSET (%x5D-7F), so admin~ is a legal username, but the bundled filter grammar rejects it at the end and TEST 46 now codifies a 401. Please fix/upgrade the parser or escape ~ as \\7e before compiling so valid directory users remain authenticatable.
| "xml2lua = 1.6-2", | ||
| "lua-resty-mediador = 0.1.2-1", | ||
| "lua-resty-ldap = 0.1.0-0", | ||
| "lua-resty-ldap = 0.3.0-0", |
There was a problem hiding this comment.
P2: lua-resty-ldap 0.3.0-0 pins lpeg = 1.0.2-1. In a clean APISIX dependency tree, applying this bump removes the currently resolved lpeg 1.1.0-2 and installs 1.0.2; graphql and jsonpath then also run against the downgraded global module. Please publish/use an LDAP rock with a compatible range such as lpeg >= 1.0.2, so APISIX retains its current LPeg version.
Description
Adds the
ldap-auth-advancedplugin (priority 2541), bringing Kongldap-auth-advancedparity for core authentication: search-then-bind (ADsAMAccountNameshape), service-account or anonymous search bind,user_dnConsumer association, LDAPS/StartTLS, LDAP filter-injection defense, and a strict 401-vs-500 auth/transport error split. This is part 1 of 3 (core authentication); group collection/authorization and credential caching/anonymous-consumer follow in separate PRs. Prior discussion in #8958.This PR also bumps
lua-resty-ldap0.1.0 -> 0.3.0. Under 0.1.0, the existingldap-authplugin'stls_verifyoption was a silent no-op (the library read a different field name); 0.3.0 fixes this, sotls_verify: trueconfigs now perform real certificate verification. This also required regeneratingt/certs/localhost_slapd_cert.pem(same key, same test CA) to add asubjectAltName(DNS:test.com, DNS:localhost, IP:127.0.0.1) — the old cert hadCN=test.combut no SAN, and nginx's hostname verification does not fall back to CN, so the newly-realldap-authTLS-verify test started failing a certificate host-mismatch check that the 0.1.0 bug had been silently masking.Which issue(s) this PR fixes:
Related: #8958
Checklist
lua-resty-ldapbump activates realtls_verifycertificate verification, previously a silent no-op under 0.1.0 -- see description)