[Freeipa-devel] [PATCH 0303] ipalib: Make sure correct attribute name is referenced for

[Martin Kosek]

On 02/19/2015 05:55 PM, Tomas Babej wrote:
> 
> On 02/19/2015 05:45 PM, Martin Kosek wrote:
>> On 02/19/2015 05:40 PM, Alexander Bokovoy wrote:
>>> On Thu, 19 Feb 2015, Tomas Babej wrote:
>>>> On 02/19/2015 05:32 PM, Martin Kosek wrote:
>>>>> On 02/19/2015 05:29 PM, Alexander Bokovoy wrote:
>>>>>> On Thu, 19 Feb 2015, Tomas Babej wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Fixes the invalid attribute name reference in the
>>>>>>> 'System: Read User Addressbook Attributes' permission.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/4883
>>>>>>>
>>>>>>> Tomas
>>>>>>>>  From 93ab1bf897151992df4bd3588386cf8fed4849d2 Mon Sep 17 00:00:00 2001
>>>>>>> From: Tomas Babej <tbabej at redhat.com>
>>>>>>> Date: Thu, 19 Feb 2015 17:10:37 +0100
>>>>>>> Subject: [PATCH] ipalib: Make sure correct attribute name is referenced for
>>>>>>> fax
>>>>>>>
>>>>>>> Fixes the invalid attribute name reference in the
>>>>>>> 'System: Read User Addressbook Attributes' permission.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/4883
>>>>>>> ---
>>>>>>> ipalib/plugins/user.py | 2 +-
>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
>>>>>>> index
>>>>>>> 56585b9f86593c0c5879139103bc71707b88e15f..abe5ee26b8e48681eeb0cbb3bcff8617e212225c
>>>>>>>
>>>>>>>
>>>>>>> 100644
>>>>>>> --- a/ipalib/plugins/user.py
>>>>>>> +++ b/ipalib/plugins/user.py
>>>>>>> @@ -276,7 +276,7 @@ class user(LDAPObject):
>>>>>>>              'ipapermright': {'read', 'search', 'compare'},
>>>>>>>              'ipapermdefaultattr': {
>>>>>>>                  'seealso', 'telephonenumber',
>>>>>>> -                'fax', 'l', 'ou', 'st', 'postalcode', 'street',
>>>>>>> +                'facsimiletelephonenumber', 'l', 'ou', 'st', 'postalcode',
>>>>>>> 'street',
>>>>>>>                  'destinationindicator', 'internationalisdnnumber',
>>>>>>>                  'physicaldeliveryofficename', 'postaladdress',
>>>>>>> 'postofficebox',
>>>>>>>                  'preferreddeliverymethod', 'registeredaddress',
>>>>>> 00core.ldif still contains 'fax' definition as an alias to
>>>>>> 'facsimileTelephoneNumber' so strictly speaking both should be allowed
>>>>>> even though 'fax' attribute name is deprecated.
>>>>> Should, but does not (I tested). This may be a gap in DS ACI evaluation.
>>>>> However, for FreeIPA side, I prefer Tomas' change, even for compatibility
>>>>> with
>>>>> other DS-es - so ACK from me.
>>>> Martin is right, however, I think Alexander was pointing out that we should
>>>> support the deprecated name 'fax', as well as 'facsimileTelephoneNumber'
>>>> directly in the 'System: Read User Addressbook Attributes' read permission.
>>>>
>>>> Am I reading this correctly?
>>> Exactly, both names should be supported in the ACI.
>> Ah, I thought you were referring to DS, not being to able to recognize the
>> alias. Although following this logic, we should for example also have ACIs for
>> commonName, given it's alias for "cn", right?
> 
> Attaching updated patch with both fax and facsimileTelephoneNumber.
> 
> However, Martin is right, the problem occurs multiple times:
> 
> attributeTypes: ( 2.5.4.6 NAME ( 'c' 'countryName' )
> attributeTypes: ( 2.5.4.3 NAME ( 'cn' 'commonName' )
> attributeTypes: ( 0.9.2342.19200300.100.1.25 NAME ( 'dc' 'domaincomponent' )
> attributeTypes: ( 2.5.4.49 NAME ( 'distinguishedName' 'dn' )
> attributeTypes: ( 2.5.4.23 NAME ( 'facsimileTelephoneNumber' 'fax' )
> attributeTypes: ( 2.5.4.7 NAME ( 'l' 'locality' 'localityname' )
> attributeTypes: ( 2.5.4.10 NAME ( 'o' 'organizationname' )
> attributeTypes: ( 2.5.4.11 NAME ( 'ou' 'organizationalUnitName' )
> attributeTypes: ( 2.5.4.4 NAME ( 'sn' 'surName' )
> attributeTypes: ( 2.5.4.8 NAME ( 'st' 'stateOrProvinceName' )
> attributeTypes: ( 2.5.4.9 NAME ( 'street' 'streetaddress' )
> attributeTypes: ( 0.9.2342.19200300.100.1.1 NAME ( 'uid' 'userid' )

I would personally still be OK only with the fax attribute (the original patch)
- so that our behavior is consistent with these attributes. Should not harm us
as our API only supports facsimileTelephoneNumber anyway.

Not a blocker though.