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

Reply via email to