[Freeipa-devel] [PATCH 0012-0019] CA ACL tracker and functional test

[Milan Kubík]

On 10/19/2015 01:38 PM, Martin Basti wrote:
>
>
> On 16.10.2015 15:43, Milan Kubík wrote:
>> On 09/30/2015 02:47 PM, Martin Basti wrote:
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On 09/24/2015 02:49 PM, Milan Kubík
>>> wrote:
>>>
>>>
>>>> Hi
>>>> all,
>>>>
>>>>
>>>>
>>>>
>>>> an update for CA ACL tests!
>>>>
>>>>
>>>>
>>>>
>>>> I, with help from M. Babinsky, managed to find a way how to change
>>>> the identity during acceptance cest run, which allows
>>>>
>>>>
>>>> to test CA ACLs (and perhaps other areas with some form of access
>>>> controll).
>>>>
>>>>
>>>>
>>>>
>>>> This allowed me to write a test for CA ACLs and certificate
>>>> profiles that checks if the ACL/profile is being used and
>>>> enforced.
>>>>
>>>>
>>>> The first several tests are based on Fraser's blogpost using SMIME
>>>> profile [1].
>>>>
>>>>
>>>>
>>>>
>>>> The master and ipa-4-2 branches diverged a bit, so I had to change
>>>> two commits when rebasing to ipa-4-2 branch.
>>>>
>>>>
>>>>
>>>>
>>>> Commits should be applied in the order (including rebased patches
>>>> I sent in an earlier email):
>>>>
>>>>
>>>>
>>>>
>>>> master:
>>>>
>>>>
>>>>     * 12 - 17
>>>>
>>>>
>>>>
>>>>
>>>> ipa-4-2:
>>>>
>>>>
>>>>     * 18, 13 - 15, 19, 17
>>>>
>>>>
>>>>
>>>>
>>>> For convenience:
>>>>
>>>>
>>>> patches on top of master:
>>>> https://github.com/apophys/freeipa/tree/acl-profile-functional
>>>>
>>>>
>>>> patches on top of ipa-4-2:
>>>> https://github.com/apophys/freeipa/tree/acl-42
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> [1]:
>>>> https://blog-ftweedal.rhcloud.com/2015/08/user-certificates-and-custom-profiles-with-freeipa-4-2/
>>>>
>>>>
>>>>
>>>> Cheers,
>>>>
>>>>
>>>> Milan
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> NACK
>>>
>>>
>>>
>>> 0)
>>>
>>> rpm file does not contain test_xmlrpc/data directory, please modify
>>> setup.py.in.
>>>
>>>
>>>
>>> 1)
>>>
>>> Code contains to much todo for my taste.
>>>
>>>
>>>
>>> 2)
>>>
>>> Please do not use filter function, use dict comprehension.
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>> Hi,
>>
>> updated patches and the numbering mess somehow curbed. The patches 
>> are rebased on top of current master and ipa-4-2.
>>
>> 0) fixed by 0021
>>
>> 1) docs for tracker extended, added more test cases
>>
>> 2) changed
>>
>>
>> -- 
>> Milan Kubik
> I have a few comments:
>
> 1)
> +# TODO: rewrite these into Tracker instances
> + at pytest.fixture(scope='class')
> +def smime_user(request):
> +    api.Command.user_add(uid=u'alice', givenname=u'Alice', sn=u'SMIME',
> +                         userpassword=u'Change123')
> +
> +    unlock_principal_password('alice', 'Change123', 'Secret123')
> +
> +    def fin():
> +        api.Command.user_del(u'alice')
> +    request.addfinalizer(fin)
> +
> +    return u'alice'
>
> I do not like hardcoded password value, as this password is used in 
> many places in the test, I sugest to use a module variable
Done
>
> 2)
> +class TestSignWithChangedProfile(XMLRPC_test):
> +    """ Test to verify that the updated profile is used."""
> +    pass  # import invalid profile, try to sign, expect fail
>
> IMO something is missing here, a test maybe?
>
Done by using profile with constraint that CSR cannot meet.
> 3)
> # noqa
> Please remove "# noqa" commets from commits

Done.

-- 
Milan Kubik

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151020/6c3b0034/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkubik-0014.5-tests-add-test-to-check-the-default-ACL.patch
Type: text/x-patch
Size: 5880 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151020/6c3b0034/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkubik-0017-3-ipatests-CA-ACL-and-cert-profile-functional-test.patch
Type: text/x-patch
Size: 16600 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151020/6c3b0034/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkubik-ipa42-0014.5-tests-add-test-to-check-the-default-ACL.patch
Type: text/x-patch
Size: 5880 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151020/6c3b0034/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkubik-ipa42-0017-3-ipatests-CA-ACL-and-cert-profile-functional-test.patch
Type: text/x-patch
Size: 16596 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151020/6c3b0034/attachment-0003.bin>