[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too jhrozek commented: """ master: e5a984093ad7921c83da75272cede2b0e52ba2d6 24d8c85fae253f988165c112af208198cf48eef6 sssd-1-14: 956fdd727f8d7a28f1456146b3b7dfee49f38626 3f3dc8c737a8e8cfc4a29d7dbaf526ec3973c7a0 """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-258106862 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too sumit-bose commented: """ CI gave an error on RHEL6 http://sssd-ci.duckdns.org/logs/job/56/21/summary.html test_ipa_subdom_server failed in the mock-build test but passed in the makes-tests test. But I think this failure is not related to your patch. """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-257882507 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too jhrozek commented: """ https://fedorahosted.org/sssd/ticket/3230 """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-257864092 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too jhrozek commented: """ (Sorry about adding and removing the label, I misread that CI needs to finish) """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-257862803 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too sumit-bose commented: """ Patch looks good and is working as expected, so ACK. I'm just waiting for CI to finish. While testing I realized that this patch only affects the initgroups lookup and I wondered if and how multiple results from GC lookups are handled by plain user lookups. It turned out that here sdap_object_in_domain() is used which in the end does checks with the help of search bases as well. Nevertheless I think you patches should be push to minimize the risk to change existing behaviour. But a ticket should be opened to replace the logic of sysdb_try_to_find_expected_dn() by sdap_object_in_domain(). """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-257847810 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too jhrozek commented: """ Thank you for the review, I (hopefully :-)) adressed all the comments and pushed new patches. """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-257631167 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too sumit-bose commented: """ There is a slight change in the logic in the first patch. If two entries were found (which should never happen) an error was returned immediately. After refactoring the next matching scheme is tried. Since the next matching scheme should include the results from the one before and error is still returned in the end but some additional cycles are spend. This can be changed by not only returning 'result' but an error code as well. But given the low probability that this kind of error will happen at all I'm not insisting here. In the second patch a comment would be nice explaining why it is sufficient to pick the first search base (because the search bases for a single domain in AD would always have the same DC components). """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-257552210 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too jhrozek commented: """ Hi, thank you for the review and the time on the phone. I pushed new patches where the first one just splits the existing code into more reusable components to avoid too much duplication later and the second one implements your suggestion. One thing that I only realized while testing the patch is that we should still prefer cn=users in the search base matching, do you agree? """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-257426175 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too sumit-bose commented: """ I think comparing with the search bases is a good idea but I think the patch won't work for the general case with some arbitrary search base. The search base has to contain all the needed domain components but may contain any number of additions RDNs. The check in the second loop of sysdb_try_to_find_expected_dn() expects that dom_basedn only contains DC RDNs and that dom_basedn_comp_num would be the first component which does not have DC. If this component is still DC than it belongs to a sub/child domain and can be skipped. So I would suggest to add the search base as an argument to sysdb_try_to_find_expected_dn() instead of the dom_basedn. Then strip all non-DC components from the search base and add a third loop similar to the second one where you compare the result with the list of results. Since all search based for a given domain must have the same DC components it is sufficient to only use the first. If a bogus first search base was added manually in sssd.conf I think we have to ask to remove it from the configuration. I think this search-base based check makes the current check in the second loop redundant, but maybe it would be better to keep it for the time being and just add a comment or ticket to carefully evaluate later if it can be removed. """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-256298615 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#49][comment] Try to match multiple results from an AD initgroups request against domain's search bases, too
URL: https://github.com/SSSD/sssd/pull/49 Title: #49: Try to match multiple results from an AD initgroups request against domain's search bases, too jhrozek commented: """ Bump. Could anyone review this patch, please? """ See the full comment at https://github.com/SSSD/sssd/pull/49#issuecomment-256086797 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
