LDAP: Don't add a user member twice when adding a primary group

https://fedorahosted.org/sssd/ticket/2406

In the AD case, deployments sometimes add groups as parents of the
primary GID group. These groups are then returned during initgroups
in the tokenGroups attribute and member/memberof links are established
between the user and the group. However, any update of these groups
would remove the links, so a sequence of calls: id -G user; id user; id
-G user would return different group memberships.

Our code errored out in the rare case when the user was *also* an LDAP
member of his primary group.

Reviewed-by: Pavel Reichl <preichl@redhat.com>

1 files changed, 33 insertions, 5 deletions

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index c8e893259..a82d2aa34 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -191,19 +191,47 @@ sdap_dn_by_primary_gid(TALLOC_CTX *mem_ctx, struct sysdb_attrs *ldap_attrs,
     return EOK;
 }
 
+static bool has_member(struct ldb_message_element *member_el,
+                       char *member)
+{
+    struct ldb_val val;
+
+    val.data = (uint8_t *) member;
+    val.length = strlen(member);
+
+    /* This is bad complexity, but the this loop should only be invoked in
+     * the very rare scenario of AD POSIX group that is primary group of
+     * some users but has user member attributes at the same time
+     */
+    if (ldb_msg_find_val(member_el, &val) != NULL) {
+        return true;
+    }
+
+    return false;
+}
+
 static void link_pgroup_members(struct sysdb_attrs *group_attrs,
                                 struct ldb_message_element *member_el,
                                 char **userdns,
                                 size_t nuserdns)
 {
-    int i;
+    int i, j;
 
+    j = 0;
     for (i=0; i < nuserdns; i++) {
-        member_el->values[member_el->num_values + i].data = (uint8_t *) \
-                                          talloc_steal(group_attrs, userdns[i]);
-        member_el->values[member_el->num_values + i].length = strlen(userdns[i]);
+        if (has_member(member_el, userdns[i])) {
+            DEBUG(SSSDBG_TRACE_INTERNAL,
+                  "Member %s already included, skipping\n", userdns[i]);
+            continue;
+        }
+
+        member_el->values[member_el->num_values + j].data = (uint8_t *) \
+                                         talloc_steal(group_attrs, userdns[i]);
+        member_el->values[member_el->num_values + j].length = \
+                                         strlen(userdns[i]);
+        j++;
     }
-    member_el->num_values += nuserdns;
+    member_el->num_values += j;
 }
 
 static int sdap_fill_memberships(struct sdap_options *opts,