Adding RFC2307 (openLDAP/Posix) style LDAP group memberships

BookStackApp / BookStack

A platform to create documentation/wiki content built with PHP & Laravel

https://www.bookstackapp.com/

MIT License

15.43k stars 1.94k forks source link

Adding RFC2307 (openLDAP/Posix) style LDAP group memberships #5281

Closed bennet0496 closed 4 weeks ago

bennet0496 commented 4 weeks ago

OpenLDAP servers (or maybe other Non-AD Servers as well), predominately used in Unix/Linux environments may not have the memberOf overlay configured and rely on RFC2307 style group memberships, where an extra query is required to resolve the username or uid against the memberUid attributes of the group. With this PR I'd like to add support these kinds of group memberships.

For this I added the following config values to the .env:

LDAP_GROUP_STYLE which has to be set to RFC2307 to activate posix groups. Any other value will default to AD/memberOf behavior
LDAP_GROUP_BASE_DN which is the base for the groups to be searched
LDAP_GROUP_MEMBER_ATTRIBUTE which is the group attribute the username/uid is matched against. Defaults to memberUid
LDAP_GROUP_FILTER is a filter for the group search, that defaults to (&(objectClass=posixGroup))

ssddanbrown commented 4 weeks ago

Thanks for offering this @bennet0496, but I'm not really keen to expand the scope of the what we maintain/support here without some decent proven user-base need to avoid a continuously growing maintenance & support scope for auth. And since there's been no significant prior request for this so far, I feel the demand for this will be quite minor.

bennet0496 commented 4 weeks ago

I get that concern. And I can't really speak in the commonality of running openLDAP this way, I just can say that it basically is the default, as it requires a bit of extra setup to get it running with the memberOf attribute, and once you have it running one way it is not trivial just switching to the other. This is the reason, that where I work, we have run Bookstack with with a similar hack since about 3 years ago (and little to no changes to it since then) where we started using it. I basically just cleaned up the code a little to make it fit better into the code base.

But I also totally get that you may not want to add more complexity for potentially a single installation. I and we would just appreciate support for this upstream as it would make updates a little easier, but if you don't want then this is fine as well and we will continue to maintain it the way we did it until now.

ssddanbrown commented 4 weeks ago

Thanks for the understanding @bennet0496. I'll therefore close this off. Feel free to create a feature request for this for others to indicate their support, and reference this PR as a potential solution for users that desire it.

If interested, I'd also be happy with exploring options to make this easier to add via the logical theme system, that way any kind of custom group sync logic could potentially be added where desired without us having to build in specific support.