URL: https://github.com/SSSD/sssd/pull/128
Author: fidencio
 Title: #128: Fix group renaming issue when "id_provider = ldap" is set
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/128/head:pr128
git checkout pr128
From cbccc1a1b89e996e375d59be445a09d1eb1a36ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Fri, 16 Feb 2018 13:55:53 +0100
Subject: [PATCH 01/15] NSS: Add InvalidateGroupById handler
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There are some situations where, from the backend, the NSS responder
will have to be notified to invalidate a group.

In order to achieve this in a clean way, let's add the
InvalidateGroupById handler and make use of it later in this very same
series.

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/responder/nss/nss_iface.c           | 16 ++++++++++++++
 src/responder/nss/nss_iface.xml         |  3 +++
 src/responder/nss/nss_iface_generated.c | 38 +++++++++++++++++++++++++++++++++
 src/responder/nss/nss_iface_generated.h |  5 +++++
 4 files changed, 62 insertions(+)

diff --git a/src/responder/nss/nss_iface.c b/src/responder/nss/nss_iface.c
index 415af9550..805e4fcdf 100644
--- a/src/responder/nss/nss_iface.c
+++ b/src/responder/nss/nss_iface.c
@@ -199,12 +199,28 @@ int nss_memorycache_update_initgroups(struct sbus_request *sbus_req,
     return iface_nss_memorycache_UpdateInitgroups_finish(sbus_req);
 }
 
+int nss_memorycache_invalidate_group_by_id(struct sbus_request *sbus_req,
+                                           void *data,
+                                           gid_t gid)
+{
+    struct resp_ctx *rctx = talloc_get_type(data, struct resp_ctx);
+    struct nss_ctx *nctx = talloc_get_type(rctx->pvt_ctx, struct nss_ctx);
+
+    DEBUG(SSSDBG_TRACE_LIBS,
+          "Invalidating group %"PRIu32" from memory cache\n", gid);
+
+    sss_mmap_cache_gr_invalidate_gid(nctx->grp_mc_ctx, gid);
+
+    return iface_nss_memorycache_InvalidateGroupById_finish(sbus_req);
+}
+
 struct iface_nss_memorycache iface_nss_memorycache = {
     { &iface_nss_memorycache_meta, 0 },
     .UpdateInitgroups = nss_memorycache_update_initgroups,
     .InvalidateAllUsers = nss_memorycache_invalidate_users,
     .InvalidateAllGroups = nss_memorycache_invalidate_groups,
     .InvalidateAllInitgroups = nss_memorycache_invalidate_initgroups,
+    .InvalidateGroupById = nss_memorycache_invalidate_group_by_id,
 };
 
 static struct sbus_iface_map iface_map[] = {
diff --git a/src/responder/nss/nss_iface.xml b/src/responder/nss/nss_iface.xml
index 27aae0197..4d8cf14f9 100644
--- a/src/responder/nss/nss_iface.xml
+++ b/src/responder/nss/nss_iface.xml
@@ -14,5 +14,8 @@
         </method>
         <method name="InvalidateAllInitgroups">
         </method>
+        <method name="InvalidateGroupById">
+            <arg name="gid" type="u" direction="in" />
+        </method>
     </interface>
 </node>
diff --git a/src/responder/nss/nss_iface_generated.c b/src/responder/nss/nss_iface_generated.c
index 4a8b704da..8d5a4584b 100644
--- a/src/responder/nss/nss_iface_generated.c
+++ b/src/responder/nss/nss_iface_generated.c
@@ -12,6 +12,9 @@
 /* invokes a handler with a 'ssau' DBus signature */
 static int invoke_ssau_method(struct sbus_request *dbus_req, void *function_ptr);
 
+/* invokes a handler with a 'u' DBus signature */
+static int invoke_u_method(struct sbus_request *dbus_req, void *function_ptr);
+
 /* arguments for org.freedesktop.sssd.nss.MemoryCache.UpdateInitgroups */
 const struct sbus_arg_meta iface_nss_memorycache_UpdateInitgroups__in[] = {
     { "user", "s" },
@@ -44,6 +47,18 @@ int iface_nss_memorycache_InvalidateAllInitgroups_finish(struct sbus_request *re
                                          DBUS_TYPE_INVALID);
 }
 
+/* arguments for org.freedesktop.sssd.nss.MemoryCache.InvalidateGroupById */
+const struct sbus_arg_meta iface_nss_memorycache_InvalidateGroupById__in[] = {
+    { "gid", "u" },
+    { NULL, }
+};
+
+int iface_nss_memorycache_InvalidateGroupById_finish(struct sbus_request *req)
+{
+   return sbus_request_return_and_finish(req,
+                                         DBUS_TYPE_INVALID);
+}
+
 /* methods for org.freedesktop.sssd.nss.MemoryCache */
 const struct sbus_method_meta iface_nss_memorycache__methods[] = {
     {
@@ -74,6 +89,13 @@ const struct sbus_method_meta iface_nss_memorycache__methods[] = {
         offsetof(struct iface_nss_memorycache, InvalidateAllInitgroups),
         NULL, /* no invoker */
     },
+    {
+        "InvalidateGroupById", /* name */
+        iface_nss_memorycache_InvalidateGroupById__in,
+        NULL, /* no out_args */
+        offsetof(struct iface_nss_memorycache, InvalidateGroupById),
+        invoke_u_method,
+    },
     { NULL, }
 };
 
@@ -86,6 +108,22 @@ const struct sbus_interface_meta iface_nss_memorycache_meta = {
     sbus_invoke_get_all, /* GetAll invoker */
 };
 
+/* invokes a handler with a 'u' DBus signature */
+static int invoke_u_method(struct sbus_request *dbus_req, void *function_ptr)
+{
+    uint32_t arg_0;
+    int (*handler)(struct sbus_request *, void *, uint32_t) = function_ptr;
+
+    if (!sbus_request_parse_or_finish(dbus_req,
+                               DBUS_TYPE_UINT32, &arg_0,
+                               DBUS_TYPE_INVALID)) {
+         return EOK; /* request handled */
+    }
+
+    return (handler)(dbus_req, dbus_req->intf->handler_data,
+                     arg_0);
+}
+
 /* invokes a handler with a 'ssau' DBus signature */
 static int invoke_ssau_method(struct sbus_request *dbus_req, void *function_ptr)
 {
diff --git a/src/responder/nss/nss_iface_generated.h b/src/responder/nss/nss_iface_generated.h
index 11fac7916..27a6d0853 100644
--- a/src/responder/nss/nss_iface_generated.h
+++ b/src/responder/nss/nss_iface_generated.h
@@ -18,6 +18,7 @@
 #define IFACE_NSS_MEMORYCACHE_INVALIDATEALLUSERS "InvalidateAllUsers"
 #define IFACE_NSS_MEMORYCACHE_INVALIDATEALLGROUPS "InvalidateAllGroups"
 #define IFACE_NSS_MEMORYCACHE_INVALIDATEALLINITGROUPS "InvalidateAllInitgroups"
+#define IFACE_NSS_MEMORYCACHE_INVALIDATEGROUPBYID "InvalidateGroupById"
 
 /* ------------------------------------------------------------------------
  * DBus handlers
@@ -44,6 +45,7 @@ struct iface_nss_memorycache {
     int (*InvalidateAllUsers)(struct sbus_request *req, void *data);
     int (*InvalidateAllGroups)(struct sbus_request *req, void *data);
     int (*InvalidateAllInitgroups)(struct sbus_request *req, void *data);
+    int (*InvalidateGroupById)(struct sbus_request *req, void *data, uint32_t arg_gid);
 };
 
 /* finish function for UpdateInitgroups */
@@ -58,6 +60,9 @@ int iface_nss_memorycache_InvalidateAllGroups_finish(struct sbus_request *req);
 /* finish function for InvalidateAllInitgroups */
 int iface_nss_memorycache_InvalidateAllInitgroups_finish(struct sbus_request *req);
 
+/* finish function for InvalidateGroupById */
+int iface_nss_memorycache_InvalidateGroupById_finish(struct sbus_request *req);
+
 /* ------------------------------------------------------------------------
  * DBus Interface Metadata
  *

From d535e1ab5f065febc31eb3fd029083734fdbe529 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 08:42:10 +0100
Subject: [PATCH 02/15] DP: Add dp_sbus_invalidate_group_memcache()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This function will be called from the data provider to the NSS
responder, which will invalidate a group in the memcache.

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/data_provider/dp.h             |  2 ++
 src/providers/data_provider/dp_resp_client.c | 40 ++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/src/providers/data_provider/dp.h b/src/providers/data_provider/dp.h
index ceb49da53..e8b2f9c8f 100644
--- a/src/providers/data_provider/dp.h
+++ b/src/providers/data_provider/dp.h
@@ -179,6 +179,8 @@ void dp_sbus_reset_groups_ncache(struct data_provider *provider,
 void dp_sbus_reset_users_memcache(struct data_provider *provider);
 void dp_sbus_reset_groups_memcache(struct data_provider *provider);
 void dp_sbus_reset_initgr_memcache(struct data_provider *provider);
+void dp_sbus_invalidate_group_memcache(struct data_provider *provider,
+                                       gid_t gid);
 
 /*
  * A dummy handler for DPM_ACCT_DOMAIN_HANDLER.
diff --git a/src/providers/data_provider/dp_resp_client.c b/src/providers/data_provider/dp_resp_client.c
index 5735188a6..d48af3288 100644
--- a/src/providers/data_provider/dp_resp_client.c
+++ b/src/providers/data_provider/dp_resp_client.c
@@ -189,3 +189,43 @@ void dp_sbus_reset_initgr_memcache(struct data_provider *provider)
     return dp_sbus_reset_memcache(provider,
                           IFACE_NSS_MEMORYCACHE_INVALIDATEALLINITGROUPS);
 }
+
+void dp_sbus_invalidate_group_memcache(struct data_provider *provider,
+                                       gid_t gid)
+{
+    struct dp_client *dp_cli;
+    DBusMessage *msg;
+    dbus_bool_t dbret;
+
+    dp_cli = provider->clients[DPC_NSS];
+    if (dp_cli == NULL) {
+        return;
+    }
+
+    msg = dbus_message_new_method_call(NULL,
+                                       NSS_MEMORYCACHE_PATH,
+                                       IFACE_NSS_MEMORYCACHE,
+                                       IFACE_NSS_MEMORYCACHE_INVALIDATEGROUPBYID);
+    if (msg == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory?!\n");
+        return;
+    }
+
+    dbret = dbus_message_append_args(msg,
+                                     DBUS_TYPE_UINT32, &gid,
+                                     DBUS_TYPE_INVALID);
+    if (!dbret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory?!\n");
+        dbus_message_unref(msg);
+        return;
+    }
+
+    DEBUG(SSSDBG_TRACE_FUNC,
+          "Ordering NSS responder to invalidate the group %"PRIu32" \n",
+          gid);
+
+    sbus_conn_send_reply(dp_client_conn(dp_cli), msg);
+    dbus_message_unref(msg);
+
+    return;
+}

From 978adff2901700f6f315cb459ee9537fdea840bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 10:00:42 +0100
Subject: [PATCH 03/15] SDAP: Pass struct data_provider to
 sdap_get_groups_send()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is needed in order to properly treat group id-collision when adding
an incomplete group.

The struct data_provider had to be passed to this function and also
added to the struct sdap_get_groups_state as it'll have to be passed
down to sdap_add_incomplete_groups().

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/ldap_id.c           | 3 ++-
 src/providers/ldap/sdap_async.h        | 3 ++-
 src/providers/ldap/sdap_async_enum.c   | 3 ++-
 src/providers/ldap/sdap_async_groups.c | 9 ++++++++-
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index 3bd68780a..bb6cd5c26 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -1036,7 +1036,8 @@ static void groups_get_search(struct tevent_req *req)
                                   dp_opt_get_int(state->ctx->opts->basic,
                                                  SDAP_SEARCH_TIMEOUT),
                                   lookup_type,
-                                  state->no_members);
+                                  state->no_members,
+                                  state->ctx->be->provider);
     if (!subreq) {
         tevent_req_error(req, ENOMEM);
         return;
diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
index 40da81fb9..d736f8bd3 100644
--- a/src/providers/ldap/sdap_async.h
+++ b/src/providers/ldap/sdap_async.h
@@ -104,7 +104,8 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx,
                                        const char *filter,
                                        int timeout,
                                        enum sdap_entry_lookup_type lookup_type,
-                                       bool no_members);
+                                       bool no_members,
+                                       void *provider);
 int sdap_get_groups_recv(struct tevent_req *req,
                          TALLOC_CTX *mem_ctx, char **timestamp);
 
diff --git a/src/providers/ldap/sdap_async_enum.c b/src/providers/ldap/sdap_async_enum.c
index ea9d51adc..6623e6753 100644
--- a/src/providers/ldap/sdap_async_enum.c
+++ b/src/providers/ldap/sdap_async_enum.c
@@ -814,7 +814,8 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX *memctx,
                                   state->attrs, state->filter,
                                   dp_opt_get_int(state->ctx->opts->basic,
                                                  SDAP_ENUM_SEARCH_TIMEOUT),
-                                  SDAP_LOOKUP_ENUMERATE, false);
+                                  SDAP_LOOKUP_ENUMERATE, false,
+                                  state->ctx->be->provider);
     if (!subreq) {
         ret = ENOMEM;
         goto fail;
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 77acded7a..1c7b2c610 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -1817,6 +1817,11 @@ struct sdap_get_groups_state {
 
     struct sdap_handle *ldap_sh;
     struct sdap_id_op *op;
+
+    /* Provider will be used to send a d-bus message to NSS responder in case
+     * group id collision has been detected. In this case we'd have to also
+     * invalidate the group in the memcache. */
+    void *provider;
 };
 
 static errno_t sdap_get_groups_next_base(struct tevent_req *req);
@@ -1833,7 +1838,8 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx,
                                        const char *filter,
                                        int timeout,
                                        enum sdap_entry_lookup_type lookup_type,
-                                       bool no_members)
+                                       bool no_members,
+                                       void *provider)
 {
     errno_t ret;
     struct tevent_req *req;
@@ -1860,6 +1866,7 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx,
     state->base_filter = filter;
     state->base_iter = 0;
     state->search_bases = sdom->group_search_bases;
+    state->provider = provider;
 
     if (!state->search_bases) {
         DEBUG(SSSDBG_CRIT_FAILURE,

From 5982085740f9b9341af03f73e26b8d385be2bce4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 10:16:20 +0100
Subject: [PATCH 04/15] SDAP: Pass struct data_provider to
 sdap_initgr_rfc2307_send()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is needed in order to properly treat group id-collision when adding
an incomplete group.

The struct data_provider had to be passed to this function and also
added to the struct sdap_initgr_rfc2307_state as it'll have to be passed
down to sdap_initgr_common_store(), which will pass it down to
sdap_add_incomplete_groups().

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/sdap_async_initgroups.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index 326294a1c..99f345074 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -388,6 +388,11 @@ struct sdap_initgr_rfc2307_state {
 
     size_t base_iter;
     struct sdap_search_base **search_bases;
+
+    /* Provider will be used to send a d-bus message to NSS responder in case
+     * group id collision has been detected. In this case we'd have to also
+     * invalidate the group in the memcache. */
+    void *provider;
 };
 
 static errno_t sdap_initgr_rfc2307_next_base(struct tevent_req *req);
@@ -398,7 +403,8 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx,
                                             struct sysdb_ctx *sysdb,
                                             struct sss_domain_info *domain,
                                             struct sdap_handle *sh,
-                                            const char *name)
+                                            const char *name,
+                                            void *provider)
 {
     struct tevent_req *req;
     struct sdap_initgr_rfc2307_state *state;
@@ -422,6 +428,7 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx,
     state->ldap_groups_count = 0;
     state->base_iter = 0;
     state->search_bases = opts->sdom->group_search_bases;
+    state->provider = provider;
 
     if (!state->search_bases) {
         DEBUG(SSSDBG_CRIT_FAILURE,
@@ -3041,7 +3048,8 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
     case SDAP_SCHEMA_RFC2307:
         subreq = sdap_initgr_rfc2307_send(state, state->ev, state->opts,
                                           state->sysdb, state->dom, state->sh,
-                                          cname);
+                                          cname,
+                                          state->id_ctx->be->provider);
         if (!subreq) {
             tevent_req_error(req, ENOMEM);
             return;

From f5db767210be77895b04cd3788782d0951191ef7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 10:41:09 +0100
Subject: [PATCH 05/15] SDAP: Pass struct data_provider to
 sdap_initgr_rfc2307bis_send()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is needed in order to properly treat group id-collision when adding
an incomplete group.

The struct data_provider had to be passed to this function and also
added to the struct sdap_initgr_rfc2307bis_state as it'll have to be
passed down to sdap_initgr_rfc2307bis_send(), which will pass it down
to sdap_initgr_common_store(), which will pass it down to
sdap_add_incomplete_groups().

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/sdap_async_initgroups.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index 99f345074..4593e8ca9 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -1567,6 +1567,11 @@ struct sdap_initgr_rfc2307bis_state {
     hash_table_t *group_hash;
     size_t num_direct_parents;
     struct sysdb_attrs **direct_groups;
+
+    /* Provider will be used to send a d-bus message to NSS responder in case
+     * group id collision has been detected. In this case we'd have to also
+     * invalidate the group in the memcache. */
+    void *provider;
 };
 
 struct sdap_nested_group {
@@ -1588,7 +1593,8 @@ static struct tevent_req *sdap_initgr_rfc2307bis_send(
         struct sdap_domain *sdom,
         struct sdap_handle *sh,
         const char *name,
-        const char *orig_dn)
+        const char *orig_dn,
+        void *provider)
 {
     errno_t ret;
     struct tevent_req *req;
@@ -1614,6 +1620,7 @@ static struct tevent_req *sdap_initgr_rfc2307bis_send(
     state->base_iter = 0;
     state->search_bases = sdom->group_search_bases;
     state->orig_dn = orig_dn;
+    state->provider = provider;
 
     if (!state->search_bases) {
         DEBUG(SSSDBG_CRIT_FAILURE,
@@ -3099,7 +3106,8 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
             subreq = sdap_initgr_rfc2307bis_send(
                     state, state->ev, state->opts,
                     state->sdom, state->sh,
-                    cname, orig_dn);
+                    cname, orig_dn,
+                    state->id_ctx->be->provider);
         }
         if (!subreq) {
             tevent_req_error(req, ENOMEM);

From 50a9626ab06d6d4d2c6500272d9e81f5713a4795 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 12:01:54 +0100
Subject: [PATCH 06/15] SDAP: Pass struct data_provider to
 sdap_get_ad_match_rule_initgroups_send()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is needed in order to properly treat group id-collision when adding
an incomplete group.

The struct data_provider had to be passed to this function and also
added to the struct sdap_ad_match_rule_initgr_state as it'll have to be
passed down to sdap_initgr_common_store(), which will pass it down to
which will pass it down to sdap_add_incomplete_groups().

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/sdap_async.h               |  3 ++-
 src/providers/ldap/sdap_async_initgroups.c    | 16 +++++++++-------
 src/providers/ldap/sdap_async_initgroups_ad.c |  9 ++++++++-
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
index d736f8bd3..ece0c5570 100644
--- a/src/providers/ldap/sdap_async.h
+++ b/src/providers/ldap/sdap_async.h
@@ -390,7 +390,8 @@ sdap_get_ad_match_rule_initgroups_send(TALLOC_CTX *mem_ctx,
                                        struct sdap_handle *sh,
                                        const char *name,
                                        const char *orig_dn,
-                                       int timeout);
+                                       int timeout,
+                                       void *provider);
 
 errno_t
 sdap_get_ad_match_rule_initgroups_recv(struct tevent_req *req);
diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index 4593e8ca9..bad348dc8 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -3095,13 +3095,15 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
             /* Take advantage of AD's extensibleMatch filter to look up
              * all parent groups in a single request.
              */
-            subreq = sdap_get_ad_match_rule_initgroups_send(state, state->ev,
-                                                            state->opts,
-                                                            state->sysdb,
-                                                            state->dom,
-                                                            state->sh,
-                                                            cname, orig_dn,
-                                                            state->timeout);
+            subreq = sdap_get_ad_match_rule_initgroups_send(
+                                                state, state->ev,
+                                                state->opts,
+                                                state->sysdb,
+                                                state->dom,
+                                                state->sh,
+                                                cname, orig_dn,
+                                                state->timeout,
+                                                state->id_ctx->be->provider);
         } else {
             subreq = sdap_initgr_rfc2307bis_send(
                     state, state->ev, state->opts,
diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c
index 30f1d3db2..66d9c136c 100644
--- a/src/providers/ldap/sdap_async_initgroups_ad.c
+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
@@ -47,6 +47,11 @@ struct sdap_ad_match_rule_initgr_state {
 
     size_t base_iter;
     struct sdap_search_base **search_bases;
+
+    /* Provider will be used to send a d-bus message to NSS responder in case
+     * group id collision has been detected. In this case we'd have to also
+     * invalidate the group in the memcache. */
+    void *provider;
 };
 
 static errno_t
@@ -64,7 +69,8 @@ sdap_get_ad_match_rule_initgroups_send(TALLOC_CTX *mem_ctx,
                                        struct sdap_handle *sh,
                                        const char *name,
                                        const char *orig_dn,
-                                       int timeout)
+                                       int timeout,
+                                       void *provider)
 {
     errno_t ret;
     struct tevent_req *req;
@@ -86,6 +92,7 @@ sdap_get_ad_match_rule_initgroups_send(TALLOC_CTX *mem_ctx,
     state->orig_dn = orig_dn;
     state->base_iter = 0;
     state->search_bases = opts->sdom->group_search_bases;
+    state->provider = provider;
 
     /* Request all of the group attributes that we know
      * about, except for 'member' because that wastes a

From aa1e4637daec22716671fc5a67802b3539f4c0bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 10:19:09 +0100
Subject: [PATCH 07/15] SDAP: Pass struct data_provider to
 sdap_initgr_common_store()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is needed in order to properly treat group id-collision when adding
an incomplete group.

The struct data_provider had to be passed to this function as it'll have
to be passed down to sdap_add_incomplete_groups().

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/sdap_async_initgroups.c    | 6 ++++--
 src/providers/ldap/sdap_async_initgroups_ad.c | 3 ++-
 src/providers/ldap/sdap_async_private.h       | 3 ++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index bad348dc8..cd05ac9ba 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -266,7 +266,8 @@ int sdap_initgr_common_store(struct sysdb_ctx *sysdb,
                              enum sysdb_member_type type,
                              char **sysdb_grouplist,
                              struct sysdb_attrs **ldap_groups,
-                             int ldap_groups_count)
+                             int ldap_groups_count,
+                             void *provider)
 {
     TALLOC_CTX *tmp_ctx;
     char **ldap_grouplist = NULL;
@@ -627,7 +628,8 @@ static void sdap_initgr_rfc2307_process(struct tevent_req *subreq)
                                    SYSDB_MEMBER_USER,
                                    sysdb_grouplist,
                                    state->ldap_groups,
-                                   state->ldap_groups_count);
+                                   state->ldap_groups_count,
+                                   state->provider);
     if (ret != EOK) {
         tevent_req_error(req, ret);
         return;
diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c
index 66d9c136c..899b17cce 100644
--- a/src/providers/ldap/sdap_async_initgroups_ad.c
+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
@@ -291,7 +291,8 @@ sdap_get_ad_match_rule_initgroups_step(struct tevent_req *subreq)
                                    SYSDB_MEMBER_USER,
                                    sysdb_grouplist,
                                    state->groups,
-                                   state->count);
+                                   state->count,
+                                   state->provider);
     if (ret != EOK) {
         DEBUG(SSSDBG_MINOR_FAILURE,
               "Could not store groups for user [%s]: [%s]\n",
diff --git a/src/providers/ldap/sdap_async_private.h b/src/providers/ldap/sdap_async_private.h
index 72507442a..2476ca7b8 100644
--- a/src/providers/ldap/sdap_async_private.h
+++ b/src/providers/ldap/sdap_async_private.h
@@ -104,7 +104,8 @@ int sdap_initgr_common_store(struct sysdb_ctx *sysdb,
                              enum sysdb_member_type type,
                              char **sysdb_grouplist,
                              struct sysdb_attrs **ldap_groups,
-                             int ldap_groups_count);
+                             int ldap_groups_count,
+                             void *provider);
 
 errno_t get_sysdb_grouplist(TALLOC_CTX *mem_ctx,
                             struct sysdb_ctx *sysdb,

From f42dcbc2206f4a91c6d8b3b141a0cdd1c6bcdebb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 12:12:58 +0100
Subject: [PATCH 08/15] SDAP: Pass struct data_provider to
 sdap_ad_get_domain_local_groups_parse_parent()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is needed in order to properly treat group id-collision when adding
an incomplete group.

The struct data_provider had to be passed to this function as it'll have
to be sdap_nested_groups_store(), which will pass it down to
sdap_add_incomplete_groups().

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/sdap_async_initgroups_ad.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c
index 899b17cce..9e770de16 100644
--- a/src/providers/ldap/sdap_async_initgroups_ad.c
+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
@@ -1549,6 +1549,7 @@ sdap_ad_get_domain_local_groups_parse_parents(TALLOC_CTX *mem_ctx,
                                               struct sss_domain_info *dom,
                                               struct sysdb_ctx *sysdb,
                                               struct sdap_options *opts,
+                                              void *provider,
                                               const char **_sysdb_name,
                                               enum sysdb_member_type *_type,
                                               char ***_add_list,
@@ -1778,14 +1779,16 @@ static void sdap_ad_get_domain_local_groups_done(struct tevent_req *subreq)
          * nested parents found during the request. The nested parents contain
          * the processed LDAP data and can be identified by a missing
          * objectclass attribute. */
-        ret = sdap_ad_get_domain_local_groups_parse_parents(state, gr,
-                                                            state->dom,
-                                                            state->sysdb,
-                                                            state->opts,
-                                                            &sysdb_name,
-                                                            &type,
-                                                            &add_list,
-                                                            &del_list);
+        ret = sdap_ad_get_domain_local_groups_parse_parents(
+                                            state, gr,
+                                            state->dom,
+                                            state->sysdb,
+                                            state->opts,
+                                            state->conn->id_ctx->be->provider,
+                                            &sysdb_name,
+                                            &type,
+                                            &add_list,
+                                            &del_list);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE,
                   "sdap_ad_get_domain_local_groups_parse_parents failed.\n");

From f87c3fdf744d4fa13c8be584f6af17574b50eb2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 12:16:39 +0100
Subject: [PATCH 09/15] SDAP: Pass struct data_provider to
 sdap_nested_groups_store()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is needed in order to properly treat group id-collision when adding
an incomplete group.

The struct data_provider had to be passed to this function and also
added to the struct sdap_initgr_nested_state as it'll have to be passed
down to sdap_add_incomplete_groups().

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/sdap_async_initgroups.c    | 13 ++++++++++---
 src/providers/ldap/sdap_async_initgroups_ad.c |  5 +++--
 src/providers/ldap/sdap_async_private.h       |  3 ++-
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index cd05ac9ba..1a9f89b2c 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -651,7 +651,8 @@ sdap_nested_groups_store(struct sysdb_ctx *sysdb,
                          struct sss_domain_info *domain,
                          struct sdap_options *opts,
                          struct sysdb_attrs **groups,
-                         unsigned long count)
+                         unsigned long count,
+                         void *provider)
 {
     errno_t ret, tret;
     TALLOC_CTX *tmp_ctx;
@@ -790,6 +791,11 @@ struct sdap_initgr_nested_state {
 
     struct sysdb_attrs **groups;
     int groups_cur;
+
+    /* Provider will be used to send a d-bus message to NSS responder in case
+     * group id collision has been detected. In this case we'd have to also
+     * invalidate the group in the memcache. */
+    void *provider;
 };
 
 static errno_t sdap_initgr_nested_deref_search(struct tevent_req *req);
@@ -1145,7 +1151,8 @@ sdap_initgr_store_groups(struct sdap_initgr_nested_state *state)
 {
     return sdap_nested_groups_store(state->sysdb, state->dom,
                                     state->opts, state->groups,
-                                    state->groups_cur);
+                                    state->groups_cur,
+                                    state->provider);
 }
 
 static errno_t
@@ -1952,7 +1959,7 @@ save_rfc2307bis_groups(struct sdap_initgr_rfc2307bis_state *state)
     talloc_zfree(values);
 
     ret = sdap_nested_groups_store(state->sysdb, state->dom, state->opts,
-                                   groups, count);
+                                   groups, count, state->provider);
     if (ret != EOK) {
         DEBUG(SSSDBG_MINOR_FAILURE, "Could not save groups [%d]: %s\n",
                   ret, strerror(ret));
diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c
index 9e770de16..4947fafaa 100644
--- a/src/providers/ldap/sdap_async_initgroups_ad.c
+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
@@ -1587,7 +1587,8 @@ sdap_ad_get_domain_local_groups_parse_parents(TALLOC_CTX *mem_ctx,
     if (gr->parents_count != 0) {
         /* Store the parents if needed */
         ret = sdap_nested_groups_store(sysdb, dom, opts,
-                                       gr->ldap_parents, gr->parents_count);
+                                       gr->ldap_parents, gr->parents_count,
+                                       provider);
         if (ret != EOK) {
             DEBUG(SSSDBG_MINOR_FAILURE, "Could not save groups [%d]: %s\n",
                       ret, strerror(ret));
@@ -1623,7 +1624,7 @@ sdap_ad_get_domain_local_groups_parse_parents(TALLOC_CTX *mem_ctx,
 
         /* make sure group exists in cache */
         groups[0]= gr->group;
-        ret = sdap_nested_groups_store(sysdb, dom, opts, groups, 1);
+        ret = sdap_nested_groups_store(sysdb, dom, opts, groups, 1, provider);
         if (ret != EOK) {
             DEBUG(SSSDBG_MINOR_FAILURE, "Could not save groups [%d]: %s\n",
                       ret, strerror(ret));
diff --git a/src/providers/ldap/sdap_async_private.h b/src/providers/ldap/sdap_async_private.h
index 2476ca7b8..53c0135ce 100644
--- a/src/providers/ldap/sdap_async_private.h
+++ b/src/providers/ldap/sdap_async_private.h
@@ -173,7 +173,8 @@ errno_t sdap_nested_groups_store(struct sysdb_ctx *sysdb,
                                  struct sss_domain_info *domain,
                                  struct sdap_options *opts,
                                  struct sysdb_attrs **groups,
-                                 unsigned long count);
+                                 unsigned long count,
+                                 void *provider);
 
 struct tevent_req *
 sdap_ad_get_domain_local_groups_send(TALLOC_CTX *mem_ctx,

From 8acea5e79485e0501a79824b43af36ccb89cca2d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 09:44:14 +0100
Subject: [PATCH 10/15] SDAP: Pass struct data_provider to
 sdap_add_incomplete_groups()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is needed in order to properly treat group id-collision when
adding an incomplete group.

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/sdap_async_groups.c     | 2 +-
 src/providers/ldap/sdap_async_initgroups.c | 7 ++++---
 src/providers/ldap/sdap_async_private.h    | 3 ++-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 1c7b2c610..a69738747 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -2076,7 +2076,7 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
 
         ret = sdap_add_incomplete_groups(state->sysdb, state->dom, state->opts,
                                          sysdb_groupnamelist, state->groups,
-                                         state->count);
+                                         state->count, state->provider);
         if (ret == EOK) {
             DEBUG(SSSDBG_TRACE_LIBS,
                   "Writing only group data without members was successful.\n");
diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index 1a9f89b2c..cfb0e6728 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -35,7 +35,8 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
                                    struct sdap_options *opts,
                                    char **sysdb_groupnames,
                                    struct sysdb_attrs **ldap_groups,
-                                   int ldap_groups_count)
+                                   int ldap_groups_count,
+                                   void *provider)
 {
     TALLOC_CTX *tmp_ctx;
     struct ldb_message *msg;
@@ -331,7 +332,7 @@ int sdap_initgr_common_store(struct sysdb_ctx *sysdb,
     if (add_groups && add_groups[0]) {
         ret = sdap_add_incomplete_groups(sysdb, domain, opts,
                                          add_groups, ldap_groups,
-                                         ldap_groups_count);
+                                         ldap_groups_count, provider);
         if (ret != EOK) {
             DEBUG(SSSDBG_CRIT_FAILURE, "Adding incomplete users failed\n");
             goto done;
@@ -683,7 +684,7 @@ sdap_nested_groups_store(struct sysdb_ctx *sysdb,
     in_transaction = true;
 
     ret = sdap_add_incomplete_groups(sysdb, domain, opts, groupnamelist,
-                                     groups, count);
+                                     groups, count, provider);
     if (ret != EOK) {
         DEBUG(SSSDBG_TRACE_FUNC, "Could not add incomplete groups [%d]: %s\n",
                    ret, strerror(ret));
diff --git a/src/providers/ldap/sdap_async_private.h b/src/providers/ldap/sdap_async_private.h
index 53c0135ce..b53d08ea7 100644
--- a/src/providers/ldap/sdap_async_private.h
+++ b/src/providers/ldap/sdap_async_private.h
@@ -151,7 +151,8 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
                                    struct sdap_options *opts,
                                    char **sysdb_groupnames,
                                    struct sysdb_attrs **ldap_groups,
-                                   int ldap_groups_count);
+                                   int ldap_groups_count,
+                                   void *provider);
 
 /* from sdap_ad_groups.c */
 errno_t sdap_check_ad_group_type(struct sss_domain_info *dom,

From a5a2fa16c7a79f85c675af6eceef0dced6432027 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 08:29:36 +0100
Subject: [PATCH 11/15] ERRORS: Add ERR_GID_DUPLICATED
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This new error will be returned from sysdb_add_incomplete_group()
when renaming a group which will case gid collision.

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/util/util_errors.c | 1 +
 src/util/util_errors.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/util/util_errors.c b/src/util/util_errors.c
index 39ce3d7dc..e2bb2a014 100644
--- a/src/util/util_errors.c
+++ b/src/util/util_errors.c
@@ -118,6 +118,7 @@ struct err_string error_to_str[] = {
     { "GetAccountDomain() not supported" }, /* ERR_GET_ACCT_DOM_NOT_SUPPORTED */
     { "The last GetAccountDomain() result is still valid" }, /* ERR_GET_ACCT_DOM_CACHED */
     { "ID is outside the allowed range" }, /* ERR_ID_OUTSIDE_RANGE */
+    { "Group ID is duplicated" }, /* ERR_GID_DUPLICATED */
     { "ERR_LAST" } /* ERR_LAST */
 };
 
diff --git a/src/util/util_errors.h b/src/util/util_errors.h
index ad4dad5f8..49501727d 100644
--- a/src/util/util_errors.h
+++ b/src/util/util_errors.h
@@ -140,6 +140,7 @@ enum sssd_errors {
     ERR_GET_ACCT_DOM_NOT_SUPPORTED,
     ERR_GET_ACCT_DOM_CACHED,
     ERR_ID_OUTSIDE_RANGE,
+    ERR_GID_DUPLICATED,
     ERR_LAST            /* ALWAYS LAST */
 };
 

From a1f0116da378811050b747e82b30ff1c3ed2f0d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 12:43:06 +0100
Subject: [PATCH 12/15] SDAP: Add
 sdap_handle_id_collision_for_incomplete_groups()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This newly added function is a helper to properly hadle group
id-collisions when renaming incomplete groups and it does:
- Deletes the group from sysdb
- Adds the new incomplete group
- Notifies the NSS responder that the entry also has to be deleted from
  the memory cache

This function will be called from
sdap_ad_save_group_membership_with_idmapping() and from
sdap_add_incomplete_groups().

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/sdap_async.h            | 11 ++++++++++
 src/providers/ldap/sdap_async_initgroups.c | 34 ++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
index ece0c5570..362599fa2 100644
--- a/src/providers/ldap/sdap_async.h
+++ b/src/providers/ldap/sdap_async.h
@@ -414,4 +414,15 @@ sdap_ad_tokengroups_initgroups_send(TALLOC_CTX *mem_ctx,
 errno_t
 sdap_ad_tokengroups_initgroups_recv(struct tevent_req *req);
 
+errno_t
+sdap_handle_id_collision_for_incomplete_groups(void *provider,
+                                               struct sss_domain_info *domain,
+                                               const char *name,
+                                               gid_t gid,
+                                               const char *original_dn,
+                                               const char *sid_str,
+                                               const char *uuid,
+                                               bool posix,
+                                               time_t now);
+
 #endif /* _SDAP_ASYNC_H_ */
diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index cfb0e6728..275522c4f 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -3571,3 +3571,37 @@ errno_t get_sysdb_grouplist_dn(TALLOC_CTX *mem_ctx,
     return get_sysdb_grouplist_ex(mem_ctx, sysdb, domain,
                                   name, grouplist, true);
 }
+
+errno_t
+sdap_handle_id_collision_for_incomplete_groups(void *provider,
+                                               struct sss_domain_info *domain,
+                                               const char *name,
+                                               gid_t gid,
+                                               const char *original_dn,
+                                               const char *sid_str,
+                                               const char *uuid,
+                                               bool posix,
+                                               time_t now)
+{
+    errno_t ret;
+
+    ret = sysdb_delete_group(domain, NULL, gid);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              "Due to an id collision, the new group with gid [\"%"PRIu32"\"] "
+              "will not be added as the old group (with the same gid) could "
+              "not be removed from the sysdb!",
+              gid);
+        return ret;
+    }
+
+    ret = sysdb_add_incomplete_group(domain, name, gid, original_dn, sid_str,
+                                     uuid, posix, now);
+    if (ret != EOK) {
+        return ret;
+    }
+
+    dp_sbus_invalidate_group_memcache(provider, gid);
+
+    return EOK;
+}

From 31d823f46df7671ef82d844509c6259419317361 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 12:51:57 +0100
Subject: [PATCH 13/15] SDAP: Properly handle group id-collision when renaming
 incomplete groups
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Resolves:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/providers/ldap/sdap_async_initgroups.c    | 13 +++++++++++++
 src/providers/ldap/sdap_async_initgroups_ad.c | 13 +++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index 275522c4f..38dfa1d7d 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -226,6 +226,19 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
                 ret = sysdb_add_incomplete_group(domain, groupname, gid,
                                                  original_dn, sid_str,
                                                  uuid, posix, now);
+                if (ret == ERR_GID_DUPLICATED) {
+                    /* In case o group id-collision, do:
+                     * - Delete the group from sysdb
+                     * - Add the new incomplete group
+                     * - Notify the NSS responder that the entry has also to be
+                     *   removed from the memory cache
+                     */
+                    ret = sdap_handle_id_collision_for_incomplete_groups(
+                                            provider, domain, groupname, gid,
+                                            original_dn, sid_str, uuid, posix,
+                                            now);
+                }
+
                 if (ret != EOK) {
                     goto done;
                 }
diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c
index 4947fafaa..b547fed9a 100644
--- a/src/providers/ldap/sdap_async_initgroups_ad.c
+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
@@ -929,6 +929,19 @@ errno_t sdap_ad_save_group_membership_with_idmapping(const char *username,
 
             ret = sysdb_add_incomplete_group(domain, name, gid,
                                              NULL, sid, NULL, false, now);
+            if (ret == ERR_GID_DUPLICATED) {
+                /* In case o group id-collision, do:
+                 * - Delete the group from sysdb
+                 * - Add the new incomplete group
+                 * - Notify the NSS responder that the entry has also to be
+                 *   removed from the memory cache
+                 */
+                ret = sdap_handle_id_collision_for_incomplete_groups(
+                                            idmap_ctx->id_ctx->be->provider,
+                                            domain, name, gid, NULL, sid, NULL,
+                                            false, now);
+            }
+
             if (ret != EOK) {
                 DEBUG(SSSDBG_MINOR_FAILURE, "Could not create incomplete "
                                              "group: [%s]\n", strerror(ret));

From b66332a869e826fecb66730df231a57fea6cbd36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 19 Feb 2018 08:53:56 +0100
Subject: [PATCH 14/15] SYSDB_OPS: Error out on id-collision when adding an
 incomplete group
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This situation can be hit when renaming a group. For now, let's just
error this out so the caller can handle it properly on its own layer.

Related:
https://pagure.io/SSSD/sssd/issue/2653

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/db/sysdb_ops.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 15915101e..e88ff04ef 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -2340,12 +2340,34 @@ int sysdb_add_incomplete_group(struct sss_domain_info *domain,
     TALLOC_CTX *tmp_ctx;
     int ret;
     struct sysdb_attrs *attrs;
+    struct ldb_message *msg;
+    const char *previous = NULL;
+    const char *group_attrs[] = { SYSDB_SID_STR, SYSDB_UUID, SYSDB_ORIG_DN, NULL };
+    const char *values[] = { sid_str, uuid, original_dn, NULL };
+    bool same = false;
 
     tmp_ctx = talloc_new(NULL);
     if (!tmp_ctx) {
         return ENOMEM;
     }
 
+    ret = sysdb_search_group_by_gid(tmp_ctx, domain, gid, group_attrs, &msg);
+    if (ret == EOK) {
+        for (int i = 0; !same && group_attrs[i] != NULL; i++) {
+            previous = ldb_msg_find_attr_as_string(msg,
+                                                   group_attrs[i],
+                                                   NULL);
+            if (previous != NULL && values[i] != NULL) {
+                same = strcmp(previous, values[i]) == 0;
+            }
+        }
+    }
+
+    if (same) {
+        ret = ERR_GID_DUPLICATED;
+        goto done;
+    }
+
     /* try to add the group */
     ret = sysdb_add_basic_group(domain, name, gid);
     if (ret) goto done;

From f5432eaeb268b568e59816bd80b74813083b89f5 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 19 Feb 2018 18:26:05 +0100
Subject: [PATCH 15/15] TESTS: Add an integration test for renaming incomplete
 groups during initgroups
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As we implemented the group renaming heuristics to rename only if we can
use another "hint" like the original DN or the SID to know the group is
the same, this patch adds two tests (positive and negative) to make sure
a group with a totally different RDN and hence different originalDN
cannot be renamed but a group whose name changed but the RDN stays the
same can be renamed.

Related:
https://pagure.io/SSSD/sssd/issue/3282

Reviewed-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/tests/intg/test_ldap.py | 131 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 129 insertions(+), 2 deletions(-)

diff --git a/src/tests/intg/test_ldap.py b/src/tests/intg/test_ldap.py
index a6659b1b7..10205be1f 100644
--- a/src/tests/intg/test_ldap.py
+++ b/src/tests/intg/test_ldap.py
@@ -94,10 +94,11 @@ def create_ldap_cleanup(request, ldap_conn, ent_list=None):
     request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list))
 
 
-def create_ldap_fixture(request, ldap_conn, ent_list=None):
+def create_ldap_fixture(request, ldap_conn, ent_list=None, cleanup=True):
     """Add LDAP entries and add teardown for removing them"""
     create_ldap_entries(ldap_conn, ent_list)
-    create_ldap_cleanup(request, ldap_conn, ent_list)
+    if cleanup:
+        create_ldap_cleanup(request, ldap_conn, ent_list)
 
 
 SCHEMA_RFC2307 = "rfc2307"
@@ -1383,3 +1384,129 @@ def test_ldap_auto_private_groups_direct_no_gid(ldap_conn, mpg_setup_no_gid):
             ", ".join(["%s" % s for s in sorted(gids)]),
             ", ".join(["%s" % s for s in sorted(user1_expected_gids)])
         )
+
+
+def rename_setup_no_cleanup(request, ldap_conn, cleanup_ent=None):
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+    ent_list.add_group_bis("user1_private", 2001)
+    ent_list.add_group_bis("group1", 2015, ["user1"])
+
+    if cleanup_ent is None:
+        create_ldap_fixture(request, ldap_conn, ent_list)
+    else:
+        # Since the entries were renamed, we need to clean up
+        # the renamed entries..
+        create_ldap_fixture(request, ldap_conn, ent_list, cleanup=False)
+        create_ldap_cleanup(request, ldap_conn, None)
+
+
+@pytest.fixture
+def rename_setup_cleanup(request, ldap_conn):
+    cleanup_ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    cleanup_ent_list.add_user("user1", 1001, 2001)
+    cleanup_ent_list.add_group_bis("new_user1_private", 2001)
+    cleanup_ent_list.add_group_bis("new_group1", 2015, ["user1"])
+
+    rename_setup_no_cleanup(request, ldap_conn, cleanup_ent_list)
+
+    conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+@pytest.fixture
+def rename_setup_with_name(request, ldap_conn):
+    rename_setup_no_cleanup(request, ldap_conn)
+
+    conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) + \
+        unindent("""
+            [nss]
+            [domain/LDAP]
+            ldap_group_name                = name
+            timeout = 3000
+        """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def test_rename_incomplete_group_same_dn(ldap_conn, rename_setup_with_name):
+    """
+    Test that if a group's name attribute changes, but the DN stays the same,
+    the incomplete group object will be renamed.
+
+    Because the RDN attribute must be present in the entry, we add another
+    attribute "name" that is purposefully different from the CN and make
+    sure the group names are reflected in name
+
+    Regression test for https://pagure.io/SSSD/sssd/issue/3282
+    """
+    pvt_dn = 'cn=user1_private,ou=Groups,' + ldap_conn.ds_inst.base_dn
+    group1_dn = 'cn=group1,ou=Groups,' + ldap_conn.ds_inst.base_dn
+
+    # Add the name we want for both private and secondary group
+    old = {'name': []}
+    new = {'name': [b"user1_group1"]}
+    ldif = ldap.modlist.modifyModlist(old, new)
+    ldap_conn.modify_s(group1_dn, ldif)
+
+    new = {'name': [b"pvt_user1"]}
+    ldif = ldap.modlist.modifyModlist(old, new)
+    ldap_conn.modify_s(pvt_dn, ldif)
+
+    # Make sure the old name shows up in the id output
+    (res, errno, grp_list) = sssd_id.get_user_groups("user1")
+    assert res == sssd_id.NssReturnCode.SUCCESS, \
+        "Could not find groups for user1, %d" % errno
+
+    assert sorted(grp_list) == sorted(["pvt_user1", "user1_group1"])
+
+    # Rename the group by changing the cn attribute, but keep the DN the same
+    old = {'name': [b"user1_group1"]}
+    new = {'name': [b"new_user1_group1"]}
+    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")
+
+    (res, errno, grp_list) = sssd_id.get_user_groups("user1")
+    assert res == sssd_id.NssReturnCode.SUCCESS, \
+        "Could not find groups for user1, %d" % errno
+
+    assert sorted(grp_list) == sorted(["pvt_user1", "new_user1_group1"])
+
+
+def test_rename_incomplete_group_rdn_changed(ldap_conn, rename_setup_cleanup):
+    """
+    Test that if a group's name attribute changes, and the DN changes with
+    the RDN. Then adding the second group will fail because we can't tell if
+    there are two duplicate groups in LDAP when saving the group or if the
+    group was renamed.
+
+    Please note that with many directories (AD, IPA), the code can rely on
+    other heuristics (SID, UUID) to find out the group is in fact the same.
+
+    Regression test for https://pagure.io/SSSD/sssd/issue/3282
+    """
+    pvt_dn = 'cn=user1_private,ou=Groups,' + ldap_conn.ds_inst.base_dn
+    group1_dn = 'cn=group1,ou=Groups,' + ldap_conn.ds_inst.base_dn
+
+    # Make sure the old name shows up in the id output
+    (res, errno, grp_list) = sssd_id.get_user_groups("user1")
+    assert res == sssd_id.NssReturnCode.SUCCESS, \
+        "Could not find groups for user1, %d" % errno
+
+    assert sorted(grp_list) == sorted(["user1_private", "group1"])
+
+    # Rename the groups, changing the RDN
+    ldap_conn.rename_s(group1_dn, "cn=new_group1")
+    ldap_conn.rename_s(pvt_dn, "cn=new_user1_private")
+
+    if subprocess.call(["sss_cache", "-GU"]) != 0:
+        raise Exception("sssd_cache failed")
+
+    # The initgroups fails and we fall back to the old names in the cache
+    assert sorted(grp_list) == sorted(["user1_private", "group1"])
_______________________________________________
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