[SSSD] [PATCHES] SDAP: refactor sdap_access

Pavel Reichl preichl at redhat.com
Wed Aug 13 08:49:58 UTC 2014


On 08/12/2014 02:16 PM, Pavel Březina wrote:
> On 08/11/2014 05:55 PM, Pavel Reichl wrote:
>> Hello,
>>
>> I was struggling with rebasing my patches in the thread "SDAP: Be able
>> to configure sssd to honor openldap account lock to restrict access via
>> ssh key" so I decided to split the patch set into smaller chunks to be
>> able to quickly react on review comments and not to fight with rebasing
>> all the time.
>>
>> Attached patches prepare code for implementing #2364
>>
>> Patch:
>> 1st) acked by Pavel
>> 2nd) Jakub requsted change:
>>> +static errno_t sdap_get_basedn_user_entry(TALLOC_CTX *mem_ctx,
>>> +                                          struct ldb_message
>>> *user_entry,
>>> +                                          const char *username,
>>> +                                          char **_basedn)
>>> +{
>>> +    const char *basedn;
>>> +    errno_t ret;
>>> +
>>> +    basedn = ldb_msg_find_attr_as_string(user_entry, SYSDB_ORIG_DN,
>>> NULL);
>>> +    if (basedn == NULL) {
>>> +        DEBUG(SSSDBG_CRIT_FAILURE,"Could not find originalDN for user
>>> [%s]\n",
>>> +              username);
>>> +        ret = EINVAL;
>>> +        goto done;
>>> +    }
>>> +
>>> +    *_basedn = talloc_strdup(mem_ctx, basedn);
>>> +    if (*_basedn == NULL) {
>>> +        DEBUG(SSSDBG_CRIT_FAILURE,
>>> +              "Could not allocate memory for originalDN\n");
>>> +        ret = ENOMEM;
>>> +        goto done;
>>> +    }
>>> Do we need to strdup the basedn here?
>>>
>> 3rd)  acked by Pavel
>> 4th) Jakub comment:
>>
>>> This function is misplaced, it immediatelly follows _send() in the 
>>> code.
>>> _recv() should be the last in the flow.
>>
>> 5th) acked by Pavel
>>
>> I hope attached patches address Jakub's concerns.
>>
>> Regards,
>>
>> Pavel Reichl
>
> Patch 1: Ack.
> Patch 2: Ack. Just the commit message needs some line wrapping.
> Patch 3: Ack.
> Patch 4:
> It is good to order function by its execution time when you work in 
> tevent. I.e. the correct order should be:
> _send
> check_next_rule (which would be nice to also rename to 
> sdap_access_next_rule or similar)
> _filter_done
> _done
> _recv

OK, I renamed the function and updated the function definition order as 
you proposed.

But I'm wondering whether according to the rule you described 
sdap_access_filter_send should  immediately follow check_next_rule as it 
is called before sdap_access_filter_done?
>
> Patch 5: Ack.
>
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SDAP-split-sdap_access_filter_get_access_done.patch
Type: text/x-patch
Size: 3813 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140813/dea6ca70/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-SDAP-refactor-sdap_access_filter_send.patch
Type: text/x-patch
Size: 3563 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140813/dea6ca70/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-SDAP-nitpicks-in-sdap_access_filter_get_access_done.patch
Type: text/x-patch
Size: 2137 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140813/dea6ca70/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-SDAP-refactor-sdap_access_filter_done.patch
Type: text/x-patch
Size: 6646 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140813/dea6ca70/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-SDAP-don-t-log-error-on-access-denied.patch
Type: text/x-patch
Size: 1191 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140813/dea6ca70/attachment-0004.bin>


More information about the sssd-devel mailing list