LDAP group sync improvements

[peteryland]

https://community.openproject.org/projects/openproject/work_packages/34049/activity

Hi,

This little patch to the LDAP group synchronisation makes the following changes:

• Allows for the possibility of nested groups (that is, allows a group to have another group as a member)
• Removes the requirement for users to install the memberOf overlay in their LDAP configurations, although I'd still recommend it anyway
• Improves efficiency since it now searches the groups directly instead of filtering across all users for each group membership

Apologies for the low quality of this contribution as I understand this should also include UI text and documentation updates. Also, I've never done any Ruby before, so I hope it's idiomatic enough to be readable.

[oliverguenther]

Hi @peteryland , thanks for the contribution. I'll take a look at the proposed changes and try to incorporate it into our repository, which will require some additions to tests and documentation. This might take a while. At the same time, I'll probably go ahead and make the actual membership attribute configurable, as some customer's have reported they use non-standard attributes to mark group members, and for us, it's basically a variable attribute name we look for.

Cheers Oliver

[peteryland]

For sure, it would definitely be much better to have the member attribute configurable. It actually felt pretty dirty using a hard-coded list of possibilities.

Anyway, thanks for considering the patch and I hope it helps. Nested groups is something that's pretty important to us as it keeps our LDAP much much cleaner. I'm actually just testing out OpenProject at the moment, but so far I'm really loving it, so thanks very much to all the devs for a great product!

[oliverguenther]

Hi @peteryland , I see your point with regards to efficiency due to the query. I'm wondering if we could avoid doing another lookup in case the user search doesn't yield anything (hinting at the fact that we're looking at a group).

My assumption is that we can test the objectClass for presence of group groupOfNames (or a configurable value to look for), as every LDAP will probably have a unique objectClass for groups. It certainly is in all LDAP integrations we maintain.

I'm not sure why someone wouldn't have memberOf overlay and nested groups. If we could rely on the memberOf overlay, we could get all nested groups in call using &(objectclass=group)(memberOf=parentDN).

[peteryland]

I'm not sure why someone wouldn't have memberOf overlay and nested groups. If we could rely on the memberOf overlay, we could get all nested groups in call using &(objectclass=group)(memberOf=parentDN).

The only issue with this approach is that you'll only get one level of nesting that way. In order to support all LDAP server implementations, it's necessary to do some form of iteration or recursion.