#50113 Issue: 50112 Port ACI test suit from TET to python3 (ACI Attr)
Closed 3 years ago by spichugi. Opened 5 years ago by aborah.
aborah/389-ds-base aci  into  master

@@ -0,0 +1,250 @@ 

+ # --- BEGIN COPYRIGHT BLOCK ---

+ # Copyright (C) 2019 Red Hat, Inc.

+ # All rights reserved.

+ #

+ # License: GPL (version 3 or any later version).

+ # See LICENSE for details.

+ # --- END COPYRIGHT BLOCK ---

+ 

+ import pytest, os, ldap

+ from lib389._constants import DEFAULT_SUFFIX, PW_DM

+ from lib389.idm.user import UserAccount

+ from lib389.idm.organization import Organization

+ from lib389.idm.organizationalunit import OrganizationalUnit

+ from lib389.cos import CosTemplate, CosClassicDefinition

+ from lib389.topologies import topology_st as topo

+ from lib389.idm.nscontainer import nsContainer

+ from lib389.idm.domain import Domain

+ from lib389.idm.nsrole import nsFilterRoles

+ 

+ 

+ 

+ DNBASE = "o=acivattr,{}".format(DEFAULT_SUFFIX)

+ ENG_USER = "cn=enguser1,ou=eng,{}".format(DNBASE)

+ SALES_UESER = "cn=salesuser1,ou=sales,{}".format(DNBASE)

+ ENG_MANAGER = "cn=engmanager1,ou=eng,{}".format(DNBASE)

+ SALES_MANAGER = "cn=salesmanager1,ou=sales,{}".format(DNBASE)

+ SALES_OU = "ou=sales,{}".format(DNBASE)

+ ENG_OU = "ou=eng,{}".format(DNBASE)

+ FILTERROLESALESROLE = "cn=FILTERROLESALESROLE,{}".format(DNBASE)

+ FILTERROLEENGROLE = "cn=FILTERROLEENGROLE,{}".format(DNBASE)

+ 

+ 

+ @pytest.fixture(scope="function")

+ def aci_of_user(request, topo):

+     aci_list = Domain(topo.standalone, DEFAULT_SUFFIX).get_attr_vals('aci')

+ 

+     def finofaci():

+         domain = Domain(topo.standalone, DEFAULT_SUFFIX)

+         domain.set('aci', None)

+         for i in aci_list:

+             domain.add("aci", i)

+ 

+     request.addfinalizer(finofaci)

+ 

+ 

+ @pytest.fixture(scope="function")

+ def _add_user(request, topo):

+     org = Organization(topo.standalone).create(properties={"o": "acivattr"}, basedn=DEFAULT_SUFFIX)

+     org.add('aci', '(targetattr="*")(targetfilter="(nsrole=*)")(version 3.0; aci "tester"; '

+                    'allow(all) userdn="ldap:///cn=enguser1,ou=eng,o=acivattr,{}";)'.format(DEFAULT_SUFFIX))

+ 

+     ou = OrganizationalUnit(topo.standalone, "ou=eng,o=acivattr,{}".format(DEFAULT_SUFFIX))

+     ou.create(properties={'ou': 'eng'})

+ 

+     ou = OrganizationalUnit(topo.standalone, "ou=sales,o=acivattr,{}".format(DEFAULT_SUFFIX))

+     ou.create(properties={'ou': 'sales'})

+ 

+     roles = nsFilterRoles(topo.standalone, DNBASE)

+     roles.create(properties={'cn':'FILTERROLEENGROLE', 'nsRoleFilter':'cn=eng*'})

+     roles.create(properties={'cn': 'FILTERROLESALESROLE', 'nsRoleFilter': 'cn=sales*'})

+ 

+     nsContainer(topo.standalone,

+                 'cn=cosClassicGenerateEmployeeTypeUsingnsroleTemplates,o=acivattr,{}'.format(DEFAULT_SUFFIX)).create(

+         properties={'cn': 'cosTemplates'})

+ 

+     properties = {'employeeType': 'EngType', 'cn':'"cn=filterRoleEngRole,o=acivattr,dc=example,dc=com",cn=cosClassicGenerateEmployeeTypeUsingnsroleTemplates,o=acivattr,dc=example,dc=com'}

+     CosTemplate(topo.standalone,'cn="cn=filterRoleEngRole,o=acivattr,dc=example,dc=com",'

+                                 'cn=cosClassicGenerateEmployeeTypeUsingnsroleTemplates,o=acivattr,{}'.format(DEFAULT_SUFFIX)).\

+         create(properties=properties)

+ 

+     properties = {'employeeType': 'SalesType', 'cn': '"cn=filterRoleSalesRole,o=acivattr,dc=example,dc=com",cn=cosClassicGenerateEmployeeTypeUsingnsroleTemplates,o=acivattr,dc=example,dc=com'}

+     CosTemplate(topo.standalone,

+                 'cn="cn=filterRoleSalesRole,o=acivattr,dc=example,dc=com",cn=cosClassicGenerateEmployeeTypeUsingnsroleTemplates,'

+                 'o=acivattr,{}'.format(DEFAULT_SUFFIX)).create(properties=properties)

+ 

+     properties = {

+         'cosTemplateDn': 'cn=cosClassicGenerateEmployeeTypeUsingnsroleTemplates,o=acivattr,{}'.format(DEFAULT_SUFFIX),

+         'cosAttribute': 'employeeType', 'cosSpecifier': 'nsrole', 'cn': 'cosClassicGenerateEmployeeTypeUsingnsrole'}

+     CosClassicDefinition(topo.standalone,

+                          'cn=cosClassicGenerateEmployeeTypeUsingnsrole,o=acivattr,{}'.format(DEFAULT_SUFFIX)).create(

+         properties=properties)

+ 

+     properties = {

+         'uid': 'salesuser1',

+         'cn': 'salesuser1',

+         'sn': 'user',

+         'uidNumber': '1000',

+         'gidNumber': '2000',

+         'homeDirectory': '/home/' + 'salesuser1',

+         'userPassword': PW_DM

+     }

+     user = UserAccount(topo.standalone, 'cn=salesuser1,ou=sales,o=acivattr,{}'.format(DEFAULT_SUFFIX))

+     user.create(properties=properties)

+ 

+     properties = {

+         'uid': 'salesmanager1',

+         'cn': 'salesmanager1',

+         'sn': 'user',

+         'uidNumber': '1000',

+         'gidNumber': '2000',

+         'homeDirectory': '/home/' + 'salesmanager1',

+         'userPassword': PW_DM,

+     }

+     user = UserAccount(topo.standalone, 'cn=salesmanager1,ou=sales,o=acivattr,{}'.format(DEFAULT_SUFFIX))

+     user.create(properties=properties)

+ 

+     properties = {

+         'uid': 'enguser1',

+         'cn': 'enguser1',

+         'sn': 'user',

+         'uidNumber': '1000',

+         'gidNumber': '2000',

+         'homeDirectory': '/home/' + 'enguser1',

+         'userPassword': PW_DM

+     }

+     user = UserAccount(topo.standalone, 'cn=enguser1,ou=eng,o=acivattr,{}'.format(DEFAULT_SUFFIX))

+     user.create(properties=properties)

+ 

+     properties = {

+         'uid': 'engmanager1',

+         'cn': 'engmanager1',

+         'sn': 'user',

+         'uidNumber': '1000',

+         'gidNumber': '2000',

+         'homeDirectory': '/home/' + 'engmanager1',

+         'userPassword': PW_DM

+     }

+     user = UserAccount(topo.standalone, 'cn=engmanager1,ou=eng,o=acivattr,{}'.format(DEFAULT_SUFFIX))

+     user.create(properties=properties)

+ 

+     def fin():

+         for DN in [ENG_USER,SALES_UESER,ENG_MANAGER,SALES_MANAGER,FILTERROLESALESROLE,FILTERROLEENGROLE,ENG_OU,SALES_OU,

+                    'cn="cn=filterRoleEngRole,o=acivattr,dc=example,dc=com",'

+                    'cn=cosClassicGenerateEmployeeTypeUsingnsroleTemplates,o=acivattr,dc=example,dc=com',

+                    'cn="cn=filterRoleSalesRole,o=acivattr,dc=example,dc=com",'

+                    'cn=cosClassicGenerateEmployeeTypeUsingnsroleTemplates,o=acivattr,{}'.format(DEFAULT_SUFFIX), 'cn=cosClassicGenerateEmployeeTypeUsingnsroleTemplates,o=acivattr,{}'.format(DEFAULT_SUFFIX),

+                    'cn=cosClassicGenerateEmployeeTypeUsingnsrole,o=acivattr,{}'.format(DEFAULT_SUFFIX), DNBASE]:

+             UserAccount(topo.standalone, DN).delete()

+ 

+     request.addfinalizer(fin)

+ 

+ 

+ REAL_EQ_ACI = '(targetattr="*")(targetfilter="(cn=engmanager1)") (version 3.0; acl "real-eq"; allow (all) userdn="ldap:///{}";)'.format(ENG_USER)

+ REAL_PRES_ACI = '(targetattr="*")(targetfilter="(cn=*)") (version 3.0; acl "real-pres"; allow (all) userdn="ldap:///{}";)'.format(ENG_USER)

+ REAL_SUB_ACI = '(targetattr="*")(targetfilter="(cn=eng*)") (version 3.0; acl "real-sub"; allow (all) userdn="ldap:///{}";)'.format(ENG_USER)

+ ROLE_EQ_ACI = '(targetattr="*")(targetfilter="(nsrole=cn=filterroleengrole,o=sun.com)") (version 3.0; acl "role-eq"; allow (all) userdn="ldap:///{}";)'.format(ENG_USER)

+ ROLE_PRES_ACI = '(targetattr="*")(targetfilter="(nsrole=*)") (version 3.0; acl "role-pres"; allow (all) userdn="ldap:///{}";)'.format(ENG_USER)

+ ROLE_SUB_ACI = '(targetattr="*")(targetfilter="(nsrole=cn=filterroleeng*)") (version 3.0; acl "role-sub"; allow (all) userdn="ldap:///{}";)'.format(ENG_USER)

+ COS_EQ_ACI = '(targetattr="*")(targetfilter="(employeetype=engtype)") (version 3.0; acl "cos-eq"; allow (all) userdn="ldap:///{}";)'.format(ENG_USER)

+ COS_PRES_ACI = '(targetattr="*")(targetfilter="(employeetype=*)") (version 3.0; acl "cos-pres"; allow (all) userdn="ldap:///{}";)'.format(ENG_USER)

+ COS_SUB_ACI = '(targetattr="*")(targetfilter="(employeetype=eng*)") (version 3.0; acl "cos-sub"; allow (all) userdn="ldap:///{}";)'.format(ENG_USER)

+ LDAPURL_ACI = '(targetattr="*")(version 3.0; acl "url"; allow (all) userdn="ldap:///o=acivattr,dc=example,dc=com??sub?(nsrole=*eng*)";)'

+ 

+ 

+ @pytest.mark.parametrize("user,entry,aci", [

+     (ENG_USER, ENG_MANAGER, REAL_EQ_ACI),

+     (ENG_USER, ENG_MANAGER, REAL_PRES_ACI),

+     (ENG_USER, ENG_MANAGER, REAL_SUB_ACI),

+     (ENG_USER, ENG_MANAGER, ROLE_PRES_ACI),

+     (ENG_USER, ENG_MANAGER, ROLE_SUB_ACI),

+     (ENG_USER, ENG_MANAGER, COS_EQ_ACI),

+     (ENG_USER, ENG_MANAGER, COS_PRES_ACI),

+     (ENG_USER, ENG_MANAGER, COS_SUB_ACI),

+     (ENG_USER, ENG_MANAGER, LDAPURL_ACI),

+ ], ids=[

+     "(ENG_USER, ENG_MANAGER, REAL_EQ_ACI)",

+     "(ENG_USER, ENG_MANAGER, REAL_PRES_ACI)",

+     "(ENG_USER, ENG_MANAGER, REAL_SUB_ACI)",

+     "(ENG_USER, ENG_MANAGER, ROLE_PRES_ACI)",

+     '(ENG_USER, ENG_MANAGER, ROLE_SUB_ACI)',

+     '(ENG_USER, ENG_MANAGER, COS_EQ_ACI)',

+     '(ENG_USER, ENG_MANAGER, COS_PRES_ACI)',

+     '(ENG_USER, ENG_MANAGER, COS_SUB_ACI)',

+     '(ENG_USER, ENG_MANAGER, LDAPURL_ACI)',

+ ])

+ def test_positive(topo, _add_user, aci_of_user, user, entry, aci):

+     """

+         :id: ba6d5e9c-786b-11e8-860d-8c16451d917b

+         :setup: server

+         :steps:

+             1. Add test entry

+             2. Add ACI

+             3. ACI role should be followed

+         :expectedresults:

+             1. Entry should be added

+             2. Operation should  succeed

+             3. Operation should  succeed

+     """

+     # set aci

+     Domain(topo.standalone, DNBASE).set("aci", aci)

+     # create connection

+     conn = UserAccount(topo.standalone, user).bind(PW_DM)

+     # according to the aci , user will  be able to change description

+     UserAccount(conn, entry).replace("description", "Fred")

+     assert UserAccount(conn, entry).present('description')

+ 

+ 

+ @pytest.mark.parametrize("user,entry,aci", [

+     (ENG_USER, SALES_MANAGER, REAL_EQ_ACI),

+     (ENG_USER, SALES_OU, REAL_PRES_ACI),

+     (ENG_USER, SALES_MANAGER, REAL_SUB_ACI),

+     (ENG_USER, SALES_MANAGER, ROLE_EQ_ACI),

+     (ENG_USER, SALES_OU, ROLE_PRES_ACI),

+     (ENG_USER, SALES_MANAGER, ROLE_SUB_ACI),

+     (ENG_USER, SALES_MANAGER, COS_EQ_ACI),

+     (ENG_USER, SALES_OU, COS_PRES_ACI),

+     (ENG_USER, SALES_MANAGER, COS_SUB_ACI),

+     (SALES_UESER, SALES_MANAGER, LDAPURL_ACI),

+     (ENG_USER, ENG_MANAGER, ROLE_EQ_ACI),

+ ], ids=[

+ 

+     "(ENG_USER, SALES_MANAGER, REAL_EQ_ACI)",

+     "(ENG_USER, SALES_OU, REAL_PRES_ACI)",

+     "(ENG_USER, SALES_MANAGER, REAL_SUB_ACI)",

+     "(ENG_USER, SALES_MANAGER, ROLE_EQ_ACI)",

+     "(ENG_USER, SALES_MANAGER, ROLE_PRES_ACI)",

+     '(ENG_USER, SALES_MANAGER, ROLE_SUB_ACI)',

+     '(ENG_USER, SALES_MANAGER, COS_EQ_ACI)',

+     '(ENG_USER, SALES_MANAGER, COS_PRES_ACI)',

+     '(ENG_USER, SALES_MANAGER, COS_SUB_ACI)',

+     '(SALES_UESER, SALES_MANAGER, LDAPURL_ACI)',

+     '(ENG_USER, ENG_MANAGER, ROLE_EQ_ACI)'

+ 

+ 

+ ])

+ def test_negative(topo, _add_user, aci_of_user, user, entry, aci):

+     """

+         :id: c4c887c2-786b-11e8-a328-8c16451d917b

+         :setup: server

+         :steps:

+             1. Add test entry

+             2. Add ACI

+             3. ACI role should be followed

+         :expectedresults:

+             1. Entry should be added

+             2. Operation should  succeed

+             3. Operation should  succeed

+     """

+     # set aci

+     Domain(topo.standalone, DNBASE).set("aci", aci)

+     # create connection

+     conn = UserAccount(topo.standalone, user).bind(PW_DM)

+     # according to the aci , user will not be able to change description

+     with pytest.raises(ldap.INSUFFICIENT_ACCESS):

+         UserAccount(conn, entry).replace("description", "Fred")

+ 

+ 

+ if __name__ == "__main__":

+     CURRENT_FILE = os.path.realpath(__file__)

+     pytest.main("-s -v %s" % CURRENT_FILE)

Port ACI test suit from TET to python3 (ACI Attr)
https://pagure.io/389-ds-base/issue/50112

Reviewed by: ???

rebased onto 4ce30e051260f0f4c72145f1af6f897c37ca220d

5 years ago

rebased onto 9be7103811dad927f0106312647e0d45a596bcff

5 years ago

Please, apply all the discussed issue fixes (from previous PRs) to this PR and rebase.
It has mostly the same problems.

Why are we using the raw entry api when we have orgunit types in lib389 instead?

There is backend and mappingtree types in lib389 that should be used instead. Additionally, you shouldn't be using extensibleObject, and these aci's probably shouldn't be here either ....

I think this change is too big to review effectively, and should be broken into "a commit per test", so that it can be reviewed properly ...

rebased onto 1c789f6ad0c5a35d2115e7030b69714390b2b556

5 years ago

rebased onto bb0706a85df856ceb60d819ce9606e24a226e2d6

5 years ago

rebased onto c82934d846ec332c7bbff53611817e9d9a4f44cd

5 years ago

rebased onto 2c106925fa49e720db29976a19f990dd0610b1a6

5 years ago

rebased onto 23e6358fd48ab7e7b3ab082ff1234ab1770f940f

5 years ago

rebased onto 1a1044e8b8f743c46843b713cac8762d6fae60fc

5 years ago

rebased onto 0992c38ea49ed6445b22da7d4d3372e6f8004f52

5 years ago

I think I said this before, but we should make an nsRole mixin that we can compose with other types instead of using Entry.

write it then? I'd rather this done correct, now, because if it's not done now, it never will be.

Don't use create_test_user here, use the actual .create constuctor.

First, this test relies on the result of another test (I think) which is wrong. If you run this with -k delete_access_to_anyone, does it work?
Second, don't use raw modify, use a type and .set.

Don't use raw bind, there is an account object for "new_conn = account.bind(password)", which doesn't break the dirsrv conn (which is required for this test to function)
Here, if the test fails, you may break teardown as dirsrv is no longer directory manager.

If there is a bug, fix it. Report it. Please do not be lazy and leave this.

Add a group of URLS with group.add('objectClass', 'groupOfURLS')

use the domain object type, and do "domain.add('aci', ...)"

Consider using the delete_branch_s instead if you need to remove all entries in the tree.

Hi,

I've added a lot of comments here, but I think I barely got 10% into the review. I think this change is too large to review effectively, or in a timely manner.

Consider breaking each item into it's own ticket that you can focus on individually. Alternately, consider submitting each test as it's own commit on this branch. @spichugi may have some ideas about this. Additionally, please don't claim there are bugs without discussing or reporting them. Finally, I would like things done correctly, because if it's not done correctly now, it will never be done or finished. Every thing we do right now, saves us hours in the futures, lays foundations for better cli and web tools and makes every single future test case you write easier.

Please contact 389-devel for advice on how to write some of these things if you need help (do not email people personally - we must work in the open to help guide future contributors with lessons.)

Thanks,

rebased onto eeff537395cb98682ef02a71f5ea5d9670ec9e74

5 years ago

rebased onto 1f2326715cdc08b1ee06b321d6776393700d4fad

5 years ago

@firstyear

For effective review , i have just added one script acivattr_test.py with working_contstants.py (which is for all script.) . After this one is merged i will push other script one by one .

All changes are done as per your suggestion

rebased onto 97bb7d8583f8a2044c8bcba9aefcbd7ca4db9944

5 years ago

First, please, don't create so big meaningless lists.
Second, you don't use it. Why do you have it here?

you can name the fixture 'test_user', it makes more sense. Also '_' is not needed for fixtures

Do all the OrganizationalUnit, test user, etc. operations here. No need to encapsulate it. It is just one liners (ous.create() or users.create())

you can define orgs = Organization(topo.standalone) one time and then use it

use the created user DSldapobjects here.

Something like:

users_list = []
users = UserAccounts(topology_st.standalone, suffix)

for num in sample(range(1000), users_num):
    num_ran = int(round(num))
    user = users.create_test_user(uid=num_ran)
    users_list.append(user)
return users_list

 for user in users_list:
        user.delete()

The constant name is hard to decipher. Please, write it in more detailed words.

No need to use 'ensure_bytes' if you use DSLDapObjects

Once again, if you use something like Domain(topo.standalone, DNBASE) - set it to the variable. So you'll have one instance of the class which you reuse

Test case descriptions are required. Write it as a first line at docstring

Avoid adding so many constants. If it is used only once at the test case - it is okay to have it as hard coded.
Or group it as a list or dictionary (you can generate the content with a simple list comprehensions)

The whole file working_contstants.py is redundant. Please, put the fixtures to the test case (or if it makes sense, use conftest.py but it is rare)

Mostly, you don;t need to use 'global'. Please, avoid it.

You can define 'test_group_id = 0' here

Actually, put it in lib389/idm/nsrole.py, and then see my comments about how to maek this composable. I think he opened a ticket for this already ...

rebased onto be4ab754b04d891e8a81424a7c24ff2954eefe06

5 years ago

rebased onto de262735acae1cf0a110200c7a17b898b9356e36

5 years ago

rebased onto e55cbea2dabecb81ce955d6e5ca4b0d3a120d473

5 years ago

Just made some modification , it has dependency on lib389/idm/nsrole.py which yet to merge .

@aborah I will review this next week.

rebased onto 98290bfaf56539eac5f75a46e4827e9d35de9fbb

5 years ago

rebased onto 2beb992ecf68b64b13e70bf7bb80b59416af772a

5 years ago

rebased onto 81a96ef04cbee96229fa4530fdbf0e23a6a27941

5 years ago

rebased onto f9f377bffdfaaa9e0a13264ab2ce791821b553b5

5 years ago

rebased onto eaaded34e57b779590bbae59dfd28bb06fda2e65

5 years ago

rebased onto b9572293f714b771fd9fb18b3ad6ba326481cba1

5 years ago

rebased onto 4aa2c5a40c42e9871560260caa3c8f163ad25d4a

5 years ago

rebased onto df239a2ea1f10c4ccd025b429035da5e0cc2329a

5 years ago

rebased onto c38d20737b2e3083f46fe3efa10b517f83d0a069

5 years ago

rebased onto 6322342059df14e9cf8ace4959252b07a6b21936

5 years ago

@firstyear , this one is ready , all changes are done as per your suggestion

This is probably better in a proper cos.py file elsewhere in lib389 for re-use.

When properties are really small like this, you may be better with:

ou = OrganisationalUnit(...)
ou.create(properties={'ou': 'sales'}

A good test: if you need to use the value more than once, you should make it a variable. If it's only used once, in one place, just inline it to the functional call.

Something to make this morereadable, is:

nsContainer(topo.standalone,
    'cn=.....'.format(...))
    .create(properties={'cn': 'cosTemplates'})

If you only use ids_positive in this one place, why not inline the array here?

Can you also comment these more? I have very little understanding "reading" this of what you are achiving by this parameterisation.

This is probably better in a proper cos.py file elsewhere in lib389 for re-use.

https://pagure.io/389-ds-base/pull-request/50228

When properties are really small like this, you may be better with:
ou = OrganisationalUnit(...)
ou.create(properties={'ou': 'sales'}

A good test: if you need to use the value more than once, you should make it a variable. If it's only used once, in one place, just inline it to the functional call.

Both are different

ou = OrganizationalUnit(topo.standalone, "ou=eng,o=acivattr,{}".format(DEFAULT_SUFFIX))
ou.create(properties={'ou': 'eng'})

ou = OrganizationalUnit(topo.standalone, "ou=sales,o=acivattr,{}".format(DEFAULT_SUFFIX))
ou.create(properties={'ou': 'sales'})

rebased onto 0fbc38d7843b593433e87c97d9e6e1298723a9ce

5 years ago

@firstyear all other changes are done as per your suggestion

rebased onto 0b2088556b5ffd7ac02233ef612f006cbc02e2fe

5 years ago

Great, please rebase to master and I will merge.

rebased onto 39386bf7a57eb0bd5e05127a41daa8a94478cc62

5 years ago

rebased onto ddf79e6

5 years ago

Pull-Request has been merged by firstyear

5 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3172

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago
Metadata