#47810 Investigate betxn plugins to ensure they return the correct error code
Closed: Fixed None Opened 5 years ago by mreynolds.

Some betxn plugins did not return error codes(they always return success). This was because before there were betxn plugins, their returns codes did not affect the if the operation was accepted or aborted. Now those errors do matter, and we need to make sure all the plugins are behaving correctly.


vals could be leaking and config->lock is not released if "goto done" at line 2086?
linked_attrs_modrdn_post_op(Slapi_PBlock pb)
{{{
2013 2079 /
Delete old dn value. */
2014 linked_attrs_mod_backpointers(old_dn, config->linktype,
2080 rc = linked_attrs_mod_backpointers(old_dn, config->linktype,
2015 2081 config->scope, LDAP_MOD_DELETE, vals);
2082 if(rc != LDAP_SUCCESS){
2083 slapi_log_error(SLAPI_LOG_FATAL, LINK_PLUGIN_SUBSYSTEM,
2084 "linked_attrs_modrdn_post_op: update failed(old dn) (%d)\n",rc);
2085 linked_attrs_unlock();
2086 goto done;
2087 }
}}}

ldap/servers/plugins/retrocl/retrocl_po.c
If you could set SLAPI_PLUGIN_FAILURE instead of "-1", it'd be nice!

The rest looks good to me.

Replying to [comment:4 nhosoi]:

vals could be leaking and config->lock is not released if "goto done" at line 2086?
linked_attrs_modrdn_post_op(Slapi_PBlock pb)
{{{
2013 2079 /
Delete old dn value. */
2014 linked_attrs_mod_backpointers(old_dn, config->linktype,
2080 rc = linked_attrs_mod_backpointers(old_dn, config->linktype,
2015 2081 config->scope, LDAP_MOD_DELETE, vals);
2082 if(rc != LDAP_SUCCESS){
2083 slapi_log_error(SLAPI_LOG_FATAL, LINK_PLUGIN_SUBSYSTEM,
2084 "linked_attrs_modrdn_post_op: update failed(old dn) (%d)\n",rc);
2085 linked_attrs_unlock();
2086 goto done;
2087 }
}}}

ldap/servers/plugins/retrocl/retrocl_po.c
If you could set SLAPI_PLUGIN_FAILURE instead of "-1", it'd be nice!

The rest looks good to me.

Thanks, new patch attached...

Thanks Noriko!

git merge ticket47810
Updating fe81bda..f2eb45b
Fast-forward
ldap/servers/plugins/linkedattrs/linked_attrs.c | 174 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------
ldap/servers/plugins/memberof/memberof.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
ldap/servers/plugins/mep/mep.c | 72 +++++++++++++++++++++++++-----------
ldap/servers/plugins/pam_passthru/pam_ptpreop.c | 3 ++
ldap/servers/plugins/retrocl/retrocl_po.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------
5 files changed, 416 insertions(+), 193 deletions(-)

git push origin master
fe81bda..f2eb45b master -> master

commit f2eb45b
Author: Mark Reynolds mreynolds@redhat.com
Date: Wed Jun 18 08:38:26 2014 -0400

RI plugin could use some improvement (noticed by coverity scan of unused result code)

b01cf4d..c5cc125 master -> master
commit c5cc125

fa8f7dc..44f84b3 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit 44f84b3

Issue found when doing a "replace" of uniquemember on a group, where the member does not contain the approrpriate objectclass for the "memberof" attribute, and the operation is allowed when it should be rejected.

44a223f..eb54f03 master -> master
commit eb54f03
Author: Mark Reynolds mreynolds@redhat.com
Date: Tue Aug 4 12:15:31 2015 -0400

e7fc143..b4b6adc 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit b4b6adc

132e716..e2f6eeb 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit e2f6eeb

There is another side effect of this ticket for memberOf plugin. {{{"memberOf"}}} attribute is not authorized for the object class {{{groupOfUniquenames}}}, so when you try to add, say, a group called "cn=inner_grp" as a {{{uniqueMember}}} of another group (ex. "cn=outer_grp") it fails with the error message
{{{
[26/Aug/2015:16:38:07 +0200] - Entry "cn=inner_grp,ou=Groupes,dc=id,dc=polytechnique,dc=edu" -- attribute "memberOf" not allowed
[26/Aug/2015:16:38:07 +0200] memberof-plugin - memberof_postop_modify: failed to add dn (cn=outer_grp,ou=Groupes,dc=id,dc=polytechnique,dc=edu) to target. Error (65)
}}}

Since the {{{memberOf}}} attribute is not allowed for group objects, the plug-in should not try to write it to this type of objects.

if the replication is enabled, the 2 lines in the previous comment precede another error line relative to replication (even though "nsDS5ReplicatedAttributeList: (objectclass=*) $ EXCLUDE entryusn memberOf") :

{{{
[31/Aug/2015:12:31:31 +0200] - Entry "cn=Groupe DSI complet,ou=Par entite,ou=Groupes Globaux,ou=Groupes,dc=id,dc=polytechnique,dc=edu" -- attribute "memberOf" not allowed
[31/Aug/2015:12:31:31 +0200] memberof-plugin - memberof_postop_modify: failed to add dn (cn=Administrateurs DRUPAL,ou=DRUPAL,ou=DSI,ou=Administration,ou=Groupes,dc=id,dc=polytechnique,dc=edu) to target. Error (65)
[31/Aug/2015:12:31:31 +0200] agmt="cn=Replication from ldap-edev.polytechnique.fr to ldap-model.polytechnique.fr" (ldap-model:636) - Can't locate CSN 55e42cfa000000020000 in the changelog (DB rc=-30988). If replication stops, the consumer may need to be reinitialized.

}}}

Created a new release note bug to further document the new behavior:

https://bugzilla.redhat.com/show_bug.cgi?id=1258624

We are still discussing an option in DS for the memberOf plugin to "auto add" a pre-configured objectclass if a schema violation occurs during a "memberOf" update...

Created a new ticket for the "auto add objectclass" feature:

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

Closing this ticket.

Metadata Update from @mreynolds:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.3.4 backlog

2 years ago

Login to comment on this ticket.