Related: RHEL-77184 - AD user in external group is not cleared when expiring the cache

Patch used to fix this ticket causes a regression (RHEL-106987) and is being reverted.
This commit is contained in:
Alexey Tikhonov 2025-08-14 16:24:36 +02:00
parent 2c9c7e1036
commit 80a9e1b7ff
2 changed files with 228 additions and 2 deletions

View File

@ -0,0 +1,222 @@
From 98a91d170f7a6074ed1bd3b8ed9161c4a11b4074 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
Date: Thu, 14 Aug 2025 16:21:00 +0200
Subject: [PATCH] Revert "ipa: improve handling of external group memberships"
This reverts commit 63a6f51069a86765417f044a62705fe20572e0da.
---
src/providers/ipa/ipa_subdomains_ext_groups.c | 152 +++---------------
1 file changed, 22 insertions(+), 130 deletions(-)
diff --git a/src/providers/ipa/ipa_subdomains_ext_groups.c b/src/providers/ipa/ipa_subdomains_ext_groups.c
index f86130d89..ba3fb3953 100644
--- a/src/providers/ipa/ipa_subdomains_ext_groups.c
+++ b/src/providers/ipa/ipa_subdomains_ext_groups.c
@@ -312,19 +312,11 @@ static errno_t add_ad_user_to_cached_groups(struct ldb_dn *user_dn,
bool *missing_groups)
{
size_t c;
- size_t d = 0;
struct sysdb_attrs *user_attrs;
size_t msgs_count;
struct ldb_message **msgs;
TALLOC_CTX *tmp_ctx;
int ret;
- const struct ldb_val *val;
- char *user_name;
- char **sysdb_ipa_group_memberships;
- char **add_groups;
- char **del_groups;
- errno_t sret;
- bool in_transaction = false;
*missing_groups = false;
@@ -334,96 +326,18 @@ static errno_t add_ad_user_to_cached_groups(struct ldb_dn *user_dn,
return ENOMEM;
}
- val = ldb_dn_get_rdn_val(user_dn);
- if (val == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "user_dn has no RDN.\n");
- ret = EINVAL;
- goto done;
- }
- user_name = talloc_strndup(tmp_ctx, (char *) val->data, val->length);
- if (user_name == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "Failed to copy user name.\n");
- ret = ENOMEM;
- goto done;
- }
-
- ret = sysdb_transaction_start(user_dom->sysdb);
- if (ret != EOK) {
- DEBUG(SSSDBG_FATAL_FAILURE, "Failed to start update transaction\n");
- goto done;
- }
-
- in_transaction = true;
-
- ret = sysdb_get_direct_parents_ex(tmp_ctx, user_dom, group_dom,
- SYSDB_MEMBER_USER, user_name,
- SYSDB_ORIG_DN,
- &sysdb_ipa_group_memberships);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, "Failed to get current IPA group memberships "
- "of user [%s].\n", user_name);
- goto done;
- }
-
- ret = diff_string_lists(tmp_ctx, groups, sysdb_ipa_group_memberships,
- &add_groups, &del_groups, NULL);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, "Failed to get difference in group lists.\n");
- goto done;
- }
-
- user_attrs = sysdb_new_attrs(tmp_ctx);
- if (user_attrs == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "sysdb_new_attrs failed.\n");
- ret = ENOMEM;
- goto done;
- }
-
- /* Add all new IPA groups to SYSDB_ORIG_MEMBEROF because they are most
- * probably removed by the previous user update and mark all new groups as
- * processed. */
for (c = 0; groups[c] != NULL; c++) {
- ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF,
- groups[c]);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_add_string failed.\n");
- goto done;
+ if (groups[c][0] == '\0') {
+ continue;
}
- groups[c][0] = '\0';
- }
-
- if (DEBUG_IS_SET(SSSDBG_TRACE_ALL)) {
- DEBUG(SSSDBG_TRACE_ALL, "New IPA groups [%zu].\n", c);
-
- for (c = 0; sysdb_ipa_group_memberships[c] != NULL; c++);
- DEBUG(SSSDBG_TRACE_ALL, "Cached IPA groups [%zu].\n", c);
-
- for (c = 0; add_groups[c] != NULL; c++);
- DEBUG(SSSDBG_TRACE_ALL, "Groups to add [%zu].\n", c);
-
- for (c = 0; del_groups[c] != NULL; c++);
- DEBUG(SSSDBG_TRACE_ALL, "Groups to delete [%zu].\n", c);
- }
-
- /* TODO: there is a similar functionality (adding and removing group
- * memberships in sysdb_update_members_ex(), but the missing group feature
- * is missing. It might be worth to evaluate if either the missing group
- * feature can be added there or if group which are missing in the cache
- * can bew handled differently here. */
-
- for (c = 0; add_groups[c] != NULL; c++) {
-
- ret = sysdb_search_groups_by_orig_dn(tmp_ctx, group_dom, add_groups[c],
+ ret = sysdb_search_groups_by_orig_dn(tmp_ctx, group_dom, groups[c],
NULL, &msgs_count, &msgs);
if (ret != EOK) {
if (ret == ENOENT) {
DEBUG(SSSDBG_TRACE_ALL, "Group [%s] not in the cache.\n",
- add_groups[c]);
+ groups[c]);
*missing_groups = true;
- talloc_free(groups[d]);
- /* add missing group back to the list */
- groups[d++] = talloc_steal(groups, add_groups[c]);
continue;
} else {
DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_entry failed.\n");
@@ -431,6 +345,9 @@ static errno_t add_ad_user_to_cached_groups(struct ldb_dn *user_dn,
}
}
+/* TODO? Do we have to remove members as well? I think not because the AD
+ * query before removes all memberships. */
+
ret = sysdb_mod_group_member(group_dom, user_dn, msgs[0]->dn,
LDB_FLAG_MOD_ADD);
if (ret != EOK && ret != EEXIST) {
@@ -438,58 +355,33 @@ static errno_t add_ad_user_to_cached_groups(struct ldb_dn *user_dn,
goto done;
}
- }
- talloc_free(groups[d]);
- groups[d] = NULL;
+ user_attrs = sysdb_new_attrs(tmp_ctx);
+ if (user_attrs == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_new_attrs failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
- for (c = 0; del_groups[c] != NULL; c++) {
- ret = sysdb_search_groups_by_orig_dn(tmp_ctx, group_dom, del_groups[c],
- NULL, &msgs_count, &msgs);
+ ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF,
+ groups[c]);
if (ret != EOK) {
- if (ret == ENOENT) {
- DEBUG(SSSDBG_TRACE_ALL,
- "Group [%s] not in the cache, skipping.\n",
- del_groups[c]);
- continue;
- } else {
- DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_entry failed.\n");
- goto done;
- }
+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_add_string failed.\n");
+ goto done;
}
- ret = sysdb_mod_group_member(group_dom, user_dn, msgs[0]->dn,
- LDB_FLAG_MOD_DELETE);
+ ret = sysdb_set_entry_attr(user_dom->sysdb, user_dn, user_attrs,
+ LDB_FLAG_MOD_ADD);
if (ret != EOK && ret != EEXIST) {
- DEBUG(SSSDBG_OP_FAILURE,
- "sysdb_mod_group_member failed to delete member.\n");
+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_set_entry_attr failed.\n");
goto done;
}
- }
- /* Update SYSDB_ORIG_MEMBEROF with the IPA groups. */
- ret = sysdb_set_entry_attr(user_dom->sysdb, user_dn, user_attrs,
- LDB_FLAG_MOD_ADD);
- if (ret != EOK && ret != EEXIST) {
- DEBUG(SSSDBG_OP_FAILURE, "Failed to add original IPA group DNs, ignored.\n");
- }
-
- ret = sysdb_transaction_commit(user_dom->sysdb);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, "Failed to commit transaction\n");
- goto done;
+ /* mark group as already processed */
+ groups[c][0] = '\0';
}
- in_transaction = false;
-
ret = EOK;
done:
- if (in_transaction) {
- sret = sysdb_transaction_cancel(user_dom->sysdb);
- if (sret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, "Could not cancel transaction\n");
- }
- }
-
talloc_free(tmp_ctx);
return ret;
--
2.50.0

View File

@ -23,7 +23,7 @@
Name: sssd
Version: 2.11.1
Release: 1%{?dist}
Release: 2%{?dist}
Summary: System Security Services Daemon
License: GPL-3.0-or-later
URL: https://github.com/SSSD/sssd/
@ -31,7 +31,7 @@ Source0: https://github.com/SSSD/sssd/releases/download/2.11.1/sssd-2.11.1.tar.g
Source1: sssd.sysusers
### Patches ###
# Patch0001:
Patch0001: 0001-Revert-ipa-improve-handling-of-external-group-member.patch
### Dependencies ###
@ -1092,6 +1092,10 @@ fi
%systemd_postun_with_restart sssd.service
%changelog
* Thu Aug 14 2025 Alexey Tikhonov <atikhono@redhat.com> - 2.11.1-2
- Related: RHEL-77184 - AD user in external group is not cleared when expiring the cache
Patch used to fix this ticket causes a regression (RHEL-106987) and is being reverted.
* Thu Jul 31 2025 Alexey Tikhonov <atikhono@redhat.com> - 2.11.1-1
- Resolves: RHEL-95058 - Rebase SSSD for RHEL 10.1
- Resolves: RHEL-77184 - AD user in external group is not cleared when expiring the cache