test: authentication, adding generic password policy tests #7728

danlavu · 2024-11-30T23:07:04Z

depends on roles, adding default domain password policy management sssd-test-framework#139
added generic password change tests
removed generic password change tests in test_ldap and made them only ppolicy tests, since generic now covers ldap
renamed test_ldap names.
updated assertions, client.auth.ssh.password_with_output, now returns a different value because of the mentioned pull request

spoore1

Couple of places missing ()'s I think.

src/tests/system/tests/test_authentication.py

danlavu · 2025-03-15T20:13:11Z

ests/test_ldap.py::test_ldap__ppolicy_change_password_with_complexity_requirement[exop] (ldap) 
tests/test_ldap.py::test_ldap__ppolicy_change_password_with_complexity_requirement[ldap_modify] (ldap) 
tests/test_ldap.py::test_ldap__ppolicy_change_password_with_complexity_requirement[exop_force] (ldap) 
tests/test_ldap.py::test_ldap__authenticate_user_with_whitespace_prefix_in_userid (ldap) 
tests/test_ldap.py::test_ldap__shadow_policy_change_password[su] (ldap) 
tests/test_ldap.py::test_ldap__shadow_policy_change_password[ssh] (ldap) 
tests/test_ldap.py::test_ldap__search_base_is_discovered_and_defaults_to_root_dse (ldap) 
tests/test_ldap.py::test_ldap__search_base_is_discovered_and_defaults_to_root_dse_users_groups_and_netgroups[ldap_user_search_base-ou=People,dc=ldap,dc=test] (ldap) 
tests/test_ldap.py::test_ldap__search_base_is_discovered_and_defaults_to_root_dse_users_groups_and_netgroups[ldap_group_search_base-ou=Groups,dc=ldap,dc=test] (ldap) 
tests/test_ldap.py::test_ldap__search_base_is_discovered_and_defaults_to_root_dse_users_groups_and_netgroups[ldap_netgroup_search_base-ou=Netgroup,dc=ldap,dc=test] (ldap) 
tests/test_ldap.py::test_ldap__lookup_user_with_search_bases (ldap) 
tests/test_ldap.py::test_ldap__lookup_and_authenticate_as_user_with_different_object_search_bases[dc=ldap,dc=test] (ldap) 
tests/test_ldap.py::test_ldap__lookup_and_authenticate_as_user_with_different_object_search_bases[dc=shanks,dc=com] (ldap) 
tests/test_ldap.py::test_ldap__password_change_no_grace_logins_left[exop-1-Expected login failure] (ldap) 
tests/test_ldap.py::test_ldap__password_change_no_grace_logins_left[exop_force-3-Expected password change request] (ldap) 
tests/test_ldap.py::test_ldap__empty_attribute (ldap) 
tests/test_ldap.py::test_ldap__limit_search_base_group (ldap) 


================ 17 passed, 670 deselected in 164.95s (0:02:44) ================
PASSED [  5%]PASSED [ 11%]PASSED [ 17%]PASSED [ 23%]PASSED [ 29%]PASSED [ 35%]PASSED [ 41%]PASSED [ 47%]PASSED [ 52%]PASSED [ 58%]PASSED [ 64%]PASSED [ 70%]PASSED [ 76%]PASSED [ 82%]PASSED [ 88%]PASSED             [ 94%]PASSED     [100%]
Process finished with exit code 0

Will post the test_authentication runs when they are done, but all the test are passing with SSSD/sssd-test-framework#139

danlavu · 2025-03-17T22:53:44Z

This is ready for review.

spoore1

Mostly comment/wording changes suggested with a few questions mixed in. Looks like a good set of tests for password policy.

src/tests/system/tests/test_authentication.py

src/tests/system/tests/test_ldap.py

src/tests/system/tests/test_authentication.py

* user is forced to changed password at login * user logins and issues a password change

* few scenarios have been removed * ppolicy tests have been made into ppolicy tests only, since normal ldap is covered by the generic provider now * renamed some of the test cases * removed su from a password change test * removed some test cases that are now covered by the new test cases

danlavu · 2025-04-08T21:27:42Z

I cancelled the CI run since they are going to fail. Here is the link to a successful run, the failure in Fedora 42 is not related.

https://github.com/SSSD/sssd/actions/runs/14335416822

spoore1

Mostly questions about docstring setup/steps wording.

src/tests/system/requirements.txt

spoore1 · 2025-04-08T21:17:28Z

src/tests/system/tests/test_authentication.py

+    :customerscenario: True
+    """
+    old_password = "Secret123"
+    invalid_password = "secret"


Should the variables match wording used in the docstrings (i.e. bad_password instead of invalid_password)?

spoore1 · 2025-04-08T21:25:22Z

src/tests/system/tests/test_authentication.py

+        3. Start SSSD
+    :steps:
+        1. Login as user
+        2. Issue password change as user with password that does not meet complexity requirements


Just a thought but if we're trying to simplify wording in docstrings, could we drop "as user with password" from this and the next step and have it still explain well enough what is happening?

spoore1 · 2025-04-08T21:28:35Z

src/tests/system/tests/test_ldap.py

+        LDAP modify.
+
+        The 'test_authentication__change_password' test is a generic provider test that already
+         covers  LDAP. This test is an edited copy that only tests LDAP with the ppolicy overlay.


Is "covers" indented an extra space?

spoore1 · 2025-04-08T22:08:13Z

src/tests/system/tests/test_ldap.py

-        2. Configure the LDAP ACI to permit user password changes
-        3. Set "ldap_pwmodify_mode"
-        4. Start SSSD
+        1. Create user 'user'


Were we going to suggest dropping specifics like 'user' from the docstring setup and steps?

spoore1 · 2025-04-08T22:11:09Z

src/tests/system/tests/test_ldap.py

-        2. Configure the LDAP ACI to permit user password changes
-        3. Set "ldap_pwmodify_mode"
-        4. Start SSSD
+        1. Create user 'user'


Do we need to state 'user' here or would removing that be in line with some of the simplification of steps/setup we've talked about?

spoore1 · 2025-04-08T22:14:10Z

src/tests/system/tests/test_ldap.py

        4. Start SSSD
    :steps:
-        1. Attempt to change the password but enter the incorrect password
+        1. Login as user
+        2. Issue password change as user with password that does not meet complexity requirements


Same question as above about shortening steps by removing "as user with password".

spoore1

After discussing with @danlavu , I'm approving this now since all of my latest questions/comments are related to docstring wording in some way. This can be addressed in a later effort.

alexey-tikhonov · 2025-04-09T14:23:13Z

F42 still fails test_ipa__hostpublickeys_by_ip (ipa) but this is probably another issue.

alexey-tikhonov · 2025-04-09T14:26:08Z

Pushed PR: #7728

master
- f8f7f84 - tests: removed overlapping test scenarios from authentication tests
- 08a3c41 - tests: adding generic password change tests
sssd-2-10
- f1a8634 - tests: removed overlapping test scenarios from authentication tests
- d678e67 - tests: adding generic password change tests

alexey-tikhonov · 2025-04-09T14:26:25Z

Conflict in sssd-2-9
Conflict in sssd-2-9-4

@danlavu, please open explicit backport requests.

danlavu added Tests branch: sssd-2-9 branch: sssd-2-10 branch: sssd-2-9-4 Corresponds to C8S labels Nov 30, 2024

danlavu marked this pull request as draft November 30, 2024 23:07

danlavu mentioned this pull request Nov 30, 2024

roles, adding default domain password policy management SSSD/sssd-test-framework#139

Merged

danlavu force-pushed the tests-authentication-password-changes branch 3 times, most recently from bfc5ed5 to aea7f4e Compare December 4, 2024 01:51

spoore1 requested changes Feb 5, 2025

View reviewed changes

src/tests/system/tests/test_authentication.py Outdated Show resolved Hide resolved

src/tests/system/tests/test_authentication.py Outdated Show resolved Hide resolved

spoore1 self-assigned this Feb 5, 2025

danlavu force-pushed the tests-authentication-password-changes branch from aea7f4e to 1242992 Compare February 5, 2025 22:49

danlavu force-pushed the tests-authentication-password-changes branch from 1242992 to 20248d2 Compare March 15, 2025 20:08

danlavu marked this pull request as ready for review March 15, 2025 20:12

danlavu marked this pull request as draft March 15, 2025 20:14

danlavu marked this pull request as ready for review March 17, 2025 22:42

danlavu force-pushed the tests-authentication-password-changes branch 2 times, most recently from bf3564c to a1a7718 Compare March 17, 2025 22:52

danlavu requested a review from aplopez March 17, 2025 22:53

danlavu assigned aplopez Mar 17, 2025

danlavu requested a review from spoore1 March 17, 2025 22:53

danlavu force-pushed the tests-authentication-password-changes branch from a1a7718 to 28554ea Compare March 18, 2025 17:05

spoore1 requested changes Mar 18, 2025

View reviewed changes

danlavu force-pushed the tests-authentication-password-changes branch 4 times, most recently from 1da3edb to 922826c Compare March 19, 2025 18:32

aplopez reviewed Mar 20, 2025

View reviewed changes

alexey-tikhonov added the Waiting for review label Apr 7, 2025

alexey-tikhonov requested a review from spoore1 April 7, 2025 13:14

danlavu force-pushed the tests-authentication-password-changes branch 6 times, most recently from 39c5577 to adb13f2 Compare April 7, 2025 23:31

Dan Lavu added 2 commits April 8, 2025 09:55

tests: adding generic password change tests

1ed87a1

* user is forced to changed password at login * user logins and issues a password change

danlavu force-pushed the tests-authentication-password-changes branch from adb13f2 to 000121d Compare April 8, 2025 13:56

spoore1 mentioned this pull request Apr 8, 2025

roles.ldap: moved the creation of an aci to the role setup method, making it a default configuration for all tests. SSSD/sssd-test-framework#170

Merged

danlavu force-pushed the tests-authentication-password-changes branch from 000121d to 1a08a2a Compare April 8, 2025 21:23

spoore1 reviewed Apr 8, 2025

View reviewed changes

spoore1 approved these changes Apr 8, 2025

View reviewed changes

danlavu added Accepted Ready to push Ready to push labels Apr 8, 2025

alexey-tikhonov removed the Waiting for review label Apr 9, 2025

alexey-tikhonov added Ready to push Ready to push and removed Ready to push Ready to push branch: sssd-2-9 branch: sssd-2-9-4 Corresponds to C8S labels Apr 9, 2025

alexey-tikhonov added Pushed and removed Accepted Ready to push Ready to push labels Apr 9, 2025

alexey-tikhonov closed this Apr 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: authentication, adding generic password policy tests #7728

test: authentication, adding generic password policy tests #7728

danlavu commented Nov 30, 2024 •

edited

Loading

spoore1 left a comment

danlavu commented Mar 15, 2025

danlavu commented Mar 17, 2025

spoore1 left a comment

danlavu commented Apr 8, 2025

spoore1 left a comment

spoore1 Apr 8, 2025

spoore1 Apr 8, 2025

spoore1 Apr 8, 2025

spoore1 Apr 8, 2025

spoore1 Apr 8, 2025

spoore1 Apr 8, 2025

spoore1 left a comment

alexey-tikhonov commented Apr 9, 2025

alexey-tikhonov commented Apr 9, 2025

alexey-tikhonov commented Apr 9, 2025

test: authentication, adding generic password policy tests #7728

test: authentication, adding generic password policy tests #7728

Conversation

danlavu commented Nov 30, 2024 • edited Loading

spoore1 left a comment

Choose a reason for hiding this comment

danlavu commented Mar 15, 2025

danlavu commented Mar 17, 2025

spoore1 left a comment

Choose a reason for hiding this comment

danlavu commented Apr 8, 2025

spoore1 left a comment

Choose a reason for hiding this comment

spoore1 Apr 8, 2025

Choose a reason for hiding this comment

spoore1 Apr 8, 2025

Choose a reason for hiding this comment

spoore1 Apr 8, 2025

Choose a reason for hiding this comment

spoore1 Apr 8, 2025

Choose a reason for hiding this comment

spoore1 Apr 8, 2025

Choose a reason for hiding this comment

spoore1 Apr 8, 2025

Choose a reason for hiding this comment

spoore1 left a comment

Choose a reason for hiding this comment

alexey-tikhonov commented Apr 9, 2025

alexey-tikhonov commented Apr 9, 2025

alexey-tikhonov commented Apr 9, 2025

danlavu commented Nov 30, 2024 •

edited

Loading