[Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes

[Jan Cholasta]

Dne 4.8.2015 v 13:50 Martin Babinsky napsal(a):
> On 08/04/2015 10:27 AM, Martin Babinsky wrote:
>> On 08/03/2015 06:41 PM, Martin Babinsky wrote:
>>> On 08/03/2015 03:39 PM, Jan Cholasta wrote:
>>>> Dne 3.8.2015 v 14:58 Martin Babinsky napsal(a):
>>>>> On 08/03/2015 02:46 PM, Jan Cholasta wrote:
>>>>>> Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):
>>>>>>> Hi,
>>>>>>>
>>>>>>> Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):
>>>>>>>> This patch fixes the inconsistency between storing certificates in
>>>>>>>> 'userCertificate'/'userCertificate;binary' attribute for the user
>>>>>>>> entries: the certificate must be stored in the latter attribute
>>>>>>>> only.
>>>>>>>>
>>>>>>>> Since a more general fix is out of 4.2.1 scope, I have implemented
>>>>>>>> some
>>>>>>>> workarounds in pre/post callbacks of user-* commands in order to
>>>>>>>> enforce
>>>>>>>> this behavior.
>>>>>>>
>>>>>>> 1)
>>>>>>>
>>>>>>> +    def convert_usercertificate_pre(self, entry_attrs, **options):
>>>>>>> +        if options.get('all', False):
>>>>>>> +            return
>>>>>>>
>>>>>>> We don't want to do any renaming when --raw is specified, not --all.
>>>>>>> Same for convert_usercertificate_post.
>>>>>>
>>>>>> Actually, the attribute should be always renamed in
>>>>>> convert_usercertificate_pre, otherwise we would modify the wrong
>>>>>> attribute. In convert_usercertificate_post, it should actually be
>>>>>> renamed only when --raw is specified.
>>>>>>
>>>>>
>>>>> If you do the rename in `convert_usercertificate_post` only when
>>>>> '--raw'
>>>>> is specified, then you get no certificate displayed when you do `ipa
>>>>> user-show` on user with userCertificate;binary attribute. Is this
>>>>> intended? (Keep in mind that `convert_usercertificate_post` should be
>>>>> called in post-callback when returning results back to user/client).
>>>>
>>>> Oops, I meant "rename only when --raw is *not* specified".
>>>>
>>>>>>>
>>>>>>>
>>>>>>> 2)
>>>>>>>
>>>>>>> +        self.obj.convert_usercertificate_pre(entry_attrs,
>>>>>>> **options)
>>>>>>>
>>>>>>> Rather than calling this directly from user_add, this should be
>>>>>>> called
>>>>>>> from baseuser.pre_common_callback(), which should be called from
>>>>>>> user_add.post_callback().
>>>>>>>
>>>>>>>
>>>>>>> 3) IMO you should change user_{add,remove}_cert to call
>>>>>>> baseuser.convert_usercertificate_{pre,post} as well, to avoid code
>>>>>>> duplication.
>>>>>>>
>>>>>>>
>>>>>>> Honza
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>> Attaching updated patch.
>>>
>>>
>>>
>>
>> I have realized that this patch also fixes
>> https://fedorahosted.org/freeipa/ticket/5173 so I have added the link to
>> the commit message.
>>
>>
>>
> Attaching updated patch.
>

Thanks, ACK.

Pushed to:
master: 3257ac6b876e9e62cae58060c96c525ff0df1ae3
ipa-4-2: 8b3ed42d6b2bab57793b9921a672ed8994469109

-- 
Jan Cholasta