mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-13 12:32:08 +02:00
Code Issues Projects Releases Wiki Activity

MDL-68183 auth: Fix the performance of forgotten password user search

Browse Source 
When searching for the user matching the given email address, we perform
the case-insensitive and accent-sensitive search. That may be expensive
as some DBs such as MySQL cannot use the index in that case. Instead,
sequential scan of all the user records is performed and the comparison
uses the LOWER function to filter the matching records. This leads to
significant performance heavy queries which in turn represent a surface
for DoS attacks.

For that reason, we first perform accent-insensitive search for
potential candidates, which can use the index. Only then we perform the
additional accent-sensitive search on this limited set or records.
This commit is contained in:
David Mudrák 2020-03-16 22:39:49 +01:00
parent 73f8c56dfc
commit 77bc884473
2 changed files with 96 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
login/lib.php
View File

@ -24,6 +24,9 @@
* @copyright Peter Bulmer
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
defined('MOODLE_INTERNAL') || die();
define('PWRESET_STATUS_NOEMAILSENT', 1);
define('PWRESET_STATUS_TOKENSENT', 2);
define('PWRESET_STATUS_OTHEREMAILSENT', 3);
@ -93,14 +96,31 @@ function core_login_process_password_reset($username, $email) {
$user = $DB->get_record('user', $userparams);
} else {
// Try to load the user record based on email address.
// this is tricky because
// This is tricky because:
// 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.
//
// The case-insensitive + accent-sensitive search may be expensive as some DBs such as MySQL cannot use the
// index in that case. For that reason, we first perform accent-insensitive search in a subselect for potential
// candidates (which can use the index) and only then perform the additional accent-sensitive search on this
// limited set of records in the outer select.
$sql = "SELECT *
FROM {user}
WHERE " . $DB->sql_equal('email', ':email1', false, true) . "
AND id IN (SELECT id
FROM {user}
WHERE mnethostid = :mnethostid
AND deleted = 0
AND suspended = 0
AND " . $DB->sql_equal('email', ':email2', false, false) . ")";
$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($email, '|'), 'mnethostid' => $CFG->mnet_localhost_id);
$user = $DB->get_record_select('user', $select, $params, '*', IGNORE_MULTIPLE);
$params = array(
'email1' => $email,
'email2' => $email,
'mnethostid' => $CFG->mnet_localhost_id,
);
$user = $DB->get_record_sql($sql, $params, IGNORE_MULTIPLE);
}
// Target user details have now been identified, or we know that there is no such account.

71
login/tests/lib_test.php
View File

@ -355,4 +355,75 @@ class core_login_lib_testcase extends advanced_testcase {
$this->assertArrayNotHasKey('email', $validationerrors);
}
}
/**
* Test searching for the user record by matching the provided email address when resetting password.
*
* Email addresses should be handled as case-insensitive but accent sensitive.
*/
public function test_core_login_process_password_reset_email_sensitivity() {
global $CFG;
require_once($CFG->libdir.'/phpmailer/moodle_phpmailer.php');
$this->resetAfterTest();
$sink = $this->redirectEmails();
$CFG->protectusernames = 0;
// In this test, we need to mock sending emails on non-ASCII email addresses. However, such email addresses do
// not pass the default `validate_email()` and Moodle does not yet provide a CFG switch to allow such emails.
// So we inject our own validation method here and revert it back once we are done. This custom validator method
// is identical to the default 'php' validator with the only difference: it has the FILTER_FLAG_EMAIL_UNICODE
// set so that it allows to use non-ASCII characters in email addresses.
$defaultvalidator = moodle_phpmailer::$validator;
moodle_phpmailer::$validator = function($address) {
return (bool) filter_var($address, FILTER_VALIDATE_EMAIL, FILTER_FLAG_EMAIL_UNICODE);
};
// Emails are treated as case-insensitive when searching for the matching user account.
$u1 = $this->getDataGenerator()->create_user(['email' => 'priliszlutouckykunupeldabelskeody@example.com']);
list($status, $notice, $url) = core_login_process_password_reset(null, 'PrIlIsZlUtOuCkYKuNupELdAbElSkEoDy@eXaMpLe.CoM');
$this->assertSame('emailresetconfirmsent', $status);
$emails = $sink->get_messages();
$this->assertCount(1, $emails);
$email = reset($emails);
$this->assertSame($u1->email, $email->to);
$sink->clear();
// There may exist two users with same emails.
$u2 = $this->getDataGenerator()->create_user(['email' => 'PRILISZLUTOUCKYKUNUPELDABELSKEODY@example.com']);
list($status, $notice, $url) = core_login_process_password_reset(null, 'PrIlIsZlUtOuCkYKuNupELdAbElSkEoDy@eXaMpLe.CoM');
$this->assertSame('emailresetconfirmsent', $status);
$emails = $sink->get_messages();
$this->assertCount(1, $emails);
$email = reset($emails);
$this->assertSame(core_text::strtolower($u2->email), core_text::strtolower($email->to));
$sink->clear();
// However, emails are accent sensitive - note this is the u1's email with a single character a -> á changed.
list($status, $notice, $url) = core_login_process_password_reset(null, 'priliszlutouckykunupeldábelskeody@example.com');
$this->assertSame('emailpasswordconfirmnotsent', $status);
$emails = $sink->get_messages();
$this->assertCount(0, $emails);
$sink->clear();
$u3 = $this->getDataGenerator()->create_user(['email' => 'PřílišŽluťoučkýKůňÚpělĎálebskéÓdy@example.com']);
list($status, $notice, $url) = core_login_process_password_reset(null, 'pŘÍLIŠžLuŤOuČkÝkŮŇúPĚLďÁLEBSKÉóDY@eXaMpLe.CoM');
$this->assertSame('emailresetconfirmsent', $status);
$emails = $sink->get_messages();
$this->assertCount(1, $emails);
$email = reset($emails);
$this->assertSame($u3->email, $email->to);
$sink->clear();
// Restore the original email address validator.
moodle_phpmailer::$validator = $defaultvalidator;
}
}