From b28247fe904d90530deef8ae6f51cc8772a89e62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Thu, 3 Jan 2013 23:29:43 +0100 Subject: [PATCH 1/2] MDL-21342 add user login lockout --- admin/settings/security.php | 5 + admin/user.php | 14 +++ auth/ldap/auth.php | 2 +- lang/en/admin.php | 23 ++++ lib/authlib.php | 191 ++++++++++++++++++++++++++++++++ lib/deprecatedlib.php | 20 ++++ lib/moodlelib.php | 74 +++++++------ lib/tests/authlib_test.php | 194 +++++++++++++++++++++++++++++++++ login/change_password.php | 4 + login/change_password_form.php | 6 +- login/forgot_password.php | 8 +- login/index.php | 4 - login/unlock_account.php | 54 +++++++++ 13 files changed, 550 insertions(+), 49 deletions(-) create mode 100644 lib/tests/authlib_test.php create mode 100644 login/unlock_account.php diff --git a/admin/settings/security.php b/admin/settings/security.php index 189bf87c329ca..94b379d171e2e 100644 --- a/admin/settings/security.php +++ b/admin/settings/security.php @@ -59,6 +59,11 @@ $temp->add(new admin_setting_configcheckbox('cronclionly', new lang_string('cronclionly', 'admin'), new lang_string('configcronclionly', 'admin'), 0)); $temp->add(new admin_setting_configpasswordunmask('cronremotepassword', new lang_string('cronremotepassword', 'admin'), new lang_string('configcronremotepassword', 'admin'), '')); + $options = array(0=>get_string('no'), 3=>3, 5=>5, 7=>7, 10=>10, 20=>20, 30=>30, 50=>50, 100=>100); + $temp->add(new admin_setting_configselect('lockoutthreshold', new lang_string('lockoutthreshold', 'admin'), new lang_string('lockoutthreshold_desc', 'admin'), 0, $options)); + $temp->add(new admin_setting_configduration('lockoutwindow', new lang_string('lockoutwindow', 'admin'), new lang_string('lockoutwindow_desc', 'admin'), 60*30)); + $temp->add(new admin_setting_configduration('lockoutduration', new lang_string('lockoutduration', 'admin'), new lang_string('lockoutduration_desc', 'admin'), 60*30)); + $temp->add(new admin_setting_configcheckbox('passwordpolicy', new lang_string('passwordpolicy', 'admin'), new lang_string('configpasswordpolicy', 'admin'), 1)); $temp->add(new admin_setting_configtext('minpasswordlength', new lang_string('minpasswordlength', 'admin'), new lang_string('configminpasswordlength', 'admin'), 8, PARAM_INT)); $temp->add(new admin_setting_configtext('minpassworddigits', new lang_string('minpassworddigits', 'admin'), new lang_string('configminpassworddigits', 'admin'), 1, PARAM_INT)); diff --git a/admin/user.php b/admin/user.php index 97f1abba14704..aac71123630fb 100644 --- a/admin/user.php +++ b/admin/user.php @@ -2,6 +2,7 @@ require_once('../config.php'); require_once($CFG->libdir.'/adminlib.php'); + require_once($CFG->libdir.'/authlib.php'); require_once($CFG->dirroot.'/user/filters/lib.php'); $delete = optional_param('delete', 0, PARAM_INT); @@ -16,6 +17,7 @@ $acl = optional_param('acl', '0', PARAM_INT); // id of user to tweak mnet ACL (requires $access) $suspend = optional_param('suspend', 0, PARAM_INT); $unsuspend = optional_param('unsuspend', 0, PARAM_INT); + $unlock = optional_param('unlock', 0, PARAM_INT); admin_externalpage_setup('editusers'); @@ -32,6 +34,7 @@ $strshowallusers = get_string('showallusers'); $strsuspend = get_string('suspenduser', 'admin'); $strunsuspend = get_string('unsuspenduser', 'admin'); + $strunlock = get_string('unlockaccount', 'admin'); $strconfirm = get_string('confirm'); if (empty($CFG->loginhttps)) { @@ -143,6 +146,14 @@ } } redirect($returnurl); + + } else if ($unlock and confirm_sesskey()) { + require_capability('moodle/user:update', $sitecontext); + + if ($user = $DB->get_record('user', array('id'=>$unlock, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0))) { + login_unlock_account($user); + } + redirect($returnurl); } // create the user filter form @@ -303,6 +314,9 @@ } } + if (login_is_lockedout($user)) { + $buttons[] = html_writer::link(new moodle_url($returnurl, array('unlock'=>$user->id, 'sesskey'=>sesskey())), html_writer::empty_tag('img', array('src'=>$OUTPUT->pix_url('t/unlock'), 'alt'=>$strunlock, 'class'=>'iconsmall')), array('title'=>$strunlock)); + } } } diff --git a/auth/ldap/auth.php b/auth/ldap/auth.php index e8abd12ce3f1e..d2730edbf356d 100644 --- a/auth/ldap/auth.php +++ b/auth/ldap/auth.php @@ -1643,7 +1643,7 @@ function ntlmsso_finish() { $username = $cf[$key]; // Here we want to trigger the whole authentication machinery // to make sure no step is bypassed... - $user = authenticate_user_login($username, $key); + $user = authenticate_user_login($username, $key, false); if ($user) { add_to_log(SITEID, 'user', 'login', "view.php?id=$USER->id&course=".SITEID, $user->id, 0, $user->id); diff --git a/lang/en/admin.php b/lang/en/admin.php index 42835f0f580b3..36b1997b16611 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -625,6 +625,28 @@ $string['location'] = 'Location'; $string['locationsettings'] = 'Location settings'; $string['locked'] = 'locked'; +$string['lockoutduration'] = 'Account lockout duration'; +$string['lockoutduration_desc'] = 'Locked out account is automatically unlocked after this duration.'; +$string['lockoutemailbody'] = 'Your account with username {$a->username} on server \'{$a->sitename}\' +was locked out after multiple invalid login attempts. + +To unlock the account immediately go to the following address + +{$a->link} + +In most mail programs, this should appear as a blue link +which you can just click on. If that doesn\'t work, +then cut and paste the address into the address +line at the top of your web browser window. + +If you need help, please contact the site administrator, +{$a->admin}'; +$string['lockoutemailsubject'] = 'Your account on {$a} was locked out'; +$string['lockouterrorunlock'] = 'Invalid account unlock information supplied.'; +$string['lockoutthreshold'] = 'Account lockout threshold'; +$string['lockoutthreshold_desc'] = 'Select number of failed login attempts that result in account lockout. This feature may be abused in denial of service attacks.'; +$string['lockoutwindow'] = 'Account lockout observation window'; +$string['lockoutwindow_desc'] = 'Observation time for lockout threshold, if there are no failed attempts the threshold counter is reset after this time.'; $string['log'] = 'Logs'; $string['logguests'] = 'Log guest access'; $string['logguests_help'] = 'This setting enables logging of actions by guest account and not logged in users. High profile sites may want to disable this logging for performance reasons. It is recommended to keep this setting enabled on production sites.'; @@ -989,6 +1011,7 @@ $string['unicoderecommended'] = 'Storing all your data in Unicode (UTF-8) is recommended. New installations should be performed into databases that have their default character set as Unicode. If you are upgrading, you should perform the UTF-8 migration process (see the Admin page).'; $string['unicoderequired'] = 'It is required that you store all your data in Unicode format (UTF-8). New installations must be performed into databases that have their default character set as Unicode. If you are upgrading, you should perform the UTF-8 migration process (see the Admin page).'; $string['uninstallplugin'] = 'Uninstall'; +$string['unlockaccount'] = 'Unlock account'; $string['unsettheme'] = 'Unset theme'; $string['unsupported'] = 'Unsupported'; $string['unsuspenduser'] = 'Activate user account'; diff --git a/lib/authlib.php b/lib/authlib.php index 7f691b75a5eef..f8aa820261247 100644 --- a/lib/authlib.php +++ b/lib/authlib.php @@ -61,6 +61,22 @@ define('AUTH_REMOVEUSER_SUSPEND', 1); define('AUTH_REMOVEUSER_FULLDELETE', 2); +/** Login attempt successful. */ +define('AUTH_LOGIN_OK', 0); + +/** Can not login because user does not exist. */ +define('AUTH_LOGIN_NOUSER', 1); + +/** Can not login because user is suspended. */ +define('AUTH_LOGIN_SUSPENDED', 2); + +/** Can not login, most probably password did not match. */ +define('AUTH_LOGIN_FAILED', 3); + +/** Can not login because user is locked out. */ +define('AUTH_LOGIN_LOCKOUT', 4); + + /** * Abstract authentication plugin. * @@ -507,3 +523,178 @@ function loginpage_idp_list($wantsurl) { } } + +/** + * Verify if user is locked out. + * + * @param stdClass $user + * @return bool true if user locked out + */ +function login_is_lockedout($user) { + global $CFG; + + if ($user->mnethostid != $CFG->mnet_localhost_id) { + return false; + } + if (isguestuser($user)) { + return false; + } + + if (empty($CFG->lockoutthreshold)) { + // Lockout not enabled. + return false; + } + + if (get_user_preferences('login_lockout_ignored', 0, $user)) { + // This preference may be used for accounts that must not be locked out. + return false; + } + + $locked = get_user_preferences('login_lockout', 0, $user); + if (!$locked) { + return false; + } + + if (empty($CFG->lockoutduration)) { + // Locked out forever. + return true; + } + + if (time() - $locked < $CFG->lockoutduration) { + return true; + } + + login_unlock_account($user); + + return false; +} + +/** + * To be called after valid user login. + * @param stdClass $user + */ +function login_attempt_valid($user) { + global $CFG; + + if ($user->mnethostid != $CFG->mnet_localhost_id) { + return; + } + if (isguestuser($user)) { + return; + } + + // Always unlock here, there might be some race conditions or leftovers when switching threshold. + login_unlock_account($user); +} + +/** + * To be called after failed user login. + * @param stdClass $user + */ +function login_attempt_failed($user) { + global $CFG; + + if ($user->mnethostid != $CFG->mnet_localhost_id) { + return; + } + if (isguestuser($user)) { + return; + } + + if (empty($CFG->lockoutthreshold)) { + // No threshold means no lockout. + // Always unlock here, there might be some race conditions or leftovers when switching threshold. + login_unlock_account($user); + return; + } + + $count = get_user_preferences('login_failed_count', 0, $user); + $last = get_user_preferences('login_failed_last', 0, $user); + + if (!empty($CFG->lockoutwindow) and time() - $last > $CFG->lockoutwindow) { + $count = 0; + } + + $count = $count+1; + + set_user_preference('login_failed_count', $count, $user); + set_user_preference('login_failed_last', time(), $user); + + if ($count >= $CFG->lockoutthreshold) { + login_lock_account($user); + } +} + +/** + * Lockout user and send notification email. + * + * @param stdClass $user + */ +function login_lock_account($user) { + global $CFG, $SESSION; + + if ($user->mnethostid != $CFG->mnet_localhost_id) { + return; + } + if (isguestuser($user)) { + return; + } + + if (get_user_preferences('login_lockout_ignored', 0, $user)) { + // This user can not be locked out. + return; + } + + $alreadylockedout = get_user_preferences('login_lockout', 0, $user); + + set_user_preference('login_lockout', time(), $user); + + if ($alreadylockedout == 0) { + $secret = random_string(15); + set_user_preference('login_lockout_secret', $secret, $user); + + // Some nasty hackery to get strings and dates localised for target user. + $sessionlang = isset($SESSION->lang) ? $SESSION->lang : null; + if (get_string_manager()->translation_exists($user->lang, false)) { + $SESSION->lang = $user->lang; + moodle_setlocale(); + } + + $site = get_site(); + $supportuser = generate_email_supportuser(); + + $data = new stdClass(); + $data->firstname = $user->firstname; + $data->lastname = $user->lastname; + $data->username = $user->username; + $data->sitename = format_string($site->fullname); + $data->link = $CFG->wwwroot.'/login/unlock_account.php?u='.$user->id.'&s='.$secret; + $data->admin = generate_email_signoff(); + + $message = get_string('lockoutemailbody', 'admin', $data); + $subject = get_string('lockoutemailsubject', 'admin', format_string($site->fullname)); + + if ($message) { + // Directly email rather than using the messaging system to ensure its not routed to a popup or jabber. + email_to_user($user, $supportuser, $subject, $message); + } + + if ($SESSION->lang !== $sessionlang) { + $SESSION->lang = $sessionlang; + moodle_setlocale(); + } + } +} + +/** + * Unlock user account and reset timers. + * + * @param stdClass $user + */ +function login_unlock_account($user) { + unset_user_preference('login_lockout', $user); + unset_user_preference('login_failed_count', $user); + unset_user_preference('login_failed_last', $user); + + // Note: do not clear the lockout secret because user might click on the link repeatedly. +} diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index 900b6e72b93e6..f03fd343fd30b 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -30,6 +30,26 @@ defined('MOODLE_INTERNAL') || die(); +/** + * Not used any more, the account lockout handling is now + * part of authenticate_user_login(). + * @deprecated + */ +function update_login_count() { + // note: remove 'errortoomanylogins' string from moodle.php too + // TODO: uncomment in Moodle 2.5, delete function in Moodle 2.6 + //debugging('update_login_count() is deprecated, all calls need to be removed'); +} + +/** + * Not used any more, replaced by proper account lockout. + * @deprecated + */ +function reset_login_count() { + // TODO: uncomment in Moodle 2.5, delete function in Moodle 2.6 + //debugging('reset_login_count() is deprecated, all calls need to be removed'); +} + /** * Unsupported session id rewriting. * @deprecated diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 9a8ddf182c45b..2f2d1ad69b136 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -3468,39 +3468,6 @@ function set_bounce_count($user,$reset=false) { } } -/** - * Keeps track of login attempts - * - * @global object - */ -function update_login_count() { - global $SESSION; - - $max_logins = 10; - - if (empty($SESSION->logincount)) { - $SESSION->logincount = 1; - } else { - $SESSION->logincount++; - } - - if ($SESSION->logincount > $max_logins) { - unset($SESSION->wantsurl); - print_error('errortoomanylogins'); - } -} - -/** - * Resets login attempts - * - * @global object - */ -function reset_login_count() { - global $SESSION; - - $SESSION->logincount = 0; -} - /** * Determines if the currently logged in user is in editing mode. * Note: originally this function had $userid parameter - it was not usable anyway @@ -4134,10 +4101,13 @@ function guest_user() { * * @param string $username User's username * @param string $password User's password - * @return user|flase A {@link $USER} object or false if error + * @param bool $ignorelockout useful when guessing is prevented by other mechanism such as captcha or SSO + * @param int $failurereason login failure reason, can be used in renderers (it may disclose if account exists) + * @return stdClass|false A {@link $USER} object or false if error */ -function authenticate_user_login($username, $password) { +function authenticate_user_login($username, $password, $ignorelockout=false, &$failurereason=null) { global $CFG, $DB; + require_once("$CFG->libdir/authlib.php"); $authsenabled = get_enabled_auth_plugins(); @@ -4146,11 +4116,13 @@ function authenticate_user_login($username, $password) { if (!empty($user->suspended)) { add_to_log(SITEID, 'login', 'error', 'index.php', $username); error_log('[client '.getremoteaddr()."] $CFG->wwwroot Suspended Login: $username ".$_SERVER['HTTP_USER_AGENT']); + $failurereason = AUTH_LOGIN_SUSPENDED; return false; } if ($auth=='nologin' or !is_enabled_auth($auth)) { add_to_log(SITEID, 'login', 'error', 'index.php', $username); error_log('[client '.getremoteaddr()."] $CFG->wwwroot Disabled Login: $username ".$_SERVER['HTTP_USER_AGENT']); + $failurereason = AUTH_LOGIN_SUSPENDED; // Legacy way to suspend user. return false; } $auths = array($auth); @@ -4159,6 +4131,7 @@ function authenticate_user_login($username, $password) { // Check if there's a deleted record (cheaply), this should not happen because we mangle usernames in delete_user(). if ($DB->get_field('user', 'id', array('username'=>$username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>1))) { error_log('[client '.getremoteaddr()."] $CFG->wwwroot Deleted Login: $username ".$_SERVER['HTTP_USER_AGENT']); + $failurereason = AUTH_LOGIN_NOUSER; return false; } @@ -4166,6 +4139,7 @@ function authenticate_user_login($username, $password) { if (!empty($CFG->authpreventaccountcreation)) { add_to_log(SITEID, 'login', 'error', 'index.php', $username); error_log('[client '.getremoteaddr()."] $CFG->wwwroot Unknown user, can not create new accounts: $username ".$_SERVER['HTTP_USER_AGENT']); + $failurereason = AUTH_LOGIN_NOUSER; return false; } @@ -4175,6 +4149,24 @@ function authenticate_user_login($username, $password) { $user->id = 0; } + if ($ignorelockout) { + // Some other mechanism protects against brute force password guessing, + // for example login form might include reCAPTCHA or this function + // is called from a SSO script. + + } else if ($user->id) { + // Verify login lockout after other ways that may prevent user login. + if (login_is_lockedout($user)) { + add_to_log(SITEID, 'login', 'error', 'index.php', $username); + error_log('[client '.getremoteaddr()."] $CFG->wwwroot Login lockout: $username ".$_SERVER['HTTP_USER_AGENT']); + $failurereason = AUTH_LOGIN_LOCKOUT; + return false; + } + + } else { + // We can not lockout non-existing accounts. + } + foreach ($auths as $auth) { $authplugin = get_auth_plugin($auth); @@ -4208,6 +4200,7 @@ function authenticate_user_login($username, $password) { } if (empty($user->id)) { + $failurereason = AUTH_LOGIN_NOUSER; return false; } @@ -4215,9 +4208,12 @@ function authenticate_user_login($username, $password) { // just in case some auth plugin suspended account add_to_log(SITEID, 'login', 'error', 'index.php', $username); error_log('[client '.getremoteaddr()."] $CFG->wwwroot Suspended Login: $username ".$_SERVER['HTTP_USER_AGENT']); + $failurereason = AUTH_LOGIN_SUSPENDED; return false; } + login_attempt_valid($user); + $failurereason = AUTH_LOGIN_OK; return $user; } @@ -4226,6 +4222,14 @@ function authenticate_user_login($username, $password) { if (debugging('', DEBUG_ALL)) { error_log('[client '.getremoteaddr()."] $CFG->wwwroot Failed Login: $username ".$_SERVER['HTTP_USER_AGENT']); } + + if ($user->id) { + login_attempt_failed($user); + $failurereason = AUTH_LOGIN_FAILED; + } else { + $failurereason = AUTH_LOGIN_NOUSER; + } + return false; } diff --git a/lib/tests/authlib_test.php b/lib/tests/authlib_test.php new file mode 100644 index 0000000000000..5594c0f983326 --- /dev/null +++ b/lib/tests/authlib_test.php @@ -0,0 +1,194 @@ +. + +/** + * Authentication related tests. + * + * @package core_auth + * @category phpunit + * @copyright 2012 Petr Skoda {@link http://skodak.org} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + + +/** + * Functional test for authentication related APIs. + */ +class authlib_testcase extends advanced_testcase { + public function test_lockout() { + global $CFG; + require_once("$CFG->libdir/authlib.php"); + + $this->resetAfterTest(); + + $oldlog = ini_get('error_log'); + ini_set('error_log', "$CFG->dataroot/testlog.log"); // Prevent standard logging. + + set_config('lockoutthreshold', 0); + set_config('lockoutwindow', 60*20); + set_config('lockoutduration', 60*30); + + $user = $this->getDataGenerator()->create_user(); + + + // Test lockout is disabled when threshold not set. + + $this->assertFalse(login_is_lockedout($user)); + login_attempt_failed($user); + login_attempt_failed($user); + login_attempt_failed($user); + login_attempt_failed($user); + $this->assertFalse(login_is_lockedout($user)); + + + // Test lockout threshold works. + + set_config('lockoutthreshold', 3); + login_attempt_failed($user); + login_attempt_failed($user); + $this->assertFalse(login_is_lockedout($user)); + ob_start(); + login_attempt_failed($user); + $output = ob_get_clean(); + $this->assertContains('noemailever', $output); + $this->assertTrue(login_is_lockedout($user)); + + + // Test unlock works. + + login_unlock_account($user); + $this->assertFalse(login_is_lockedout($user)); + + + // Test lockout window works. + + login_attempt_failed($user); + login_attempt_failed($user); + $this->assertFalse(login_is_lockedout($user)); + set_user_preference('login_failed_last', time()-60*20-10, $user); + login_attempt_failed($user); + $this->assertFalse(login_is_lockedout($user)); + + + // Test valid login resets window. + + login_attempt_valid($user); + $this->assertFalse(login_is_lockedout($user)); + login_attempt_failed($user); + login_attempt_failed($user); + $this->assertFalse(login_is_lockedout($user)); + + + // Test lock duration works. + + ob_start(); // Prevent nomailever notice. + login_attempt_failed($user); + $output = ob_get_clean(); + $this->assertContains('noemailever', $output); + $this->assertTrue(login_is_lockedout($user)); + set_user_preference('login_lockout', time()-60*30+10, $user); + $this->assertTrue(login_is_lockedout($user)); + set_user_preference('login_lockout', time()-60*30-10, $user); + $this->assertFalse(login_is_lockedout($user)); + + + // Test lockout ignored pref works. + + set_user_preference('login_lockout_ignored', 1, $user); + login_attempt_failed($user); + login_attempt_failed($user); + login_attempt_failed($user); + login_attempt_failed($user); + $this->assertFalse(login_is_lockedout($user)); + + ini_set('error_log', $oldlog); + } + + public function test_authenticate_user_login() { + global $CFG; + + $this->resetAfterTest(); + + $oldlog = ini_get('error_log'); + ini_set('error_log', "$CFG->dataroot/testlog.log"); // Prevent standard logging. + + set_config('lockoutthreshold', 0); + set_config('lockoutwindow', 60*20); + set_config('lockoutduration', 60*30); + + $_SERVER['HTTP_USER_AGENT'] = 'no browser'; // Hack around missing user agent in CLI scripts. + + $user1 = $this->getDataGenerator()->create_user(array('username'=>'username1', 'password'=>'password1')); + $user2 = $this->getDataGenerator()->create_user(array('username'=>'username2', 'password'=>'password2', 'suspended'=>1)); + $user3 = $this->getDataGenerator()->create_user(array('username'=>'username3', 'password'=>'password3', 'auth'=>'nologin')); + + $result = authenticate_user_login('username1', 'password1'); + $this->assertInstanceOf('stdClass', $result); + $this->assertEquals($user1->id, $result->id); + + $reason = null; + $result = authenticate_user_login('username1', 'password1', false, $reason); + $this->assertInstanceOf('stdClass', $result); + $this->assertEquals(AUTH_LOGIN_OK, $reason); + + $reason = null; + $result = authenticate_user_login('username1', 'nopass', false, $reason); + $this->assertFalse($result); + $this->assertEquals(AUTH_LOGIN_FAILED, $reason); + + $reason = null; + $result = authenticate_user_login('username2', 'password2', false, $reason); + $this->assertFalse($result); + $this->assertEquals(AUTH_LOGIN_SUSPENDED, $reason); + + $reason = null; + $result = authenticate_user_login('username3', 'password3', false, $reason); + $this->assertFalse($result); + $this->assertEquals(AUTH_LOGIN_SUSPENDED, $reason); + + $reason = null; + $result = authenticate_user_login('username4', 'password3', false, $reason); + $this->assertFalse($result); + $this->assertEquals(AUTH_LOGIN_NOUSER, $reason); + + + set_config('lockoutthreshold', 3); + $reason = null; + $result = authenticate_user_login('username1', 'nopass', false, $reason); + $this->assertFalse($result); + $this->assertEquals(AUTH_LOGIN_FAILED, $reason); + $result = authenticate_user_login('username1', 'nopass', false, $reason); + $this->assertFalse($result); + $this->assertEquals(AUTH_LOGIN_FAILED, $reason); + ob_start(); // Prevent nomailever notice. + $result = authenticate_user_login('username1', 'nopass', false, $reason); + ob_end_clean(); + $this->assertFalse($result); + $this->assertEquals(AUTH_LOGIN_FAILED, $reason); + + $result = authenticate_user_login('username1', 'password1', false, $reason); + $this->assertFalse($result); + $this->assertEquals(AUTH_LOGIN_LOCKOUT, $reason); + + $result = authenticate_user_login('username1', 'password1', true, $reason); + $this->assertInstanceOf('stdClass', $result); + $this->assertEquals(AUTH_LOGIN_OK, $reason); + + ini_set('error_log', $oldlog); + } +} diff --git a/login/change_password.php b/login/change_password.php index e2cc58d5e181f..4318175774271 100644 --- a/login/change_password.php +++ b/login/change_password.php @@ -26,6 +26,7 @@ require('../config.php'); require_once('change_password_form.php'); +require_once($CFG->libdir.'/authlib.php'); $id = optional_param('id', SITEID, PARAM_INT); // current course $return = optional_param('return', 0, PARAM_BOOL); // redirect after password change @@ -110,6 +111,9 @@ print_error('errorpasswordupdate', 'auth'); } + // Reset login lockout - we want to prevent any accidental confusion here. + login_unlock_account($user); + // register success changing password unset_user_preference('auth_forcepasswordchange', $USER); unset_user_preference('create_password', $USER); diff --git a/login/change_password_form.php b/login/change_password_form.php index 6b8fffe1fd454..885e0c7015e03 100644 --- a/login/change_password_form.php +++ b/login/change_password_form.php @@ -73,16 +73,12 @@ function validation($data, $files) { global $USER; $errors = parent::validation($data, $files); - update_login_count(); - // ignore submitted username - if (!$user = authenticate_user_login($USER->username, $data['password'])) { + if (!$user = authenticate_user_login($USER->username, $data['password'], true)) { $errors['password'] = get_string('invalidlogin'); return $errors; } - reset_login_count(); - if ($data['newpassword1'] <> $data['newpassword2']) { $errors['newpassword1'] = get_string('passwordsdiffer'); $errors['newpassword2'] = get_string('passwordsdiffer'); diff --git a/login/forgot_password.php b/login/forgot_password.php index d16ff7e4bb63b..8f51b6df59e77 100644 --- a/login/forgot_password.php +++ b/login/forgot_password.php @@ -27,6 +27,7 @@ */ require('../config.php'); +require_once($CFG->libdir.'/authlib.php'); require_once('forgot_password_form.php'); $p_secret = optional_param('p', false, PARAM_RAW); @@ -63,8 +64,6 @@ /// user clicked on link in email message ///===================== - update_login_count(); - $user = $DB->get_record('user', array('username'=>$p_username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0, 'suspended'=>0)); if ($user and ($user->auth === 'nologin' or !is_enabled_auth($user->auth))) { @@ -83,6 +82,9 @@ print_error('cannotresetguestpwd'); } + // Reset login lockout even of the password reset fails. + login_unlock_account($user); + // make sure user is allowed to change password require_capability('moodle/user:changeownpassword', $systemcontext, $user->id); @@ -94,8 +96,6 @@ $user->secret = ''; $DB->set_field('user', 'secret', $user->secret, array('id'=>$user->id)); - reset_login_count(); - $changepasswordurl = "{$CFG->httpswwwroot}/login/change_password.php"; $a = new stdClass(); $a->email = $user->email; diff --git a/login/index.php b/login/index.php index 6276a57899349..cd3b558b5a7fd 100644 --- a/login/index.php +++ b/login/index.php @@ -148,8 +148,6 @@ die; } - update_login_count(); - if ($user) { // language setup @@ -242,8 +240,6 @@ } } - reset_login_count(); - // test the session actually works by redirecting to self $SESSION->wantsurl = $urltogo; redirect(new moodle_url(get_login_url(), array('testsession'=>$USER->id))); diff --git a/login/unlock_account.php b/login/unlock_account.php new file mode 100644 index 0000000000000..087729a3de9fa --- /dev/null +++ b/login/unlock_account.php @@ -0,0 +1,54 @@ +. + +/** + * Reset locked-out accounts. + * + * @package core_auth + * @copyright 2012 Petr Skoda {@link http://skodak.org} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +require('../config.php'); +require_once($CFG->libdir.'/authlib.php'); + +$userid = optional_param('u', 0, PARAM_INT); +$secret = optional_param('s', '', PARAM_RAW); + +$PAGE->set_url('/login/unlock_account.php'); +$PAGE->set_context(context_system::instance()); + +// Override wanted URL, we do not want to end up here again after login! +$SESSION->wantsurl = "$CFG->wwwroot/"; + +// Do not disclose details about existence or status of user accounts here. + +if (!$user = $DB->get_record('user', array('id'=>$userid, 'deleted'=>0, 'suspended'=>0))) { + print_error('lockouterrorunlock', 'admin', get_login_url()); +} + +$usersecret = get_user_preferences('login_lockout_secret', false, $user); + +if ($secret === $usersecret) { + login_unlock_account($user); + if ($USER->id == $user->id) { + redirect("$CFG->wwwroot/"); + } else { + redirect(get_login_url()); + } +} + +print_error('lockouterrorunlock', 'admin', get_login_url()); From c4844bf45cf04f8c5ecb99818d51aac74be69027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Fri, 4 Jan 2013 15:17:14 +0100 Subject: [PATCH 2/2] MDL-21342 deprecate unused login functions --- lang/en/moodle.php | 1 - lib/deprecatedlib.php | 9 ++++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 16430f767ef24..3fc8e8705f8a1 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -649,7 +649,6 @@ $string['errorcreatingactivity'] = 'Unable to create an instance of activity \'{$a}\''; $string['errorfiletoobig'] = 'The file was bigger than the limit of {$a} bytes'; $string['errornouploadrepo'] = 'There is no upload repository enabled for this site'; -$string['errortoomanylogins'] = 'Sorry, you have exceeded the allowed number of login attempts. Restart your browser.'; $string['errorwhenconfirming'] = 'You are not confirmed yet because an error occurred. If you clicked on a link in an email to get here, make sure that the line in your email wasn\'t broken or wrapped. You may have to use cut and paste to reconstruct the link properly.'; $string['everybody'] = 'Everybody'; $string['executeat'] = 'Execute at'; diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index f03fd343fd30b..4ace5edf3cc2b 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -36,9 +36,8 @@ * @deprecated */ function update_login_count() { - // note: remove 'errortoomanylogins' string from moodle.php too - // TODO: uncomment in Moodle 2.5, delete function in Moodle 2.6 - //debugging('update_login_count() is deprecated, all calls need to be removed'); + // TODO: delete function in Moodle 2.6 + debugging('update_login_count() is deprecated, all calls need to be removed'); } /** @@ -46,8 +45,8 @@ function update_login_count() { * @deprecated */ function reset_login_count() { - // TODO: uncomment in Moodle 2.5, delete function in Moodle 2.6 - //debugging('reset_login_count() is deprecated, all calls need to be removed'); + // TODO: delete function in Moodle 2.6 + debugging('reset_login_count() is deprecated, all calls need to be removed'); } /**