MDL-62922 core_user: Check auth mechanism before validating password

* External authentication mechanisms (e.g. via oauth2, etc.) don't store
  password in the user table, so we shouldn't be requiring password in
  such case when creating users via the core_user_create_users WS
  function.
This commit is contained in:
Jun Pataleta 2018-10-18 15:24:46 +08:00
parent cb7f6a6f99
commit eb1a477635
2 changed files with 72 additions and 29 deletions

View File

@ -161,7 +161,6 @@ class core_user_external extends external_api {
$transaction = $DB->start_delegated_transaction();
$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) {
@ -194,7 +193,8 @@ class core_user_external extends external_api {
}
// Make sure we have a password or have to create one.
if (empty($user['password']) && empty($user['createpassword'])) {
$authplugin = get_auth_plugin($user['auth']);
if ($authplugin->is_internal() && empty($user['password']) && empty($user['createpassword'])) {
throw new invalid_parameter_exception('Invalid password: you must provide a password, or set createpassword.');
}
@ -213,11 +213,15 @@ class core_user_external extends external_api {
$createpassword = !empty($user['createpassword']);
unset($user['createpassword']);
if ($createpassword) {
$user['password'] = '';
$updatepassword = false;
$updatepassword = false;
if ($authplugin->is_internal()) {
if ($createpassword) {
$user['password'] = '';
} else {
$updatepassword = true;
}
} else {
$updatepassword = true;
$user['password'] = AUTH_PASSWORD_NOT_CACHED;
}
// Create the user data now!

View File

@ -484,7 +484,7 @@ class core_user_externallib_testcase extends externallib_advanced_testcase {
* Test create_users
*/
public function test_create_users() {
global $USER, $CFG, $DB;
global $DB;
$this->resetAfterTest(true);
@ -517,46 +517,85 @@ class core_user_externallib_testcase extends externallib_advanced_testcase {
'interests' => 'badminton, basketball, cooking, '
);
// User with an authentication method done externally.
$user2 = array(
'username' => 'usernametest2',
'firstname' => 'First Name User Test 2',
'lastname' => 'Last Name User Test 2',
'email' => 'usertest2@example.com',
'auth' => 'oauth2'
);
$context = context_system::instance();
$roleid = $this->assignUserCapability('moodle/user:create', $context->id);
$this->assignUserCapability('moodle/user:editprofile', $context->id, $roleid);
// Call the external function.
$createdusers = core_user_external::create_users(array($user1));
$createdusers = core_user_external::create_users(array($user1, $user2));
// We need to execute the return values cleaning process to simulate the web service server.
$createdusers = external_api::clean_returnvalue(core_user_external::create_users_returns(), $createdusers);
// Check we retrieve the good total number of created users + no error on capability.
$this->assertEquals(1, count($createdusers));
$this->assertCount(2, $createdusers);
foreach($createdusers as $createduser) {
$dbuser = $DB->get_record('user', array('id' => $createduser['id']));
$this->assertEquals($dbuser->username, $user1['username']);
$this->assertEquals($dbuser->idnumber, $user1['idnumber']);
$this->assertEquals($dbuser->firstname, $user1['firstname']);
$this->assertEquals($dbuser->lastname, $user1['lastname']);
$this->assertEquals($dbuser->email, $user1['email']);
$this->assertEquals($dbuser->description, $user1['description']);
$this->assertEquals($dbuser->city, $user1['city']);
$this->assertEquals($dbuser->country, $user1['country']);
$this->assertEquals($dbuser->department, $user1['department']);
$this->assertEquals($dbuser->institution, $user1['institution']);
$this->assertEquals($dbuser->phone1, $user1['phone1']);
$this->assertEquals($dbuser->maildisplay, $user1['maildisplay']);
$this->assertEquals('atto', get_user_preferences('htmleditor', null, $dbuser));
$this->assertEquals(null, get_user_preferences('invalidpreference', null, $dbuser));
// Confirm user interests have been saved.
$interests = core_tag_tag::get_item_tags_array('core', 'user', $createduser['id'], core_tag_tag::BOTH_STANDARD_AND_NOT,
0, false);
// There should be 3 user interests.
$this->assertCount(3, $interests);
if ($createduser['username'] === $user1['username']) {
$usertotest = $user1;
$this->assertEquals('atto', get_user_preferences('htmleditor', null, $dbuser));
$this->assertEquals(null, get_user_preferences('invalidpreference', null, $dbuser));
// Confirm user interests have been saved.
$interests = core_tag_tag::get_item_tags_array('core', 'user', $createduser['id'],
core_tag_tag::BOTH_STANDARD_AND_NOT, 0, false);
// There should be 3 user interests.
$this->assertCount(3, $interests);
} else if ($createduser['username'] === $user2['username']) {
$usertotest = $user2;
}
foreach ($dbuser as $property => $value) {
if ($property === 'password') {
if ($usertotest === $user2) {
// External auth mechanisms don't store password in the user table.
$this->assertEquals(AUTH_PASSWORD_NOT_CACHED, $value);
} else {
// Skip hashed passwords.
continue;
}
}
// Confirm that the values match.
if (isset($usertotest[$property])) {
$this->assertEquals($usertotest[$property], $value);
}
}
}
// Call without required capability
$this->unassignUserCapability('moodle/user:create', $context->id, $roleid);
$this->expectException('required_capability_exception');
$createdusers = core_user_external::create_users(array($user1));
core_user_external::create_users(array($user1));
}
/**
* Test create_users with password and createpassword parameter not set.
*/
public function test_create_users_empty_password() {
$this->resetAfterTest();
$this->setAdminUser();
$user = [
'username' => 'usernametest1',
'firstname' => 'First Name User Test 1',
'lastname' => 'Last Name User Test 1',
'email' => 'usertest1@example.com',
];
// This should throw an exception because either password or createpassword param must be passed for auth_manual.
$this->expectException(invalid_parameter_exception::class);
core_user_external::create_users([$user]);
}
/**