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 <fidencio(a)redhat.com>
wrote:
On Wed, Apr 26, 2017 at 11:34 AM, Pavel Březina <notifications(a)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/7465d487ef52fd1cc704e2e67c6...
> -
>
> *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_...
> .
>
Best Regards,
"""
See the full comment at
https://github.com/SSSD/sssd/pull/246#issuecomment-297515332