[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