#51232 Issue 50260 - Fix test according to #51222 fix
Closed 2 years ago by spichugi. Opened 2 years ago by spichugi.
spichugi/389-ds-base i50260  into  master

@@ -18,12 +18,14 @@ 

  from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES

  from lib389.idm.organizationalunit import OrganizationalUnits

  from lib389.idm.group import Groups, Group

+ from lib389.idm.domain import Domain

  from lib389._constants import DEFAULT_SUFFIX

  

  pytestmark = pytest.mark.tier1

  

  logging.getLogger(__name__).setLevel(logging.DEBUG)

  log = logging.getLogger(__name__)

+ USER_PASSWORD = 'password'

  

  

  def test_betxt_7bit(topology_st):
@@ -253,6 +255,15 @@ 

              5. Success

  

      """

+     # Add ACI so we can test that non-DM user can't delete managed entry

+     domain = Domain(topology_st.standalone, DEFAULT_SUFFIX)

+     ACI_TARGET = f"(target = \"ldap:///{DEFAULT_SUFFIX}\")"

+     ACI_TARGETATTR = "(targetattr = *)"

+     ACI_ALLOW = "(version 3.0; acl \"Admin Access\"; allow (all) "

+     ACI_SUBJECT = "(userdn = \"ldap:///anyone\");)"

+     ACI_BODY = ACI_TARGET + ACI_TARGETATTR + ACI_ALLOW + ACI_SUBJECT

+     domain.add('aci', ACI_BODY)

+ 

      # Start plugins

      topology_st.standalone.config.set('nsslapd-dynamic-plugins', 'on')

      mep_plugin = ManagedEntriesPlugin(topology_st.standalone)
@@ -270,15 +281,15 @@ 

      mep_template1 = mep_templates.create(properties={

          'cn': 'MEP template',

          'mepRDNAttr': 'cn',

-         'mepStaticAttr': 'objectclass: posixGroup|objectclass: extensibleObject'.split('|'),

+         'mepStaticAttr': 'objectclass: groupOfNames|objectclass: extensibleObject'.split('|'),

          'mepMappedAttr': 'cn: $cn|uid: $cn|gidNumber: $uidNumber'.split('|')

      })

      mep_configs = MEPConfigs(topology_st.standalone)

      mep_configs.create(properties={'cn': 'config',

-                                                 'originScope': ou_people.dn,

-                                                 'originFilter': 'objectclass=posixAccount',

-                                                 'managedBase': ou_groups.dn,

-                                                 'managedTemplate': mep_template1.dn})

+                                    'originScope': ou_people.dn,

+                                    'originFilter': 'objectclass=posixAccount',

+                                    'managedBase': ou_groups.dn,

+                                    'managedTemplate': mep_template1.dn})

  

      # Add an entry that meets the MEP scope

      users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX,
@@ -291,6 +302,8 @@ 

          'gidNumber': '20011',

          'homeDirectory': '/home/test-user1'

      })

+     user.reset_password(USER_PASSWORD)

+     user_bound_conn = user.bind(USER_PASSWORD)

  

      # Add group

      groups = Groups(topology_st.standalone, DEFAULT_SUFFIX)
@@ -304,22 +317,25 @@ 

  

      # Test MEP be txn pre op failure does not corrupt entry cache

      # Should get the same exception for both rename attempts

+     # Try to remove the entry while bound as Admin (non-DM)

+     managed_groups_user_conn = Groups(user_bound_conn, ou_groups.dn, rdn=None)

+     managed_entry_user_conn = managed_groups_user_conn.get(user.rdn)

      with pytest.raises(ldap.UNWILLING_TO_PERFORM):

-         mep_group.rename("cn=modrdn group")

- 

+         managed_entry_user_conn.rename("cn=modrdn group")

      with pytest.raises(ldap.UNWILLING_TO_PERFORM):

-         mep_group.rename("cn=modrdn group")

+         managed_entry_user_conn.rename("cn=modrdn group")

  

      # Mess with MEP so it fails

      mep_plugin.disable()

-     mep_group.delete()

+     users_mep_group = UserAccounts(topology_st.standalone, mep_group.dn, rdn=None)

+     users_mep_group.create_test_user(1001)

      mep_plugin.enable()

  

      # Add another group to verify entry cache is not corrupted

      test_group = groups.create(properties={'cn': 'test_group'})

  

-     # Delete user, should fail in MEP be txn post op, and user should still be a member

-     with pytest.raises(ldap.NO_SUCH_OBJECT):

+     # Try to delete user - it fails because managed entry can't be deleted

+     with pytest.raises(ldap.NOT_ALLOWED_ON_NONLEAF):

          user.delete()

  

      # Verify membership is intact

Description: Managed Entry plugin behaviour was fixed and
returned codes were cleaned up. Now we allow to continue
modrdn and delete managing entry operations execution
even when managed entry doesn't exists.
Also allow 'cn=directory manager' to delete managed entry
on direct update.

https://pagure.io/389-ds-base/issue/50260
https://pagure.io/389-ds-base/issue/51222

Reviewed by: ?

LGTM

Actually, I'm not sure about this. It looks like the behavior of MEP was changed. In this test, what were failures and now expected to pass. So it is probably not testing entry cache corruption :-( I thought your fix was to reject deletes of MEP entries, but in this test now expects failures to be successes - that does not make sense to me. In order to test the entry cache corruption you need updates to fail, then you need to make sure that the updates done by the be txn plugins (before the failure) are not present in the cache. Since the operations are passing I don't think you are testing this correctly. I need to look at this closer I guess...

1 new commit added

  • Make the managing entry removal fail
2 years ago

2 new commits added

  • Make the managing entry removal fail
  • Issue 50260 - Fix test according to #51222 fix
2 years ago

LGTM

Actually, I'm not sure about this. It looks like the behavior of MEP was changed. In this test, what were failures and now expected to pass. So it is probably not testing entry cache corruption :-( I thought your fix was to reject deletes of MEP entries, but in this test now expects failures to be successes - that does not make sense to me. In order to test the entry cache corruption you need updates to fail, then you need to make sure that the updates done by the be txn plugins (before the failure) are not present in the cache. Since the operations are passing I don't think you are testing this correctly. I need to look at this closer I guess...

Right... I've changed the way it fails.
Will it work?

rebased onto 066a7b4

2 years ago

Pull-Request has been merged by spichugi

2 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/4285

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
Metadata