[Freeipa-devel] [PATCH] 14 ipa permission-add does not fail if using invalid attribute

[Rob Crittenden]

Ondrej Hamada wrote:
> On 02/27/2012 03:22 PM, Rob Crittenden wrote:
>> Ondrej Hamada wrote:
>>> When adding or modifying permission with both type and attributes
>>> specified, check whether the attributes are allowed for specified type.
>>> In case of disallowed attributes the InvalidSyntax error is raised.
>>>
>>> New tests were also added to the unit-tests.
>>>
>>> https://fedorahosted.org/freeipa/ticket/2293
>>>
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
>> NACK. You should use obj.object_class_config to determine if the
>> default list of objectclasses comes from LDAP.
>>
>> I think that may be it, otherwise the patch reads ok.
>>
>> I'm very glad to see unit tests!
>>
>> rob
> Corrected
>

Sorry, found a couple of more things I should have found the first review.

Please use the dn module to construct dn_ipaconfig. Or you can also get 
the DN on-the-fly since the config object using get_dn().

Probably just as safe to call: if obj.object_class_config: ... rather 
than hasattr. I suppose its just a style thing.

I wonder if ObjectclassViolation is a better exception. SyntaxError 
means the data type is wrong, not that it isn't allowed.

rob