Skip to content

Commit

Permalink
MDL-52781 core_user: add validation to user insertion and updating me…
Browse files Browse the repository at this point in the history 
…thod

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.
  • Loading branch information
@lameze
lameze committed Apr 21, 2016
1 parent 4ce0931 commit 7a06720
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 18 deletions.
41 changes: 24 additions & 17 deletions user/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
Expand All @@ -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;
}
Expand All @@ -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);

Expand Down Expand Up @@ -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');
}
}
Expand All @@ -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) {
Expand Down
52 changes: 51 additions & 1 deletion user/tests/userlib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,29 @@ public function test_user_update_user() {
$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();
}

/**
Expand All @@ -115,7 +138,7 @@ public function test_create_users() {
'email' => 'usertest1@example.com',
'description' => 'This is a description for user 1',
'city' => 'Perth',
'country' => 'au'
'country' => 'AU'
);

// Create user and capture event.
Expand Down Expand Up @@ -152,6 +175,33 @@ public function test_create_users() {
$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();
}

/**
Expand Down

0 comments on commit 7a06720

Please sign in to comment.