On 02/01/2012 04:28 PM, Rich Megginson wrote:
On 02/01/2012 02:16 PM, Mark Reynolds wrote:
Hi Everyone,
There is an issue with the PAM plugin, that when it performs a
successful bind we actually return error 1 to plugins_call_func(),
which essentially causes the abort of the all plugin processing: the
rest of pre-op, the backend call, and all of post-op. PAM has
completed the bind and already returned the result, so it returns 1
to stop the DS from doing the rest of bind op. Makes sense...
Where is the code that calls the preop?
However, with the Account Policy plugin, when tracking the "last bind
time", binding thru PAM won't update the entry, even though the bind
was successful. This is because the successful PAM bind essentially
aborted all the pre-op and post-op plugins. I feel that we should
still call the post op plugins in this scenario. The pre-op plugins
should still be aborted, because the operation was already completed
- there's nothing to reject at that point.
So to get around plugins like this, I am proposing a new plugin
pre-op return code(either use 1, or -2). This return code implies
that the operation was fully completed, and that we should also
process the post op plugins - but do not send the operation to the
backend.
So to the recap, the new return code says the operation was fully
completed by the plugin, but we still want to process just the
post-op plugins.
I know this will impact documentation, there might be unforeseen
issues, and this is also a rare situation(only PAM plugin seems to
behave like this). Saying that, I still think its the valid solution
to this type of problem. It could also allow future plugins for be
more versatile/robust.
Please let me know your thoughts.
I don't think we need a new return code - I just think we need to
diff --git a/ldap/servers/slapd/bind.c b/ldap/servers/slapd/bind.c
index 1d860b6..bd7df3d 100644
--- a/ldap/servers/slapd/bind.c
+++ b/ldap/servers/slapd/bind.c
@@ -796,6 +796,10 @@ do_bind( Slapi_PBlock *pb )
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &rc );
plugin_call_plugins( pb, SLAPI_PLUGIN_POST_BIND_FN );
+ } else {
+ /* even if the pre-op plugins returned an error, we still
need
+ to call the post-op plugins */
+ plugin_call_plugins( pb, SLAPI_PLUGIN_POST_BIND_FN );
}
} else {
send_ldap_result( pb, LDAP_UNWILLING_TO_PERFORM, NULL,
I thought about this, but I thought we needed a new code because isn't
it by design: if pre-op fails, it should all fail? PAM really isn't
failing, it just doesn't have a better mechanism to handle its behaviour.
The above code would work fine, so maybe this could be a discussion for
a later time. I was just looking for a more "global" solution. So, I
will use this approach for now so I can get the bug resolved. If
there's time in the future maybe we can come back to this. Like I said
it's not something that is needed, but it would allow for more creative
plugins :-)
Thanks,
Mark
Looking at bind.c - there is a lot of special case code that makes
sure to call the POST_BIND plugins in various error cases. This seems
like just another error case that we should handle by calling the
POST_BIND plugins.
Thanks,
Mark
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
