GUACAMOLE-954: Add LDAP support for nested user groups #1091
Conversation
Personally I think it would be fine to have this feature included, provided we clearly document when it is expected to work and when it probably will not work. Many other software applications out there that leverage LDAP have ways of configuring the type of LDAP server you're pointing to in order to make use of specific features, and I agree that it would be useful for folks pointing at AD-based LDAP servers. |
| if (config.getNestedGroups()) { | ||
|
|
||
| // Add support for nested groups using LDAP_MATCHING_RULE_IN_CHAIN | ||
| // (memberOf:1.2.840.113556.1.4.1941:=<UserDN>) | ||
| // Matching rule OID for LDAP_MATCHING_RULE_IN_CHAIN | ||
| // ** This possibly only supports Active Directory ** | ||
| ExtensibleNode node = new ExtensibleNode("member"); | ||
| filterValue = null; | ||
|
|
||
| // Explicitly set the matching rule ID and dnAttributes | ||
| node.setMatchingRuleId("1.2.840.113556.1.4.1941"); | ||
| node.setDnAttributes(false); | ||
| node.setValue(new Value(userIDorDN)); | ||
| groupFilter = new AndNode( | ||
| groupFilter, node | ||
| ); | ||
| } |
There was a problem hiding this comment.
I have a few concerns for this:
- Why is
filterValueset tonull, and what is the implication of passing throughnullto thequeryService.search()method? - The OID for LDAP_MATCHING_RULE_IN_CHAIN should probably be defined as a
final Stringabove - if the Apache LDAP API doesn't already have that defined somewhere?
There was a problem hiding this comment.
@necouchman I believe in my testing the group filter applied with the filtervalue as userIDorDN did not work. It seemed like one or the other and I was trying to avoid duplicate code calling the function twice.
Looks like from the doc, it probably was so the groups could be found outside of where the user's account object was located:
The value that should be searched search for within the attributes of objects within the LDAP directory. If null, the search will test only for the presence of at least one of the given attributes on each object, regardless of the value of those attributes.
I am not aware of any constant for that value, but I agree it makes sense to add it to a variable.
There was a problem hiding this comment.
Okay - yes, please define that OID as a constant wherever it makes sense.
2 participants
This is the start of a pull request to address GUACAMOLE-954 in a performant way, but unfortunately this feature only works with Active Directory (as far as I am aware).
I added a new config boolean due to this called 'nested-groups' to enable/disable the AD-specific query.
It makes use of the LDAP_MATCHING_RULE_IN_CHAIN mentioned in the Jira with a specific OID.
I understand if the team is not keen on merging a feature that is only for a specific vendor's LDAP implementation, but this is performant and very useful for us in our AD environment so I would like to try and get it added for the others who requested it and likely using AD as well.