From 0536984f7b3e9d6e143936b0eda92b510f63d304 Mon Sep 17 00:00:00 2001 From: Mark Reynolds 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