From 1316cd8b2252c2543cf2ef2186956a8833037b1e Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 21 Jul 2022 09:28:46 -0400 Subject: [PATCH] Disabling gracelimit does not prevent LDAP binds Originally the code treated 0 as disabled. This was changed during the review process to -1 but one remnant was missed effetively allowing gracelimit 0 to also mean disabled. Add explicit tests for testing with gracelimit = 0 and gracelimit = -1. Also remove some extranous "str(self.master.domain.basedn)" lines from some of the tests. Fixes: https://pagure.io/freeipa/issue/9206 Signed-off-by: Rob Crittenden Reviewed-By: Francisco Trivino --- .../ipa-graceperiod/ipa_graceperiod.c | 2 +- ipatests/test_integration/test_pwpolicy.py | 55 ++++++++++++++++++- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-graceperiod/ipa_graceperiod.c b/daemons/ipa-slapi-plugins/ipa-graceperiod/ipa_graceperiod.c index a3f57cb4b..345e1dee7 100644 --- a/daemons/ipa-slapi-plugins/ipa-graceperiod/ipa_graceperiod.c +++ b/daemons/ipa-slapi-plugins/ipa-graceperiod/ipa_graceperiod.c @@ -479,7 +479,7 @@ static int ipagraceperiod_preop(Slapi_PBlock *pb) if (pwresponse_requested) { slapi_pwpolicy_make_response_control(pb, -1, grace_limit - grace_user_time , -1); } - } else if ((grace_limit > 0) && (grace_user_time >= grace_limit)) { + } else if (grace_user_time >= grace_limit) { LOG_TRACE("%s password is expired and out of grace limit\n", dn); errstr = "Password is expired.\n"; ret = LDAP_INVALID_CREDENTIALS; diff --git a/ipatests/test_integration/test_pwpolicy.py b/ipatests/test_integration/test_pwpolicy.py index 6d6698284..41d6e9070 100644 --- a/ipatests/test_integration/test_pwpolicy.py +++ b/ipatests/test_integration/test_pwpolicy.py @@ -36,7 +36,7 @@ class TestPWPolicy(IntegrationTest): cls.master.run_command(['ipa', 'group-add-member', POLICY, '--users', USER]) cls.master.run_command(['ipa', 'pwpolicy-add', POLICY, - '--priority', '1']) + '--priority', '1', '--gracelimit', '-1']) cls.master.run_command(['ipa', 'passwd', USER], stdin_text='{password}\n{password}\n'.format( password=PASSWORD @@ -265,7 +265,6 @@ class TestPWPolicy(IntegrationTest): def test_graceperiod_expired(self): """Test the LDAP bind grace period""" - str(self.master.domain.basedn) dn = "uid={user},cn=users,cn=accounts,{base_dn}".format( user=USER, base_dn=str(self.master.domain.basedn)) @@ -308,7 +307,6 @@ class TestPWPolicy(IntegrationTest): def test_graceperiod_not_replicated(self): """Test that the grace period is reset on password reset""" - str(self.master.domain.basedn) dn = "uid={user},cn=users,cn=accounts,{base_dn}".format( user=USER, base_dn=str(self.master.domain.basedn)) @@ -341,3 +339,54 @@ class TestPWPolicy(IntegrationTest): ) assert 'passwordgraceusertime: 0' in result.stdout_text.lower() self.reset_password(self.master) + + def test_graceperiod_zero(self): + """Test the LDAP bind with zero grace period""" + dn = "uid={user},cn=users,cn=accounts,{base_dn}".format( + user=USER, base_dn=str(self.master.domain.basedn)) + + self.master.run_command( + ["ipa", "pwpolicy-mod", POLICY, "--gracelimit", "0", ], + ) + + # Resetting the password will mark it as expired + self.reset_password(self.master) + + # Now grace is done and binds should fail. + result = self.master.run_command( + ["ldapsearch", "-e", "ppolicy", "-D", dn, + "-w", PASSWORD, "-b", dn], raiseonerr=False + ) + assert result.returncode == 49 + + assert 'Password is expired' in result.stderr_text + assert 'Password expired, 0 grace logins remain' in result.stderr_text + + def test_graceperiod_disabled(self): + """Test the LDAP bind with grace period disabled (-1)""" + str(self.master.domain.basedn) + dn = "uid={user},cn=users,cn=accounts,{base_dn}".format( + user=USER, base_dn=str(self.master.domain.basedn)) + + # This can fail if gracelimit is already -1 so ignore it + self.master.run_command( + ["ipa", "pwpolicy-mod", POLICY, "--gracelimit", "-1",], + raiseonerr=False, + ) + + # Ensure the password is expired + self.reset_password(self.master) + + result = self.kinit_as_user(self.master, PASSWORD, PASSWORD) + + for _i in range(0, 10): + result = self.master.run_command( + ["ldapsearch", "-e", "ppolicy", "-D", dn, + "-w", PASSWORD, "-b", dn] + ) + + # With graceperiod disabled it should not increment + result = tasks.ldapsearch_dm( + self.master, dn, ['passwordgraceusertime',], + ) + assert 'passwordgraceusertime: 0' in result.stdout_text.lower() -- 2.37.2