Skip to content

Honor ldap filters#8534

Merged
alexey-tikhonov merged 1 commit intoSSSD:masterfrom
ondrejv2:test
Mar 26, 2026
Merged

Honor ldap filters#8534
alexey-tikhonov merged 1 commit intoSSSD:masterfrom
ondrejv2:test

Conversation

@ondrejv2
Copy link
Copy Markdown
Contributor

fixes bug #8533

@gemini-code-assist
Copy link
Copy Markdown

Warning

Gemini is experiencing higher than usual traffic and was unable to create the review. Please try again in a few hours by commenting /gemini review.

@ondrejv2
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown

@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 bug #8533 by adding a check to filter out entries in sdap_nested_group_lookup_recv based on their type, ensuring that only entries matching the expected type are processed. This change prevents incorrect entries from being included in nested group lookups.

Comment thread src/providers/ldap/sdap_async_nested_groups.c Outdated
Copy link
Copy Markdown
Contributor

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

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

Hi,

thank you for the fix, ACK.

I think Gemini's comments can be ignored, especially the return EOK;.

bye,
Sumit

Comment thread src/providers/ldap/sdap_async_nested_groups.c Outdated
Comment thread src/providers/ldap/sdap_async_nested_groups.c Outdated
@alexey-tikhonov
Copy link
Copy Markdown
Member

In sdap_nested_group_single_step_process()

    case SDAP_NESTED_GROUP_DN_IGNORE:
        /* Mapping was found but required attribute is missing */
        DEBUG(SSSDBG_TRACE_FUNC, "Ignoring [%s] because of missing attributes\n",
                                 state->current_member->dn);
        break;

both debug message and comment needs update.

Comment thread src/providers/ldap/sdap_async_nested_groups.c Outdated
Comment thread src/providers/ldap/sdap_async_nested_groups.c Outdated
Comment thread src/providers/ldap/sdap_async_nested_groups.c Outdated
@alexey-tikhonov alexey-tikhonov added the no-backport This should go to target branch only. label Mar 24, 2026
@alexey-tikhonov
Copy link
Copy Markdown
Member

Note: #8526 will need to cherry-pick this once merged.

@alexey-tikhonov alexey-tikhonov self-assigned this Mar 25, 2026
@alexey-tikhonov
Copy link
Copy Markdown
Member

@ondrejv2, please see comments inline.

@alexey-tikhonov
Copy link
Copy Markdown
Member

In sdap_nested_group_single_step_process()

    case SDAP_NESTED_GROUP_DN_IGNORE:
        /* Mapping was found but required attribute is missing */
        DEBUG(SSSDBG_TRACE_FUNC, "Ignoring [%s] because of missing attributes\n",
                                 state->current_member->dn);
        break;

both debug message and comment needs update.

@ondrejv2, this needs updates as 'SDAP_NESTED_GROUP_DN_IGNORE' now can also mean "filtered out intentionally".

Comment thread src/providers/ldap/sdap_async_nested_groups.c Outdated
Comment thread src/providers/ldap/sdap_async_nested_groups.c Outdated
@alexey-tikhonov
Copy link
Copy Markdown
Member

alexey-tikhonov commented Mar 26, 2026

@ondrejv2, thank you for updates.
The only open thread remains:
#8534 (comment)

@sumit-bose
Copy link
Copy Markdown
Contributor

Hi,

jfyi, I'm still fine with the latest version.

bye,
Sumit

@alexey-tikhonov
Copy link
Copy Markdown
Member

ACK

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
@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) (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🟢 ci / system (fedora-44) (success)
🟢 ci / system (fedora-45) (success)
➖ 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.

@alexey-tikhonov alexey-tikhonov merged commit 96d3823 into SSSD:master Mar 26, 2026
2 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accepted no-backport This should go to target branch only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants