[Freeipa-devel] [PATCH] 530 trust plugin: Fix typo in attribute name

[Martin Kosek]

On 04/28/2014 12:38 PM, Alexander Bokovoy wrote:
> On Mon, 28 Apr 2014, Martin Kosek wrote:
>> On 04/28/2014 11:14 AM, Alexander Bokovoy wrote:
>>> On Fri, 18 Apr 2014, Petr Viktorin wrote:
>>>> From 00756cf2c9682b32dba3388e07dda3fad916e284 Mon Sep 17 00:00:00 2001
>>>> From: Petr Viktorin <pviktori at redhat.com>
>>>> Date: Thu, 17 Apr 2014 19:06:52 +0200
>>>> Subject: [PATCH] trust plugin: Remove ipatrustauth{incoming,outgoing} from
>>>> default attrs
>>>>
>>>> These attributes contain secrets for the trusts and should not be returned
>>>> by default.
>>>> ---
>>>> ipalib/plugins/trust.py | 7 +++----
>>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
>>>> index
>>>> f57cf7d891928903fdbee67697b96db4ad2679b7..8fff1cae306559fb42209cbd1aaabcbd9046a27b
>>>>
>>>> 100644
>>>> --- a/ipalib/plugins/trust.py
>>>> +++ b/ipalib/plugins/trust.py
>>>> @@ -306,12 +306,11 @@ class trust(LDAPObject):
>>>>     object_name_plural = _('trusts')
>>>>     object_class = ['ipaNTTrustedDomain']
>>>>     default_attributes = ['cn', 'ipantflatname', 'ipanttrusteddomainsid',
>>>> -        'ipanttrusttype', 'ipanttrustattributes', 'ipanttrustdirection',
>>>> 'ipanttrustpartner',
>>>> -        'ipantauthtrustoutgoing', 'ipanttrustauthincoming',
>>>> 'ipanttrustforesttrustinfo',
>>>> +        'ipanttrusttype', 'ipanttrustattributes', 'ipanttrustdirection',
>>>> +        'ipanttrustpartner', 'ipanttrustforesttrustinfo',
>>>>         'ipanttrustposixoffset', 'ipantsupportedencryptiontypes' ]
>>>>     search_display_attributes = ['cn', 'ipantflatname',
>>>> -                                 'ipanttrusteddomainsid', 'ipanttrusttype',
>>>> -                                 'ipantsidblacklistincoming',
>>>> 'ipantsidblacklistoutgoing' ]
>>>> +                                 'ipanttrusteddomainsid', 'ipanttrusttype']
>>>>
>>>>     label = _('Trusts')
>>>>     label_singular = _('Trust')
>>>
>>> ACK.
>>> This all looks fine, I only have one question -- SID blacklists now
>>> became invisible by default to anyone. Even admins can't see them other
>>> than with --all. I'm not sure they are really that important to deny
>>> access to, but it makes sense to reduce their visibility to normal
>>> users.
>>
>> I think we should differentiate 2 aspects of trust plugin patches:
>>
>> 1) SID blacklist access: with current patches, only admin, trust admins and
>> people with appropriate permission/privilege will have access to SID
>> blacklists. Normal users will only see basic trust attributes (see patch
>> 529.2). If this is not OK, and everyone should be able to see them, please yell.
> I'm OK with this state.
> 
> 
>> 2) SID blacklist visibility: with proposed patches, SID blacklist will be
>> hidden unless --all option is passed. I think this is rather a UX question than
>> functional question. Whether SID blacklist should be printed with each
>> trust-find or trust-show.
>>
>> I would personally not show them to make the display simpler and would remove
>> search_display_attributes entirely (or make blacklist shown in trust-show, but
>> not with trust-find)  but I do not insist.
> The only potential issue in not showing them is that this breaks
> whatever QE tests expected in past. I.e. if they were expecting SID
> blacklist to be shown, that would be broken. To my knowledge there is no
> real dependency on such data in the QE scripts yet.

I would personally drive FreeIPA development by what we see as the right
behavior, not by state of a potentially old QE scripts. Tests should reflect
such changes in the code and adhere to them, not vice versa, IMO.

Martin