Skip to content

Commit

Permalink
MDL-71068 login: Fix edge cases with $CFG->protectusernames
Browse files Browse the repository at this point in the history
  • Loading branch information
brendanheywood committed Mar 17, 2021
1 parent fc335f5 commit 7a825b6
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 11 deletions.
2 changes: 1 addition & 1 deletion lang/en/moodle.php
Expand Up @@ -683,7 +683,7 @@
If you need help, please contact the site administrator,
{$a->admin}';
$string['emailpasswordconfirmationsubject'] = '{$a}: Change password confirmation';
$string['emailpasswordconfirmmaybesent'] = '<p>If you supplied a correct username or email address then an email should have been sent to you.</p>
$string['emailpasswordconfirmmaybesent'] = '<p>If you supplied a correct username or unique email address then an email should have been sent to you.</p>
<p>It contains easy instructions to confirm and complete this password change.
If you continue to have difficulty, please contact the site administrator.</p>';
$string['emailpasswordconfirmnoemail'] = '<p>The user account you specified does not have a recorded email address.</p>
Expand Down
12 changes: 9 additions & 3 deletions login/lib.php
Expand Up @@ -387,7 +387,9 @@ function core_login_validate_forgot_password_data($data) {
$user = get_complete_user_data('email', $data['email'], null, true);
if (empty($user->confirmed)) {
send_confirmation_email($user);
$errors['email'] = get_string('confirmednot');
if (empty($CFG->protectusernames)) {
$errors['email'] = get_string('confirmednot');
}
}
} catch (dml_missing_record_exception $missingexception) {
// User not found. Show error when $CFG->protectusernames is turned off.
Expand All @@ -396,15 +398,19 @@ function core_login_validate_forgot_password_data($data) {
}
} catch (dml_multiple_records_exception $multipleexception) {
// Multiple records found. Ask the user to enter a username instead.
$errors['email'] = get_string('forgottenduplicate');
if (empty($CFG->protectusernames)) {
$errors['email'] = get_string('forgottenduplicate');
}
}
}

} else {
if ($user = get_complete_user_data('username', $data['username'])) {
if (empty($user->confirmed)) {
send_confirmation_email($user);
$errors['email'] = get_string('confirmednot');
if (empty($CFG->protectusernames)) {
$errors['username'] = get_string('confirmednot');
}
}
}
if (!$user and empty($CFG->protectusernames)) {
Expand Down
29 changes: 22 additions & 7 deletions login/tests/lib_test.php
Expand Up @@ -257,24 +257,34 @@ public function forgot_password_data_provider() {
['username' => get_string('usernamenotfound')],
['protectusernames' => 0]
],
'Valid username, unconfirmed username' => [
'Valid username, unconfirmed username, username protection on' => [
['username' => 's1'],
['email' => get_string('confirmednot')],
[],
['confirmed' => 0]
],
'Invalid email' => [
['email' => 's1-example.com'],
['email' => get_string('invalidemail')]
],
'Multiple accounts with the same email' => [
'Multiple accounts with the same email, username protection on' => [
['email' => 's1@example.com'],
[],
['allowaccountssameemail' => 1]
],
'Multiple accounts with the same email, username protection off' => [
['email' => 's1@example.com'],
['email' => get_string('forgottenduplicate')],
['allowaccountssameemail' => 1, 'protectusernames' => 0]
],
'Multiple accounts with the same email but with different case, username protection is on' => [
['email' => 'S1@EXAMPLE.COM'],
[],
['allowaccountssameemail' => 1]
],
'Multiple accounts with the same email but with different case' => [
'Multiple accounts with the same email but with different case, username protection is off' => [
['email' => 'S1@EXAMPLE.COM'],
['email' => get_string('forgottenduplicate')],
['allowaccountssameemail' => 1]
['allowaccountssameemail' => 1, 'protectusernames' => 0]
],
'Non-existent email, username protection on' => [
['email' => 's2@example.com']
Expand All @@ -290,11 +300,16 @@ public function forgot_password_data_provider() {
'Valid email, different case' => [
['email' => 'S1@EXAMPLE.COM']
],
'Valid email, unconfirmed user' => [
'Valid email, unconfirmed user, username protection is on' => [
['email' => 's1@example.com'],
['email' => get_string('confirmednot')],
[],
['confirmed' => 0]
],
'Valid email, unconfirmed user, username protection is off' => [
['email' => 's1@example.com'],
['email' => get_string('confirmednot')],
['confirmed' => 0, 'protectusernames' => 0]
],
];
}

Expand Down

0 comments on commit 7a825b6

Please sign in to comment.