MDL-68183 auth: Fix the performance of signup_validate_data search query

When searching for other users with the same 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 in a subselect, 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-17 14:10:47 +01:00
parent 1e19d154ba
commit d23fba1deb
2 changed files with 73 additions and 4 deletions
+15 -4
View File
@@ -1027,14 +1027,25 @@ function signup_validate_data($data, $files) {
$errors['email'] = get_string('invalidemail');
} else if (empty($CFG->allowaccountssameemail)) {
// Make a case-insensitive query for the given email address.
$select = $DB->sql_equal('email', ':email', false) . ' AND mnethostid = :mnethostid';
// Emails in Moodle as case-insensitive and accents-sensitive. Such a combination can lead to very slow queries
// on some DBs such as MySQL. So we first get the list of candidate users in a subselect via more effective
// accent-insensitive query that can make use of the index and only then we search within that limited subset.
$sql = "SELECT 'x'
FROM {user}
WHERE " . $DB->sql_equal('email', ':email1', false, true) . "
AND id IN (SELECT id
FROM {user}
WHERE " . $DB->sql_equal('email', ':email2', false, false) . "
AND mnethostid = :mnethostid)";
$params = array(
'email' => $data['email'],
'email1' => $data['email'],
'email2' => $data['email'],
'mnethostid' => $CFG->mnet_localhost_id,
);
// If there are other user(s) that already have the same email, show an error.
if ($DB->record_exists_select('user', $select, $params)) {
if ($DB->record_exists_sql($sql, $params)) {
$forgotpasswordurl = new moodle_url('/login/forgot_password.php');
$forgotpasswordlink = html_writer::link($forgotpasswordurl, get_string('emailexistshintlink'));
$errors['email'] = get_string('emailexists') . ' ' . get_string('emailexistssignuphint', 'moodle', $forgotpasswordlink);
+58
View File
@@ -345,4 +345,62 @@ class core_authlib_testcase extends advanced_testcase {
$this->assertInstanceOf('coding_exception', $e);
}
}
/**
* Test the {@link signup_validate_data()} duplicate email validation.
*/
public function test_signup_validate_data_same_email() {
global $CFG;
require_once($CFG->libdir . '/authlib.php');
require_once($CFG->dirroot . '/user/profile/lib.php');
$this->resetAfterTest();
$CFG->registerauth = 'email';
$CFG->passwordpolicy = false;
// In this test, we want to check accent-sensitive email search. However, accented 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);
};
// Check that two users cannot share the same email address if the site is configured so.
// Emails in Moodle are supposed to be case-insensitive (and accent-sensitive but accents are not yet supported).
$CFG->allowaccountssameemail = false;
$u1 = $this->getDataGenerator()->create_user([
'username' => 'abcdef',
'email' => '[email protected]',
]);
$formdata = [
'username' => 'newuser',
'firstname' => 'First',
'lastname' => 'Last',
'password' => 'weak',
'email' => '[email protected]',
];
$errors = signup_validate_data($formdata, []);
$this->assertContains('This email address is already registered.', $errors['email']);
// Emails are accent-sensitive though so if we change a -> á in the u1's email, it should pass.
// Please note that Moodle does not normally support such emails yet. We test the DB search sensitivity here.
$formdata['email'] = 'á[email protected]';
$errors = signup_validate_data($formdata, []);
$this->assertArrayNotHasKey('email', $errors);
// Check that users can share the same email if the site is configured so.
$CFG->allowaccountssameemail = true;
$formdata['email'] = '[email protected]';
$errors = signup_validate_data($formdata, []);
$this->assertArrayNotHasKey('email', $errors);
// Restore the original email address validator.
moodle_phpmailer::$validator = $defaultvalidator;
}
}