[ZBX-22254] LDAP JIT "Group name attribute" setting for memberOf is wrong and misleading Created: 2023 Jan 25 Updated: 2024 Apr 10 Resolved: 2023 Jun 09 |
|
Status: | Closed |
Project: | ZABBIX BUGS AND ISSUES |
Component/s: | Documentation (D), Frontend (F) |
Affects Version/s: | 6.4 (plan) |
Fix Version/s: | 6.4.4rc1, 7.0.0alpha2, 7.0 (plan) |
Type: | Problem report | Priority: | Trivial |
Reporter: | Marcos de Oliveira | Assignee: | Gregory Chalenko |
Resolution: | Fixed | Votes: | 0 |
Labels: | LDAP, frontend, ldap, trivial, usability | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified |
Team: | |
Sprint: | Sprint 99 (Apr 2023), Sprint 100 (May 2023), Sprint 101 (Jun 2023) |
Story Points: | 0.25 |
Description |
`Group Name Attribute` should not be configurable for `memberOf` config. It is misleading, as it is used in a regex to match the returned memberOf attribute and not the actual group name. You could argue that it is in fact the "Group name attribute", as it actually is the first RDN in the LDAP group's DN, but then, the regex in code is wrong, as currently (`CN=(?<groupname>[^,])`), it matches the string after the attribute name (CN=) until it finds a comma (`(?<groupname>[^,])`), but commas are allowed in Common Name. So if a user has a group with a comma in it, like "My, Group", for example, it will match only until the comma (`My`, in that case). Also, it is considering case, so specifying `cn` will fail, while leaving it empty, will pass, as regex will be constructed like `=(?<groupname>[^,]+)`. I have three suggestions for resolving this:
Those two have drawbacks, but IMO, solution 1 is preferred, as it is less costly, because there is no need to query LDAP for every group the user is a member of. The drawback is that users must specify the entire DN of a group. But for that, you would just need to document that the actual memberOf value is expected (a DN) for the pattern. Users could still be able to specify wildcards, like "CN=My, Group*".
Anyway, I'll be using the "groupOfNames" option, since that allows for more granularity and recursiveness. I'll propose a DOC change explaining how to match groups recursively with the lowest possible cost, using groupOfNames and LDAP_MATCHING_RULE_IN_CHAIN. |
Comments |
Comment by Marcos de Oliveira [ 2023 Jan 25 ] |
I forgot to mention how I would do option 2. To lower the cost, I'd do a query to LDAP with each group DN as `base` with `objectClass=*` as filter, asking just for the specified attribute (like CN or sAMAccountName, which are both the group's name in AD), like: $this->search($group_dn, "objectClass=*", $placeholders, $this->cnf['group_name']) As you you'd be passing $group_dn as base, I don't know if it would be necessary, but would definitely be safer, to use ldap_read instead of ldap_search, as ldap_read does a query using LDAP_SCOPE_BASE instead of LDAP_SCOPE_SUBTREE. |
Comment by Gregory Chalenko [ 2023 Apr 14 ] |
Fixed in development branch feature/ZBX-22254-6.4, PR#5533. |
Comment by Gregory Chalenko [ 2023 Jun 01 ] |
Fixed in:
|