diff --git a/lang/en/error.php b/lang/en/error.php index 1c7de748706..f15f401e74d 100644 --- a/lang/en/error.php +++ b/lang/en/error.php @@ -364,6 +364,7 @@ $string['invaliduserid'] = 'Invalid user id'; $string['invaliduserfield'] = 'Invalid user field: {$a}'; $string['invaliduserdata'] = 'Invalid user data: {$a}'; $string['invalidusername'] = 'The given username contains invalid characters'; +$string['invalidusernameblank'] = 'The username cannot be blank'; $string['invalidxmlfile'] = '"{$a}" is not a valid XML file'; $string['iplookupfailed'] = 'Cannot find geo information about this IP address {$a}'; $string['iplookupprivate'] = 'Cannot display lookup of private IP address'; @@ -565,6 +566,7 @@ $string['userautherror'] = 'Unknown auth plugin'; $string['userauthunsupported'] = 'Auth plugin not supported here'; $string['useremailduplicate'] = 'Duplicate address'; $string['usermustbemnet'] = 'Users in the MNET access control list must be remote MNET users'; +$string['usernamelowercase'] = 'The username must be in lower case'; $string['usernotaddederror'] = 'User not added - error'; $string['usernotaddedregistered'] = 'User not added - already registered'; $string['usernotavailable'] = 'The details of this user are not available to you'; diff --git a/user/externallib.php b/user/externallib.php index 83da6dcd000..4ba0f6b4278 100644 --- a/user/externallib.php +++ b/user/externallib.php @@ -157,6 +157,13 @@ class core_user_external extends external_api { $userids = array(); $createpassword = false; foreach ($params['users'] as $user) { + // Make sure that the username, firstname and lastname are not blank. + foreach (array('username', 'firstname', 'lastname') as $fieldname) { + if (trim($user[$fieldname]) === '') { + throw new invalid_parameter_exception('The field '.$fieldname.' cannot be blank'); + } + } + // Make sure that the username doesn't already exist. if ($DB->record_exists('user', array('username' => $user['username'], 'mnethostid' => $CFG->mnet_localhost_id))) { throw new invalid_parameter_exception('Username already exists: '.$user['username']); diff --git a/user/lib.php b/user/lib.php index 985c402daf6..f7008b5fe60 100644 --- a/user/lib.php +++ b/user/lib.php @@ -48,12 +48,16 @@ function user_create_user($user, $updatepassword = true, $triggerevent = true) { } // Check username. + if (trim($user->username) === '') { + throw new moodle_exception('invalidusernameblank'); + } + if ($user->username !== core_text::strtolower($user->username)) { throw new moodle_exception('usernamelowercase'); - } else { - if ($user->username !== core_user::clean_field($user->username, 'username')) { - throw new moodle_exception('invalidusername'); - } + } + + if ($user->username !== core_user::clean_field($user->username, 'username')) { + throw new moodle_exception('invalidusername'); } // Save the password in a temp value for later. diff --git a/user/tests/externallib_test.php b/user/tests/externallib_test.php index 3afffd85ba1..00261fa57ce 100644 --- a/user/tests/externallib_test.php +++ b/user/tests/externallib_test.php @@ -544,6 +544,84 @@ class core_user_externallib_testcase extends externallib_advanced_testcase { $createdusers = core_user_external::create_users(array($user1)); } + /** + * Test create_users with invalid parameters + * + * @dataProvider data_create_users_invalid_parameter + * @param array $data User data to attempt to register. + * @param string $expectmessage Expected exception message. + */ + public function test_create_users_invalid_parameter(array $data, $expectmessage) { + global $USER, $CFG, $DB; + + $this->resetAfterTest(true); + $this->assignUserCapability('moodle/user:create', SYSCONTEXTID); + + $this->expectException('invalid_parameter_exception'); + $this->expectExceptionMessage($expectmessage); + + core_user_external::create_users(array($data)); + } + + /** + * Data provider for {@link self::test_create_users_invalid_parameter()}. + * + * @return array + */ + public function data_create_users_invalid_parameter() { + return [ + 'blank_username' => [ + 'data' => [ + 'username' => '', + 'firstname' => 'Foo', + 'lastname' => 'Bar', + 'email' => 'foobar@example.com', + 'createpassword' => 1, + ], + 'expectmessage' => 'The field username cannot be blank', + ], + 'blank_firtname' => [ + 'data' => [ + 'username' => 'foobar', + 'firstname' => "\t \n", + 'lastname' => 'Bar', + 'email' => 'foobar@example.com', + 'createpassword' => 1, + ], + 'expectmessage' => 'The field firstname cannot be blank', + ], + 'blank_lastname' => [ + 'data' => [ + 'username' => 'foobar', + 'firstname' => '0', + 'lastname' => ' ', + 'email' => 'foobar@example.com', + 'createpassword' => 1, + ], + 'expectmessage' => 'The field lastname cannot be blank', + ], + 'invalid_email' => [ + 'data' => [ + 'username' => 'foobar', + 'firstname' => 'Foo', + 'lastname' => 'Bar', + 'email' => '@foobar', + 'createpassword' => 1, + ], + 'expectmessage' => 'Email address is invalid', + ], + 'missing_password' => [ + 'data' => [ + 'username' => 'foobar', + 'firstname' => 'Foo', + 'lastname' => 'Bar', + 'email' => 'foobar@example.com', + ], + 'expectmessage' => 'Invalid password: you must provide a password, or set createpassword', + ], + ]; + } + /** * Test delete_users */ diff --git a/user/tests/userlib_test.php b/user/tests/userlib_test.php index bc5347c5ef3..b7f6d6c4567 100644 --- a/user/tests/userlib_test.php +++ b/user/tests/userlib_test.php @@ -241,6 +241,55 @@ class core_userliblib_testcase extends advanced_testcase { $this->assertDebuggingNotCalled(); } + /** + * Test that {@link user_create_user()} throws exception when invalid username is provided. + * + * @dataProvider data_create_user_invalid_username + * @param string $username Invalid username + * @param string $expectmessage Expected exception message + */ + public function test_create_user_invalid_username($username, $expectmessage) { + global $CFG; + + $this->resetAfterTest(); + $CFG->extendedusernamechars = false; + + $user = [ + 'username' => $username, + ]; + + $this->expectException('moodle_exception'); + $this->expectExceptionMessage($expectmessage); + + user_create_user($user); + } + + /** + * Data provider for {@link self::test_create_user_invalid_username()}. + * + * @return array + */ + public function data_create_user_invalid_username() { + return [ + 'empty_string' => [ + '', + 'The username cannot be blank', + ], + 'only_whitespace' => [ + "\t\t \t\n ", + 'The username cannot be blank', + ], + 'lower_case' => [ + 'Mudrd8mz', + 'The username must be in lower case', + ], + 'extended_chars' => [ + 'dmudrák', + 'The given username contains invalid characters', + ], + ]; + } + /** * Test function user_count_login_failures(). */