dirsrvtests/tickets/ticket47963_test.py | 191 +++++++++++++++++++++++++++++++
ldap/servers/plugins/memberof/memberof.c | 47 +++----
2 files changed, 214 insertions(+), 24 deletions(-)
New commits:
commit 248b471c86dbd7ed6a86e9def90a0d7a9858ed91
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Tue Feb 3 12:52:40 2015 -0500
Ticket 47963 - memberof skip nested groups breaks the plugin
Bug Description: The previous patch broke the memberOf plugin - it
basically wouldn't do anything. The skip was being
done too early.
Fix Description: Move the "recursion skip" to the appropriate location.
https://fedorahosted.org/389/ticket/47963
Reviewed by: nhosoi(Thanks!)
diff --git a/dirsrvtests/tickets/ticket47963_test.py
b/dirsrvtests/tickets/ticket47963_test.py
new file mode 100644
index 0000000..7d0afeb
--- /dev/null
+++ b/dirsrvtests/tickets/ticket47963_test.py
@@ -0,0 +1,191 @@
+import os
+import sys
+import time
+import ldap
+import logging
+import pytest
+from lib389 import DirSrv, Entry, tools, tasks
+from lib389.tools import DirSrvTools
+from lib389._constants import *
+from lib389.properties import *
+from lib389.tasks import *
+
+logging.getLogger(__name__).setLevel(logging.DEBUG)
+log = logging.getLogger(__name__)
+
+installation1_prefix = None
+
+
+class TopologyStandalone(object):
+ def __init__(self, standalone):
+ standalone.open()
+ self.standalone = standalone
+
+
+(a)pytest.fixture(scope="module")
+def topology(request):
+ global installation1_prefix
+ if installation1_prefix:
+ args_instance[SER_DEPLOYED_DIR] = installation1_prefix
+
+ # Creating standalone instance ...
+ standalone = DirSrv(verbose=False)
+ args_instance[SER_HOST] = HOST_STANDALONE
+ args_instance[SER_PORT] = PORT_STANDALONE
+ args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE
+ args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX
+ args_standalone = args_instance.copy()
+ standalone.allocate(args_standalone)
+ instance_standalone = standalone.exists()
+ if instance_standalone:
+ standalone.delete()
+ standalone.create()
+ standalone.open()
+
+ # Clear out the tmp dir
+ standalone.clearTmpDir(__file__)
+
+ return TopologyStandalone(standalone)
+
+
+def test_ticket47963(topology):
+ '''
+ Test that the memberOf plugin works correctly after setting:
+
+ memberofskipnested: on
+
+ '''
+ PLUGIN_DN = 'cn=' + PLUGIN_MEMBER_OF + ',cn=plugins,cn=config'
+ USER_DN = 'uid=test_user,' + DEFAULT_SUFFIX
+ GROUP_DN1 = 'cn=group1,' + DEFAULT_SUFFIX
+ GROUP_DN2 = 'cn=group2,' + DEFAULT_SUFFIX
+ GROUP_DN3 = 'cn=group3,' + DEFAULT_SUFFIX
+
+ #
+ # Enable the plugin and configure the skiop nest attribute, then restart the server
+ #
+ topology.standalone.plugins.enable(name=PLUGIN_MEMBER_OF)
+ try:
+ topology.standalone.modify_s(PLUGIN_DN, [(ldap.MOD_REPLACE,
'memberofskipnested', 'on')])
+ except ldap.LDAPError, e:
+ log.error('test_automember: Failed to modify config entry: error ' +
e.message['desc'])
+ assert False
+
+ topology.standalone.restart(timeout=10)
+
+ #
+ # Add our groups, users, memberships, etc
+ #
+ try:
+ topology.standalone.add_s(Entry((USER_DN, {
+ 'objectclass': 'top extensibleObject'.split(),
+ 'uid': 'test_user'
+ })))
+ except ldap.LDAPError, e:
+ log.error('Failed to add teset user: error ' +
e.message['desc'])
+ assert False
+
+ try:
+ topology.standalone.add_s(Entry((GROUP_DN1, {
+ 'objectclass': 'top groupOfNames groupOfUniqueNames
extensibleObject'.split(),
+ 'cn': 'group1',
+ 'member': USER_DN
+ })))
+ except ldap.LDAPError, e:
+ log.error('Failed to add group1: error ' + e.message['desc'])
+ assert False
+
+ try:
+ topology.standalone.add_s(Entry((GROUP_DN2, {
+ 'objectclass': 'top groupOfNames groupOfUniqueNames
extensibleObject'.split(),
+ 'cn': 'group2',
+ 'member': USER_DN
+ })))
+ except ldap.LDAPError, e:
+ log.error('Failed to add group2: error ' + e.message['desc'])
+ assert False
+
+ # Add group with no member(yet)
+ try:
+ topology.standalone.add_s(Entry((GROUP_DN3, {
+ 'objectclass': 'top groupOfNames groupOfUniqueNames
extensibleObject'.split(),
+ 'cn': 'group'
+ })))
+ except ldap.LDAPError, e:
+ log.error('Failed to add group3: error ' + e.message['desc'])
+ assert False
+ time.sleep(1)
+
+ #
+ # Test we have the correct memberOf values in the user entry
+ #
+ try:
+ member_filter = ('(&(memberOf=' + GROUP_DN1 + ')(memberOf=' +
GROUP_DN2 + '))')
+ entries = topology.standalone.search_s(USER_DN, ldap.SCOPE_BASE, member_filter)
+ if not entries:
+ log.fatal('User is missing expected memberOf attrs')
+ assert False
+ except ldap.LDAPError, e:
+ log.fatal('Search for user1 failed: ' + e.message['desc'])
+ assert False
+
+ # Add the user to the group
+ try:
+ topology.standalone.modify_s(GROUP_DN3, [(ldap.MOD_ADD, 'member',
USER_DN)])
+ except ldap.LDAPError, e:
+ log.error('Failed to member to group: error ' +
e.message['desc'])
+ assert False
+ time.sleep(1)
+
+ # Check that the test user is a "memberOf" all three groups
+ try:
+ member_filter = ('(&(memberOf=' + GROUP_DN1 + ')(memberOf=' +
GROUP_DN2 +
+ ')(memberOf=' + GROUP_DN3 + '))')
+ entries = topology.standalone.search_s(USER_DN, ldap.SCOPE_BASE, member_filter)
+ if not entries:
+ log.fatal('User is missing expected memberOf attrs')
+ assert False
+ except ldap.LDAPError, e:
+ log.fatal('Search for user1 failed: ' + e.message['desc'])
+ assert False
+
+ #
+ # Delete group2, and check memberOf values in the user entry
+ #
+ try:
+ topology.standalone.delete_s(GROUP_DN2)
+ except ldap.LDAPError, e:
+ log.error('Failed to delete test group2: ' + e.message['desc'])
+ assert False
+ time.sleep(1)
+
+ try:
+ member_filter = ('(&(memberOf=' + GROUP_DN1 + ')(memberOf=' +
GROUP_DN3 + '))')
+ entries = topology.standalone.search_s(USER_DN, ldap.SCOPE_BASE, member_filter)
+ if not entries:
+ log.fatal('User incorrect memberOf attrs')
+ assert False
+ except ldap.LDAPError, e:
+ log.fatal('Search for user1 failed: ' + e.message['desc'])
+ assert False
+
+ log.info('Test complete')
+
+
+def test_ticket47963_final(topology):
+ topology.standalone.delete()
+ log.info('Testcase PASSED')
+
+
+def run_isolated():
+ global installation1_prefix
+ installation1_prefix = None
+
+ topo = topology(True)
+ test_ticket47963(topo)
+ test_ticket47963_final(topo)
+
+
+if __name__ == '__main__':
+ run_isolated()
+
diff --git a/ldap/servers/plugins/memberof/memberof.c
b/ldap/servers/plugins/memberof/memberof.c
index b8e2386..27ccedf 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -691,7 +691,7 @@ memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,
char *cookie = NULL;
int all_backends = memberof_config_get_all_backends();
Slapi_DN *entry_scope = memberof_config_get_entry_scope();
- Slapi_DN *entry_scope_exclude_subtree =
memberof_config_get_entry_scope_exclude_subtree();
+ Slapi_DN *entry_scope_exclude_subtree =
memberof_config_get_entry_scope_exclude_subtree();
int types_name_len = 0;
int num_types = 0;
int dn_len = slapi_sdn_get_ndn_len(sdn);
@@ -703,7 +703,7 @@ memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,
return (rc);
}
- if (entry_scope_exclude_subtree && slapi_sdn_issuffix(sdn,
entry_scope_exclude_subtree)) {
+ if (entry_scope_exclude_subtree && slapi_sdn_issuffix(sdn,
entry_scope_exclude_subtree)) {
return (rc);
}
@@ -2024,7 +2024,7 @@ Slapi_ValueSet *
memberof_get_groups(MemberOfConfig *config, Slapi_DN *member_sdn)
{
Slapi_ValueSet *groupvals = slapi_valueset_new();
- Slapi_ValueSet *group_norm_vals = slapi_valueset_new();
+ Slapi_ValueSet *group_norm_vals = slapi_valueset_new();
Slapi_Value *memberdn_val =
slapi_value_new_string(slapi_sdn_get_ndn(member_sdn));
slapi_value_set_flags(memberdn_val, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
@@ -2034,7 +2034,7 @@ memberof_get_groups(MemberOfConfig *config, Slapi_DN *member_sdn)
memberof_get_groups_r(config, member_sdn, &data);
slapi_value_free(&memberdn_val);
- slapi_valueset_free(group_norm_vals);
+ slapi_valueset_free(group_norm_vals);
return groupvals;
}
@@ -2057,12 +2057,13 @@ int memberof_get_groups_callback(Slapi_Entry *e, void
*callback_data)
{
Slapi_DN *group_sdn = slapi_entry_get_sdn(e);
char *group_ndn = slapi_entry_get_ndn(e);
- char *group_dn = slapi_entry_get_dn(e);
+ char *group_dn = slapi_entry_get_dn(e);
Slapi_Value *group_ndn_val = 0;
- Slapi_Value *group_dn_val = 0;
+ Slapi_Value *group_dn_val = 0;
Slapi_ValueSet *groupvals = *((memberof_get_groups_data*)callback_data)->groupvals;
- Slapi_ValueSet *group_norm_vals =
*((memberof_get_groups_data*)callback_data)->group_norm_vals;
- Slapi_DN *entry_scope_exclude_subtree =
memberof_config_get_entry_scope_exclude_subtree();
+ Slapi_ValueSet *group_norm_vals =
*((memberof_get_groups_data*)callback_data)->group_norm_vals;
+ Slapi_DN *entry_scope_exclude_subtree =
memberof_config_get_entry_scope_exclude_subtree();
+ MemberOfConfig *config = ((memberof_get_groups_data*)callback_data)->config;
int rc = 0;
if(slapi_is_shutting_down()){
@@ -2116,18 +2117,19 @@ int memberof_get_groups_callback(Slapi_Entry *e, void
*callback_data)
goto bail;
}
- /* if the group does not belong to an excluded subtree, adds it to the valueset
*/
- if (!(entry_scope_exclude_subtree && slapi_sdn_issuffix(group_sdn,
entry_scope_exclude_subtree))) {
- /* Push group_dn_val into the valueset. This memory is now owned
- * by the valueset. */
- group_dn_val = slapi_value_new_string(group_dn);
- slapi_valueset_add_value_ext(groupvals, group_dn_val,
SLAPI_VALUE_FLAG_PASSIN);
- slapi_valueset_add_value_ext(group_norm_vals, group_ndn_val,
SLAPI_VALUE_FLAG_PASSIN);
- }
-
- /* now recurse to find parent groups of e */
- memberof_get_groups_r(((memberof_get_groups_data*)callback_data)->config,
- group_sdn, callback_data);
+ /* if the group does not belong to an excluded subtree, adds it to the valueset */
+ if (!(entry_scope_exclude_subtree && slapi_sdn_issuffix(group_sdn,
entry_scope_exclude_subtree))) {
+ /* Push group_dn_val into the valueset. This memory is now owned
+ * by the valueset. */
+ group_dn_val = slapi_value_new_string(group_dn);
+ slapi_valueset_add_value_ext(groupvals, group_dn_val, SLAPI_VALUE_FLAG_PASSIN);
+ slapi_valueset_add_value_ext(group_norm_vals, group_ndn_val,
SLAPI_VALUE_FLAG_PASSIN);
+ }
+ if(!config->skip_nested || config->fixup_task){
+ /* now recurse to find parent groups of e */
+ memberof_get_groups_r(((memberof_get_groups_data*)callback_data)->config,
+ group_sdn, callback_data);
+ }
bail:
return rc;
@@ -2844,10 +2846,7 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void
*callback_data)
memberof_del_dn_data del_data = {0, config->memberof_attr};
Slapi_ValueSet *groups = 0;
- if(!config->skip_nested || config->fixup_task){
- /* get a list of all of the groups this user belongs to */
- groups = memberof_get_groups(config, sdn);
- }
+ groups = memberof_get_groups(config, sdn);
/* If we found some groups, replace the existing memberOf attribute
* with the found values. */