From 866f03de46617ee155d346a0d379761157dde7b4 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Wed, 22 Oct 2014 14:11:11 +1300 Subject: [PATCH] MDL-47800 auth: Add option to logout on pw change --- admin/settings/security.php | 3 +++ lang/en/admin.php | 2 ++ lib/classes/session/manager.php | 6 +++++- lib/tests/session_manager_test.php | 21 ++++++++++++++++++++- login/change_password.php | 4 ++++ login/lib.php | 3 +++ user/editadvanced.php | 6 ++++++ 7 files changed, 43 insertions(+), 2 deletions(-) diff --git a/admin/settings/security.php b/admin/settings/security.php index a75b6e1d01e..b455946eb04 100644 --- a/admin/settings/security.php +++ b/admin/settings/security.php @@ -86,6 +86,9 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page 1800, $pwresetoptions); $temp->add($adminsetting); + $temp->add(new admin_setting_configcheckbox('passwordchangelogout', + new lang_string('passwordchangelogout', 'admin'), + new lang_string('passwordchangelogout_desc', 'admin'), 0)); $temp->add(new admin_setting_configcheckbox('groupenrolmentkeypolicy', new lang_string('groupenrolmentkeypolicy', 'admin'), new lang_string('groupenrolmentkeypolicy_desc', 'admin'), 1)); $temp->add(new admin_setting_configcheckbox('disableuserimages', new lang_string('disableuserimages', 'admin'), new lang_string('configdisableuserimages', 'admin'), 0)); $temp->add(new admin_setting_configcheckbox('emailchangeconfirmation', new lang_string('emailchangeconfirmation', 'admin'), new lang_string('configemailchangeconfirmation', 'admin'), 1)); diff --git a/lang/en/admin.php b/lang/en/admin.php index 8f5ebf8a44f..a97f3cc4e09 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -773,6 +773,8 @@ $string['order1'] = 'First'; $string['order2'] = 'Second'; $string['order3'] = 'Third'; $string['order4'] = 'Fourth'; +$string['passwordchangelogout'] = 'Logout others after password change'; +$string['passwordchangelogout_desc'] = 'Terminate all other user sessions after change of password. Password changes from bulk user uploads are ignored.'; $string['passwordpolicy'] = 'Password policy'; $string['passwordresettime'] = 'Maximum time to validate password reset request'; $string['pathtoclam'] = 'clam AV path'; diff --git a/lib/classes/session/manager.php b/lib/classes/session/manager.php index 84e26eddbb3..18526963bf9 100644 --- a/lib/classes/session/manager.php +++ b/lib/classes/session/manager.php @@ -608,12 +608,16 @@ class manager { /** * Terminate all sessions of given user unconditionally. * @param int $userid + * @param string $keepsid keep this sid if present */ - public static function kill_user_sessions($userid) { + public static function kill_user_sessions($userid, $keepsid = null) { global $DB; $sessions = $DB->get_records('sessions', array('userid'=>$userid), 'id DESC', 'id, sid'); foreach ($sessions as $session) { + if ($keepsid and $keepsid === $session->sid) { + continue; + } self::kill_session($session->sid); } } diff --git a/lib/tests/session_manager_test.php b/lib/tests/session_manager_test.php index b9db594c0c2..909038952ed 100644 --- a/lib/tests/session_manager_test.php +++ b/lib/tests/session_manager_test.php @@ -304,7 +304,26 @@ class core_session_manager_testcase extends advanced_testcase { \core\session\manager::kill_user_sessions($userid); $this->assertEquals(1, $DB->count_records('sessions')); - $this->assertFalse($DB->record_exists('sessions', array('userid'=>$userid))); + $this->assertFalse($DB->record_exists('sessions', array('userid' => $userid))); + + $record->userid = $userid; + $record->sid = md5('pokus3'); + $DB->insert_record('sessions', $record); + + $record->userid = $userid; + $record->sid = md5('pokus4'); + $DB->insert_record('sessions', $record); + + $record->userid = $userid; + $record->sid = md5('pokus5'); + $DB->insert_record('sessions', $record); + + $this->assertEquals(3, $DB->count_records('sessions', array('userid' => $userid))); + + \core\session\manager::kill_user_sessions($userid, md5('pokus5')); + + $this->assertEquals(1, $DB->count_records('sessions', array('userid' => $userid))); + $this->assertEquals(1, $DB->count_records('sessions', array('userid' => $userid, 'sid' => md5('pokus5')))); } public function test_kill_all_sessions() { diff --git a/login/change_password.php b/login/change_password.php index cb433d5c5c4..eedfddff48f 100644 --- a/login/change_password.php +++ b/login/change_password.php @@ -115,6 +115,10 @@ if ($mform->is_cancelled()) { print_error('errorpasswordupdate', 'auth'); } + if (!empty($CFG->passwordchangelogout)) { + \core\session\manager::kill_user_sessions($USER->id, session_id()); + } + // Reset login lockout - we want to prevent any accidental confusion here. login_unlock_account($USER); diff --git a/login/lib.php b/login/lib.php index dd357f463bb..2996a4c6820 100644 --- a/login/lib.php +++ b/login/lib.php @@ -239,6 +239,9 @@ function core_login_process_password_set($token) { if (!$userauth->user_update_password($user, $data->password)) { print_error('errorpasswordupdate', 'auth'); } + if (!empty($CFG->passwordchangelogout)) { + \core\session\manager::kill_user_sessions($user->id, session_id()); + } // Reset login lockout (if present) before a new password is set. login_unlock_account($user); // Clear any requirement to change passwords. diff --git a/user/editadvanced.php b/user/editadvanced.php index 3843bcf4648..b276937f265 100644 --- a/user/editadvanced.php +++ b/user/editadvanced.php @@ -213,6 +213,12 @@ if ($usernew = $userform->get_data()) { print_error('cannotupdatepasswordonextauth', '', '', $usernew->auth); } unset_user_preference('create_password', $usernew); // Prevent cron from generating the password. + + if (!empty($CFG->passwordchangelogout)) { + // We can use SID of other user safely here because they are unique, + // the problem here is we do not want to logout admin here when changing own password. + \core\session\manager::kill_user_sessions($usernew->id, session_id()); + } } }