From 1bb4ff9ed2313fb3c2bd1418258c5bcec557b6a5 Mon Sep 17 00:00:00 2001

From: Rob Crittenden <rcritten@redhat.com>

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 <rcritten@redhat.com>

Reviewed-By: Francisco Trivino <ftrivino@redhat.com>

---

 .../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 a3f57cb4bd7a2a66d70fae98cca0f62a8f0c017f..345e1dee7d163167373ca82dedb1e827f0e1bc8c 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 6d66982848ac5a0061b47d30fad022be055c93e4..41d6e9070a90c2bde7b3182ad6ecf1a923bba203 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