[SSSD] [PATCHES] Improve group handling in the PAC responder

Sumit Bose sbose at redhat.com
Tue Jan 8 08:29:46 UTC 2013


On Mon, Jan 07, 2013 at 01:22:26PM +0100, Jakub Hrozek wrote:
> On Mon, Jan 07, 2013 at 10:30:41AM +0100, Jakub Hrozek wrote:
> > On Fri, Jan 04, 2013 at 05:15:22PM -0500, Simo Sorce wrote:
> > > On Fri, 2013-01-04 at 17:15 +0100, Sumit Bose wrote:
> > > > On Fri, Jan 04, 2013 at 04:31:03PM +0100, Jakub Hrozek wrote:
> > > > > On Fri, Dec 21, 2012 at 12:31:32PM +0100, Sumit Bose wrote:
> > > > > > On Wed, Dec 19, 2012 at 08:10:02AM -0500, Simo Sorce wrote:
> > > > > > > On Tue, 2012-12-18 at 21:43 +0100, Sumit Bose wrote:
> > > > > > > > 
> > > > > > > > I started to change it but didn't like the resulting code, because it
> > > > > > > > turned out to be complex and hard to follow. Since the original struct
> > > > > > > > only user 8 or 12 bytes per group (depending if your are on 32bits or
> > > > > > > > on
> > > > > > > > 64bits) I would prefer to keep the old struct for this round of
> > > > > > > > patches.
> > > > > > > > 
> > > > > > > Sorry can't agree, space is only one of the factors, although completely
> > > > > > > unnecessarily using 2x or 3x number of groups space is ugly enough,
> > > > > > > But the main issues are really in the code this structure
> > > > > > > allows/produces.
> > > > > > > 
> > > > > > > Still a nack for me.
> > > > > > 
> > > > > > The attached patch should be applied on top of the other patches and
> > > > > > changes the struct as you suggested in your first comment.
> > > > > > 
> > > > > > bye,
> > > > > > Sumit
> > > > > 
> > > > > Why not squash the patch into the patch that introduces the structure?
> > > > 
> > > > The structure was introduced in the first patch and then used in some of
> > > > the other patches. I didn't squash the changes into the related patches
> > > > to not invalidate the reviews of the others and to allow a better
> > > > comparison between the old and the new structure.
> > > 
> > > 
> > > Ok I reviewd all patches except the last, which actually makes it harder
> > > as I would have to wrap my head again around 3/4 patches I already
> > > reviewed, so it would be really nice if it were folded in.
> > > I'll let Jakub decide what to do, maybe he will review the patch on ti's
> > > own and just ack it :)
> > 
> > Sumit explained it would be difficult to squash the patch into the
> > previous set. In general, it would be nicer to have only a single set,
> > but given our current constraints, I'm fine as long as the resulting
> > code works and looks sane.
> > 
> > I'll review the patch separately.
> 
> The last patch looks OK to me, I only asked Sumit to add some comments
> to place that wasn't so obvious in get_gids_from_pac().
> 
> I haven't read the previous patches too carefully, but I trust Simo's review.

Simo, Jakub, thank you very much for the review. The new set of patches
address all your latest comments.

bye,
Sumit
-------------- next part --------------
From d85f738db2aa9f5a7579e0b71fa4cd1a1bc1a8ff Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 23 Nov 2012 18:35:08 +0100
Subject: [PATCH 01/13] Use struct pac_grp instead of gid_t for groups from
 PAC

To be able to handle groupmemberships from other domains more data than
just the gid must be kept for groups given in the PAC.
---
 src/responder/pac/pacsrv.h       |   11 ++++++++---
 src/responder/pac/pacsrv_cmd.c   |    8 ++++----
 src/responder/pac/pacsrv_utils.c |   24 +++++++++++++-----------
 src/tests/pac_responder-tests.c  |   19 +++++++++++--------
 4 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
index 8b73d99..8cd4928 100644
--- a/src/responder/pac/pacsrv.h
+++ b/src/responder/pac/pacsrv.h
@@ -71,6 +71,11 @@ struct grp_info {
     struct ldb_dn *dn;
 };
 
+struct pac_grp {
+    gid_t gid;
+    struct sss_domain_info *grp_dom;
+};
+
 int pac_cmd_execute(struct cli_ctx *cctx);
 
 struct sss_cmd_table *get_pac_cmds(void);
@@ -98,7 +103,7 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
                           struct local_mapping_ranges *range_map,
                           struct dom_sid *domain_sid,
                           struct PAC_LOGON_INFO *logon_info,
-                          size_t *_gid_count, gid_t **_gids);
+                          size_t *_gid_count, struct pac_grp **_gids);
 
 errno_t get_data_from_pac(TALLOC_CTX *mem_ctx,
                           uint8_t *pac_blob, size_t pac_len,
@@ -115,9 +120,9 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
                        size_t cur_grp_num,
                        struct grp_info *cur_gid_list,
                        size_t new_gid_num,
-                       gid_t *new_gid_list,
+                       struct pac_grp *new_gid_list,
                        size_t *_add_gid_num,
-                       gid_t **_add_gid_list,
+                       struct pac_grp **_add_gid_list,
                        size_t *_del_gid_num,
                        struct grp_info ***_del_gid_list);
 #endif /* __PACSRV_H__ */
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 277cf4b..9f201f5 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -60,13 +60,13 @@ struct pac_req_ctx {
     struct dom_sid2 *domain_sid;
 
     size_t gid_count;
-    gid_t *gids;
+    struct pac_grp *gids;
 
     size_t current_grp_count;
     struct grp_info *current_grp_list;
 
     size_t add_gid_count;
-    gid_t *add_gids;
+    struct pac_grp *add_gids;
 
     size_t del_grp_count;
     struct grp_info **del_grp_list;
@@ -581,7 +581,7 @@ static errno_t pac_save_memberships_next(struct tevent_req *req)
     }
 
     while (state->gid_iter < pr_ctx->add_gid_count) {
-        gid = pr_ctx->add_gids[state->gid_iter];
+        gid = pr_ctx->add_gids[state->gid_iter].gid;
 
         ret = pac_store_membership(state->pr_ctx, state->group_dom->sysdb,
                                     state->user_dn, state->gid_iter);
@@ -671,7 +671,7 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
         return ENOMEM;
     }
 
-    gid = pr_ctx->add_gids[gid_iter];
+    gid = pr_ctx->add_gids[gid_iter].gid;
 
     ret = sysdb_search_group_by_gid(tmp_ctx, group_sysdb,
                                     gid, group_attrs, &group);
diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index 53113fb..6e0f4bf 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -389,13 +389,13 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
                           struct local_mapping_ranges *range_map,
                           struct dom_sid *domain_sid,
                           struct PAC_LOGON_INFO *logon_info,
-                          size_t *_gid_count, gid_t **_gids)
+                          size_t *_gid_count, struct pac_grp **_gids)
 {
     int ret;
     size_t g = 0;
     size_t s;
     struct netr_SamInfo3 *info3;
-    gid_t *gids = NULL;
+    struct pac_grp *gids = NULL;
 
     info3 = &logon_info->info3;
 
@@ -405,7 +405,7 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    gids = talloc_array(mem_ctx, gid_t, info3->sidcount);
+    gids = talloc_zero_array(mem_ctx, struct pac_grp, info3->sidcount);
     if (gids == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
         ret = ENOMEM;
@@ -414,13 +414,14 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
 
     for(s = 0; s < info3->sidcount; s++) {
         if (dom_sid_in_domain(domain_sid, info3->sids[s].sid)) {
-            ret = local_sid_to_id(range_map, info3->sids[s].sid, &gids[g]);
+            ret = local_sid_to_id(range_map, info3->sids[s].sid,
+                                  &gids[g].gid);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, ("get_rid failed.\n"));
                 goto done;
             }
             DEBUG(SSSDBG_TRACE_ALL, ("Found extra group "
-                                     "with gid [%d].\n", gids[g]));
+                                     "with gid [%d].\n", gids[g].gid));
             g++;
         }
     }
@@ -627,9 +628,9 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
                        size_t cur_grp_num,
                        struct grp_info *cur_grp_list,
                        size_t new_gid_num,
-                       gid_t *new_gid_list,
+                       struct pac_grp *new_gid_list,
                        size_t *_add_gid_num,
-                       gid_t **_add_gid_list,
+                       struct pac_grp **_add_gid_list,
                        size_t *_del_grp_num,
                        struct grp_info ***_del_grp_list)
 {
@@ -639,7 +640,7 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
     hash_key_t key;
     hash_value_t value;
     size_t add_gid_num = 0;
-    gid_t *add_gid_list = NULL;
+    struct pac_grp *add_gid_list = NULL;
     size_t del_grp_num = 0;
     struct grp_info **del_grp_list = NULL;
     TALLOC_CTX *tmp_ctx = NULL;
@@ -666,7 +667,7 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
 
     if (cur_grp_num == 0 && new_gid_num != 0) {
         add_gid_num = new_gid_num;
-        add_gid_list = talloc_array(tmp_ctx, gid_t, add_gid_num);
+        add_gid_list = talloc_array(tmp_ctx, struct pac_grp, add_gid_num);
         if (add_gid_list == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
             ret = ENOMEM;
@@ -721,13 +722,14 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
     }
 
     for (c = 0; c < new_gid_num; c++) {
-        key.ul = (unsigned long) new_gid_list[c];
+        key.ul = (unsigned long) new_gid_list[c].gid;
 
         ret = hash_delete(table, &key);
         if (ret == HASH_ERROR_KEY_NOT_FOUND) {
             /* gid not found, must be added */
             add_gid_num++;
-            add_gid_list = talloc_realloc(tmp_ctx, add_gid_list, gid_t, add_gid_num);
+            add_gid_list = talloc_realloc(tmp_ctx, add_gid_list, struct pac_grp,
+                                          add_gid_num);
             if (add_gid_list == NULL) {
                 DEBUG(SSSDBG_OP_FAILURE, ("talloc_realloc failed.\n"));
                 ret = ENOMEM;
diff --git a/src/tests/pac_responder-tests.c b/src/tests/pac_responder-tests.c
index 02cc242..11870ce 100644
--- a/src/tests/pac_responder-tests.c
+++ b/src/tests/pac_responder-tests.c
@@ -76,13 +76,16 @@ START_TEST(pac_test_get_gids_to_add_and_remove)
     int ret;
     size_t c;
     size_t add_gid_count = 0;
-    gid_t *add_gids = NULL;
+    struct pac_grp *add_gids = NULL;
     size_t del_gid_count = 0;
     struct grp_info **del_gids = NULL;
 
-    gid_t gid_list_2[] = {2};
-    gid_t gid_list_3[] = {3};
-    gid_t gid_list_23[] = {2, 3};
+    struct pac_grp pac_grp_2 = {2, NULL};
+    struct pac_grp pac_grp_3 = {3, NULL};
+
+    struct pac_grp gid_list_2[] = {pac_grp_2};
+    struct pac_grp gid_list_3[] = {pac_grp_3};
+    struct pac_grp gid_list_23[] = {pac_grp_2, pac_grp_3};
 
     struct grp_info grp_info_1 = {1, NULL, NULL};
     struct grp_info grp_info_2 = {2, NULL, NULL};
@@ -93,10 +96,10 @@ START_TEST(pac_test_get_gids_to_add_and_remove)
         size_t cur_gid_count;
         struct grp_info *cur_gids;
         size_t gid_count;
-        gid_t *gids;
+        struct pac_grp *gids;
         int exp_ret;
         size_t exp_add_gid_count;
-        gid_t *exp_add_gids;
+        struct pac_grp *exp_add_gids;
         size_t exp_del_gid_count;
         struct grp_info *exp_del_gids;
     } a_and_r_data[] = {
@@ -155,10 +158,10 @@ START_TEST(pac_test_get_gids_to_add_and_remove)
          * only look at lists with 1 element. TODO: add code to compare lists
          * with more than 1 member. */
         if (add_gid_count == 1) {
-            fail_unless(add_gids[0] ==  a_and_r_data[c].exp_add_gids[0],
+            fail_unless(add_gids[0].gid ==  a_and_r_data[c].exp_add_gids[0].gid,
                         "Unexpected gid to add for test data #%d, " \
                         "expected [%d], got [%d]",
-                        c, a_and_r_data[c].exp_add_gids[0], add_gids[0]);
+                        c, a_and_r_data[c].exp_add_gids[0].gid, add_gids[0].gid);
         }
 
         if (del_gid_count == 1) {
-- 
1.7.7.6

-------------- next part --------------
From 3646195585f673458c44c218fd1f2a5482253a6c Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 26 Nov 2012 12:33:11 +0100
Subject: [PATCH 02/13] Add find_domain_by_id()

Currently domains can only be searched by name in the global domain
list. To make it easier to find the domain for a given SID
find_domain_by_id() which returns a pointer to the domain or subdomain
entry in the global domain list if a matching id was found.
---
 src/responder/pac/pacsrv.h       |    3 ++
 src/responder/pac/pacsrv_utils.c |   39 ++++++++++++++++++++++++++++++
 src/tests/pac_responder-tests.c  |   49 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 0 deletions(-)

diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
index 8cd4928..4d3a316 100644
--- a/src/responder/pac/pacsrv.h
+++ b/src/responder/pac/pacsrv.h
@@ -125,4 +125,7 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
                        struct pac_grp **_add_gid_list,
                        size_t *_del_gid_num,
                        struct grp_info ***_del_gid_list);
+
+struct sss_domain_info *find_domain_by_id(struct sss_domain_info *domains,
+                                          const char *id_str);
 #endif /* __PACSRV_H__ */
diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index 6e0f4bf..d79adb1 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -71,6 +71,45 @@ errno_t local_sid_to_id(struct local_mapping_ranges *map, struct dom_sid *sid,
     return EOK;
 }
 
+struct sss_domain_info *find_domain_by_id(struct sss_domain_info *domains,
+                                          const char *id_str)
+{
+    struct sss_domain_info *dom;
+    struct sss_domain_info *ret_dom = NULL;
+    size_t c;
+
+    if (id_str == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Missing domain id.\n"));
+        return NULL;
+    }
+
+    for (dom = domains; dom; dom = dom->next) {
+        if (dom->domain_id == NULL) {
+            continue;
+        }
+
+        if (strcasecmp(dom->domain_id, id_str) == 0) {
+            ret_dom = dom;
+            break;
+        }
+
+        for (c = 0; c < dom->subdomain_count; c++) {
+            if (strcasecmp(dom->subdomains[c]->domain_id, id_str) == 0) {
+                ret_dom = dom->subdomains[c];
+                break;
+            }
+        }
+
+    }
+
+    if (!ret_dom) {
+        DEBUG(SSSDBG_OP_FAILURE, ("No domain with domain ID [%s] found",
+                                  id_str));
+    }
+
+    return ret_dom;
+}
+
 /**
  * Add a new remote domain and the corresponding ID range to the context of
  * the libsss_idmap. Without this it is not possible to find the Posix UID for
diff --git a/src/tests/pac_responder-tests.c b/src/tests/pac_responder-tests.c
index 11870ce..81cee3c 100644
--- a/src/tests/pac_responder-tests.c
+++ b/src/tests/pac_responder-tests.c
@@ -177,6 +177,54 @@ START_TEST(pac_test_get_gids_to_add_and_remove)
 }
 END_TEST
 
+#define NUM_DOMAINS 10
+START_TEST(pac_test_find_domain_by_id)
+{
+    struct sss_domain_info *dom;
+    struct sss_domain_info **domains;
+    size_t c;
+    char *id;
+
+    dom = find_domain_by_id(NULL, NULL);
+    fail_unless(dom == NULL, "Domain returned without any input.");
+
+    dom = find_domain_by_id(NULL, "id");
+    fail_unless(dom == NULL, "Domain returned without domain list.");
+
+    domains = talloc_zero_array(global_talloc_context, struct sss_domain_info *,
+                                NUM_DOMAINS);
+    for (c = 0; c < NUM_DOMAINS; c++) {
+        domains[c] = talloc_zero(domains, struct sss_domain_info);
+        fail_unless(domains[c] != NULL, "talloc_zero failed.");
+
+        domains[c]->domain_id = talloc_asprintf(domains[c],
+                                                "ID-of-domains-%zu", c);
+        fail_unless(domains[c]->domain_id != NULL, "talloc_asprintf failed.");
+        if (c > 0) {
+            domains[c-1]->next = domains[c];
+        }
+    }
+
+    dom = find_domain_by_id(domains[0], NULL);
+    fail_unless(dom == NULL, "Domain returned without search domain.");
+
+    dom = find_domain_by_id(domains[0], "DOES-NOT_EXISTS");
+    fail_unless(dom == NULL, "Domain returned with non existing id.");
+
+    for (c = 0; c < NUM_DOMAINS; c++) {
+        id = talloc_asprintf(global_talloc_context, "ID-of-domains-%zu", c);
+        fail_unless(id != NULL, "talloc_asprintf failed.\n");
+
+        dom = find_domain_by_id(domains[0], id);
+        fail_unless(dom == domains[c], "Wrong domain returned for id [%s].",
+                                       id);
+
+        talloc_free(id);
+    }
+
+    talloc_free(domains);
+}
+END_TEST
 
 Suite *idmap_test_suite (void)
 {
@@ -190,6 +238,7 @@ Suite *idmap_test_suite (void)
     tcase_add_test(tc_pac, pac_test_local_sid_to_id);
     tcase_add_test(tc_pac, pac_test_seondary_local_sid_to_id);
     tcase_add_test(tc_pac, pac_test_get_gids_to_add_and_remove);
+    tcase_add_test(tc_pac, pac_test_find_domain_by_id);
 
     suite_add_tcase(s, tc_pac);
 
-- 
1.7.7.6

-------------- next part --------------
From 028e713225f59a409a990b40d6cb95e79185ff08 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 26 Nov 2012 22:16:25 +0100
Subject: [PATCH 03/13] IDMAP: add sss_idmap_smb_sid_to_unix()

To avoid a conversion on the caller side a new call is added to
libsss_idmap which converts a Samba dom_sid structure to a Posix ID.
---
 src/lib/idmap/sss_idmap.c |   22 ++++++++++++++++++++++
 src/lib/idmap/sss_idmap.h |   17 +++++++++++++++++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index 3615e9d..1764b6f 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -422,6 +422,28 @@ done:
     return err;
 }
 
+enum idmap_error_code sss_idmap_smb_sid_to_unix(struct sss_idmap_ctx *ctx,
+                                                struct dom_sid *smb_sid,
+                                                uint32_t *id)
+{
+    enum idmap_error_code err;
+    char *sid;
+
+    CHECK_IDMAP_CTX(ctx, IDMAP_CONTEXT_INVALID);
+
+    err = sss_idmap_smb_sid_to_sid(ctx, smb_sid, &sid);
+    if (err != IDMAP_SUCCESS) {
+        goto done;
+    }
+
+    err = sss_idmap_sid_to_unix(ctx, sid, id);
+
+done:
+    ctx->free_func(sid, ctx->alloc_pvt);
+
+    return err;
+}
+
 enum idmap_error_code sss_idmap_unix_to_dom_sid(struct sss_idmap_ctx *ctx,
                                                 uint32_t id,
                                                 struct sss_dom_sid **_dom_sid)
diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h
index 22a4d54..ced7074 100644
--- a/src/lib/idmap/sss_idmap.h
+++ b/src/lib/idmap/sss_idmap.h
@@ -200,6 +200,23 @@ enum idmap_error_code sss_idmap_bin_sid_to_unix(struct sss_idmap_ctx *ctx,
                                                 uint32_t *id);
 
 /**
+ * @brief Translate a Samba dom_sid stucture to a unix UID or GID
+ *
+ * @param[in] ctx     Idmap context
+ * @param[in] smb_sid Samba dom_sid structure
+ * @param[out] id     Returned unix UID or GID
+ *
+ * @return
+ *  - #IDMAP_NO_DOMAIN:     No domains are added to the idmap context
+ *  - #IDMAP_SID_INVALID:   Invalid SID provided
+ *  - #IDMAP_SID_UNKNOWN:   SID cannot be found in the domains added to the
+ *                          idmap context
+ */
+enum idmap_error_code sss_idmap_smb_sid_to_unix(struct sss_idmap_ctx *ctx,
+                                                struct dom_sid *smb_sid,
+                                                uint32_t *id);
+
+/**
  * @brief Translate unix UID or GID to a SID
  *
  * @param[in] ctx  Idmap context
-- 
1.7.7.6

-------------- next part --------------
From 212c26387ce65985f480d598c93710d37380730b Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 26 Nov 2012 14:31:47 +0100
Subject: [PATCH 04/13] Update domain ID for local domain as well

Currently only the flat name of the configured domain is updated if it
is not already set. This patch updates the domain ID as well. This is
typically the case when trust support is enabled on the server side
while sssd is running.
---
 src/responder/common/responder_get_domains.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/responder/common/responder_get_domains.c b/src/responder/common/responder_get_domains.c
index 468dd76..c98c47b 100644
--- a/src/responder/common/responder_get_domains.c
+++ b/src/responder/common/responder_get_domains.c
@@ -329,7 +329,7 @@ process_subdomains(struct sss_domain_info *domain)
         --c;
     }
 
-    if (domain->flat_name == NULL) {
+    if (domain->flat_name == NULL || domain->domain_id == NULL) {
         ret = sysdb_master_domain_get_info(domain, domain->sysdb, &master_info);
         if (ret != EOK) {
                 DEBUG(SSSDBG_FUNC_DATA, ("sysdb_master_domain_get_info " \
@@ -337,7 +337,19 @@ process_subdomains(struct sss_domain_info *domain)
                 goto done;
         }
 
-        domain->flat_name = talloc_strdup(domain, master_info->flat_name);
+        if (domain->flat_name == NULL) {
+            domain->flat_name = talloc_strdup(domain, master_info->flat_name);
+            if (domain->flat_name == NULL) {
+                DEBUG(SSSDBG_MINOR_FAILURE, ("talloc_strdup failed, ignoring"));
+            }
+        }
+
+        if (domain->domain_id == NULL) {
+            domain->domain_id = talloc_strdup(domain, master_info->id);
+            if (domain->domain_id == NULL) {
+                DEBUG(SSSDBG_MINOR_FAILURE, ("talloc_strdup failed, ignoring"));
+            }
+        }
         talloc_free(master_info);
         DEBUG(SSSDBG_TRACE_LIBS, ("Adding flat name [%s] to domain [%s].\n",
                                   domain->flat_name, domain->name));
-- 
1.7.7.6

-------------- next part --------------
From 5cce6fb3495fd5e41ac11ad4a4eef331b9334091 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 27 Nov 2012 11:46:47 +0100
Subject: [PATCH 05/13] Always get user data from PAC

Currently some user specific data from the PAC is only read when the
user is not already in the cache. Since some of this information is
needed later on, e.g. the domain SID the user belongs to, with this
patch the data is read always from the PAC.
---
 src/responder/pac/pacsrv_cmd.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 9f201f5..d2609f5 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -380,18 +380,18 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx)
         goto done;
     }
 
+    ret = get_pwd_from_pac(tmp_ctx, pr_ctx->pac_ctx, pr_ctx->dom,
+                           pr_ctx->logon_info, &pwd, &user_attrs);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("get_pwd_from_pac failed.\n"));
+        goto done;
+    }
+
     ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->fq_name, attrs,
                                     &msg);
     if (ret == EOK) {
         /* TODO: check id uid and gid are equal. */
     } else if (ret == ENOENT) {
-        ret = get_pwd_from_pac(tmp_ctx, pr_ctx->pac_ctx, pr_ctx->dom,
-                               pr_ctx->logon_info, &pwd, &user_attrs);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, ("get_pwd_from_pac failed.\n"));
-            goto done;
-        }
-
         ret = sysdb_store_user(sysdb, pwd->pw_name, NULL,
                                pwd->pw_uid, pwd->pw_gid, pwd->pw_gecos,
                                pwd->pw_dir,
-- 
1.7.7.6

-------------- next part --------------
From d73f19323be3ef2f5be5aab0df785218f5144bc6 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 26 Nov 2012 13:50:48 +0100
Subject: [PATCH 06/13] Save domain and GID for groups from the configured
 domain

Currently users from subdomains can only be members of groups from the
configured domain and to access those groups a pointer to the domain
struct of the configured domain is used.  This patch sets the dom_grp
member of struct pac_grp to point to the domain struct of the configured
for groups from this domain. This is a first step to allow group
membership for groups from subdomains as well. For those groups a
pointer to the related subdomain structure will be saved.
---
 src/responder/pac/pacsrv.h       |    1 +
 src/responder/pac/pacsrv_cmd.c   |   36 +++++++++++++++++++-----------------
 src/responder/pac/pacsrv_utils.c |   27 +++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
index 4d3a316..71fcf8e 100644
--- a/src/responder/pac/pacsrv.h
+++ b/src/responder/pac/pacsrv.h
@@ -100,6 +100,7 @@ errno_t get_my_domain_data(struct pac_ctx *pac_ctx,
                            struct local_mapping_ranges **_range_map);
 
 errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
+                          struct pac_ctx *pac_ctx,
                           struct local_mapping_ranges *range_map,
                           struct dom_sid *domain_sid,
                           struct PAC_LOGON_INFO *logon_info,
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index d2609f5..16aad5d 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -82,9 +82,8 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx);
 static void pac_get_group_done(struct tevent_req *subreq);
 static errno_t pac_save_memberships_next(struct tevent_req *req);
 static errno_t pac_store_membership(struct pac_req_ctx *pr_ctx,
-                                     struct sysdb_ctx *group_sysdb,
-                                     struct ldb_dn *user_dn,
-                                     int gid_iter);
+                                    struct ldb_dn *user_dn,
+                                    int gid_iter);
 struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx);
 static void pac_save_memberships_done(struct tevent_req *req);
 
@@ -232,9 +231,9 @@ static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx)
         goto done;
     }
 
-    ret = get_gids_from_pac(pr_ctx, my_range_map, my_dom_sid,
-                            pr_ctx->logon_info, &pr_ctx->gid_count,
-                            &pr_ctx->gids);
+    ret = get_gids_from_pac(pr_ctx, pr_ctx->pac_ctx,
+                            my_range_map, my_dom_sid, pr_ctx->logon_info,
+                            &pr_ctx->gid_count, &pr_ctx->gids);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("get_gids_from_pac failed.\n"));
         goto done;
@@ -564,6 +563,7 @@ static errno_t pac_save_memberships_next(struct tevent_req *req)
 {
     errno_t ret;
     uint32_t gid;
+    struct sss_domain_info *grp_dom;
     struct tevent_req *subreq;
     struct pac_save_memberships_state *state;
     struct pac_req_ctx *pr_ctx;
@@ -581,16 +581,18 @@ static errno_t pac_save_memberships_next(struct tevent_req *req)
     }
 
     while (state->gid_iter < pr_ctx->add_gid_count) {
-        gid = pr_ctx->add_gids[state->gid_iter].gid;
 
-        ret = pac_store_membership(state->pr_ctx, state->group_dom->sysdb,
-                                    state->user_dn, state->gid_iter);
+        ret = pac_store_membership(state->pr_ctx, state->user_dn,
+                                   state->gid_iter);
         if (ret == EOK) {
             state->gid_iter++;
             continue;
         } else if (ret == ENOENT) {
+            gid = pr_ctx->add_gids[state->gid_iter].gid;
+            grp_dom = pr_ctx->add_gids[state->gid_iter].grp_dom;
+
             subreq = sss_dp_get_account_send(state, pr_ctx->cctx->rctx,
-                                             state->group_dom, true,
+                                             grp_dom, true,
                                              SSS_DP_GROUP, NULL,
                                              gid, NULL);
             if (subreq == NULL) {
@@ -632,8 +634,7 @@ static void pac_get_group_done(struct tevent_req *subreq)
         goto error;
     }
 
-    ret = pac_store_membership(state->pr_ctx, state->group_dom->sysdb,
-                                state->user_dn, state->gid_iter);
+    ret = pac_store_membership(state->pr_ctx, state->user_dn, state->gid_iter);
     if (ret != EOK) {
         goto error;
     }
@@ -654,14 +655,14 @@ error:
 
 static errno_t
 pac_store_membership(struct pac_req_ctx *pr_ctx,
-                      struct sysdb_ctx *group_sysdb,
-                      struct ldb_dn *user_dn,
-                      int gid_iter)
+                     struct ldb_dn *user_dn,
+                     int gid_iter)
 {
     TALLOC_CTX *tmp_ctx;
     struct sysdb_attrs *user_attrs;
     struct ldb_message *group;
     uint32_t gid;
+    struct sss_domain_info *grp_dom;
     errno_t ret;
     const char *orig_group_dn;
     const char *group_attrs[] = { SYSDB_ORIG_DN, NULL };
@@ -672,8 +673,9 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
     }
 
     gid = pr_ctx->add_gids[gid_iter].gid;
+    grp_dom = pr_ctx->add_gids[gid_iter].grp_dom;
 
-    ret = sysdb_search_group_by_gid(tmp_ctx, group_sysdb,
+    ret = sysdb_search_group_by_gid(tmp_ctx, grp_dom->sysdb,
                                     gid, group_attrs, &group);
     if (ret != EOK) {
         DEBUG(SSSDBG_TRACE_INTERNAL, ("sysdb_search_group_by_gid for gid [%d]" \
@@ -682,7 +684,7 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
         goto done;
     }
 
-    ret = sysdb_mod_group_member(group_sysdb, user_dn, group->dn,
+    ret = sysdb_mod_group_member(grp_dom->sysdb, user_dn, group->dn,
                                  LDB_FLAG_MOD_ADD);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("sysdb_mod_group_member failed.\n"));
diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index d79adb1..217e27a 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -425,6 +425,7 @@ bool dom_sid_in_domain(const struct dom_sid *domain_sid,
  * domain and convert them to GIDs.
  */
 errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
+                          struct pac_ctx *pac_ctx,
                           struct local_mapping_ranges *range_map,
                           struct dom_sid *domain_sid,
                           struct PAC_LOGON_INFO *logon_info,
@@ -435,6 +436,15 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
     size_t s;
     struct netr_SamInfo3 *info3;
     struct pac_grp *gids = NULL;
+    struct sss_domain_info *grp_dom;
+    char *sid_str;
+    enum idmap_error_code err;
+
+    if (pac_ctx == NULL || range_map == NULL || domain_sid == NULL ||
+        logon_info == NULL || _gid_count == NULL || _gids == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Missing parameter.\n"));
+        return EINVAL;
+    }
 
     info3 = &logon_info->info3;
 
@@ -451,6 +461,22 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
+
+    err = sss_idmap_smb_sid_to_sid(pac_ctx->idmap_ctx, domain_sid,
+                                   &sid_str);
+    if (err != IDMAP_SUCCESS) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sss_idmap_smb_sid_to_sid failed.\n"));
+        ret = EFAULT;
+        goto done;
+    }
+
+    grp_dom =  find_domain_by_id(pac_ctx->rctx->domains, sid_str);
+    if (grp_dom == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("find_domain_by_id failed.\n"));
+        ret = EINVAL;
+        goto done;
+    }
+
     for(s = 0; s < info3->sidcount; s++) {
         if (dom_sid_in_domain(domain_sid, info3->sids[s].sid)) {
             ret = local_sid_to_id(range_map, info3->sids[s].sid,
@@ -459,6 +485,7 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
                 DEBUG(SSSDBG_OP_FAILURE, ("get_rid failed.\n"));
                 goto done;
             }
+            gids[g].grp_dom = grp_dom;
             DEBUG(SSSDBG_TRACE_ALL, ("Found extra group "
                                      "with gid [%d].\n", gids[g].gid));
             g++;
-- 
1.7.7.6

-------------- next part --------------
From 45d98861d350808252aafb029e8e751a21753d57 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 27 Nov 2012 13:43:42 +0100
Subject: [PATCH 07/13] Remote groups do not have an original DN attribute

Groups from subdomains will not have an attribute holding the original
DN because in general it will not be available. This attribute is only
used by IPA HABC to improve performance and remote groups cannot be used
for access control.
---
 src/responder/pac/pacsrv_cmd.c |   74 ++++++++++++++++++---------------------
 1 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 16aad5d..49164ab 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -320,18 +320,14 @@ static errno_t pac_user_get_grp_info(TALLOC_CTX *mem_ctx,
 
             tmp_str = ldb_msg_find_attr_as_string(res->msgs[c + 1],
                                                   SYSDB_ORIG_DN, NULL);
-            if (tmp_str == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE, ("Missing original DN.\n"));
-                ret = EINVAL;
-                goto done;
-            }
-
-            current_grp_list[c].orig_dn = talloc_strdup(current_grp_list,
-                                                        tmp_str);
-            if (current_grp_list[c].orig_dn == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
-                ret = ENOMEM;
-                goto done;
+            if (tmp_str != NULL) {
+                current_grp_list[c].orig_dn = talloc_strdup(current_grp_list,
+                                                            tmp_str);
+                if (current_grp_list[c].orig_dn == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
+                    ret = ENOMEM;
+                    goto done;
+                }
             }
 
             current_grp_list[c].dn = ldb_dn_copy(current_grp_list,
@@ -523,11 +519,13 @@ pac_save_memberships_delete(struct pac_save_memberships_state *state)
             goto done;
         }
 
-        ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF,
-                                     pr_ctx->del_grp_list[c]->orig_dn);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
-            goto done;
+        if (pr_ctx->del_grp_list[c]->orig_dn != NULL) {
+            ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF,
+                                         pr_ctx->del_grp_list[c]->orig_dn);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
+                goto done;
+            }
         }
     }
 
@@ -692,31 +690,27 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
     }
 
     orig_group_dn = ldb_msg_find_attr_as_string(group, SYSDB_ORIG_DN, NULL);
-    if (orig_group_dn == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, ("Original DN not found.\n"));
-        ret = EINVAL;
-        goto done;
-    }
-
-    user_attrs = sysdb_new_attrs(tmp_ctx);
-    if (user_attrs == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_new_attrs failed.\n"));
-        ret = ENOMEM;
-        goto done;
-    }
+    if (orig_group_dn != NULL) {
+        user_attrs = sysdb_new_attrs(tmp_ctx);
+        if (user_attrs == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_new_attrs failed.\n"));
+            ret = ENOMEM;
+            goto done;
+        }
 
-    ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF,
-                                 orig_group_dn);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
-        goto done;
-    }
+        ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF,
+                                     orig_group_dn);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
+            goto done;
+        }
 
-    ret = sysdb_set_entry_attr(pr_ctx->dom->sysdb, user_dn, user_attrs,
-                              LDB_FLAG_MOD_ADD);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_set_entry_attr failed.\n"));
-        goto done;
+        ret = sysdb_set_entry_attr(pr_ctx->dom->sysdb, user_dn, user_attrs,
+                                  LDB_FLAG_MOD_ADD);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_set_entry_attr failed.\n"));
+            goto done;
+        }
     }
 
 done:
-- 
1.7.7.6

-------------- next part --------------
From a2b3304ff47ef0e5bbe7699114fdca8b1e331650 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 26 Nov 2012 22:16:49 +0100
Subject: [PATCH 08/13] Read remote groups from PAC

Read the group membership of the remote domain the user belongs to from
the PAC and add them to the cache.

Fixes: https://fedorahosted.org/sssd/ticket/1666
---
 src/responder/pac/pacsrv_utils.c |   55 +++++++++++++++++++++++++++++++++++--
 1 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index 217e27a..2daced2 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -437,8 +437,9 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
     struct netr_SamInfo3 *info3;
     struct pac_grp *gids = NULL;
     struct sss_domain_info *grp_dom;
-    char *sid_str;
+    char *sid_str = NULL;
     enum idmap_error_code err;
+    struct dom_sid *grp_sid = NULL;
 
     if (pac_ctx == NULL || range_map == NULL || domain_sid == NULL ||
         logon_info == NULL || _gid_count == NULL || _gids == NULL) {
@@ -448,13 +449,14 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
 
     info3 = &logon_info->info3;
 
-    if (info3->sidcount == 0) {
+    if (info3->sidcount == 0 && info3->base.groups.count == 0) {
         DEBUG(SSSDBG_TRACE_ALL, ("No extra groups found.\n"));
         ret = EOK;
         goto done;
     }
 
-    gids = talloc_zero_array(mem_ctx, struct pac_grp, info3->sidcount);
+    gids = talloc_zero_array(mem_ctx, struct pac_grp,
+                             info3->sidcount + info3->base.groups.count);
     if (gids == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
         ret = ENOMEM;
@@ -492,9 +494,56 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
         }
     }
 
+    talloc_zfree(sid_str);
+    err = sss_idmap_smb_sid_to_sid(pac_ctx->idmap_ctx, info3->base.domain_sid,
+                                   &sid_str);
+    if (err != IDMAP_SUCCESS) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sss_idmap_smb_sid_to_sid failed.\n"));
+        ret = EFAULT;
+        goto done;
+    }
+
+    grp_dom =  find_domain_by_id(pac_ctx->rctx->domains, sid_str);
+    if (grp_dom == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("find_domain_by_id failed.\n"));
+        ret = EINVAL;
+        goto done;
+    }
+
+    err = sss_idmap_sid_to_smb_sid(pac_ctx->idmap_ctx, sid_str, &grp_sid);
+    if (err != IDMAP_SUCCESS) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sss_idmap_sid_to_smb_sid failed.\n"));
+        ret = EFAULT;
+        goto done;
+    }
+
+    grp_sid->num_auths++;
+
+    for (s = 0; s < info3->base.groups.count; s++) {
+        grp_sid->sub_auths[grp_sid->num_auths - 1] =
+                                                info3->base.groups.rids[s].rid;
+        err = sss_idmap_smb_sid_to_unix(pac_ctx->idmap_ctx, grp_sid,
+                                        &gids[g].gid);
+        if (err != IDMAP_SUCCESS) {
+            DEBUG(SSSDBG_FATAL_FAILURE, ("sss_idmap_smb_sid_to_unix failed for"
+                                         "[%s] [%d].\n", sid_str,
+                                         info3->base.groups.rids[s].rid));
+            ret = ENOENT;
+            goto done;
+        }
+
+        gids[g].grp_dom = grp_dom;
+        DEBUG(SSSDBG_TRACE_ALL, ("Found extra group "
+                                 "with gid [%d].\n", gids[g].gid));
+        g++;
+    }
+
     ret = EOK;
 
 done:
+    talloc_free(sid_str);
+    talloc_free(grp_sid);
+
     if (ret == EOK) {
         *_gid_count = g;
         *_gids = gids;
-- 
1.7.7.6

-------------- next part --------------
From c0409de86ac4c23160191f4c72dcda2ffd38e10f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 27 Nov 2012 12:42:02 +0100
Subject: [PATCH 09/13] Translate LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS to EEXIST

Currently only the LDB error code indicating that an entry already
exists is translated to EEXIST. To make debugging easier and return a
better indication of the reason for an error in the logs this patch
translates the LDB error code for an already existing attribute or value
to EEXIST as well.
---
 src/db/sysdb.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/db/sysdb.c b/src/db/sysdb.c
index a49fa50..b0bea9a 100644
--- a/src/db/sysdb.c
+++ b/src/db/sysdb.c
@@ -737,6 +737,7 @@ int sysdb_error_to_errno(int ldberr)
         return ENOENT;
     case LDB_ERR_BUSY:
         return EBUSY;
+    case LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS:
     case LDB_ERR_ENTRY_ALREADY_EXISTS:
         return EEXIST;
     case LDB_ERR_INVALID_ATTRIBUTE_SYNTAX:
-- 
1.7.7.6

-------------- next part --------------
From b64c37f13a12c0c88caa1f115b60606f4e74de46 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 3 Dec 2012 16:09:24 +0100
Subject: [PATCH 10/13] Use hash table to collect GIDs from PAC to avoid dups

To avoid duplicated entries in the group list all gids are added to a
hash table first.

Fixes: https://fedorahosted.org/sssd/ticket/1672
---
 src/responder/pac/pacsrv_utils.c |  104 +++++++++++++++++++++++++++++++-------
 1 files changed, 86 insertions(+), 18 deletions(-)

diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index 2daced2..4c3ecb2 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -432,7 +432,8 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
                           size_t *_gid_count, struct pac_grp **_gids)
 {
     int ret;
-    size_t g = 0;
+    size_t g;
+    size_t gid_count = 0;
     size_t s;
     struct netr_SamInfo3 *info3;
     struct pac_grp *gids = NULL;
@@ -440,6 +441,13 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
     char *sid_str = NULL;
     enum idmap_error_code err;
     struct dom_sid *grp_sid = NULL;
+    uint32_t id;
+    hash_table_t *gid_table;
+    hash_key_t key;
+    hash_value_t value;
+    struct hash_iter_context_t *iter;
+    hash_entry_t *entry;
+    TALLOC_CTX *tmp_ctx = NULL;
 
     if (pac_ctx == NULL || range_map == NULL || domain_sid == NULL ||
         logon_info == NULL || _gid_count == NULL || _gids == NULL) {
@@ -455,14 +463,22 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    gids = talloc_zero_array(mem_ctx, struct pac_grp,
-                             info3->sidcount + info3->base.groups.count);
-    if (gids == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_new failed.\n"));
         ret = ENOMEM;
         goto done;
     }
 
+    ret = sss_hash_create(tmp_ctx, info3->sidcount + info3->base.groups.count,
+                          &gid_table);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sss_hash_create failed.\n"));
+        goto done;
+    }
+
+    key.type = HASH_KEY_ULONG;
+    value.type = HASH_VALUE_PTR;
 
     err = sss_idmap_smb_sid_to_sid(pac_ctx->idmap_ctx, domain_sid,
                                    &sid_str);
@@ -482,15 +498,24 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
     for(s = 0; s < info3->sidcount; s++) {
         if (dom_sid_in_domain(domain_sid, info3->sids[s].sid)) {
             ret = local_sid_to_id(range_map, info3->sids[s].sid,
-                                  &gids[g].gid);
+                                  &id);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, ("get_rid failed.\n"));
                 goto done;
             }
-            gids[g].grp_dom = grp_dom;
-            DEBUG(SSSDBG_TRACE_ALL, ("Found extra group "
-                                     "with gid [%d].\n", gids[g].gid));
-            g++;
+
+            key.ul = id;
+            value.ptr = grp_dom;
+
+            ret = hash_enter(gid_table, &key, &value);
+            if (ret != HASH_SUCCESS) {
+                DEBUG(SSSDBG_OP_FAILURE, ("hash_enter failed [%d][%s].\n",
+                                          ret, hash_error_string(ret)));
+                ret = EIO;
+                goto done;
+            }
+
+            DEBUG(SSSDBG_TRACE_ALL, ("Found extra group with gid [%d].\n", id));
         }
     }
 
@@ -523,7 +548,7 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
         grp_sid->sub_auths[grp_sid->num_auths - 1] =
                                                 info3->base.groups.rids[s].rid;
         err = sss_idmap_smb_sid_to_unix(pac_ctx->idmap_ctx, grp_sid,
-                                        &gids[g].gid);
+                                        &id);
         if (err != IDMAP_SUCCESS) {
             DEBUG(SSSDBG_FATAL_FAILURE, ("sss_idmap_smb_sid_to_unix failed for"
                                          "[%s] [%d].\n", sid_str,
@@ -532,12 +557,55 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
             goto done;
         }
 
-        gids[g].grp_dom = grp_dom;
-        DEBUG(SSSDBG_TRACE_ALL, ("Found extra group "
-                                 "with gid [%d].\n", gids[g].gid));
+        key.ul = id;
+        value.ptr = grp_dom;
+
+        ret = hash_enter(gid_table, &key, &value);
+        if (ret != HASH_SUCCESS) {
+            DEBUG(SSSDBG_OP_FAILURE, ("hash_enter failed [%d][%s].\n",
+                                      ret, hash_error_string(ret)));
+            ret = EIO;
+            goto done;
+        }
+
+        DEBUG(SSSDBG_TRACE_ALL, ("Found extra group with gid [%d].\n", id));
+    }
+
+    gid_count = hash_count(gid_table);
+    if (gid_count == 0) {
+        DEBUG(SSSDBG_TRACE_FUNC, ("No groups found.\n"));
+        ret = EOK;
+        goto done;
+    }
+
+    gids = talloc_zero_array(tmp_ctx, struct pac_grp, gid_count);
+    if (gids == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
+        ret = ENOMEM;
+        goto done;
+    }
+
+    iter = new_hash_iter_context(gid_table);
+    if (iter == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("new_hash_iter_context failed.\n"));
+        ret = EIO;
+        goto done;
+    }
+
+    g = 0;
+    while ((entry = iter->next(iter)) != NULL) {
+        gids[g].gid = entry->key.ul;
+        gids[g].grp_dom = entry->value.ptr;
         g++;
     }
 
+    if (gid_count != g) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Number of hash entries and groups do not "
+                                  "match.\n"));
+        ret = EINVAL;
+        goto done;
+    }
+
     ret = EOK;
 
 done:
@@ -545,12 +613,12 @@ done:
     talloc_free(grp_sid);
 
     if (ret == EOK) {
-        *_gid_count = g;
-        *_gids = gids;
-    } else {
-        talloc_free(gids);
+        *_gid_count = gid_count;
+        *_gids = talloc_steal(mem_ctx, gids);
     }
 
+    talloc_free(tmp_ctx);
+
     return ret;
 }
 
-- 
1.7.7.6

-------------- next part --------------
From e0acf860177d44fe7e5d5a05d7fcb3a4ebc57605 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 4 Dec 2012 13:55:13 +0100
Subject: [PATCH 11/13] Add tests for get_gids_from_pac()

---
 Makefile.am                     |    1 +
 src/tests/pac_responder-tests.c |  256 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 257 insertions(+), 0 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b6fd359..c887e03 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1077,6 +1077,7 @@ pac_responder_tests_LDADD = \
     $(TALLOC_LIBS) \
     $(NDR_KRB5PAC_LIBS) \
     $(SSSD_LIBS) \
+    -lm \
     libsss_idmap.la \
     libsss_debug.la \
     libsss_util.la \
diff --git a/src/tests/pac_responder-tests.c b/src/tests/pac_responder-tests.c
index 81cee3c..b2a1457 100644
--- a/src/tests/pac_responder-tests.c
+++ b/src/tests/pac_responder-tests.c
@@ -23,11 +23,23 @@
 #include <check.h>
 
 #include <stdbool.h>
+#include <math.h>
 #include <util/data_blob.h>
 #include <gen_ndr/security.h>
 
 #include "tests/common.h"
 #include "responder/pac/pacsrv.h"
+#include "lib/idmap/sss_idmap.h"
+
+struct dom_sid test_dom_sid = {1, 4, {0, 0, 0, 0, 0, 5},
+                               {21, 2127521184, 1604012920, 1887927527, 0,
+                                0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+const char *test_dom_sid_str = "S-1-5-21-2127521184-1604012920-1887927527";
+
+struct dom_sid test_remote_dom_sid = {1, 4, {0, 0, 0, 0, 0, 5},
+                                      {21, 123, 456, 789, 0,
+                                       0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+const char *test_remote_dom_sid_str = "S-1-5-21-123-456-789";
 
 struct dom_sid test_smb_sid = {1, 5, {0, 0, 0, 0, 0, 5},
                                {21, 2127521184, 1604012920, 1887927527, 1123,
@@ -43,6 +55,90 @@ struct local_mapping_ranges test_map = {{1200000, 1399999},
                                         {1000, 200999},
                                         {201000, 400999}};
 
+static void *idmap_talloc(size_t size, void *pvt)
+{
+    return talloc_size(pvt, size);
+}
+
+static void idmap_talloc_free(void *ptr, void *pvt)
+{
+    talloc_free(ptr);
+}
+
+struct pac_ctx *pac_ctx;
+
+#define IDMAP_RANGE_MIN 1234
+#define IDMAP_RANGE_MAX 9876543
+
+void pac_setup(void) {
+    enum idmap_error_code err;
+    struct sss_idmap_range remote_range = {IDMAP_RANGE_MIN, IDMAP_RANGE_MAX};
+    struct sss_domain_info *sd;
+
+    pac_ctx = talloc_zero(global_talloc_context, struct pac_ctx);
+    fail_unless(pac_ctx != NULL, "talloc_zero failed.\n");
+
+    pac_ctx->rctx = talloc_zero(pac_ctx, struct resp_ctx);
+    fail_unless(pac_ctx->rctx != NULL, "talloc_zero failed.");
+
+    pac_ctx->rctx->domains = talloc_zero(pac_ctx->rctx, struct sss_domain_info);
+    fail_unless(pac_ctx->rctx->domains != NULL, "talloc_zero failed.");
+
+    pac_ctx->rctx->domains->name = talloc_strdup(pac_ctx->rctx->domains,
+                                                 "TEST.DOM");
+    fail_unless(pac_ctx->rctx->domains->name != NULL, "talloc_strdup failed.");
+
+    pac_ctx->rctx->domains->flat_name = talloc_strdup(pac_ctx->rctx->domains,
+                                                      "TESTDOM");
+    fail_unless(pac_ctx->rctx->domains->flat_name != NULL,
+                "talloc_strdup failed.");
+
+    pac_ctx->rctx->domains->domain_id = talloc_strdup(pac_ctx->rctx->domains,
+                                                      test_dom_sid_str);
+    fail_unless(pac_ctx->rctx->domains->domain_id != NULL,
+                "talloc_strdup failed.");
+
+    pac_ctx->rctx->domains->subdomain_count = 1;
+    pac_ctx->rctx->domains->subdomains = talloc_zero_array(pac_ctx->rctx->domains,
+                                                       struct sss_domain_info *,
+                                       pac_ctx->rctx->domains->subdomain_count);
+    fail_unless(pac_ctx->rctx->domains->subdomains != NULL,
+                "talloc_array_zero failed");
+
+    sd = talloc_zero(pac_ctx->rctx->domains->subdomains,
+                     struct sss_domain_info);
+    fail_unless(sd != NULL, "talloc_zero failed.");
+
+    sd->name = talloc_strdup(sd, "remote.dom");
+    fail_unless(sd->name != NULL, "talloc_strdup failed");
+
+    sd->flat_name = talloc_strdup(sd, "REMOTEDOM");
+    fail_unless(sd->flat_name != NULL, "talloc_strdup failed");
+
+    sd->domain_id = talloc_strdup(sd, test_remote_dom_sid_str);
+    fail_unless(sd->domain_id != NULL, "talloc_strdup failed");
+
+    pac_ctx->rctx->domains->subdomains[0] = sd;
+
+    err = sss_idmap_init(idmap_talloc, pac_ctx, idmap_talloc_free,
+                         &pac_ctx->idmap_ctx);
+
+    fail_unless(err == IDMAP_SUCCESS, "sss_idmap_init failed.");
+    fail_unless(pac_ctx->idmap_ctx != NULL, "sss_idmap_init returned NULL.");
+
+    err = sss_idmap_add_domain(pac_ctx->idmap_ctx, "remote.dom",
+                               test_remote_dom_sid_str, &remote_range);
+
+    pac_ctx->my_dom_sid = &test_dom_sid;
+
+    pac_ctx->range_map = &test_map;
+}
+
+void pac_teardown(void)
+{
+    talloc_free(pac_ctx);
+}
+
 
 START_TEST(pac_test_local_sid_to_id)
 {
@@ -226,6 +322,161 @@ START_TEST(pac_test_find_domain_by_id)
 }
 END_TEST
 
+START_TEST(pac_test_get_gids_from_pac)
+{
+    int ret;
+    size_t c;
+    size_t g;
+    size_t t;
+    size_t gid_count;
+    struct pac_grp *gids;
+    struct PAC_LOGON_INFO *logon_info;
+    bool found;
+    gid_t exp_gid;
+    struct timeval start_time;
+    struct timeval end_time;
+    struct timeval diff_time;
+
+    ret = get_gids_from_pac(NULL, NULL, NULL, NULL, NULL, NULL, NULL);
+    fail_unless(ret == EINVAL, "Unexpected return value for NULL parameters");
+
+    logon_info = talloc_zero(global_talloc_context, struct PAC_LOGON_INFO);
+    fail_unless(logon_info != NULL, "talloc_zero failed.\n");
+
+    ret = get_gids_from_pac(global_talloc_context, pac_ctx, pac_ctx->range_map,
+                            pac_ctx->my_dom_sid, logon_info, &gid_count, &gids);
+    fail_unless(ret == EOK, "Failed with empty PAC");
+    fail_unless(gid_count == 0, "O groups expected, got [%d]", gid_count);
+    fail_unless(gids == NULL, "Expected NULL gid array.");
+
+    logon_info->info3.base.domain_sid = &test_smb_sid_2nd; /* unknown SID */
+    logon_info->info3.base.groups.count = 10;
+    logon_info->info3.base.groups.rids = talloc_array(logon_info,
+                                           struct samr_RidWithAttribute,
+                                           logon_info->info3.base.groups.count);
+    fail_unless(logon_info->info3.base.groups.rids != NULL, "talloc_array failed.");
+
+    for (c = 0; c < logon_info->info3.base.groups.count; c++) {
+        logon_info->info3.base.groups.rids[c].rid = 500 + c;
+    }
+
+    ret = get_gids_from_pac(global_talloc_context, pac_ctx, pac_ctx->range_map,
+                            pac_ctx->my_dom_sid, logon_info, &gid_count, &gids);
+    fail_unless(ret == EINVAL, "Unexpected return code [%d] with unknown SID.",
+                               ret);
+
+    /* change SID to a known one */
+    logon_info->info3.base.domain_sid = &test_remote_dom_sid;
+
+    ret = get_gids_from_pac(global_talloc_context, pac_ctx, pac_ctx->range_map,
+                            pac_ctx->my_dom_sid, logon_info, &gid_count, &gids);
+    fail_unless(ret == EOK, "Failed with 10 RIDs in PAC");
+    fail_unless(gid_count == logon_info->info3.base.groups.count,
+                "[%d] groups expected, got [%d]",
+                logon_info->info3.base.groups.count, gid_count);
+    fail_unless(gids != NULL, "Expected gid array.");
+
+    for (c = 0; c < logon_info->info3.base.groups.count; c++) {
+        found = false;
+        exp_gid = IDMAP_RANGE_MIN + 500 + c;
+        for (g = 0; g < gid_count; g++) {
+            if (gids[g].gid == exp_gid) {
+                found = true;
+                break;
+            }
+        }
+        fail_unless(found, "[%d] not found in group list", exp_gid);
+    }
+
+    talloc_free(gids);
+    gids = NULL;
+
+    /* duplicated RIDs */
+    for (c = 0; c < logon_info->info3.base.groups.count; c++) {
+        logon_info->info3.base.groups.rids[c].rid = 500;
+    }
+
+    ret = get_gids_from_pac(global_talloc_context, pac_ctx, pac_ctx->range_map,
+                            pac_ctx->my_dom_sid, logon_info, &gid_count, &gids);
+    fail_unless(ret == EOK, "Failed with 10 duplicated RIDs in PAC");
+    fail_unless(gid_count == 1, "[%d] groups expected, got [%d]", 1, gid_count);
+    fail_unless(gids != NULL, "Expected gid array.");
+    fail_unless(gids[0].gid == IDMAP_RANGE_MIN + 500,
+                "Wrong gid returned, got [%d], expected [%d].", gids[0].gid,
+                                                         IDMAP_RANGE_MIN + 500);
+    talloc_free(gids);
+    gids = NULL;
+
+    logon_info->info3.sidcount = 2;
+    logon_info->info3.sids = talloc_zero_array(logon_info, struct netr_SidAttr,
+                                               logon_info->info3.sidcount);
+    fail_unless(logon_info->info3.sids != NULL, "talloc_zero_array failed.");
+
+    logon_info->info3.sids[0].sid = &test_smb_sid;
+    logon_info->info3.sids[1].sid = &test_smb_sid_2nd;
+
+    ret = get_gids_from_pac(global_talloc_context, pac_ctx, pac_ctx->range_map,
+                            pac_ctx->my_dom_sid, logon_info, &gid_count, &gids);
+    fail_unless(ret == EOK, "Failed with 10 duplicated RIDs and local SIDS in PAC");
+    fail_unless(gid_count == 3, "[%d] groups expected, got [%d]", 3, gid_count);
+    fail_unless(gids != NULL, "Expected gid array.");
+
+    gid_t exp_gids[] = {IDMAP_RANGE_MIN + 500, test_id, test_id_2nd, 0};
+
+    for (c = 0; exp_gids[c] != 0; c++) {
+        found = false;
+        for (g = 0; g < gid_count; g++) {
+            if (gids[g].gid == exp_gids[c]) {
+                found = true;
+                break;
+            }
+        }
+        fail_unless(found, "[%d] not found in group list", exp_gids[c]);
+    }
+
+    talloc_free(gids);
+    gids = NULL;
+
+    talloc_free(logon_info->info3.base.groups.rids);
+
+    for (t = 0; t < 7; t++) {
+        logon_info->info3.base.groups.count = powl(10, t);
+        logon_info->info3.base.groups.rids = talloc_array(logon_info,
+                                               struct samr_RidWithAttribute,
+                                               logon_info->info3.base.groups.count);
+        fail_unless(logon_info->info3.base.groups.rids != NULL, "talloc_array failed.");
+
+        for (c = 0; c < logon_info->info3.base.groups.count; c++) {
+            logon_info->info3.base.groups.rids[c].rid = 500 + c;
+        }
+
+        ret = gettimeofday(&start_time, NULL);
+        fail_unless(ret == 0, "gettimeofday failed.");
+
+        ret = get_gids_from_pac(global_talloc_context, pac_ctx, pac_ctx->range_map,
+                                pac_ctx->my_dom_sid, logon_info, &gid_count, &gids);
+        fail_unless(ret == EOK, "Unexpected return code [%d].", ret);
+
+        ret = gettimeofday(&end_time, NULL);
+        fail_unless(ret == 0, "gettimeofday failed.");
+
+        timersub(&end_time, &start_time, &diff_time);
+        fprintf(stderr, "Testcase [%zu], number of groups [%u], " \
+                        "duration [%ds %dus]\n", t,
+                        logon_info->info3.base.groups.count,
+                        (int) diff_time.tv_sec,
+                        (int) diff_time.tv_usec);
+
+        talloc_free(gids);
+        gids = NULL;
+
+        talloc_free(logon_info->info3.base.groups.rids);
+    }
+
+    talloc_free(logon_info);
+}
+END_TEST
+
 Suite *idmap_test_suite (void)
 {
     Suite *s = suite_create ("PAC responder");
@@ -235,10 +486,15 @@ Suite *idmap_test_suite (void)
                               leak_check_setup,
                               leak_check_teardown);
 
+    tcase_add_checked_fixture(tc_pac,
+                              pac_setup,
+                              pac_teardown);
+
     tcase_add_test(tc_pac, pac_test_local_sid_to_id);
     tcase_add_test(tc_pac, pac_test_seondary_local_sid_to_id);
     tcase_add_test(tc_pac, pac_test_get_gids_to_add_and_remove);
     tcase_add_test(tc_pac, pac_test_find_domain_by_id);
+    tcase_add_test(tc_pac, pac_test_get_gids_from_pac);
 
     suite_add_tcase(s, tc_pac);
 
-- 
1.7.7.6

-------------- next part --------------
From f2f257088f6f3bf7de12694c783e569a53b38f98 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 18 Dec 2012 14:09:05 +0100
Subject: [PATCH 12/13] PAC responder: check if existing user differs

If some of the Posix attributes of an user existing in the cache differ
from the data given in the current PAC the old user entry is drop and a
new one is created with the data from the PAC.
---
 src/responder/pac/pacsrv.h       |    2 +
 src/responder/pac/pacsrv_cmd.c   |   33 ++++++++++++++++++-----------
 src/responder/pac/pacsrv_utils.c |   42 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 13 deletions(-)

diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
index 71fcf8e..0bf2a07 100644
--- a/src/responder/pac/pacsrv.h
+++ b/src/responder/pac/pacsrv.h
@@ -129,4 +129,6 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
 
 struct sss_domain_info *find_domain_by_id(struct sss_domain_info *domains,
                                           const char *id_str);
+
+bool new_and_cached_user_differs(struct passwd *pwd, struct ldb_message *msg);
 #endif /* __PACSRV_H__ */
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 49164ab..375285f 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -382,26 +382,33 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx)
         goto done;
     }
 
-    ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->fq_name, attrs,
-                                    &msg);
+    ret = sysdb_search_user_by_uid(tmp_ctx, sysdb, pwd->pw_uid, attrs, &msg);
     if (ret == EOK) {
-        /* TODO: check id uid and gid are equal. */
-    } else if (ret == ENOENT) {
-        ret = sysdb_store_user(sysdb, pwd->pw_name, NULL,
-                               pwd->pw_uid, pwd->pw_gid, pwd->pw_gecos,
-                               pwd->pw_dir,
-                               pwd->pw_shell, NULL, user_attrs, NULL,
-                               pr_ctx->dom->user_timeout, 0);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_store_user failed [%d][%s].\n",
-                                      ret, strerror(ret)));
+        if (new_and_cached_user_differs(pwd, msg)) {
+            ret = sysdb_delete_user(sysdb, NULL, pwd->pw_uid);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_delete_user failed.\n"));
+                goto done;
+            }
+        } else {
             goto done;
         }
-    } else {
+    } else if (ret != EOK && ret != ENOENT) {
         DEBUG(SSSDBG_OP_FAILURE, ("sysdb_search_user_by_name failed.\n"));
         goto done;
     }
 
+    ret = sysdb_store_user(sysdb, pwd->pw_name, NULL,
+                           pwd->pw_uid, pwd->pw_gid, pwd->pw_gecos,
+                           pwd->pw_dir,
+                           pwd->pw_shell, NULL, user_attrs, NULL,
+                           pr_ctx->dom->user_timeout, 0);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_store_user failed [%d][%s].\n",
+                                  ret, strerror(ret)));
+        goto done;
+    }
+
     ret = EOK;
 
 done:
diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index 4c3ecb2..8328d6f 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -963,3 +963,45 @@ done:
 
     return ret;
 }
+
+static bool compare_string_with_attr(const char *val, struct ldb_message *msg,
+                                     const char *attr)
+{
+    const char *str;
+
+    str = ldb_msg_find_attr_as_string(msg, attr, NULL);
+    if ((str == NULL && val == NULL) ||
+        (str != NULL && val != NULL && strcmp(str, val) == 0)) {
+        return true;
+    }
+
+    return false;
+}
+
+bool new_and_cached_user_differs(struct passwd *pwd, struct ldb_message *msg)
+{
+    if (pwd == NULL || msg == NULL) {
+        return true;
+    }
+
+    if (!compare_string_with_attr(pwd->pw_name, msg, SYSDB_NAME)) {
+        DEBUG(SSSDBG_TRACE_FUNC, ("Names differ."));
+        return true;
+    }
+    if (!compare_string_with_attr(pwd->pw_gecos, msg, SYSDB_GECOS)) {
+        DEBUG(SSSDBG_TRACE_FUNC, ("Gecos fields differ."));
+        return true;
+    }
+
+    if (!compare_string_with_attr(pwd->pw_dir, msg, SYSDB_HOMEDIR)) {
+        DEBUG(SSSDBG_TRACE_FUNC, ("Home directories differ."));
+        return true;
+    }
+
+    if (!compare_string_with_attr(pwd->pw_shell, msg, SYSDB_SHELL)) {
+        DEBUG(SSSDBG_TRACE_FUNC, ("Shells differ."));
+        return true;
+    }
+
+    return false;
+}
-- 
1.7.7.6

-------------- next part --------------
From 02efb618423e67f107ce0aa9bca4b12c650d3d5d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 20 Dec 2012 21:10:49 +0100
Subject: [PATCH 13/13] Refactor gid handling in the PAC responder

Instead of using a single array of gid-domain_pointer pairs, Simo
suggested to use a gid array for each domain an store it with a pointer
to the domain.
---
 src/responder/pac/pacsrv.h       |   11 +-
 src/responder/pac/pacsrv_cmd.c   |   52 ++++++---
 src/responder/pac/pacsrv_utils.c |  223 +++++++++++++++++++++++++++-----------
 src/tests/pac_responder-tests.c  |   57 ++++++----
 4 files changed, 238 insertions(+), 105 deletions(-)

diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
index 0bf2a07..c087c01 100644
--- a/src/responder/pac/pacsrv.h
+++ b/src/responder/pac/pacsrv.h
@@ -71,9 +71,10 @@ struct grp_info {
     struct ldb_dn *dn;
 };
 
-struct pac_grp {
-    gid_t gid;
+struct pac_dom_grps {
     struct sss_domain_info *grp_dom;
+    size_t gid_count;
+    gid_t *gids;
 };
 
 int pac_cmd_execute(struct cli_ctx *cctx);
@@ -104,7 +105,7 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
                           struct local_mapping_ranges *range_map,
                           struct dom_sid *domain_sid,
                           struct PAC_LOGON_INFO *logon_info,
-                          size_t *_gid_count, struct pac_grp **_gids);
+                          size_t *_gid_count, struct pac_dom_grps **_gids);
 
 errno_t get_data_from_pac(TALLOC_CTX *mem_ctx,
                           uint8_t *pac_blob, size_t pac_len,
@@ -121,9 +122,9 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
                        size_t cur_grp_num,
                        struct grp_info *cur_gid_list,
                        size_t new_gid_num,
-                       struct pac_grp *new_gid_list,
+                       struct pac_dom_grps *new_gid_list,
                        size_t *_add_gid_num,
-                       struct pac_grp **_add_gid_list,
+                       struct pac_dom_grps **_add_gid_list,
                        size_t *_del_gid_num,
                        struct grp_info ***_del_gid_list);
 
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 375285f..bc89744 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -60,13 +60,13 @@ struct pac_req_ctx {
     struct dom_sid2 *domain_sid;
 
     size_t gid_count;
-    struct pac_grp *gids;
+    struct pac_dom_grps *gids;
 
     size_t current_grp_count;
     struct grp_info *current_grp_list;
 
     size_t add_gid_count;
-    struct pac_grp *add_gids;
+    struct pac_dom_grps *add_gids;
 
     size_t del_grp_count;
     struct grp_info **del_grp_list;
@@ -83,7 +83,8 @@ static void pac_get_group_done(struct tevent_req *subreq);
 static errno_t pac_save_memberships_next(struct tevent_req *req);
 static errno_t pac_store_membership(struct pac_req_ctx *pr_ctx,
                                     struct ldb_dn *user_dn,
-                                    int gid_iter);
+                                    gid_t gid,
+                                    struct sss_domain_info *grp_dom);
 struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx);
 static void pac_save_memberships_done(struct tevent_req *req);
 
@@ -417,7 +418,8 @@ done:
 }
 
 struct pac_save_memberships_state {
-    int gid_iter;
+    size_t gid_iter;
+    size_t dom_iter;
     struct ldb_dn *user_dn;
 
     struct pac_req_ctx *pr_ctx;
@@ -440,6 +442,7 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx)
     }
 
     state->gid_iter = 0;
+    state->dom_iter = 0;
     state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->fq_name);
     if (state->user_dn == NULL) {
         ret = ENOMEM;
@@ -585,17 +588,28 @@ static errno_t pac_save_memberships_next(struct tevent_req *req)
         return EINVAL;
     }
 
-    while (state->gid_iter < pr_ctx->add_gid_count) {
+    while (pr_ctx->add_gids[state->dom_iter].grp_dom != NULL) {
 
-        ret = pac_store_membership(state->pr_ctx, state->user_dn,
-                                   state->gid_iter);
+        if (pr_ctx->add_gids[state->dom_iter].gids == NULL ||
+            pr_ctx->add_gids[state->dom_iter].gid_count == 0) {
+            state->dom_iter++;
+            state->gid_iter = 0;
+            continue;
+        }
+
+
+        gid = pr_ctx->add_gids[state->dom_iter].gids[state->gid_iter];
+        grp_dom = pr_ctx->add_gids[state->dom_iter].grp_dom;
+
+        ret = pac_store_membership(state->pr_ctx, state->user_dn, gid, grp_dom);
         if (ret == EOK) {
             state->gid_iter++;
+            if (state->gid_iter >= pr_ctx->add_gids[state->dom_iter].gid_count) {
+                state->dom_iter++;
+                state->gid_iter = 0;
+            }
             continue;
         } else if (ret == ENOENT) {
-            gid = pr_ctx->add_gids[state->gid_iter].gid;
-            grp_dom = pr_ctx->add_gids[state->gid_iter].grp_dom;
-
             subreq = sss_dp_get_account_send(state, pr_ctx->cctx->rctx,
                                              grp_dom, true,
                                              SSS_DP_GROUP, NULL,
@@ -629,6 +643,9 @@ static void pac_get_group_done(struct tevent_req *subreq)
     dbus_uint16_t err_maj;
     dbus_uint32_t err_min;
     char *err_msg;
+    gid_t gid;
+    struct sss_domain_info *grp_dom;
+    struct pac_req_ctx *pr_ctx = state->pr_ctx;
 
     ret = sss_dp_get_account_recv(req, subreq,
                                   &err_maj, &err_min,
@@ -639,11 +656,17 @@ static void pac_get_group_done(struct tevent_req *subreq)
         goto error;
     }
 
-    ret = pac_store_membership(state->pr_ctx, state->user_dn, state->gid_iter);
+    gid = pr_ctx->add_gids[state->dom_iter].gids[state->gid_iter];
+    grp_dom = pr_ctx->add_gids[state->dom_iter].grp_dom;
+    ret = pac_store_membership(state->pr_ctx, state->user_dn, gid, grp_dom);
     if (ret != EOK) {
         goto error;
     }
     state->gid_iter++;
+    if (state->gid_iter >= pr_ctx->add_gids[state->dom_iter].gid_count) {
+        state->dom_iter++;
+        state->gid_iter = 0;
+    }
 
     ret = pac_save_memberships_next(req);
     if (ret == EOK) {
@@ -661,13 +684,11 @@ error:
 static errno_t
 pac_store_membership(struct pac_req_ctx *pr_ctx,
                      struct ldb_dn *user_dn,
-                     int gid_iter)
+                     gid_t gid, struct sss_domain_info *grp_dom)
 {
     TALLOC_CTX *tmp_ctx;
     struct sysdb_attrs *user_attrs;
     struct ldb_message *group;
-    uint32_t gid;
-    struct sss_domain_info *grp_dom;
     errno_t ret;
     const char *orig_group_dn;
     const char *group_attrs[] = { SYSDB_ORIG_DN, NULL };
@@ -677,9 +698,6 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
         return ENOMEM;
     }
 
-    gid = pr_ctx->add_gids[gid_iter].gid;
-    grp_dom = pr_ctx->add_gids[gid_iter].grp_dom;
-
     ret = sysdb_search_group_by_gid(tmp_ctx, grp_dom->sysdb,
                                     gid, group_attrs, &group);
     if (ret != EOK) {
diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index 8328d6f..a55ba0f 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -420,6 +420,73 @@ bool dom_sid_in_domain(const struct dom_sid *domain_sid,
     return true;
 }
 
+
+static errno_t get_dom_grps_from_hash(TALLOC_CTX *mem_ctx,
+                                      hash_table_t *gid_table,
+                                      struct sss_domain_info *grp_dom,
+                                      struct pac_dom_grps *dom_grps)
+{
+    int ret;
+    size_t gid_count;
+    size_t g;
+    struct hash_iter_context_t *iter;
+    hash_entry_t *entry;
+    gid_t *gids = NULL;
+
+    if (grp_dom == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Missing domain for groups.\n"));
+        return EINVAL;
+    }
+
+    gid_count = hash_count(gid_table);
+    if (gid_count == 0) {
+        DEBUG(SSSDBG_TRACE_FUNC, ("No groups found.\n"));
+        ret = EOK;
+        goto done;
+    }
+
+    gids = talloc_zero_array(mem_ctx, gid_t, gid_count);
+    if (gids == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
+        ret = ENOMEM;
+        goto done;
+    }
+
+
+    iter = new_hash_iter_context(gid_table);
+    if (iter == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("new_hash_iter_context failed.\n"));
+        ret = EIO;
+        goto done;
+    }
+
+    g = 0;
+    while ((entry = iter->next(iter)) != NULL) {
+        gids[g] = entry->key.ul;
+        g++;
+    }
+
+    if (gid_count != g) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Number of hash entries and groups do not "
+                                  "match.\n"));
+        ret = EINVAL;
+        goto done;
+    }
+
+    ret = EOK;
+
+done:
+    if (ret != EOK) {
+        talloc_free(gids);
+    } else {
+        dom_grps->grp_dom = grp_dom;
+        dom_grps->gid_count = gid_count;
+        dom_grps->gids = gids;
+    }
+
+    return ret;
+}
+
 /**
  * Find all Posix GIDs from a PAC by searching for group SIDs from the local
  * domain and convert them to GIDs.
@@ -429,14 +496,13 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
                           struct local_mapping_ranges *range_map,
                           struct dom_sid *domain_sid,
                           struct PAC_LOGON_INFO *logon_info,
-                          size_t *_gid_count, struct pac_grp **_gids)
+                          size_t *_gid_count, struct pac_dom_grps **_gids)
 {
     int ret;
-    size_t g;
     size_t gid_count = 0;
     size_t s;
     struct netr_SamInfo3 *info3;
-    struct pac_grp *gids = NULL;
+    struct pac_dom_grps *gids = NULL;
     struct sss_domain_info *grp_dom;
     char *sid_str = NULL;
     enum idmap_error_code err;
@@ -445,8 +511,6 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
     hash_table_t *gid_table;
     hash_key_t key;
     hash_value_t value;
-    struct hash_iter_context_t *iter;
-    hash_entry_t *entry;
     TALLOC_CTX *tmp_ctx = NULL;
 
     if (pac_ctx == NULL || range_map == NULL || domain_sid == NULL ||
@@ -470,8 +534,17 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sss_hash_create(tmp_ctx, info3->sidcount + info3->base.groups.count,
-                          &gid_table);
+    /* Currently three group containers are allocated, one for the IPA domain, one
+     * for the trusted AD domain and an empty one to indicate the end of the
+     * list. */
+    gids = talloc_zero_array(tmp_ctx, struct pac_dom_grps, 3);
+    if (gids == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_zero_array failed.\n"));
+        ret = ENOMEM;
+        goto done;
+    }
+
+    ret = sss_hash_create(tmp_ctx, info3->sidcount, &gid_table);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("sss_hash_create failed.\n"));
         goto done;
@@ -519,6 +592,20 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
         }
     }
 
+    ret = get_dom_grps_from_hash(gids, gid_table, grp_dom, &gids[0]);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("get_dom_grps_from_hash failed.\n"));
+        goto done;
+    }
+    gid_count += gids[0].gid_count;
+
+    talloc_free(gid_table);
+    ret = sss_hash_create(tmp_ctx, info3->base.groups.count, &gid_table);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sss_hash_create failed.\n"));
+        goto done;
+    }
+
     talloc_zfree(sid_str);
     err = sss_idmap_smb_sid_to_sid(pac_ctx->idmap_ctx, info3->base.domain_sid,
                                    &sid_str);
@@ -571,40 +658,12 @@ errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
         DEBUG(SSSDBG_TRACE_ALL, ("Found extra group with gid [%d].\n", id));
     }
 
-    gid_count = hash_count(gid_table);
-    if (gid_count == 0) {
-        DEBUG(SSSDBG_TRACE_FUNC, ("No groups found.\n"));
-        ret = EOK;
-        goto done;
-    }
-
-    gids = talloc_zero_array(tmp_ctx, struct pac_grp, gid_count);
-    if (gids == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
-        ret = ENOMEM;
-        goto done;
-    }
-
-    iter = new_hash_iter_context(gid_table);
-    if (iter == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, ("new_hash_iter_context failed.\n"));
-        ret = EIO;
-        goto done;
-    }
-
-    g = 0;
-    while ((entry = iter->next(iter)) != NULL) {
-        gids[g].gid = entry->key.ul;
-        gids[g].grp_dom = entry->value.ptr;
-        g++;
-    }
-
-    if (gid_count != g) {
-        DEBUG(SSSDBG_OP_FAILURE, ("Number of hash entries and groups do not "
-                                  "match.\n"));
-        ret = EINVAL;
+    ret = get_dom_grps_from_hash(gids, gid_table, grp_dom, &gids[1]);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("get_dom_grps_from_hash failed.\n"));
         goto done;
     }
+    gid_count += gids[1].gid_count;
 
     ret = EOK;
 
@@ -811,24 +870,26 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
                        size_t cur_grp_num,
                        struct grp_info *cur_grp_list,
                        size_t new_gid_num,
-                       struct pac_grp *new_gid_list,
+                       struct pac_dom_grps *new_gid_list,
                        size_t *_add_gid_num,
-                       struct pac_grp **_add_gid_list,
+                       struct pac_dom_grps **_add_gid_list,
                        size_t *_del_grp_num,
                        struct grp_info ***_del_grp_list)
 {
     int ret;
     size_t c;
+    size_t g;
     hash_table_t *table;
     hash_key_t key;
     hash_value_t value;
     size_t add_gid_num = 0;
-    struct pac_grp *add_gid_list = NULL;
+    struct pac_dom_grps *add_gid_list = NULL;
     size_t del_grp_num = 0;
     struct grp_info **del_grp_list = NULL;
     TALLOC_CTX *tmp_ctx = NULL;
     unsigned long value_count;
     hash_value_t *values;
+    size_t new_dom_num = 0;
 
     if ((cur_grp_num != 0 && cur_grp_list == NULL) ||
         (new_gid_num != 0 && new_gid_list == NULL)) {
@@ -848,17 +909,37 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
+    if (new_gid_num != 0) {
+        for (new_dom_num = 0; new_gid_list[new_dom_num].grp_dom != NULL;
+             new_dom_num++);
+    }
+
     if (cur_grp_num == 0 && new_gid_num != 0) {
         add_gid_num = new_gid_num;
-        add_gid_list = talloc_array(tmp_ctx, struct pac_grp, add_gid_num);
+        add_gid_list = talloc_zero_array(tmp_ctx, struct pac_dom_grps,
+                                         new_dom_num + 1);
         if (add_gid_list == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
             ret = ENOMEM;
             goto done;
         }
 
-        for (c = 0; c < add_gid_num; c++) {
-            add_gid_list[c] = new_gid_list[c];
+        for (c = 0; c < new_dom_num; c++) {
+            add_gid_list[c].grp_dom = new_gid_list[c].grp_dom;
+            add_gid_list[c].gid_count = new_gid_list[c].gid_count;
+            if (new_gid_list[c].gid_count != 0) {
+                add_gid_list[c].gids = talloc_zero_array(add_gid_list, gid_t,
+                                                     new_gid_list[c].gid_count);
+                if (add_gid_list[c].gids == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE, ("talloc_zero_array failed.\n"));
+                    ret = ENOMEM;
+                    goto done;
+                }
+
+                for (g = 0; g < new_gid_list[c].gid_count; g++) {
+                    add_gid_list[c].gids[g] = new_gid_list[c].gids[g];
+                }
+            }
         }
 
         ret = EOK;
@@ -904,27 +985,45 @@ errno_t diff_gid_lists(TALLOC_CTX *mem_ctx,
         }
     }
 
-    for (c = 0; c < new_gid_num; c++) {
-        key.ul = (unsigned long) new_gid_list[c].gid;
+    add_gid_list = talloc_zero_array(tmp_ctx, struct pac_dom_grps,
+                                     new_dom_num + 1);
+    if (add_gid_list == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
+        ret = ENOMEM;
+        goto done;
+    }
 
-        ret = hash_delete(table, &key);
-        if (ret == HASH_ERROR_KEY_NOT_FOUND) {
-            /* gid not found, must be added */
-            add_gid_num++;
-            add_gid_list = talloc_realloc(tmp_ctx, add_gid_list, struct pac_grp,
-                                          add_gid_num);
-            if (add_gid_list == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE, ("talloc_realloc failed.\n"));
-                ret = ENOMEM;
+    for (c = 0; c < new_dom_num; c++) {
+        add_gid_list[c].grp_dom = new_gid_list[c].grp_dom;
+        add_gid_list[c].gid_count = 0;
+
+        for (g = 0; g < new_gid_list[c].gid_count; g++) {
+            key.ul = (unsigned long) new_gid_list[c].gids[g];
+
+            ret = hash_delete(table, &key);
+            if (ret == HASH_ERROR_KEY_NOT_FOUND) {
+                /* gid not found, must be added */
+                add_gid_list[c].gid_count++;
+                add_gid_list[c].gids = talloc_realloc(add_gid_list,
+                                                     add_gid_list,
+                                                     gid_t,
+                                                     add_gid_list[c].gid_count);
+                if (add_gid_list[c].gids == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE, ("talloc_realloc failed.\n"));
+                    ret = ENOMEM;
+                    goto done;
+                }
+
+                add_gid_list[c].gids[add_gid_list[c].gid_count - 1] =
+                                                        new_gid_list[c].gids[g];
+            } else if (ret != HASH_SUCCESS) {
+                DEBUG(SSSDBG_OP_FAILURE, ("hash_delete failed.\n"));
+                ret = EIO;
                 goto done;
             }
-
-            add_gid_list[add_gid_num - 1] = new_gid_list[c];
-        } else if (ret != HASH_SUCCESS) {
-            DEBUG(SSSDBG_OP_FAILURE, ("hash_delete failed.\n"));
-            ret = EIO;
-            goto done;
         }
+
+        add_gid_num += add_gid_list[c].gid_count;
     }
 
     /* the remaining entries in the hash are not in the new list anymore and
diff --git a/src/tests/pac_responder-tests.c b/src/tests/pac_responder-tests.c
index b2a1457..7d352a0 100644
--- a/src/tests/pac_responder-tests.c
+++ b/src/tests/pac_responder-tests.c
@@ -172,16 +172,25 @@ START_TEST(pac_test_get_gids_to_add_and_remove)
     int ret;
     size_t c;
     size_t add_gid_count = 0;
-    struct pac_grp *add_gids = NULL;
+    struct pac_dom_grps *add_gids = NULL;
     size_t del_gid_count = 0;
     struct grp_info **del_gids = NULL;
+    struct sss_domain_info grp_dom;
 
-    struct pac_grp pac_grp_2 = {2, NULL};
-    struct pac_grp pac_grp_3 = {3, NULL};
+    memset(&grp_dom, 0, sizeof(grp_dom));
 
-    struct pac_grp gid_list_2[] = {pac_grp_2};
-    struct pac_grp gid_list_3[] = {pac_grp_3};
-    struct pac_grp gid_list_23[] = {pac_grp_2, pac_grp_3};
+    gid_t gid_list_2[] = {2};
+    gid_t gid_list_3[] = {3};
+    gid_t gid_list_23[] = {2, 3};
+    struct pac_dom_grps empty_dom = {NULL, 0, NULL};
+
+    struct pac_dom_grps pac_grp_2 = {&grp_dom, 1, gid_list_2};
+    struct pac_dom_grps pac_grp_3 = {&grp_dom, 1, gid_list_3};
+    struct pac_dom_grps pac_grp_23 = {&grp_dom, 2, gid_list_23};
+
+    struct pac_dom_grps dom_grp_list_2[] = {pac_grp_2, empty_dom};
+    struct pac_dom_grps dom_grp_list_3[] = {pac_grp_3, empty_dom};
+    struct pac_dom_grps dom_grp_list_23[] = {pac_grp_23, empty_dom};
 
     struct grp_info grp_info_1 = {1, NULL, NULL};
     struct grp_info grp_info_2 = {2, NULL, NULL};
@@ -192,18 +201,18 @@ START_TEST(pac_test_get_gids_to_add_and_remove)
         size_t cur_gid_count;
         struct grp_info *cur_gids;
         size_t gid_count;
-        struct pac_grp *gids;
+        struct pac_dom_grps *gids;
         int exp_ret;
         size_t exp_add_gid_count;
-        struct pac_grp *exp_add_gids;
+        struct pac_dom_grps *exp_add_gids;
         size_t exp_del_gid_count;
         struct grp_info *exp_del_gids;
     } a_and_r_data[] = {
-            {1, grp_list_1, 1, gid_list_2, EOK, 1, gid_list_2, 1, grp_list_1},
+            {1, grp_list_1, 1, dom_grp_list_2, EOK, 1, dom_grp_list_2, 1, grp_list_1},
             {1, grp_list_1, 0, NULL, EOK, 0, NULL, 1, grp_list_1},
-            {0, NULL, 1, gid_list_2, EOK, 1, gid_list_2, 0, NULL},
-            {2, grp_list_12, 1, gid_list_2, EOK,  0, NULL, 1, grp_list_1},
-            {2, grp_list_12, 2, gid_list_23, EOK, 1, gid_list_3, 1, grp_list_1},
+            {0, NULL, 1, dom_grp_list_2, EOK, 1, dom_grp_list_2, 0, NULL},
+            {2, grp_list_12, 1, dom_grp_list_2, EOK,  0, NULL, 1, grp_list_1},
+            {2, grp_list_12, 2, dom_grp_list_23, EOK, 1, dom_grp_list_3, 1, grp_list_1},
             {0, NULL, 0, NULL, 0, 0, NULL, 0, NULL}
     };
 
@@ -254,10 +263,10 @@ START_TEST(pac_test_get_gids_to_add_and_remove)
          * only look at lists with 1 element. TODO: add code to compare lists
          * with more than 1 member. */
         if (add_gid_count == 1) {
-            fail_unless(add_gids[0].gid ==  a_and_r_data[c].exp_add_gids[0].gid,
+            fail_unless(add_gids[0].gids[0] ==  a_and_r_data[c].exp_add_gids[0].gids[0],
                         "Unexpected gid to add for test data #%d, " \
                         "expected [%d], got [%d]",
-                        c, a_and_r_data[c].exp_add_gids[0].gid, add_gids[0].gid);
+                        c, a_and_r_data[c].exp_add_gids[0].gids[0], add_gids[0].gids[0]);
         }
 
         if (del_gid_count == 1) {
@@ -326,10 +335,11 @@ START_TEST(pac_test_get_gids_from_pac)
 {
     int ret;
     size_t c;
+    size_t d;
     size_t g;
     size_t t;
     size_t gid_count;
-    struct pac_grp *gids;
+    struct pac_dom_grps *gids;
     struct PAC_LOGON_INFO *logon_info;
     bool found;
     gid_t exp_gid;
@@ -380,7 +390,7 @@ START_TEST(pac_test_get_gids_from_pac)
         found = false;
         exp_gid = IDMAP_RANGE_MIN + 500 + c;
         for (g = 0; g < gid_count; g++) {
-            if (gids[g].gid == exp_gid) {
+            if (gids[1].gids[g] == exp_gid) {
                 found = true;
                 break;
             }
@@ -401,8 +411,8 @@ START_TEST(pac_test_get_gids_from_pac)
     fail_unless(ret == EOK, "Failed with 10 duplicated RIDs in PAC");
     fail_unless(gid_count == 1, "[%d] groups expected, got [%d]", 1, gid_count);
     fail_unless(gids != NULL, "Expected gid array.");
-    fail_unless(gids[0].gid == IDMAP_RANGE_MIN + 500,
-                "Wrong gid returned, got [%d], expected [%d].", gids[0].gid,
+    fail_unless(gids[1].gids[0] == IDMAP_RANGE_MIN + 500,
+                "Wrong gid returned, got [%d], expected [%d].", gids[1].gids[0],
                                                          IDMAP_RANGE_MIN + 500);
     talloc_free(gids);
     gids = NULL;
@@ -425,9 +435,14 @@ START_TEST(pac_test_get_gids_from_pac)
 
     for (c = 0; exp_gids[c] != 0; c++) {
         found = false;
-        for (g = 0; g < gid_count; g++) {
-            if (gids[g].gid == exp_gids[c]) {
-                found = true;
+        for (d = 0; d < 2; d++) {
+            for (g = 0; g < gids[d].gid_count; g++) {
+                if (gids[d].gids[g] == exp_gids[c]) {
+                    found = true;
+                    break;
+                }
+            }
+            if (found) {
                 break;
             }
         }
-- 
1.7.7.6



More information about the sssd-devel mailing list