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

[Tomas Babej]

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.

Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0405-5-idviews-Add-user-certificate-attribute-to-user-ID-ov.patch
Type: text/x-patch
Size: 16086 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160428/82405b01/attachment.bin>