From e5cb97a16fa44e6944e234b9cf509ddb614559a3 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Mon, 9 Dec 2013 16:57:35 -0500
Subject: [PATCH 52/65] Ticket 47622 - Automember betxnpreoperation -
transaction not aborted when group entry does not exist
Bug Description: If the group defined in the automember plugin does not exist, than any add operation
that should trigger an update, succeeds even though the automember update failed.
Fix Description: Return an error if a automember post operation update fails - previously we always
returned success.
Updated plugin_call_func() to check the result of betxn postop plugins.
Also added return text to the result message when a betxn plugin fails. This is
useful for clients to explain why the operation failed.
https://fedorahosted.org/389/ticket/47622
Jenkins: passed
Valgrind: passed
Coverity: passed
Reviewed by: rmeggins(Thanks!)
(cherry picked from commit 1214168a222a35627b2bb9964600fad0246558cd)
(cherry picked from commit 6de4616f2506b4e093429cc1093e4ad21b22e6c9)
---
ldap/servers/plugins/automember/automember.c | 151 ++++++++++++++++++++++-----
ldap/servers/slapd/back-ldbm/ldbm_add.c | 4 +-
ldap/servers/slapd/back-ldbm/ldbm_delete.c | 5 +
ldap/servers/slapd/back-ldbm/ldbm_modify.c | 2 +
ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 3 +
ldap/servers/slapd/plugin.c | 3 +-
6 files changed, 137 insertions(+), 31 deletions(-)
diff --git a/ldap/servers/plugins/automember/automember.c b/ldap/servers/plugins/automember/automember.c
index c7168cb..3214ea1 100644
--- a/ldap/servers/plugins/automember/automember.c
+++ b/ldap/servers/plugins/automember/automember.c
@@ -103,8 +103,8 @@ static struct automemberRegexRule *automember_parse_regex_rule(char *rule_string
static void automember_free_regex_rule(struct automemberRegexRule *rule);
static int automember_parse_grouping_attr(char *value, char **grouping_attr,
char **grouping_value);
-static void automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileDesc *ldif_fd);
-static void automember_add_member_value(Slapi_Entry *member_e, const char *group_dn,
+static int automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileDesc *ldif_fd);
+static int automember_add_member_value(Slapi_Entry *member_e, const char *group_dn,
char *grouping_attr, char *grouping_value, PRFileDesc *ldif_fd);
const char *fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val);
@@ -1401,7 +1401,7 @@ automember_parse_grouping_attr(char *value, char **grouping_attr, char **groupin
* Determines which target groups need to be updated according to
* the rules in config, then performs the updates.
*/
-static void
+static int
automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileDesc *ldif_fd)
{
PRCList *rule = NULL;
@@ -1412,10 +1412,11 @@ automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileD
Slapi_DN *last = NULL;
PRCList *curr_exclusion = NULL;
char **vals = NULL;
+ int rc = 0;
int i = 0;
if (!config || !e) {
- return;
+ return -1;
}
slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM,
@@ -1555,15 +1556,23 @@ automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileD
if (PR_CLIST_IS_EMPTY(&targets)) {
/* Add to each default group. */
for (i = 0; config->default_groups && config->default_groups[i]; i++) {
- automember_add_member_value(e, config->default_groups[i],
- config->grouping_attr, config->grouping_value, ldif_fd);
+ if(automember_add_member_value(e, config->default_groups[i], config->grouping_attr,
+ config->grouping_value, ldif_fd))
+ {
+ rc = SLAPI_PLUGIN_FAILURE;
+ goto out;
+ }
}
} else {
/* Update the target groups. */
dnitem = (struct automemberDNListItem *)PR_LIST_HEAD(&targets);
while ((PRCList *)dnitem != &targets) {
- automember_add_member_value(e, slapi_sdn_get_dn(dnitem->dn),
- config->grouping_attr, config->grouping_value, ldif_fd);
+ if(automember_add_member_value(e, slapi_sdn_get_dn(dnitem->dn),config->grouping_attr,
+ config->grouping_value, ldif_fd))
+ {
+ rc = SLAPI_PLUGIN_FAILURE;
+ goto out;
+ }
dnitem = (struct automemberDNListItem *)PR_NEXT_LINK((PRCList *)dnitem);
}
}
@@ -1582,6 +1591,9 @@ automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileD
slapi_ch_free((void**)&dnitem);
}
+out:
+
+ return rc;
}
/*
@@ -1589,7 +1601,7 @@ automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileD
*
* Adds a member entry to a group.
*/
-static void
+static int
automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, char *grouping_attr,
char *grouping_value, PRFileDesc *ldif_fd)
{
@@ -1600,6 +1612,7 @@ automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, char *g
char *vals[2];
char *member_value = NULL;
int freeit = 0;
+ int rc = 0;
/* If grouping_value is dn, we need to fetch the dn instead. */
if (slapi_attr_type_cmp(grouping_value, "dn", SLAPI_TYPE_CMP_EXACT) == 0) {
@@ -1649,6 +1662,7 @@ automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, char *g
"a \"%s\" value to group \"%s\" (%s).\n",
member_value, grouping_attr, group_dn,
ldap_err2string(result));
+ rc = result;
}
} else {
slapi_log_error(SLAPI_LOG_FATAL, AUTOMEMBER_PLUGIN_SUBSYSTEM,
@@ -1662,8 +1676,9 @@ out:
if (freeit) {
slapi_ch_free_string(&member_value);
}
-
slapi_pblock_destroy(mod_pb);
+
+ return rc;
}
@@ -1833,6 +1848,7 @@ automember_add_post_op(Slapi_PBlock *pb)
Slapi_DN *sdn = NULL;
struct configEntry *config = NULL;
PRCList *list = NULL;
+ int rc = SLAPI_PLUGIN_SUCCESS;
slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
"--> automember_add_post_op\n");
@@ -1848,8 +1864,9 @@ automember_add_post_op(Slapi_PBlock *pb)
}
} else {
slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM,
- "automember_add_post_op: Error "
- "retrieving dn\n");
+ "automember_add_post_op: Error retrieving dn\n");
+
+ rc = SLAPI_PLUGIN_FAILURE;
goto bail;
}
@@ -1863,12 +1880,11 @@ automember_add_post_op(Slapi_PBlock *pb)
if (e) {
/* If the entry is a tombstone, just bail. */
- Slapi_Value *tombstone =
- slapi_value_new_string(SLAPI_ATTR_VALUE_TOMBSTONE);
- int rc = slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS,
- tombstone);
+ Slapi_Value *tombstone = slapi_value_new_string(SLAPI_ATTR_VALUE_TOMBSTONE);
+ int is_tombstone = slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS,
+ tombstone);
slapi_value_free(&tombstone);
- if (rc) {
+ if (is_tombstone) {
return SLAPI_PLUGIN_SUCCESS;
}
@@ -1891,7 +1907,10 @@ automember_add_post_op(Slapi_PBlock *pb)
if (slapi_dn_issuffix(slapi_sdn_get_dn(sdn), config->scope) &&
(slapi_filter_test_simple(e, config->filter) == 0)) {
/* Find out what membership changes are needed and make them. */
- automember_update_membership(config, e, NULL);
+ if(automember_update_membership(config, e, NULL)){
+ rc = SLAPI_PLUGIN_FAILURE;
+ break;
+ }
}
list = PR_NEXT_LINK(list);
@@ -1904,11 +1923,21 @@ automember_add_post_op(Slapi_PBlock *pb)
"automember_add_post_op: Error "
"retrieving post-op entry %s\n", slapi_sdn_get_dn(sdn));
}
+
bail:
slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
- "<-- automember_add_post_op\n");
+ "<-- automember_add_post_op (%d)\n", rc);
- return SLAPI_PLUGIN_SUCCESS;
+ if(rc){
+ char errtxt[SLAPI_DSE_RETURNTEXT_SIZE];
+ int result = LDAP_UNWILLING_TO_PERFORM;
+
+ PR_snprintf(errtxt, SLAPI_DSE_RETURNTEXT_SIZE, "Automember Plugin update unexpectedly failed.\n");
+ slapi_pblock_set(pb, SLAPI_RESULT_CODE, &result);
+ slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, &errtxt);
+ }
+
+ return rc;
}
/*
@@ -2216,7 +2245,11 @@ void automember_rebuild_task_thread(void *arg){
if (slapi_dn_issuffix(slapi_entry_get_dn(entries[i]), config->scope) &&
(slapi_filter_test_simple(entries[i], config->filter) == 0))
{
- automember_update_membership(config, entries[i], NULL);
+ if(automember_update_membership(config, entries[i], NULL)){
+ result = SLAPI_PLUGIN_FAILURE;
+ automember_config_unlock();
+ goto out;
+ }
}
list = PR_NEXT_LINK(list);
}
@@ -2416,7 +2449,7 @@ void automember_export_task_thread(void *arg){
/* make sure the plugin is still up, as this loop could run for awhile */
if (!g_plugin_started) {
automember_config_unlock();
- result = -1;
+ result = SLAPI_DSE_CALLBACK_ERROR;
goto out;
}
if (!PR_CLIST_IS_EMPTY(g_automember_config)) {
@@ -2426,7 +2459,11 @@ void automember_export_task_thread(void *arg){
if (slapi_dn_issuffix(slapi_sdn_get_dn(td->base_dn), config->scope) &&
(slapi_filter_test_simple(entries[i], config->filter) == 0))
{
- automember_update_membership(config, entries[i], ldif_fd);
+ if(automember_update_membership(config, entries[i], ldif_fd)){
+ result = SLAPI_DSE_CALLBACK_ERROR;
+ automember_config_unlock();
+ goto out;
+ }
}
list = PR_NEXT_LINK(list);
}
@@ -2624,7 +2661,13 @@ void automember_map_task_thread(void *arg){
if (slapi_dn_issuffix(slapi_entry_get_dn_const(e), config->scope) &&
(slapi_filter_test_simple(e, config->filter) == 0))
{
- automember_update_membership(config, e, ldif_fd_out);
+ if(automember_update_membership(config, e, ldif_fd_out)){
+ result = SLAPI_DSE_CALLBACK_ERROR;
+ slapi_entry_free(e);
+ slapi_ch_free_string(&entrystr);
+ automember_config_unlock();
+ goto out;
+ }
}
list = PR_NEXT_LINK(list);
}
@@ -2635,7 +2678,7 @@ void automember_map_task_thread(void *arg){
slapi_task_log_notice(task, "Automember map task, skipping invalid entry.");
slapi_task_log_status(task, "Automember map task, skipping invalid entry.");
}
- slapi_ch_free((void **)&entrystr);
+ slapi_ch_free_string(&entrystr);
}
automember_config_unlock();
@@ -2666,6 +2709,9 @@ automember_modrdn_post_op(Slapi_PBlock *pb)
Slapi_DN *old_sdn = NULL;
Slapi_DN *new_sdn = NULL;
Slapi_Entry *post_e = NULL;
+ struct configEntry *config = NULL;
+ PRCList *list = NULL;
+ int rc = SLAPI_PLUGIN_SUCCESS;
slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
"--> automember_modrdn_post_op\n");
@@ -2684,7 +2730,7 @@ automember_modrdn_post_op(Slapi_PBlock *pb)
slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM,
"automember_modrdn_post_op: Error "
"retrieving post-op entry\n");
- return 0;
+ return SLAPI_PLUGIN_FAILURE;
}
if ((old_sdn = automember_get_sdn(pb))) {
@@ -2694,11 +2740,58 @@ automember_modrdn_post_op(Slapi_PBlock *pb)
slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM,
"automember_modrdn_post_op: Error "
"retrieving dn\n");
+ return SLAPI_PLUGIN_FAILURE;
}
- slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
- "<-- automember_modrdn_post_op\n");
+ /* If replication, just bail. */
+ if (automember_isrepl(pb)) {
+ return SLAPI_PLUGIN_SUCCESS;
+ }
- return 0;
+ /*
+ * Check if a config entry applies to the entry(post modrdn)
+ */
+ automember_config_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ automember_config_unlock();
+ return SLAPI_PLUGIN_SUCCESS;
+ }
+
+ if (!PR_CLIST_IS_EMPTY(g_automember_config)) {
+ list = PR_LIST_HEAD(g_automember_config);
+ while (list != g_automember_config) {
+ config = (struct configEntry *)list;
+
+ /* Does the entry meet scope and filter requirements? */
+ if (slapi_dn_issuffix(slapi_sdn_get_dn(new_sdn), config->scope) &&
+ (slapi_filter_test_simple(post_e, config->filter) == 0)) {
+ /* Find out what membership changes are needed and make them. */
+ if(automember_update_membership(config, post_e, NULL)){
+ rc = SLAPI_PLUGIN_FAILURE;
+ break;
+ }
+ }
+
+ list = PR_NEXT_LINK(list);
+ }
+ }
+
+ automember_config_unlock();
+
+ if(rc){
+ char errtxt[SLAPI_DSE_RETURNTEXT_SIZE];
+ int result = LDAP_UNWILLING_TO_PERFORM;
+
+ PR_snprintf(errtxt, SLAPI_DSE_RETURNTEXT_SIZE, "Automember Plugin update unexpectedly failed. "
+ "Please see the server errors log for more information.\n");
+ slapi_pblock_set(pb, SLAPI_RESULT_CODE, &result);
+ slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, &errtxt);
+ }
+
+ slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
+ "<-- automember_modrdn_post_op (%d)\n", rc);
+ return rc;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c
index fa1e9bc..e5b9eeb 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_add.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c
@@ -357,7 +357,7 @@ ldbm_back_add( Slapi_PBlock *pb )
/* make sure opreturn is set for the postop plugins */
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &rc);
}
-
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
/*
@@ -795,6 +795,7 @@ ldbm_back_add( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval);
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
@@ -1046,6 +1047,7 @@ ldbm_back_add( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval);
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
index 6725123..367ab99 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
@@ -325,6 +325,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &rc );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
/* the flag could be set in a preop plugin (e.g., USN) */
@@ -354,6 +355,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
ldap_result_code ?
&ldap_result_code : &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
@@ -603,6 +605,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
ldap_result_code ?
&ldap_result_code : &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
}
@@ -633,6 +636,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
&ldap_result_code : &rc );
}
/* retval is -1 */
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry );
@@ -1105,6 +1109,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
index b5bdb41..f3b099d 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
@@ -582,6 +582,7 @@ ldbm_back_modify( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval);
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
@@ -752,6 +753,7 @@ ldbm_back_modify( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval);
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
index 4908751..1162fdb 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
@@ -466,6 +466,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &rc );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
/*
@@ -890,6 +891,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
@@ -1130,6 +1132,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c
index 5f66ab2..1ca4dc5 100644
--- a/ldap/servers/slapd/plugin.c
+++ b/ldap/servers/slapd/plugin.c
@@ -1467,7 +1467,8 @@ plugin_call_func (struct slapdplugin *list, int operation, Slapi_PBlock *pb, int
}
else if (SLAPI_PLUGIN_BEPREOPERATION == list->plg_type ||
SLAPI_PLUGIN_BETXNPREOPERATION == list->plg_type ||
- SLAPI_PLUGIN_BEPOSTOPERATION == list->plg_type)
+ SLAPI_PLUGIN_BEPOSTOPERATION == list->plg_type ||
+ SLAPI_PLUGIN_BETXNPOSTOPERATION == list->plg_type )
{
/*
* respect fatal error SLAPI_PLUGIN_FAILURE (-1);
--
1.8.1.4