diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 8f412ae2c49..93947e30b86 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -4694,9 +4694,11 @@ function update_internal_user_password($user, $password, $fasthash = false) { * @param string $field The user field to be checked for a given value. * @param string $value The value to match for $field. * @param int $mnethostid + * @param bool $throwexception If true, it will throw an exception when there's no record found or when there are multiple records + * found. Otherwise, it will just return false. * @return mixed False, or A {@link $USER} object. */ -function get_complete_user_data($field, $value, $mnethostid = null) { +function get_complete_user_data($field, $value, $mnethostid = null, $throwexception = false) { global $CFG, $DB; if (!$field || !$value) { @@ -4707,7 +4709,7 @@ function get_complete_user_data($field, $value, $mnethostid = null) { $field = core_text::strtolower($field); // List of case insensitive fields. - $caseinsensitivefields = ['username']; + $caseinsensitivefields = ['username', 'email']; // Build the WHERE clause for an SQL query. $params = array('fieldval' => $value); @@ -4734,8 +4736,18 @@ function get_complete_user_data($field, $value, $mnethostid = null) { } // Get all the basic user data. - if (! $user = $DB->get_record_select('user', $constraints, $params)) { - return false; + try { + // Make sure that there's only a single record that matches our query. + // For example, when fetching by email, multiple records might match the query as there's no guarantee that email addresses + // are unique. Therefore we can't reliably tell whether the user profile data that we're fetching is the correct one. + $user = $DB->get_record_select('user', $constraints, $params, '*', MUST_EXIST); + } catch (dml_exception $exception) { + if ($throwexception) { + throw $exception; + } else { + // Return false when no records or multiple records were found. + return false; + } } // Get various settings and preferences. diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index ac907c45238..14782b45011 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -4295,6 +4295,27 @@ class core_moodlelib_testcase extends advanced_testcase { 'Fetch data using an invalid username' => [ 'username', 's2', false ], + 'Fetch by email' => [ + 'email', 's1@example.com', true + ], + 'Fetch data using a non-existent email' => [ + 'email', 's2@example.com', false + ], + 'Fetch data using a non-existent email, throw exception' => [ + 'email', 's2@example.com', false, dml_missing_record_exception::class + ], + 'Multiple accounts with the same email' => [ + 'email', 's1@example.com', false, 1 + ], + 'Multiple accounts with the same email, throw exception' => [ + 'email', 's1@example.com', false, 1, dml_multiple_records_exception::class + ], + 'Fetch data using a valid user ID' => [ + 'id', true, true + ], + 'Fetch data using a non-existent user ID' => [ + 'id', false, false + ], ]; } @@ -4305,10 +4326,15 @@ class core_moodlelib_testcase extends advanced_testcase { * @param string $field The field to use for the query. * @param string|boolean $value The field value. When fetching by ID, set true to fetch valid user ID, false otherwise. * @param boolean $success Whether we expect for the fetch to succeed or return false. + * @param int $allowaccountssameemail Value for $CFG->allowaccountssameemail. + * @param string $expectedexception The exception to be expected. */ - public function test_get_complete_user_data($field, $value, $success) { + public function test_get_complete_user_data($field, $value, $success, $allowaccountssameemail = 0, $expectedexception = '') { $this->resetAfterTest(); + // Set config settings we need for our environment. + set_config('allowaccountssameemail', $allowaccountssameemail); + // Generate the user data. $generator = $this->getDataGenerator(); $userdata = [ @@ -4317,6 +4343,11 @@ class core_moodlelib_testcase extends advanced_testcase { ]; $user = $generator->create_user($userdata); + if ($allowaccountssameemail) { + // Create another user with the same email address. + $generator->create_user(['email' => 's1@example.com']); + } + // Since the data provider can't know what user ID to use, do a special handling for ID field tests. if ($field === 'id') { if ($value) { @@ -4327,7 +4358,15 @@ class core_moodlelib_testcase extends advanced_testcase { $value = $user->id + 1; } } - $fetcheduser = get_complete_user_data($field, $value); + + // When an exception is expected. + $throwexception = false; + if ($expectedexception) { + $this->expectException($expectedexception); + $throwexception = true; + } + + $fetcheduser = get_complete_user_data($field, $value, null, $throwexception); if ($success) { $this->assertEquals($user->id, $fetcheduser->id); $this->assertEquals($user->username, $fetcheduser->username); diff --git a/lib/upgrade.txt b/lib/upgrade.txt index c1b8e1a23b8..55db34b0098 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -6,6 +6,9 @@ information provided here is intended especially for developers. * Behat timeout constants behat_base::TIMEOUT, EXTENDED_TIMEOUT, and REDUCED_TIMEOUT will be deprecated in 3.7. Please instead use the functions behat_base::get_timeout(), get_extended_timeout(), and get_reduced_timeout(). These allow for timeouts to be increased by a setting in config.php. +* New optional parameter $throwexception for \get_complete_user_data(). If true, an exception will be thrown when there's no + matching record found or when there are multiple records found for the given field value. If false, it will simply return false. + Defaults to false when not set. === 3.5.5 === diff --git a/login/lib.php b/login/lib.php index 8927e03f6be..2ca4e47eca1 100644 --- a/login/lib.php +++ b/login/lib.php @@ -355,17 +355,20 @@ function core_login_validate_forgot_password_data($data) { if (!validate_email($data['email'])) { $errors['email'] = get_string('invalidemail'); - } else if ($DB->count_records('user', array('email' => $data['email'])) > 1) { - $errors['email'] = get_string('forgottenduplicate'); - } else { - if ($user = get_complete_user_data('email', $data['email'])) { + try { + $user = get_complete_user_data('email', $data['email'], null, true); if (empty($user->confirmed)) { $errors['email'] = get_string('confirmednot'); } - } - if (!$user and empty($CFG->protectusernames)) { - $errors['email'] = get_string('emailnotfound'); + } catch (dml_missing_record_exception $missingexception) { + // User not found. Show error when $CFG->protectusernames is turned off. + if (empty($CFG->protectusernames)) { + $errors['email'] = get_string('emailnotfound'); + } + } catch (dml_multiple_records_exception $multipleexception) { + // Multiple records found. Ask the user to enter a username instead. + $errors['email'] = get_string('forgottenduplicate'); } } diff --git a/login/tests/lib_test.php b/login/tests/lib_test.php index fccb928e751..160d9def977 100644 --- a/login/tests/lib_test.php +++ b/login/tests/lib_test.php @@ -271,6 +271,11 @@ class core_login_lib_testcase extends advanced_testcase { ['email' => get_string('forgottenduplicate')], ['allowaccountssameemail' => 1] ], + 'Multiple accounts with the same email but with different case' => [ + ['email' => 'S1@EXAMPLE.COM'], + ['email' => get_string('forgottenduplicate')], + ['allowaccountssameemail' => 1] + ], 'Non-existent email, username protection on' => [ ['email' => 's2@example.com'] ],