[SSSD] [PATCH] LDAP: tokengroups do not work with id_provider=ldap

Pavel Reichl preichl at redhat.com
Thu Jul 17 15:46:29 UTC 2014


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 at 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 at 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 at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel





More information about the sssd-devel mailing list