>From f56facb917927669d1d9230753440656a714de29 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sat, 16 Aug 2014 19:54:45 +0200 Subject: [PATCH] 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. --- src/providers/ldap/sdap_async_groups.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index a21b0332e0acc111d387cc39fe8009794a4baa43..92506daf9cc438d4e9cd6247d62db93b29d994c8 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -202,7 +202,7 @@ static int sdap_fill_memberships(struct sdap_options *opts, size_t nuserdns) { struct ldb_message_element *el; - int i, j; + int i, ii, j; int ret; errno_t hret; hash_key_t key; @@ -285,12 +285,30 @@ static int sdap_fill_memberships(struct sdap_options *opts, } el->num_values = j; + j = 0; for (i=0; i < nuserdns; i++) { - el->values[el->num_values + i].data = (uint8_t *) \ + /* 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 + */ + for (ii = 0; ii < el->num_values; ii++) { + if (strcmp(userdns[i], (const char *) el->values[ii].data) == 0) { + DEBUG(SSSDBG_TRACE_INTERNAL, + "Member %s already included, skipping\n", userdns[i]); + break; + } + } + + if (ii < el->num_values) { + continue; + } + + el->values[el->num_values + j].data = (uint8_t *) \ talloc_steal(group_attrs, userdns[i]); - el->values[el->num_values + i].length = strlen(userdns[i]); + el->values[el->num_values + j].length = strlen(userdns[i]); + j++; } - el->num_values += nuserdns; + el->num_values += j; ret = EOK; -- 1.9.3