-
Notifications
You must be signed in to change notification settings - Fork 270
[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
base: sssd-2-9-4
Are you sure you want to change the base?
[autobackport: sssd-2-9-4] cache_req: use sysdb_search_user_by_upn_with_view_res() #8324
Conversation
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 <atikhono@redhat.com> Reviewed-by: Pavel Březina <pbrezina@redhat.com> (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 <atikhono@redhat.com> Reviewed-by: Pavel Březina <pbrezina@redhat.com> (cherry picked from commit 43f22b9)
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Pavel Březina <pbrezina@redhat.com> (cherry picked from commit fe61b85)
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Pavel Březina <pbrezina@redhat.com> (cherry picked from commit 6d8f9d7)
Add a system test to verify that IPA ID view overrides are correctly applied when looking up a user by email address. The test creates a user with an email, applies ID view overrides (login, uid, gid, home), and verifies that the overridden values are returned when looking up the user by: - original name - overridden name - email address Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com> Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Pavel Březina <pbrezina@redhat.com> (cherry picked from commit 6413f60)
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.
| } | ||
|
|
||
| *out_res = orig_obj; | ||
| return ret; |
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.
The function may incorrectly return ENOENT when an object is found but has no overrides. If sysdb_add_overrides_to_object returns ENOENT, it signifies that no override was found, but the original object (orig_obj) is still valid. In this case, the function should return EOK to indicate that the lookup was successful. Returning ENOENT would incorrectly signal to the caller that the user was not found.
return EOK;| <<<<<<< 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@example.com", 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@example.com", | ||
| ) | ||
|
|
||
| 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@example.com") | ||
| 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.
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")