Ticket 48813 - password history is not updated when an admin resets the password

Bug Description:  When an admin resets a password the current password is not
                  stored in the password history. This incorrectly allows the
                  user to reuse the previous password after the reset.

Fix Description:  When a password is being reset by an "admin", still grab the
                  old password so we can correctly update the password history.

https://fedorahosted.org/389/ticket/48813

Reviewed by: nhosoi(Thanks!)

(cherry picked from commit 9c310b09c481a32b1a012371c688c65156b33472)
(cherry picked from commit b357e443d06f32bcad2f410868299d43153dca62)

dirsrvtests/tests/suites/password/pwp_history_test.py

@@ -0,0 +1,264 @@
+import os
+import ldap
+import logging
+import pytest
+from lib389 import DirSrv, Entry
+from lib389._constants import *
+from lib389.properties import *
+from lib389.tasks import *
+from lib389.utils import *
+
+logging.getLogger(__name__).setLevel(logging.DEBUG)
+log = logging.getLogger(__name__)
+
+
+class TopologyStandalone(object):
+    """ Topology class """
+    def __init__(self, standalone):
+        """ init """
+        standalone.open()
+        self.standalone = standalone
+
+
[email protected](scope="module")
+def topology(request):
+    """
+    Creating standalone instance ...
+    """
+    standalone = DirSrv(verbose=False)
+    args_instance[SER_HOST] = HOST_STANDALONE
+    args_instance[SER_PORT] = PORT_STANDALONE
+    args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE
+    args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX
+    args_standalone = args_instance.copy()
+    standalone.allocate(args_standalone)
+    instance_standalone = standalone.exists()
+    if instance_standalone:
+        standalone.delete()
+    standalone.create()
+    standalone.open()
+
+    # Delete each instance in the end
+    def fin():
+        """ Clean up instance """
+        standalone.delete()
+    request.addfinalizer(fin)
+
+    # Clear out the tmp dir
+    standalone.clearTmpDir(__file__)
+
+    return TopologyStandalone(standalone)
+
+
+def test_pwp_history_test(topology):
+    """
+    Test password policy history feature:
+        - Test password history is enforced
+        - Test password history works after an Admin resets the password
+        - Test that the correct number of passwords are stored in history
+    """
+
+    USER_DN = 'uid=testuser,' + DEFAULT_SUFFIX
+
+    #
+    # Configure password history policy and add a test user
+    #
+    try:
+        topology.standalone.modify_s("cn=config",
+                                     [(ldap.MOD_REPLACE,
+                                       'passwordHistory', 'on'),
+                                      (ldap.MOD_REPLACE,
+                                       'passwordInHistory', '3'),
+                                      (ldap.MOD_REPLACE,
+                                       'passwordChange', 'on'),
+                                      (ldap.MOD_REPLACE,
+                                       'passwordStorageScheme', 'CLEAR')])
+        log.info('Configured password policy.')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to configure password policy: ' + str(e))
+        assert False
+
+    try:
+        topology.standalone.add_s(Entry((USER_DN, {
+                                  'objectclass': ['top', 'extensibleObject'],
+                                  'sn': 'user',
+                                  'cn': 'test user',
+                                  'uid': 'testuser',
+                                  'userpassword': 'password'})))
+    except ldap.LDAPError as e:
+        log.fatal('Failed to add test user' + USER_DN + ': error ' + str(e))
+        assert False
+
+    #
+    # Test that password history is enforced.
+    #
+    try:
+        topology.standalone.simple_bind_s(USER_DN, 'password')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to bind as user: ' + str(e))
+        assert False
+
+    # Attempt to change password to the same password
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword', 'password')])
+        log.info('Incorrectly able to to set password to existing password.')
+        assert False
+    except ldap.CONSTRAINT_VIOLATION:
+        log.info('Password change correctly rejected')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to attempt to change password: ' + str(e))
+        assert False
+
+    #
+    # Keep changing password until we fill the password history (3)
+    #
+
+    # password1
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword', 'password1')])
+    except ldap.LDAPError as e:
+        log.fatal('Failed to change password: ' + str(e))
+        assert False
+    try:
+        topology.standalone.simple_bind_s(USER_DN, 'password1')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to bind as user using "password1": ' + str(e))
+        assert False
+
+    # password2
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword', 'password2')])
+    except ldap.LDAPError as e:
+        log.fatal('Failed to change password: ' + str(e))
+        assert False
+    try:
+        topology.standalone.simple_bind_s(USER_DN, 'password2')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to bind as user using "password2": ' + str(e))
+        assert False
+
+    # password3
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword', 'password3')])
+    except ldap.LDAPError as e:
+        log.fatal('Failed to change password: ' + str(e))
+        assert False
+    try:
+        topology.standalone.simple_bind_s(USER_DN, 'password3')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to bind as user using "password3": ' + str(e))
+        assert False
+
+    # password4
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword', 'password4')])
+    except ldap.LDAPError as e:
+        log.fatal('Failed to change password: ' + str(e))
+        assert False
+    try:
+        topology.standalone.simple_bind_s(USER_DN, 'password4')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to bind as user using "password4": ' + str(e))
+        assert False
+
+    #
+    # Check that we only have 3 passwords stored in history\
+    #
+    try:
+        entry = topology.standalone.search_s(USER_DN, ldap.SCOPE_BASE,
+                                             'objectclass=*',
+                                             ['passwordHistory'])
+        pwds = entry[0].getValues('passwordHistory')
+        if len(pwds) != 3:
+            log.fatal('Incorrect number of passwords stored in histry: %d' %
+                      len(pwds))
+            assert False
+        else:
+            log.info('Correct number of passwords found in history.')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to get user entry: ' + str(e))
+        assert False
+
+    #
+    # Attempt to change the password to previous passwords
+    #
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword', 'password1')])
+        log.info('Incorrectly able to to set password to previous password1.')
+        assert False
+    except ldap.CONSTRAINT_VIOLATION:
+        log.info('Password change correctly rejected')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to attempt to change password: ' + str(e))
+        assert False
+
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword', 'password2')])
+        log.info('Incorrectly able to to set password to previous password2.')
+        assert False
+    except ldap.CONSTRAINT_VIOLATION:
+        log.info('Password change correctly rejected')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to attempt to change password: ' + str(e))
+        assert False
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword', 'password3')])
+        log.info('Incorrectly able to to set password to previous password3.')
+        assert False
+    except ldap.CONSTRAINT_VIOLATION:
+        log.info('Password change correctly rejected')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to attempt to change password: ' + str(e))
+        assert False
+
+    #
+    # Reset password by Directory Manager(admin reset)
+    #
+    try:
+        topology.standalone.simple_bind_s(DN_DM, PASSWORD)
+    except ldap.LDAPError as e:
+        log.fatal('Failed to bind as rootDN: ' + str(e))
+        assert False
+
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword',
+                                                'password-reset')])
+    except ldap.LDAPError as e:
+        log.fatal('Failed to attempt to reset password: ' + str(e))
+        assert False
+
+    # Try and change the password to the previous password before the reset
+    try:
+        topology.standalone.simple_bind_s(USER_DN, 'password-reset')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to bind as user: ' + str(e))
+        assert False
+
+    try:
+        topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE,
+                                                'userpassword', 'password4')])
+        log.info('Incorrectly able to to set password to previous password4.')
+        assert False
+    except ldap.CONSTRAINT_VIOLATION:
+        log.info('Password change correctly rejected')
+    except ldap.LDAPError as e:
+        log.fatal('Failed to attempt to change password: ' + str(e))
+        assert False
+
+    log.info('Test suite PASSED.')
+
+
+if __name__ == '__main__':
+    # Run isolated
+    # -s for DEBUG mode
+    CURRENT_FILE = os.path.realpath(__file__)
+    pytest.main("-s %s" % CURRENT_FILE)

ldap/servers/slapd/modify.c

@@ -1259,6 +1259,10 @@ static int op_shared_allow_pw_change (Slapi_PBlock *pb, LDAPMod *mod, char **old
 		 * just return success.
 		 */
 		if(pw_is_pwp_admin(pb, pwpolicy)){
+			if (!SLAPI_IS_MOD_DELETE(mod->mod_op) && pwpolicy->pw_history){
+				/* Updating pw history, get the old password */
+				get_old_pw(pb, &sdn, old_pw);
+			}
 			rc = 1;
 			goto done;
 		}

ldap/servers/slapd/proto-slap.h

@@ -906,6 +906,7 @@ int check_pw_syntax( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals,
 	char **old_pw, Slapi_Entry *e, int mod_op );
 int check_pw_syntax_ext( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals,
 	char **old_pw, Slapi_Entry *e, int mod_op, Slapi_Mods *smods );
+void get_old_pw( Slapi_PBlock *pb, const Slapi_DN *sdn, char **old_pw);
 int check_account_lock( Slapi_PBlock *pb, Slapi_Entry * bind_target_entry, int pwresponse_req, int account_inactivation_only /*no wire/no pw policy*/);
 int check_pw_minage( Slapi_PBlock *pb, const Slapi_DN *sdn, struct berval **vals) ;
 void add_password_attrs( Slapi_PBlock *pb, Operation *op, Slapi_Entry *e );

ldap/servers/slapd/pw.c

@@ -1084,6 +1084,35 @@ retry:
 
 }
 
+/*
+ * Get the old password -used by password admin so we properly
+ * update pw history when reseting a password.
+ */
+void
+get_old_pw( Slapi_PBlock *pb, const Slapi_DN *sdn, char **old_pw )
+{
+    Slapi_Entry *e = NULL;
+    Slapi_Value **va = NULL;
+    Slapi_Attr *attr = NULL;
+    char *dn = (char*)slapi_sdn_get_ndn(sdn);
+
+    e = get_entry ( pb, dn );
+    if ( e == NULL ) {
+        return;
+    }
+
+    /* get current password, and remember it  */
+    attr = attrlist_find(e->e_attrs, "userpassword");
+    if ( attr && !valueset_isempty(&attr->a_present_values) ) {
+        va = valueset_get_valuearray(&attr->a_present_values);
+        *old_pw = slapi_ch_strdup(slapi_value_get_string(va[0]));
+    } else {
+        *old_pw = NULL;
+    }
+
+    slapi_entry_free(e);
+}
+
 /*
  * Basically, h0 and h1 must be longer than GENERALIZED_TIME_LENGTH.
  */