Recursive group member search strategy #64
Conversation
| options[:size] = 1 | ||
| options[:scope] = Net::LDAP::SearchScope_BaseObject | ||
| options[:attributes] ||= [] | ||
| search(options).first |
There was a problem hiding this comment.
Not a blocker since this was here previously, but does it feel misleading that the method is named bind and what it actually does is search?
There was a problem hiding this comment.
@jch yeah, it's definitely confusing. I think I'm going to make some changes to the API for membership validators in this release which will eventually be breaking so it might be a good time to fix this (in a follow up PR).
|
Tests look good. I liked your inline comments on the recursive strategy. It's more verbose, but made it much easier to understand. 👍 to ship unless you had specific areas you'd like me to review |
|
@jch right on. I'm going to generalize the attributes since it's hardcoded right now, along with better tests, and merge this. The other items can be worked on separately, after this is merged. |
| entries.concat entries_by_dn(dns) unless dns.empty? | ||
|
|
||
| entries | ||
| end |
There was a problem hiding this comment.
Added this method to handle handle the various member attributes better.
|
Pretty happy with where this is at for now. @jch one last 👓 before merge? |
Recursive group member search strategy
|
🤘 |
This adds a recursive search strategy to find all members of a given group, including nested groups.
This implementation has three desirable qualities for a search strategy like this:
Unfortunately,
memberattributes are DNs which requires a search per DN, resulting in as many queries as there are unique DNs to search for, resulting in potentially huge query volume. One option is to experiment with querying based on the RDN, but that could result in ill-defined behavior.Still to do:
member)experiment with extracting RDNs from DNs to do bulk searches for member entries(separate PR)benchmark against(separate issue)Group#membersandGroup#subgroups(classicstrategy)cc @jch @github/ldap