diff --git a/user/selector/lib.php b/user/selector/lib.php index ee0619a4a13..82782c419d7 100644 --- a/user/selector/lib.php +++ b/user/selector/lib.php @@ -105,6 +105,7 @@ abstract class user_selector_base { $this->accesscontext = context_system::instance(); } + // Populate the list of additional user identifiers to display. if (isset($options['extrafields'])) { $this->extrafields = $options['extrafields']; } else if (!empty($CFG->showuseridentity) && @@ -113,6 +114,27 @@ abstract class user_selector_base { } else { $this->extrafields = array(); } + + // Filter out hidden identifiers if the user can't see them. + $hiddenfields = array_filter(explode(',', $CFG->hiddenuserfields)); + $hiddenidentifiers = array_intersect($this->extrafields, $hiddenfields); + + if ($hiddenidentifiers) { + if ($this->accesscontext->get_course_context(false)) { + // We are somewhere inside a course. + $canviewhiddenuserfields = has_capability('moodle/course:viewhiddenuserfields', $this->accesscontext); + + } else { + // We are not inside a course. + $canviewhiddenuserfields = has_capability('moodle/user:viewhiddendetails', $this->accesscontext); + } + + if (!$canviewhiddenuserfields) { + // Remove hidden identifiers from the list. + $this->extrafields = array_diff($this->extrafields, $hiddenidentifiers); + } + } + if (isset($options['exclude']) && is_array($options['exclude'])) { $this->exclude = $options['exclude']; } diff --git a/user/tests/fixtures/testable_user_selector.php b/user/tests/fixtures/testable_user_selector.php new file mode 100644 index 00000000000..73bdf987ccf --- /dev/null +++ b/user/tests/fixtures/testable_user_selector.php @@ -0,0 +1,65 @@ +. + +/** + * Provides {@link testable_user_selector} class. + * + * @package core_user + * @subpackage fixtures + * @category test + * @copyright 2018 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Testable subclass of the user selector base class. + * + * @copyright 2018 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class testable_user_selector extends user_selector_base { + + /** + * Basic implementation of the users finder. + * + * @param string $search + * @return array of (string)optgroupname => array of users + */ + public function find_users($search) { + global $DB; + + list($wherecondition, $whereparams) = $this->search_sql($search, 'u'); + list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext); + $params = array_merge($whereparams, $sortparams); + $fields = $this->required_fields_sql('u'); + + $sql = "SELECT $fields + FROM {user} u + WHERE $wherecondition + ORDER BY $sort"; + + $found = $DB->get_records_sql($sql, $params); + + if (empty($found)) { + return []; + } + + return [get_string('potusers', 'core_role') => $found]; + } + +} diff --git a/user/tests/userselector_test.php b/user/tests/userselector_test.php new file mode 100644 index 00000000000..ae7ed6c78f1 --- /dev/null +++ b/user/tests/userselector_test.php @@ -0,0 +1,264 @@ +. + +/** + * Provides {@link core_user_selector_testcase} class. + * + * @package core_user + * @category test + * @copyright 2018 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot.'/user/selector/lib.php'); +require_once($CFG->dirroot.'/user/tests/fixtures/testable_user_selector.php'); + +/** + * Tests for the implementation of {@link user_selector_base} class. + * + * @copyright 2018 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_user_selector_testcase extends advanced_testcase { + + /** + * Setup the environment for the tests. + */ + protected function setup_hidden_siteidentity() { + global $CFG, $DB; + + $CFG->showuseridentity = 'idnumber,country,city'; + $CFG->hiddenuserfields = 'country,city'; + + $env = new stdClass(); + + $env->student = $this->getDataGenerator()->create_user(); + $env->teacher = $this->getDataGenerator()->create_user(); + $env->manager = $this->getDataGenerator()->create_user(); + + $env->course = $this->getDataGenerator()->create_course(); + $env->coursecontext = context_course::instance($env->course->id); + + $env->teacherrole = $DB->get_record('role', array('shortname' => 'teacher')); + $env->studentrole = $DB->get_record('role', array('shortname' => 'student')); + $env->managerrole = $DB->get_record('role', array('shortname' => 'manager')); + + role_assign($env->studentrole->id, $env->student->id, $env->coursecontext->id); + role_assign($env->teacherrole->id, $env->teacher->id, $env->coursecontext->id); + role_assign($env->managerrole->id, $env->manager->id, SYSCONTEXTID); + + return $env; + } + + /** + * No identity fields are not shown to student user (no permission to view identity fields). + */ + public function test_hidden_siteidentity_fields_no_access() { + $this->resetAfterTest(); + $env = $this->setup_hidden_siteidentity(); + $this->setUser($env->student); + + $selector = new testable_user_selector('test'); + + foreach ($selector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectNotHasAttribute('idnumber', $user); + $this->assertObjectNotHasAttribute('country', $user); + $this->assertObjectNotHasAttribute('city', $user); + } + } + } + + /** + * Teacher can see students' identity fields only within the course. + */ + public function test_hidden_siteidentity_fields_course_only_access() { + $this->resetAfterTest(); + $env = $this->setup_hidden_siteidentity(); + $this->setUser($env->teacher); + + $systemselector = new testable_user_selector('test'); + $courseselector = new testable_user_selector('test', ['accesscontext' => $env->coursecontext]); + + foreach ($systemselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectNotHasAttribute('idnumber', $user); + $this->assertObjectNotHasAttribute('country', $user); + $this->assertObjectNotHasAttribute('city', $user); + } + } + + foreach ($courseselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectHasAttribute('idnumber', $user); + $this->assertObjectHasAttribute('country', $user); + $this->assertObjectHasAttribute('city', $user); + } + } + } + + /** + * Teacher can be prevented from seeing students' identity fields even within the course. + */ + public function test_hidden_siteidentity_fields_course_prevented_access() { + $this->resetAfterTest(); + $env = $this->setup_hidden_siteidentity(); + $this->setUser($env->teacher); + + assign_capability('moodle/course:viewhiddenuserfields', CAP_PREVENT, $env->teacherrole->id, $env->coursecontext->id); + + $courseselector = new testable_user_selector('test', ['accesscontext' => $env->coursecontext]); + + foreach ($courseselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectHasAttribute('idnumber', $user); + $this->assertObjectNotHasAttribute('country', $user); + $this->assertObjectNotHasAttribute('city', $user); + } + } + } + + /** + * Manager can see students' identity fields anywhere. + */ + public function test_hidden_siteidentity_fields_anywhere_access() { + $this->resetAfterTest(); + $env = $this->setup_hidden_siteidentity(); + $this->setUser($env->manager); + + $systemselector = new testable_user_selector('test'); + $courseselector = new testable_user_selector('test', ['accesscontext' => $env->coursecontext]); + + foreach ($systemselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectHasAttribute('idnumber', $user); + $this->assertObjectHasAttribute('country', $user); + $this->assertObjectHasAttribute('city', $user); + } + } + + foreach ($courseselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectHasAttribute('idnumber', $user); + $this->assertObjectHasAttribute('country', $user); + $this->assertObjectHasAttribute('city', $user); + } + } + } + + /** + * Manager can be prevented from seeing hidden fields outside the course. + */ + public function test_hidden_siteidentity_fields_schismatic_access() { + $this->resetAfterTest(); + $env = $this->setup_hidden_siteidentity(); + $this->setUser($env->manager); + + // Revoke the capability to see hidden user fields outside the course. + // Note that inside the course, the manager can still see the hidden identifiers as this is currently + // controlled by a separate capability for legacy reasons. This is counter-intuitive behaviour and is + // likely to be fixed in MDL-51630. + assign_capability('moodle/user:viewhiddendetails', CAP_PREVENT, $env->managerrole->id, SYSCONTEXTID, true); + + $systemselector = new testable_user_selector('test'); + $courseselector = new testable_user_selector('test', ['accesscontext' => $env->coursecontext]); + + foreach ($systemselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectHasAttribute('idnumber', $user); + $this->assertObjectNotHasAttribute('country', $user); + $this->assertObjectNotHasAttribute('city', $user); + } + } + + foreach ($courseselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectHasAttribute('idnumber', $user); + $this->assertObjectHasAttribute('country', $user); + $this->assertObjectHasAttribute('city', $user); + } + } + } + + /** + * Two capabilities must be currently set to prevent manager from seeing hidden fields. + */ + public function test_hidden_siteidentity_fields_hard_to_prevent_access() { + $this->resetAfterTest(); + $env = $this->setup_hidden_siteidentity(); + $this->setUser($env->manager); + + assign_capability('moodle/user:viewhiddendetails', CAP_PREVENT, $env->managerrole->id, SYSCONTEXTID, true); + assign_capability('moodle/course:viewhiddenuserfields', CAP_PREVENT, $env->managerrole->id, SYSCONTEXTID, true); + + $systemselector = new testable_user_selector('test'); + $courseselector = new testable_user_selector('test', ['accesscontext' => $env->coursecontext]); + + foreach ($systemselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectHasAttribute('idnumber', $user); + $this->assertObjectNotHasAttribute('country', $user); + $this->assertObjectNotHasAttribute('city', $user); + } + } + + foreach ($courseselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectHasAttribute('idnumber', $user); + $this->assertObjectNotHasAttribute('country', $user); + $this->assertObjectNotHasAttribute('city', $user); + } + } + } + + /** + * For legacy reasons, user selectors supported ability to override $CFG->showuseridentity. + * + * However, this was found as violating the principle of respecting site privacy settings. So the feature has been + * dropped in Moodle 3.6. + */ + public function test_hidden_siteidentity_fields_explicit_extrafields() { + $this->resetAfterTest(); + $env = $this->setup_hidden_siteidentity(); + $this->setUser($env->manager); + + $implicitselector = new testable_user_selector('test'); + $explicitselector = new testable_user_selector('test', ['extrafields' => ['email', 'department']]); + + foreach ($implicitselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectHasAttribute('idnumber', $user); + $this->assertObjectHasAttribute('country', $user); + $this->assertObjectHasAttribute('city', $user); + $this->assertObjectNotHasAttribute('email', $user); + $this->assertObjectNotHasAttribute('department', $user); + } + } + + foreach ($explicitselector->find_users('') as $found) { + foreach ($found as $user) { + $this->assertObjectNotHasAttribute('idnumber', $user); + $this->assertObjectNotHasAttribute('country', $user); + $this->assertObjectNotHasAttribute('city', $user); + $this->assertObjectHasAttribute('email', $user); + $this->assertObjectHasAttribute('department', $user); + } + } + } +}