In winbindd_ads.c the function query_user_list() generates a list of *all* users in AD - not just the ones with valid (SFU) attributes. If you are relying on SFU attributes (e.g. using idmap_ad) then any users without SFU attributes won't be valid and will be excluded by the idmap backend when it makes a further, failing, LDAP query per-user. A simple extension to the LDAP filter used in query_user_list() can ask only for user objects that have (an) SFU attribute(s).
Of course this filtering isn't appropriate when the idmap backend is expected to provide uidNumber and gidNumber (e.g. idmap_rid). So the patch I'm going to offer applies the filter optionally. In my example, the filtering happens if 'winbind nss info = sfu' is in effect. Arguably a new parameter could be introduced but we have lots of those already.
Where the number of users with SFU attributes is small compared to the total number of users in AD this produces an enormous performance improvement.
Created attachment 1887 [details]
Restrict query_user_list results to users with SFU attribute
This is a gendiff patch that searches for users that have the uidNumber attribute (or whatever AD calls it) iff 'winbind nss info = sfu' is in effect.
If there is no memory for the extended filter, it falls back to getting all users, which saves an error path but of course in that case something is likely to fail later on anyway.
The problem I see with this approach is this:
what if you just rely on the tdb based idmapping and just want to have homedir, shell, etc. filled in by SFU attributes ? Then all non SFU users will be excluded in the query already.
We could have a keyword like "sfu_only" for the "winbind nss info" though...
In reply to comment #2:
Yes, point taken, and I had that scenario in mind when I posted the bug initially. I probably chose a poor attribute to filter on: at the time I was more interested in seeing if it would improve performance and reduce LDAP traffic.
Actually the technique will work for any SFU or RFC2307 attribute - it just filters users that have a value set for an attribute. So you could use (er...) logonShell instead. The point of the patch is to avoid getting users that have *none* of the SFU/RFC2307 attributes, based on the assumption that if they have one of them, they are likely to have all (or all that this site wants to set).
So how about filtering on a different attribute, one from the more traditional 'nss info' set, such as homeDirectory, logonShell, gecos?
(Or indeed all of them: that might be the path of least surprise: if you configure 'winbind nss info = sfu' then no user appears until they have *all* the necessary attributes.)
> We could have a keyword like "sfu_only" for the "winbind nss info" though...
If it used a better choice of attribute(s) to filter on, I don't think this patch would create any surprises for someone configuring 'sfu' or 'rfc2307' so I'd prefer not to see another keyword.
Thanks for looking at this.
Too late for enhancements for 3.5. Raising version.
a filtering like this which is also inconstently only targeted for a specific idmap backend will not be added.