Skip to content

[autobackport: sssd-2-9] Fix for test_access_control_simple__permits_user_login_based_on_group samba failure#8293

Merged
alexey-tikhonov merged 2 commits intoSSSD:sssd-2-9from
sssd-bot:SSSD-sssd-backport-pr8263-to-sssd-2-9
Dec 12, 2025
Merged

[autobackport: sssd-2-9] Fix for test_access_control_simple__permits_user_login_based_on_group samba failure#8293
alexey-tikhonov merged 2 commits intoSSSD:sssd-2-9from
sssd-bot:SSSD-sssd-backport-pr8263-to-sssd-2-9

Conversation

@sssd-bot
Copy link
Contributor

@sssd-bot sssd-bot commented Dec 10, 2025

This is an automatic backport of PR#8263 Fix for test_access_control_simple__permits_user_login_based_on_group samba failure to branch sssd-2-9, created by @justin-stephenson.

Please make sure this backport is correct.

Note

The commits were cherry-picked without conflicts.

You can push changes to this pull request

git remote add sssd-bot git@github.com:sssd-bot/sssd.git
git fetch sssd-bot refs/heads/SSSD-sssd-backport-pr8263-to-sssd-2-9
git checkout SSSD-sssd-backport-pr8263-to-sssd-2-9
git push sssd-bot SSSD-sssd-backport-pr8263-to-sssd-2-9 --force

Original commits
b0146ae - util: Add string_begins_with() helper
358a708 - simple: Resolve group names in SID format
4482fac - tests: Remove preferred topology from simple access test

Backported commits

  • 17292e2 - util: Add string_begins_with() helper
  • 989129e - simple: Resolve group names in SID format
  • 7106a0e - tests: Remove preferred topology from simple access test

Original Pull Request Body

gemini-code-assist[bot]
gemini-code-assist bot reviewed Dec 10, 2025
View reviewed changes
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 backports a fix for an issue where group names in SID format were not being resolved correctly, causing test failures with Samba. The changes introduce a new utility function string_begins_with() to detect SID-formatted names and adjust the group processing logic in simple_access_check. While the overall logic seems correct, I've found a critical issue in the new string_begins_with() function where it doesn't handle a NULL prefix, which could lead to a crash. I've also pointed out a missing test case for this scenario.

@alexey-tikhonov
Copy link
Member

alexey-tikhonov commented Dec 10, 2025

tests: Remove preferred topology from simple access test is empty against sssd-2-9, can be dropped.

@justin-stephenson
Copy link
Contributor

justin-stephenson commented Dec 10, 2025

tests: Remove preferred topology from simple access test is empty against sssd-2-9, can be dropped.

Dropped.

@justin-stephenson justin-stephenson force-pushed the SSSD-sssd-backport-pr8263-to-sssd-2-9 branch from 7106a0e to 989129e Compare December 10, 2025 16:26
sumit-bose
sumit-bose approved these changes Dec 12, 2025
View reviewed changes
Copy link
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,

backported patches are looking like to original ones, ACK.

bye,
Sumit

@alexey-tikhonov alexey-tikhonov removed their request for review December 12, 2025 14:30
@alexey-tikhonov alexey-tikhonov removed their assignment Dec 12, 2025
@alexey-tikhonov alexey-tikhonov added no-backport This should go to target branch only. Accepted labels Dec 12, 2025
@justin-stephenson @sssd-bot
util: Add string_begins_with() helper
4006db4 
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
(cherry picked from commit b0146ae)
@justin-stephenson @sssd-bot
simple: Resolve group names in SID format
d32f274 
After changes from 'Dont store GID for non-posix groups', the simple
access provider was not identifying group with names in SID format as
group that needs to be resolved because they are no longer stored
temporarily as non-POSIX.

Add code to check for, and resolve any group names which are SIDs
returned from initgroups (AD provider).

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
(cherry picked from commit 358a708)
@sssd-bot
Copy link
Contributor Author

sssd-bot commented Dec 12, 2025

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

🟢 CodeQL (success)
🟢 rpm-build:centos-stream-9-x86_64:upstream (success)
🟢 Build / make-distcheck (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-9) (success)
🟢 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 SSSD-sssd-backport-pr8263-to-sssd-2-9 branch from 989129e to d32f274 Compare December 12, 2025 14:31
@alexey-tikhonov alexey-tikhonov merged commit 10456f2 into SSSD:sssd-2-9 Dec 12, 2025
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sumit-bose sumit-bose sumit-bose approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@sumit-bose sumit-bose

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sssd-bot @alexey-tikhonov @justin-stephenson @sumit-bose