From 5e195ddf368b705f674ece2faf64261f66e20c23 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 26 Aug 2014 11:23:20 +0200 Subject: 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 --- src/providers/ldap/sdap_async_groups.c | 38 +++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) (limited to 'src/providers') 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, -- cgit