From b033b0dda972e885f63234aa81dca317c8234c2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Tue, 23 May 2023 12:21:44 +0200 Subject: [PATCH] ipa: correctly remove missing attributes on netgroup update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a netgroup is updated, previously it did not remove the missing attributes. This caused an issue especially when a member was removed. Resolves: https://github.com/SSSD/sssd/issues/6652 Reviewed-by: Alejandro López Reviewed-by: Iker Pedrosa Reference: https://github.com/SSSD/sssd/commit/b033b0dda972e885f63234aa81dca317c8234c2c Conflict: NA --- src/db/sysdb.c | 9 ++ src/db/sysdb.h | 1 + src/providers/ipa/ipa_netgroups.c | 35 +++++++- src/tests/system/tests/test_netgroups.py | 108 +++++++++++++++++++++++ 4 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 src/tests/system/tests/test_netgroups.py diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 649e79fca..1faa11b16 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -523,6 +523,15 @@ static int sysdb_attrs_add_val_int(struct sysdb_attrs *attrs, return EOK; } + +int sysdb_attrs_add_empty(struct sysdb_attrs *attrs, const char *name) +{ + struct ldb_message_element *el; + + /* Calling this will create the element if it does not exist. */ + return sysdb_attrs_get_el_ext(attrs, name, true, &el); +} + int sysdb_attrs_add_val(struct sysdb_attrs *attrs, const char *name, const struct ldb_val *val) { diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 2f20692cc..887a9630e 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -398,6 +398,7 @@ enum sysdb_obj_type { extern const char *sysdb_ts_cache_attrs[]; /* values are copied in the structure, allocated on "attrs" */ +int sysdb_attrs_add_empty(struct sysdb_attrs *attrs, const char *name); int sysdb_attrs_add_val(struct sysdb_attrs *attrs, const char *name, const struct ldb_val *val); int sysdb_attrs_add_val_safe(struct sysdb_attrs *attrs, diff --git a/src/providers/ipa/ipa_netgroups.c b/src/providers/ipa/ipa_netgroups.c index 52d90af4f..57f11a507 100644 --- a/src/providers/ipa/ipa_netgroups.c +++ b/src/providers/ipa/ipa_netgroups.c @@ -70,7 +70,10 @@ static errno_t ipa_save_netgroup(TALLOC_CTX *mem_ctx, struct ldb_message_element *el; struct sysdb_attrs *netgroup_attrs; const char *name = NULL; + char **missing; + int missing_index; int ret; + int i; size_t c; ret = sysdb_attrs_get_el(attrs, @@ -90,6 +93,23 @@ static errno_t ipa_save_netgroup(TALLOC_CTX *mem_ctx, goto fail; } + missing = talloc_zero_array(netgroup_attrs, char *, attrs->num + 1); + if (missing == NULL) { + ret = ENOMEM; + goto fail; + } + + for (i = 0, missing_index = 0; i < attrs->num; i++) { + if (attrs->a[i].num_values == 0) { + missing[missing_index] = talloc_strdup(missing, attrs->a[i].name); + if (missing[missing_index] == NULL) { + ret = ENOMEM; + goto fail; + } + missing_index++; + } + } + ret = sysdb_attrs_get_el(attrs, SYSDB_ORIG_DN, &el); if (ret) { goto fail; @@ -138,7 +158,6 @@ static errno_t ipa_save_netgroup(TALLOC_CTX *mem_ctx, if (el->num_values == 0) { DEBUG(SSSDBG_TRACE_LIBS, "No original members for netgroup [%s]\n", name); - } else { DEBUG(SSSDBG_TRACE_LIBS, "Adding original members to netgroup [%s]\n", name); @@ -173,7 +192,7 @@ static errno_t ipa_save_netgroup(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_FUNC, "Storing info for netgroup %s\n", name); - ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, NULL, + ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, 0); if (ret) goto fail; @@ -866,6 +885,18 @@ static int ipa_netgr_process_all(struct ipa_get_netgroups_state *state) hash_iterate(state->new_netgroups, extract_netgroups, state); for (i = 0; i < state->netgroups_count; i++) { + /* Make sure these attributes always exist, so we can remove them if + * there are no members. */ + ret = sysdb_attrs_add_empty(state->netgroups[i], SYSDB_NETGROUP_MEMBER); + if (ret != EOK) { + goto done; + } + + ret = sysdb_attrs_add_empty(state->netgroups[i], SYSDB_NETGROUP_TRIPLE); + if (ret != EOK) { + goto done; + } + /* load all its member netgroups, translate */ DEBUG(SSSDBG_TRACE_INTERNAL, "Extracting netgroup members of netgroup %d\n", i); ret = sysdb_attrs_get_string_array(state->netgroups[i], diff --git a/src/tests/system/tests/test_netgroups.py b/src/tests/system/tests/test_netgroups.py new file mode 100644 index 000000000..6b6bc8e8b --- /dev/null +++ b/src/tests/system/tests/test_netgroups.py @@ -0,0 +1,108 @@ +""" +Netgroup tests. + +:requirement: netgroup +""" + +from __future__ import annotations + +import pytest +from sssd_test_framework.roles.client import Client +from sssd_test_framework.roles.generic import GenericProvider +from sssd_test_framework.topology import KnownTopologyGroup + + +@pytest.mark.tier(1) +@pytest.mark.ticket(gh=6652, bz=2162552) +@pytest.mark.topology(KnownTopologyGroup.AnyProvider) +def test_netgroups__add_remove_netgroup_triple(client: Client, provider: GenericProvider): + """ + :title: Netgroup triple is correctly removed from cached record + :setup: + 1. Create local user "user-1" + 2. Create netgroup "ng-1" + 3. Add "(-,user-1,)" triple to the netgroup + 4. Start SSSD + :steps: + 1. Run "getent netgroup ng-1" + 2. Remove "(-,user-1,)" triple from "ng-1" + 3. Invalidate netgroup in cache "sssctl cache-expire -n ng-1" + 4. Run "getent netgroup ng-1" + :expectedresults: + 1. "(-,user-1,)" is present in the netgroup + 2. Triple was removed from the netgroup + 3. Cached record was invalidated + 4. "(-,user-1,)" is not present in the netgroup + :customerscenario: True + """ + user = provider.user("user-1").add() + ng = provider.netgroup("ng-1").add().add_member(user=user) + + client.sssd.start() + + result = client.tools.getent.netgroup("ng-1") + assert result is not None + assert result.name == "ng-1" + assert len(result.members) == 1 + assert "(-, user-1)" in result.members + + ng.remove_member(user=user) + client.sssctl.cache_expire(netgroups=True) + + result = client.tools.getent.netgroup("ng-1") + assert result is not None + assert result.name == "ng-1" + assert len(result.members) == 0 + + +@pytest.mark.tier(1) +@pytest.mark.ticket(gh=6652, bz=2162552) +@pytest.mark.topology(KnownTopologyGroup.AnyProvider) +def test_netgroups__add_remove_netgroup_member(client: Client, provider: GenericProvider): + """ + :title: Netgroup member is correctly removed from cached record + :setup: + 1. Create local user "user-1" + 2. Create local user "user-2" + 3. Create netgroup "ng-1" + 4. Create netgroup "ng-2" + 5. Add "(-,user-1,)" triple to the netgroup "ng-1" + 6. Add "(-,user-2,)" triple to the netgroup "ng-2" + 7. Add "ng-1" as a member to "ng-2" + 8. Start SSSD + :steps: + 1. Run "getent netgroup ng-2" + 2. Remove "ng-1" from "ng-2" + 3. Invalidate netgroup "ng-2" in cache "sssctl cache-expire -n ng-2" + 4. Run "getent netgroup ng-2" + :expectedresults: + 1. "(-,user-1,)", "(-,user-2,)" is present in the netgroup + 2. Netgroup member was removed from the netgroup + 3. Cached record was invalidated + 4. "(-,user-1,)" is not present in the netgroup, only "(-,user-2,)" + :customerscenario: True + """ + u1 = provider.user("user-1").add() + u2 = provider.user("user-2").add() + + ng1 = provider.netgroup("ng-1").add().add_member(user=u1) + ng2 = provider.netgroup("ng-2").add().add_member(user=u2, ng=ng1) + + client.sssd.start() + + result = client.tools.getent.netgroup("ng-2") + assert result is not None + assert result.name == "ng-2" + assert len(result.members) == 2 + assert "(-, user-1)" in result.members + assert "(-, user-2)" in result.members + + ng2.remove_member(ng=ng1) + client.sssctl.cache_expire(netgroups=True) + + result = client.tools.getent.netgroup("ng-2") + assert result is not None + assert result.name == "ng-2" + assert len(result.members) == 1 + assert "(-, user-1)" not in result.members + assert "(-, user-2)" in result.members -- 2.27.0