[Freeipa-devel] [RANT] --setattr validation is a minefield.

[Rob Crittenden]

Petr Viktorin wrote:
> On 05/14/2012 04:00 PM, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On Thu, 2012-05-10 at 15:19 +0200, Petr Viktorin wrote:
>>>> On 04/10/2012 07:53 PM, Martin Kosek wrote:
>>>>> On Tue, 2012-04-10 at 19:25 +0200, Petr Viktorin wrote:
>>>>>> On 04/10/2012 07:07 PM, Martin Kosek wrote:
>>>>>>> On Tue, 2012-04-10 at 17:03 +0200, Jan Cholasta wrote:
>>>>>>>> On 10.4.2012 16:00, Petr Viktorin wrote:
>>>>> [snip]
>>>>>>>> Like you said above, we should either not validate
>>>>>>>> --{set,add,del}attr
>>>>>>>> or don't allow them on known attributes.
>>>>>>>
>>>>>>> IMHO, validating attributes we manage in the same way for both
>>>>>>> --setattr
>>>>>>> and standard attrs is not that wrong. It is a good precaution,
>>>>>>> because
>>>>>>> if we let an unvalidated value in, it can make even a bigger mess
>>>>>>> later
>>>>>>> in our pre_callbacks or post_callbacks where we assume that at this
>>>>>>> point everything is valid.
>>>>>>
>>>>>> Then we should validate *exactly* the same way, including not
>>>>>> allowing
>>>>>> no_update attributes to be updated.
>>>>>
>>>>> That makes some sense, I could agree with that.
>>>>>
>>>>
>>>> Now that I have a ticket on this
>>>> (https://fedorahosted.org/freeipa/ticket/2580), I would like to get
>>>> some
>>>> wider agreement here.
>>>>
>>>> The no_update/no_create attributes are mainly "enabled" flags
>>>> (ipaenabledflag, nsaccountlock, idnszoneactive), administrative
>>>> (krbprincipalname, ipauniqueid, ipacertificatesubjectbase), DNS record
>>>> type and data, and various virtual attributes.
>>>>
>>>> If setattr etc. is disabled for all of these, it will mainly matter for
>>>> the "enabled" flags. To be honest I don't know why we only allow
>>>> modifying those through special commands.
>>>> If there's some security reason for that, then setattr etc. should be
>>>> disabled for them; otherwise I think they should be changeable through
>>>> xyz-mod.
>>>
>>> I am not aware of any security reasons why we use special commands for
>>> enabling/disabling objects. I assume this is to make it different from
>>> standard object attribute changes and make sure that user does not
>>> disable the object "by accident" when doing a mod operation. Rob, maybe
>>> you remember the reason for this interface....
>>>
>>> But since we already have this approach, we should keep it and implement
>>> missing "xyz-enable" and "xyz-disable" command so that user's using
>>> *attr interface to play with enabled/disabled attributes can switch to
>>> the specialized commands.
>>>
>>> So far, I noticed that only DNS zone object misses the enable/disable
>>> commands, I created a ticket to fix that:
>>>
>>> https://fedorahosted.org/freeipa/ticket/2754
>>>
>>>> Either way, setattr etc. should honor the no_update flags. Any
>>>> objections?
>>>>
>>>
>>> Nope - as long as ticket 2754 is fixed.
>>>
>>> Martin
>>>
>>
>> I think those are there so they don't show up for the -mod command since
>> we have a separate interface for doing it.
>>
>> rob
>
> For that purpose, would a no_cli flag be better than no_create+no_update?
>
>
> I found one more no_update attribute that might be useful with
> --setattr: externalhost.
>

I think the flags are really only considered on the cli now. If a 
different flag can make the intention clearer I'm ok with that. Having 
two flags seems more flexible though.

rob