On Thu, 2014-07-17 at 09:56 +0200, Lukas Slebodnik wrote:
[snip]
You will change patch to address Jakub comments.
As you command m'lord. ;-)
So, you can also change
few nitpicks ;-) I would ignore them if patches were already ACK-ed.
>From 77f41aea31afd642d26057018181394c65d81000 Mon Sep 17 00:00:00 2001
>From: Pavel Reichl <preichl(a)redhat.com>
>Date: Tue, 17 Jun 2014 17:16:14 +0100
>Subject: [PATCH 1/2] LDAP: tokengroups do not work with id_provider=ldap
>
>With plain LDAP provider we already have a sdap_handle, so it should be possible
>that in the case where sdom->pvt == NULL sdap_id_op_connect_send() can be
>skipped and sdap_get_ad_tokengroups_send() can be already send with the
>sdap_handle passed to sdap_ad_tokengroups_initgr_mapping_send(). So we should
>only fail if sdom->pvt == NULL and sh == NULL.
>
>if find_subdomain_by_sid() failed we can check if there is only one domain in
>the domain list (state->domain) and in this case continue with this domain since
>the LDAP provider does not know about sub-domains and hence can only have one
>configured domain.
>
>Resolves:
>https://fedorahosted.org/sssd/ticket/2345
>---
//snip
>+
>+static errno_t handle_missing_pvt(TALLOC_CTX *mem_ctx,
>+ struct tevent_context *ev,
>+ struct sdap_options *opts,
>+ const char *orig_dn,
>+ int timeout,
>+ const char *username,
>+ struct sdap_handle *sh,
>+ struct tevent_req *req,
>+ tevent_req_fn callback)
>+{
>+ struct tevent_req *subreq = NULL;
>+ errno_t ret;
>+
>+ if (sh != NULL) {
>+ /* plain LDAP provider already has a sdap_handle */
>+ subreq = sdap_get_ad_tokengroups_send(mem_ctx, ev, opts, sh, username,
>+ orig_dn, timeout);
>+ if (subreq == NULL) {
>+ ret = ENOMEM;
>+ tevent_req_error(req, ENOMEM);
^^^^^^
We call in this way: tevent_req_error(req, ret);
There are some exceptions, but it this case you can use it.
Fixed.
>+ goto done;
>+ }
>+
>+ tevent_req_set_callback(subreq, callback, req);
>+ ret = EOK;
>+ goto done;
>+
>+ } else {
>+ ret = EINVAL;
>+ goto done;
>+ }
>+
>+done:
>+ return ret;
>+}
>From 2056ad183feaf41423610f7f1705dad827bb8470 Mon Sep 17 00:00:00 2001
>From: Pavel Reichl <preichl(a)redhat.com>
>Date: Thu, 10 Jul 2014 10:48:42 +0100
>Subject: [PATCH 2/2] SDAP: Continue resolving SID even if some fail
>
>Resolving groups obtained via Token-Groups in case of disabled ID mapping may
>lead to failure as non-posix groups are not resolved. This patch amends
>sdap_ad_resolve_sids_done() not to abruptly finish request if ENOENT is
>returned.
>
>Resolves:
>https://fedorahosted.org/sssd/ticket/2345
>---
> src/providers/ldap/sdap_async_initgroups_ad.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c
b/src/providers/ldap/sdap_async_initgroups_ad.c
>index
1550f6ceab7fe7ad6c4aed3af674407b92802752..f6a45fb70173355010289dfae30b0a898c61f166 100644
>--- a/src/providers/ldap/sdap_async_initgroups_ad.c
>+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
>@@ -646,7 +646,12 @@ static void sdap_ad_resolve_sids_done(struct tevent_req
*subreq)
>
> ret = groups_get_recv(subreq, &dp_error, &sdap_error);
> talloc_zfree(subreq);
>- if (ret != EOK || sdap_error != EOK || dp_error != DP_ERR_OK) {
>+
>+ if (ret == EOK && sdap_error == ENOENT && dp_error == DP_ERR_OK)
{
>+ DEBUG(SSSDBG_CRIT_FAILURE,
>+ "Unable to resolve SID %s - will try next sid.\n",
>+ state->current_sid);
>+ } else if (ret != EOK || (sdap_error != EOK) || dp_error != DP_ERR_OK) {
^^^^^^^^^^^^^^^^^^^
Could you explain why you added parentheses just here?
They weren't in previous code.
Sorry, I can't explain. Fixed in patch sent today.
> DEBUG(SSSDBG_CRIT_FAILURE, "Unable to resolve SID
%s [dp_error: %d, "
> "sdap_error: %d, ret: %d]: %s\n", state->current_sid,
dp_error,
> sdap_error, ret, strerror(ret));
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel