MDL-52781 auth_db: deprecate clean_data method.

    The old clean_data method has been deprecated as the user_create_user and user_updated user
    will be responsible by validating the user data.

diff --git a/auth/db/auth.php b/auth/db/auth.php
index 8c2428d..ebebaa1 100644 (file)
--- a/auth/db/auth.php
+++ b/auth/db/auth.php
@@ -328,7 +328,6 @@ class auth_plugin_db extends auth_plugin_base {
                         $updateuser = new stdClass();
                         $updateuser->id   = $user->id;
                         $updateuser->suspended = 1;
                         $updateuser = new stdClass();
                         $updateuser->id   = $user->id;
                         $updateuser->suspended = 1;

-                        $updateuser = $this->clean_data($updateuser);

                         user_update_user($updateuser, false);
                         $trace->output(get_string('auth_dbsuspenduser', 'auth_db', array('name'=>$user->username, 'id'=>$user->id)), 1);
                     }
                         user_update_user($updateuser, false);
                         $trace->output(get_string('auth_dbsuspenduser', 'auth_db', array('name'=>$user->username, 'id'=>$user->id)), 1);
                     }


@@ -415,7 +414,6 @@ class auth_plugin_db extends auth_plugin_base {
                         $updateuser = new stdClass();
                         $updateuser->id = $olduser->id;
                         $updateuser->suspended = 0;
                         $updateuser = new stdClass();
                         $updateuser->id = $olduser->id;
                         $updateuser->suspended = 0;

-                        $updateuser = $this->clean_data($updateuser);

                         user_update_user($updateuser);
                         $trace->output(get_string('auth_dbreviveduser', 'auth_db', array('name' => $username,
                             'id' => $olduser->id)), 1);
                         user_update_user($updateuser);
                         $trace->output(get_string('auth_dbreviveduser', 'auth_db', array('name' => $username,
                             'id' => $olduser->id)), 1);


@@ -438,7 +436,6 @@ class auth_plugin_db extends auth_plugin_base {
                     $trace->output(get_string('auth_dbinsertuserduplicate', 'auth_db', array('username'=>$user->username, 'auth'=>$collision->auth)), 1);
                     continue;
                 }
                     $trace->output(get_string('auth_dbinsertuserduplicate', 'auth_db', array('username'=>$user->username, 'auth'=>$collision->auth)), 1);
                     continue;
                 }

-                $user = $this->clean_data($user);

                 try {
                     $id = user_create_user($user, false); // It is truly a new user.
                     $trace->output(get_string('auth_dbinsertuser', 'auth_db', array('name'=>$user->username, 'id'=>$id)), 1);
                 try {
                     $id = user_create_user($user, false); // It is truly a new user.
                     $trace->output(get_string('auth_dbinsertuser', 'auth_db', array('name'=>$user->username, 'id'=>$id)), 1);


@@ -580,7 +577,6 @@ class auth_plugin_db extends auth_plugin_base {
         }
         if ($needsupdate) {
             require_once($CFG->dirroot . '/user/lib.php');
         }
         if ($needsupdate) {
             require_once($CFG->dirroot . '/user/lib.php');

-            $updateuser = $this->clean_data($updateuser);

             user_update_user($updateuser);
         }
         return $DB->get_record('user', array('id'=>$userid, 'deleted'=>0));
             user_update_user($updateuser);
         }
         return $DB->get_record('user', array('id'=>$userid, 'deleted'=>0));


@@ -913,26 +909,14 @@ class auth_plugin_db extends auth_plugin_base {
 
     /**
      * Clean the user data that comes from an external database.
 
     /**
      * Clean the user data that comes from an external database.

-     *
+     * @deprecated since 3.1, please use core_user::clean_data() instead.

      * @param array $user the user data to be validated against properties definition.
      * @return stdClass $user the cleaned user data.
      */
     public function clean_data($user) {
      * @param array $user the user data to be validated against properties definition.
      * @return stdClass $user the cleaned user data.
      */
     public function clean_data($user) {

-        if (empty($user)) {
-            return $user;
-        }
-
-        foreach ($user as $field => $value) {
-            // Get the property parameter type and do the cleaning.
-            try {
-                $property = core_user::get_property_definition($field);
-                $user->$field = clean_param($value, $property['type']);
-            } catch (coding_exception $e) {
-                debugging("The property '$field' could not be cleaned.", DEBUG_DEVELOPER);
-            }
-        }
-
-        return $user;
+        debugging('The method clean_data() has been deprecated, please use core_user::clean_data() instead.',
+            DEBUG_DEVELOPER);
+        return core_user::clean_data($user);

     }
 }
 
     }
 }
 

diff --git a/auth/db/tests/db_test.php b/auth/db/tests/db_test.php
index e0d68d3..f1471cf 100644 (file)
--- a/auth/db/tests/db_test.php
+++ b/auth/db/tests/db_test.php
@@ -121,7 +121,9 @@ class auth_db_testcase extends advanced_testcase {
         set_config('table', $CFG->prefix.'auth_db_users', 'auth/db');
         set_config('fielduser', 'name', 'auth/db');
         set_config('fieldpass', 'pass', 'auth/db');
         set_config('table', $CFG->prefix.'auth_db_users', 'auth/db');
         set_config('fielduser', 'name', 'auth/db');
         set_config('fieldpass', 'pass', 'auth/db');

-
+        set_config('field_map_lastname', 'lastname', 'auth/db');
+        set_config('field_updatelocal_lastname', 'oncreate', 'auth/db');
+        set_config('field_lock_lastname', 'unlocked', 'auth/db');

         // Setu up field mappings.
 
         set_config('field_map_email', 'email', 'auth/db');
         // Setu up field mappings.
 
         set_config('field_map_email', 'email', 'auth/db');


@@ -149,7 +151,7 @@ class auth_db_testcase extends advanced_testcase {
     public function test_plugin() {
         global $DB, $CFG;
 
     public function test_plugin() {
         global $DB, $CFG;
 

-        $this->resetAfterTest(false);
+        $this->resetAfterTest(true);

 
         // NOTE: It is strongly discouraged to create new tables in advanced_testcase classes,
         //       but there is no other simple way to test ext database enrol sync, so let's
 
         // NOTE: It is strongly discouraged to create new tables in advanced_testcase classes,
         //       but there is no other simple way to test ext database enrol sync, so let's


@@ -416,60 +418,31 @@ class auth_db_testcase extends advanced_testcase {
         $extdbuser1 = (object)array('name'=>'u1', 'pass'=>'heslo', 'email'=>'u1@example.com');
         $extdbuser1->id = $DB->insert_record('auth_db_users', $extdbuser1);
 
         $extdbuser1 = (object)array('name'=>'u1', 'pass'=>'heslo', 'email'=>'u1@example.com');
         $extdbuser1->id = $DB->insert_record('auth_db_users', $extdbuser1);
 

-        // User with malicious data on the name.
+        // User with malicious data on the name (won't be imported).

         $extdbuser2 = (object)array('name'=>'user<script>alert(1);</script>xss', 'pass'=>'heslo', 'email'=>'xssuser@example.com');
         $extdbuser2->id = $DB->insert_record('auth_db_users', $extdbuser2);
 
         $extdbuser2 = (object)array('name'=>'user<script>alert(1);</script>xss', 'pass'=>'heslo', 'email'=>'xssuser@example.com');
         $extdbuser2->id = $DB->insert_record('auth_db_users', $extdbuser2);
 

+        $extdbuser3 = (object)array('name'=>'u3', 'pass'=>'heslo', 'email'=>'u3@example.com',
+                'lastname' => 'user<script>alert(1);</script>xss');
+        $extdbuser3->id = $DB->insert_record('auth_db_users', $extdbuser3);

         $trace = new null_progress_trace();
 
         // Let's test user sync make sure still works as expected..
         $auth->sync_users($trace, true);
         $trace = new null_progress_trace();
 
         // Let's test user sync make sure still works as expected..
         $auth->sync_users($trace, true);

-
-        // Get the user on moodle user table.
-        $user2 = $DB->get_record('user', array('email'=> $extdbuser2->email, 'auth'=>'db'));
-
-        // The malicious code should be sanitized.
-        $this->assertEquals($user2->username, 'userscriptalert1scriptxss');
-        $this->assertNotEquals($user2->username, $extdbuser2->name);
-
+        $this->assertDebuggingCalled("The property 'lastname' has invalid data and has been cleaned.");

         // User with correct data, should be equal to external db.
         $user1 = $DB->get_record('user', array('email'=> $extdbuser1->email, 'auth'=>'db'));
         $this->assertEquals($extdbuser1->name, $user1->username);
         $this->assertEquals($extdbuser1->email, $user1->email);
 
         // User with correct data, should be equal to external db.
         $user1 = $DB->get_record('user', array('email'=> $extdbuser1->email, 'auth'=>'db'));
         $this->assertEquals($extdbuser1->name, $user1->username);
         $this->assertEquals($extdbuser1->email, $user1->email);
 

-        // Now, let's update the name.
-        $extdbuser2->name = 'user no xss anymore';
-        $DB->update_record('auth_db_users', $extdbuser2);
+        // Get the user on moodle user table.
+        $user2 = $DB->get_record('user', array('email'=> $extdbuser2->email, 'auth'=>'db'));
+        $user3 = $DB->get_record('user', array('email'=> $extdbuser3->email, 'auth'=>'db'));

 
 

-        // Run sync again to update the user data.
-        $auth->sync_users($trace, true);
+        $this->assertEmpty($user2);
+        $this->assertEquals($extdbuser3->name, $user3->username);
+        $this->assertEquals('useralert(1);xss', $user3->lastname);

 
 

-        // The user information should be updated.
-        $user2 = $DB->get_record('user', array('username' => 'usernoxssanymore', 'auth' => 'db'));
-        // The spaces should be removed, as it's the username.
-        $this->assertEquals($user2->username, 'usernoxssanymore');
-
-        // Now let's test just the clean_data() method isolated.
-        // Testing PARAM_USERNAME, PARAM_NOTAGS, PARAM_RAW_TRIMMED and others.
-        $user3 = new stdClass();
-        $user3->firstname = 'John <script>alert(1)</script> Doe';
-        $user3->username = 'john%#&~%*_doe';
-        $user3->email = ' john@testing.com ';
-        $user3->deleted = 'no';
-        $user3->description = '<b>A description <script>alert(123)</script>about myself.</b>';
-        $user3cleaned = $auth->clean_data($user3);
-
-        // Expected results.
-        $this->assertEquals($user3cleaned->firstname, 'John alert(1) Doe');
-        $this->assertEquals($user3cleaned->email, 'john@testing.com');
-        $this->assertEquals($user3cleaned->deleted, 0);
-        $this->assertEquals($user3->description, '<b>A description about myself.</b>');
-        $this->assertEquals($user3->username, 'john_doe');
-
-        // Try to clean an invalid property (fullname).
-        $user3->fullname = 'John Doe';
-        $auth->clean_data($user3);
-        $this->assertDebuggingCalled("The property 'fullname' could not be cleaned.");

         $this->cleanup_auth_database();
     }
 }
         $this->cleanup_auth_database();
     }
 }