Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ldap-auth): use lua-resty-ldap instead of lualdap #7590

Merged
merged 6 commits into from
Aug 4, 2022
Merged

feat(ldap-auth): use lua-resty-ldap instead of lualdap #7590

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
58ceb28
feat(ldap-auth): use lua-resty-ldap instead of lualdap
kingluo Aug 2, 2022
8a3ca1e
fix spaces in zh doc
kingluo Aug 2, 2022
b60a68c
fix format of t/certs/localhost_slapd_key.pem
kingluo Aug 2, 2022
52b4ee9
fix format of ci/pod/docker-compose.plugin.yml
kingluo Aug 2, 2022
ef84a13
make it compatible with old config
kingluo Aug 2, 2022
dd4e2a3
fix default values and doc
kingluo Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions apisix/plugins/ldap-auth.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,12 @@ local schema = {
title = "work with route or service object",
properties = {
base_dn = { type = "string" },
ldap_host = { type = "string" },
ldap_port = { type = "number" },
ldap_uri = { type = "string" },
use_tls = { type = "boolean" },
verify_ldap_host = { type = "boolean" },
uid = { type = "string" }
},
required = {"base_dn","ldap_host","ldap_port"},
required = {"base_dn","ldap_uri"},
}

local consumer_schema = {
Expand Down Expand Up @@ -140,12 +139,14 @@ function _M.rewrite(conf, ctx)
-- 2. try authenticate the user against the ldap server
local uid = conf.uid or "cn"

local ldap_host, ldap_port = core.utils.parse_addr(conf.ldap_uri)

local userdn = uid .. "=" .. user.username .. "," .. conf.base_dn
local ldapconf = {
timeout = 10000,
start_tls = false,
ldap_host = conf.ldap_host,
ldap_port = conf.ldap_port,
ldap_host = ldap_host,
ldap_port = ldap_port or 389,
ldaps = conf.use_tls,
verify_ldap_host = conf.verify_ldap_host,
base_dn = conf.base_dn,
Expand All @@ -154,7 +155,7 @@ function _M.rewrite(conf, ctx)
}
local res, err = ldap.ldap_authenticate(user.username, user.password, ldapconf)
if not res then
core.log.error(err)
core.log.warn("ldap-auth: ", err)
Copy link
Member

Choose a reason for hiding this comment

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

The prefix should be more meaningful

return 401, { message = "Invalid user authorization" }
end

Expand Down
10 changes: 4 additions & 6 deletions docs/en/latest/plugins/ldap-auth.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ For Route:
| Name | Type | Required | Default | Description |
|----------|---------|----------|---------|------------------------------------------------------------------------|
| base_dn | string | True | | Base dn of the LDAP server. For example, `ou=users,dc=example,dc=org`. |
| ldap_host| string | True | | host of the LDAP server. |
| ldap_port| number | True | | port of the LDAP server. |
| use_tls | boolean | False | `false` | If set to `true` uses TLS. |
| verify_ldap_host| boolean | False | `false` | Whether to verify the server certificate when `use_tls` is enabled; If set to `true`, you must set `ssl_trusted_certificate` in `config.yaml`, and make sure the `ldap_host` matches the host in server certificate. |
| ldap_uri | string | True | | URI of the LDAP server. |
| use_tls | boolean | False | `false` | If set to `true` uses TLS. |
| verify_ldap_host| boolean | False | `false` | Whether to verify the server certificate when `use_tls` is enabled; If set to `true`, you must set `ssl_trusted_certificate` in `config.yaml`, and make sure the host of `ldap_uri` matches the host in server certificate. |
Copy link
Member

Choose a reason for hiding this comment

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

This field doesn't have default value in the code. Why add a default value in the doc?

Copy link
Member

Choose a reason for hiding this comment

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

According to the description, we should name this field tls_verify? It doesn't verify the host but the TLS relative stuff.

| uid | string | False | `cn` | uid attribute. |

## Enabling the plugin
Expand Down Expand Up @@ -80,8 +79,7 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13
"plugins": {
"ldap-auth": {
"base_dn": "ou=users,dc=example,dc=org",
"ldap_host": "localhost",
"ldap_port": "1389",
"ldap_uri": "localhost:1389",
"uid": "cn"
},
},
Expand Down
8 changes: 3 additions & 5 deletions docs/zh/latest/plugins/ldap-auth.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,9 @@ Route 端：
| 名称 | 类型 | 必选项 | 默认值 | 描述 |
|----------|---------|----------|---------|------------------------------------------------------------------------|
| base_dn | string | 是 | | LDAP 服务器的 dn，例如：`ou=users,dc=example,dc=org`。|
| ldap_host| string | 是 | | LDAP 服务器的主机名。 |
| ldap_port| number | 是 | | LDAP 服务器的端口号。 |
| ldap_uri | string | 是 | | LDAP 服务器的 URI。 |
| use_tls | boolean | 否 | false | 如果设置为 `true` 则表示启用 TLS。 |
| verify_ldap_host| boolean | 否 | false | 是否校验 LDAP 服务器的证书。如果设置为 `true`，你必须设置 `config.yaml` 里面的 `ssl_trusted_certificate`，并且确保 `ldap_host` 和服务器证书中的 host 匹配。 |
| verify_ldap_host| boolean | 否 | false | 是否校验 LDAP 服务器的证书。如果设置为 `true`，你必须设置 `config.yaml` 里面的 `ssl_trusted_certificate`，并且确保 `ldap_uri` 里的 host 和服务器证书中的 host 匹配。 |
| uid | string | 否 | cn | UID 属性。 |

## 启用插件
Expand Down Expand Up @@ -78,8 +77,7 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13
"plugins": {
"ldap-auth": {
"base_dn": "ou=users,dc=example,dc=org",
"ldap_host": "localhost",
"ldap_port": "1389",
"ldap_uri": "localhost:1389",
"uid": "cn"
},
},
Expand Down
18 changes: 7 additions & 11 deletions t/plugin/ldap-auth.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ done
location /t {
content_by_lua_block {
local plugin = require("apisix.plugins.ldap-auth")
local ok, err = plugin.check_schema({base_dn = 123, ldap_host = "127.0.0.1", ldap_port=1389})
local ok, err = plugin.check_schema({base_dn = 123, ldap_uri = "127.0.0.1:1389"})
if not ok then
ngx.say(err)
end
Expand Down Expand Up @@ -115,8 +115,7 @@ passed
"plugins": {
"ldap-auth": {
"base_dn": "ou=users,dc=example,dc=org",
"ldap_host": "127.0.0.1",
"ldap_port": 1389,
"ldap_uri": "127.0.0.1:1389",
"uid": "cn"
}
},
Expand Down Expand Up @@ -231,8 +230,7 @@ find consumer user01
"plugins": {
"ldap-auth": {
"base_dn": "ou=users,dc=example,dc=org",
"ldap_host": "127.0.0.1",
"ldap_port": 1389,
"ldap_uri": "127.0.0.1:1389",
"uid": "cn"
}
},
Expand Down Expand Up @@ -306,7 +304,7 @@ find consumer user01
ngx.HTTP_GET,
nil,
[[
{"title":"work with route or service object","required":["base_dn","ldap_host","ldap_port"],"properties":{"base_dn":{"type":"string"},"ldap_host":{"type":"string"},"ldap_port":{"type":"number"},"use_tls":{"type":"boolean"},"verify_ldap_host":{"type":"boolean"},"disable":{"type":"boolean"},"uid":{"type":"string"}},"type":"object"}
{"title":"work with route or service object","required":["base_dn","ldap_uri"],"properties":{"base_dn":{"type":"string"},"ldap_uri":{"type":"string"},"use_tls":{"type":"boolean"},"verify_ldap_host":{"type":"boolean"},"disable":{"type":"boolean"},"uid":{"type":"string"}},"type":"object"}
]]
)
ngx.status = code
Expand Down Expand Up @@ -342,7 +340,7 @@ find consumer user01
ngx.HTTP_GET,
nil,
[[
{"title":"work with route or service object","required":["base_dn","ldap_host","ldap_port"],"properties":{"base_dn":{"type":"string"},"ldap_host":{"type":"string"},"ldap_port":{"type":"number"},"use_tls":{"type":"boolean"},"verify_ldap_host":{"type":"boolean"},"disable":{"type":"boolean"},"uid":{"type":"string"}},"type":"object"} ]]
{"title":"work with route or service object","required":["base_dn","ldap_uri"],"properties":{"base_dn":{"type":"string"},"ldap_uri":{"type":"string"},"use_tls":{"type":"boolean"},"verify_ldap_host":{"type":"boolean"},"disable":{"type":"boolean"},"uid":{"type":"string"}},"type":"object"} ]]
)
ngx.status = code
}
Expand All @@ -361,8 +359,7 @@ find consumer user01
"plugins": {
"ldap-auth": {
"base_dn": "ou=users,dc=example,dc=org",
"ldap_host": "localhost",
"ldap_port": 1636,
"ldap_uri": "localhost:1636",
"uid": "cn",
"use_tls": true
}
Expand Down Expand Up @@ -411,8 +408,7 @@ find consumer user01
"plugins": {
"ldap-auth": {
"base_dn": "ou=users,dc=example,dc=org",
"ldap_host": "localhost",
"ldap_port": 1636,
"ldap_uri": "localhost:1636",
"uid": "cn",
"use_tls": true,
"verify_ldap_host": true
Expand Down