From f1fc0d3219ef5e1c29f72f7408071eb2154f0585 Mon Sep 17 00:00:00 2001 From: Peter Bulmer Date: Sat, 14 Sep 2013 13:28:25 +1200 Subject: [PATCH 1/8] MDL-23692 forgotpw: Move functionality to lib. Move the password reset request handling into a function. Move the password reset confirmation handling into a function. --- login/forgot_password.php | 141 +++---------------------------------- login/lib.php | 144 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 132 deletions(-) create mode 100644 login/lib.php diff --git a/login/forgot_password.php b/login/forgot_password.php index c77dac7012b..fbb513dbfd5 100644 --- a/login/forgot_password.php +++ b/login/forgot_password.php @@ -28,6 +28,7 @@ require('../config.php'); require_once($CFG->libdir.'/authlib.php'); +require_once('lib.php'); require_once('forgot_password_form.php'); $p_secret = optional_param('p', false, PARAM_RAW); @@ -59,136 +60,12 @@ if (isloggedin() and !isguestuser()) { redirect($CFG->wwwroot.'/index.php', get_string('loginalready'), 5); } -if ($p_secret !== false) { -///===================== -/// user clicked on link in email message -///===================== - - $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))) { - // bad luck - user is not able to login, do not let them reset password - $user = false; - } - - if (!empty($user) and $user->secret === '') { - echo $OUTPUT->header(); - print_error('secretalreadyused'); - } else if (!empty($user) and $user->secret == $p_secret) { - // make sure that url relates to a valid user - - // check this isn't guest user - if (isguestuser($user)) { - 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); - - if (!reset_password_and_mail($user)) { - print_error('cannotresetmail'); - } - - // Clear secret so that it can not be used again - $user->secret = ''; - $DB->set_field('user', 'secret', $user->secret, array('id'=>$user->id)); - - $changepasswordurl = "{$CFG->httpswwwroot}/login/change_password.php"; - $a = new stdClass(); - $a->email = $user->email; - $a->link = $changepasswordurl; - - echo $OUTPUT->header(); - notice(get_string('emailpasswordsent', '', $a), $changepasswordurl); - - } else { - if (!empty($user) and strlen($p_secret) === 15) { - // somebody probably tries to hack in by guessing secret - stop them! - $DB->set_field('user', 'secret', '', array('id'=>$user->id)); - } - echo $OUTPUT->header(); - print_error('forgotteninvalidurl'); - } - - die; //never reached +if (empty($p_secret)) { + // This is a new password reset request + // identify the user & send confirmation email + forgotpw_process_request(); +} else { + // User clicked on confirmation link in email message + // validate the token & set new password + forgotpw_process_pwset($p_secret, $p_username); } - -$mform = new login_forgot_password_form(); - -if ($mform->is_cancelled()) { - redirect(get_login_url()); - -} else if ($data = $mform->get_data()) { -/// find the user in the database and mail info - - // first try the username - if (!empty($data->username)) { - $username = core_text::strtolower($data->username); // mimic the login page process, if they forget username they need to use email for reset - $user = $DB->get_record('user', array('username'=>$username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0, 'suspended'=>0)); - - } else { - // this is tricky because - // 1/ the email is not guaranteed to be unique - TODO: send email with all usernames to select the correct account for pw reset - // 2/ mailbox may be case sensitive, the email domain is case insensitive - let's pretend it is all case-insensitive - - $select = $DB->sql_like('email', ':email', false, true, false, '|'). " AND mnethostid = :mnethostid AND deleted=0 AND suspended=0"; - $params = array('email'=>$DB->sql_like_escape($data->email, '|'), 'mnethostid'=>$CFG->mnet_localhost_id); - $user = $DB->get_record_select('user', $select, $params, '*', IGNORE_MULTIPLE); - } - - if ($user and !empty($user->confirmed)) { - - $userauth = get_auth_plugin($user->auth); - if (has_capability('moodle/user:changeownpassword', $systemcontext, $user->id)) { - // send email - } - - if ($userauth->can_reset_password() and is_enabled_auth($user->auth) - and has_capability('moodle/user:changeownpassword', $systemcontext, $user->id)) { - // send reset password confirmation - - // set 'secret' string - $user->secret = random_string(15); - $DB->set_field('user', 'secret', $user->secret, array('id'=>$user->id)); - - if (!send_password_change_confirmation_email($user)) { - print_error('cannotmailconfirm'); - } - - } else { - if (!send_password_change_info($user)) { - print_error('cannotmailconfirm'); - } - } - } - - echo $OUTPUT->header(); - - if (empty($user->email) or !empty($CFG->protectusernames)) { - // Print general confirmation message - notice(get_string('emailpasswordconfirmmaybesent'), $CFG->wwwroot.'/index.php'); - - } else { - // Confirm email sent - $protectedemail = preg_replace('/([^@]*)@(.*)/', '******@$2', $user->email); // obfuscate the email address to protect privacy - $stremailpasswordconfirmsent = get_string('emailpasswordconfirmsent', '', $protectedemail); - notice($stremailpasswordconfirmsent, $CFG->wwwroot.'/index.php'); - } - - die; // never reached -} - -// make sure we really are on the https page when https login required -$PAGE->verify_https_required(); - - -/// DISPLAY FORM - -echo $OUTPUT->header(); -echo $OUTPUT->box(get_string('passwordforgotteninstructions2'), 'generalbox boxwidthnormal boxaligncenter'); -$mform->display(); - -echo $OUTPUT->footer(); diff --git a/login/lib.php b/login/lib.php new file mode 100644 index 00000000000..f5b862d629f --- /dev/null +++ b/login/lib.php @@ -0,0 +1,144 @@ +is_cancelled()) { + redirect(get_login_url()); + + } else if ($data = $mform->get_data()) { + // User has submitted form data. Find the user in the database and mail info. + // First try the username. + if (!empty($data->username)) { + $username = core_text::strtolower($data->username); // mimic the login page process, if they forget username they need to use email for reset + $user = $DB->get_record('user', array('username'=>$username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0, 'suspended'=>0)); + + } else { + // this is tricky because + // 1/ the email is not guaranteed to be unique - TODO: send email with all usernames to select the correct account for pw reset + // 2/ mailbox may be case sensitive, the email domain is case insensitive - let's pretend it is all case-insensitive + + $select = $DB->sql_like('email', ':email', false, true, false, '|'). " AND mnethostid = :mnethostid AND deleted=0 AND suspended=0"; + $params = array('email'=>$DB->sql_like_escape($data->email, '|'), 'mnethostid'=>$CFG->mnet_localhost_id); + $user = $DB->get_record_select('user', $select, $params, '*', IGNORE_MULTIPLE); + } + + if ($user and !empty($user->confirmed)) { + + $userauth = get_auth_plugin($user->auth); + if (has_capability('moodle/user:changeownpassword', $systemcontext, $user->id)) { + // send email + } + + if ($userauth->can_reset_password() and is_enabled_auth($user->auth) + and has_capability('moodle/user:changeownpassword', $systemcontext, $user->id)) { + // send reset password confirmation + + // set 'secret' string + $user->secret = random_string(15); + $DB->set_field('user', 'secret', $user->secret, array('id'=>$user->id)); + + if (!send_password_change_confirmation_email($user)) { + print_error('cannotmailconfirm'); + } + + } else { + if (!send_password_change_info($user)) { + print_error('cannotmailconfirm'); + } + } + } + + echo $OUTPUT->header(); + + if (empty($user->email) or !empty($CFG->protectusernames)) { + // Print general confirmation message + notice(get_string('emailpasswordconfirmmaybesent'), $CFG->wwwroot.'/index.php'); + + } else { + // Confirm email sent + $protectedemail = preg_replace('/([^@]*)@(.*)/', '******@$2', $user->email); // obfuscate the email address to protect privacy + $stremailpasswordconfirmsent = get_string('emailpasswordconfirmsent', '', $protectedemail); + notice($stremailpasswordconfirmsent, $CFG->wwwroot.'/index.php'); + } + + die; // never reached + } + + // make sure we really are on the https page when https login required + $PAGE->verify_https_required(); + + + /// DISPLAY FORM + + echo $OUTPUT->header(); + echo $OUTPUT->box(get_string('passwordforgotteninstructions2'), 'generalbox boxwidthnormal boxaligncenter'); + $mform->display(); + + echo $OUTPUT->footer(); +} + +/* This function processes a user's submitted token to validate the request to set a new password + If the user's token is validated, they are emailed with a new password. +*/ +function forgotpw_process_pwset($token, $username) { + global $DB, $CFG, $OUTPUT; + $systemcontext = context_system::instance(); + $user = $DB->get_record('user', array('username'=>$username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0, 'suspended'=>0)); + + if ($user and ($user->auth === 'nologin' or !is_enabled_auth($user->auth))) { + // bad luck - user is not able to login, do not let them reset password + $user = false; + } + + if (!empty($user) and $user->secret === '') { + echo $OUTPUT->header(); + print_error('secretalreadyused'); + } else if (!empty($user) and $user->secret == $token) { + // make sure that url relates to a valid user + + // check this isn't guest user + if (isguestuser($user)) { + 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); + + if (!reset_password_and_mail($user)) { + print_error('cannotresetmail'); + } + + // Clear secret so that it can not be used again + $user->secret = ''; + $DB->set_field('user', 'secret', $user->secret, array('id'=>$user->id)); + + $changepasswordurl = "{$CFG->httpswwwroot}/login/change_password.php"; + $a = new stdClass(); + $a->email = $user->email; + $a->link = $changepasswordurl; + + echo $OUTPUT->header(); + notice(get_string('emailpasswordsent', '', $a), $changepasswordurl); + + } else { + if (!empty($user) and strlen($token) === 15) { + // somebody probably tries to hack in by guessing secret - stop them! + $DB->set_field('user', 'secret', '', array('id'=>$user->id)); + } + echo $OUTPUT->header(); + print_error('forgotteninvalidurl'); + } + + die; //never reached +} +?> From a56e73b4a2fa773e9e96fb5e84398ae1c317b347 Mon Sep 17 00:00:00 2001 From: Peter Bulmer Date: Fri, 26 Jul 2013 10:35:53 +1200 Subject: [PATCH 2/8] MDL-23692 forgotpw: Remove redundant code. --- login/lib.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/login/lib.php b/login/lib.php index f5b862d629f..6f62b9c8d93 100644 --- a/login/lib.php +++ b/login/lib.php @@ -32,10 +32,6 @@ function forgotpw_process_request() { if ($user and !empty($user->confirmed)) { $userauth = get_auth_plugin($user->auth); - if (has_capability('moodle/user:changeownpassword', $systemcontext, $user->id)) { - // send email - } - if ($userauth->can_reset_password() and is_enabled_auth($user->auth) and has_capability('moodle/user:changeownpassword', $systemcontext, $user->id)) { // send reset password confirmation From f888862eb018bbb7876620b57ba6d95e15a99db6 Mon Sep 17 00:00:00 2001 From: Peter Bulmer Date: Fri, 26 Jul 2013 14:50:18 +1200 Subject: [PATCH 3/8] MDL-23692 forgotpw: Comments & layout changes. No functional change. --- login/lib.php | 51 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/login/lib.php b/login/lib.php index 6f62b9c8d93..6326ac86f04 100644 --- a/login/lib.php +++ b/login/lib.php @@ -1,4 +1,7 @@ get_data()) { - // User has submitted form data. Find the user in the database and mail info. - // First try the username. + // Requesting user has submitted form data. + // Find the user account in the database which the requesting user claims to own: if (!empty($data->username)) { + // Username has been specified - load the user record based on that. $username = core_text::strtolower($data->username); // mimic the login page process, if they forget username they need to use email for reset $user = $DB->get_record('user', array('username'=>$username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0, 'suspended'=>0)); } else { + // Try to load the user record based on email address: // this is tricky because // 1/ the email is not guaranteed to be unique - TODO: send email with all usernames to select the correct account for pw reset // 2/ mailbox may be case sensitive, the email domain is case insensitive - let's pretend it is all case-insensitive @@ -29,8 +34,10 @@ function forgotpw_process_request() { $user = $DB->get_record_select('user', $select, $params, '*', IGNORE_MULTIPLE); } + // Target user details have now been identified, or we know that there is no such account. + // Send email address to account's email address if appropriate: + $pwresetstatus = PWRESET_STATUS_NOEMAILSENT; if ($user and !empty($user->confirmed)) { - $userauth = get_auth_plugin($user->auth); if ($userauth->can_reset_password() and is_enabled_auth($user->auth) and has_capability('moodle/user:changeownpassword', $systemcontext, $user->id)) { @@ -40,30 +47,52 @@ function forgotpw_process_request() { $user->secret = random_string(15); $DB->set_field('user', 'secret', $user->secret, array('id'=>$user->id)); - if (!send_password_change_confirmation_email($user)) { + if (send_password_change_confirmation_email($user)) { + $pwresetstatus = PWRESET_STATUS_TOKENSENT; + } else { print_error('cannotmailconfirm'); } } else { - if (!send_password_change_info($user)) { + if (send_password_change_info($user)) { + $pwresetstatus = PWRESET_STATUS_OTHEREMAILSENT; + } else { print_error('cannotmailconfirm'); } } } + // Any email has now been sent. + // Next display results to requesting user if settings permit: echo $OUTPUT->header(); - if (empty($user->email) or !empty($CFG->protectusernames)) { - // Print general confirmation message + if (!empty($CFG->protectusernames)) { + // Neither confirm, nor deny existance of any username or email address in database. + // Print general (non-commital) message notice(get_string('emailpasswordconfirmmaybesent'), $CFG->wwwroot.'/index.php'); - - } else { - // Confirm email sent + die; // never reached + } elseif (empty($user) || empty($user->email)) { + // Protect usernames is off, and we either couldn't find the user, or they don't have an email set. + // Still print non-commital message + // This is a big usability problem - need to tell users when we couldn't find the details they requested. + notice(get_string('emailpasswordconfirmmaybesent'), $CFG->wwwroot.'/index.php'); + die; // never reached + } elseif ($pwresetstatus == PWRESET_STATUS_NOEMAILSENT) { + // User found, protectusernames is off, but user is not confirmed + // Pretend we sent them an email + // This is a big usability problem - need to tell users why we didn't send them an email $protectedemail = preg_replace('/([^@]*)@(.*)/', '******@$2', $user->email); // obfuscate the email address to protect privacy $stremailpasswordconfirmsent = get_string('emailpasswordconfirmsent', '', $protectedemail); notice($stremailpasswordconfirmsent, $CFG->wwwroot.'/index.php'); + die; // never reached + } else { + // Confirm email sent + $protectedemail = preg_replace('/([^@]*)@(.*)/', '******@$2', $user->email); // obfuscate the email address to protect privacy + // This is a small usability problem - may be obfuscating the email address which the user has just supplied. + $stremailpasswordconfirmsent = get_string('emailpasswordconfirmsent', '', $protectedemail); + notice($stremailpasswordconfirmsent, $CFG->wwwroot.'/index.php'); + die; // never reached } - die; // never reached } From 4f3f6524e8f407073eff02c013b33db33047e87f Mon Sep 17 00:00:00 2001 From: Peter Bulmer Date: Fri, 26 Jul 2013 15:22:00 +1200 Subject: [PATCH 4/8] MDL-23692 forgotpw: Improve failure notices. --- lang/en/moodle.php | 5 +++++ login/lib.php | 13 ++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lang/en/moodle.php b/lang/en/moodle.php index b41fd326ef0..f3f03d01cfa 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -606,6 +606,11 @@ $string['emailpasswordconfirmationsubject'] = '{$a}: Change password confirmatio $string['emailpasswordconfirmmaybesent'] = '

If you supplied a correct username or email address then an email should have been sent to you.

It contains easy instructions to confirm and complete this password change. If you continue to have difficulty, please contact the site administrator.

'; +$string['emailpasswordconfirmnoemail'] = '

The user account you specified does not have a recorded email address.

+

Please contact the site administrator.

'; +$string['emailpasswordconfirmnotsent'] = '

The user detail you supplied does not identify an existing user account.

+

Please check the detail you entered, then try again. +If you continue to have difficulty, please contact the site administrator.

'; $string['emailpasswordconfirmsent'] = 'An email should have been sent to your address at {$a}.
It contains easy instructions to confirm and complete this password change. If you continue to have difficulty, contact the site administrator.'; diff --git a/login/lib.php b/login/lib.php index 6326ac86f04..7d8f63b48ed 100644 --- a/login/lib.php +++ b/login/lib.php @@ -71,11 +71,14 @@ function forgotpw_process_request() { // Print general (non-commital) message notice(get_string('emailpasswordconfirmmaybesent'), $CFG->wwwroot.'/index.php'); die; // never reached - } elseif (empty($user) || empty($user->email)) { - // Protect usernames is off, and we either couldn't find the user, or they don't have an email set. - // Still print non-commital message - // This is a big usability problem - need to tell users when we couldn't find the details they requested. - notice(get_string('emailpasswordconfirmmaybesent'), $CFG->wwwroot.'/index.php'); + } elseif (empty($user)) { + // Protect usernames is off, and we couldn't find the user with details specified. + // Print failure advice: + notice(get_string('emailpasswordconfirmnotsent'), $CFG->wwwroot.'/forgot_password.php'); + die; // never reached + } elseif (empty($user->email)) { + // User doesn't have an email set - can't send a password change confimation email. + notice(get_string('emailpasswordconfirmnoemail'), $CFG->wwwroot.'/index.php'); die; // never reached } elseif ($pwresetstatus == PWRESET_STATUS_NOEMAILSENT) { // User found, protectusernames is off, but user is not confirmed From f4491c2adee5efc819b2d9d4962249023ef549f5 Mon Sep 17 00:00:00 2001 From: Peter Bulmer Date: Thu, 1 Aug 2013 11:05:26 +1200 Subject: [PATCH 5/8] MDL-23692 forgotpw: Move redirection logic to fn. Moving to a function because the same logic will be needed in a new place in later commits. --- login/index.php | 27 ++------------------------- login/lib.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/login/index.php b/login/index.php index 45d1a580d7f..def89e973f7 100644 --- a/login/index.php +++ b/login/index.php @@ -25,6 +25,7 @@ */ require('../config.php'); +require_once('lib.php'); // Try to prevent searching for sites that allow sign-up. if (!isset($CFG->additionalhtmlhead)) { @@ -193,31 +194,7 @@ if ($frm and isset($frm->username)) { // Login WITH set_moodle_cookie($USER->username); } - /// Prepare redirection - if (user_not_fully_set_up($USER)) { - $urltogo = $CFG->wwwroot.'/user/edit.php'; - // We don't delete $SESSION->wantsurl yet, so we get there later - - } else if (isset($SESSION->wantsurl) and (strpos($SESSION->wantsurl, $CFG->wwwroot) === 0 or strpos($SESSION->wantsurl, str_replace('http://', 'https://', $CFG->wwwroot)) === 0)) { - $urltogo = $SESSION->wantsurl; /// Because it's an address in this site - unset($SESSION->wantsurl); - - } else { - // no wantsurl stored or external - go to homepage - $urltogo = $CFG->wwwroot.'/'; - unset($SESSION->wantsurl); - } - - // If the url to go to is the same as the site page, check for default homepage. - if ($urltogo == ($CFG->wwwroot . '/')) { - $home_page = get_home_page(); - // Go to my-moodle page instead of site homepage if defaulthomepage set to homepage_my - if ($home_page == HOMEPAGE_MY && !is_siteadmin() && !isguestuser()) { - if ($urltogo == $CFG->wwwroot or $urltogo == $CFG->wwwroot.'/' or $urltogo == $CFG->wwwroot.'/index.php') { - $urltogo = $CFG->wwwroot.'/my/'; - } - } - } + $urltogo = get_postlogin_redirection(); /// check if user password has expired /// Currently supported only for ldap-authentication module diff --git a/login/lib.php b/login/lib.php index 7d8f63b48ed..fb9a26752f7 100644 --- a/login/lib.php +++ b/login/lib.php @@ -169,4 +169,35 @@ function forgotpw_process_pwset($token, $username) { die; //never reached } + +/* Determine where a user should be redirected after they have been logged in +*/ +function get_postlogin_redirection() { + global $CFG, $SESSION, $USER; + /// Prepare redirection + if (user_not_fully_set_up($USER)) { + $urltogo = $CFG->wwwroot.'/user/edit.php'; + // We don't delete $SESSION->wantsurl yet, so we get there later + + } else if (isset($SESSION->wantsurl) and (strpos($SESSION->wantsurl, $CFG->wwwroot) === 0 or strpos($SESSION->wantsurl, str_replace('http://', 'https://', $CFG->wwwroot)) === 0)) { + $urltogo = $SESSION->wantsurl; /// Because it's an address in this site + unset($SESSION->wantsurl); + } else { + // no wantsurl stored or external - go to homepage + $urltogo = $CFG->wwwroot.'/'; + unset($SESSION->wantsurl); + } + + // If the url to go to is the same as the site page, check for default homepage. + if ($urltogo == ($CFG->wwwroot . '/')) { + $home_page = get_home_page(); + // Go to my-moodle page instead of site homepage if defaulthomepage set to homepage_my + if ($home_page == HOMEPAGE_MY && !is_siteadmin() && !isguestuser()) { + if ($urltogo == $CFG->wwwroot or $urltogo == $CFG->wwwroot.'/' or $urltogo == $CFG->wwwroot.'/index.php') { + $urltogo = $CFG->wwwroot.'/my/'; + } + } + } + return $urltogo; +} ?> From 92de749fc715108098df5f874d1cfb22af31bf38 Mon Sep 17 00:00:00 2001 From: Peter Bulmer Date: Sat, 14 Sep 2013 21:39:04 +1200 Subject: [PATCH 6/8] MDL-23692 forgotpw: Simplify forgotpw process. --- admin/settings/security.php | 16 +++ lang/en/admin.php | 2 + lang/en/moodle.php | 26 ++++- lib/cronlib.php | 9 ++ lib/db/install.xml | 15 ++- lib/db/upgrade.php | 21 ++++ lib/moodlelib.php | 12 ++- login/forgot_password.php | 14 +-- login/lib.php | 202 ++++++++++++++++++++++++------------ login/set_password_form.php | 86 +++++++++++++++ version.php | 2 +- 11 files changed, 327 insertions(+), 78 deletions(-) create mode 100644 login/set_password_form.php diff --git a/admin/settings/security.php b/admin/settings/security.php index 3d63ce6a303..acb30489094 100644 --- a/admin/settings/security.php +++ b/admin/settings/security.php @@ -70,6 +70,22 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page $temp->add(new admin_setting_configtext('minpasswordupper', new lang_string('minpasswordupper', 'admin'), new lang_string('configminpasswordupper', 'admin'), 1, PARAM_INT)); $temp->add(new admin_setting_configtext('minpasswordnonalphanum', new lang_string('minpasswordnonalphanum', 'admin'), new lang_string('configminpasswordnonalphanum', 'admin'), 1, PARAM_INT)); $temp->add(new admin_setting_configtext('maxconsecutiveidentchars', new lang_string('maxconsecutiveidentchars', 'admin'), new lang_string('configmaxconsecutiveidentchars', 'admin'), 0, PARAM_INT)); + $pwresetoptions = array( + 300 => new lang_string('numminutes', '', 5), + 900 => new lang_string('numminutes', '', 15), + 1800 => new lang_string('numminutes', '', 30), + 2700 => new lang_string('numminutes', '', 45), + 3600 => new lang_string('numminutes', '', 60), + 7200 => new lang_string('numminutes', '', 120), + 14400 => new lang_string('numminutes', '', 240) + ); + $adminsetting = new admin_setting_configselect( + 'pwresettime', + new lang_string('passwordresettime','admin'), + new lang_string('configpasswordresettime','admin'), + 1800, + $pwresetoptions); + $temp->add($adminsetting); $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 71a73d89c36..d506b8e8913 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -277,6 +277,7 @@ $string['confignotloggedinroleid'] = 'Users who are not logged in to the site wi $string['configopentogoogle'] = 'If you enable this setting, then Google will be allowed to enter your site as a Guest. In addition, people coming in to your site via a Google search will automatically be logged in as a Guest. Note that this only provides transparent access to courses that already allow guest access.'; $string['configoverride'] = 'Defined in config.php'; $string['configpasswordpolicy'] = 'Turning this on will make Moodle check user passwords against a valid password policy. Use the settings below to specify your policy (they will be ignored if you set this to \'No\').'; +$string['configpasswordresettime'] = 'This specifies the amount of time people have to validate a password reset request before it expires. Usually 30 minutes is a good value.'; $string['configpathtoclam'] = 'Path to clam AV. Probably something like /usr/bin/clamscan or /usr/bin/clamdscan. You need this in order for clam AV to run.'; $string['configpathtodu'] = 'Path to du. Probably something like /usr/bin/du. If you enter this, pages that display directory contents will run much faster for directories with a lot of files.'; $string['configperfdebug'] = 'If you turn this on, performance info will be printed in the footer of the standard theme'; @@ -778,6 +779,7 @@ $string['order2'] = 'Second'; $string['order3'] = 'Third'; $string['order4'] = 'Fourth'; $string['passwordpolicy'] = 'Password policy'; +$string['passwordresettime'] = 'Maximum time to validate password reset request'; $string['pathconvert'] = 'Path of convert binary'; $string['pathdvips'] = 'Path of dvips binary'; $string['pathlatex'] = 'Path of latex binary'; diff --git a/lang/en/moodle.php b/lang/en/moodle.php index f3f03d01cfa..691b3cc737b 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -535,6 +535,7 @@ $string['editthisactivity'] = 'Edit this activity'; $string['editthiscategory'] = 'Edit this category'; $string['edituser'] = 'Edit user accounts'; $string['email'] = 'Email address'; +$string['emailalreadysent'] = 'A Password reset email has already been sent. Please check your email.'; $string['emailactive'] = 'Email activated'; $string['emailagain'] = 'Email (again)'; $string['emailconfirm'] = 'Confirm your account'; @@ -609,7 +610,7 @@ If you continue to have difficulty, please contact the site administrator.

'; $string['emailpasswordconfirmnoemail'] = '

The user account you specified does not have a recorded email address.

Please contact the site administrator.

'; $string['emailpasswordconfirmnotsent'] = '

The user detail you supplied does not identify an existing user account.

-

Please check the detail you entered, then try again. +

Please check the information you entered, then try again. If you continue to have difficulty, please contact the site administrator.

'; $string['emailpasswordconfirmsent'] = 'An email should have been sent to your address at {$a}.
It contains easy instructions to confirm and complete this password change. @@ -651,6 +652,24 @@ $string['emailpasswordsent'] = 'Thank you for confirming the change of password. An email containing your new password has been sent to your address at
{$a->email}.
The new password was automatically generated - you might like to change your password to something easier to remember.'; +$string['emailresetconfirmation'] = 'Hi {$a->firstname}, + +A password reset was requested for your account \'{$a->username}\' at {$a->sitename}. + +To confirm this request, and set a new password for your account, please +go to the following web address: + +{$a->link} +(This link is valid for {$a->resetminutes} minutes from the time this reset was first requested) + +If this password reset was not requested by you, no action is needed. + +If you need help, please contact the site administrator, +{$a->admin}'; +$string['emailresetconfirmationsubject'] = '{$a}: Password reset request'; +$string['emailresetconfirmsent'] = 'An email has been sent to your address at {$a}. +
It contains easy instructions to confirm and complete this password change. +If you continue to have difficulty, contact the site administrator.'; $string['emptydragdropregion'] = 'empty region'; $string['enable'] = 'Enable'; $string['encryptedcode'] = 'Encrypted code'; @@ -1226,6 +1245,7 @@ $string['noreplybouncemessage'] = 'You have replied to a no-reply email address. Following is the content of your email:'; $string['noreplybouncesubject'] = '{$a} - bounced email.'; $string['noreplyname'] = 'Do not reply to this email'; +$string['noresetrecord'] = 'There is no record of that reset request. Please initiate a new password reset request.'; $string['noresults'] = 'No results'; $string['normal'] = 'Normal'; $string['normalfilter'] = 'Normal search'; @@ -1304,6 +1324,7 @@ $string['passwordnohelp'] = 'No help is available to find your lost password. Pl $string['passwordrecovery'] = 'Yes, help me log in'; $string['passwordsdiffer'] = 'These passwords do not match'; $string['passwordsent'] = 'Password has been sent'; +$string['passwordset'] = 'Your password has been set.'; $string['passwordsenttext'] = '

An email has been sent to your address at {$a->email}.

Please check your email for your new password

The new password was automatically generated, so you might like to @@ -1418,6 +1439,7 @@ $string['resetcomponent'] = 'Component'; $string['resetcourse'] = 'Reset course'; $string['resetinfo'] = 'This page allows you to empty a course of user data, while retaining the activities and other settings. Please be warned that by choosing items below and submitting this page you will delete your chosen user data from this course forever!'; $string['resetnotimplemented'] = 'Reset not implemented'; +$string['resetrecordexpired'] = 'The password reset link you used is more than {$a} minutes old and has expired. Please initiate a new password reset.'; $string['resetstartdate'] = 'Reset start date'; $string['resetstatus'] = 'Status'; $string['resettask'] = 'Task'; @@ -1542,6 +1564,8 @@ $string['separateandconnectedinfo'] = 'The scale based on the theory of separate $string['servererror'] = 'An error occurred whilst communicating with the server'; $string['serverlocaltime'] = 'Server\'s local time'; $string['setcategorytheme'] = 'Set category theme'; +$string['setpassword'] = 'Set Password'; +$string['setpasswordinstructions'] = 'Please enter and repeat your new password below, then click "Set Password".
Your new password will be saved, and you will be logged in.'; $string['settings'] = 'Settings'; $string['shortname'] = 'Short name'; // @deprecated MDL-34652 - use shortnamecourse or shortnameuser or some own context specific string $string['shortnamecollisionwarning'] = '[*] = This shortname is already in use by a course and will need to be changed upon approval'; diff --git a/lib/cronlib.php b/lib/cronlib.php index 0f95c752bb8..99f9cd69d58 100644 --- a/lib/cronlib.php +++ b/lib/cronlib.php @@ -173,6 +173,15 @@ function cron_run() { mtrace(' Deleting temporary files...'); cron_delete_from_temp(); + // Cleanup user password reset records + // Delete any reset request records which are expired by more than a day. + // (We keep recently expired requests around so we can give a different error msg to users who + // are trying to user a recently expired reset attempt). + $pwresettime = isset($CFG->pwresettime) ? $CFG->pwresettime : 1800; + $earliestvalid = time() - $pwresettime - DAYSECS; + $DB->delete_records_select('user_password_resets', "timerequested < ?", array($earliestvalid)); + mtrace(' Cleaned up old password reset records'); + mtrace("...finished clean-up tasks"); } // End of occasional clean-up tasks diff --git a/lib/db/install.xml b/lib/db/install.xml index b0c04d8825e..bfdcd79aab8 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -3050,5 +3050,18 @@ + + + + + + + + + + + + +
diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index d68ce2833d9..f9de797f520 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2555,5 +2555,26 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2013092001.02); } + if ($oldversion < 2013092700.02) { + // Create a new 'user_password_resets' table. + $table = new xmldb_table('user_password_resets'); + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null, null); + $table->add_field('userid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null, null); + $table->add_field('timerequested', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null, null); + $table->add_field('timererequested', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, 0, null); + $table->add_field('token', XMLDB_TYPE_CHAR, '32', null, XMLDB_NOTNULL, null, null, null); + + // Adding keys to table. + $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id')); + $table->add_key('fk_userid', XMLDB_KEY_FOREIGN, array('userid'), 'user', array('id')); + + // Conditionally launch create table. + if (!$dbman->table_exists($table)) { + $dbman->create_table($table); + } + upgrade_main_savepoint(true, 2013092700.02); + } + + return true; } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 9d1b6091fd0..769d87a9ad4 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -5947,23 +5947,27 @@ function send_confirmation_email($user) { * Sends a password change confirmation email. * * @param stdClass $user A {@link $USER} object + * @param stdClass $resetrecord An object tracking metadata regarding password reset request * @return bool Returns true if mail was sent OK and false if there was an error. */ -function send_password_change_confirmation_email($user) { +function send_password_change_confirmation_email($user, $resetrecord) { global $CFG; $site = get_site(); $supportuser = core_user::get_support_user(); + $pwresetmins = isset($CFG->pwresettime) ? floor($CFG->pwresettime / MINSECS) : 30; $data = new stdClass(); $data->firstname = $user->firstname; $data->lastname = $user->lastname; + $data->username = $user->username; $data->sitename = format_string($site->fullname); - $data->link = $CFG->httpswwwroot .'/login/forgot_password.php?p='. $user->secret .'&s='. urlencode($user->username); + $data->link = $CFG->httpswwwroot .'/login/forgot_password.php?token='. $resetrecord->token; $data->admin = generate_email_signoff(); + $data->resetminutes = $pwresetmins; - $message = get_string('emailpasswordconfirmation', '', $data); - $subject = get_string('emailpasswordconfirmationsubject', '', format_string($site->fullname)); + $message = get_string('emailresetconfirmation', '', $data); + $subject = get_string('emailresetconfirmationsubject', '', format_string($site->fullname)); // Directly email rather than using the messaging system to ensure its not routed to a popup or jabber. return email_to_user($user, $supportuser, $subject, $message); diff --git a/login/forgot_password.php b/login/forgot_password.php index fbb513dbfd5..e00906e0e61 100644 --- a/login/forgot_password.php +++ b/login/forgot_password.php @@ -28,11 +28,11 @@ require('../config.php'); require_once($CFG->libdir.'/authlib.php'); -require_once('lib.php'); +require_once(__DIR__ . '/lib.php'); require_once('forgot_password_form.php'); +require_once('set_password_form.php'); -$p_secret = optional_param('p', false, PARAM_RAW); -$p_username = optional_param('s', false, PARAM_RAW); +$token = optional_param('token', false, PARAM_RAW); //HTTPS is required in this page when $CFG->loginhttps enabled $PAGE->https_required(); @@ -60,12 +60,12 @@ if (isloggedin() and !isguestuser()) { redirect($CFG->wwwroot.'/index.php', get_string('loginalready'), 5); } -if (empty($p_secret)) { - // This is a new password reset request - // identify the user & send confirmation email +if (empty($token)) { + // This is a new password reset request. + // Process the request; identify the user & send confirmation email: forgotpw_process_request(); } else { // User clicked on confirmation link in email message // validate the token & set new password - forgotpw_process_pwset($p_secret, $p_username); + forgotpw_process_pwset($token); } diff --git a/login/lib.php b/login/lib.php index fb9a26752f7..eaa5595485d 100644 --- a/login/lib.php +++ b/login/lib.php @@ -2,6 +2,7 @@ define('PWRESET_STATUS_NOEMAILSENT', 1); define('PWRESET_STATUS_TOKENSENT', 2); define('PWRESET_STATUS_OTHEREMAILSENT', 3); +define('PWRESET_STATUS_ALREADYSENT', 4); /* This function processes a user's request to set a new password in the event they forgot the old one. If no user identifier has been supplied, it displays a form where they can submit their identifier. @@ -39,31 +40,54 @@ function forgotpw_process_request() { $pwresetstatus = PWRESET_STATUS_NOEMAILSENT; if ($user and !empty($user->confirmed)) { $userauth = get_auth_plugin($user->auth); - if ($userauth->can_reset_password() and is_enabled_auth($user->auth) - and has_capability('moodle/user:changeownpassword', $systemcontext, $user->id)) { - // send reset password confirmation - - // set 'secret' string - $user->secret = random_string(15); - $DB->set_field('user', 'secret', $user->secret, array('id'=>$user->id)); - - if (send_password_change_confirmation_email($user)) { - $pwresetstatus = PWRESET_STATUS_TOKENSENT; - } else { - print_error('cannotmailconfirm'); - } - - } else { + if (!$userauth->can_reset_password() or !is_enabled_auth($user->auth) + or !has_capability('moodle/user:changeownpassword', $systemcontext, $user->id)) { if (send_password_change_info($user)) { $pwresetstatus = PWRESET_STATUS_OTHEREMAILSENT; } else { print_error('cannotmailconfirm'); } + } else { + // The account the requesting user claims to be is entitled to change their password. + // Check if they have an existing password reset in progress: + $resetinprogress = $DB->get_record('user_password_resets', array('userid' => $user->id)); + if (empty($resetinprogress)) { + // Completely new reset request - common case + $resetrecord = create_reset_record($user); + $sendemail = true; + } elseif ($resetinprogress->timerequested < (time() - $CFG->pwresettime)) { + // Preexisting, but expired request - delete old record & create new one. + // Uncommon case - expired requests are cleaned up by cron. + $DB->delete_records('user_password_resets', array('id' => $resetinprogress->id)); + $resetrecord = create_reset_record($user); + $sendemail = true; + } elseif (empty($resetinprogress->timererequested)) { + // Preexisting, valid request. This is the first time user has re-requested the reset + // Re-sending the same email once can actually help in certain circumstances + // eg by reducing the delay caused by greylisting. + $resetinprogress->timererequested = time(); + $DB->update_record('user_password_resets', $resetinprogress); + $resetrecord = $resetinprogress; + $sendemail = true; + } else { + // Preexisting, valid request. User has already re-requested email. + $pwresetstatus = PWRESET_STATUS_ALREADYSENT; + $sendemail = false; + } + + if ($sendemail) { + $sendresult = send_password_change_confirmation_email($user, $resetrecord); + if ($sendresult) { + $pwresetstatus = PWRESET_STATUS_TOKENSENT; + } else { + print_error('cannotmailconfirm'); + } + } } } // Any email has now been sent. - // Next display results to requesting user if settings permit: + // Display results to requesting user if settings permit: echo $OUTPUT->header(); if (!empty($CFG->protectusernames)) { @@ -79,6 +103,12 @@ function forgotpw_process_request() { } elseif (empty($user->email)) { // User doesn't have an email set - can't send a password change confimation email. notice(get_string('emailpasswordconfirmnoemail'), $CFG->wwwroot.'/index.php'); + die; // never reached. + } else if ($pwresetstatus == PWRESET_STATUS_ALREADYSENT) { + // User found, protectusernames is off, but user has already (re) requested a reset. + // Don't send a 3rd reset email. + $stremailalreadysent = get_string('emailalreadysent'); + notice($stremailalreadysent, $CFG->wwwroot.'/index.php'); die; // never reached } elseif ($pwresetstatus == PWRESET_STATUS_NOEMAILSENT) { // User found, protectusernames is off, but user is not confirmed @@ -92,8 +122,8 @@ function forgotpw_process_request() { // Confirm email sent $protectedemail = preg_replace('/([^@]*)@(.*)/', '******@$2', $user->email); // obfuscate the email address to protect privacy // This is a small usability problem - may be obfuscating the email address which the user has just supplied. - $stremailpasswordconfirmsent = get_string('emailpasswordconfirmsent', '', $protectedemail); - notice($stremailpasswordconfirmsent, $CFG->wwwroot.'/index.php'); + $stremailresetconfirmsent = get_string('emailresetconfirmsent', '', $protectedemail); + notice($stremailresetconfirmsent, $CFG->wwwroot.'/index.php'); die; // never reached } die; // never reached @@ -113,61 +143,105 @@ function forgotpw_process_request() { } /* This function processes a user's submitted token to validate the request to set a new password - If the user's token is validated, they are emailed with a new password. + If the user's token is validated, they are prompted to set a new password. + * @param string $token the one-use identifier which should verify the password reset request as being valid */ -function forgotpw_process_pwset($token, $username) { - global $DB, $CFG, $OUTPUT; +function forgotpw_process_pwset($token) { + global $DB, $CFG, $OUTPUT, $PAGE, $SESSION; $systemcontext = context_system::instance(); - $user = $DB->get_record('user', array('username'=>$username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0, 'suspended'=>0)); + $pwresettime = isset($CFG->pwresettime) ? $CFG->pwresettime : 1800; + $sql = 'SELECT u.*, upr.token, upr.timerequested, upr.id as tokenid FROM ' . $CFG->prefix . 'user u' . + ' INNER JOIN ' . $CFG->prefix . 'user_password_resets upr ON upr.userid = u.id ' . + 'WHERE upr.token = ?'; + $user = $DB->get_record_sql($sql, array($token)); - if ($user and ($user->auth === 'nologin' or !is_enabled_auth($user->auth))) { - // bad luck - user is not able to login, do not let them reset password - $user = false; + $forgotpasswordurl = "{$CFG->httpswwwroot}/login/forgot_password.php"; + if (empty($user) or ($user->timerequested < (time() - $pwresettime - DAYSECS))) { + // There is no valid reset request record - not even a recently expired one. + // (suspicious) + // Direct the user to the forgot password page to request a password reset + echo $OUTPUT->header(); + notice(get_string('noresetrecord'), $forgotpasswordurl); + die; // never reached + } + if ($user->timerequested < (time() - $pwresettime)) { + // There is a reset record, but it's expired + // Direct the user to the forgot password page to request a password reset + $pwresetmins = floor($pwresettime / MINSECS); + echo $OUTPUT->header(); + notice(get_string('resetrecordexpired', '', $pwresetmins), $forgotpasswordurl); + die; // never reached } - if (!empty($user) and $user->secret === '') { - echo $OUTPUT->header(); - print_error('secretalreadyused'); - } else if (!empty($user) and $user->secret == $token) { - // make sure that url relates to a valid user - - // check this isn't guest user - if (isguestuser($user)) { - 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); - - if (!reset_password_and_mail($user)) { - print_error('cannotresetmail'); - } - - // Clear secret so that it can not be used again - $user->secret = ''; - $DB->set_field('user', 'secret', $user->secret, array('id'=>$user->id)); - - $changepasswordurl = "{$CFG->httpswwwroot}/login/change_password.php"; - $a = new stdClass(); - $a->email = $user->email; - $a->link = $changepasswordurl; - - echo $OUTPUT->header(); - notice(get_string('emailpasswordsent', '', $a), $changepasswordurl); - - } else { - if (!empty($user) and strlen($token) === 15) { - // somebody probably tries to hack in by guessing secret - stop them! - $DB->set_field('user', 'secret', '', array('id'=>$user->id)); - } + if ($user->auth === 'nologin' or !is_enabled_auth($user->auth)) { + // bad luck - user is not able to login, do not let them set password echo $OUTPUT->header(); print_error('forgotteninvalidurl'); + die; // never reached } - die; //never reached + // check this isn't guest user + if (isguestuser($user)) { + print_error('cannotresetguestpwd'); + } + + // Token is correct, and unexpired. + $mform = new login_set_password_form(null, null, 'post', '', 'autocomplete="yes"'); + $data = $mform->get_data(); + if (empty($data)) { + // User hasn't submitted form, they got here directly from email link. + // Display the form: + $setdata = new stdClass(); + $setdata->username = $user->username; + $setdata->username2 = $user->username; + $setdata->token = $user->token; + $mform->set_data($setdata); + $PAGE->verify_https_required(); + echo $OUTPUT->header(); + echo $OUTPUT->box(get_string('setpasswordinstructions'), 'generalbox boxwidthnormal boxaligncenter'); + $mform->display(); + echo $OUTPUT->footer(); + return; + } else { + // User has submitted form. + // Delete this token so it can't be used again: + $DB->delete_records('user_password_resets', array('id' => $user->tokenid)); + $userauth = get_auth_plugin($user->auth); + if (!$userauth->user_update_password($user, $data->password)) { + print_error('errorpasswordupdate', 'auth'); + } + add_to_log(SITEID, 'user', 'set password', "view.php?id=$user->id&course=" . SITEID, $user->id); + // Reset login lockout (if present) before a new password is set: + login_unlock_account($user); + // Clear any requirement to change passwords + unset_user_preference('auth_forcepasswordchange', $user); + unset_user_preference('create_password', $user); + + if (!empty($user->lang)) { + // unset previous session language - use user preference instead + unset($SESSION->lang); + } + add_to_log(SITEID, 'user', 'login', "view.php?id=$user->id&course=".SITEID, $user->id, 0, $user->id); + complete_user_login($user); + $urltogo = get_postlogin_redirection(); + unset($SESSION->wantsurl); + redirect($urltogo, get_string('passwordset'), 1); + } +} + +/* + * Create a new record in the database to track a new password set request for user + * @param object $user the user record, the requester would like a new password set for. + * @return record created +*/ +function create_reset_record ($user) { + global $CFG, $DB; + $resetrecord = new stdClass(); + $resetrecord->timerequested = time(); + $resetrecord->userid = $user->id; + $resetrecord->token = random_string(32); + $resetrecord->id = $DB->insert_record('user_password_resets', $resetrecord); + return $resetrecord; } /* Determine where a user should be redirected after they have been logged in diff --git a/login/set_password_form.php b/login/set_password_form.php new file mode 100644 index 00000000000..e68f0941aab --- /dev/null +++ b/login/set_password_form.php @@ -0,0 +1,86 @@ +. + +/** + * Set password form definition. + * + * @package core + * @subpackage auth + * @copyright 2006 Petr Skoda {@link http://skodak.org} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +require_once $CFG->libdir.'/formslib.php'; + +class login_set_password_form extends moodleform { + + function definition() { + global $USER, $CFG; + + $mform = $this->_form; + $mform->setDisableShortforms(true); + $mform->addElement('header', 'setpassword', get_string('setpassword'), ''); + + // Include the username in the form so browsers will recognise that a password is being set. + $mform->addElement('text', 'username','','style="display: none;" autocomplete="on"'); + $mform->setType('username', PARAM_RAW); + // Token gives authority to change password. + $mform->addElement('hidden', 'token', ''); + $mform->setType('token', PARAM_ALPHANUM); + + // visible elements + $mform->addElement('static', 'username2', get_string('username')); + + if (!empty($CFG->passwordpolicy)){ + $mform->addElement('static', 'passwordpolicyinfo', '', print_password_policy()); + } + $mform->addElement('password', 'password', get_string('newpassword'), 'autocomplete="on"'); + $mform->addRule('password', get_string('required'), 'required', null, 'client'); + $mform->setType('password', PARAM_RAW); + + $mform->addElement('password', 'password2', get_string('newpassword').' ('.get_String('again').')', 'autocomplete="on"'); + $mform->addRule('password2', get_string('required'), 'required', null, 'client'); + $mform->setType('password2', PARAM_RAW); + + // buttons + $this->add_action_buttons(true); + } + + // perform extra password change validation + function validation($data, $files) { + global $USER; + $errors = parent::validation($data, $files); + + // Ignore submitted username. + // TODO: Validate submitted token. + if ($data['password'] <> $data['password2']) { + $errors['password'] = get_string('passwordsdiffer'); + $errors['password2'] = get_string('passwordsdiffer'); + return $errors; + } + + $errmsg = '';//prevents eclipse warnings + if (!check_password_policy($data['password'], $errmsg)) { + $errors['password'] = $errmsg; + $errors['password2'] = $errmsg; + return $errors; + } + + return $errors; + } +} diff --git a/version.php b/version.php index 14acbaabe85..8b32c0f3d15 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2013092700.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2013092700.02; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. From 2f0dd8d5bd8022c707099e8bdd2d7340073e128b Mon Sep 17 00:00:00 2001 From: Peter Bulmer Date: Mon, 5 Aug 2013 14:14:55 +1200 Subject: [PATCH 7/8] MDL-23692 forgotpw: Coding style tidyups. --- admin/settings/security.php | 14 +-- lang/en/moodle.php | 6 +- lib/db/upgrade.php | 1 - login/forgot_password.php | 9 +- login/forgot_password_form.php | 27 ++++- login/index.php | 2 +- login/lib.php | 198 +++++++++++++++++++-------------- login/set_password_form.php | 40 +++++-- 8 files changed, 177 insertions(+), 120 deletions(-) diff --git a/admin/settings/security.php b/admin/settings/security.php index acb30489094..62712e37ea3 100644 --- a/admin/settings/security.php +++ b/admin/settings/security.php @@ -71,13 +71,13 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page $temp->add(new admin_setting_configtext('minpasswordnonalphanum', new lang_string('minpasswordnonalphanum', 'admin'), new lang_string('configminpasswordnonalphanum', 'admin'), 1, PARAM_INT)); $temp->add(new admin_setting_configtext('maxconsecutiveidentchars', new lang_string('maxconsecutiveidentchars', 'admin'), new lang_string('configmaxconsecutiveidentchars', 'admin'), 0, PARAM_INT)); $pwresetoptions = array( - 300 => new lang_string('numminutes', '', 5), - 900 => new lang_string('numminutes', '', 15), - 1800 => new lang_string('numminutes', '', 30), - 2700 => new lang_string('numminutes', '', 45), - 3600 => new lang_string('numminutes', '', 60), - 7200 => new lang_string('numminutes', '', 120), - 14400 => new lang_string('numminutes', '', 240) + 300 => new lang_string('numminutes', '', 5), + 900 => new lang_string('numminutes', '', 15), + 1800 => new lang_string('numminutes', '', 30), + 2700 => new lang_string('numminutes', '', 45), + 3600 => new lang_string('numminutes', '', 60), + 7200 => new lang_string('numminutes', '', 120), + 14400 => new lang_string('numminutes', '', 240) ); $adminsetting = new admin_setting_configselect( 'pwresettime', diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 691b3cc737b..dc16b7c733e 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -535,7 +535,7 @@ $string['editthisactivity'] = 'Edit this activity'; $string['editthiscategory'] = 'Edit this category'; $string['edituser'] = 'Edit user accounts'; $string['email'] = 'Email address'; -$string['emailalreadysent'] = 'A Password reset email has already been sent. Please check your email.'; +$string['emailalreadysent'] = 'A password reset email has already been sent. Please check your email.'; $string['emailactive'] = 'Email activated'; $string['emailagain'] = 'Email (again)'; $string['emailconfirm'] = 'Confirm your account'; @@ -1564,8 +1564,8 @@ $string['separateandconnectedinfo'] = 'The scale based on the theory of separate $string['servererror'] = 'An error occurred whilst communicating with the server'; $string['serverlocaltime'] = 'Server\'s local time'; $string['setcategorytheme'] = 'Set category theme'; -$string['setpassword'] = 'Set Password'; -$string['setpasswordinstructions'] = 'Please enter and repeat your new password below, then click "Set Password".
Your new password will be saved, and you will be logged in.'; +$string['setpassword'] = 'Set password'; +$string['setpasswordinstructions'] = 'Please enter and repeat your new password below, then click "Set password".
Your new password will be saved, and you will be logged in.'; $string['settings'] = 'Settings'; $string['shortname'] = 'Short name'; // @deprecated MDL-34652 - use shortnamecourse or shortnameuser or some own context specific string $string['shortnamecollisionwarning'] = '[*] = This shortname is already in use by a course and will need to be changed upon approval'; diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index f9de797f520..f8885d4415a 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2575,6 +2575,5 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2013092700.02); } - return true; } diff --git a/login/forgot_password.php b/login/forgot_password.php index e00906e0e61..883eb98c5c3 100644 --- a/login/forgot_password.php +++ b/login/forgot_password.php @@ -1,5 +1,4 @@ loginhttps enabled $PAGE->https_required(); @@ -62,10 +61,10 @@ if (isloggedin() and !isguestuser()) { if (empty($token)) { // This is a new password reset request. - // Process the request; identify the user & send confirmation email: - forgotpw_process_request(); + // Process the request; identify the user & send confirmation email. + core_login_process_password_reset_request(); } else { // User clicked on confirmation link in email message // validate the token & set new password - forgotpw_process_pwset($token); + core_login_process_password_set($token); } diff --git a/login/forgot_password_form.php b/login/forgot_password_form.php index 997dcd553cd..84272c115ad 100644 --- a/login/forgot_password_form.php +++ b/login/forgot_password_form.php @@ -1,5 +1,4 @@ . +/** + * Forgot password page. + * + * @package core + * @subpackage auth + * @copyright 1999 onwards Martin Dougiamas http://dougiamas.com + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->libdir.'/formslib.php'); + /** * Reset forgotten password form definition. * @@ -23,13 +34,11 @@ * @copyright 2006 Petr Skoda {@link http://skodak.org} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ - -defined('MOODLE_INTERNAL') || die(); - -require_once $CFG->libdir.'/formslib.php'; - class login_forgot_password_form extends moodleform { + /** + * Define the forgot password form. + */ function definition() { $mform = $this->_form; $mform->setDisableShortforms(true); @@ -51,6 +60,12 @@ class login_forgot_password_form extends moodleform { $mform->addElement('submit', 'submitbuttonemail', $submitlabel); } + /** + * Validate user input from the forgot password form. + * @param array $data array of submitted form fields. + * @param array $files submitted with the form. + * @return array errors occuring during validation. + */ function validation($data, $files) { global $CFG, $DB; diff --git a/login/index.php b/login/index.php index def89e973f7..a4e29fc9456 100644 --- a/login/index.php +++ b/login/index.php @@ -194,7 +194,7 @@ if ($frm and isset($frm->username)) { // Login WITH set_moodle_cookie($USER->username); } - $urltogo = get_postlogin_redirection(); + $urltogo = core_login_get_return_url(); /// check if user password has expired /// Currently supported only for ldap-authentication module diff --git a/login/lib.php b/login/lib.php index eaa5595485d..ab1ef1f83c2 100644 --- a/login/lib.php +++ b/login/lib.php @@ -1,14 +1,40 @@ . + +/** + * + * Login library file of login/password related Moodle functions. + * + * @package core + * @subpackage lib + * @copyright Catalyst IT + * @copyright Peter Bulmer + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ define('PWRESET_STATUS_NOEMAILSENT', 1); define('PWRESET_STATUS_TOKENSENT', 2); define('PWRESET_STATUS_OTHEREMAILSENT', 3); define('PWRESET_STATUS_ALREADYSENT', 4); -/* This function processes a user's request to set a new password in the event they forgot the old one. - If no user identifier has been supplied, it displays a form where they can submit their identifier. - Where they have supplied identifier, the function will check their status, and send email as appropriate. -*/ -function forgotpw_process_request() { +/** + * Processes a user's request to set a new password in the event they forgot the old one. + * If no user identifier has been supplied, it displays a form where they can submit their identifier. + * Where they have supplied identifier, the function will check their status, and send email as appropriate. + */ +function core_login_process_password_reset_request() { global $DB, $OUTPUT, $CFG, $PAGE; $systemcontext = context_system::instance(); $mform = new login_forgot_password_form(); @@ -18,25 +44,26 @@ function forgotpw_process_request() { } else if ($data = $mform->get_data()) { // Requesting user has submitted form data. - // Find the user account in the database which the requesting user claims to own: + // Next find the user account in the database which the requesting user claims to own. if (!empty($data->username)) { // Username has been specified - load the user record based on that. - $username = core_text::strtolower($data->username); // mimic the login page process, if they forget username they need to use email for reset - $user = $DB->get_record('user', array('username'=>$username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0, 'suspended'=>0)); - + $username = core_text::strtolower($data->username); // Mimic the login page process. + $userparams = array('username' => $username, 'mnethostid' => $CFG->mnet_localhost_id, 'deleted' => 0, 'suspended' => 0); + $user = $DB->get_record('user', $userparams); } else { - // Try to load the user record based on email address: + // Try to load the user record based on email address. // this is tricky because - // 1/ the email is not guaranteed to be unique - TODO: send email with all usernames to select the correct account for pw reset - // 2/ mailbox may be case sensitive, the email domain is case insensitive - let's pretend it is all case-insensitive + // 1/ the email is not guaranteed to be unique - TODO: send email with all usernames to select the account for pw reset + // 2/ mailbox may be case sensitive, the email domain is case insensitive - let's pretend it is all case-insensitive. - $select = $DB->sql_like('email', ':email', false, true, false, '|'). " AND mnethostid = :mnethostid AND deleted=0 AND suspended=0"; - $params = array('email'=>$DB->sql_like_escape($data->email, '|'), 'mnethostid'=>$CFG->mnet_localhost_id); + $select = $DB->sql_like('email', ':email', false, true, false, '|') . + " AND mnethostid = :mnethostid AND deleted=0 AND suspended=0"; + $params = array('email' => $DB->sql_like_escape($data->email, '|'), 'mnethostid' => $CFG->mnet_localhost_id); $user = $DB->get_record_select('user', $select, $params, '*', IGNORE_MULTIPLE); } // Target user details have now been identified, or we know that there is no such account. - // Send email address to account's email address if appropriate: + // Send email address to account's email address if appropriate. $pwresetstatus = PWRESET_STATUS_NOEMAILSENT; if ($user and !empty($user->confirmed)) { $userauth = get_auth_plugin($user->auth); @@ -49,20 +76,20 @@ function forgotpw_process_request() { } } else { // The account the requesting user claims to be is entitled to change their password. - // Check if they have an existing password reset in progress: + // Next, check if they have an existing password reset in progress. $resetinprogress = $DB->get_record('user_password_resets', array('userid' => $user->id)); if (empty($resetinprogress)) { - // Completely new reset request - common case - $resetrecord = create_reset_record($user); + // Completely new reset request - common case. + $resetrecord = core_login_generate_password_reset($user); $sendemail = true; - } elseif ($resetinprogress->timerequested < (time() - $CFG->pwresettime)) { + } else if ($resetinprogress->timerequested < (time() - $CFG->pwresettime)) { // Preexisting, but expired request - delete old record & create new one. // Uncommon case - expired requests are cleaned up by cron. $DB->delete_records('user_password_resets', array('id' => $resetinprogress->id)); - $resetrecord = create_reset_record($user); + $resetrecord = core_login_generate_password_reset($user); $sendemail = true; - } elseif (empty($resetinprogress->timererequested)) { - // Preexisting, valid request. This is the first time user has re-requested the reset + } else if (empty($resetinprogress->timererequested)) { + // Preexisting, valid request. This is the first time user has re-requested the reset. // Re-sending the same email once can actually help in certain circumstances // eg by reducing the delay caused by greylisting. $resetinprogress->timererequested = time(); @@ -87,53 +114,53 @@ function forgotpw_process_request() { } // Any email has now been sent. - // Display results to requesting user if settings permit: + // Next display results to requesting user if settings permit. echo $OUTPUT->header(); if (!empty($CFG->protectusernames)) { // Neither confirm, nor deny existance of any username or email address in database. - // Print general (non-commital) message + // Print general (non-commital) message. notice(get_string('emailpasswordconfirmmaybesent'), $CFG->wwwroot.'/index.php'); - die; // never reached - } elseif (empty($user)) { + die; // Never reached. + } else if (empty($user)) { // Protect usernames is off, and we couldn't find the user with details specified. - // Print failure advice: + // Print failure advice. notice(get_string('emailpasswordconfirmnotsent'), $CFG->wwwroot.'/forgot_password.php'); - die; // never reached - } elseif (empty($user->email)) { + die; // Never reached. + } else if (empty($user->email)) { // User doesn't have an email set - can't send a password change confimation email. notice(get_string('emailpasswordconfirmnoemail'), $CFG->wwwroot.'/index.php'); - die; // never reached. + die; // Never reached. } else if ($pwresetstatus == PWRESET_STATUS_ALREADYSENT) { // User found, protectusernames is off, but user has already (re) requested a reset. // Don't send a 3rd reset email. $stremailalreadysent = get_string('emailalreadysent'); notice($stremailalreadysent, $CFG->wwwroot.'/index.php'); - die; // never reached - } elseif ($pwresetstatus == PWRESET_STATUS_NOEMAILSENT) { - // User found, protectusernames is off, but user is not confirmed - // Pretend we sent them an email - // This is a big usability problem - need to tell users why we didn't send them an email - $protectedemail = preg_replace('/([^@]*)@(.*)/', '******@$2', $user->email); // obfuscate the email address to protect privacy + die; // Never reached. + } else if ($pwresetstatus == PWRESET_STATUS_NOEMAILSENT) { + // User found, protectusernames is off, but user is not confirmed. + // Pretend we sent them an email. + // This is a big usability problem - need to tell users why we didn't send them an email. + // Obfuscate email address to protect privacy. + $protectedemail = preg_replace('/([^@]*)@(.*)/', '******@$2', $user->email); $stremailpasswordconfirmsent = get_string('emailpasswordconfirmsent', '', $protectedemail); notice($stremailpasswordconfirmsent, $CFG->wwwroot.'/index.php'); - die; // never reached + die; // Never reached. } else { - // Confirm email sent - $protectedemail = preg_replace('/([^@]*)@(.*)/', '******@$2', $user->email); // obfuscate the email address to protect privacy + // Confirm email sent. (Obfuscate email address to protect privacy). + $protectedemail = preg_replace('/([^@]*)@(.*)/', '******@$2', $user->email); // This is a small usability problem - may be obfuscating the email address which the user has just supplied. $stremailresetconfirmsent = get_string('emailresetconfirmsent', '', $protectedemail); notice($stremailresetconfirmsent, $CFG->wwwroot.'/index.php'); - die; // never reached + die; // Never reached. } - die; // never reached + die; // Never reached. } - // make sure we really are on the https page when https login required + // Make sure we really are on the https page when https login required. $PAGE->verify_https_required(); - - /// DISPLAY FORM + // DISPLAY FORM. echo $OUTPUT->header(); echo $OUTPUT->box(get_string('passwordforgotteninstructions2'), 'generalbox boxwidthnormal boxaligncenter'); @@ -142,45 +169,46 @@ function forgotpw_process_request() { echo $OUTPUT->footer(); } -/* This function processes a user's submitted token to validate the request to set a new password - If the user's token is validated, they are prompted to set a new password. - * @param string $token the one-use identifier which should verify the password reset request as being valid -*/ -function forgotpw_process_pwset($token) { +/** This function processes a user's submitted token to validate the request to set a new password. + * If the user's token is validated, they are prompted to set a new password. + * @param string $token the one-use identifier which should verify the password reset request as being valid. + * @return null + */ +function core_login_process_password_set($token) { global $DB, $CFG, $OUTPUT, $PAGE, $SESSION; - $systemcontext = context_system::instance(); $pwresettime = isset($CFG->pwresettime) ? $CFG->pwresettime : 1800; - $sql = 'SELECT u.*, upr.token, upr.timerequested, upr.id as tokenid FROM ' . $CFG->prefix . 'user u' . - ' INNER JOIN ' . $CFG->prefix . 'user_password_resets upr ON upr.userid = u.id ' . - 'WHERE upr.token = ?'; + $sql = "SELECT u.*, upr.token, upr.timerequested, upr.id as tokenid + FROM {user} u + JOIN {user_password_resets} upr ON upr.userid = u.id + WHERE upr.token = ?"; $user = $DB->get_record_sql($sql, array($token)); $forgotpasswordurl = "{$CFG->httpswwwroot}/login/forgot_password.php"; if (empty($user) or ($user->timerequested < (time() - $pwresettime - DAYSECS))) { // There is no valid reset request record - not even a recently expired one. // (suspicious) - // Direct the user to the forgot password page to request a password reset + // Direct the user to the forgot password page to request a password reset. echo $OUTPUT->header(); notice(get_string('noresetrecord'), $forgotpasswordurl); - die; // never reached + die; // Never reached. } if ($user->timerequested < (time() - $pwresettime)) { - // There is a reset record, but it's expired - // Direct the user to the forgot password page to request a password reset + // There is a reset record, but it's expired. + // Direct the user to the forgot password page to request a password reset. $pwresetmins = floor($pwresettime / MINSECS); echo $OUTPUT->header(); notice(get_string('resetrecordexpired', '', $pwresetmins), $forgotpasswordurl); - die; // never reached + die; // Never reached. } if ($user->auth === 'nologin' or !is_enabled_auth($user->auth)) { - // bad luck - user is not able to login, do not let them set password + // Bad luck - user is not able to login, do not let them set password. echo $OUTPUT->header(); print_error('forgotteninvalidurl'); - die; // never reached + die; // Never reached. } - // check this isn't guest user + // Check this isn't guest user. if (isguestuser($user)) { print_error('cannotresetguestpwd'); } @@ -190,7 +218,7 @@ function forgotpw_process_pwset($token) { $data = $mform->get_data(); if (empty($data)) { // User hasn't submitted form, they got here directly from email link. - // Display the form: + // Next, display the form. $setdata = new stdClass(); $setdata->username = $user->username; $setdata->username2 = $user->username; @@ -204,38 +232,37 @@ function forgotpw_process_pwset($token) { return; } else { // User has submitted form. - // Delete this token so it can't be used again: + // Delete this token so it can't be used again. $DB->delete_records('user_password_resets', array('id' => $user->tokenid)); $userauth = get_auth_plugin($user->auth); if (!$userauth->user_update_password($user, $data->password)) { print_error('errorpasswordupdate', 'auth'); } add_to_log(SITEID, 'user', 'set password', "view.php?id=$user->id&course=" . SITEID, $user->id); - // Reset login lockout (if present) before a new password is set: + // Reset login lockout (if present) before a new password is set. login_unlock_account($user); - // Clear any requirement to change passwords + // Clear any requirement to change passwords. unset_user_preference('auth_forcepasswordchange', $user); unset_user_preference('create_password', $user); if (!empty($user->lang)) { - // unset previous session language - use user preference instead + // Unset previous session language - use user preference instead. unset($SESSION->lang); } add_to_log(SITEID, 'user', 'login', "view.php?id=$user->id&course=".SITEID, $user->id, 0, $user->id); complete_user_login($user); - $urltogo = get_postlogin_redirection(); + $urltogo = core_login_get_return_url(); unset($SESSION->wantsurl); redirect($urltogo, get_string('passwordset'), 1); } } -/* - * Create a new record in the database to track a new password set request for user +/** Create a new record in the database to track a new password set request for user. * @param object $user the user record, the requester would like a new password set for. - * @return record created -*/ -function create_reset_record ($user) { - global $CFG, $DB; + * @return record created. + */ +function core_login_generate_password_reset ($user) { + global $DB; $resetrecord = new stdClass(); $resetrecord->timerequested = time(); $resetrecord->userid = $user->id; @@ -244,29 +271,31 @@ function create_reset_record ($user) { return $resetrecord; } -/* Determine where a user should be redirected after they have been logged in -*/ -function get_postlogin_redirection() { +/** Determine where a user should be redirected after they have been logged in. + * @return string url the user should be redirected to. + */ +function core_login_get_return_url() { global $CFG, $SESSION, $USER; - /// Prepare redirection + // Prepare redirection. if (user_not_fully_set_up($USER)) { $urltogo = $CFG->wwwroot.'/user/edit.php'; - // We don't delete $SESSION->wantsurl yet, so we get there later + // We don't delete $SESSION->wantsurl yet, so we get there later. - } else if (isset($SESSION->wantsurl) and (strpos($SESSION->wantsurl, $CFG->wwwroot) === 0 or strpos($SESSION->wantsurl, str_replace('http://', 'https://', $CFG->wwwroot)) === 0)) { - $urltogo = $SESSION->wantsurl; /// Because it's an address in this site + } else if (isset($SESSION->wantsurl) and (strpos($SESSION->wantsurl, $CFG->wwwroot) === 0 + or strpos($SESSION->wantsurl, str_replace('http://', 'https://', $CFG->wwwroot)) === 0)) { + $urltogo = $SESSION->wantsurl; // Because it's an address in this site. unset($SESSION->wantsurl); } else { - // no wantsurl stored or external - go to homepage + // No wantsurl stored or external - go to homepage. $urltogo = $CFG->wwwroot.'/'; unset($SESSION->wantsurl); } // If the url to go to is the same as the site page, check for default homepage. if ($urltogo == ($CFG->wwwroot . '/')) { - $home_page = get_home_page(); - // Go to my-moodle page instead of site homepage if defaulthomepage set to homepage_my - if ($home_page == HOMEPAGE_MY && !is_siteadmin() && !isguestuser()) { + $homepage = get_home_page(); + // Go to my-moodle page instead of site homepage if defaulthomepage set to homepage_my. + if ($homepage == HOMEPAGE_MY && !is_siteadmin() && !isguestuser()) { if ($urltogo == $CFG->wwwroot or $urltogo == $CFG->wwwroot.'/' or $urltogo == $CFG->wwwroot.'/index.php') { $urltogo = $CFG->wwwroot.'/my/'; } @@ -274,4 +303,3 @@ function get_postlogin_redirection() { } return $urltogo; } -?> diff --git a/login/set_password_form.php b/login/set_password_form.php index e68f0941aab..8cfb6ab6eea 100644 --- a/login/set_password_form.php +++ b/login/set_password_form.php @@ -25,11 +25,23 @@ defined('MOODLE_INTERNAL') || die(); -require_once $CFG->libdir.'/formslib.php'; +require_once($CFG->libdir.'/formslib.php'); +/** + * Set forgotten password form definition. + * + * @package core + * @subpackage auth + * @copyright 2006 Petr Skoda {@link http://skodak.org} + * @copyright 2013 Peter Bulmer + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ class login_set_password_form extends moodleform { - function definition() { + /** + * Define the set password form. + */ + public function definition() { global $USER, $CFG; $mform = $this->_form; @@ -37,44 +49,48 @@ class login_set_password_form extends moodleform { $mform->addElement('header', 'setpassword', get_string('setpassword'), ''); // Include the username in the form so browsers will recognise that a password is being set. - $mform->addElement('text', 'username','','style="display: none;" autocomplete="on"'); + $mform->addElement('text', 'username', '', 'style="display: none;" autocomplete="on"'); $mform->setType('username', PARAM_RAW); // Token gives authority to change password. $mform->addElement('hidden', 'token', ''); $mform->setType('token', PARAM_ALPHANUM); - // visible elements + // Visible elements. $mform->addElement('static', 'username2', get_string('username')); - if (!empty($CFG->passwordpolicy)){ + if (!empty($CFG->passwordpolicy)) { $mform->addElement('static', 'passwordpolicyinfo', '', print_password_policy()); } $mform->addElement('password', 'password', get_string('newpassword'), 'autocomplete="on"'); $mform->addRule('password', get_string('required'), 'required', null, 'client'); $mform->setType('password', PARAM_RAW); - $mform->addElement('password', 'password2', get_string('newpassword').' ('.get_String('again').')', 'autocomplete="on"'); + $strpasswordagain = get_string('newpassword') . ' (' . get_string('again') . ')'; + $mform->addElement('password', 'password2', $strpasswordagain, 'autocomplete="on"'); $mform->addRule('password2', get_string('required'), 'required', null, 'client'); $mform->setType('password2', PARAM_RAW); - // buttons $this->add_action_buttons(true); } - // perform extra password change validation - function validation($data, $files) { + /** + * Perform extra password change validation. + * @param array $data submitted form fields. + * @param array $files submitted with the form. + * @return array errors occuring during validation. + */ + public function validation($data, $files) { global $USER; $errors = parent::validation($data, $files); // Ignore submitted username. - // TODO: Validate submitted token. - if ($data['password'] <> $data['password2']) { + if ($data['password'] !== $data['password2']) { $errors['password'] = get_string('passwordsdiffer'); $errors['password2'] = get_string('passwordsdiffer'); return $errors; } - $errmsg = '';//prevents eclipse warnings + $errmsg = ''; // Prevents eclipse warnings. if (!check_password_policy($data['password'], $errmsg)) { $errors['password'] = $errmsg; $errors['password2'] = $errmsg; From 60a67d78e865b4184e40861b9507338faea3fe3e Mon Sep 17 00:00:00 2001 From: Peter Bulmer Date: Mon, 7 Oct 2013 23:29:29 +1300 Subject: [PATCH 8/8] MDL-23692 forgotpw:adhere to site autocomp setting --- login/set_password_form.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/login/set_password_form.php b/login/set_password_form.php index 8cfb6ab6eea..62e8b5c0e3e 100644 --- a/login/set_password_form.php +++ b/login/set_password_form.php @@ -43,13 +43,19 @@ class login_set_password_form extends moodleform { */ public function definition() { global $USER, $CFG; + // Prepare a string showing whether the site wants login password autocompletion to be available to user. + if (empty($CFG->loginpasswordautocomplete)) { + $autocomplete = 'autocomplete="on"'; + } else { + $autocomplete = ''; + } $mform = $this->_form; $mform->setDisableShortforms(true); $mform->addElement('header', 'setpassword', get_string('setpassword'), ''); // Include the username in the form so browsers will recognise that a password is being set. - $mform->addElement('text', 'username', '', 'style="display: none;" autocomplete="on"'); + $mform->addElement('text', 'username', '', 'style="display: none;" ' . $autocomplete); $mform->setType('username', PARAM_RAW); // Token gives authority to change password. $mform->addElement('hidden', 'token', ''); @@ -61,12 +67,12 @@ class login_set_password_form extends moodleform { if (!empty($CFG->passwordpolicy)) { $mform->addElement('static', 'passwordpolicyinfo', '', print_password_policy()); } - $mform->addElement('password', 'password', get_string('newpassword'), 'autocomplete="on"'); + $mform->addElement('password', 'password', get_string('newpassword'), $autocomplete); $mform->addRule('password', get_string('required'), 'required', null, 'client'); $mform->setType('password', PARAM_RAW); $strpasswordagain = get_string('newpassword') . ' (' . get_string('again') . ')'; - $mform->addElement('password', 'password2', $strpasswordagain, 'autocomplete="on"'); + $mform->addElement('password', 'password2', $strpasswordagain, $autocomplete); $mform->addRule('password2', get_string('required'), 'required', null, 'client'); $mform->setType('password2', PARAM_RAW);