> On Sun, 2012-06-10 at 15:32 -0400, Stephen Gallagher wrote: > > This is the second set of patches. These aren't quite ready for a > > complete review. They are functional, but they need some discussion. > > > > These patches attempt to implement > > https://fedorahosted.org/sssd/ticket/1367. > > > > For details on the "magic" filters, see > > http://msdn.microsoft.com/en-us/library/windows/desktop/aa746475% > > 28v=vs.85%29.aspx (http://goo.gl/Czjlf if that doesn't come through > > email well). > > > > > > > > Patch 0006: Add new option "ldap_use_matching_rule_in_chain". This patch > > is very incomplete. It only adds the new option. It is missing > > SSSDConfig API entries and manpages. It is included because it's > > necessary to test the other two patches. > > Ok, I wasn't sure I'd get back to this today, which is why I sent that > patch out incomplete. I found the time, so I'm reattaching these > patches, now with API and manpage entries. > > I didn't change the other two.
Ok, here is the first round. I didn't focus my review on details, just the concept and that all parts look to be sane. Patch #0006: see comment below. Other than that the patch is fine. Patch #0007: Setting ldap_use_matching_rule_in_chain should be conditionally dependant on the right schema being used (just to avoid any inconsistencies). Currently you test only for schema != rfc2307, I'd be more defensive about that. Definition of SDAP_AD_MATCHING_RULE would be semantically better in patch #0006. attrs argument for sdap_get_ad_match_rule_members_send() Is it necessary? You don't use it anywhere. I don't think your approach for copying origDN values in sdap_ad_match_rule_members_process() is correct. If user is without origDN, the sdap_nested_group_populate_users() will fail prior to your test here. Patch #0008: Preliminary Ack I will do the testing during the second round. Could you please send me a config file so I don't have to set up testing AD server and can use your config instead? Thanks Jan
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
