Hi,

this is my suggestion to solve https://fedorahosted.org/sssd/ticket/2948
"Handle overriden name of members in the memberUid attribute".

The first two patches are for the IPA provider and make sure that all
ghost members in a group get resolved because otherwise we cannot
determine if the name is overridden or not. This adds an overhead to
group lookups, especially for larger groups but I think it is an
requirement which cannot be skipped.

The third patch adds a sysdb call to recursively resolve all
user-members of a group. Since the groups in SSSD's cache are
hierarchically organized the member attribute only contains direct
user and group members. To get all users the group members must be
resolved recursively.

Finally the forth patch applies the code-path which is already used for
non-default views to the default case as well and adds a new list of
members, with correctly overridden names (hopefully :-) which is then
used in fill_grmem(). This adds some overhead to the overall group
processing in the NSS responder (as can be seen in the test changes
because the members are returned in different order in some cases). But
I think because the of memory cache this is acceptable and might even
help to remove the memberuid attribute in future and make the memberof
plugin simpler.

I worked on an alternative approach as well which tried to make the
memberof plugin aware of the defaultOverrideName attribute. My wip tree
is at
https://fedorapeople.org/cgit/sbose/public_git/sssd.git/log/?h=memberof_default_view
but so far it does not work properly. Additionally I would prefer to not
touch the memberof plugin.

bye,
Sumit
From 3a4ce5d188c2c97372aa92bb1e1596f8eca8c6e5 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Tue, 12 Jul 2016 17:09:00 +0200
Subject: [PATCH 1/4] IPA: make ipa_resolve_user_list_{send|recv} public and
 allow AD users

---
 src/providers/ipa/ipa_id.c | 20 ++++++++++++++++----
 src/providers/ipa/ipa_id.h |  8 ++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ipa_id.c
index 
cfc25b9b322d78bbc64de96227a03ed0604d3391..7f0cd5689dbfde390c162eef340e1120247b703c
 100644
--- a/src/providers/ipa/ipa_id.c
+++ b/src/providers/ipa/ipa_id.c
@@ -71,7 +71,7 @@ struct ipa_resolve_user_list_state {
 static errno_t ipa_resolve_user_list_get_user_step(struct tevent_req *req);
 static void ipa_resolve_user_list_get_user_done(struct tevent_req *subreq);
 
-static struct tevent_req *
+struct tevent_req *
 ipa_resolve_user_list_send(TALLOC_CTX *memctx, struct tevent_context *ev,
                            struct ipa_id_ctx *ipa_ctx,
                            const char *domain_name,
@@ -132,7 +132,14 @@ static errno_t ipa_resolve_user_list_get_user_step(struct 
tevent_req *req)
 
     DEBUG(SSSDBG_TRACE_ALL, "Trying to resolve user [%s].\n", 
ar->filter_value);
 
-    subreq = ipa_id_get_account_info_send(state, state->ev, state->ipa_ctx, 
ar);
+    if (strcasecmp(state->domain_name,
+                   state->ipa_ctx->sdap_id_ctx->be->domain->name) != 0) {
+        subreq = ipa_subdomain_account_send(state, state->ev, state->ipa_ctx,
+                                            ar);
+    } else {
+        subreq = ipa_id_get_account_info_send(state, state->ev, state->ipa_ctx,
+                                              ar);
+    }
     if (subreq == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "sdap_handle_acct_req_send failed.\n");
         return ENOMEM;
@@ -151,7 +158,12 @@ static void ipa_resolve_user_list_get_user_done(struct 
tevent_req *subreq)
                                             struct 
ipa_resolve_user_list_state);
     int ret;
 
-    ret = ipa_id_get_account_info_recv(subreq, &state->dp_error);
+    if (strcasecmp(state->domain_name,
+                   state->ipa_ctx->sdap_id_ctx->be->domain->name) != 0) {
+        ret = ipa_subdomain_account_recv(subreq, &state->dp_error);
+    } else {
+        ret = ipa_id_get_account_info_recv(subreq, &state->dp_error);
+    }
     talloc_zfree(subreq);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "sdap_handle_acct request failed: %d\n", ret);
@@ -182,7 +194,7 @@ done:
     return;
 }
 
-static int ipa_resolve_user_list_recv(struct tevent_req *req, int *dp_error)
+int ipa_resolve_user_list_recv(struct tevent_req *req, int *dp_error)
 {
     struct ipa_resolve_user_list_state *state = tevent_req_data(req,
                                             struct 
ipa_resolve_user_list_state);
diff --git a/src/providers/ipa/ipa_id.h b/src/providers/ipa/ipa_id.h
index 
410c2086833d811571195e05a7cab0e2c82fa5b9..4b25498821c6cd0d574c4ce78e4ece29f60459f7
 100644
--- a/src/providers/ipa/ipa_id.h
+++ b/src/providers/ipa/ipa_id.h
@@ -135,4 +135,12 @@ struct tevent_req 
*ipa_get_subdom_acct_process_pac_send(TALLOC_CTX *mem_ctx,
                                                    struct ldb_message 
*user_msg);
 
 errno_t ipa_get_subdom_acct_process_pac_recv(struct tevent_req *req);
+
+struct tevent_req *
+ipa_resolve_user_list_send(TALLOC_CTX *memctx, struct tevent_context *ev,
+                           struct ipa_id_ctx *ipa_ctx,
+                           const char *domain_name,
+                           struct ldb_message_element *users);
+int ipa_resolve_user_list_recv(struct tevent_req *req, int *dp_error);
+
 #endif
-- 
2.1.0

From a47bc1d1acd0e8eae2ccdab0cb285c965cd1189d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Tue, 12 Jul 2016 17:09:40 +0200
Subject: [PATCH 2/4] IPA: expand ghost members of AD groups in server-mode

---
 src/providers/ipa/ipa_subdomains_id.c | 81 ++++++++++++++++++++++++++++++++++-
 1 file changed, 80 insertions(+), 1 deletion(-)

diff --git a/src/providers/ipa/ipa_subdomains_id.c 
b/src/providers/ipa/ipa_subdomains_id.c
index 
0a89ef5a68802fb7d5f9faeb9ef1d9f9cf8d14f2..a6cf3a9072c4cc6995a5813dd52397b90dc245c8
 100644
--- a/src/providers/ipa/ipa_subdomains_id.c
+++ b/src/providers/ipa/ipa_subdomains_id.c
@@ -1201,6 +1201,69 @@ fail:
     return;
 }
 
+static void ipa_check_ghost_members_done(struct tevent_req *subreq);
+static errno_t ipa_check_ghost_members(struct tevent_req *req)
+{
+    struct ipa_get_ad_acct_state *state = tevent_req_data(req,
+                                                struct ipa_get_ad_acct_state);
+    errno_t ret;
+    struct tevent_req *subreq;
+    struct ldb_message_element *ghosts = NULL;
+
+
+    if (state->obj_msg == NULL) {
+        ret = get_object_from_cache(state, state->obj_dom, state->ar,
+                                    &state->obj_msg);
+        if (ret == ENOENT) {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  "Object not found, ending request\n");
+            return EOK;
+        } else if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "get_object_from_cache failed.\n");
+            return ret;
+        }
+    }
+
+    ghosts = ldb_msg_find_element(state->obj_msg, SYSDB_GHOST);
+
+    if (ghosts != NULL) {
+        /* Resolve ghost members */
+        subreq = ipa_resolve_user_list_send(state, state->ev,
+                                            state->ipa_ctx,
+                                            state->obj_dom->name,
+                                            ghosts);
+        if (subreq == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE, "ipa_resolve_user_list_send failed.\n");
+            return ENOMEM;
+        }
+        tevent_req_set_callback(subreq, ipa_check_ghost_members_done, req);
+        return EAGAIN;
+    }
+
+    return EOK;
+}
+
+static void ipa_check_ghost_members_done(struct tevent_req *subreq)
+{
+    struct tevent_req *req = tevent_req_callback_data(subreq,
+                                                struct tevent_req);
+    struct ipa_get_ad_acct_state *state = tevent_req_data(req,
+                                          struct ipa_get_ad_acct_state);
+    int ret;
+
+    ret = ipa_resolve_user_list_recv(subreq, NULL);
+    talloc_zfree(subreq);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "ipa_resolve_user_list request failed [%d]\n",
+                                  ret);
+        tevent_req_error(req, ret);
+        return;
+    }
+
+    tevent_req_done(req);
+    return;
+}
+
 static errno_t ipa_get_ad_apply_override_step(struct tevent_req *req)
 {
     struct ipa_get_ad_acct_state *state = tevent_req_data(req,
@@ -1228,11 +1291,27 @@ static errno_t ipa_get_ad_apply_override_step(struct 
tevent_req *req)
     entry_type = (state->ar->entry_type & BE_REQ_TYPE_MASK);
     if (entry_type != BE_REQ_INITGROUPS
             && entry_type != BE_REQ_USER
-            && entry_type != BE_REQ_BY_SECID) {
+            && entry_type != BE_REQ_BY_SECID
+            && entry_type != BE_REQ_GROUP) {
         tevent_req_done(req);
         return EOK;
     }
 
+    /* expand ghost members, if any, to get group members with overrides
+     * right. */
+    if (entry_type == BE_REQ_GROUP) {
+        ret = ipa_check_ghost_members(req);
+        if (ret == EOK) {
+            tevent_req_done(req);
+            return EOK;
+        } else if (ret == EAGAIN) {
+            return EOK;
+        } else {
+            DEBUG(SSSDBG_OP_FAILURE, "ipa_check_ghost_members failed.\n");
+            return ret;
+        }
+    }
+
     /* Replace ID with name in search filter */
     if ((entry_type == BE_REQ_USER && state->ar->filter_type == 
BE_FILTER_IDNUM)
             || (entry_type == BE_REQ_INITGROUPS
-- 
2.1.0

From a2f8440cf9848488353b320cf355a90529840879 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Wed, 20 Jul 2016 18:42:27 +0200
Subject: [PATCH 3/4] sysdb: add sysdb_get_user_members_recursively()

---
 src/db/sysdb.h     |   5 ++
 src/db/sysdb_ops.c | 205 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 210 insertions(+)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 
407ce3c18a7077e8fe45c3c9c7576ae626105122..489cb4d1b1af81711d93ed1eb0cb734955c3f8c0
 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -1260,6 +1260,11 @@ errno_t sysdb_get_sids_of_members(TALLOC_CTX *mem_ctx,
                                   const char ***_dns,
                                   size_t *_n);
 
+errno_t sysdb_get_user_members_recursively(TALLOC_CTX *mem_ctx,
+                                           struct sss_domain_info *dom,
+                                           struct ldb_dn *group_dn,
+                                           struct ldb_result **members);
+
 errno_t sysdb_handle_original_uuid(const char *orig_name,
                                    struct sysdb_attrs *src_attrs,
                                    const char *src_name,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 
4755ea3427b99a51d73b7b9134e357cf2b987613..358e0bb6d493d313ff2e2280b92506f266d3e722
 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -4706,6 +4706,211 @@ done:
     return ret;
 }
 
+static
+errno_t sysdb_get_user_members_recursively_int(TALLOC_CTX *mem_ctx,
+                                               struct sss_domain_info *dom,
+                                               struct ldb_dn *group_dn,
+                                               hash_table_t *users,
+                                               hash_table_t *groups)
+{
+    errno_t ret;
+    size_t c;
+    size_t m_count;
+    struct ldb_message **members;
+    const char *oc;
+    hash_key_t key;
+    hash_value_t value;
+    int hret;
+
+    /* Get all elemets pointed to by group members */
+    ret = sysdb_asq_search(mem_ctx, dom, group_dn, NULL, SYSDB_MEMBER,
+                           NULL /* requesting all attributes */,
+                           &m_count, &members);
+    if (ret != EOK) {
+        if (ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_asq_search failed.\n");
+        }
+        goto done;
+    }
+
+    key.type = HASH_KEY_STRING;
+
+    for (c = 0; c < m_count; c++) {
+
+        oc = ldb_msg_find_attr_as_string(members[c], SYSDB_OBJECTCLASS, NULL);
+        if (oc == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "Missing objectclass in object [%s].\n",
+                  ldb_dn_get_linearized(members[c]->dn));
+            ret = EINVAL;
+            goto done;
+        }
+
+        if (strcmp(oc, SYSDB_USER_CLASS) == 0) {
+            key.str = discard_const(ldb_dn_get_linearized(members[c]->dn));
+            if (key.str == NULL) {
+                DEBUG(SSSDBG_CRIT_FAILURE, "ldb_dn_get_linearized failed.\n");
+                ret = EFAULT;
+                goto done;
+            }
+
+            value.type = HASH_VALUE_PTR;
+            value.ptr = members[c];
+
+            hret = hash_enter(users, &key, &value);
+            if (hret != HASH_SUCCESS) {
+                DEBUG(SSSDBG_OP_FAILURE, "hash_enter failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
+
+        } else if (strcmp(oc, SYSDB_GROUP_CLASS) == 0) {
+            key.str = discard_const(ldb_dn_get_linearized(members[c]->dn));
+            if (key.str == NULL) {
+                DEBUG(SSSDBG_CRIT_FAILURE, "ldb_dn_get_linearized failed.\n");
+                ret = EFAULT;
+                goto done;
+            }
+
+            hret = hash_lookup(groups, &key, &value);
+            if (hret == HASH_SUCCESS) {
+                /* Already seen this group, skipping */
+                continue;
+            } else if (hret != HASH_ERROR_KEY_NOT_FOUND) {
+                DEBUG(SSSDBG_OP_FAILURE, "hash_lookup failed.\n");
+                ret = EFAULT;
+                goto done;
+            }
+
+            /* group was not found in the hash, adding and processing it */
+
+            value.type = HASH_VALUE_PTR;
+            value.ptr = members[c];
+
+            hret = hash_enter(groups, &key, &value);
+            if (hret != HASH_SUCCESS) {
+                DEBUG(SSSDBG_OP_FAILURE, "hash_enter failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
+
+            ret = sysdb_get_user_members_recursively_int(mem_ctx, dom,
+                                                         members[c]->dn,
+                                                         users, groups);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE,
+                      "sysdb_get_user_members_recursively_int failed.\n");
+                goto done;
+            }
+
+        } else {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "Unexpected objectclass [%s] in object [%s].\n", oc,
+                  ldb_dn_get_linearized(members[c]->dn));
+            ret = EINVAL;
+            goto done;
+        }
+    }
+
+    ret = EOK;
+
+done:
+
+    return ret;
+}
+
+errno_t sysdb_get_user_members_recursively(TALLOC_CTX *mem_ctx,
+                                           struct sss_domain_info *dom,
+                                           struct ldb_dn *group_dn,
+                                           struct ldb_result **members)
+{
+    TALLOC_CTX *tmp_ctx;
+    hash_table_t *users;
+    hash_table_t *groups;
+    int ret;
+    int hret;
+    size_t c;
+    unsigned long count;
+    struct ldb_result *res;
+    hash_value_t *values;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
+
+    ret = sss_hash_create(tmp_ctx, 10, &users);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "sss_hash_create failed.\n");
+        goto done;
+    }
+
+    ret = sss_hash_create(tmp_ctx, 10, &groups);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "sss_hash_create failed.\n");
+        goto done;
+    }
+
+    ret = sysdb_get_user_members_recursively_int(tmp_ctx, dom, group_dn, users,
+                                                 groups);
+    if (ret != EOK) {
+        if (ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "sysdb_get_user_members_recursively_int failed.\n");
+        }
+        goto done;
+    }
+
+    res = talloc_zero(tmp_ctx, struct ldb_result);
+    if (res == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
+        ret = ENOMEM;
+        goto done;
+    }
+
+    hret = hash_values(users, &count, &values);
+    if (hret != HASH_SUCCESS) {
+        DEBUG(SSSDBG_OP_FAILURE, "hash_values failed.\n");
+        ret = ENOMEM;
+        goto done;
+    }
+    if (count <= UINTMAX_MAX) {
+        res->count = count;
+    } else {
+        DEBUG(SSSDBG_CRIT_FAILURE, "More then [%llu] results.\n", UINTMAX_MAX);
+        ret = EINVAL;
+        goto done;
+    }
+    if (res->count == 0) {
+        ret = EOK;
+        goto done;
+    }
+
+    res->msgs = talloc_array(res, struct ldb_message *, res->count);
+    if (res->msgs == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_array failed.\n");
+        ret = ENOMEM;
+        goto done;
+    }
+
+    for (c = 0; c < res->count; c++) {
+        res->msgs[c] = (struct ldb_message *) talloc_steal(res->msgs,
+                                                           values[c].ptr);
+    }
+
+    ret = EOK;
+
+done:
+    if (ret == EOK) {
+        *members = talloc_steal(mem_ctx, res);
+    } else if (ret == ENOENT) {
+        DEBUG(SSSDBG_TRACE_FUNC, "No such entry\n");
+    } else {
+        DEBUG(SSSDBG_OP_FAILURE, "Error: %d (%s)\n", ret, strerror(ret));
+    }
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
 errno_t sysdb_handle_original_uuid(const char *orig_name,
                                    struct sysdb_attrs *src_attrs,
                                    const char *src_name,
-- 
2.1.0

From 1ebdd37552bac2ce05519efaf932946f79640a04 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Thu, 7 Jul 2016 18:54:02 +0200
Subject: [PATCH 4/4] views: properly override group member names

Resolves https://fedorahosted.org/sssd/ticket/2948
---
 src/db/sysdb.h                  |   3 +-
 src/db/sysdb_search.c           |  99 ++++++++++++++++-------------
 src/db/sysdb_views.c            | 136 ++++++++++++++++++----------------------
 src/responder/nss/nsssrv_cmd.c  |   7 ++-
 src/tests/cmocka/test_nss_srv.c |  26 ++++----
 5 files changed, 138 insertions(+), 133 deletions(-)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 
489cb4d1b1af81711d93ed1eb0cb734955c3f8c0..e10cf1bb26d31a0bb598a85ac38a931017c52600
 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -575,7 +575,8 @@ errno_t sysdb_add_overrides_to_object(struct 
sss_domain_info *domain,
                                       const char **req_attrs);
 
 errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,
-                                         struct ldb_message *obj);
+                                         struct ldb_message *obj,
+                                         bool expect_override_dn);
 
 errno_t sysdb_getpwnam_with_views(TALLOC_CTX *mem_ctx,
                                   struct sss_domain_info *domain,
diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
index 
e40b36c38e28992e185447497d1bf69cabc09821..cfee5784dbadd692f30d0758e7e5c3c9fb2814cb
 100644
--- a/src/db/sysdb_search.c
+++ b/src/db/sysdb_search.c
@@ -771,28 +771,33 @@ int sysdb_getgrnam_with_views(TALLOC_CTX *mem_ctx,
 
     /* If there are views we have to check if override values must be added to
      * the original object. */
-    if (DOM_HAS_VIEWS(domain) && orig_obj->count == 1) {
-        if (!is_local_view(domain->view_name)) {
-            el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST);
-            if (el != NULL && el->num_values != 0) {
-                DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost "
-                      "entries which must be resolved before overrides can be "
-                      "applied.\n",
-                      ldb_dn_get_linearized(orig_obj->msgs[0]->dn));
-                ret = ENOENT;
+    if (orig_obj->count == 1) {
+        if (DOM_HAS_VIEWS(domain)) {
+            if (!is_local_view(domain->view_name)) {
+                el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST);
+                if (el != NULL && el->num_values != 0) {
+                    DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost 
"
+                          "entries which must be resolved before overrides can 
be "
+                          "applied.\n",
+                          ldb_dn_get_linearized(orig_obj->msgs[0]->dn));
+                    ret = ENOENT;
+                    goto done;
+                }
+            }
+
+            ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0],
+                           override_obj == NULL ? NULL : override_obj 
->msgs[0],
+                           NULL);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object 
failed.\n");
                 goto done;
             }
         }
 
-        ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0],
-                          override_obj == NULL ? NULL : override_obj ->msgs[0],
-                          NULL);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object 
failed.\n");
-            goto done;
-        }
-
-        ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0]);
+        /* Must be called even without views to check to
+         * SYSDB_DEFAULT_OVERRIDE_NAME */
+        ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0],
+                                               DOM_HAS_VIEWS(domain));
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE,
                   "sysdb_add_group_member_overrides failed.\n");
@@ -922,28 +927,33 @@ int sysdb_getgrgid_with_views(TALLOC_CTX *mem_ctx,
 
     /* If there are views we have to check if override values must be added to
      * the original object. */
-    if (DOM_HAS_VIEWS(domain) && orig_obj->count == 1) {
-        if (!is_local_view(domain->view_name)) {
-            el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST);
-            if (el != NULL && el->num_values != 0) {
-                DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost "
-                      "entries which must be resolved before overrides can be "
-                      "applied.\n",
-                      ldb_dn_get_linearized(orig_obj->msgs[0]->dn));
-                ret = ENOENT;
+    if (orig_obj->count == 1) {
+        if (DOM_HAS_VIEWS(domain)) {
+            if (!is_local_view(domain->view_name)) {
+                el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST);
+                if (el != NULL && el->num_values != 0) {
+                    DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost 
"
+                          "entries which must be resolved before overrides can 
be "
+                          "applied.\n",
+                          ldb_dn_get_linearized(orig_obj->msgs[0]->dn));
+                    ret = ENOENT;
+                    goto done;
+                }
+            }
+
+            ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0],
+                              override_obj == NULL ? NULL : override_obj 
->msgs[0],
+                              NULL);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object 
failed.\n");
                 goto done;
             }
         }
 
-        ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0],
-                          override_obj == NULL ? NULL : override_obj ->msgs[0],
-                          NULL);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object 
failed.\n");
-            goto done;
-        }
-
-        ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0]);
+        /* Must be called even without views to check to
+         * SYSDB_DEFAULT_OVERRIDE_NAME */
+        ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0],
+                                               DOM_HAS_VIEWS(domain));
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE,
                   "sysdb_add_group_member_overrides failed.\n");
@@ -1157,8 +1167,8 @@ int sysdb_enumgrent_filter_with_views(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    if (DOM_HAS_VIEWS(domain)) {
-        for (c = 0; c < res->count; c++) {
+    for (c = 0; c < res->count; c++) {
+        if (DOM_HAS_VIEWS(domain)) {
             ret = sysdb_add_overrides_to_object(domain, res->msgs[c], NULL,
                                                 NULL);
             /* enumeration assumes that the cache is up-to-date, hence we do 
not
@@ -1167,13 +1177,14 @@ int sysdb_enumgrent_filter_with_views(TALLOC_CTX 
*mem_ctx,
                 DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object 
failed.\n");
                 goto done;
             }
+        }
 
-            ret = sysdb_add_group_member_overrides(domain, res->msgs[c]);
-            if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE,
-                      "sysdb_add_group_member_overrides failed.\n");
-                goto done;
-            }
+        ret = sysdb_add_group_member_overrides(domain, res->msgs[c],
+                                               DOM_HAS_VIEWS(domain));
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "sysdb_add_group_member_overrides failed.\n");
+            goto done;
         }
     }
 
diff --git a/src/db/sysdb_views.c b/src/db/sysdb_views.c
index 
2b89e5ca41f719e1217ef3b9e0fd683656e05d42..79f513d13ba41212a6cd84e1d9e609df6acba29c
 100644
--- a/src/db/sysdb_views.c
+++ b/src/db/sysdb_views.c
@@ -1348,14 +1348,13 @@ done:
 }
 
 errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,
-                                         struct ldb_message *obj)
+                                         struct ldb_message *obj,
+                                         bool expect_override_dn)
 {
     int ret;
     size_t c;
-    struct ldb_message_element *members;
+    struct ldb_result *res_members;
     TALLOC_CTX *tmp_ctx;
-    struct ldb_dn *member_dn;
-    struct ldb_result *member_obj;
     struct ldb_result *override_obj;
     static const char *member_attrs[] = SYSDB_PW_ATTRS;
     const char *override_dn_str;
@@ -1366,12 +1365,6 @@ errno_t sysdb_add_group_member_overrides(struct 
sss_domain_info *domain,
     char *val;
     struct sss_domain_info *orig_dom;
 
-    members = ldb_msg_find_element(obj, SYSDB_MEMBER);
-    if (members == NULL || members->num_values == 0) {
-        DEBUG(SSSDBG_TRACE_ALL, "Group has no members.\n");
-        return EOK;
-    }
-
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");
@@ -1379,38 +1372,30 @@ errno_t sysdb_add_group_member_overrides(struct 
sss_domain_info *domain,
         goto done;
     }
 
-    for (c = 0; c < members->num_values; c++) {
-        member_dn = ldb_dn_from_ldb_val(tmp_ctx, domain->sysdb->ldb,
-                                        &members->values[c]);
-        if (member_dn == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_from_ldb_val failed.\n");
-            ret = ENOMEM;
-            goto done;
-        }
+    ret = sysdb_get_user_members_recursively(tmp_ctx, domain, obj->dn,
+                                             &res_members);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              "sysdb_get_user_members_recursively failed.\n");
+        goto done;
+    }
 
-        ret = ldb_search(domain->sysdb->ldb, member_dn, &member_obj, member_dn,
-                         LDB_SCOPE_BASE, member_attrs, NULL);
-        if (ret != LDB_SUCCESS) {
-            ret = sysdb_error_to_errno(ret);
-            goto done;
-        }
+    for (c = 0; c < res_members->count; c++) {
 
-        if (member_obj->count != 1) {
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "Base search for member object returned [%d] results.\n",
-                  member_obj->count);
-            ret = EINVAL;
-            goto done;
-        }
-
-        if (ldb_msg_find_attr_as_uint64(member_obj->msgs[0],
+        if (ldb_msg_find_attr_as_uint64(res_members->msgs[c],
                                         SYSDB_UIDNUM, 0) == 0) {
             /* Skip non-POSIX-user members i.e. groups and non-POSIX users */
             continue;
         }
 
-        override_dn_str = ldb_msg_find_attr_as_string(member_obj->msgs[0],
-                                                      SYSDB_OVERRIDE_DN, NULL);
+        if (expect_override_dn) {
+            override_dn_str = ldb_msg_find_attr_as_string(res_members->msgs[c],
+                                                          SYSDB_OVERRIDE_DN,
+                                                          NULL);
+        } else {
+            override_dn_str = ldb_dn_get_linearized(res_members->msgs[c]->dn);
+        }
+
         if (override_dn_str == NULL) {
             if (is_local_view(domain->view_name)) {
                 /* LOCAL view doesn't have to have overrideDN specified. */
@@ -1420,12 +1405,12 @@ errno_t sysdb_add_group_member_overrides(struct 
sss_domain_info *domain,
 
             DEBUG(SSSDBG_CRIT_FAILURE,
                   "Missing override DN for object [%s].\n",
-                  ldb_dn_get_linearized(member_obj->msgs[0]->dn));
+                  ldb_dn_get_linearized(res_members->msgs[c]->dn));
             ret = ENOENT;
             goto done;
         }
 
-        override_dn = ldb_dn_new(member_obj, domain->sysdb->ldb,
+        override_dn = ldb_dn_new(res_members, domain->sysdb->ldb,
                                  override_dn_str);
         if (override_dn == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_new failed.\n");
@@ -1433,22 +1418,27 @@ errno_t sysdb_add_group_member_overrides(struct 
sss_domain_info *domain,
             goto done;
         }
 
-        orig_name = ldb_msg_find_attr_as_string(member_obj->msgs[0],
+        orig_name = ldb_msg_find_attr_as_string(res_members->msgs[c],
                                                 SYSDB_NAME,
                                                 NULL);
         if (orig_name == NULL) {
             DEBUG(SSSDBG_CRIT_FAILURE, "Object [%s] has no name.\n",
-                  ldb_dn_get_linearized(member_obj->msgs[0]->dn));
+                  ldb_dn_get_linearized(res_members->msgs[c]->dn));
             ret = EINVAL;
             goto done;
         }
 
-        memberuid = NULL;
-        if (ldb_dn_compare(member_obj->msgs[0]->dn, override_dn) != 0) {
+        /* start with default view name, if it exists or use NULL */
+        memberuid = ldb_msg_find_attr_as_string(res_members->msgs[c],
+                                                SYSDB_DEFAULT_OVERRIDE_NAME,
+                                                NULL);
+
+        /* If there is an override object, check if the name is overridden */
+        if (ldb_dn_compare(res_members->msgs[c]->dn, override_dn) != 0) {
             DEBUG(SSSDBG_TRACE_ALL, "Checking override for object [%s].\n",
-                  ldb_dn_get_linearized(member_obj->msgs[0]->dn));
+                  ldb_dn_get_linearized(res_members->msgs[c]->dn));
 
-            ret = ldb_search(domain->sysdb->ldb, member_obj, &override_obj,
+            ret = ldb_search(domain->sysdb->ldb, res_members, &override_obj,
                              override_dn, LDB_SCOPE_BASE, member_attrs, NULL);
             if (ret != LDB_SUCCESS) {
                 ret = sysdb_error_to_errno(ret);
@@ -1458,43 +1448,44 @@ errno_t sysdb_add_group_member_overrides(struct 
sss_domain_info *domain,
             if (override_obj->count != 1) {
                 DEBUG(SSSDBG_CRIT_FAILURE,
                      "Base search for override object returned [%d] 
results.\n",
-                     member_obj->count);
+                    override_obj->count);
                 ret = EINVAL;
                 goto done;
             }
 
             memberuid = ldb_msg_find_attr_as_string(override_obj->msgs[0],
                                                     SYSDB_NAME,
-                                                    NULL);
+                                                    memberuid);
+        }
 
-            if (memberuid != NULL) {
-                ret = sss_parse_internal_fqname(tmp_ctx, orig_name,
-                                                NULL, &orig_domain);
-                if (ret != EOK) {
-                    DEBUG(SSSDBG_OP_FAILURE,
-                         "sss_parse_internal_fqname failed to split [%s].\n",
-                         orig_name);
+        /* add domain name if memberuid is a short name */
+        if (memberuid != NULL && strchr(memberuid, '@') == NULL) {
+            ret = sss_parse_internal_fqname(tmp_ctx, orig_name,
+                                            NULL, &orig_domain);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE,
+                     "sss_parse_internal_fqname failed to split [%s].\n",
+                     orig_name);
+                goto done;
+            }
+
+            if (orig_domain != NULL) {
+                orig_dom = find_domain_by_name(get_domains_head(domain),
+                                               orig_domain, true);
+                if (orig_dom == NULL) {
+                    DEBUG(SSSDBG_CRIT_FAILURE,
+                          "Cannot find domain with name [%s].\n",
+                          orig_domain);
+                    ret = ERR_DOMAIN_NOT_FOUND;
                     goto done;
                 }
-
-                if (orig_domain != NULL) {
-                    orig_dom = find_domain_by_name(get_domains_head(domain),
-                                                   orig_domain, true);
-                    if (orig_dom == NULL) {
-                        DEBUG(SSSDBG_CRIT_FAILURE,
-                              "Cannot find domain with name [%s].\n",
-                              orig_domain);
-                        ret = ERR_DOMAIN_NOT_FOUND;
-                        goto done;
-                    }
-                    memberuid = sss_create_internal_fqname(tmp_ctx, memberuid,
-                                                           orig_dom->name);
-                    if (memberuid == NULL) {
-                        DEBUG(SSSDBG_OP_FAILURE,
-                              "sss_create_internal_fqname failed.\n");
-                        ret = ENOMEM;
-                        goto done;
-                    }
+                memberuid = sss_create_internal_fqname(tmp_ctx, memberuid,
+                                                       orig_dom->name);
+                if (memberuid == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                          "sss_create_internal_fqname failed.\n");
+                    ret = ENOMEM;
+                    goto done;
                 }
             }
         }
@@ -1521,9 +1512,6 @@ errno_t sysdb_add_group_member_overrides(struct 
sss_domain_info *domain,
         DEBUG(SSSDBG_TRACE_ALL, "Added [%s] to [%s].\n", memberuid,
                                 OVERRIDE_PREFIX SYSDB_MEMBERUID);
 
-        /* Free all temporary data of the current member to avoid memory usage
-         * spikes. All temporary data should be allocated below member_dn. */
-        talloc_free(member_dn);
     }
 
     ret = EOK;
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 
1ae17969688fa29734ca14fd2b152decef1fdbca..4e84b3202cbf367e70a47a3c7edb06e357657538
 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -2976,7 +2976,12 @@ static int fill_grent(struct sss_packet *packet,
 
         memnum = 0;
         if (!dom->ignore_group_members) {
-            el = sss_view_ldb_msg_find_element(dom, msg, SYSDB_MEMBERUID);
+            /* unconditionally prefer OVERRIDE_PREFIX SYSDB_MEMBERUID, it
+             * might contain override names from the default view */
+            el = ldb_msg_find_element(msg, OVERRIDE_PREFIX SYSDB_MEMBERUID);
+            if (el == NULL) {
+                el = ldb_msg_find_element(msg, SYSDB_MEMBERUID);
+            }
             if (el) {
                 ret = fill_members(packet, nctx->rctx, dom, nctx, el,
                                    &rzero, &rsize, &memnum);
diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index 
82a304feed864b09168d0f3e06a4e1bb120df7e4..892bebe63816a0dac0317dd1fd0135523f0e64d7
 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -1342,8 +1342,8 @@ static int test_nss_getgrnam_members_check(uint32_t 
status,
     int ret;
     uint32_t nmem;
     struct group gr;
-    const char *exp_members[] = { testmember1.pw_name,
-                                  testmember2.pw_name };
+    const char *exp_members[] = { testmember2.pw_name,
+                                  testmember1.pw_name };
     struct group expected = {
         .gr_gid = testgroup_members.gr_gid,
         .gr_name = testgroup_members.gr_name,
@@ -1429,10 +1429,10 @@ static int 
test_nss_getgrnam_members_check_fqdn(uint32_t status,
     assert_non_null(tmp_ctx);
 
     exp_members[0] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names,
-                                   nss_test_ctx->tctx->dom, 
testmember1.pw_name);
+                                   nss_test_ctx->tctx->dom, 
testmember2.pw_name);
     assert_non_null(exp_members[0]);
     exp_members[1] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names,
-                                   nss_test_ctx->tctx->dom, 
testmember2.pw_name);
+                                   nss_test_ctx->tctx->dom, 
testmember1.pw_name);
     assert_non_null(exp_members[1]);
 
     expected.gr_name = sss_tc_fqname(tmp_ctx,
@@ -1619,8 +1619,8 @@ static int test_nss_getgrnam_check_mix_dom(uint32_t 
status,
     tmp_ctx = talloc_new(nss_test_ctx);
     assert_non_null(tmp_ctx);
 
-    exp_members[0] = testmember1.pw_name;
-    exp_members[1] = testmember2.pw_name;
+    exp_members[0] = testmember2.pw_name;
+    exp_members[1] = testmember1.pw_name;
     exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,
                                    nss_test_ctx->subdom, submember1.pw_name);
     assert_non_null(exp_members[2]);
@@ -1683,10 +1683,10 @@ static int 
test_nss_getgrnam_check_mix_dom_fqdn(uint32_t status,
     assert_non_null(tmp_ctx);
 
     exp_members[0] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names,
-                                   nss_test_ctx->tctx->dom, 
testmember1.pw_name);
+                                   nss_test_ctx->tctx->dom, 
testmember2.pw_name);
     assert_non_null(exp_members[0]);
     exp_members[1] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names,
-                                   nss_test_ctx->tctx->dom, 
testmember2.pw_name);
+                                   nss_test_ctx->tctx->dom, 
testmember1.pw_name);
     assert_non_null(exp_members[1]);
     exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,
                                    nss_test_ctx->subdom, submember1.pw_name);
@@ -1752,16 +1752,16 @@ static int test_nss_getgrnam_check_mix_subdom(uint32_t 
status,
     tmp_ctx = talloc_new(nss_test_ctx);
     assert_non_null(tmp_ctx);
 
-    exp_members[0] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,
-                                   nss_test_ctx->subdom, submember1.pw_name);
-    assert_non_null(exp_members[0]);
     exp_members[1] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,
-                                   nss_test_ctx->subdom, submember2.pw_name);
+                                   nss_test_ctx->subdom, submember1.pw_name);
     assert_non_null(exp_members[1]);
+    exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,
+                                   nss_test_ctx->subdom, submember2.pw_name);
+    assert_non_null(exp_members[2]);
     /* Important: this member is from a non-qualified domain, so his name will
      * not be qualified either
      */
-    exp_members[2] = testmember1.pw_name;
+    exp_members[0] = testmember1.pw_name;
 
     expected.gr_name = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,
                                      nss_test_ctx->subdom, 
testsubdomgroup.gr_name);
-- 
2.1.0

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to