MDL-59847 core: Fix display of hidden identity fields in user selectors
authorDavid Mudrák <david@moodle.com>
Fri, 13 Jul 2018 10:58:12 +0000 (12:58 +0200)
committerJake Dallimore <jake@moodle.com>
Thu, 2 Aug 2018 02:22:05 +0000 (10:22 +0800)
User selectors now respect the user's permission to view other users'
hidden profile fields.

user/selector/lib.php
user/tests/fixtures/testable_user_selector.php [new file with mode: 0644]
user/tests/userselector_test.php [new file with mode: 0644]

index ee0619a..82782c4 100644 (file)
@@ -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 (file)
index 0000000..73bdf98
--- /dev/null
@@ -0,0 +1,65 @@
+<?php
+// This file is part of Moodle - https://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Provides {@link testable_user_selector} class.
+ *
+ * @package     core_user
+ * @subpackage  fixtures
+ * @category    test
+ * @copyright   2018 David Mudrák <david@moodle.com>
+ * @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 <david@moodle.com>
+ * @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 (file)
index 0000000..ae7ed6c
--- /dev/null
@@ -0,0 +1,264 @@
+<?php
+// This file is part of Moodle - https://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Provides {@link core_user_selector_testcase} class.
+ *
+ * @package     core_user
+ * @category    test
+ * @copyright   2018 David Mudrák <david@moodle.com>
+ * @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 <david@moodle.com>
+ * @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);
+            }
+        }
+    }
+}