#50314 Issue: 50313 - Making an NestedRoleDefinition type in src/lib389/lib389/idm/role.py
Closed 2 years ago by spichugi. Opened 3 years ago by aborah.
aborah/389-ds-base nested  into  master

@@ -6,13 +6,20 @@ 

  # See LICENSE for details.

  # --- END COPYRIGHT BLOCK ---

  

- import pytest, os

+ 

+ """

+ Importing necessary Modules.

+ """

+ 

+ import os

+ import pytest

+ 

  from lib389._constants import PW_DM, DEFAULT_SUFFIX

  from lib389.idm.user import UserAccount, UserAccounts

  from lib389.idm.organization import Organization

  from lib389.idm.organizationalunit import OrganizationalUnit

  from lib389.topologies import topology_st as topo

- from lib389.idm.role import FilterRoles, ManagedRole, ManagedRoles

+ from lib389.idm.role import FilterRoles, ManagedRoles, NestedRoles

  from lib389.idm.domain import Domain

  

  
@@ -45,11 +52,11 @@ 

          'ou': 'eng',

      }

  

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

-     ou.create(properties=properties)

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

+     ou_ou.create(properties=properties)

      properties = {'ou': 'sales'}

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

-     ou.create(properties=properties)

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

+     ou_ou.create(properties=properties)

  

      roles = FilterRoles(topo.standalone, DNBASE)

      roles.create(properties={'cn': 'FILTERROLEENGROLE', 'nsRoleFilter': 'cn=eng*'})
@@ -64,7 +71,8 @@ 

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

          'userPassword': PW_DM

      }

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

+     user = UserAccount(topo.standalone,

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

      user.create(properties=properties)

  

      properties = {
@@ -76,7 +84,8 @@ 

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

          'userPassword': PW_DM,

      }

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

+     user = UserAccount(topo.standalone,

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

      user.create(properties=properties)

  

      properties = {
@@ -88,7 +97,8 @@ 

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

          'userPassword': PW_DM

      }

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

+     user = UserAccount(topo.standalone,

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

      user.create(properties=properties)

  

      properties = {
@@ -100,24 +110,29 @@ 

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

          'userPassword': PW_DM

      }

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

+     user = UserAccount(topo.standalone,

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

      user.create(properties=properties)

  

-     #user with cn=sales* will automatically memeber of nsfilterrole cn=filterrolesalesrole,o=acivattr,dc=example,dc=com

-     assert UserAccount(topo.standalone, 'cn=salesuser1,ou=sales,o=acivattr,dc=example,dc=com').get_attr_val_utf8(

-         'nsrole') == 'cn=filterrolesalesrole,o=acivattr,dc=example,dc=com'

+     # user with cn=sales* will automatically memeber of nsfilterrole

+     # cn=filterrolesalesrole,o=acivattr,dc=example,dc=com

+     assert UserAccount(topo.standalone,

+                        'cn=salesuser1,ou=sales,o=acivattr,dc=example,dc=com').\

+                get_attr_val_utf8('nsrole') == 'cn=filterrolesalesrole,o=acivattr,dc=example,dc=com'

      # same goes to SALES_MANAGER

      assert UserAccount(topo.standalone, SALES_MANAGER).get_attr_val_utf8(

          'nsrole') == 'cn=filterrolesalesrole,o=acivattr,dc=example,dc=com'

-     # user with cn=eng* will automatically memeber of nsfilterrole cn=filterroleengrole,o=acivattr,dc=example,dc=com

-     assert UserAccount(topo.standalone, 'cn=enguser1,ou=eng,o=acivattr,dc=example,dc=com').get_attr_val_utf8(

-         'nsrole') == 'cn=filterroleengrole,o=acivattr,dc=example,dc=com'

+     # user with cn=eng* will automatically memeber of nsfilterrole

+     # cn=filterroleengrole,o=acivattr,dc=example,dc=com

+     assert UserAccount(topo.standalone, 'cn=enguser1,ou=eng,o=acivattr,dc=example,dc=com').\

+                get_attr_val_utf8('nsrole') == 'cn=filterroleengrole,o=acivattr,dc=example,dc=com'

      # same goes to ENG_MANAGER

      assert UserAccount(topo.standalone, ENG_MANAGER).get_attr_val_utf8(

          'nsrole') == 'cn=filterroleengrole,o=acivattr,dc=example,dc=com'

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

-                SALES_OU, DNBASE]:

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

+     for dn_dn in [ENG_USER, SALES_UESER, ENG_MANAGER, SALES_MANAGER,

+                   FILTERROLESALESROLE, FILTERROLEENGROLE, ENG_OU,

+                   SALES_OU, DNBASE]:

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

  

  

  def test_managedrole(topo):
@@ -140,14 +155,14 @@ 

      # Create user and Assign the role to the entry

      uas = UserAccounts(topo.standalone, DEFAULT_SUFFIX, rdn=None)

      uas.create(properties={

-             'uid': 'Fail',

-             'cn': 'Fail',

-             'sn': 'user',

-             'uidNumber': '1000',

-             'gidNumber': '2000',

-             'homeDirectory': '/home/' + 'Fail',

-             'nsRoleDN': role.dn,

-             'userPassword': PW_DM

+         'uid': 'Fail',

+         'cn': 'Fail',

+         'sn': 'user',

+         'uidNumber': '1000',

+         'gidNumber': '2000',

+         'homeDirectory': '/home/' + 'Fail',

+         'nsRoleDN': role.dn,

+         'userPassword': PW_DM

          })

  

      # Create user and do not Assign any role to the entry
@@ -163,20 +178,23 @@ 

          })

  

      # Assert that Manage role entry is created and its searchable

-     assert ManagedRoles(topo.standalone, DEFAULT_SUFFIX).list()[0].dn == 'cn=ROLE1,dc=example,dc=com'

+     assert ManagedRoles(topo.standalone, DEFAULT_SUFFIX).list()[0].dn \

+            == 'cn=ROLE1,dc=example,dc=com'

  

      # Set an aci that will deny  ROLE1 manage role

-     Domain(topo.standalone, DEFAULT_SUFFIX).add('aci', '(targetattr=*)(version 3.0; aci "role aci"; deny(all) roledn="ldap:///{}";)'.format(role.dn),)

+     Domain(topo.standalone, DEFAULT_SUFFIX).\

+         add('aci', '(targetattr=*)(version 3.0; aci "role aci";'

+                    ' deny(all) roledn="ldap:///{}";)'.format(role.dn),)

  

      # Crate a connection with cn=Fail which is member of ROLE1

      conn = UserAccount(topo.standalone, "uid=Fail,{}".format(DEFAULT_SUFFIX)).bind(PW_DM)

      # Access denied to ROLE1 members

-     assert 0 == len(ManagedRoles(conn, DEFAULT_SUFFIX).list())

+     assert not ManagedRoles(conn, DEFAULT_SUFFIX).list()

  

      # Now create a connection with cn=Success which is not a member of ROLE1

      conn = UserAccount(topo.standalone, "uid=Success,{}".format(DEFAULT_SUFFIX)).bind(PW_DM)

      # Access allowed here

-     assert 1 == len(ManagedRoles(conn, DEFAULT_SUFFIX).list())

+     assert ManagedRoles(conn, DEFAULT_SUFFIX).list()

  

      for i in uas.list():

          i.delete()
@@ -185,6 +203,92 @@ 

          i.delete()

  

  

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

+ def _final(request, topo):

+     """

+     Removes and Restores ACIs after the test.

+     """

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

+ 

+     def finofaci():

+         """

+         Removes and Restores ACIs and other users after the test.

+         """

+         domain = Domain(topo.standalone, DEFAULT_SUFFIX)

+         domain.remove_all('aci')

+ 

+         managed_roles = ManagedRoles(topo.standalone, DEFAULT_SUFFIX)

+         nested_roles = NestedRoles(topo.standalone, DEFAULT_SUFFIX)

+         users = UserAccounts(topo.standalone, DEFAULT_SUFFIX)

+ 

+         for i in managed_roles.list() + nested_roles.list() + users.list():

+             i.delete()

+ 

+         for i in aci_list:

+             domain.add("aci", i)

+ 

+     request.addfinalizer(finofaci)

+ 

+ 

+ def test_nestedrole(topo, _final):

+     """

+         :id: d52a9cw0-3bg6-11e9-9b7b-8c16451d917t

+         :setup: Standalone server

+         :steps:

+             1. Add test entry

+             2. Add ACI

+             3. Search managed role entries

+         :expectedresults:

+             1. Entry should be added

+             2. Operation should  succeed

+             3. Operation should  succeed

+     """

+     # Create Managed role entry

+     managed_roles = ManagedRoles(topo.standalone, DEFAULT_SUFFIX)

+     managed_role1 = managed_roles.create(properties={"cn": 'managed_role1'})

+     managed_role2 = managed_roles.create(properties={"cn": 'managed_role2'})

+ 

+     # Create nested role entry

+     nested_roles = NestedRoles(topo.standalone, DEFAULT_SUFFIX)

+     nested_role = nested_roles.create(properties={"cn": 'nested_role',

+                                                   "nsRoleDN": [managed_role1.dn, managed_role2.dn]})

+ 

+     # Create user and assign managed role to it

+     users = UserAccounts(topo.standalone, DEFAULT_SUFFIX)

+     user1 = users.create_test_user(uid=1, gid=1)

+     user1.set('nsRoleDN', managed_role1.dn)

+     user1.set('userPassword', PW_DM)

+ 

+     # Create another user and assign managed role to it

+     user2 = users.create_test_user(uid=2, gid=2)

+     user2.set('nsRoleDN', managed_role2.dn)

+     user2.set('userPassword', PW_DM)

+ 

+     # Create another user and do not assign any role to it

+     user3 = users.create_test_user(uid=3, gid=3)

+     user3.set('userPassword', PW_DM)

+ 

+     # Create a ACI with deny access to nested role entry

+     Domain(topo.standalone, DEFAULT_SUFFIX).\

+         add('aci', f'(targetattr=*)(version 3.0; aci '

+                    f'"role aci"; deny(all) roledn="ldap:///{nested_role.dn}";)')

+ 

+     # Create connection with 'uid=test_user_1,ou=People,dc=example,dc=com' member of managed_role1

+     # and search while bound as the user

+     conn = users.get('test_user_1').bind(PW_DM)

+     assert not UserAccounts(conn, DEFAULT_SUFFIX).list()

+ 

+     # Create connection with 'uid=test_user_2,ou=People,dc=example,dc=com' member of managed_role2

+     # and search while bound as the user

+     conn = users.get('test_user_2').bind(PW_DM)

+     assert not UserAccounts(conn, DEFAULT_SUFFIX).list()

+ 

+     # Create connection with 'uid=test_user_3,ou=People,dc=example,dc=com' and

+     # search while bound as the user

+     conn = users.get('test_user_3').bind(PW_DM)

+     assert UserAccounts(conn, DEFAULT_SUFFIX).list()

+ 

+ 

  if __name__ == "__main__":

      CURRENT_FILE = os.path.realpath(__file__)

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

file modified
+49 -1
@@ -112,4 +112,52 @@ 

          ]

          self._filterattrs = ['cn']

          self._basedn = basedn

-         self._childobject = ManagedRole 

\ No newline at end of file

+         self._childobject = ManagedRole

+ 

+ 

+ class NestedRole(DSLdapObject):

+     """A single instance of NestedRole entry to create NestedRole role.

+ 

+         :param instance: An instance

+         :type instance: lib389.DirSrv

+         :param dn: Entry DN

+         :type dn: str

+ 

+     """

+     def __init__(self, instance, dn=None):

+         super(NestedRole, self).__init__(instance, dn)

+         self._must_attributes = ['cn', 'nsRoleDN']

+         self._rdn_attribute = 'cn'

+         self._create_objectclasses = [

+             'top',

+             'nsRoleDefinition',

+             'nsComplexRoleDefinition',

+             'ldapSubEntry',

+             'nsNestedRoleDefinition'

+         ]

+ 

+ 

+ class NestedRoles(DSLdapObjects):

+     """DSLdapObjects that represents all NestedRoles entries in suffix.

+ 

+         This instance is used mainly for search operation  NestedRoles role

+ 

+         :param instance: An instance

+         :type instance: lib389.DirSrv

+         :param basedn: Suffix DN

+         :type basedn: str

+         :param rdn: The DN that will be combined wit basedn

+         :type rdn: str

+         """

+     def __init__(self, instance, basedn):

+         super(NestedRoles, self).__init__(instance)

+         self._objectclasses = [

+             'top',

+             'nsRoleDefinition',

+             'nsComplexRoleDefinition',

+             'ldapSubEntry',

+             'nsNestedRoleDefinition'

+         ]

+         self._filterattrs = ['cn']

+         self._basedn = basedn

+         self._childobject = NestedRole

Making an NestedRoleDefinition type in src/lib389/lib389/idm/role.py

https://pagure.io/389-ds-base/issue/50313

Reviewed by: ???

Looks like we also need LDAPsubentry objectClass here.

If you are mentioning the objectClass here - it should be nsNestedRoleDefinition

We also need 'self._must_attributes' here which should contain nsRoleDN and cn

Typo. And variable should be lowercase. Please, run pytest-pylint here, also

Instead of UserAccount(topo.standalone, f'uid=test_user_1,ou=People,{DEFAULT_SUFFIX}') you can use existing UserAccounts instance and get users from it by selector.

The goal here is not to create a connection but to search while bound as the user

Looks like we also need LDAPsubentry objectClass here.

That will not be required , as it will come by default

If you are mentioning the objectClass here - it should be nsNestedRoleDefinition

In previous reviews @tbordaz asked me to remove all the 'ns' (except from object classes ) , for this one i will change it to NestedRole

rebased onto b1450d43b64f7b372aa19170d846b90817a065bd

3 years ago

@spichugi , all other changes are done as per your suggestion

Please, add another attribute I've asked to add. The definition won't work without it.

I think it makes sense to join the lists and run delete() operation on all of them. It will be 2 lines instead of 6.

Issue: 50313 - Making an NestedRoleDefinition type - is an unneccessary long line, I think.
And follow the basic commit message rules, please.
It should be something like - Issue: 50313 - Add a NestedRole type to lib389

And the commit message body should have some info about the test you've added too.

Typo. And variable should be lowercase. Please, run pytest-pylint here, also

Please, don't ignore my comments. :) You still have a bunch of pylint issues.
The code will be much more maintainable if we follow basic lint rules.

Just notice this, I would expect a role definition to be a subentry. This applies to managed/filtered/nested roles.
Could you confirm that the 'objectclass: ldapsubentry' was missing in original role definitions ?

I think that once the test is completed, all items added by the test should be deleted (roles (managed and nested), users and ACI).
This deletion should occur if the test succeeds or not (so may be using a python directtive like 'finally')

Just notice this, I would expect a role definition to be a subentry. This applies to managed/filtered/nested roles.
Could you confirm that the 'objectclass: ldapsubentry' was missing in original role definitions ?

objectclass , ldapsubentry will come by default . No need to put differently

I think that once the test is completed, all items added by the test should be deleted (roles (managed and nested), users and ACI).
This deletion should occur if the test succeeds or not (so may be using a python directtive like 'finally')

I have added one fixture to clean up the (roles (managed and nested), users and ACI).

rebased onto 7220c724858c1c54b21cc8df00d1ae28cab5f4be

3 years ago

rebased onto abc70b1ffdeb18ba7f39846e61d34f157c8af62d

3 years ago

@spichugi , all changes are done as per your suggestion

@tbordaz , @spichugi , checkout the out put for ldapsubentry , its coming by default without putting it separately

(Pdb) NestedRoles(topo.standalone, DEFAULT_SUFFIX).list()
[<role.NestedRole object at 0x7f7b38225470>]
(Pdb) NestedRoles(topo.standalone, DEFAULT_SUFFIX).list()[0].dn
'cn=nested_role,dc=example,dc=com'
(Pdb) NestedRoles(topo.standalone, DEFAULT_SUFFIX).list()[0]._unsafe_raw_entry()
dn: cn=nested_role,dc=example,dc=com
cn: nested_role
objectClass: top
objectClass: nsRoleDefinition
objectClass: nsComplexRoleDefinition
objectClass: nsNestedRoleDefinition
objectClass: ldapSubEntry

Even though it is created automatically, we better have it as a part of the DSLdapObject and DSLdapObjects.
The explicitness will ensure that everything is in order.

rebased onto c5c0ef2472d5d98eff838ace2e27f682388fd81a

3 years ago

Even though it is created automatically, we better have it as a part of the DSLdapObject and DSLdapObjects.
The explicitness will ensure that everything is in order. '

Done

@spichugi , just to be sure. Did ldapsubentry come from DSLdapObject or DSLdapObjects ?

@spichugi , just to be sure. Did ldapsubentry come from DSLdapObject or DSLdapObjects ?

DSLdapObject._create_objectclasses sets the objectClasses that will be created in a new entry
DSLdapObjects._objectclasses sets the objectClasses that will be used as a filter for the searches.

The commit message body should follow the same consistent imperative rule as the subject. And the class name should be consistent too. Something like this:

Add the NestedRole and the NestedRoles classes to src/lib389/lib389/idm/role.py
Add one test case that will test that the new class NestedRoles is
working fine.

rebased onto 9724e8b

3 years ago

@spichugi , all changes are done as per your suggestion

Pull-Request has been merged by spichugi

3 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/3373

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

2 years ago