Hi,

I am working on [1] LDAP provider doesn't show group member

There is one point what I am not able to understand clearly.

I prepared environment by:

ipa user-add --first=Test --last=User1 --email=u...@domain.sssd cmt_user_1
ipa group-add cmt_group_1
ipa group-add-member --users=cmt_user_1 cmt_group_1

And sssd.conf is attached. I used patch 0007 [2] to simpler grep of interesting messages. And I applied patches 0001 to 0006 (I am waiting for their push to master).


[1] https://fedorahosted.org/sssd/ticket/3186
[2] 0007-WIP-Debuging-of-attr-member-in-ldap-answer.patch


What I saw is:


# COMMAND ON CLIENT:
[root@mirach sssd]# systemctl daemon-reload && sudo su -c "truncate -s0 /var/log/sssd/*.log" && sudo su -c "rm -f /var/lib/sss/db/*" && sudo su -c "rm -f /var/lib/sss/mc/*" && sudo systemctl restart sssd.service && getent group cmt_group_1 && grep '>>>' *.log


# There is user missing:
cmt_group_1:*:1703800166:


# SSSD LOG:
[sdap_get_generic_ext_step] (0x0400): >>> calling ldap_search_ext with [(&(cn=cmt_group_1)(objectClass=posixGroup)(cn=*)(&(gidNumber=*)(!(gidNumber=0))))][cn=groups,cn=accounts,dc=beta].
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [objectClass]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [cn]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [userPassword]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [gidNumber]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [member]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [modifyTimestamp]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [entryUSN]
[sdap_parse_entry] (0x0080): >>> ----- filter map:
[sdap_parse_entry] (0x0080): >>>   [posixGroup]
[sdap_parse_entry] (0x0080): >>>   [(null)]
[sdap_parse_entry] (0x0080): >>>   [cn]
[sdap_parse_entry] (0x0080): >>>   [userPassword]
[sdap_parse_entry] (0x0080): >>>   [gidNumber]
[sdap_parse_entry] (0x0080): >>>   [member]
[sdap_parse_entry] (0x0080): >>>   [(null)]
[sdap_parse_entry] (0x0080): >>>   [(null)]
[sdap_parse_entry] (0x0080): >>>   [modifyTimestamp]
[sdap_parse_entry] (0x0080): >>>   [entryUSN]
[sdap_parse_entry] (0x0080): >>>   [(null)]
[sdap_parse_entry] (0x0080): >>>   [(null)]
[sdap_parse_entry] (0x0080): >>> ----- attributes in ldap answer:
[sdap_parse_entry] (0x0020): >>>   [objectClass]
[sdap_parse_entry] (0x0020): >>>   [cn]
[sdap_parse_entry] (0x0020): >>>   [gidNumber]
[sdap_parse_entry] (0x0020): >>>   [modifyTimestamp]
[sdap_parse_entry] (0x0020): >>>   [entryUSN]
[sdap_get_groups_process] (0x0400): >>> Group contains:
[sdap_get_groups_process] (0x0400): >>>   0: [originalDN]
[sdap_get_groups_process] (0x0400): >>>   1: [name]
[sdap_get_groups_process] (0x0400): >>>   2: [gidNumber]
[sdap_get_groups_process] (0x0400): >>>   3: [originalModifyTimestamp]
[sdap_get_groups_process] (0x0400): >>>   4: [entryUSN]
[sdap_nested_group_process_send] (0x2000): >>> About to process group [cn=cmt_group_1,cn=groups,cn=accounts,dc=beta]


# LDAPSEARCH ON LDAP SERVER:
[pcech@algol ~]$ ldapsearch -H ldap://localhost -D uid=admin,cn=users,cn=accounts,dc=beta -w myspulin -b cn=groups,cn=accounts,dc=beta "(&(cn=cmt_group_1)(objectClass=posixGroup)(cn=*)(&(gidNumber=*)(!(gidNumber=0))))"

# extended LDIF
#
# LDAPv3
# base <cn=groups,cn=accounts,dc=beta> with scope subtree
# filter: (&(cn=cmt_group_1)(objectClass=posixGroup)(cn=*)(&(gidNumber=*)(!(gidNumber=0))))
# requesting: ALL
#

# cmt_group_1, groups, accounts, beta
dn: cn=cmt_group_1,cn=groups,cn=accounts,dc=beta
objectClass: ipaobject
objectClass: top
objectClass: ipausergroup
objectClass: posixgroup
objectClass: groupofnames
objectClass: nestedgroup
cn: cmt_group_1
ipaUniqueID: db0b564e-7efe-11e6-a757-5254001a3efa
gidNumber: 1703800166
member: uid=cmt_user_1,cn=users,cn=accounts,dc=beta

# search result
search: 2
result: 0 Success

# numResponses: 2
# numEntries: 1


Question is why didn't SSSD obtain member attribute if I saw it in ldapsearch call? Is there something what I miss?

Lukas pointed out that it may not be the correct way to use IPA for handling users in groups in LDAP server. I understand I am working on AD case too.

But -- is it really possible to get different data via SSSD and ldapsearch() with the same filter and base?


Regards

--
Petr^4 Čech
[domain/ldap.beta]

id_provider = ldap
auth_provider = ldap
ldap_schema = rfc2307bis
ldap_uri = ldaps://algol.beta/
ldap_search_base = dc=beta
ldap_user_search_base = cn=users,cn=accounts,dc=beta
ldap_group_search_base = cn=groups,cn=accounts,dc=beta
ldap_netgroup_search_base = dc=beta
ldap_tls_cacert = /etc/ipa/ca.crt
entry_cache_timeout = 30
debug_level = 0xFFFFF0
timeout = 50000


[domain/ipa.beta]

cache_credentials = True
krb5_store_password_if_offline = True
ipa_domain = beta
id_provider = ipa
auth_provider = ipa
access_provider = ipa
ipa_hostname = mirach.beta
chpass_provider = ipa
dyndns_update = True
ipa_server = _srv_, algol.beta
dyndns_iface = ens3
ldap_tls_cacert = /etc/ipa/ca.crt
entry_cache_timeout = 30
debug_level = 0xFFFF0


[sssd]
services = nss, sudo, pam, ssh
domains = ldap.beta
debug_level = 0xFFFFF0


[nss]
homedir_substring = /home
>From d49c32634e4a21e2706cc51f6a34dc94a5186522 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C4=8Cech?= <pc...@redhat.com>
Date: Wed, 21 Sep 2016 15:13:34 +0200
Subject: [PATCH 7/7] WIP: Debuging of attr member in ldap answer

Explanation

Resolves:
https://fedorahosted.org/sssd/ticket/3186
---
 configure.ac                                  |  2 +-
 src/providers/ldap/sdap.c                     | 24 ++++++++++++++++++------
 src/providers/ldap/sdap_async.c               |  4 ++--
 src/providers/ldap/sdap_async_groups.c        |  6 ++++++
 src/providers/ldap/sdap_async_nested_groups.c |  6 +++---
 5 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/configure.ac b/configure.ac
index 8760a8561979664a02bc3c2b2b2994a73825f2c3..8dd1ebbfe2f91699874671a5d82365d7a8e2a53a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -11,7 +11,7 @@ m4_ifdef([AC_USE_SYSTEM_EXTENSIONS],
     [AC_USE_SYSTEM_EXTENSIONS],
     [AC_GNU_SOURCE])
 
-CFLAGS="$CFLAGS -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE"
+CFLAGS="$CFLAGS -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -O0"
 
 
 AM_INIT_AUTOMAKE([-Wall -Wno-portability foreign subdir-objects tar-pax
diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index dc7d5e0caf223c3ee3c43054aa44e796f1b37766..49879cf4897d4a5ad2e0aba3f023d66b1946d828 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -477,7 +477,19 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
             goto done;
         }
     }
+
+
+    DEBUG(SSSDBG_MINOR_FAILURE, ">>> ----- filter map:\n");
+    for (int ijk = 0; ijk < attrs_num; ijk++) {
+        DEBUG(SSSDBG_MINOR_FAILURE, ">>>   [%s]\n", map[ijk].name);
+    }
+
+
+    DEBUG(SSSDBG_MINOR_FAILURE, ">>> ----- attributes in ldap answer:\n");
     while (str) {
+
+        DEBUG(SSSDBG_CRIT_FAILURE, ">>>   [%s]\n", str);
+
         base64 = false;
 
         ret = sdap_parse_range(tmp_ctx, str, &base_attr, &range_offset,
@@ -497,7 +509,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
             break;
         default:
             DEBUG(SSSDBG_CRIT_FAILURE,
-                  "Could not determine if attribute [%s] was ranged\n", str);
+                  ">>> Could not determine if attribute [%s] was ranged\n", str);
             goto done;
         }
 
@@ -535,19 +547,19 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
             if (!vals) {
                 ldap_get_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
                 if (lerrno != LDAP_SUCCESS) {
-                    DEBUG(SSSDBG_CRIT_FAILURE, "LDAP Library error: %d(%s)\n",
+                    DEBUG(SSSDBG_CRIT_FAILURE, ">>> LDAP Library error: %d(%s)\n",
                           lerrno, sss_ldap_err2string(lerrno));
                     ret = EIO;
                     goto done;
                 }
 
                 DEBUG(SSSDBG_TRACE_LIBS,
-                      "Attribute [%s] has no values, skipping.\n", str);
+                      ">>> Attribute [%s] has no values, skipping.\n", str);
 
             } else {
                 if (!vals[0]) {
                     DEBUG(SSSDBG_CRIT_FAILURE,
-                          "Missing value after ldap_get_values() ??\n");
+                          ">>> Missing value after ldap_get_values() ??\n");
                     ldap_value_free_len(vals);
                     ret = EINVAL;
                     goto done;
@@ -555,7 +567,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
                 for (i = 0; vals[i]; i++) {
                     if (vals[i]->bv_len == 0) {
                         DEBUG(SSSDBG_TRACE_LIBS,
-                              "Value of attribute [%s] is empty. "
+                              ">>> Value of attribute [%s] is empty. "
                                "Skipping this value.\n", str);
                         continue;
                     }
@@ -615,7 +627,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
 
     ldap_get_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
     if (lerrno) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "LDAP Library error: %d(%s)\n",
+        DEBUG(SSSDBG_CRIT_FAILURE, ">>> LDAP Library error: %d(%s)\n",
               lerrno, sss_ldap_err2string(lerrno));
         ret = EIO;
         goto done;
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index f374112935a7befa1d059df97f3119c14d8f5da5..7a4237e97c261baa9cec618b5cef3b348717f401 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -1329,7 +1329,7 @@ static errno_t sdap_get_generic_ext_step(struct tevent_req *req)
     talloc_zfree(state->op);
 
     DEBUG(SSSDBG_TRACE_FUNC,
-         "calling ldap_search_ext with [%s][%s].\n",
+         ">>> calling ldap_search_ext with [%s][%s].\n",
           state->filter ? state->filter : "no filter",
           state->search_base);
     if (DEBUG_IS_SET(SSSDBG_TRACE_LIBS)) {
@@ -1338,7 +1338,7 @@ static errno_t sdap_get_generic_ext_step(struct tevent_req *req)
         if (state->attrs) {
             for (i = 0; state->attrs[i]; i++) {
                 DEBUG(SSSDBG_TRACE_LIBS,
-                      "Requesting attrs: [%s]\n", state->attrs[i]);
+                      ">>> Requesting attrs: [%s]\n", state->attrs[i]);
             }
         }
     }
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 08dfa01b1bc0cbde94928a2b577fd55667fbd48a..75fd814efe9694439013fe31823ac4b9d248da41 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -1995,6 +1995,12 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
     DEBUG(SSSDBG_TRACE_FUNC,
           "Search for groups, returned %zu results.\n", count);
 
+    DEBUG(SSSDBG_TRACE_FUNC, ">>> Group contains:\n");
+    for (int j = 0; j < groups[0]->num; j++) {
+        DEBUG(SSSDBG_TRACE_FUNC, ">>>   %d: [%s]\n", j, groups[0]->a[j].name);
+    }
+
+
     if (state->lookup_type == SDAP_LOOKUP_WILDCARD || \
             state->lookup_type == SDAP_LOOKUP_ENUMERATE || \
         count == 0) {
diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c
index 3e3329c0e8fba1915e2e065abb0cb3f21be36e6f..440d9d355dad6e4a1e599ed27045414c20e71036 100644
--- a/src/providers/ldap/sdap_async_nested_groups.c
+++ b/src/providers/ldap/sdap_async_nested_groups.c
@@ -1055,7 +1055,7 @@ sdap_nested_group_process_send(TALLOC_CTX *mem_ctx,
         goto immediately;
     }
 
-    DEBUG(SSSDBG_TRACE_INTERNAL, "About to process group [%s]\n", orig_dn);
+    DEBUG(SSSDBG_TRACE_INTERNAL, ">>> About to process group [%s]\n", orig_dn);
     PROBE(SDAP_NESTED_GROUP_PROCESS_SEND, state->group_dn);
 
     /* get member list, both direct and external */
@@ -1117,7 +1117,7 @@ sdap_nested_group_process_send(TALLOC_CTX *mem_ctx,
      */
 
     DEBUG(SSSDBG_TRACE_INTERNAL,
-          "Looking up %d/%d members of group [%s]\n",
+          ">>> Looking up %d/%d members of group [%s]\n",
           state->num_missing_total,
           state->members ? state->members->num_values : 0,
           orig_dn);
@@ -1132,7 +1132,7 @@ sdap_nested_group_process_send(TALLOC_CTX *mem_ctx,
                                               state->members, orig_dn,
                                               state->nesting_level);
     } else {
-        DEBUG(SSSDBG_TRACE_INTERNAL, "Members of group [%s] will be "
+        DEBUG(SSSDBG_TRACE_INTERNAL, ">>> Members of group [%s] will be "
                                       "processed individually\n", orig_dn);
         state->deref = false;
         subreq = sdap_nested_group_single_send(state, ev, group_ctx,
-- 
2.7.4

>From f21c467a11f82d75f31d68e9d918746ff5a6cc2d Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 16 Aug 2016 09:32:18 +0200
Subject: [PATCH 1/7] SYSDB: Adding message to inform which cache is used

Resolves:
https://fedorahosted.org/sssd/ticket/3060
---
 src/db/sysdb_ops.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 5d9c9fb24a149f8215b3027dcb4b0e1a183e4b43..32666f5d67621ec1f7f8122a27a087eacf4ca08a 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -27,6 +27,11 @@
 #include "util/cert.h"
 #include <time.h>
 
+
+#define SSS_SYSDB_NO_CACHE 0x0
+#define SSS_SYSDB_CACHE 0x1
+#define SSS_SYSDB_TS_CACHE 0x2
+
 static uint32_t get_attr_as_uint32(struct ldb_message *msg, const char *attr)
 {
     const struct ldb_val *v = ldb_msg_find_ldb_val(msg, attr);
@@ -1176,14 +1181,66 @@ done:
     return ret;
 }
 
+static errno_t get_attr_storage(TALLOC_CTX *mem_ctx,
+                                int state_mask,
+                                char **_storage)
+{
+    TALLOC_CTX *tmp_ctx;
+    char **storage_list = NULL;
+    char *storage;
+    errno_t ret;
+
+    tmp_ctx = talloc_new(NULL);
+    if (!tmp_ctx) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    if ((state_mask |= SSS_SYSDB_CACHE) != 0) {
+        ret = add_string_to_list(tmp_ctx, "cache", &storage_list);
+        if (ret != EOK) {
+            ret = ENOMEM;
+            goto done;
+        }
+    }
+
+    if ((state_mask |= SSS_SYSDB_TS_CACHE) != 0) {
+        ret = add_string_to_list(tmp_ctx, "ts_cache", &storage_list);
+        if (ret != EOK) {
+            ret = ENOMEM;
+            goto done;
+        }
+    }
+
+    storage = talloc_strdup(tmp_ctx, "");
+    if (storage_list != NULL) {
+        for (int i = 0; storage_list[i] != NULL; i++) {
+            storage = talloc_strdup_append(storage, storage_list[i]);
+            if (storage_list[i + 1] != NULL) {
+                storage = talloc_strdup_append(storage, ", ");
+            }
+        }
+    }
+
+    *_storage = talloc_steal(mem_ctx, storage);
+    ret = EOK;
+
+done:
+    talloc_zfree(tmp_ctx);
+    return ret;
+}
+
 int sysdb_set_entry_attr(struct sysdb_ctx *sysdb,
                          struct ldb_dn *entry_dn,
                          struct sysdb_attrs *attrs,
                          int mod_op)
 {
+    TALLOC_CTX *tmp_ctx;
     bool sysdb_write = true;
     errno_t ret = EOK;
     errno_t tret = EOK;
+    int state_mask = SSS_SYSDB_NO_CACHE;
+    char *used_storage = NULL;
 
     sysdb_write = sysdb_entry_attrs_diff(sysdb, entry_dn, attrs, mod_op);
     if (sysdb_write == true) {
@@ -1192,6 +1249,8 @@ int sysdb_set_entry_attr(struct sysdb_ctx *sysdb,
             DEBUG(SSSDBG_MINOR_FAILURE,
                   "Cannot set attrs for %s, %d [%s]\n",
                   ldb_dn_get_linearized(entry_dn), ret, sss_strerror(ret));
+        } else {
+            state_mask |= SSS_SYSDB_CACHE;
         }
     }
 
@@ -1201,9 +1260,31 @@ int sysdb_set_entry_attr(struct sysdb_ctx *sysdb,
             DEBUG(SSSDBG_MINOR_FAILURE,
                 "Cannot set ts attrs for %s\n", ldb_dn_get_linearized(entry_dn));
             /* Not fatal */
+        } else {
+            state_mask |= SSS_SYSDB_TS_CACHE;
         }
     }
 
+    if (state_mask != SSS_SYSDB_NO_CACHE) {
+
+        tmp_ctx = talloc_new(NULL);
+        if (tmp_ctx == NULL) {
+            goto done;
+        }
+
+        ret = get_attr_storage(tmp_ctx, state_mask, &used_storage);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_MINOR_FAILURE, "Function get_attr_storage failed.\n");
+            talloc_zfree(tmp_ctx);
+            goto done;
+        }
+        DEBUG(SSSDBG_TRACE_INTERNAL, "Attrs [%s] set for entry [%s].\n",
+                                     used_storage,
+                                     ldb_dn_get_linearized(entry_dn));
+        talloc_zfree(tmp_ctx);
+    }
+
+done:
     return ret;
 }
 
-- 
2.7.4

>From 005befc5ca5fe0a75bc9fb17d32edfdd0c8950fe Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 16 Aug 2016 09:33:46 +0200
Subject: [PATCH 2/7] SYSDB: Adding message about reason why cache changed

Resolves:
https://fedorahosted.org/sssd/ticket/3060
---
 src/db/sysdb.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/db/sysdb.c b/src/db/sysdb.c
index 6f0b1b9e9b52bede68f03cb5674f65b91cc28c98..b67769ed11fc0796d1987f09aa568c2db4a0ffab 100644
--- a/src/db/sysdb.c
+++ b/src/db/sysdb.c
@@ -1821,7 +1821,8 @@ bool sysdb_msg_attrs_modts_differs(struct ldb_message *old_entry,
     return true;
 }
 
-static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg,
+static bool sysdb_ldb_msg_difference(struct ldb_dn *entry_dn,
+                                     struct ldb_message *db_msg,
                                      struct ldb_message *mod_msg)
 {
     struct ldb_message_element *mod_msg_el;
@@ -1848,6 +1849,9 @@ static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg,
                  */
                 if (mod_msg_el->num_values > 0) {
                     /* We can ignore additions of timestamp attributes */
+                    DEBUG(SSSDBG_TRACE_INTERNAL,
+                          "Added attr [%s] to entry [%s]\n",
+                          mod_msg_el->name, ldb_dn_get_linearized(entry_dn));
                     return true;
                 }
                 break;
@@ -1855,12 +1859,15 @@ static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg,
 
             el_differs = ldb_msg_element_compare(db_msg_el, mod_msg_el);
             if (el_differs) {
-                /* We are replacing or extending element, there is a difference. If
-                 * some values already exist and ldb_add is not permissive,
+                /* We are replacing or extending element, there is a difference.
+                 * If some values already exist and ldb_add is not permissive,
                  * ldb will throw an error, but that's not our job to check..
                  */
                 if (is_ts_cache_attr(mod_msg_el->name) == false) {
                     /* We can ignore changes to timestamp attributes */
+                    DEBUG(SSSDBG_TRACE_INTERNAL,
+                          "Replaced/extended attr [%s] of entry [%s]\n",
+                          mod_msg_el->name, ldb_dn_get_linearized(entry_dn) );
                     return true;
                 }
             }
@@ -1869,6 +1876,9 @@ static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg,
             db_msg_el = ldb_msg_find_element(db_msg, mod_msg_el->name);
             if (db_msg_el != NULL) {
                 /* We are deleting a valid element, there is a difference */
+                DEBUG(SSSDBG_TRACE_INTERNAL,
+                      "Deleted attr [%s] of entry [%s].\n",
+                      mod_msg_el->name, ldb_dn_get_linearized(entry_dn));
                 return true;
             }
             break;
@@ -1892,10 +1902,16 @@ bool sysdb_entry_attrs_diff(struct sysdb_ctx *sysdb,
     const char *attrnames[attrs->num+1];
 
     if (sysdb->ldb_ts == NULL) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+              "Entry [%s] differs, reason: there is no ts_cache yet.\n",
+              ldb_dn_get_linearized(entry_dn));
         return true;
     }
 
     if (is_ts_ldb_dn(entry_dn) == false) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+              "Entry [%s] differs, reason: ts_cache doesn't trace this type of entry.\n",
+              ldb_dn_get_linearized(entry_dn));
         return true;
     }
 
@@ -1930,7 +1946,7 @@ bool sysdb_entry_attrs_diff(struct sysdb_ctx *sysdb,
         goto done;
     }
 
-    differs = sysdb_ldb_msg_difference(res->msgs[0], new_entry_msg);
+    differs = sysdb_ldb_msg_difference(entry_dn, res->msgs[0], new_entry_msg);
 done:
     talloc_free(tmp_ctx);
     return differs;
-- 
2.7.4

>From 4cbbea453cbe366239fb69c64f0b3dbde82f3b4c Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 16 Aug 2016 14:59:06 +0200
Subject: [PATCH 3/7] SYSDB: Adding wrappers for ldb_* operations

This patch adds 4 wrappers:
* sss_ldb_add()
* sss_ldb_delete()
* sss_ldb_modify()
* sss_ldb_rename()

Those wrappers write ldif message to log if
debug_level = SSSDBG_LDB.

Adding and modifying produce full ldif.
Deleting and renaming produce only short message.

If SSSDBG_LDB is not set, wrappers collapse to normal
ldb_* functions without additonial memory consumption.

Resolves:
https://fedorahosted.org/sssd/ticket/3060
---
 Makefile.am                |   2 +
 src/db/sysdb_ldb_wrapper.c | 138 +++++++++++++++++++++++++++++++++++++++++++++
 src/db/sysdb_ldb_wrapper.h |  35 ++++++++++++
 src/util/debug.h           |   1 +
 4 files changed, 176 insertions(+)
 create mode 100644 src/db/sysdb_ldb_wrapper.c
 create mode 100644 src/db/sysdb_ldb_wrapper.h

diff --git a/Makefile.am b/Makefile.am
index f792ed6a6b531d9e6e2c886c2fbe64e1e2345b73..591bd513a544d383d5812225bf0e4ab01389dd9a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -644,6 +644,7 @@ dist_noinst_HEADERS = \
     src/db/sysdb_private.h \
     src/db/sysdb_services.h \
     src/db/sysdb_ssh.h \
+    src/db/sysdb_ldb_wrapper.h \
     src/confdb/confdb.h \
     src/confdb/confdb_private.h \
     src/confdb/confdb_setup.h \
@@ -892,6 +893,7 @@ pkglib_LTLIBRARIES += libsss_util.la
 libsss_util_la_SOURCES = \
     src/confdb/confdb.c \
     src/db/sysdb.c \
+    src/db/sysdb_ldb_wrapper.c \
     src/db/sysdb_ops.c \
     src/db/sysdb_search.c \
     src/db/sysdb_selinux.c \
diff --git a/src/db/sysdb_ldb_wrapper.c b/src/db/sysdb_ldb_wrapper.c
new file mode 100644
index 0000000000000000000000000000000000000000..9aa32648987a241088d2701298e7404a76db2d52
--- /dev/null
+++ b/src/db/sysdb_ldb_wrapper.c
@@ -0,0 +1,138 @@
+/*
+   SSSD
+
+   System Database -- ldb wrappers
+
+   Copyright (C) Petr Cech <pc...@redhat.com>	2016
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <string.h>
+#include <ldb.h>
+#include "util/util.h"
+
+struct sss_ldif_vprint_ctx {
+    char *ldif;
+};
+
+static int ldif_vprintf_fn(void *private_data, const char *fmt, ...)
+{
+    struct sss_ldif_vprint_ctx *print_ctx;
+    va_list ap;
+    int lenght = 0;
+
+    /* Note that the function should return the number of
+     * bytes written, or a negative error code.
+     */
+
+    print_ctx = talloc_get_type(private_data, struct sss_ldif_vprint_ctx);
+
+    if (print_ctx == NULL) {
+        return (-1 * ENOMEM);
+    }
+
+    if (fmt != NULL) {
+        va_start(ap, fmt);
+
+        if (print_ctx->ldif != NULL) {
+            lenght = strlen(print_ctx->ldif);
+        }
+
+        print_ctx->ldif = talloc_vasprintf_append_buffer(print_ctx->ldif,
+                                                         fmt, ap);
+        if (print_ctx->ldif == NULL) {
+            return (-1 * ENOENT);
+        }
+
+        lenght = strlen(print_ctx->ldif) - lenght;
+        va_end(ap);
+    }
+
+    return lenght;
+}
+
+static void sss_ldb_ldif2log(enum ldb_changetype changetype,
+                             struct ldb_context *ldb,
+                             const struct ldb_message *message)
+{
+    int ret;
+    struct ldb_ldif ldif;
+    struct sss_ldif_vprint_ctx *ldb_print_ctx;
+
+    ldb_print_ctx = talloc_zero(ldb, struct sss_ldif_vprint_ctx);
+    if (ldb_print_ctx == NULL) {
+        return;
+    }
+    ldb_print_ctx->ldif = NULL;
+
+    ldif.changetype = changetype;
+    ldif.msg = discard_const_p(struct ldb_message, message);
+
+    ret = ldb_ldif_write(ldb, ldif_vprintf_fn, ldb_print_ctx, &ldif);
+    if (ret < 0) {
+        ret = -1 * ret;
+        DEBUG(SSSDBG_MINOR_FAILURE, "ldb_ldif_write() failed with [%d][%s].\n",
+                                    ret, sss_strerror(ret));
+        goto done;
+    }
+
+    DEBUG(SSSDBG_LDB, "ldif\n[\n%s\n]\n", ldb_print_ctx->ldif);
+
+done:
+    talloc_free(ldb_print_ctx->ldif);
+    talloc_free(ldb_print_ctx);
+
+    return;
+}
+
+int sss_ldb_add(struct ldb_context *ldb, const struct ldb_message *message)
+{
+    if (DEBUG_IS_SET(SSSDBG_LDB) == true) {
+        sss_ldb_ldif2log(LDB_CHANGETYPE_ADD, ldb, message);
+    }
+
+    return ldb_add(ldb, message);
+}
+
+int sss_ldb_delete(struct ldb_context *ldb, struct ldb_dn *dn)
+{
+    if (DEBUG_IS_SET(SSSDBG_LDB) == true) {
+        DEBUG(SSSDBG_LDB, "Deleting [%s]\n", ldb_dn_get_rdn_name(dn));
+    }
+
+    return ldb_delete(ldb, dn);
+}
+
+int sss_ldb_modify(struct ldb_context *ldb, const struct ldb_message *message)
+{
+    if (DEBUG_IS_SET(SSSDBG_LDB) == true) {
+        sss_ldb_ldif2log(LDB_CHANGETYPE_MODIFY, ldb, message);
+    }
+
+    return ldb_modify(ldb, message);
+}
+
+int sss_ldb_rename(struct ldb_context *ldb,
+                   struct ldb_dn *olddn,
+                   struct ldb_dn *newdn)
+{
+    if (DEBUG_IS_SET(SSSDBG_LDB) == true) {
+        DEBUG(SSSDBG_LDB, "Renaming [%s] to [%s]\n",
+                          ldb_dn_get_rdn_name(olddn),
+                          ldb_dn_get_rdn_name(newdn));
+    }
+
+    return ldb_rename(ldb, olddn, newdn);
+}
diff --git a/src/db/sysdb_ldb_wrapper.h b/src/db/sysdb_ldb_wrapper.h
new file mode 100644
index 0000000000000000000000000000000000000000..07a42f0f110ded75c4ad5048e23dd13429a449c1
--- /dev/null
+++ b/src/db/sysdb_ldb_wrapper.h
@@ -0,0 +1,35 @@
+/*
+   SSSD
+
+   System Database -- ldb wrappers
+
+   Copyright (C) Petr Cech <pc...@redhat.com>	2016
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __SYSDB_LDB_WRAPPER_H__
+#define __SYSDB_LDB_WRAPPER_H__
+
+int sss_ldb_add(struct ldb_context *ldb, const struct ldb_message *message);
+
+int sss_ldb_delete(struct ldb_context *ldb, struct ldb_dn *dn);
+
+int sss_ldb_modify(struct ldb_context *ldb, const struct ldb_message *message);
+
+int sss_ldb_rename(struct ldb_context *ldb,
+                   struct ldb_dn *olddn,
+                   struct ldb_dn *newdn);
+
+#endif /* __SYSDB_LDB_WRAPPER_H__ */
diff --git a/src/util/debug.h b/src/util/debug.h
index 2a1bd4ffd30817d7128805996c21105fe40982a2..610a65b6b6b9a41c9d7062b118abe5f015e10d68 100644
--- a/src/util/debug.h
+++ b/src/util/debug.h
@@ -67,6 +67,7 @@ int get_fd_from_debug_file(void);
 #define SSSDBG_TRACE_INTERNAL 0x2000   /* level 8 */
 #define SSSDBG_TRACE_ALL      0x4000   /* level 9 */
 #define SSSDBG_BE_FO          0x8000   /* level 9 */
+#define SSSDBG_LDB            0x10000  /* level 9 */
 #define SSSDBG_IMPORTANT_INFO SSSDBG_OP_FAILURE
 
 #define SSSDBG_INVALID        -1
-- 
2.7.4

>From 4101533792212c052dc110a87df3578edc078494 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C4=8Cech?= <pc...@redhat.com>
Date: Wed, 31 Aug 2016 12:28:48 +0200
Subject: [PATCH 4/7] MEMBEROF: Don't resolve members if they are removed

If we need remove ghost (SYSDB_GHOST, DB_GHOST) attribute
from group we use empty structure.

This doesn't mean that there is pointer to NULL but
it means that there is zero elements.
Ghost attribute is array not string.

Resolves:
https://fedorahosted.org/sssd/ticket/2940
---
 src/ldb_modules/memberof.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/ldb_modules/memberof.c b/src/ldb_modules/memberof.c
index af7147ee7cc9299d4040d63a637373842dcee02a..b50127dfd3cf864b71d75be92d9b0cdc254227f2 100644
--- a/src/ldb_modules/memberof.c
+++ b/src/ldb_modules/memberof.c
@@ -2920,7 +2920,8 @@ static int memberof_mod(struct ldb_module *module, struct ldb_request *req)
     mod_ctx->ghel = ldb_msg_find_element(mod_ctx->msg, DB_GHOST);
 
     /* continue with normal ops if there are no members and no ghosts */
-    if (mod_ctx->membel == NULL && mod_ctx->ghel == NULL) {
+    if (mod_ctx->membel == NULL &&
+        (mod_ctx->ghel == NULL || mod_ctx->ghel->num_values == 0)) {
         mod_ctx->terminate = true;
         return mbof_orig_mod(mod_ctx);
     }
-- 
2.7.4

>From 6f04a73fda507f04add2ce3b618114f3f84eeee8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C4=8Cech?= <pc...@redhat.com>
Date: Mon, 12 Sep 2016 15:18:07 +0200
Subject: [PATCH 5/7] LDAP: Removing of member link from group

Co-author: Sumit Bose

Resolves:
https://fedorahosted.org/sssd/ticket/2940
---
 src/providers/ldap/sdap_async_groups.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 72760b75acae4cb6ce15c72f16dae8e859d89847..08dfa01b1bc0cbde94928a2b577fd55667fbd48a 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -878,6 +878,8 @@ static int sdap_save_grpmem(TALLOC_CTX *memctx,
     size_t nuserdns = 0;
     struct sss_domain_info *group_dom = NULL;
     int ret;
+    const char *remove_attrs[] = {SYSDB_MEMBER, SYSDB_ORIG_MEMBER, SYSDB_GHOST,
+                                  NULL};
 
     if (dom->ignore_group_members) {
         DEBUG(SSSDBG_CRIT_FAILURE,
@@ -962,6 +964,13 @@ static int sdap_save_grpmem(TALLOC_CTX *memctx,
     if (el->num_values == 0 && nuserdns == 0) {
         DEBUG(SSSDBG_TRACE_FUNC,
               "No members for group [%s]\n", group_name);
+
+        ret = sysdb_remove_attrs(group_dom, group_name, SYSDB_MEMBER_GROUP,
+                                 discard_const(remove_attrs));
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_remove_attrs failed.\n");
+            goto fail;
+        }
     } else {
         DEBUG(SSSDBG_TRACE_FUNC,
               "Adding member users to group [%s]\n", group_name);
-- 
2.7.4

>From 9774710acbc38b6d057043d12818c26ef86550ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C4=8Cech?= <pc...@redhat.com>
Date: Fri, 9 Sep 2016 06:28:01 +0200
Subject: [PATCH 6/7] TESTS: Adding intg. tests on nested groups

Resolves:
https://fedorahosted.org/sssd/ticket/2940
---
 src/tests/intg/test_ldap.py | 157 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 157 insertions(+)

diff --git a/src/tests/intg/test_ldap.py b/src/tests/intg/test_ldap.py
index 11792f54be2879e92c318bd323d33e7c19dbdae9..7f0b8ff18298bff81527286575c887f8a21195c4 100644
--- a/src/tests/intg/test_ldap.py
+++ b/src/tests/intg/test_ldap.py
@@ -794,3 +794,160 @@ def test_extra_attribute_already_exists(ldap_conn, extra_attributes):
                                   user, domain, extra_attribute)
 
     assert val == given_name
+
+
+@pytest.fixture
+def add_user_to_group(request, ldap_conn):
+    """
+    Adding user to group
+    """
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+    ent_list.add_group_bis("group1", 20001, member_uids=["user1"])
+    create_ldap_fixture(request, ldap_conn, ent_list)
+    create_conf_fixture(request,
+                        format_rfc2307bis_deref_conf(
+                            ldap_conn,
+                            SCHEMA_RFC2307_BIS))
+    create_sssd_fixture(request)
+    return None
+
+
+def test_add_user_to_group(ldap_conn, add_user_to_group):
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=2001))
+    ent.assert_group_by_name("group1", dict(mem=ent.contains_only("user1")))
+
+
+@pytest.fixture
+def remove_user_from_group(request, ldap_conn):
+    """
+    Adding user to group
+    """
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+    ent_list.add_user("user2", 1002, 2002)
+    ent_list.add_group_bis("group1", 20001, member_uids=["user1", "user2"])
+    create_ldap_fixture(request, ldap_conn, ent_list)
+    create_conf_fixture(request,
+                        format_rfc2307bis_deref_conf(
+                            ldap_conn,
+                            SCHEMA_RFC2307_BIS))
+    create_sssd_fixture(request)
+    return None
+
+
+def test_remove_user_from_group(ldap_conn, remove_user_from_group):
+    """
+    Removing two users from group, step by step
+    """
+    group1_dn = 'cn=group1,ou=Groups,' + ldap_conn.ds_inst.base_dn
+
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=2001))
+    ent.assert_passwd_by_name("user2", dict(name="user2", uid=1002, gid=2002))
+    ent.assert_group_by_name("group1",
+                             dict(mem=ent.contains_only("user1", "user2")))
+
+    # removing of user2 from group1
+    old = {'member': ["uid=user1,ou=Users,dc=example,dc=com",
+                      "uid=user2,ou=Users,dc=example,dc=com"]}
+    new = {'member': ["uid=user1,ou=Users,dc=example,dc=com"]}
+
+    ldif = ldap.modlist.modifyModlist(old, new)
+    ldap_conn.modify_s(group1_dn, ldif)
+
+    if subprocess.call(["sss_cache", "-GU"]) != 0:
+        raise Exception("sssd_cache failed")
+
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=2001))
+    ent.assert_passwd_by_name("user2", dict(name="user2", uid=1002, gid=2002))
+    ent.assert_group_by_name("group1", dict(mem=ent.contains_only("user1")))
+
+    # removing of user1 from group1
+    old = {'member': ["uid=user1,ou=Users,dc=example,dc=com"]}
+    new = {'member': []}
+
+    ldif = ldap.modlist.modifyModlist(old, new)
+    ldap_conn.modify_s(group1_dn, ldif)
+
+    if subprocess.call(["sss_cache", "-GU"]) != 0:
+        raise Exception("sssd_cache failed")
+
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=2001))
+    ent.assert_passwd_by_name("user2", dict(name="user2", uid=1002, gid=2002))
+    ent.assert_group_by_name("group1", dict(mem=ent.contains_only()))
+
+
+@pytest.fixture
+def remove_user_from_nested_group(request, ldap_conn):
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+    ent_list.add_user("user2", 1002, 2002)
+    ent_list.add_group_bis("group1", 20001, member_uids=["user1"])
+    ent_list.add_group_bis("group2", 20002, member_uids=["user2"])
+    ent_list.add_group_bis("group3", 20003, member_gids=["group1", "group2"])
+    create_ldap_fixture(request, ldap_conn, ent_list)
+    create_conf_fixture(request,
+                        format_rfc2307bis_deref_conf(
+                            ldap_conn,
+                            SCHEMA_RFC2307_BIS))
+    create_sssd_fixture(request)
+    return None
+
+
+def test_remove_user_from_nested_group(ldap_conn,
+                                       remove_user_from_nested_group):
+
+    group3_dn = 'cn=group3,ou=Groups,' + ldap_conn.ds_inst.base_dn
+
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=2001))
+    ent.assert_passwd_by_name("user2", dict(name="user2", uid=1002, gid=2002))
+
+    ent.assert_group_by_name("group1",
+                             dict(mem=ent.contains_only("user1")))
+    ent.assert_group_by_name("group2",
+                             dict(mem=ent.contains_only("user2")))
+
+    ent.assert_group_by_name("group3",
+                             dict(mem=ent.contains_only("user1",
+                                                        "user2")))
+
+    # removing of group2 from group3
+    old = {'member': ["cn=group1,ou=Groups,dc=example,dc=com",
+                      "cn=group2,ou=Groups,dc=example,dc=com"]}
+    new = {'member': ["cn=group1,ou=Groups,dc=example,dc=com"]}
+
+    ldif = ldap.modlist.modifyModlist(old, new)
+    ldap_conn.modify_s(group3_dn, ldif)
+
+    if subprocess.call(["sss_cache", "-GU"]) != 0:
+        raise Exception("sssd_cache failed")
+
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=2001))
+    ent.assert_passwd_by_name("user2", dict(name="user2", uid=1002, gid=2002))
+
+    ent.assert_group_by_name("group1",
+                             dict(mem=ent.contains_only("user1")))
+    ent.assert_group_by_name("group2",
+                             dict(mem=ent.contains_only("user2")))
+    ent.assert_group_by_name("group3",
+                             dict(mem=ent.contains_only("user1")))
+
+    # removing of group1 from group3
+    old = {'member': ["cn=group1,ou=Groups,dc=example,dc=com"]}
+    new = {'member': []}
+
+    ldif = ldap.modlist.modifyModlist(old, new)
+    ldap_conn.modify_s(group3_dn, ldif)
+
+    if subprocess.call(["sss_cache", "-GU"]) != 0:
+        raise Exception("sssd_cache failed")
+
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=2001))
+    ent.assert_passwd_by_name("user2", dict(name="user2", uid=1002, gid=2002))
+
+    ent.assert_group_by_name("group1",
+                             dict(mem=ent.contains_only("user1")))
+    ent.assert_group_by_name("group2",
+                             dict(mem=ent.contains_only("user2")))
+    ent.assert_group_by_name("group3",
+                             dict(mem=ent.contains_only()))
-- 
2.7.4

_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to