Open runyaga opened 11 years ago
Are these criteria fit for all versions of AD?
Pull requests are welcome.
@adaugherity since you fixed AD issues, may you look at this one here and do comment/ close (other action)?
I will have to research the appropriateness of these filters. In my setup, I'm not using any extra search filter at all and don't have any issues with seeing "person" or "group" objects in Plone that shouldn't be there (e.g. computers, distribution lists) but that doesn't mean other people aren't.
Rather than silently set _extra_user_filter, it would be more appropriate to expose it in the configuration form. There is also the problem of #8 where without having it in the form, saving the form blows it away. I'd consider working on the form except that apparently the form is not compatible with Plone 5 (#24) so I don't want to put work into something that may be discarded soon.
I also discovered that a recent commit to PloneLDAP (collective/Products.PloneLDAP#5) enables a similar filter for AD (the filter has been there for a while, but a typo kept it from being applied), however in my brief testing I'm not seeing it (perhaps because we are apparently overwriting it?) Between this fix (assuming it works and the problem is in fact with p.a.ldap) and my LDAP group <=> Plone role fix (collective/Products.PloneLDAP#2), Products.PloneLDAP looks overdue for a release (last was 2012-11-30), but I don't know who the release engineer is there.
Additional criteria for AD should be:
(|(groupType:1.2.840.113556.1.4.803:=2147483648)(&(sAMAccountType=805306368)(!(userAccountControl:1.2.840.113556.1.4.803:=2))))
This ensures that all ldap queries are filtered specifically for "either Groups (including DL) and active user accounts)"
This should be the default. (groupType:1.2.840.113556.1.4.803:=2147483648) could be removed if you are not using LDAP groups.