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

[Rob Crittenden]

Ondrej Hamada wrote:
> On 02/28/2012 09:57 PM, Rob Crittenden wrote:
>> 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.
> Done.
>>
>> I wonder if ObjectclassViolation is a better exception. SyntaxError
>> means the data type is wrong, not that it isn't allowed.
> I agree that it makes more sense and I've updated the patch that way,
> but the documentation says: "permission operation fails with schema
> syntax errors" - maybe we should also update the documentation.
>>
>> rob
>>
>>
>
>

I'll open a ticket on clarifying SyntaxError.

ACK, pushed to master and ipa-2-2

rob