Skip to content

sdap: do not require GID for non-POSIX group#8442

Merged
alexey-tikhonov merged 1 commit into
SSSD:masterfrom
sumit-bose:nested_group_members_fix
Feb 17, 2026
Merged

sdap: do not require GID for non-POSIX group#8442
alexey-tikhonov merged 1 commit into
SSSD:masterfrom
sumit-bose:nested_group_members_fix

Conversation

@sumit-bose

Copy link
Copy Markdown
Contributor

In 85b632d the attribute for the GID was removed from non-POSIX groups. Currently sdap_save_group() still requires the attribute and this patch removes this.

sdap_save_group() is currently only used in the code path handling nested groups. To verify the change a test was added were indirect group-members are coming from a nested non-POSIX group.

Resolves: #8441

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

Copy link
Copy Markdown

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 sdap_save_group() incorrectly required a GID for non-POSIX groups. The change wraps the GID retrieval logic within a check for posix_group, ensuring that the GID is only required for POSIX groups. This aligns with the intended behavior of allowing non-POSIX groups without GIDs. A new test case has been added to test_identity.py which effectively validates this fix by checking indirect group membership through a nested non-POSIX group. The changes are correct and the test coverage is good.

@justin-stephenson justin-stephenson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the quick fix!

Comment thread src/tests/system/tests/test_identity.py Outdated
Comment thread src/tests/system/tests/test_identity.py
Comment thread src/tests/system/tests/test_identity.py Outdated
Comment thread src/tests/system/tests/test_identity.py
Comment thread src/providers/ldap/sdap_async_groups.c
Comment thread src/providers/ldap/sdap_async_groups.c
@alexey-tikhonov alexey-tikhonov added the coverity Trigger a coverity scan label Feb 16, 2026
@alexey-tikhonov

Copy link
Copy Markdown
Member

Note: Covscan is green.

@alexey-tikhonov alexey-tikhonov added Accepted and removed coverity Trigger a coverity scan labels Feb 16, 2026
In 85b632d the attribute for the GID
was removed from non-POSIX groups. Currently sdap_save_group() still
requires the attribute and this patch removes this.

sdap_save_group() is currently only used in the code path handling
nested groups. To verify the change a test was added were indirect
group-members are coming from a nested non-POSIX group.

Resolves: SSSD#8441
Reviewed-by: Alexey Tikhonov <[email protected]>
Reviewed-by: Justin Stephenson <[email protected]>
@sssd-bot

Copy link
Copy Markdown
Contributor

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


🟢 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-44-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 / intgcheck (fedora-45) (failure)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🔴 ci / system (fedora-44) (failure)
🔴 ci / system (fedora-45) (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 nested_group_members_fix branch from 1cb28c9 to 13eec36 Compare February 17, 2026 08:45
@alexey-tikhonov
alexey-tikhonov merged commit ad173e0 into SSSD:master Feb 17, 2026
10 of 15 checks passed
abbra pushed a commit to freeipa/freeipa that referenced this pull request Apr 29, 2026
The regression was introduced in 2.12.0, we can't xfail older versions.
For more details see: SSSD/sssd#8442

Fixes: https://pagure.io/freeipa/issue/9981
Signed-off-by: David Hanina <[email protected]>
Reviewed-By: Florence Blanc-Renaud <[email protected]>
abbra pushed a commit to freeipa/freeipa that referenced this pull request May 4, 2026
The regression was introduced in 2.12.0, we can't xfail older versions.
For more details see: SSSD/sssd#8442

Fixes: https://pagure.io/freeipa/issue/9981
Signed-off-by: David Hanina <[email protected]>
Reviewed-By: Florence Blanc-Renaud <[email protected]>
abbra pushed a commit to freeipa/freeipa that referenced this pull request May 4, 2026
The regression was introduced in 2.12.0, we can't xfail older versions.
For more details see: SSSD/sssd#8442

Fixes: https://pagure.io/freeipa/issue/9981
Signed-off-by: David Hanina <[email protected]>
Reviewed-By: Florence Blanc-Renaud <[email protected]>
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.

Failed to resolve indirect group-members of nested non-POSIX group

4 participants