[SSSD] [PATCHES] Improve group handling for subdomain users

Sumit Bose sbose at redhat.com
Wed Nov 7 12:13:17 UTC 2012


Hi,

this series of patches fixes https://fedorahosted.org/sssd/ticket/1604 .
To be precise only 0005 is the fix for the ticket, but the others add
some necessary improvements, especially the removal of a group
membership.

bye,
Sumit
-------------- next part --------------
From b84d722f4924190265b89dc69b6e3bb46aafb0b3 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 7 Nov 2012 11:29:51 +0100
Subject: [PATCH 1/5] Do not remove a group if it has members from subdomains

Currently it is only checked if an expired group still has members of
the local domain. If not, the group is delete from the cache. With this
patch the whole cache, i.e. including subdomains, is searched for
members.
---
 src/providers/ldap/ldap_id_cleanup.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c
index e65356d..9c2faab 100644
--- a/src/providers/ldap/ldap_id_cleanup.c
+++ b/src/providers/ldap/ldap_id_cleanup.c
@@ -395,6 +395,7 @@ static int cleanup_groups(TALLOC_CTX *memctx,
     int ret;
     int i;
     const char *posix;
+    struct ldb_dn *base_dn;
 
     tmpctx = talloc_new(memctx);
     if (!tmpctx) {
@@ -436,10 +437,12 @@ static int cleanup_groups(TALLOC_CTX *memctx,
         posix = ldb_msg_find_attr_as_string(msgs[i], SYSDB_POSIX, NULL);
         if (!posix || strcmp(posix, "TRUE") == 0) {
             /* Search for users that are members of this group, or
-             * that have this group as their primary GID
+             * that have this group as their primary GID.
+             * Include subdomain users as well.
              */
             gid = (gid_t) ldb_msg_find_attr_as_uint(msgs[i], SYSDB_GIDNUM, 0);
-            subfilter = talloc_asprintf(tmpctx, "(|(%s=%s)(%s=%lu))",
+            subfilter = talloc_asprintf(tmpctx, "(&(%s=%s)(|(%s=%s)(%s=%lu)))",
+                                        SYSDB_OBJECTCLASS, SYSDB_USER_CLASS,
                                         SYSDB_MEMBEROF, dn,
                                         SYSDB_GIDNUM, (long unsigned) gid);
         } else {
@@ -451,8 +454,16 @@ static int cleanup_groups(TALLOC_CTX *memctx,
             goto done;
         }
 
-        ret = sysdb_search_users(tmpctx, sysdb,
-                                 subfilter, NULL, &u_count, &u_msgs);
+        base_dn = sysdb_base_dn(sysdb, tmpctx);
+        if (base_dn == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE, ("Failed to build base dn\n"));
+            ret = ENOMEM;
+            goto done;
+        }
+
+        ret = sysdb_search_entry(tmpctx, sysdb, base_dn,
+                                 LDB_SCOPE_SUBTREE, subfilter, NULL,
+                                 &u_count, &u_msgs);
         if (ret == ENOENT) {
             const char *name;
 
-- 
1.7.7.6

-------------- next part --------------
From 9d51b6601669fd9b65d54e31376e6ac12011c773 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 7 Nov 2012 11:34:09 +0100
Subject: [PATCH 2/5] Add get_gids_to_add_and_remove() with test

This patch adds a new call which compares a list of current GIDs with a
list of new GIDs and return a list of GIDs which are currently missing
and must be added and another list of GIDs which are not used anymore
and must be deleted. The method is the same as used by
diff_string_lists().
---
 src/responder/pac/pacsrv.h       |   10 +++
 src/responder/pac/pacsrv_utils.c |  155 ++++++++++++++++++++++++++++++++++++++
 src/tests/pac_responder-tests.c  |  104 +++++++++++++++++++++++++-
 3 files changed, 267 insertions(+), 2 deletions(-)

diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
index e088e21..08172f8 100644
--- a/src/responder/pac/pacsrv.h
+++ b/src/responder/pac/pacsrv.h
@@ -106,4 +106,14 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
                          struct PAC_LOGON_INFO *logon_info,
                          struct passwd **_pwd,
                          struct sysdb_attrs **_attrs);
+
+errno_t get_gids_to_add_and_remove(TALLOC_CTX *mem_ctx,
+                                   size_t current_gid_count,
+                                   gid_t *current_gids,
+                                   size_t gid_count,
+                                   gid_t *gids,
+                                   size_t *_add_gid_count,
+                                   gid_t **_add_gids,
+                                   size_t *_del_gid_count,
+                                   gid_t **_del_gids);
 #endif /* __PACSRV_H__ */
diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index 101960f..2ac29b5 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -616,3 +616,158 @@ done:
 
     return ret;
 }
+
+errno_t get_gids_to_add_and_remove(TALLOC_CTX *mem_ctx,
+                                   size_t current_gid_count,
+                                   gid_t *current_gids,
+                                   size_t gid_count,
+                                   gid_t *gids,
+                                   size_t *_add_gid_count,
+                                   gid_t **_add_gids,
+                                   size_t *_del_gid_count,
+                                   gid_t **_del_gids)
+{
+    int ret;
+    size_t c;
+    hash_table_t *table;
+    hash_key_t key;
+    hash_value_t value;
+    size_t add_gid_count = 0;
+    gid_t *add_gids = NULL;
+    size_t del_gid_count = 0;
+    gid_t *del_gids = NULL;
+    TALLOC_CTX *tmp_ctx = NULL;
+    unsigned long key_count;
+    hash_key_t *keys;
+
+    if ((current_gid_count != 0 && current_gids == NULL) ||
+        (gid_count != 0 && gids == NULL)) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Missing group array.\n"));
+        return EINVAL;
+    }
+
+    if (current_gid_count == 0 && gid_count == 0) {
+        ret = EOK;
+        goto done;
+    }
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_new failed.\n"));
+        ret = ENOMEM;
+        goto done;
+    }
+
+    if (current_gid_count == 0 && gid_count != 0) {
+        add_gid_count = gid_count;
+        add_gids = talloc_array(tmp_ctx, gid_t, add_gid_count);
+        if (add_gids == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
+            ret = ENOMEM;
+            goto done;
+        }
+
+        for (c = 0; c < add_gid_count; c++) {
+            add_gids[c] = gids[c];
+        }
+
+        ret = EOK;
+        goto done;
+    }
+
+    if (current_gid_count != 0 && gid_count == 0) {
+        del_gid_count = current_gid_count;
+        del_gids = talloc_array(tmp_ctx, gid_t, del_gid_count);
+        if (del_gids == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
+            ret = ENOMEM;
+            goto done;
+        }
+
+        for (c = 0; c < del_gid_count; c++) {
+            del_gids[c] = current_gids[c];
+        }
+
+        ret = EOK;
+        goto done;
+    }
+
+    /* Add all current GIDs to a hash and then compare with the new ones in a
+     * single loop */
+    ret = sss_hash_create(tmp_ctx, current_gid_count, &table);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sss_hash_create failed.\n"));
+        goto done;
+    }
+
+    key.type = HASH_KEY_ULONG;
+    value.type = HASH_VALUE_UNDEF;
+    for (c = 0; c < current_gid_count; c++) {
+        key.ul = (unsigned long) current_gids[c];
+
+        ret = hash_enter(table, &key, &value);
+        if (ret != HASH_SUCCESS) {
+            DEBUG(SSSDBG_OP_FAILURE, ("hash_enter failed.\n"));
+            ret = EIO;
+            goto done;
+        }
+    }
+
+    for (c = 0; c < gid_count; c++) {
+        key.ul = (unsigned long) gids[c];
+
+        ret = hash_delete(table, &key);
+        if (ret == HASH_ERROR_KEY_NOT_FOUND) {
+            /* gid not found, must be added */
+            add_gid_count++;
+            add_gids = talloc_realloc(tmp_ctx, add_gids, gid_t, add_gid_count);
+            if (add_gids == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, ("talloc_realloc failed.\n"));
+                ret = ENOMEM;
+                goto done;
+            }
+
+            add_gids[add_gid_count - 1] = gids[c];
+        } else if (ret != HASH_SUCCESS) {
+            DEBUG(SSSDBG_OP_FAILURE, ("hash_delete failed.\n"));
+            ret = EIO;
+            goto done;
+        }
+    }
+
+    /* the remaining entries in the hash are not in the new list anymore and
+     * must be deleted */
+    ret = hash_keys(table, &key_count, &keys);
+    if (ret != HASH_SUCCESS) {
+        DEBUG(SSSDBG_OP_FAILURE, ("hash_keys failed.\n"));
+        ret = EIO;
+        goto done;
+    }
+
+    del_gid_count = key_count;
+    del_gids = talloc_array(tmp_ctx, gid_t, del_gid_count);
+    if (del_gids == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
+        ret = ENOMEM;
+        goto done;
+    }
+
+    for (c = 0; c < del_gid_count; c++) {
+        del_gids[c] = (gid_t) keys[c].ul;
+    }
+
+    ret = EOK;
+
+done:
+
+    if (ret == EOK) {
+        *_add_gid_count = add_gid_count;
+        *_add_gids = talloc_steal(mem_ctx, add_gids);
+        *_del_gid_count = del_gid_count;
+        *_del_gids = talloc_steal(mem_ctx, del_gids);
+    }
+
+    talloc_free(tmp_ctx);
+
+    return ret;
+}
diff --git a/src/tests/pac_responder-tests.c b/src/tests/pac_responder-tests.c
index 720793c..d9fa059 100644
--- a/src/tests/pac_responder-tests.c
+++ b/src/tests/pac_responder-tests.c
@@ -70,18 +70,118 @@ START_TEST(pac_test_seondary_local_sid_to_id)
 }
 END_TEST
 
+START_TEST(pac_test_get_gids_to_add_and_remove)
+{
+    TALLOC_CTX *mem_ctx;
+    int ret;
+    size_t c;
+    size_t add_gid_count = 0;
+    gid_t *add_gids = NULL;
+    size_t del_gid_count = 0;
+    gid_t *del_gids = NULL;
+
+    gid_t gid_list_1[] = {1};
+    gid_t gid_list_2[] = {2};
+    gid_t gid_list_3[] = {3};
+    gid_t gid_list_12[] = {1, 2};
+    gid_t gid_list_23[] = {2, 3};
+    struct a_and_r_data {
+        size_t cur_gid_count;
+        gid_t *cur_gids;
+        size_t gid_count;
+        gid_t *gids;
+        int exp_ret;
+        size_t exp_add_gid_count;
+        gid_t *exp_add_gids;
+        size_t exp_del_gid_count;
+        gid_t *exp_del_gids;
+    } a_and_r_data[] = {
+            {1, gid_list_1, 1, gid_list_2, EOK, 1, gid_list_2, 1, gid_list_1},
+            {1, gid_list_1, 0, NULL, EOK, 0, NULL, 1, gid_list_1},
+            {0, NULL, 1, gid_list_2, EOK, 1, gid_list_2, 0, NULL},
+            {2, gid_list_12, 1, gid_list_2, EOK,  0, NULL, 1, gid_list_1},
+            {2, gid_list_12, 2, gid_list_23, EOK, 1, gid_list_3, 1, gid_list_1},
+            {0, NULL, 0, NULL, 0, 0, NULL, 0, NULL}
+    };
+
+    mem_ctx = talloc_new(NULL);
+    fail_unless(mem_ctx != NULL, "talloc_new failed.");
+
+    ret = get_gids_to_add_and_remove(mem_ctx, 0, NULL, 0, NULL,
+                                     &add_gid_count, &add_gids,
+                                     &del_gid_count, &del_gids);
+    fail_unless(ret == EOK, "get_gids_to_add_and_remove failed with empty " \
+                            "groups.");
+
+    ret = get_gids_to_add_and_remove(mem_ctx, 1, NULL, 0, NULL,
+                                     &add_gid_count, &add_gids,
+                                     &del_gid_count, &del_gids);
+    fail_unless(ret == EINVAL, "get_gids_to_add_and_remove failed with " \
+                               "invalid current groups.");
+
+    ret = get_gids_to_add_and_remove(mem_ctx, 0, NULL, 1, NULL,
+                                     &add_gid_count, &add_gids,
+                                     &del_gid_count, &del_gids);
+    fail_unless(ret == EINVAL, "get_gids_to_add_and_remove failed with " \
+                               "invalid new groups.");
+
+    for (c = 0; a_and_r_data[c].cur_gids != NULL ||
+                a_and_r_data[c].gids != NULL; c++) {
+        ret = get_gids_to_add_and_remove(mem_ctx,
+                                         a_and_r_data[c].cur_gid_count,
+                                         a_and_r_data[c].cur_gids,
+                                         a_and_r_data[c].gid_count,
+                                         a_and_r_data[c].gids,
+                                         &add_gid_count, &add_gids,
+                                         &del_gid_count, &del_gids);
+        fail_unless(ret == a_and_r_data[c].exp_ret,
+                    "Unexpected return value for test data #%d, " \
+                    "expected [%d], got [%d]",
+                    c, a_and_r_data[c].exp_ret, ret);
+        fail_unless(add_gid_count ==  a_and_r_data[c].exp_add_gid_count,
+                    "Unexpected numer of groups to add for test data #%d, " \
+                    "expected [%d], got [%d]",
+                    c, a_and_r_data[c].exp_add_gid_count, add_gid_count);
+        fail_unless(del_gid_count ==  a_and_r_data[c].exp_del_gid_count,
+                    "Unexpected numer of groups to delete for test data #%d, " \
+                    "expected [%d], got [%d]",
+                    c, a_and_r_data[c].exp_del_gid_count, del_gid_count);
+
+        /* The lists might be returned in any order, to make tests simple we
+         * 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],
+                        "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]);
+        }
+
+        if (del_gid_count == 1) {
+            fail_unless(del_gids[0] ==  a_and_r_data[c].exp_del_gids[0],
+                        "Unexpected gid to delete for test data #%d, " \
+                        "expected [%d], got [%d]",
+                        c, a_and_r_data[c].exp_del_gids[0], del_gids[0]);
+        }
+    }
+
+    talloc_free(mem_ctx);
+}
+END_TEST
+
 
 Suite *idmap_test_suite (void)
 {
     Suite *s = suite_create ("PAC responder");
 
     TCase *tc_pac = tcase_create("PAC responder tests");
-    /*tcase_add_checked_fixture(tc_init,
+    tcase_add_checked_fixture(tc_pac,
                               leak_check_setup,
-                              leak_check_teardown);*/
+                              leak_check_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);
 
     suite_add_tcase(s, tc_pac);
 
-- 
1.7.7.6

-------------- next part --------------
From 9bfb75134ede4d6fe335e2492416e974f239008d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 7 Nov 2012 11:53:13 +0100
Subject: [PATCH 3/5] Use sysdb_initgroups() instead of
 sysdb_get_user_by_name()

To be able to efficiently store group memberships we need to know the
current memberships of a user. Instead of just looking up the user in
the cache sysdb_initgroups() is used which returns the user entry
together with all groups the user is a member of.
---
 src/responder/pac/pacsrv_cmd.c |   44 +++++++++++++++++++++++++++++++++------
 1 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 7777983..051f697 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -60,11 +60,15 @@ struct pac_req_ctx {
 
     size_t gid_count;
     gid_t *gids;
+
+    size_t current_gid_count;
+    gid_t *current_gids;
 };
 
 static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx);
 static void pac_get_domains_done(struct tevent_req *req);
-static errno_t save_pac_user(struct pac_req_ctx *pr_ctx);
+static errno_t save_pac_user(TALLOC_CTX *mem_ctx, struct pac_req_ctx *pr_ctx,
+                             size_t *_current_gid_count, gid_t **_current_gids);
 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,
@@ -188,7 +192,8 @@ static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx)
     struct dom_sid *my_dom_sid;
     struct local_mapping_ranges *my_range_map;
 
-    ret = save_pac_user(pr_ctx);
+    ret = save_pac_user(pr_ctx, pr_ctx, &pr_ctx->current_gid_count,
+                        &pr_ctx->current_gids);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("save_pac_user failed.\n"));
         goto done;
@@ -223,15 +228,18 @@ done:
     return ret;
 }
 
-static errno_t save_pac_user(struct pac_req_ctx *pr_ctx)
+static errno_t save_pac_user(TALLOC_CTX *mem_ctx, struct pac_req_ctx *pr_ctx,
+                             size_t *_current_gid_count, gid_t **_current_gids)
 {
     struct sysdb_ctx *sysdb;
     int ret;
-    const char *attrs[] = {SYSDB_NAME, SYSDB_UIDNUM, SYSDB_GIDNUM, NULL};
-    struct ldb_message *msg;
     struct passwd *pwd = NULL;
     TALLOC_CTX *tmp_ctx = NULL;
     struct sysdb_attrs *user_attrs = NULL;
+    struct ldb_result *res = NULL;
+    gid_t *current_gids = NULL;
+    size_t current_gid_count = 0;
+    size_t c;
 
     sysdb = pr_ctx->dom->sysdb;
     if (sysdb == NULL) {
@@ -247,9 +255,29 @@ 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->user_name, attrs,
-                                    &msg);
+    ret = sysdb_initgroups(tmp_ctx, sysdb, pr_ctx->user_name, &res);
     if (ret == EOK) {
+        /* First result is the user entry then the groups follow */
+        if (res->count > 1) {
+            current_gid_count = res->count - 1;
+            current_gids = talloc_array(mem_ctx, gid_t, current_gid_count);
+            if (current_gids == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
+                ret = ENOMEM;
+                goto done;
+            }
+
+            for (c = 0; c < current_gid_count; c++) {
+                current_gids[c] = ldb_msg_find_attr_as_uint64(res->msgs[c + 1],
+                                                              SYSDB_GIDNUM, 0);
+                if (current_gids[c] == 0) {
+                    DEBUG(SSSDBG_OP_FAILURE, ("Missing GID.\n"));
+                    ret = EINVAL;
+                    goto done;
+                }
+            }
+        }
+
         /* 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,
@@ -274,6 +302,8 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx)
         goto done;
     }
 
+    *_current_gid_count = current_gid_count;
+    *_current_gids = current_gids;
     ret = EOK;
 
 done:
-- 
1.7.7.6

-------------- next part --------------
From 608034c250400b59d3d989cbb32d3fa9ab1eea67 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 7 Nov 2012 12:01:27 +0100
Subject: [PATCH 4/5] Get lists of GIDs to be added and deleted and use them

Currently the user was just added to all local groups which are given in
the PAC. With this patch the user is added only to groups he is
currently not a member of and deleted from groups which are not found in
the PAC anymore.
---
 src/responder/pac/pacsrv_cmd.c |   85 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 051f697..6c12bec 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -63,6 +63,12 @@ struct pac_req_ctx {
 
     size_t current_gid_count;
     gid_t *current_gids;
+
+    size_t add_gid_count;
+    gid_t *add_gids;
+
+    size_t del_gid_count;
+    gid_t *del_gids;
 };
 
 static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx);
@@ -214,6 +220,15 @@ static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx)
         goto done;
     }
 
+    ret = get_gids_to_add_and_remove(pr_ctx, pr_ctx->current_gid_count,
+                                     pr_ctx->current_gids,
+                                     pr_ctx->gid_count,
+                                     pr_ctx->gids,
+                                     &pr_ctx->add_gid_count,
+                                     &pr_ctx->add_gids,
+                                     &pr_ctx->del_gid_count,
+                                     &pr_ctx->del_gids);
+
     req = pac_save_memberships_send(pr_ctx);
     if (req == NULL) {
         ret = ENOMEM;
@@ -319,6 +334,9 @@ struct pac_save_memberships_state {
     struct sss_domain_info *group_dom;
 };
 
+static errno_t
+pac_save_memberships_delete(struct pac_save_memberships_state *state);
+
 struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx)
 {
     struct pac_save_memberships_state *state;
@@ -346,6 +364,12 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx)
         state->group_dom = pr_ctx->dom;
     }
 
+    ret = pac_save_memberships_delete(state);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("pac_save_memberships_delete failed.\n"));
+        goto done;
+    }
+
     ret = pac_save_memberships_next(req);
     if (ret == EOK) {
         tevent_req_done(req);
@@ -361,6 +385,52 @@ done:
     return req;
 }
 
+static errno_t
+pac_save_memberships_delete(struct pac_save_memberships_state *state)
+{
+    int ret;
+    size_t c;
+    struct pac_req_ctx *pr_ctx;
+    struct ldb_message *group;
+
+    pr_ctx = state->pr_ctx;
+
+    if (pr_ctx->del_gid_count == 0) {
+        return EOK;
+    }
+
+    if (pr_ctx->del_gids == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Missing group list.\n"));
+        return EINVAL;
+    }
+
+    for (c = 0; c < pr_ctx->del_gid_count; c++) {
+        ret = sysdb_search_group_by_gid(state, state->group_dom->sysdb,
+                                        pr_ctx->del_gids[c], NULL, &group);
+        if (ret == EOK) {
+            ret = sysdb_mod_group_member(state->group_dom->sysdb, state->user_dn,
+                                         group->dn, LDB_FLAG_MOD_DELETE);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_mod_group_member failed.\n"));
+                goto done;
+            }
+        } else if (ret == ENOENT) {
+            continue;
+        } else {
+            DEBUG(SSSDBG_TRACE_INTERNAL, ("sysdb_search_group_by_gid for " \
+                                          "gid [%d] failed [%d][%s].\n",
+                                          pr_ctx->del_gids[c], ret,
+                                          strerror(ret)));
+            goto done;
+        }
+    }
+
+    ret = EOK;
+done:
+
+    return ret;
+}
+
 static errno_t pac_save_memberships_next(struct tevent_req *req)
 {
     errno_t ret;
@@ -372,8 +442,17 @@ static errno_t pac_save_memberships_next(struct tevent_req *req)
     state = tevent_req_data(req, struct pac_save_memberships_state);
     pr_ctx = state->pr_ctx;
 
-    while (state->gid_iter < pr_ctx->gid_count) {
-        gid = pr_ctx->gids[state->gid_iter];
+    if (pr_ctx->add_gid_count == 0) {
+        return EOK;
+    }
+
+    if (pr_ctx->add_gids == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Missing group list.\n"));
+        return EINVAL;
+    }
+
+    while (state->gid_iter < pr_ctx->add_gid_count) {
+        gid = pr_ctx->add_gids[state->gid_iter];
 
         ret = pac_store_membership(state->pr_ctx, state->group_dom->sysdb,
                                     state->user_dn, state->gid_iter);
@@ -462,7 +541,7 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
         return ENOMEM;
     }
 
-    gid = pr_ctx->gids[gid_iter];
+    gid = pr_ctx->add_gids[gid_iter];
 
     ret = sysdb_search_group_by_gid(tmp_ctx, group_sysdb,
                                     gid, NULL, &group);
-- 
1.7.7.6

-------------- next part --------------
From 64502919dd9dd75b82e2a16808d2c3ed8250a72d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 7 Nov 2012 12:09:55 +0100
Subject: [PATCH 5/5] Store the original group DN in the subdomain user object

For user of the local domain the server-side DN of the groups the user
is a member of is stored with the user object in the cache and used to
improve performance e.g. by the HBAC code. Since subdomain users should
be handled by HBAC as well the group DN is stored in the same way as for
users of the local domain.
---
 src/responder/pac/pacsrv_cmd.c |   53 ++++++++++++++++++++-------------------
 1 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 6c12bec..1b4ff04 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -530,11 +530,12 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
                       int gid_iter)
 {
     TALLOC_CTX *tmp_ctx;
-    const char *group_name;
-    struct sysdb_attrs *group_attrs;
+    struct sysdb_attrs *user_attrs;
     struct ldb_message *group;
     uint32_t gid;
     errno_t ret;
+    const char *orig_group_dn;
+    const char *group_attrs[] = { SYSDB_ORIG_DN, NULL };
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
@@ -544,46 +545,46 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
     gid = pr_ctx->add_gids[gid_iter];
 
     ret = sysdb_search_group_by_gid(tmp_ctx, group_sysdb,
-                                    gid, NULL, &group);
+                                    gid, group_attrs, &group);
     if (ret != EOK) {
+        DEBUG(SSSDBG_TRACE_INTERNAL, ("sysdb_search_group_by_gid for gid [%d]" \
+                                      "failed [%d][%s].\n",
+                                      gid, ret, strerror(ret)));
         goto done;
     }
 
-    group_name = ldb_msg_find_attr_as_string(group, SYSDB_NAME, NULL);
-    if (group_name == NULL) {
-        ret = EIO;
+    ret = sysdb_mod_group_member(group_sysdb, user_dn, group->dn,
+                                 LDB_FLAG_MOD_ADD);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_mod_group_member failed.\n"));
         goto done;
     }
 
-    group_attrs = talloc_zero(tmp_ctx, struct sysdb_attrs);
-    if (group_attrs == NULL) {
-        ret = ENOMEM;
+    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;
     }
-    group_attrs->num = 1;
-    group_attrs->a = ldb_msg_find_element(group, SYSDB_MEMBER);
-    if (group_attrs->a == NULL) {
-        group_attrs->a = talloc_zero(group_attrs, struct ldb_message_element);
-        if (group_attrs->a == NULL) {
-            ret = ENOMEM;
-            goto done;
-        }
-        group_attrs->a[0].name = talloc_strdup(group_attrs->a, SYSDB_MEMBER);
-        if (group_attrs->a[0].name == NULL) {
-            ret = ENOMEM;
-            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;
     }
 
-    ret = sysdb_attrs_add_string(group_attrs, SYSDB_MEMBER,
-                                 ldb_dn_get_linearized(user_dn));
+    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_store_group(group_sysdb, group_name, gid,
-                            group_attrs, pr_ctx->dom->group_timeout, 0);
+    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;
     }
 
-- 
1.7.7.6



More information about the sssd-devel mailing list