[autobackport: sssd-2-9] adding subid test #8294
Conversation
(cherry picked from commit 449913a)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request backports a new test file, src/tests/system/tests/test_ipa.py, which adds a comprehensive suite of system tests for SSSD's integration with FreeIPA. The new tests cover a wide range of features, including SSH public keys, ID views, HBAC rules, smart card authentication, and the subordinate ID (subid) functionality mentioned in the PR title. While the majority of the tests are well-structured, I've identified two tests related to ID views that don't actually validate SSSD's behavior on the client. Instead, they test the IPA server's command-line interface. These tests should be either corrected to verify client-side functionality or removed as they are out of scope for the SSSD test suite.
| def test_ipa__idview_append_user_cert(client: Client, ipa: IPA, moduledatadir: str): | ||
| """ | ||
| :title: ID view overrides user certificate from file contents, but value is appended | ||
| :setup: | ||
| 1. Create ID view and apply view to client | ||
| 2. Add a user that overrides the user's certificate | ||
| :steps: | ||
| 1. Look up user certificate | ||
| :expectedresults: | ||
| 1. Certificate contains expected data and matches file contents | ||
| :customerscenario: False | ||
| """ | ||
| ipa.idview("testview1").add(description="This is a new view") | ||
| ipa.idview("testview1").apply(hosts=[f"{client.host.hostname}"]) | ||
|
|
||
| with open(f"{moduledatadir}/certificate") as f: | ||
| certificate_content = f.read().strip() | ||
|
|
||
| ipa.user("user-1").add().iduseroverride().add_override( | ||
| "testview1", | ||
| certificate=certificate_content, | ||
| ) | ||
|
|
||
| client.sssd.restart() | ||
|
|
||
| result = ipa.user("user-1").iduseroverride().show_override("testview1") | ||
|
|
||
| assert certificate_content in result.get("usercertificate", [""])[0], "Certificate content mismatch!" |
There was a problem hiding this comment.
This test verifies that a user certificate override is correctly stored on the IPA server by using the ipa ... show_override command. However, it fails to test the SSSD component on the client. A proper test should verify that SSSD correctly retrieves and makes this overridden certificate available on the client machine. This could be verified using a client-side tool like sssctl user-show user-1 and checking for the certificate, or by attempting an operation that requires the certificate on the client (e.g., smartcard login). As it stands, the test provides a false sense of security that the SSSD feature is working, while only testing the IPA server's configuration storage.
| def test_ipa__idview_fails_to_apply_on_ipa_master(ipa: IPA): | ||
| """ | ||
| :title: ID views does not work on IPA master | ||
| :setup: | ||
| 1. Add IPA ID view with description | ||
| :steps: | ||
| 1. Apply ID view to IPA master | ||
| :expectedresults: | ||
| 1. Applying ID view fails | ||
| :customerscenario: False | ||
| """ | ||
| ipa.idview("testview1").add(description="This is a new view") | ||
| result = ipa.idview("testview1").apply(hosts=f"{ipa.host.hostname}") | ||
|
|
||
| assert result.rc == 1, "An IPA ID view should not apply on server!" | ||
|
|
||
| assert ( | ||
| "ID View cannot be applied to IPA master" in result.stdout | ||
| ), "Did not get an error message when trying to apply ID view on server!" |
There was a problem hiding this comment.
This test is designed to check the behavior of the ipa command-line tool on the IPA server, specifically that an ID View cannot be applied to the master itself. This test does not involve or validate any SSSD functionality on the client. While this might be a valid test for the FreeIPA project, it is out of scope for the SSSD test suite and should be removed.
|
@alexey-tikhonov Did you want this subid test ported to 2-9? |
4 participants
This is an automatic backport of PR#8225 adding subid test to branch sssd-2-9, created by @danlavu.
Caution
@danlavu 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
449913a - adding subid test
Backported commits
Conflicting Files Information (check for deleted and re-added files)
Original Pull Request Body