diff --git a/lib/moodlelib.php b/lib/moodlelib.php index ad2c738b7ac..321560d3856 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -4791,9 +4791,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) { @@ -4804,7 +4806,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); @@ -4831,8 +4833,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 fc742d5f948..db9ce6d793b 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -4313,6 +4313,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 + ], ]; } @@ -4323,10 +4344,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 = [ @@ -4335,6 +4361,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) { @@ -4345,7 +4376,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 5207a6ecf24..158187b52f6 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -39,6 +39,9 @@ attribute on forms to avoid collisions in forms loaded in AJAX requests. - Finally, the self-conversations for all remaining users without them will be created and starred. Besides, from now, a self-conversation will be created and starred by default to all the new users (even when $CFG->messaging is disabled). +* 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.6 === @@ -654,13 +657,13 @@ the groupid field. $OUTPUT->download_dataformat_selector() instead. when building Xpath, or pass the unescaped value when using the named selector. * Add new file_is_executable(), to consistently check for executables even in Windows (PHP bug #41062). -* Introduced new callbacks for plugin developers. +* Introduced new hooks for plugin developers. - _pre_course_category_delete($category) - _pre_course_delete($course) - _pre_course_module_delete($cm) - _pre_block_delete($instance) - _pre_user_delete($user) - These callbacks allow developers to use the item in question before it is deleted by core. For example, if your plugin is + These hooks allow developers to use the item in question before it is deleted by core. For example, if your plugin is a module (plugins located in the mod folder) called 'xxx' and you wish to interact with the user object before it is deleted then the function to create would be mod_xxx_pre_user_delete($user) in mod/xxx/lib.php. * pear::Net::GeoIP has been removed. diff --git a/login/lib.php b/login/lib.php index eb29ae1fcac..42643335021 100644 --- a/login/lib.php +++ b/login/lib.php @@ -355,18 +355,21 @@ 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)) { send_confirmation_email($user); $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'] ],