Skip to content

ipa s2n: do not try to update user-private-group#8002

Merged
alexey-tikhonov merged 2 commits intoSSSD:masterfrom
sumit-bose:mpg_after_view_change
Jan 10, 2026
Merged

ipa s2n: do not try to update user-private-group#8002
alexey-tikhonov merged 2 commits intoSSSD:masterfrom
sumit-bose:mpg_after_view_change

Conversation

@sumit-bose
Copy link
Contributor

When an IPA client requests the details about a trusted user from the
IPA server including its memberships the server will return the name of
all groups including the user-private-group. Since this group is not a
cached object on its own it is not needed to try to update it as a group
but it will be updated when the user object is updated.

This has to be taken into account especially after a client is assigned
to a new id-view because now the SYSDB_OVERRIDE_DN attribute is required
and all cached objects which are missing it must be updated. If the
user-private-group was found for update it should be skipped because the
calls to update group objects in the cache cannot handle
user-private-groups. This is expected behavior as user-private-groups
are not objects on their own.

@sumit-bose sumit-bose marked this pull request as draft June 17, 2025 13:44
@alexey-tikhonov
Copy link
Member

Is this related to https://issues.redhat.com/browse/RHEL-94545 ?

@pbrezina pbrezina force-pushed the master branch 2 times, most recently from f5d64b3 to b854636 Compare November 4, 2025 14:27
@sumit-bose sumit-bose force-pushed the mpg_after_view_change branch from 88f9155 to 2c72fc0 Compare December 22, 2025 18:33
@sumit-bose sumit-bose marked this pull request as ready for review December 22, 2025 18:33
@alexey-tikhonov
Copy link
Member

@sumit-bose, is there a related ticket?
I guess this needs a backport to sssd-2-9 as well?

@sumit-bose
Copy link
Contributor Author

@sumit-bose, is there a related ticket? I guess this needs a backport to sssd-2-9 as well?

Yes, this issue is present in sssd-2-9.

@aplopez
Copy link
Contributor

aplopez commented Jan 9, 2026

Failing tests seem to be failing on every PR.

Copy link
Contributor

@aplopez aplopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sumit-bose sumit-bose force-pushed the mpg_after_view_change branch from 2c72fc0 to 5c947a3 Compare January 9, 2026 15:59
@alexey-tikhonov
Copy link
Member

/gemini review

@alexey-tikhonov alexey-tikhonov added the coverity Trigger a coverity scan label Jan 9, 2026
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses an issue where SSSD would fail to resolve a trusted user after a view change if the user has a user-private-group. The root cause is that SSSD was incorrectly trying to update the user-private-group as a regular group object, which is not supported. The fix is to check the object's category and only attempt to update real group objects, skipping user-private-groups. The change is implemented correctly in ipa_s2n_exop.c by fetching and checking the SYSDB_OBJECTCATEGORY attribute before adding a group to the list of objects to be updated. A new system test is also added in test_ipa_trusts.py which accurately reproduces the scenario and validates the fix. The changes look good and address the issue effectively.

@alexey-tikhonov
Copy link
Member

Note: Covscan is green.

@alexey-tikhonov alexey-tikhonov added Accepted and removed coverity Trigger a coverity scan labels Jan 10, 2026
When an IPA client requests the details about a trusted user from the
IPA server including its memberships the server will return the name of
all groups including the user-private-group. Since this group is not a
cached object on its own it is not needed to try to update it as a group
but it will be updated when the user object is updated.

This has to be taken into account especially after a client is assigned
to a new id-view because now the SYSDB_OVERRIDE_DN attribute is required
and all cached objects which are missing it must be updated. If the
user-private-group was found for update it should be skipped because the
calls to update group objects in the cache cannot handle
user-private-groups. This is expected behavior as user-private-groups
are not objects on their own.

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Alejandro López <allopez@redhat.com>
After a new view is applied to a client SSSD should make sure that the
cache entries are updated properly and all cached users can still be
resolved properly.

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Alejandro López <allopez@redhat.com>
@sssd-bot
Copy link
Contributor

The pull request was accepted by @alexey-tikhonov with the following PR CI status:


🟢 CodeFactor (success)
🟢 CodeQL (success)
🟢 osh-diff-scan:fedora-rawhide-x86_64:upstream (success)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
🟢 Build / freebsd (success)
🟢 Build / make-distcheck (success)
🟢 ci / intgcheck (centos-10) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🔴 ci / system (fedora-44) (failure)
➖ Coverity scan / coverity (skipped)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)


There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@sssd-bot sssd-bot force-pushed the mpg_after_view_change branch from 5c947a3 to 6878634 Compare January 10, 2026 16:56
@alexey-tikhonov alexey-tikhonov merged commit 08c2ccf into SSSD:master Jan 10, 2026
11 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants