Browse Source

Ticket 47398 - memberOf on a user is converted to lowercase

Bug Description:
	In order to compare the groups that an entry is memberof, all
	entry DN is normalized. When finally we decide to add (in a target entry)
	the group DN as a memberof value, we store the lowercase normalized value.

Fix Description:
	We keep the processus of comparison with normalized value but at the
	time we do the actual MOD on the target entry, we use the group entry
	DN (normalized but not lowercase) rather that the strict normalized one.
	Changes in memberof_get_groups_callback are for ADD,DEL,MOD ops
	Change in memberof_modop_one_replace_r is for MODRDN of leaf entry
	Change in memberof_replace_dn_from_groups is for MODRDN on group entry

	When retrieving the groups that an entry is memberof, the lowercase normalized DNs
	of the groups are kept in the callback_data (group_norm_vals).
	Because to know if a new group is in the groups already evaluated it needs
	lowercase normalized value.

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

Reviewed by: Noriko Hosoi (Thanks Noriko for the review and the comments. I change slightly
the fix description to take your comment into account).

Platforms tested: F17 acceptance and unit tests.
	Verify that the number of call of slapi_dn_normalization_ext is identical
	with/without the fix. (does not break https://fedorahosted.org/389/ticket/412)

Flag Day: no

Doc impact: no
Thierry bordaz (tbordaz) 12 years ago
parent
commit
04b61372fa
1 changed files with 24 additions and 16 deletions
  1. 24 16
      ldap/servers/plugins/memberof/memberof.c

+ 24 - 16
ldap/servers/plugins/memberof/memberof.c

@@ -91,6 +91,7 @@ typedef struct _memberof_get_groups_data
         MemberOfConfig *config;
         MemberOfConfig *config;
         Slapi_Value *memberdn_val;
         Slapi_Value *memberdn_val;
         Slapi_ValueSet **groupvals;
         Slapi_ValueSet **groupvals;
+        Slapi_ValueSet **group_norm_vals;
 } memberof_get_groups_data;
 } memberof_get_groups_data;
 
 
 /*** function prototypes ***/
 /*** function prototypes ***/
@@ -726,8 +727,8 @@ memberof_replace_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config,
 	 * using the same grouping attribute. */
 	 * using the same grouping attribute. */
 	for (i = 0; config->groupattrs && config->groupattrs[i]; i++)
 	for (i = 0; config->groupattrs && config->groupattrs[i]; i++)
 	{
 	{
-		replace_dn_data data = {(char *)slapi_sdn_get_ndn(pre_sdn),
-		                        (char *)slapi_sdn_get_ndn(post_sdn),
+		replace_dn_data data = {(char *)slapi_sdn_get_dn(pre_sdn),
+		                        (char *)slapi_sdn_get_dn(post_sdn),
 		                        config->groupattrs[i]};
 		                        config->groupattrs[i]};
 
 
 		groupattrs[0] = config->groupattrs[i];
 		groupattrs[0] = config->groupattrs[i];
@@ -1361,7 +1362,7 @@ memberof_modop_one_replace_r(Slapi_PBlock *pb, MemberOfConfig *config,
 
 
 			if(LDAP_MOD_REPLACE == mod_op)
 			if(LDAP_MOD_REPLACE == mod_op)
 			{
 			{
-				replace_val[0] = (char *)slapi_sdn_get_ndn(replace_with_sdn);
+				replace_val[0] = (char *)slapi_sdn_get_dn(replace_with_sdn);
 				replace_val[1] = 0;
 				replace_val[1] = 0;
 
 
 				replace_mod.mod_op = LDAP_MOD_ADD;
 				replace_mod.mod_op = LDAP_MOD_ADD;
@@ -1688,15 +1689,17 @@ Slapi_ValueSet *
 memberof_get_groups(MemberOfConfig *config, Slapi_DN *member_sdn)
 memberof_get_groups(MemberOfConfig *config, Slapi_DN *member_sdn)
 {
 {
 	Slapi_ValueSet *groupvals = slapi_valueset_new();
 	Slapi_ValueSet *groupvals = slapi_valueset_new();
+        Slapi_ValueSet *group_norm_vals = slapi_valueset_new();
 	Slapi_Value *memberdn_val = 
 	Slapi_Value *memberdn_val = 
 	                      slapi_value_new_string(slapi_sdn_get_ndn(member_sdn));
 	                      slapi_value_new_string(slapi_sdn_get_ndn(member_sdn));
 	slapi_value_set_flags(memberdn_val, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
 	slapi_value_set_flags(memberdn_val, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
 
 
-	memberof_get_groups_data data = {config, memberdn_val, &groupvals};
+	memberof_get_groups_data data = {config, memberdn_val, &groupvals, &group_norm_vals};
 
 
 	memberof_get_groups_r(config, member_sdn, &data);
 	memberof_get_groups_r(config, member_sdn, &data);
 
 
 	slapi_value_free(&memberdn_val);
 	slapi_value_free(&memberdn_val);
+        slapi_valueset_free(group_norm_vals);
 
 
 	return groupvals;
 	return groupvals;
 }
 }
@@ -1718,9 +1721,12 @@ memberof_get_groups_r(MemberOfConfig *config, Slapi_DN *member_sdn,
 int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)
 int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)
 {
 {
 	Slapi_DN *group_sdn = slapi_entry_get_sdn(e);
 	Slapi_DN *group_sdn = slapi_entry_get_sdn(e);
-	char *group_dn = slapi_entry_get_ndn(e);
-	Slapi_Value *group_dn_val = 0;
+	char *group_ndn = slapi_entry_get_ndn(e);
+        char *group_dn = slapi_entry_get_dn(e);
+	Slapi_Value *group_ndn_val = 0;
+        Slapi_Value *group_dn_val = 0;
 	Slapi_ValueSet *groupvals = *((memberof_get_groups_data*)callback_data)->groupvals;
 	Slapi_ValueSet *groupvals = *((memberof_get_groups_data*)callback_data)->groupvals;
+        Slapi_ValueSet *group_norm_vals = *((memberof_get_groups_data*)callback_data)->group_norm_vals;
 	int rc = 0;
 	int rc = 0;
 
 
 	if(slapi_is_shutting_down()){
 	if(slapi_is_shutting_down()){
@@ -1737,21 +1743,21 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)
 
 
 	}
 	}
 	/* get the DN of the group */
 	/* get the DN of the group */
-	group_dn_val = slapi_value_new_string(group_dn);
+	group_ndn_val = slapi_value_new_string(group_ndn);
 	/* group_dn is case-normalized */
 	/* group_dn is case-normalized */
-	slapi_value_set_flags(group_dn_val, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
+	slapi_value_set_flags(group_ndn_val, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
 
 
 	/* check if e is the same as our original member entry */
 	/* check if e is the same as our original member entry */
 	if (0 == memberof_compare(((memberof_get_groups_data*)callback_data)->config,
 	if (0 == memberof_compare(((memberof_get_groups_data*)callback_data)->config,
-		&((memberof_get_groups_data*)callback_data)->memberdn_val, &group_dn_val))
+		&((memberof_get_groups_data*)callback_data)->memberdn_val, &group_ndn_val))
 	{
 	{
 		/* A recursive group caused us to find our original
 		/* A recursive group caused us to find our original
 		 * entry we passed to memberof_get_groups().  We just
 		 * entry we passed to memberof_get_groups().  We just
 		 * skip processing this entry. */
 		 * skip processing this entry. */
 		slapi_log_error( SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
 		slapi_log_error( SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
 			"memberof_get_groups_callback: group recursion"
 			"memberof_get_groups_callback: group recursion"
-			" detected in %s\n" ,group_dn);
-		slapi_value_free(&group_dn_val);
+			" detected in %s\n" ,group_ndn);
+		slapi_value_free(&group_ndn_val);
 		goto bail;
 		goto bail;
 
 
 	}
 	}
@@ -1760,8 +1766,8 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)
 	 * in config.  We only need this attribute for it's syntax so the comparison can be
 	 * in config.  We only need this attribute for it's syntax so the comparison can be
 	 * performed.  Since all of the grouping attributes are validated to use the Dinstinguished
 	 * performed.  Since all of the grouping attributes are validated to use the Dinstinguished
 	 * Name syntax, we can safely just use the first group_slapiattr. */
 	 * Name syntax, we can safely just use the first group_slapiattr. */
-	if (groupvals && slapi_valueset_find(
-		((memberof_get_groups_data*)callback_data)->config->group_slapiattrs[0], groupvals, group_dn_val))
+	if (group_norm_vals && slapi_valueset_find(
+		((memberof_get_groups_data*)callback_data)->config->group_slapiattrs[0], group_norm_vals, group_ndn_val))
 	{
 	{
 		/* we either hit a recursive grouping, or an entry is
 		/* we either hit a recursive grouping, or an entry is
 		 * a member of a group through multiple paths.  Either
 		 * a member of a group through multiple paths.  Either
@@ -1769,15 +1775,17 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)
 		 * already gone through this part of the grouping hierarchy. */
 		 * already gone through this part of the grouping hierarchy. */
 		slapi_log_error( SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
 		slapi_log_error( SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
 			"memberof_get_groups_callback: possible group recursion"
 			"memberof_get_groups_callback: possible group recursion"
-			" detected in %s\n" ,group_dn);
-		slapi_value_free(&group_dn_val);
+			" detected in %s\n" ,group_ndn);
+		slapi_value_free(&group_ndn_val);
 		goto bail;
 		goto bail;
 	}
 	}
 
 
 	/* Push group_dn_val into the valueset.  This memory is now owned
 	/* Push group_dn_val into the valueset.  This memory is now owned
 	 * by the valueset. */ 
 	 * 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(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 */
 	/* now recurse to find parent groups of e */
 	memberof_get_groups_r(((memberof_get_groups_data*)callback_data)->config,
 	memberof_get_groups_r(((memberof_get_groups_data*)callback_data)->config,
 		group_sdn, callback_data);
 		group_sdn, callback_data);