URL: https://github.com/SSSD/sssd/pull/246 Title: #246: filter_users and filter_groups stop working properly in v 1.15
fidencio commented: """ On Wed, Apr 26, 2017 at 6:34 PM, Fabiano Fidêncio <fiden...@redhat.com> wrote: > > > On Wed, Apr 26, 2017 at 11:34 AM, Pavel Březina <notificati...@github.com> > wrote: > >> >> - >> >> *NSS: Use fqnames when performing a ncache check* >> This should be done in the ncache module, not in the callers. >> >> > Well, I'm not exactly sure about this. > Currently we the situation we have in the code is that we call ncache > check using both fully qualified names and non fully qualified names ... so > patching it in ncache module itself may end up causing more issues. > > Any input here is appreciated :-) > Nevermind, got it :-) > > >> >> - >> - >> >> *RESPONDER: Make nss_get_name_from_msg() part of responder_utils* >> Nikolai already did the same for his tlog integration and he need >> this function also in providers. Can you cherry-pick this commit instead >> so >> Nikolai doesn't have to solve conflicts? >> 7465d48 >> >> <https://github.com/SSSD/sssd/pull/136/commits/7465d487ef52fd1cc704e2e67c62d427143f0af1> >> - >> >> *CACHE_REQ: Add a new cache_req_ncache_filter_fn() plugin function* >> >> /**+ * Filter the result through the negative cache.+ *+ * This is useful >> for plugins which don't use name as an input+ * takes but can be affected by >> filter_users and filter_groups ^ token+ * options.+ */ >> +typedef errno_t >> +(*cache_req_ncache_filter_fn)(struct sss_nc_ctx *ncache, >> + struct sss_domain_info *domain, >> + char *name); >> >> * **CACHE_REQ: Make use of cache_req_ncache_filter_fn()** >> ```c >> static errno_t >> cache_req_search_get_name_from_msg(TALLOC_CTX *mem_ctx, >> struct ldb_message *msg, >> struct sss_domain_info *domain, >> bool override_space, >> char **_name) >> { >> TALLOC_CTX *tmp_ctx; >> const char *name; >> char *output_name; >> char *fqname; >> errno_t ret; >> >> tmp_ctx = talloc_new(NULL); >> if (tmp_ctx == NULL) { >> return ENOMEM; >> } >> >> name = sss_get_name_from_msg(domain, msg); >> >> ^^^ name can be NULL and we don't want to fail with EINVAL in this case >> >> @pbrezina ... What do we want to do in this case? It would fail with EINVAL later on when filling the grent/pwent anyways, no? > output_name = sss_output_name(tmp_ctx, name, domain->case_preserve, > override_space); if (output_name == NULL) { > DEBUG(SSSDBG_CRIT_FAILURE, "sss_output_name() failed\n"); ret = > ENOMEM; goto done; } fqname = > sss_create_internal_fqname(tmp_ctx, output_name, domain->name); if (fqname > == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "sss_create_internal_fqname() > failed\n"); ret = ENOMEM; goto done; } *_name = > talloc_steal(mem_ctx, fqname); ret = EOK;done: talloc_free(tmp_ctx); > return ret;} >> >> Besides these small things, the approach you've taken is good. I would >> just like you to do more thing -- move code that deals with creating the >> new result into separate function(s) into cache_req_result.c and enable >> this also for enumeration so you can remove the check (and the first patch) >> from nss responder. >> > And last comment ... the introduced functions are not dealing with cache_req_result at all. What they do is changing the ldb_result and moving this part of the code to the cache_req_result.c does seem a little bit weird to me. — >> You are receiving this because you authored the thread. >> Reply to this email directly, view it on GitHub >> <https://github.com/SSSD/sssd/pull/246#issuecomment-297319252>, or mute >> the thread >> <https://github.com/notifications/unsubscribe-auth/AAG4eoKAi8oQnPiyuUdS6y_WWXWU8bihks5rzw-vgaJpZM4NGkQK> >> . >> > > Best Regards, """ See the full comment at https://github.com/SSSD/sssd/pull/246#issuecomment-297515332
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
