From 0536984f7b3e9d6e143936b0eda92b510f63d304 Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

Date: Tue, 4 Aug 2015 12:15:31 -0400

Subject: [PATCH 33/39] Ticket 47810 - memberOf plugin not properly rejecting

 updates

Bug Description:  When the memberOf plugin tries to add memberOf attribute to

                  an entry during a mod-replace on a group, even though the

                  update to the user entry fails, but plugin still allows

                  the member to be added to the group.

Fix Description:  During a mod/replace check and return an error if the member

                  update fails.

https://fedorahosted.org/389/ticket/47810

Reviewed by: nhosoi(Thanks!)

(cherry picked from commit eb54f03e240402a4bd16f9cde1d66539805f56ea)

(cherry picked from commit b4b6adcec7d810c7893fd9cb888fa906b9ffa836)

---

 dirsrvtests/suites/betxns/betxn_test.py  | 64 +++++++++++++++++++++++++++++++-

 ldap/servers/plugins/memberof/memberof.c | 13 ++++---

 2 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/dirsrvtests/suites/betxns/betxn_test.py b/dirsrvtests/suites/betxns/betxn_test.py

index 93c4c31..5da6e50 100644

--- a/dirsrvtests/suites/betxns/betxn_test.py

+++ b/dirsrvtests/suites/betxns/betxn_test.py

@@ -3,7 +3,7 @@

 # All rights reserved.

 #

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

-# See LICENSE for details. 

+# See LICENSE for details.

 # --- END COPYRIGHT BLOCK ---

 #

 import os

@@ -174,6 +174,67 @@ def test_betxn_attr_uniqueness(topology):

     log.info('test_betxn_attr_uniqueness: PASSED')

+def test_betxn_memberof(topology):

+    ENTRY1_DN = 'cn=group1,' + DEFAULT_SUFFIX

+    ENTRY2_DN = 'cn=group2,' + DEFAULT_SUFFIX

+    PLUGIN_DN = 'cn=' + PLUGIN_MEMBER_OF + ',cn=plugins,cn=config'

+

+    # Enable and configure memberOf plugin

+    topology.standalone.plugins.enable(name=PLUGIN_MEMBER_OF)

+    try:

+        topology.standalone.modify_s(PLUGIN_DN, [(ldap.MOD_REPLACE, 'memberofgroupattr', 'member')])

+    except ldap.LDAPError, e:

+        log.fatal('test_betxn_memberof: Failed to update config(member): error ' + e.message['desc'])

+        assert False

+

+    # Add our test entries

+    try:

+        topology.standalone.add_s(Entry((ENTRY1_DN, {'objectclass': "top groupofnames".split(),

+                                     'cn': 'group1'})))

+    except ldap.LDAPError, e:

+        log.error('test_betxn_memberof: Failed to add group1:' +

+                  ENTRY1_DN + ', error ' + e.message['desc'])

+        assert False

+

+    try:

+        topology.standalone.add_s(Entry((ENTRY2_DN, {'objectclass': "top groupofnames".split(),

+                                     'cn': 'group1'})))

+    except ldap.LDAPError, e:

+        log.error('test_betxn_memberof: Failed to add group2:' +

+                  ENTRY2_DN + ', error ' + e.message['desc'])

+        assert False

+

+    #

+    # Test mod replace

+    #

+

+    # Add group2 to group1 - it should fail with objectclass violation

+    try:

+        topology.standalone.modify_s(ENTRY1_DN, [(ldap.MOD_REPLACE, 'member', ENTRY2_DN)])

+        log.fatal('test_betxn_memberof: Group2 was incorrectly allowed to be added to group1')

+        assert False

+    except ldap.LDAPError, e:

+        log.info('test_betxn_memberof: Group2 was correctly rejected (mod replace): error ' + e.message['desc'])

+

+    #

+    # Test mod add

+    #

+

+    # Add group2 to group1 - it should fail with objectclass violation

+    try:

+        topology.standalone.modify_s(ENTRY1_DN, [(ldap.MOD_ADD, 'member', ENTRY2_DN)])

+        log.fatal('test_betxn_memberof: Group2 was incorrectly allowed to be added to group1')

+        assert False

+    except ldap.LDAPError, e:

+        log.info('test_betxn_memberof: Group2 was correctly rejected (mod add): error ' + e.message['desc'])

+

+    #

+    # Done

+    #

+

+    log.info('test_betxn_memberof: PASSED')

+

+

 def test_betxn_final(topology):

     topology.standalone.delete()

     log.info('betxn test suite PASSED')

@@ -187,6 +248,7 @@ def run_isolated():

     test_betxn_init(topo)

     test_betxt_7bit(topo)

     test_betxn_attr_uniqueness(topo)

+    test_betxn_memberof(topo)

     test_betxn_final(topo)

diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c

index 144285b..da52bc8 100644

--- a/ldap/servers/plugins/memberof/memberof.c

+++ b/ldap/servers/plugins/memberof/memberof.c

@@ -2373,6 +2373,7 @@ memberof_replace_list(Slapi_PBlock *pb, MemberOfConfig *config,

 	struct slapi_entry *post_e = NULL;

 	Slapi_Attr *pre_attr = 0;

 	Slapi_Attr *post_attr = 0;

+	int rc = 0;

 	int i = 0;

 	slapi_pblock_get( pb, SLAPI_ENTRY_PRE_OP, &pre_e );

@@ -2449,14 +2450,14 @@ memberof_replace_list(Slapi_PBlock *pb, MemberOfConfig *config,

 				in pre, not in post, delete from entry

 				not in pre, in post, add to entry

 			*/

-			while(pre_index < pre_total || post_index < post_total)

+			while(rc == 0 && (pre_index < pre_total || post_index < post_total))

 			{

 				if(pre_index == pre_total)

 				{

 					/* add the rest of post */

 					slapi_sdn_set_normdn_byref(sdn,

 					            slapi_value_get_string(post_array[post_index]));

-					memberof_add_one(pb, config, group_sdn, sdn);

+					rc = memberof_add_one(pb, config, group_sdn, sdn);

 					post_index++;

 				}

@@ -2465,7 +2466,7 @@ memberof_replace_list(Slapi_PBlock *pb, MemberOfConfig *config,

 					/* delete the rest of pre */

 					slapi_sdn_set_normdn_byref(sdn,

 					            slapi_value_get_string(pre_array[pre_index]));

-					memberof_del_one(pb, config, group_sdn, sdn);

+					rc = memberof_del_one(pb, config, group_sdn, sdn);

 					pre_index++;

 				}

@@ -2482,7 +2483,7 @@ memberof_replace_list(Slapi_PBlock *pb, MemberOfConfig *config,

 						/* delete pre array */

 						slapi_sdn_set_normdn_byref(sdn,

 					            slapi_value_get_string(pre_array[pre_index]));

-						memberof_del_one(pb, config, group_sdn, sdn);

+						rc = memberof_del_one(pb, config, group_sdn, sdn);

 						pre_index++;

 					}

@@ -2491,7 +2492,7 @@ memberof_replace_list(Slapi_PBlock *pb, MemberOfConfig *config,

 						/* add post array */

 						slapi_sdn_set_normdn_byref(sdn,

 					            slapi_value_get_string(post_array[post_index]));

-						memberof_add_one(pb, config, group_sdn, sdn);

+						rc = memberof_add_one(pb, config, group_sdn, sdn);

 						post_index++;

 					}

@@ -2509,7 +2510,7 @@ memberof_replace_list(Slapi_PBlock *pb, MemberOfConfig *config,

 		}

 	}

-	return 0;

+	return rc;

 }

 /* memberof_load_array()

-- 

1.9.3