L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-50705 auth_db: apply standard cleaning to all fields

Browse Source 
Also unit tests were added to cover the new clean_data() method.
This commit is contained in:
Simey Lameze
2016-03-09 18:23:36 +01:00
committed by Eloy Lafuente (stronk7)
parent 3b214760fb
commit f4fcb1c4f7
2 changed files with 101 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
auth/db/auth.php
+28
View File
@@ -300,6 +300,7 @@ class auth_plugin_db extends auth_plugin_base {
$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);
}
@@ -381,6 +382,7 @@ class auth_plugin_db extends auth_plugin_base {
foreach($add_users as $user) {
$username = $user;
if ($this->config->removeuser == AUTH_REMOVEUSER_SUSPEND) {
if ($old_user = $DB->get_record('user', array('username'=>$username, 'deleted'=>0, 'suspended'=>1, 'mnethostid'=>$CFG->mnet_localhost_id, 'auth'=>$this->authtype))) {
$DB->set_field('user', 'suspended', 0, array('id'=>$old_user->id));
$trace->output(get_string('auth_dbreviveduser', 'auth_db', array('name'=>$username, 'id'=>$old_user->id)), 1);
@@ -412,6 +414,7 @@ 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;
}
$user = $this->clean_data($user);
try {
$id = $DB->insert_record ('user', $user); // it is truly a new user
@@ -553,6 +556,7 @@ class auth_plugin_db extends auth_plugin_base {
}
}
}
if ($updated) {
$DB->set_field('user', 'timemodified', time(), array('id'=>$userid));
@@ -870,6 +874,30 @@ class auth_plugin_db extends auth_plugin_base {
error_reporting($CFG->debug);
ob_end_flush();
}
/**
* Clean the user data that comes from an external database.
*
* @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;
}
}
auth/db/tests/db_test.php
+73
View File
@@ -374,4 +374,77 @@ class auth_db_testcase extends advanced_testcase {
$this->cleanup_auth_database();
}
/**
* Testing the clean_data() method.
*/
public function test_clean_data() {
global $DB;
$this->resetAfterTest(false);
$this->preventResetByRollback();
$this->init_auth_database();
$auth = get_auth_plugin('db');
$auth->db_init();
// Create users on external table.
$extdbuser1 = (object)array('name'=>'u1', 'pass'=>'heslo', 'email'=>'[email protected]');
$extdbuser1->id = $DB->insert_record('auth_db_users', $extdbuser1);
// User with malicious data on the name.
$extdbuser2 = (object)array('name'=>'user<script>alert(1);</script>xss', 'pass'=>'heslo', 'email'=>'[email protected]');
$extdbuser2->id = $DB->insert_record('auth_db_users', $extdbuser2);
$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);
// 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);
// Run sync again to update the user data.
$auth->sync_users($trace, true);
// 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 = ' [email protected] ';
$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, '[email protected]');
$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();
}
}