Merge branch 'MDL-29318-master' of git://github.com/junpataleta/moodle
authorAdrian Greeve <abgreeve@gmail.com>
Thu, 18 Apr 2019 05:48:33 +0000 (13:48 +0800)
committerAdrian Greeve <abgreeve@gmail.com>
Thu, 18 Apr 2019 05:48:33 +0000 (13:48 +0800)
lib/moodlelib.php
lib/tests/moodlelib_test.php
lib/upgrade.txt
login/lib.php
login/tests/lib_test.php

index ad2c738..321560d 100644 (file)
@@ -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.
index fc742d5..db9ce6d 100644 (file)
@@ -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);
index 5207a6e..158187b 100644 (file)
@@ -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.
     - <component>_pre_course_category_delete($category)
     - <component>_pre_course_delete($course)
     - <component>_pre_course_module_delete($cm)
     - <component>_pre_block_delete($instance)
     - <component>_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.
index eb29ae1..4264333 100644 (file)
@@ -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');
             }
         }
 
index fccb928..160d9de 100644 (file)
@@ -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']
             ],