From f09a2fc6ec26c405c73fefe2624b9f7cc8427f2c Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Fri, 7 Jun 2024 13:20:38 +0100
Subject: [PATCH] MDL-82132 user: less restrictive returning first/lastname
 details.

The `viewfullnames` capability check was overeager in restricting
the inclusion of user first and lastname properties in the returned
structure. Especially given that the same data was almost always
present in the fullname property of the same structure.
---
 user/lib.php                | 27 ++++++++++++++++-----------
 user/tests/userlib_test.php | 24 +++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/user/lib.php b/user/lib.php
index 96ab8c28ea9..6f82000dd40 100644
--- a/user/lib.php
+++ b/user/lib.php
@@ -373,23 +373,28 @@ function user_get_user_details($user, $course = null, array $userfields = array(
         return null;
     }
 
-    $userdetails = array();
-    $userdetails['id'] = $user->id;
+    // User ID and fullname are always included.
+    $userdetails = [
+        'id' => $user->id,
+        'fullname' => fullname($user, $canviewfullnames),
+    ];
+
+    // User first/lastname included if capability check passes, or the same is present in fullname.
+    $dummyusername = core_user::get_dummy_fullname($context, ['override' => $canviewfullnames]);
+    if (in_array('firstname', $userfields) &&
+            ($canviewfullnames || core_text::strrpos($dummyusername, 'firstname') !== false)) {
+        $userdetails['firstname'] = $user->firstname;
+    }
+    if (in_array('lastname', $userfields) &&
+            ($canviewfullnames || core_text::strrpos($dummyusername, 'lastname') !== false)) {
+        $userdetails['lastname'] = $user->lastname;
+    }
 
     if (in_array('username', $userfields)) {
         if ($currentuser or has_capability('moodle/user:viewalldetails', $context)) {
             $userdetails['username'] = $user->username;
         }
     }
-    if ($isadmin or $canviewfullnames) {
-        if (in_array('firstname', $userfields)) {
-            $userdetails['firstname'] = $user->firstname;
-        }
-        if (in_array('lastname', $userfields)) {
-            $userdetails['lastname'] = $user->lastname;
-        }
-    }
-    $userdetails['fullname'] = fullname($user, $canviewfullnames);
 
     if (in_array('customfields', $userfields)) {
         $categories = profile_get_user_fields_with_data_by_category($user->id);
diff --git a/user/tests/userlib_test.php b/user/tests/userlib_test.php
index 337ecf7fadb..edf1de5c185 100644
--- a/user/tests/userlib_test.php
+++ b/user/tests/userlib_test.php
@@ -850,16 +850,21 @@ final class userlib_test extends \advanced_testcase {
         accesslib_clear_all_caches_for_unit_testing();
 
         // Get student details as a user with super system capabilities.
+        $this->setAdminUser();
         $result = user_get_user_details($student, $course1);
         $this->assertEquals($student->id, $result['id']);
         $this->assertEquals($studentfullname, $result['fullname']);
+        $this->assertEquals($student->firstname, $result['firstname']);
+        $this->assertEquals($student->lastname, $result['lastname']);
         $this->assertEquals($course1->id, $result['enrolledcourses'][0]['id']);
 
-        $this->setUser($teacher);
         // Get student details as a user who can only see this user in a course.
+        $this->setUser($teacher);
         $result = user_get_user_details($student, $course1);
         $this->assertEquals($student->id, $result['id']);
         $this->assertEquals($studentfullname, $result['fullname']);
+        $this->assertEquals($student->firstname, $result['firstname']);
+        $this->assertEquals($student->lastname, $result['lastname']);
         $this->assertEquals($course1->id, $result['enrolledcourses'][0]['id']);
 
         // Get student details with required fields.
@@ -867,6 +872,23 @@ final class userlib_test extends \advanced_testcase {
         $this->assertCount(2, $result);
         $this->assertEquals($student->id, $result['id']);
         $this->assertEquals($studentfullname, $result['fullname']);
+        $this->assertArrayNotHasKey('firstname', $result);
+        $this->assertArrayNotHasKey('lastname', $result);
+        $this->assertArrayNotHasKey('enrolledcourses', $result);
+
+        // Change fullname display format for a user with viewfullnames capability.
+        set_config('fullnamedisplay', 'firstname');
+        $result = user_get_user_details($student, $course1);
+        $this->assertEquals($studentfullname, $result['fullname']);
+        $this->assertEquals($student->firstname, $result['firstname']);
+        $this->assertEquals($student->lastname, $result['lastname']);
+
+        // Now check for a user without viewfullnames capability.
+        $this->setUser($student);
+        $result = user_get_user_details($teacher, $course1);
+        $this->assertEquals($teacher->firstname, $result['fullname']);
+        $this->assertEquals($teacher->firstname, $result['firstname']);
+        $this->assertArrayNotHasKey('lastname', $result);
 
         // Get exception for invalid required fields.
         $this->expectException('moodle_exception');