Skip to content

Commit

Permalink
MDL-59847 core: Fix display of hidden identity fields in user lists
Browse files Browse the repository at this point in the history 
Places that display list of users (such as course participants page)
with additional identifier fields now respect the user's permission to
view other users' hidden profile fields.
  • Loading branch information
@mudrd8mz
mudrd8mz committed Jul 27, 2018
1 parent a1d8976 commit ec92a07
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 19 deletions.
39 changes: 26 additions & 13 deletions lib/moodlelib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3601,25 +3601,38 @@ function get_extra_user_fields($context, $already = array()) {
return array();
}

// Split showuseridentity on comma.
if (empty($CFG->showuseridentity)) {
// Explode gives wrong result with empty string.
$extra = array();
} else {
$extra = explode(',', $CFG->showuseridentity);
}
$renumber = false;
// Split showuseridentity on comma (filter needed in case the showuseridentity is empty).
$extra = array_filter(explode(',', $CFG->showuseridentity));

foreach ($extra as $key => $field) {
if (in_array($field, $already)) {
unset($extra[$key]);
$renumber = true;
}
}
if ($renumber) {
// For consistency, if entries are removed from array, renumber it
// so they are numbered as you would expect.
$extra = array_merge($extra);

// If the identity fields are also among hidden fields, make sure the user can see them.
$hiddenfields = array_filter(explode(',', $CFG->hiddenuserfields));
$hiddenidentifiers = array_intersect($extra, $hiddenfields);

if ($hiddenidentifiers) {
if ($context->get_course_context(false)) {
// We are somewhere inside a course.
$canviewhiddenuserfields = has_capability('moodle/course:viewhiddenuserfields', $context);

} else {
// We are not inside a course.
$canviewhiddenuserfields = has_capability('moodle/user:viewhiddendetails', $context);
}

if (!$canviewhiddenuserfields) {
// Remove hidden identifiers from the list.
$extra = array_diff($extra, $hiddenidentifiers);
}
}

// Re-index the entries.
$extra = array_values($extra);

return $extra;
}

Expand Down
125 changes: 119 additions & 6 deletions lib/tests/moodlelib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1465,16 +1465,14 @@ public function test_unset_user_preference_for_current_user() {
$this->assertNull(get_user_preferences('test_pref'));
}

public function test_get_extra_user_fields() {
/**
* Test essential features implementation of {@link get_extra_user_fields()} as the admin user with all capabilities.
*/
public function test_get_extra_user_fields_essentials() {
global $CFG, $USER, $DB;
$this->resetAfterTest();

$this->setAdminUser();

// It would be really nice if there were a way to 'mock' has_capability
// checks (either to return true or false) but as there is not, this
// test doesn't test the capability check. Presumably, anyone running
// unit tests will have the capability.
$context = context_system::instance();

// No fields.
Expand Down Expand Up @@ -1502,6 +1500,121 @@ public function test_get_extra_user_fields() {
$this->assertEquals(array('zombie'), get_extra_user_fields($context, array('frog')));
}

/**
* Prepare environment for couple of tests related to permission checks in {@link get_extra_user_fields()}.
*
* @return stdClass
*/
protected function environment_for_get_extra_user_fields_tests() {
global $CFG, $DB;

$CFG->showuseridentity = 'idnumber,country,city';
$CFG->hiddenuserfields = 'country,city';

$env = new stdClass();

$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'));

$env->student = $this->getDataGenerator()->create_user();
$env->teacher = $this->getDataGenerator()->create_user();
$env->manager = $this->getDataGenerator()->create_user();

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 shown to student user (no permission to view identity fields).
*/
public function test_get_extra_user_fields_no_access() {

$this->resetAfterTest();
$env = $this->environment_for_get_extra_user_fields_tests();
$this->setUser($env->student);

$this->assertEquals(array(), get_extra_user_fields($env->coursecontext));
$this->assertEquals(array(), get_extra_user_fields(context_system::instance()));
}

/**
* Teacher can see students' identity fields only within the course.
*/
public function test_get_extra_user_fields_course_only_access() {

$this->resetAfterTest();
$env = $this->environment_for_get_extra_user_fields_tests();
$this->setUser($env->teacher);

$this->assertEquals(array('idnumber', 'country', 'city'), get_extra_user_fields($env->coursecontext));
$this->assertEquals(array(), get_extra_user_fields(context_system::instance()));
}

/**
* Teacher can be prevented from seeing students' identity fields even within the course.
*/
public function test_get_extra_user_fields_course_prevented_access() {

$this->resetAfterTest();
$env = $this->environment_for_get_extra_user_fields_tests();
$this->setUser($env->teacher);

assign_capability('moodle/course:viewhiddenuserfields', CAP_PREVENT, $env->teacherrole->id, $env->coursecontext->id);
$this->assertEquals(array('idnumber'), get_extra_user_fields($env->coursecontext));
}

/**
* Manager can see students' identity fields anywhere.
*/
public function test_get_extra_user_fields_anywhere_access() {

$this->resetAfterTest();
$env = $this->environment_for_get_extra_user_fields_tests();
$this->setUser($env->manager);

$this->assertEquals(array('idnumber', 'country', 'city'), get_extra_user_fields($env->coursecontext));
$this->assertEquals(array('idnumber', 'country', 'city'), get_extra_user_fields(context_system::instance()));
}

/**
* Manager can be prevented from seeing hidden fields outside the course.
*/
public function test_get_extra_user_fields_schismatic_access() {

$this->resetAfterTest();
$env = $this->environment_for_get_extra_user_fields_tests();
$this->setUser($env->manager);

assign_capability('moodle/user:viewhiddendetails', CAP_PREVENT, $env->managerrole->id, SYSCONTEXTID, true);
$this->assertEquals(array('idnumber'), get_extra_user_fields(context_system::instance()));
// 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->assertEquals(array('idnumber', 'country', 'city'), get_extra_user_fields($env->coursecontext));
}

/**
* Two capabilities must be currently set to prevent manager from seeing hidden fields.
*/
public function test_get_extra_user_fields_hard_to_prevent_access() {

$this->resetAfterTest();
$env = $this->environment_for_get_extra_user_fields_tests();
$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);

$this->assertEquals(array('idnumber'), get_extra_user_fields(context_system::instance()));
$this->assertEquals(array('idnumber'), get_extra_user_fields($env->coursecontext));
}

public function test_get_extra_user_fields_sql() {
global $CFG, $USER, $DB;
$this->resetAfterTest();
Expand Down

0 comments on commit ec92a07

Please sign in to comment.