[SSSD] [PATCHES] Various view related improvements

Sumit Bose sbose at redhat.com
Mon Jan 12 11:31:48 UTC 2015


On Fri, Jan 09, 2015 at 02:46:27PM +0100, Jakub Hrozek wrote:
> On Wed, Dec 17, 2014 at 11:47:59AM +0100, Sumit Bose wrote:
> > On Wed, Dec 17, 2014 at 09:33:32AM +0100, Lukas Slebodnik wrote:
> > > On (17/12/14 09:31), Lukas Slebodnik wrote:
> > > >On (11/12/14 11:10), Sumit Bose wrote:
> > > >>Hi,
> > > >>
> > > >>this patch set contains some improvements for the view related code on
> > > >>IPA client. Most of them are performance related, patches 4,5 and 6 make
> > > >>sure that during group requests all members are properly resovled if a
> > > >>non-default view is applied to be able to handle user name overrides.
> > > >>
> > > >>bye,
> > > >>Sumit
> > > >
> > > >>From c6dcdb109a1d6daa8f91b197a29f4551d9de8d8a Mon Sep 17 00:00:00 2001
> > > >>From: Sumit Bose <sbose at redhat.com>
> > > >>Date: Fri, 5 Dec 2014 11:11:49 +0100
> > > >>Subject: [PATCH 4/7] IPA: process_members() optionally return missing members
> > > >> list
> > > >>
> > > >>---
> > > >> src/providers/ipa/ipa_s2n_exop.c | 85 +++++++++++++++++++++++++++++++---------
> > > >> 1 file changed, 67 insertions(+), 18 deletions(-)
> > > >>
> > > >There is a warning from static analyzer. Currently it is false possitive and
> > > >it isn't big chance to do such mistake, but small change in code can fix it.
> > > >See html report: left_operand_is_garbage.html.gz
> > > >
> > > And one more time with attachment :-)
> > 
> > Thank you, fixed version attached.
> > 
> > bye,
> > Sumit
> > 
> > > 
> > > LS
> > 
> > 
> > > _______________________________________________
> > > sssd-devel mailing list
> > > sssd-devel at lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > 
> 
> The patches are mostly good, but I see some problems even without them,
> so maybe it makes sense to discuss them anyway.
> 
> When the cache is empty and I run "id -G $user", the groups are saved to
> the cache with DNs looking like this:
>     name=unigroup at AD.EXAMPLE.COM,cn=groups,cn=AD.EXAMPLE.COM,cn=sysdb
> 
> But later when adding the user to groups, the member-add function looks
> for:
>     name=unigroup at ad.example.com,cn=groups,cn=AD.EXAMPLE.COM,cn=sysdb
> 
> Looks like there is a casing mismatch...I know we made originalDN
> case-insensitive but most probably not the sysdb DN. The full log
> message is:
>     (Fri Jan  9 14:25:19 2015) [sssd[be[ipa.example.com]]] [sysdb_update_members_ex] (0x0020): Could not add member [psuser at AD.EXAMPLE.COM] to group [name=unigroup at ad.example.com,cn=groups,cn=AD.EXAMPLE.COM,cn=sysdb]. Skipping.

Thank you for the review. This should happen if your AD domain name (not
the real name) contains upper-case characters. This spelling is kept in
sysdb but get lower-cased e.g. in the getgrnam() output. I've add a 8th
patch which should fix it.

> 
> 
> Some comments on the patches inline..
> 
> > From 3911cf48ca4f0788476ce8fb50c175dbd0d83e4f Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Thu, 4 Dec 2014 13:26:32 +0100
> > Subject: [PATCH 1/7] IPA: do not look up overrides on client with default view
> > 
> > The IPA extdom plugin returns the data with the default view already
> > applied hence it is on needed to look up the override data if the client
> > has the default view assigned.
> 
> ACK
> 
> > From affa294d04ed7fef4c1d9cfd92098f22f91600ea Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Fri, 5 Dec 2014 11:03:48 +0100
> > Subject: [PATCH 2/7] IPA: make version check more precise
> > 
> > The call protected by the check does not only expect the version 1 of
> > the extdom plugin is used but a specific response type as well. Since
> > version 1 can return older response types as well we want to be on the
> > safe side.
> 
> ACK
> 
> > From 37cc08b7d2a678f31191794e653aa89202148962 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Fri, 5 Dec 2014 11:06:26 +0100
> > Subject: [PATCH 3/7] IPA: add missing break
> > 
> > The current request already returned the SID, we do not need to request
> > it separately.
> 
> ACK
> 
> > From 6f0f58448b86052c848b78151dbc39b0d3c99737 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Fri, 5 Dec 2014 11:11:49 +0100
> > Subject: [PATCH 4/7] IPA: process_members() optionally return missing members
> >  list
> > 
> > ---
> >  src/providers/ipa/ipa_s2n_exop.c | 84 +++++++++++++++++++++++++++++++---------
> >  1 file changed, 66 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
> > index 6bf492623f638a9900f997a937e05cd66fca09a2..3b02e67e36caa8ee05a702dc3434c965ecee4682 100644
> > --- a/src/providers/ipa/ipa_s2n_exop.c
> > +++ b/src/providers/ipa/ipa_s2n_exop.c
> > @@ -1222,7 +1222,8 @@ fail:
> >  
> >  static errno_t process_members(struct sss_domain_info *domain,
> >                                 struct sysdb_attrs *group_attrs,
> > -                               char **members)
> > +                               char **members,
> > +                               TALLOC_CTX *mem_ctx, char ***_missing_members)
> >  {
> >      int ret;
> >      size_t c;
> > @@ -1231,6 +1232,8 @@ static errno_t process_members(struct sss_domain_info *domain,
> >      const char *dn_str;
> >      struct sss_domain_info *obj_domain;
> >      struct sss_domain_info *parent_domain;
> > +    char **missing_members = NULL;
> > +    size_t miss_count = 0;
> >  
> >      if (members == NULL) {
> >          DEBUG(SSSDBG_TRACE_INTERNAL, "No members\n");
> 
> I think you should also set *_missing_members to NULL if there are no
> members.

fixed

> 
> > From c111a183e3ceda85781bf3383a869bf8d8e555d9 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Tue, 9 Dec 2014 17:04:30 +0100
> > Subject: [PATCH 5/7] IPA: rename ipa_s2n_get_groups_send() to
> >  ipa_s2n_get_fqlist_send()
> > 
> > ---
> >  src/providers/ipa/ipa_s2n_exop.c | 100 +++++++++++++++++++--------------------
> >  1 file changed, 50 insertions(+), 50 deletions(-)
> > 
> > diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
> > index 3b02e67e36caa8ee05a702dc3434c965ecee4682..f3a6aa99408d2df3448a9642ef99b5f2b7082248 100644
> > --- a/src/providers/ipa/ipa_s2n_exop.c
> > +++ b/src/providers/ipa/ipa_s2n_exop.c
> > @@ -877,38 +877,38 @@ done:
> >      return ret;
> >  }
> >  
> > -struct ipa_s2n_get_groups_state {
> > +struct ipa_s2n_get_fqlist_state {
> >      struct tevent_context *ev;
> >      struct ipa_id_ctx *ipa_ctx;
> >      struct sss_domain_info *dom;
> >      struct sdap_handle *sh;
> >      struct req_input req_input;
> > -    char **group_list;
> > -    size_t group_idx;
> > +    char **fqname_list;
> > +    size_t fqname_idx;
> >      int exop_timeout;
> >      struct resp_attrs *attrs;
> >      struct sss_domain_info *obj_domain;
> >      struct sysdb_attrs *override_attrs;
> >  };
> >  
> > -static errno_t ipa_s2n_get_groups_step(struct tevent_req *req);
> > -static void ipa_s2n_get_groups_get_override_done(struct tevent_req *subreq);
> > -static void ipa_s2n_get_groups_next(struct tevent_req *subreq);
> > -static errno_t ipa_s2n_get_groups_save_step(struct tevent_req *req);
> > +static errno_t ipa_s2n_get_fqlist_step(struct tevent_req *req);
> > +static void ipa_s2n_get_fqlist_get_override_done(struct tevent_req *subreq);
> > +static void ipa_s2n_get_fqlist_next(struct tevent_req *subreq);
> > +static errno_t ipa_s2n_get_fqlist_save_step(struct tevent_req *req);
> >  
> > -static struct tevent_req *ipa_s2n_get_groups_send(TALLOC_CTX *mem_ctx,
> > +static struct tevent_req *ipa_s2n_get_fqlist_send(TALLOC_CTX *mem_ctx,
> >                                                    struct tevent_context *ev,
> >                                                    struct ipa_id_ctx *ipa_ctx,
> >                                                    struct sss_domain_info *dom,
> >                                                    struct sdap_handle *sh,
> >                                                    int exop_timeout,
> > -                                                  char **group_list)
> > +                                                  char **fqname_list)
> >  {
> >      int ret;
> > -    struct ipa_s2n_get_groups_state *state;
> > +    struct ipa_s2n_get_fqlist_state *state;
> >      struct tevent_req *req;
> >  
> > -    req = tevent_req_create(mem_ctx, &state, struct ipa_s2n_get_groups_state);
> > +    req = tevent_req_create(mem_ctx, &state, struct ipa_s2n_get_fqlist_state);
> >      if (req == NULL) {
> >          return NULL;
> >      }
> > @@ -917,17 +917,17 @@ static struct tevent_req *ipa_s2n_get_groups_send(TALLOC_CTX *mem_ctx,
> >      state->ipa_ctx = ipa_ctx;
> >      state->dom = dom;
> >      state->sh = sh;
> > -    state->group_list = group_list;
> > -    state->group_idx = 0;
> > +    state->fqname_list = fqname_list;
> > +    state->fqname_idx = 0;
> >      state->req_input.type = REQ_INP_NAME;
> >      state->req_input.inp.name = NULL;
> >      state->exop_timeout = exop_timeout;
> >      state->attrs = NULL;
> >      state->override_attrs = NULL;
> >  
> > -    ret = ipa_s2n_get_groups_step(req);
> > +    ret = ipa_s2n_get_fqlist_step(req);
> >      if (ret != EOK) {
> > -        DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_groups_step failed.\n");
> > +        DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_fqlist_step failed.\n");
> >          goto done;
> >      }
> >  
> > @@ -940,25 +940,25 @@ done:
> >      return req;
> >  }
> >  
> > -static errno_t ipa_s2n_get_groups_step(struct tevent_req *req)
> > +static errno_t ipa_s2n_get_fqlist_step(struct tevent_req *req)
> >  {
> >      int ret;
> > -    struct ipa_s2n_get_groups_state *state = tevent_req_data(req,
> > -                                               struct ipa_s2n_get_groups_state);
> > +    struct ipa_s2n_get_fqlist_state *state = tevent_req_data(req,
> > +                                               struct ipa_s2n_get_fqlist_state);
> >      struct berval *bv_req;
> >      struct tevent_req *subreq;
> >      struct sss_domain_info *parent_domain;
> > -    char *group_name = NULL;
> > +    char *fqname = NULL;
> >      char *domain_name = NULL;
> >  
> >      parent_domain = get_domains_head(state->dom);
> >  
> >      ret = sss_parse_name(state, parent_domain->names,
> > -                         state->group_list[state->group_idx],
> > -                         &domain_name, &group_name);
> > +                         state->fqname_list[state->fqname_idx],
> > +                         &domain_name, &fqname);
> 
> Shouldn't this variable be named just 'name'? The fqname is just being
> parsed into (name,domain), right?

yes, I've changed it to short_name to be more explicit becase there are
so many names.

> 
> > From 0d742401943698789deb2dd56159713717d56a1d Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Fri, 5 Dec 2014 11:12:42 +0100
> > Subject: [PATCH 6/7] IPA: resolve missing members
> 
> I couldn't do much testing b/c of the problems described earlier, but is
> it possible there would be some nested group members that would require
> BE_RE_GROUP also?

about the problems see comment above. I think at this stage all nested
group memberships should be already unrolled and no special handleing
needed.

> 
> > +            if (missing_list != NULL) {
> > +                subreq = ipa_s2n_get_fqlist_send(state, state->ev,
> > +                                                 state->ipa_ctx, state->dom,
> > +                                                 state->sh, state->exop_timeout,
> > +                                                 BE_REQ_USER,
> > +                                                 REQ_FULL_WITH_MEMBERS,
> > +                                                 missing_list);
> >                  if (subreq == NULL) {
> >                      DEBUG(SSSDBG_OP_FAILURE,
> >                            "ipa_s2n_get_fqlist_send failed.\n");
> 
> > From 6b7ab672002e3c7f60b2284f679415acc0924388 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Tue, 9 Dec 2014 17:48:46 +0100
> > Subject: [PATCH 7/7] IPA: set SYSDB_INITGR_EXPIRE for RESP_USER_GROUPLIST
> > 
> > Since RESP_USER_GROUPLIST contains all group memberships it is
> > effectively an initgroups request hence SYSDB_INITGR_EXPIRE will be set.
> 
> ACK

New version attached.

bye,
Sumit
-------------- next part --------------
From b8301ed7be7bc2bbb49d59c9b19b7be3d51a035e Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 4 Dec 2014 13:26:32 +0100
Subject: [PATCH 1/8] IPA: do not look up overrides on client with default view

The IPA extdom plugin returns the data with the default view already
applied hence it is on needed to look up the override data if the client
has the default view assigned.
---
 src/providers/ipa/ipa_s2n_exop.c | 63 +++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 55450c7029391a99bfc33b8446765f71c4d0928a..1d9a79a383e47fcdd37f30a24327ae76facea5b9 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -888,11 +888,13 @@ struct ipa_s2n_get_groups_state {
     int exop_timeout;
     struct resp_attrs *attrs;
     struct sss_domain_info *obj_domain;
+    struct sysdb_attrs *override_attrs;
 };
 
 static errno_t ipa_s2n_get_groups_step(struct tevent_req *req);
 static void ipa_s2n_get_groups_get_override_done(struct tevent_req *subreq);
 static void ipa_s2n_get_groups_next(struct tevent_req *subreq);
+static errno_t ipa_s2n_get_groups_save_step(struct tevent_req *req);
 
 static struct tevent_req *ipa_s2n_get_groups_send(TALLOC_CTX *mem_ctx,
                                                   struct tevent_context *ev,
@@ -921,6 +923,7 @@ static struct tevent_req *ipa_s2n_get_groups_send(TALLOC_CTX *mem_ctx,
     state->req_input.inp.name = NULL;
     state->exop_timeout = exop_timeout;
     state->attrs = NULL;
+    state->override_attrs = NULL;
 
     ret = ipa_s2n_get_groups_step(req);
     if (ret != EOK) {
@@ -1018,6 +1021,18 @@ static void ipa_s2n_get_groups_next(struct tevent_req *subreq)
         goto fail;
     }
 
+    if (strcmp(state->ipa_ctx->view_name, SYSDB_DEFAULT_VIEW_NAME) == 0) {
+        ret = ipa_s2n_get_groups_save_step(req);
+        if (ret == EOK) {
+            tevent_req_done(req);
+        } else if (ret != EAGAIN) {
+            DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_groups_save_step failed.\n");
+            goto fail;
+        }
+
+        return;
+    }
+
     ret = sysdb_attrs_get_string(state->attrs->sysdb_attrs, SYSDB_SID_STR,
                                  &sid_str);
     if (ret != EOK) {
@@ -1059,39 +1074,55 @@ static void ipa_s2n_get_groups_get_override_done(struct tevent_req *subreq)
                                                       struct tevent_req);
     struct ipa_s2n_get_groups_state *state = tevent_req_data(req,
                                                struct ipa_s2n_get_groups_state);
-    struct sysdb_attrs *override_attrs = NULL;
 
-    ret = ipa_get_ad_override_recv(subreq, NULL, state, &override_attrs);
+    ret = ipa_get_ad_override_recv(subreq, NULL, state, &state->override_attrs);
     talloc_zfree(subreq);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "IPA override lookup failed: %d\n", ret);
         goto fail;
     }
 
+    ret = ipa_s2n_get_groups_save_step(req);
+    if (ret == EOK) {
+        tevent_req_done(req);
+    } else if (ret != EAGAIN) {
+        DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_groups_save_step failed.\n");
+        goto fail;
+    }
+
+    return;
+
+fail:
+    tevent_req_error(req,ret);
+    return;
+}
+
+static errno_t ipa_s2n_get_groups_save_step(struct tevent_req *req)
+{
+    int ret;
+    struct ipa_s2n_get_groups_state *state = tevent_req_data(req,
+                                               struct ipa_s2n_get_groups_state);
+
     ret = ipa_s2n_save_objects(state->dom, &state->req_input, state->attrs,
-                               NULL, state->ipa_ctx->view_name, override_attrs);
+                               NULL, state->ipa_ctx->view_name,
+                               state->override_attrs);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_save_objects failed.\n");
-        goto fail;
+        return ret;
     }
 
     state->group_idx++;
     if (state->group_list[state->group_idx] == NULL) {
-        tevent_req_done(req);
-        return;
+        return EOK;
     }
 
     ret = ipa_s2n_get_groups_step(req);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_groups_step failed.\n");
-        goto fail;
+        return ret;
     }
 
-    return;
-
-fail:
-    tevent_req_error(req,ret);
-    return;
+    return EAGAIN;
 }
 
 static int ipa_s2n_get_groups_recv(struct tevent_req *req)
@@ -1484,7 +1515,9 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
         ret = ENOENT;
     }
 
-    if (ret == ENOENT) {
+    if (ret == ENOENT
+            || strcmp(state->ipa_ctx->view_name,
+                      SYSDB_DEFAULT_VIEW_NAME) == 0) {
         ret = ipa_s2n_save_objects(state->dom, state->req_input, state->attrs,
                                    state->simple_attrs, NULL, NULL);
         if (ret != EOK) {
@@ -2046,7 +2079,9 @@ static void ipa_s2n_get_groups_done(struct tevent_req  *subreq)
         goto fail;
     }
 
-    if (state->override_attrs == NULL) {
+    if (state->override_attrs == NULL
+            && strcmp(state->ipa_ctx->view_name,
+                      SYSDB_DEFAULT_VIEW_NAME) != 0) {
         subreq = ipa_get_ad_override_send(state, state->ev,
                            state->ipa_ctx->sdap_id_ctx,
                            state->ipa_ctx->ipa_options,
-- 
2.1.0

-------------- next part --------------
From 7942ebf653ca387cffa0b8a2b1404736bc0ae93f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 5 Dec 2014 11:03:48 +0100
Subject: [PATCH 2/8] IPA: make version check more precise

The call protected by the check does not only expect the version 1 of
the extdom plugin is used but a specific response type as well. Since
version 1 can return older response types as well we want to be on the
safe side.
---
 src/providers/ipa/ipa_s2n_exop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 1d9a79a383e47fcdd37f30a24327ae76facea5b9..b02387a827b7da27d944db8f7fa326f153e12715 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -759,7 +759,7 @@ static errno_t s2n_response_to_attrs(TALLOC_CTX *mem_ctx,
             attrs->a.user.pw_uid = uid;
             attrs->a.user.pw_gid = gid;
 
-            if (is_v1) {
+            if (is_v1 && type == RESP_USER_GROUPLIST) {
                 ret = add_v1_user_data(ber, attrs);
                 if (ret != EOK) {
                     DEBUG(SSSDBG_OP_FAILURE, "add_v1_user_data failed.\n");
@@ -798,7 +798,7 @@ static errno_t s2n_response_to_attrs(TALLOC_CTX *mem_ctx,
 
             attrs->a.group.gr_gid = gid;
 
-            if (is_v1) {
+            if (is_v1 && type == RESP_GROUP_MEMBERS) {
                 ret = add_v1_group_data(ber, attrs);
                 if (ret != EOK) {
                     DEBUG(SSSDBG_OP_FAILURE, "add_v1_group_data failed.\n");
-- 
2.1.0

-------------- next part --------------
From 534998ec4ddc1c202263b59303bf1339f35c970d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 5 Dec 2014 11:06:26 +0100
Subject: [PATCH 3/8] IPA: add missing break

The current request already returned the SID, we do not need to request
it separately.
---
 src/providers/ipa/ipa_s2n_exop.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index b02387a827b7da27d944db8f7fa326f153e12715..68f4cb7d8ff778342c5280652f0a62753c7c7b2c 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -1451,6 +1451,7 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
 
                 return;
             }
+            break;
         }
 
         if (state->req_input->type == REQ_INP_SECID
-- 
2.1.0

-------------- next part --------------
From 8c0dc8e05c96c95494ee51a60e41acb2fc5540d0 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 5 Dec 2014 11:11:49 +0100
Subject: [PATCH 4/8] IPA: process_members() optionally return missing members
 list

---
 src/providers/ipa/ipa_s2n_exop.c | 85 +++++++++++++++++++++++++++++++---------
 1 file changed, 67 insertions(+), 18 deletions(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 68f4cb7d8ff778342c5280652f0a62753c7c7b2c..bc476b469d02ea5f80d7e792693f5d25db889b74 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -1222,7 +1222,8 @@ fail:
 
 static errno_t process_members(struct sss_domain_info *domain,
                                struct sysdb_attrs *group_attrs,
-                               char **members)
+                               char **members,
+                               TALLOC_CTX *mem_ctx, char ***_missing_members)
 {
     int ret;
     size_t c;
@@ -1231,9 +1232,12 @@ static errno_t process_members(struct sss_domain_info *domain,
     const char *dn_str;
     struct sss_domain_info *obj_domain;
     struct sss_domain_info *parent_domain;
+    char **missing_members = NULL;
+    size_t miss_count = 0;
 
     if (members == NULL) {
         DEBUG(SSSDBG_TRACE_INTERNAL, "No members\n");
+        *_missing_members = NULL;
         return EOK;
     }
 
@@ -1243,6 +1247,17 @@ static errno_t process_members(struct sss_domain_info *domain,
         return ENOMEM;
     }
 
+    if (_missing_members != NULL && mem_ctx != NULL) {
+        /* count members */
+        for (c = 0; members[c] != NULL; c++);
+        missing_members = talloc_zero_array(tmp_ctx, char *, c + 1);
+        if (missing_members == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE, "talloc_array_zero failed.\n");
+            ret = ENOMEM;
+            goto done;
+        }
+    }
+
     parent_domain = get_domains_head(domain);
 
     for (c = 0; members[c] != NULL; c++) {
@@ -1256,27 +1271,46 @@ static errno_t process_members(struct sss_domain_info *domain,
         ret = sysdb_search_user_by_name(tmp_ctx, obj_domain, members[c], NULL,
                                         &msg);
         if (ret == EOK) {
-            dn_str = ldb_dn_get_linearized(msg->dn);
-            if (dn_str == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_get_linearized failed.\n");
-                goto done;
-            }
+            if (group_attrs != NULL) {
+                dn_str = ldb_dn_get_linearized(msg->dn);
+                if (dn_str == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_get_linearized failed.\n");
+                    goto done;
+                }
 
-            DEBUG(SSSDBG_TRACE_ALL, "Adding member [%s][%s]\n",
-                                    members[c], dn_str);
+                DEBUG(SSSDBG_TRACE_ALL, "Adding member [%s][%s]\n",
+                                        members[c], dn_str);
 
-            ret = sysdb_attrs_add_string(group_attrs, SYSDB_MEMBER, dn_str);
-            if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_add_string failed.\n");
-                goto done;
+                ret = sysdb_attrs_add_string(group_attrs, SYSDB_MEMBER, dn_str);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                          "sysdb_attrs_add_string failed.\n");
+                    goto done;
+                }
             }
         } else if (ret == ENOENT) {
-            DEBUG(SSSDBG_TRACE_ALL, "Adding ghost member [%s]\n", members[c]);
+            if (group_attrs != NULL) {
+                DEBUG(SSSDBG_TRACE_ALL, "Adding ghost member [%s]\n",
+                                        members[c]);
 
-            ret = sysdb_attrs_add_string(group_attrs, SYSDB_GHOST, members[c]);
-            if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_add_string failed.\n");
-                goto done;
+                ret = sysdb_attrs_add_string(group_attrs, SYSDB_GHOST,
+                                             members[c]);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                          "sysdb_attrs_add_string failed.\n");
+                    goto done;
+                }
+            }
+
+            if (missing_members != NULL) {
+                missing_members[miss_count] = talloc_strdup(missing_members,
+                                                            members[c]);
+                if (missing_members[miss_count] == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+                    ret = ENOMEM;
+                    goto done;
+                }
+                miss_count++;
             }
         } else {
             DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_name failed.\n");
@@ -1284,6 +1318,21 @@ static errno_t process_members(struct sss_domain_info *domain,
         }
     }
 
+    if (_missing_members != NULL)  {
+        if (miss_count == 0) {
+            *_missing_members = NULL;
+        } else {
+            if (mem_ctx != NULL) {
+                *_missing_members = talloc_steal(mem_ctx, missing_members);
+            } else {
+                DEBUG(SSSDBG_CRIT_FAILURE,
+                      "Missing memory context for missing members list.\n");
+                ret = EINVAL;
+                goto done;
+            }
+        }
+    }
+
     ret = EOK;
 done:
     talloc_free(tmp_ctx);
@@ -1986,7 +2035,7 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
             }
 
             ret = process_members(dom, attrs->sysdb_attrs,
-                                  attrs->a.group.gr_mem);
+                                  attrs->a.group.gr_mem, NULL, NULL);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, "process_members failed.\n");
                 goto done;
-- 
2.1.0

-------------- next part --------------
From b03eb36352b88e7eb69b2da59419f43ad3d1d82f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 9 Dec 2014 17:04:30 +0100
Subject: [PATCH 5/8] IPA: rename ipa_s2n_get_groups_send() to
 ipa_s2n_get_fqlist_send()

---
 src/providers/ipa/ipa_s2n_exop.c | 100 +++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 50 deletions(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index bc476b469d02ea5f80d7e792693f5d25db889b74..505c2325f2a74e26816bb59ad3c7d4810cc64dbf 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -877,38 +877,38 @@ done:
     return ret;
 }
 
-struct ipa_s2n_get_groups_state {
+struct ipa_s2n_get_fqlist_state {
     struct tevent_context *ev;
     struct ipa_id_ctx *ipa_ctx;
     struct sss_domain_info *dom;
     struct sdap_handle *sh;
     struct req_input req_input;
-    char **group_list;
-    size_t group_idx;
+    char **fqname_list;
+    size_t fqname_idx;
     int exop_timeout;
     struct resp_attrs *attrs;
     struct sss_domain_info *obj_domain;
     struct sysdb_attrs *override_attrs;
 };
 
-static errno_t ipa_s2n_get_groups_step(struct tevent_req *req);
-static void ipa_s2n_get_groups_get_override_done(struct tevent_req *subreq);
-static void ipa_s2n_get_groups_next(struct tevent_req *subreq);
-static errno_t ipa_s2n_get_groups_save_step(struct tevent_req *req);
+static errno_t ipa_s2n_get_fqlist_step(struct tevent_req *req);
+static void ipa_s2n_get_fqlist_get_override_done(struct tevent_req *subreq);
+static void ipa_s2n_get_fqlist_next(struct tevent_req *subreq);
+static errno_t ipa_s2n_get_fqlist_save_step(struct tevent_req *req);
 
-static struct tevent_req *ipa_s2n_get_groups_send(TALLOC_CTX *mem_ctx,
+static struct tevent_req *ipa_s2n_get_fqlist_send(TALLOC_CTX *mem_ctx,
                                                   struct tevent_context *ev,
                                                   struct ipa_id_ctx *ipa_ctx,
                                                   struct sss_domain_info *dom,
                                                   struct sdap_handle *sh,
                                                   int exop_timeout,
-                                                  char **group_list)
+                                                  char **fqname_list)
 {
     int ret;
-    struct ipa_s2n_get_groups_state *state;
+    struct ipa_s2n_get_fqlist_state *state;
     struct tevent_req *req;
 
-    req = tevent_req_create(mem_ctx, &state, struct ipa_s2n_get_groups_state);
+    req = tevent_req_create(mem_ctx, &state, struct ipa_s2n_get_fqlist_state);
     if (req == NULL) {
         return NULL;
     }
@@ -917,17 +917,17 @@ static struct tevent_req *ipa_s2n_get_groups_send(TALLOC_CTX *mem_ctx,
     state->ipa_ctx = ipa_ctx;
     state->dom = dom;
     state->sh = sh;
-    state->group_list = group_list;
-    state->group_idx = 0;
+    state->fqname_list = fqname_list;
+    state->fqname_idx = 0;
     state->req_input.type = REQ_INP_NAME;
     state->req_input.inp.name = NULL;
     state->exop_timeout = exop_timeout;
     state->attrs = NULL;
     state->override_attrs = NULL;
 
-    ret = ipa_s2n_get_groups_step(req);
+    ret = ipa_s2n_get_fqlist_step(req);
     if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_groups_step failed.\n");
+        DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_fqlist_step failed.\n");
         goto done;
     }
 
@@ -940,25 +940,25 @@ done:
     return req;
 }
 
-static errno_t ipa_s2n_get_groups_step(struct tevent_req *req)
+static errno_t ipa_s2n_get_fqlist_step(struct tevent_req *req)
 {
     int ret;
-    struct ipa_s2n_get_groups_state *state = tevent_req_data(req,
-                                               struct ipa_s2n_get_groups_state);
+    struct ipa_s2n_get_fqlist_state *state = tevent_req_data(req,
+                                               struct ipa_s2n_get_fqlist_state);
     struct berval *bv_req;
     struct tevent_req *subreq;
     struct sss_domain_info *parent_domain;
-    char *group_name = NULL;
+    char *short_name = NULL;
     char *domain_name = NULL;
 
     parent_domain = get_domains_head(state->dom);
 
     ret = sss_parse_name(state, parent_domain->names,
-                         state->group_list[state->group_idx],
-                         &domain_name, &group_name);
+                         state->fqname_list[state->fqname_idx],
+                         &domain_name, &short_name);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to parse name '%s' [%d]: %s\n",
-                                    state->group_list[state->group_idx],
+                                    state->fqname_list[state->fqname_idx],
                                     ret, sss_strerror(ret));
         return ret;
     }
@@ -974,7 +974,7 @@ static errno_t ipa_s2n_get_groups_step(struct tevent_req *req)
         state->obj_domain = parent_domain;
     }
 
-    state->req_input.inp.name = group_name;
+    state->req_input.inp.name = short_name;
 
     ret = s2n_encode_request(state, state->obj_domain->name, BE_REQ_GROUP,
                              REQ_FULL_WITH_MEMBERS,
@@ -990,18 +990,18 @@ static errno_t ipa_s2n_get_groups_step(struct tevent_req *req)
         DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_exop_send failed.\n");
         return ENOMEM;
     }
-    tevent_req_set_callback(subreq, ipa_s2n_get_groups_next, req);
+    tevent_req_set_callback(subreq, ipa_s2n_get_fqlist_next, req);
 
     return EOK;
 }
 
-static void ipa_s2n_get_groups_next(struct tevent_req *subreq)
+static void ipa_s2n_get_fqlist_next(struct tevent_req *subreq)
 {
     int ret;
     struct tevent_req *req = tevent_req_callback_data(subreq,
                                                       struct tevent_req);
-    struct ipa_s2n_get_groups_state *state = tevent_req_data(req,
-                                               struct ipa_s2n_get_groups_state);
+    struct ipa_s2n_get_fqlist_state *state = tevent_req_data(req,
+                                               struct ipa_s2n_get_fqlist_state);
     char *retoid = NULL;
     struct berval *retdata = NULL;
     const char *sid_str;
@@ -1022,11 +1022,11 @@ static void ipa_s2n_get_groups_next(struct tevent_req *subreq)
     }
 
     if (strcmp(state->ipa_ctx->view_name, SYSDB_DEFAULT_VIEW_NAME) == 0) {
-        ret = ipa_s2n_get_groups_save_step(req);
+        ret = ipa_s2n_get_fqlist_save_step(req);
         if (ret == EOK) {
             tevent_req_done(req);
         } else if (ret != EAGAIN) {
-            DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_groups_save_step failed.\n");
+            DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_fqlist_save_step failed.\n");
             goto fail;
         }
 
@@ -1058,7 +1058,7 @@ static void ipa_s2n_get_groups_next(struct tevent_req *subreq)
         ret = ENOMEM;
         goto fail;
     }
-    tevent_req_set_callback(subreq, ipa_s2n_get_groups_get_override_done, req);
+    tevent_req_set_callback(subreq, ipa_s2n_get_fqlist_get_override_done, req);
 
     return;
 
@@ -1067,13 +1067,13 @@ fail:
     return;
 }
 
-static void ipa_s2n_get_groups_get_override_done(struct tevent_req *subreq)
+static void ipa_s2n_get_fqlist_get_override_done(struct tevent_req *subreq)
 {
     int ret;
     struct tevent_req *req = tevent_req_callback_data(subreq,
                                                       struct tevent_req);
-    struct ipa_s2n_get_groups_state *state = tevent_req_data(req,
-                                               struct ipa_s2n_get_groups_state);
+    struct ipa_s2n_get_fqlist_state *state = tevent_req_data(req,
+                                               struct ipa_s2n_get_fqlist_state);
 
     ret = ipa_get_ad_override_recv(subreq, NULL, state, &state->override_attrs);
     talloc_zfree(subreq);
@@ -1082,11 +1082,11 @@ static void ipa_s2n_get_groups_get_override_done(struct tevent_req *subreq)
         goto fail;
     }
 
-    ret = ipa_s2n_get_groups_save_step(req);
+    ret = ipa_s2n_get_fqlist_save_step(req);
     if (ret == EOK) {
         tevent_req_done(req);
     } else if (ret != EAGAIN) {
-        DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_groups_save_step failed.\n");
+        DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_fqlist_save_step failed.\n");
         goto fail;
     }
 
@@ -1097,11 +1097,11 @@ fail:
     return;
 }
 
-static errno_t ipa_s2n_get_groups_save_step(struct tevent_req *req)
+static errno_t ipa_s2n_get_fqlist_save_step(struct tevent_req *req)
 {
     int ret;
-    struct ipa_s2n_get_groups_state *state = tevent_req_data(req,
-                                               struct ipa_s2n_get_groups_state);
+    struct ipa_s2n_get_fqlist_state *state = tevent_req_data(req,
+                                               struct ipa_s2n_get_fqlist_state);
 
     ret = ipa_s2n_save_objects(state->dom, &state->req_input, state->attrs,
                                NULL, state->ipa_ctx->view_name,
@@ -1111,21 +1111,21 @@ static errno_t ipa_s2n_get_groups_save_step(struct tevent_req *req)
         return ret;
     }
 
-    state->group_idx++;
-    if (state->group_list[state->group_idx] == NULL) {
+    state->fqname_idx++;
+    if (state->fqname_list[state->fqname_idx] == NULL) {
         return EOK;
     }
 
-    ret = ipa_s2n_get_groups_step(req);
+    ret = ipa_s2n_get_fqlist_step(req);
     if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_groups_step failed.\n");
+        DEBUG(SSSDBG_OP_FAILURE, "ipa_s2n_get_fqlist_step failed.\n");
         return ret;
     }
 
     return EAGAIN;
 }
 
-static int ipa_s2n_get_groups_recv(struct tevent_req *req)
+static int ipa_s2n_get_fqlist_recv(struct tevent_req *req)
 {
     TEVENT_REQ_RETURN_ON_ERROR(req);
 
@@ -1426,7 +1426,7 @@ done:
     return ret;
 }
 
-static void ipa_s2n_get_groups_done(struct tevent_req  *subreq);
+static void ipa_s2n_get_fqlist_done(struct tevent_req  *subreq);
 static void ipa_s2n_get_user_get_override_done(struct tevent_req *subreq);
 static void ipa_s2n_get_user_done(struct tevent_req *subreq)
 {
@@ -1485,17 +1485,17 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
             }
 
             if (missing_groups != NULL) {
-                subreq = ipa_s2n_get_groups_send(state, state->ev,
+                subreq = ipa_s2n_get_fqlist_send(state, state->ev,
                                                  state->ipa_ctx, state->dom,
                                                  state->sh, state->exop_timeout,
                                                  missing_groups);
                 if (subreq == NULL) {
                     DEBUG(SSSDBG_OP_FAILURE,
-                          "ipa_s2n_get_groups_send failed.\n");
+                          "ipa_s2n_get_fqlist_send failed.\n");
                     ret = ENOMEM;
                     goto done;
                 }
-                tevent_req_set_callback(subreq, ipa_s2n_get_groups_done,
+                tevent_req_set_callback(subreq, ipa_s2n_get_fqlist_done,
                                         req);
 
                 return;
@@ -2089,7 +2089,7 @@ done:
     return ret;
 }
 
-static void ipa_s2n_get_groups_done(struct tevent_req  *subreq)
+static void ipa_s2n_get_fqlist_done(struct tevent_req  *subreq)
 {
     int ret;
     struct tevent_req *req = tevent_req_callback_data(subreq,
@@ -2099,10 +2099,10 @@ static void ipa_s2n_get_groups_done(struct tevent_req  *subreq)
     const char *sid_str;
     struct be_acct_req *ar;
 
-    ret = ipa_s2n_get_groups_recv(subreq);
+    ret = ipa_s2n_get_fqlist_recv(subreq);
     talloc_zfree(subreq);
     if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "s2n get_groups request failed.\n");
+        DEBUG(SSSDBG_OP_FAILURE, "s2n get_fqlist request failed.\n");
         tevent_req_error(req, ret);
         return;
     }
-- 
2.1.0

-------------- next part --------------
From 20f5ba31de50f409867c4fba38c49b5e09cf91af Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 5 Dec 2014 11:12:42 +0100
Subject: [PATCH 6/8] IPA: resolve missing members

---
 src/providers/ipa/ipa_s2n_exop.c | 62 +++++++++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 14 deletions(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 505c2325f2a74e26816bb59ad3c7d4810cc64dbf..0aa12f371e8aa0d58311391a27c668aa929a5b80 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -886,6 +886,8 @@ struct ipa_s2n_get_fqlist_state {
     char **fqname_list;
     size_t fqname_idx;
     int exop_timeout;
+    int entry_type;
+    enum request_types request_type;
     struct resp_attrs *attrs;
     struct sss_domain_info *obj_domain;
     struct sysdb_attrs *override_attrs;
@@ -897,12 +899,14 @@ static void ipa_s2n_get_fqlist_next(struct tevent_req *subreq);
 static errno_t ipa_s2n_get_fqlist_save_step(struct tevent_req *req);
 
 static struct tevent_req *ipa_s2n_get_fqlist_send(TALLOC_CTX *mem_ctx,
-                                                  struct tevent_context *ev,
-                                                  struct ipa_id_ctx *ipa_ctx,
-                                                  struct sss_domain_info *dom,
-                                                  struct sdap_handle *sh,
-                                                  int exop_timeout,
-                                                  char **fqname_list)
+                                                struct tevent_context *ev,
+                                                struct ipa_id_ctx *ipa_ctx,
+                                                struct sss_domain_info *dom,
+                                                struct sdap_handle *sh,
+                                                int exop_timeout,
+                                                int entry_type,
+                                                enum request_types request_type,
+                                                char **fqname_list)
 {
     int ret;
     struct ipa_s2n_get_fqlist_state *state;
@@ -922,6 +926,8 @@ static struct tevent_req *ipa_s2n_get_fqlist_send(TALLOC_CTX *mem_ctx,
     state->req_input.type = REQ_INP_NAME;
     state->req_input.inp.name = NULL;
     state->exop_timeout = exop_timeout;
+    state->entry_type = entry_type;
+    state->request_type = request_type;
     state->attrs = NULL;
     state->override_attrs = NULL;
 
@@ -976,8 +982,8 @@ static errno_t ipa_s2n_get_fqlist_step(struct tevent_req *req)
 
     state->req_input.inp.name = short_name;
 
-    ret = s2n_encode_request(state, state->obj_domain->name, BE_REQ_GROUP,
-                             REQ_FULL_WITH_MEMBERS,
+    ret = s2n_encode_request(state, state->obj_domain->name, state->entry_type,
+                             state->request_type,
                              &state->req_input, &bv_req);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "s2n_encode_request failed.\n");
@@ -1439,7 +1445,7 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
     struct berval *retdata = NULL;
     struct resp_attrs *attrs = NULL;
     struct berval *bv_req = NULL;
-    char **missing_groups = NULL;
+    char **missing_list = NULL;
     struct ldb_dn **group_dn_list = NULL;
     const char *sid_str;
     struct be_acct_req *ar;
@@ -1478,17 +1484,46 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
         if (attrs->response_type == RESP_USER_GROUPLIST) {
             ret = get_group_dn_list(state, state->dom,
                                     attrs->ngroups, attrs->groups,
-                                    &group_dn_list, &missing_groups);
+                                    &group_dn_list, &missing_list);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, "get_group_dn_list failed.\n");
                 goto done;
             }
 
-            if (missing_groups != NULL) {
+            if (missing_list != NULL) {
                 subreq = ipa_s2n_get_fqlist_send(state, state->ev,
                                                  state->ipa_ctx, state->dom,
                                                  state->sh, state->exop_timeout,
-                                                 missing_groups);
+                                                 BE_REQ_GROUP,
+                                                 REQ_FULL_WITH_MEMBERS,
+                                                 missing_list);
+                if (subreq == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                          "ipa_s2n_get_fqlist_send failed.\n");
+                    ret = ENOMEM;
+                    goto done;
+                }
+                tevent_req_set_callback(subreq, ipa_s2n_get_fqlist_done,
+                                        req);
+
+                return;
+            }
+            break;
+        } else if (attrs->response_type == RESP_GROUP_MEMBERS) {
+            ret = process_members(state->dom, NULL, attrs->a.group.gr_mem,
+                                  state, &missing_list);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, "process_members failed.\n");
+                goto done;
+            }
+
+            if (missing_list != NULL) {
+                subreq = ipa_s2n_get_fqlist_send(state, state->ev,
+                                                 state->ipa_ctx, state->dom,
+                                                 state->sh, state->exop_timeout,
+                                                 BE_REQ_USER,
+                                                 REQ_FULL_WITH_MEMBERS,
+                                                 missing_list);
                 if (subreq == NULL) {
                     DEBUG(SSSDBG_OP_FAILURE,
                           "ipa_s2n_get_fqlist_send failed.\n");
@@ -1503,8 +1538,7 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
             break;
         }
 
-        if (state->req_input->type == REQ_INP_SECID
-                || attrs->response_type == RESP_GROUP_MEMBERS) {
+        if (state->req_input->type == REQ_INP_SECID) {
             /* We already know the SID, we do not have to read it. */
             break;
         }
-- 
2.1.0

-------------- next part --------------
From 8b0bed39bd9ebce3abf8d80b26cdf1355362bc58 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 9 Dec 2014 17:48:46 +0100
Subject: [PATCH 7/8] IPA: set SYSDB_INITGR_EXPIRE for RESP_USER_GROUPLIST

Since RESP_USER_GROUPLIST contains all group memberships it is
effectively an initgroups request hence SYSDB_INITGR_EXPIRE will be set.
---
 src/providers/ipa/ipa_s2n_exop.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 0aa12f371e8aa0d58311391a27c668aa929a5b80..e7c2d9bb97908746eb5ab6cacc6fc58d353dea06 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -1911,6 +1911,20 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
                 }
             }
 
+            if (attrs->response_type == RESP_USER_GROUPLIST) {
+                /* Since RESP_USER_GROUPLIST contains all group memberships it
+                 * is effectively an initgroups request hence
+                 * SYSDB_INITGR_EXPIRE will be set.*/
+                ret = sysdb_attrs_add_time_t(attrs->sysdb_attrs,
+                                             SYSDB_INITGR_EXPIRE,
+                                             time(NULL) + timeout);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                          "sysdb_attrs_add_time_t failed.\n");
+                    goto done;
+                }
+            }
+
             gid = 0;
             if (dom->mpg == false) {
                 gid = attrs->a.user.pw_gid;
-- 
2.1.0

-------------- next part --------------
From 046542d949ea08f1f221d4e7d780f8e102b2a1f9 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 12 Jan 2015 12:12:33 +0100
Subject: [PATCH 8/8] IPA: get_groups_dns() allow real search as well

---
 src/providers/ipa/ipa_s2n_exop.c | 43 ++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index e7c2d9bb97908746eb5ab6cacc6fc58d353dea06..c457495b7e3a3d69e8f37228f68af7d470a9edfb 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -1646,13 +1646,15 @@ done:
 }
 
 static errno_t get_groups_dns(TALLOC_CTX *mem_ctx, struct sss_domain_info *dom,
-                              char **name_list, char ***_dn_list)
+                              char **name_list, bool do_search,
+                              char ***_dn_list)
 {
     int ret;
     TALLOC_CTX *tmp_ctx;
     int c;
     struct sss_domain_info *root_domain;
     char **dn_list;
+    struct ldb_message *msg;
 
     if (name_list == NULL) {
         *_dn_list = NULL;
@@ -1697,15 +1699,32 @@ static errno_t get_groups_dns(TALLOC_CTX *mem_ctx, struct sss_domain_info *dom,
             goto done;
         }
 
-        /* This might fail if some unexpected cases are used. But current
-         * sysdb code which handles group membership constructs DNs this way
-         * as well, IPA names are lowercased and AD names by default will be
-         * lowercased as well. If there are really use-cases which cause an
-         * issue here, sysdb_group_strdn() has to be replaced by a proper
-         * search. */
-        dn_list[c] = sysdb_group_strdn(dn_list, dom->name, name_list[c]);
+        if (do_search) {
+            ret = sysdb_search_group_by_name(tmp_ctx, dom, name_list[c], NULL,
+                                             &msg);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE,
+                      "sysdb_search_group_by_name failed.\n");
+                goto done;
+            }
+
+            dn_list[c] = ldb_dn_alloc_linearized(dn_list, msg->dn);
+        } else {
+            /* This might fail if some unexpected cases are used. But current
+             * sysdb code which handles group membership constructs DNs this way
+             * as well, IPA names are lowercased and AD names by default will be
+             * lowercased as well. One exceptions are upper-case characters in
+             * AD domain names (not realm name). In this case the unmodified
+             * version of the domain name is used in the sysdb DNs as part of
+             * the fully qualified name and the name of the domain container.
+             * If the fully-qualified name is completely lower-cased, e.g. in
+             * the response of the extdom plugin a proper search must be used
+             * and do_search should be set to True. */
+            dn_list[c] = sysdb_group_strdn(dn_list, dom->name, name_list[c]);
+        }
         if (dn_list[c] == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_group_strdn failed.\n");
+            DEBUG(SSSDBG_OP_FAILURE, "%s failed.\n",
+                  do_search ? "ldb_dn_alloc_linearized" : "sysdb_group_strdn");
             ret = ENOMEM;
             goto done;
         }
@@ -1988,13 +2007,15 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
                     goto done;
                 }
 
-                ret = get_groups_dns(tmp_ctx, dom, add_groups, &add_groups_dns);
+                ret = get_groups_dns(tmp_ctx, dom, add_groups, true,
+                                     &add_groups_dns);
                 if (ret != EOK) {
                     DEBUG(SSSDBG_OP_FAILURE, "get_groups_dns failed.\n");
                     goto done;
                 }
 
-                ret = get_groups_dns(tmp_ctx, dom, del_groups, &del_groups_dns);
+                ret = get_groups_dns(tmp_ctx, dom, del_groups, false,
+                                     &del_groups_dns);
                 if (ret != EOK) {
                     DEBUG(SSSDBG_OP_FAILURE, "get_groups_dns failed.\n");
                     goto done;
-- 
2.1.0



More information about the sssd-devel mailing list