From f6be98b9423f18451d7e84865c7f54064fafbdec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Fri, 17 Aug 2018 12:57:35 +0200 Subject: [PATCH] MDL-63174 user: user_create_user to throw exception on empty username The core API function user_create_user() did not check the case when the given username was empty. Also adding a missing string 'usernamelowercase' for the existing lower case check and unit tests. --- lang/en/error.php | 2 ++ user/lib.php | 12 ++++++--- user/tests/userlib_test.php | 49 +++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) 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/lib.php b/user/lib.php index b6af1f3d66d..7a22e74befa 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/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(). */