Test: HBAC affecting AD-users ipa-group membership#8074
Closed
shridhargadekar wants to merge 3 commits intoSSSD:masterfrom
Closed
Test: HBAC affecting AD-users ipa-group membership#8074shridhargadekar wants to merge 3 commits intoSSSD:masterfrom
shridhargadekar wants to merge 3 commits intoSSSD:masterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a new system test to reproduce a bug where an AD user's IPA group membership is lost after an HBAC rule is applied. My review identifies a critical error that would prevent the test from running, as well as opportunities to improve the test's robustness and clarity. Specifically, I've pointed out an undefined variable, suggested replacing a fixed time.sleep with a more reliable polling mechanism to avoid flaky tests, and provided a revised docstring to correct several inconsistencies and accurately describe the test's behavior.
db23eff to
faf56b4
Compare
12a44a1 to
d3caef6
Compare
jakub-vavra-cz
requested changes
Aug 19, 2025
Contributor
jakub-vavra-cz
left a comment
There was a problem hiding this comment.
- Minor tweaks.
- Q: Is the fix needed also for 10.0 (sssd-2-10)?
- Shouldn't the test pass on some fedora that is not affected by the regression?
Member
Product patch causing a regression was backported everywhere:
All supported Fedora version are sssd-2.11.11 based that is affected. |
c1fd489 to
654403b
Compare
Member
|
@shridhargadekar, could you please rebase? |
654403b to
9f4bb4f
Compare
sumit-bose
reviewed
Sep 11, 2025
9f4bb4f to
153822e
Compare
153822e to
0b2dbe2
Compare
0b2dbe2 to
08bdca6
Compare
Recently add_ad_user_to_cached_groups() was modified to better handle adding and removing group memberships of users from trusted domains in groups of the local IPA domain. Before the change group members were only added and with the change a removal was possible as well. This caused an issues with an in-out parameter which contains the full list of IPA group memberships at input and the list of group missing in the cache as output. Since add_ad_user_to_cached_groups() is called twice, the second time after the missing groups were read from the IPA server, this caused and unexpected removal of group memberships since the second call to add_ad_user_to_cached_groups() was done with the list of missing groups and not with the full list. With this patch a dedicated list is used for the missing groups to avoid the described issues. Resolves: SSSD#7921
The ipa_add_trusted_memberships_send() request will use groups_get_send() to lookup missing groups. groups_get_send() can currently only lookup "proper" groups which besides other items means that the group name must be stored under the LDAP attribute given by the 'ldap_group_name' option. So currently it does not make sense to try to lookup other objects where the RDN attribute name is different than this value because those will always be treated as missing in the cache and always trigger an LDAP search which will fail. This will typically happen for iPAAssociation objects which are used to connect users and hosts with HBAC and sudo rules. If in future a more generic search is used this filter can be removed.
08bdca6 to
a25e638
Compare
sumit-bose
reviewed
Sep 12, 2025
| trusted.user(ad_user_name).add() | ||
| aduser_fqn = trusted.fqn(ad_user_name) | ||
|
|
||
| ipa.host.conn.exec(["ipa", "trust-find"]) |
Contributor
There was a problem hiding this comment.
Hi,
why is this command needed?
bye,
Sumit
Contributor
Author
There was a problem hiding this comment.
Hi,
These commands are not related to the test case. During the local containerized testing with pytest .... the aduser would not be detected. While troubleshooting the issue, I noticed that in the local environment, the AD-user was fetched after I execute ipa trust-find. It could be local environment thing.
I could drop it.
Thanks,
sumit-bose
reviewed
Sep 12, 2025
b2ab728 to
5614704
Compare
3b5b095 to
fb26104
Compare
In a IPA-AD trust environment, AD-user has membership of a external-ipa group. This external-ipa-group is member of IPA-posix group. When a HBAC rule is added and is the ipa-posix member is added as member for that rule, AD-user misses that IPA-group-membership
fb26104 to
2b6bdb9
Compare
Member
|
Included in #8094 |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In a IPA-AD trust environment, AD-user has membership of a external-ipa group. This external-ipa-group is member of IPA-posix group. When a HBAC rule is added and is the ipa-posix member is added as member for that rule, AD-user misses that IPA-group-membership