[Freeipa-devel] [PATCH 0405] idviews: Add user certificate attribute to user ID overrides

[Jan Cholasta]

On 28.4.2016 12:19, Tomas Babej wrote:
>
>
> On 04/19/2016 08:20 AM, Jan Cholasta wrote:
>> On 13.4.2016 14:13, Tomas Babej wrote:
>>> On 04/13/2016 09:55 AM, Tomas Babej wrote:
>>>> On 04/07/2016 01:53 PM, Sumit Bose wrote:
>>>>> On Mon, Apr 04, 2016 at 04:27:02PM +0200, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 1.4.2016 16:53, Tomas Babej wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> this extends the user ID overrides with capability to store the user
>>>>>>> certificate.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/4955
>>>>>>
>>>>>> The preferred way of managing certificates nowadays is using
>>>>>> $OBJ-add-cert
>>>>>> and $OBJ-remove-cert commands, you should add them here as well.
>>>>>>
>>>>>> I would even go as far as not allowing to modify certificates using
>>>>>> idoverrideuser-mod - in user-mod and host-mod, it's there just for
>>>>>> backward
>>>>>> compatibility, which is not the case here. But I don't have a
>>>>>> strong opinion
>>>>>> on that.
>>>>>>
>>>>>> For consistency with user-find and host-find, the full certificate
>>>>>> blob
>>>>>> should not be shown in idoverrideuser-find. You can do that by setting
>>>>>> search_display_attributes attribute on the idoverrideuser class
>>>>>> appropriately.
>>>>>
>>>>> I tested the current patch with my related patches for SSSD and all is
>>>>> working as expected.
>>>>>
>>>>> bye,
>>>>> Sumit
>>>>>
>>>>>>
>>>>>> Honza
>>>>>>
>>>>>> --
>>>>>> Jan Cholasta
>>>>>>
>>>>>> --
>>>>>> Manage your subscription for the Freeipa-devel mailing list:
>>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>>> Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
>>>>>
>>>>
>>>> Thanks for the reviews,
>>>>
>>>> attaching a updated patch that addresses Honza's comments.
>>>>
>>>> Tomas
>>>>
>>>
>>> Sending an improved version addressing a couple of additional issues.
>>
>> 1) This bit in idoverrideuser_add.pre_callback() is redundant, as the
>> certificate will always be DER here already:
>>
>>         # Normalize the certificate to DER format
>>         certs = options.get('usercertificate', [])
>>         certs_der = [x509.normalize_certificate(c) for c in certs]
>>         entry_attrs['usercertificate'] = certs_der
>>
>>
>> 2) You need to call convert_usercertificate_pre() in
>> idoverrideuser_mod.pre_callback() and convert_usercertificate_post() in
>> idoverrideuser_{mod,find,show}.post_callback() as well.
>>
>> Honza
>>
>
> Updated patch attached, mentioned issues should be fixed, I also removed
> one redundant import which escaped my careful eye.

Thanks, ACK.

Added ticket URL and pushed to master: 
6adf86378108cdf8b0825277431419a5e803aeb5

-- 
Jan Cholasta