[SSSD] [PATCH] cache_req: support UPN

Pavel Březina pbrezina at redhat.com
Thu Oct 8 11:21:29 UTC 2015


On 10/07/2015 04:28 PM, Sumit Bose wrote:
> On Wed, Oct 07, 2015 at 04:22:31PM +0200, Sumit Bose wrote:
>> On Wed, Oct 07, 2015 at 02:42:18PM +0200, Pavel Březina wrote:
>>> On 10/06/2015 03:16 PM, Pavel Březina wrote:
>>>> On 10/01/2015 05:06 PM, Sumit Bose wrote:
>>>>> On Thu, Sep 17, 2015 at 11:06:30AM +0200, Pavel Březina wrote:
>>>>>> On 09/17/2015 10:32 AM, Petr Cech wrote:
>>>>>>> Hi Pavel!
>>>>>>>
>>>>>>> There is some code between my last end and this continuation. I was
>>>>>>> read
>>>>>>> it and did't find anything wrong.
>>>>>>
>>>>>> Hi Petr,
>>>>>> thank you for you review. New patches are attached. All comments from
>>>>>> the
>>>>>> previous mail should be addressed.
>>>>>>
>>>>>
>>>>>
>>>>> [PATCH 2/3] cache_req: add support for UPN
>>>>>
>>>>>> diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
>>>>>> index
>>>>>> 5f33b225a31e1c1add7f776215d500fb09e127ca..a72a8bac594dc10ff9aa337399f27d9936830a8e
>>>>>> 100644
>>>>>> --- a/src/db/sysdb_search.c
>>>>>> +++ b/src/db/sysdb_search.c
>>>>>> @@ -295,6 +295,59 @@ static char *enum_filter(TALLOC_CTX *mem_ctx,
>>>>>>       return filter;
>>>>>>   }
>>>>>>
>>>>>> +int sysdb_getpwupn(TALLOC_CTX *mem_ctx,
>>>>>> +                   struct sss_domain_info *domain,
>>>>>> +                   const char *upn,
>>>>>> +                   struct ldb_result **_res)
>>>>>> +{
>>>>>> +    TALLOC_CTX *tmp_ctx;
>>>>>> +    struct ldb_message *msg;
>>>>>> +    struct ldb_result *res;
>>>>>> +    static const char *attrs[] = SYSDB_PW_ATTRS;
>>>>>> +    errno_t ret;
>>>>>> +
>>>>>> +    tmp_ctx = talloc_new(NULL);
>>>>>> +    if (tmp_ctx == NULL) {
>>>>>> +        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
>>>>>> +        return ENOMEM;
>>>>>> +    }
>>>>>> +
>>>>>> +    ret = sysdb_search_user_by_upn(tmp_ctx, domain, upn, attrs, &msg);
>>>>>> +    if (ret != EOK && ret != ENOENT) {
>>>>>> +        DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn()
>>>>>> failed.\n");
>>>>>> +        goto done;
>>>>>> +    }
>>>>>
>>>>> The call stack here would be
>>>>>
>>>>> sysdb_getpwupn()->sysdb_search_user_by_upn()->sysdb_search_entry()->ldb_search()
>>>>>
>>>>>
>>>>> and the returned results, starting from ldb_search()
>>>>>
>>>>> ldb_result -> ldb_message (array) -> ldb_message -> ldb_result
>>>>>
>>>>> I think it would be easier and safe some allocations if it would be
>>>>>
>>>>> sysdb_search_user_by_upn()->sysdb_getpwupn()->ldb_search() .
>>>>>
>>>>> Do you think this change makes sense?
>>>>
>>>> Hi, thanks for the review. Done.
>>>>
>>>> Although I created a new function sysdb_search_user_by_upn_res since you
>>>> can't pass attrs into sysdb_getpwnupn (unless we are inconsistent with
>>>> api).
>>>>
>>>>>
>>>>         const char *orig_name;
>>>>>
>>>>> Instead of adding a new member which again will hold the originally
>>>>> provided name I would suggest to refactor the change added by
>>>>> e87b2a6e94c1066b3044fe683825ff5b4f8716c2 (cache_req: parse input name if
>>>>> needed) by introducing a member called e.g. parsed_name which keeps the
>>>>> name returned by sss_parse_inp_recv() and return it in cache_req_recv().
>>>>>
>>>>> In e87b2a6e94c1066b3044fe683825ff5b4f8716c2 it made sense to overwrite
>>>>> orig_name because no additional member was needed. But now I think the
>>>>> code would be more clear if orig_name will be kept unmodified.
>>>>
>>>> Done in separate patch so the changes are more visible. It can be
>>>> squashed to the second patch.
>>>
>>> Sumit found that test won't pass. New patchset is attached.
>>>
>>>
>>
>> Thank you, the tests pass now. I've seen a compiler warning which is
>> seen with the first patch I attached.
>>
>> During testing I found that UPN lookups for sub-domain users do not
>> work even with the original nss responder code. I attached fixes for
>> your patches and the nss responder since the sysdb change depends on you
>> patches.
>
> and now with patches ...

Ack. Thank you!



More information about the sssd-devel mailing list