From afa3e0ff32ba7e2acaae1a1ee8dfae755c468177 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Tue, 17 Jul 2018 19:07:22 +0200
Subject: [PATCH] MDL-59847 core: Stop supporting custom identity fields in
 user selectors

The user_selector classes supported custom list of extra identity
fields. But they should obey the configured user policy and respect the
privacy setting made by site administrators. So the list of user
identifiers should never be hard-coded, but the setting
$CFG->showuseridentity should be always respected.
---
 lib/upgrade.txt                  |  4 ++++
 user/selector/lib.php            | 33 +++++++-------------------------
 user/tests/userselector_test.php | 12 +++++++-----
 3 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/lib/upgrade.txt b/lib/upgrade.txt
index f2e8d0b5243..83ae4a1ae30 100644
--- a/lib/upgrade.txt
+++ b/lib/upgrade.txt
@@ -22,6 +22,10 @@ information provided here is intended especially for developers.
     - I set the field "<field_string>" to multiline
     - I follow "<link_string>"" in the open menu
 * Removed the lib/password_compat/lib/password.php file.
+* The user_selector classes do not support custom list of extra identity fields any more. They obey the configured user
+  policy and respect the privacy setting made by site administrators. The list of user identifiers should never be
+  hard-coded. Instead, the setting $CFG->showuseridentity should be always respected, which has always been the default
+  behaviour (MDL-59847).
 
 === 3.5 ===
 
diff --git a/user/selector/lib.php b/user/selector/lib.php
index 82782c419d7..34a553a008d 100644
--- a/user/selector/lib.php
+++ b/user/selector/lib.php
@@ -105,35 +105,16 @@ abstract class user_selector_base {
             $this->accesscontext = context_system::instance();
         }
 
-        // Populate the list of additional user identifiers to display.
+        // Check if some legacy code tries to override $CFG->showuseridentity.
         if (isset($options['extrafields'])) {
-            $this->extrafields = $options['extrafields'];
-        } else if (!empty($CFG->showuseridentity) &&
-                has_capability('moodle/site:viewuseridentity', $this->accesscontext)) {
-            $this->extrafields = explode(',', $CFG->showuseridentity);
-        } else {
-            $this->extrafields = array();
+            debugging('The user_selector classes do not support custom list of extra identity fields any more. '.
+                'Instead, the user identity fields defined by the site administrator will be used to respect '.
+                'the configured privacy setting.', DEBUG_DEVELOPER);
+            unset($options['extrafields']);
         }
 
-        // 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);
-            }
-        }
+        // Populate the list of additional user identifiers to display.
+        $this->extrafields = get_extra_user_fields($this->accesscontext);
 
         if (isset($options['exclude']) && is_array($options['exclude'])) {
             $this->exclude = $options['exclude'];
diff --git a/user/tests/userselector_test.php b/user/tests/userselector_test.php
index ae7ed6c78f1..fcb39b1ca86 100644
--- a/user/tests/userselector_test.php
+++ b/user/tests/userselector_test.php
@@ -241,6 +241,8 @@ class core_user_selector_testcase extends advanced_testcase {
         $implicitselector = new testable_user_selector('test');
         $explicitselector = new testable_user_selector('test', ['extrafields' => ['email', 'department']]);
 
+        $this->assertDebuggingCalled();
+
         foreach ($implicitselector->find_users('') as $found) {
             foreach ($found as $user) {
                 $this->assertObjectHasAttribute('idnumber', $user);
@@ -253,11 +255,11 @@ class core_user_selector_testcase extends advanced_testcase {
 
         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);
+                $this->assertObjectHasAttribute('idnumber', $user);
+                $this->assertObjectHasAttribute('country', $user);
+                $this->assertObjectHasAttribute('city', $user);
+                $this->assertObjectNotHasAttribute('email', $user);
+                $this->assertObjectNotHasAttribute('department', $user);
             }
         }
     }