-
Notifications
You must be signed in to change notification settings - Fork 271
[autobackport: sssd-2-9-4] cache_req: use sysdb_search_user_by_upn_with_view_res() #8324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[autobackport: sssd-2-9-4] cache_req: use sysdb_search_user_by_upn_with_view_res() #8324
Conversation
There was a problem hiding this 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 changes to improve user lookups by UPN or email address when using ID views. It introduces a new function sysdb_search_user_by_upn_with_view_res and updates the cache_req plugin to use it, ensuring that user overrides are correctly applied. The old sysdb_getpwupn function is removed. My review has identified a critical logical issue in the new function that could lead to lookup failures, and an unresolved merge conflict in one of the test files that needs to be addressed.
| <<<<<<< HEAD | ||
| ======= | ||
| result = client.tools.getent.passwd("user1") | ||
| assert result is not None, "User 'user1' not found!" | ||
| assert result.uid == 999999, f"User's uid {result.uid} does not match override value!" | ||
| assert result.gid == 888888, f"User's gid {result.gid} does not match override value!" | ||
| assert result.home == "/home/o-user1", "User's homedir does not match override value!" | ||
|
|
||
| result = client.tools.getent.passwd("o-user1") | ||
| assert result is not None, "User 'o-user1' not found by override name!" | ||
| assert result.uid == 999999, f"Local override uid {result.uid} does not match override value!" | ||
| assert result.gid == 888888, f"Local override gid {result.gid} does not match override value!" | ||
| assert result.home == "/home/o-user1", "User's override name homedir does not match override value!" | ||
|
|
||
|
|
||
| @pytest.mark.importance("high") | ||
| @pytest.mark.topology([KnownTopology.LDAP, KnownTopology.AD, KnownTopology.Samba]) | ||
| def test_sss_overrides__overriding_username_and_attributes_lookup_by_email(client: Client, provider: GenericProvider): | ||
| """ | ||
| :title: Locally overriding the name and POSIX attributes of a user and lookup with the email address | ||
| :setup: | ||
| 1. Create POSIX user "user1" with email "[email protected]", email is | ||
| used because the UPN is not supported in all roles of the framework | ||
| 2. Configure SSSD with "ldap_id_mapping = false" and start SSSD | ||
| 3. Create local override for "user1" | ||
| 4. Restart SSSD, this is necessary to enable local overrides | ||
| :steps: | ||
| 1. Lookup user by the original name, check the uid and gid | ||
| 2. Lookup user by the overridden name, check the uid and gid | ||
| 3. Lookup user by email, check the uid and gid | ||
| :expectedresults: | ||
| 1. User is found and uid and gid match new values | ||
| 2. User is found and uid and gid match new values | ||
| 3. User is found and uid and gid match new values | ||
| :customerscenario: True | ||
| """ | ||
| provider.user("user1").add( | ||
| uid=999011, | ||
| gid=999011, | ||
| home="/home/user1", | ||
| gecos="user", | ||
| shell="/bin/bash", | ||
| password="Secret123", | ||
| email="[email protected]", | ||
| ) | ||
|
|
||
| client.sssd.domain["ldap_id_mapping"] = "False" | ||
| client.sssd.start() | ||
|
|
||
| client.sss_override.user("user1").add(name="o-user1", uid=999999, gid=888888, home="/home/o-user1") | ||
|
|
||
| client.sssd.restart() | ||
|
|
||
| result = client.tools.getent.passwd("user1") | ||
| assert result is not None, "User not found!" | ||
| assert result.uid == 999999, "User's uid does not match override value!" | ||
| assert result.gid == 888888, "User's gid does not match override value!" | ||
| assert result.home == "/home/o-user1", "User's homedir does not match override value!" | ||
|
|
||
| result = client.tools.getent.passwd("o-user1") | ||
| assert result is not None, "User not found by override name!" | ||
| assert result.uid == 999999, "Local override uid does not match override value!" | ||
| assert result.gid == 888888, "Local override gid does not match override value!" | ||
| assert result.home == "/home/o-user1", "User's override name homedir does not match override value!" | ||
|
|
||
| result = client.tools.getent.passwd("[email protected]") | ||
| assert result is not None, "User not found by email!" | ||
| assert result.uid == 999999, "Local override uid does not match override value!" | ||
| assert result.gid == 888888, "Local override gid does not match override value!" | ||
| assert result.home == "/home/o-user1", "User's override name homedir does not match override value!" | ||
|
|
||
|
|
||
| @pytest.mark.importance("high") | ||
| @pytest.mark.topology([KnownTopology.LDAP, KnownTopology.AD, KnownTopology.Samba]) | ||
| @pytest.mark.preferred_topology(KnownTopology.LDAP) | ||
| def test_sss_override__group_attributes_and_members(client: Client, provider: GenericProvider): | ||
| """ | ||
| :title: Override group attributes and members | ||
| :setup: | ||
| 1. Create user and group, make the user a group member | ||
| 2. Configure SSSD with "ldap_id_mapping = false" and start SSSD | ||
| 3. Create local user and group override and restart SSSD | ||
| :steps: | ||
| 1. Lookup group name and overridden name, check gid and group members | ||
| 2. Update group gid value for override, check gid and group members | ||
| :expectedresults: | ||
| 1. Group is found by both names, gid matches override and user is a member | ||
| 2. Group is found by both names, gid matches new override value and user is a member | ||
| :customerscenario: False | ||
| """ | ||
| user = provider.user("user1").add(uid=777777, gid=777777) | ||
| provider.group("group1").add(gid=999999).add_member(user) | ||
| client.sssd.domain["ldap_id_mapping"] = "False" | ||
| client.sssd.start() | ||
|
|
||
| client.sss_override.user("user1").add(name="o-user1") | ||
| client.sss_override.group("group1").add(name="o-group1") | ||
| client.sssd.restart() | ||
|
|
||
| for i in ["group1", "o-group1"]: | ||
| result = client.tools.getent.group(i) | ||
| assert result is not None, f"Group {i} not found!" | ||
| assert result.gid == 999999, f"Group gid {result.gid} does not match original value!" | ||
| assert "o-user1" in result.members, "Local override user 'o-user1' not found in group!" | ||
|
|
||
| client.sss_override.group("group1").add(name="o-group1", gid=888888) | ||
| for i in ["group1", "o-group1"]: | ||
| result = client.tools.getent.group(i) | ||
| assert result is not None, f"Group not {i} found!" | ||
| assert result.gid == 888888, f"Group gid {result.gid} does not match override value!" | ||
| assert "o-user1" in result.members, "Local override username 'o-user1' not found in group!" | ||
|
|
||
|
|
||
| @pytest.mark.importance("high") | ||
| @pytest.mark.topology([KnownTopology.LDAP, KnownTopology.AD, KnownTopology.Samba]) | ||
| @pytest.mark.preferred_topology(KnownTopology.LDAP) | ||
| def test_sss_override__root_user_cannot_be_used(client: Client, provider: GenericProvider): | ||
| """ | ||
| :title: Root user cannot be used for overrides | ||
| :setup: | ||
| 1. Create users, one named root | ||
| 2. Configure SSSD with "ldap_id_mapping = false", "use_fully_qualified_names = False" and start SSSD | ||
| 3. Override username to root and set override attributes to root values | ||
| 4. Restart SSSD | ||
| :steps: | ||
| 1. Lookup the root user and check his uid and gid | ||
| 2. Lookup the root user and use the sss service | ||
| 3. Lookup user and check his uid and gid | ||
| :expectedresults: | ||
| 1. The root user uid and gid has not been modified | ||
| 2. root user is not found | ||
| 3. User found and uid and gid is not roots | ||
| :customerscenario: False | ||
| """ | ||
| provider.user("user1").add(uid=999011, gid=999011) | ||
| provider.user("root").add(uid=999012, gid=999012) | ||
| client.sssd.domain["ldap_id_mapping"] = "False" | ||
| client.sssd.domain["use_fully_qualified_names"] = "False" | ||
| client.sssd.start() | ||
| >>>>>>> 6d8f9d7e9 (tests: lookup user with overrides with email) | ||
| client.sss_override.user("user1").add(name="root", uid=0, gid=0) | ||
|
|
||
| client.sssd.restart() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Please also include #8325 |
65ff1eb to
7084989
Compare
|
7084989 to
9a0c114
Compare
The new call will apply overrides to a user object which was searched by UPN or email address before returning it. Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Pavel Březina <[email protected]> (cherry picked from commit 794e80f)
To make sure any overrides are applied to the user even when searched by UPN or email address sysdb_search_user_by_upn_with_view_res() is now used in the cache request code. Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Pavel Březina <[email protected]> (cherry picked from commit 43f22b9)
Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Pavel Březina <[email protected]> (cherry picked from commit fe61b85)
Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Pavel Březina <[email protected]> (cherry picked from commit 6d8f9d7)
In sysdb_search_user_by_upn_with_view_res() sysdb_add_overrides_to_object() can return ENOENT if there is no id-override for the given user. This is expected and should not be treated as an error. Reviewed-by: Alexey Tikhonov <[email protected]> (cherry picked from commit 72a42d5)
|
The pull request was accepted by @alexey-tikhonov with the following PR CI status: 🟢 CodeQL (success) There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging. |
9a0c114 to
738db18
Compare
This is an automatic backport of PR#7998 cache_req: use sysdb_search_user_by_upn_with_view_res() to branch sssd-2-9-4, created by @sumit-bose.
Caution
@sumit-bose The patches did not apply cleanly. It is necessary to resolve conflicts before merging this pull request. Commits that introduced conflict are marked with
CONFLICT!.You can push changes to this pull request
Original commits
794e80f - sysdb: add sysdb_search_user_by_upn_with_view_res()
43f22b9 - cache_req: use sysdb_search_user_by_upn_with_view_res()
fe61b85 - sysdb:: remove sysdb_getpwupn()
6d8f9d7 - tests: lookup user with overrides with email
6413f60 - tests: add IPA ID view test for user lookup by email
Backported commits
Conflicting Files Information (check for deleted and re-added files)
On branch SSSD-sssd-backport-pr7998-to-sssd-2-9-4
You are currently cherry-picking commit 6413f60.
(fix conflicts and run "git cherry-pick --continue")
(use "git cherry-pick --skip" to skip this patch)
(use "git cherry-pick --abort" to cancel the cherry-pick operation)
Unmerged paths:
(use "git add/rm ..." as appropriate to mark resolution)
deleted by us: src/tests/system/tests/test_ipa.py
no changes added to commit (use "git add" and/or "git commit -a")