MDL-52781 core_user: add validation to user insertion and updating method
authorSimey Lameze <simey@moodle.com>
Mon, 11 Apr 2016 04:11:40 +0000 (12:11 +0800)
committerSimey Lameze <simey@moodle.com>
Thu, 21 Apr 2016 07:24:35 +0000 (15:24 +0800)
The new validation were added to user_create_user and user_update_user,
displaying debug message if some invalid data has been found.
Also the unit tests of those methods has been changed to match the methods behaviour.

user/lib.php
user/tests/userlib_test.php

index a063ea3..91008f6 100644 (file)
@@ -45,7 +45,7 @@ function user_create_user($user, $updatepassword = true, $triggerevent = true) {
     if ($user->username !== core_text::strtolower($user->username)) {
         throw new moodle_exception('usernamelowercase');
     } else {
-        if ($user->username !== clean_param($user->username, PARAM_USERNAME)) {
+        if ($user->username !== core_user::clean_field($user->username, 'username')) {
             throw new moodle_exception('invalidusername');
         }
     }
@@ -62,17 +62,11 @@ function user_create_user($user, $updatepassword = true, $triggerevent = true) {
         unset($user->password);
     }
 
-    // Make sure calendartype, if set, is valid.
-    if (!empty($user->calendartype)) {
-        $availablecalendartypes = \core_calendar\type_factory::get_list_of_calendar_types();
-        if (empty($availablecalendartypes[$user->calendartype])) {
-            $user->calendartype = $CFG->calendartype;
-        }
-    } else {
+    // Apply default values for user preferences that are stored in users table.
+    if (!isset($user->calendartype)) {
         $user->calendartype = $CFG->calendartype;
     }
 
-    // Apply default values for user preferences that are stored in users table.
     if (!isset($user->maildisplay)) {
         $user->maildisplay = $CFG->defaultpreference_maildisplay;
     }
@@ -95,6 +89,15 @@ function user_create_user($user, $updatepassword = true, $triggerevent = true) {
     $user->timecreated = time();
     $user->timemodified = $user->timecreated;
 
+    // Validate user data object.
+    $uservalidation = core_user::validate($user);
+    if ($uservalidation !== true) {
+        foreach ($uservalidation as $field => $message) {
+            debugging("The property '$field' has invalid data and has been cleaned.", DEBUG_DEVELOPER);
+            $user->$field = core_user::clean_field($user->$field, $field);
+        }
+    }
+
     // Insert the user into the database.
     $newuserid = $DB->insert_record('user', $user);
 
@@ -139,7 +142,7 @@ function user_update_user($user, $updatepassword = true, $triggerevent = true) {
         if ($user->username !== core_text::strtolower($user->username)) {
             throw new moodle_exception('usernamelowercase');
         } else {
-            if ($user->username !== clean_param($user->username, PARAM_USERNAME)) {
+            if ($user->username !== core_user::clean_field($user->username, 'username')) {
                 throw new moodle_exception('invalidusername');
             }
         }
@@ -158,18 +161,22 @@ function user_update_user($user, $updatepassword = true, $triggerevent = true) {
     }
 
     // Make sure calendartype, if set, is valid.
-    if (!empty($user->calendartype)) {
-        $availablecalendartypes = \core_calendar\type_factory::get_list_of_calendar_types();
-        // If it doesn't exist, then unset this value, we do not want to update the user's value.
-        if (empty($availablecalendartypes[$user->calendartype])) {
-            unset($user->calendartype);
-        }
-    } else {
+    if (empty($user->calendartype)) {
         // Unset this variable, must be an empty string, which we do not want to update the calendartype to.
         unset($user->calendartype);
     }
 
     $user->timemodified = time();
+
+    // Validate user data object.
+    $uservalidation = core_user::validate($user);
+    if ($uservalidation !== true) {
+        foreach ($uservalidation as $field => $message) {
+            debugging("The property '$field' has invalid data and has been cleaned.", DEBUG_DEVELOPER);
+            $user->$field = core_user::clean_field($user->$field, $field);
+        }
+    }
+
     $DB->update_record('user', $user);
 
     if ($updatepassword) {
index faa852e..edf2273 100644 (file)
@@ -92,6 +92,29 @@ class core_userliblib_testcase extends advanced_testcase {
         $this->assertCount(1, $events);
         $event = array_pop($events);
         $this->assertInstanceOf('\core\event\user_password_updated', $event);
+
+        // Test user data validation.
+        $user->username = 'johndoe123';
+        $user->auth = 'shibolth';
+        $user->country = 'WW';
+        $user->lang = 'xy';
+        $user->theme = 'somewrongthemename';
+        $user->timezone = 'Paris';
+        $user->url = 'wwww.somewrong@#$url.com.aus';
+        $debugmessages = $this->getDebuggingMessages();
+        user_update_user($user, true, false);
+        $this->assertDebuggingCalledCount(6, $debugmessages);
+
+        // Now, with valid user data.
+        $user->username = 'johndoe321';
+        $user->auth = 'shibboleth';
+        $user->country = 'AU';
+        $user->lang = 'en';
+        $user->theme = 'clean';
+        $user->timezone = 'Australia/Perth';
+        $user->url = 'www.moodle.org';
+        user_update_user($user, true, false);
+        $this->assertDebuggingNotCalled();
     }
 
     /**
@@ -115,7 +138,7 @@ class core_userliblib_testcase extends advanced_testcase {
             'email' => 'usertest1@example.com',
             'description' => 'This is a description for user 1',
             'city' => 'Perth',
-            'country' => 'au'
+            'country' => 'AU'
             );
 
         // Create user and capture event.
@@ -152,6 +175,33 @@ class core_userliblib_testcase extends advanced_testcase {
         $events = $sink->get_events();
         $sink->close();
         $this->assertCount(0, $events);
+
+        // Test user data validation, first some invalid data.
+        $user['username'] = 'johndoe123';
+        $user['auth'] = 'shibolth';
+        $user['country'] = 'WW';
+        $user['lang'] = 'xy';
+        $user['theme'] = 'somewrongthemename';
+        $user['timezone'] = 'Paris';
+        $user['url'] = 'wwww.somewrong@#$url.com.aus';
+        $debugmessages = $this->getDebuggingMessages();
+        $user['id'] = user_create_user($user, true, false);
+        $this->assertDebuggingCalledCount(6, $debugmessages);
+        $dbuser = $DB->get_record('user', array('id' => $user['id']));
+        $this->assertEquals($dbuser->country, 0);
+        $this->assertEquals($dbuser->lang, 'en');
+        $this->assertEquals($dbuser->timezone, 'Australia/Perth');
+
+        // Now, with valid user data.
+        $user['username'] = 'johndoe321';
+        $user['auth'] = 'shibboleth';
+        $user['country'] = 'AU';
+        $user['lang'] = 'en';
+        $user['theme'] = 'clean';
+        $user['timezone'] = 'Australia/Perth';
+        $user['url'] = 'www.moodle.org';
+        user_create_user($user, true, false);
+        $this->assertDebuggingNotCalled();
     }
 
     /**