feat(ldap-auth): add hide_credentials - #13832
Conversation
The plugin decodes the LDAP username and password from the Authorization header and, after a successful bind, forwards that header to the upstream untouched. Directory credentials are usually reusable well beyond the API being called, and the upstream may be a low trust or multi tenant service. Add the `hide_credentials` option the other auth plugins already have (`basic-auth`, `jwt-auth`, `key-auth`, `hmac-auth`), defaulting to false to keep the current behaviour.
dosubot
Bot
added
size:L
There was a problem hiding this comment.
Pull request overview
Adds a hide_credentials option to the ldap-auth plugin so operators can prevent forwarding the incoming Authorization header (which contains reusable LDAP credentials) to upstream services, while keeping default behavior unchanged for backward compatibility.
Changes:
- Add
hide_credentials(defaultfalse) toapisix/plugins/ldap-auth.luaand stripAuthorizationwhen enabled. - Add regression tests validating both the default forwarding behavior and the hiding behavior.
- Document the new attribute in both English and Chinese plugin docs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apisix/plugins/ldap-auth.lua | Adds hide_credentials config and removes Authorization header when enabled. |
| t/plugin/ldap-auth.t | Adds tests to verify Authorization is forwarded by default and removed when hide_credentials=true. |
| docs/en/latest/plugins/ldap-auth.md | Documents the new hide_credentials attribute. |
| docs/zh/latest/plugins/ldap-auth.md | Documents the new hide_credentials attribute in Chinese. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| core.request.set_header(ctx, "Authorization", nil) | ||
| end | ||
|
|
||
| core.log.info("hit basic-auth access") |
There was a problem hiding this comment.
Agreed, that line predates this PR but it is right below the code I touched, so fixed in eadc0da: it logs hit ldap-auth access now.
There was a problem hiding this comment.
Split off would have meant a second PR for a five-word log line, so I mentioned it in the description instead — it is its own commit if you would rather it went elsewhere.
Description
ldap-authdecodes the username and password from theAuthorizationheader, binds to the directory with them, and then forwards that header to the upstream as it arrived. LDAP credentials are usually the organisation-wide ones and reusable well beyond the API being called, so handing them to every upstream behind the gateway is worth being able to turn off —basic-auth,jwt-auth,key-authandhmac-authall havehide_credentialsfor exactly this.This adds the same option to
ldap-auth, defaulting tofalseso existing routes behave as before.One unrelated line rides along in its own commit: the success path logged
hit basic-auth access, a copy-paste frombasic-auth, and it sits right below the code this PR touches. Happy to drop it if you'd rather keep the PR to one thing.Which issue(s) this PR fixes:
N/A
Checklist