From 01d058a0377a69fbbe5e59de7df7ba4726513db1 Mon Sep 17 00:00:00 2001
From: Jake Dallimore <jake@moodle.com>
Date: Thu, 17 Aug 2017 11:14:33 +0800
Subject: [PATCH] MDL-59825 user: user_can_view_profile() checks all of a
 $user's courses

This function used to check only those courses shared by both users
when it should have been checking all courses in which $user is
enrolled. Managers can view a user's course profile without necessarily
sharing the course (being enrolled in) with the $user.
---
 user/lib.php                | 14 ++++++++------
 user/tests/userlib_test.php | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/user/lib.php b/user/lib.php
index c395660c4ea..afd43c7376d 100644
--- a/user/lib.php
+++ b/user/lib.php
@@ -1153,19 +1153,21 @@ function user_can_view_profile($user, $course = null, $usercontext = null) {
     }
 
     if (isset($course)) {
-        $sharedcourses = array($course);
+        $userscourses = array($course);
     } else {
-        $sharedcourses = enrol_get_shared_courses($USER->id, $user->id, true);
+        // This returns context information, so we can preload below.
+        $userscourses = enrol_get_all_users_courses($user->id);
     }
 
-    if (empty($sharedcourses)) {
+    if (empty($userscourses)) {
         return false;
     }
 
-    foreach ($sharedcourses as $sharedcourse) {
-        $coursecontext = context_course::instance($sharedcourse->id);
+    foreach ($userscourses as $userscourse) {
+        context_helper::preload_from_record($userscourse);
+        $coursecontext = context_course::instance($userscourse->id);
         if (has_capability('moodle/user:viewdetails', $coursecontext)) {
-            if (!groups_user_groups_visible($sharedcourse, $user->id)) {
+            if (!groups_user_groups_visible($userscourse, $user->id)) {
                 // Not a member of the same group.
                 continue;
             }
diff --git a/user/tests/userlib_test.php b/user/tests/userlib_test.php
index 7c393c1a148..8947d746fc2 100644
--- a/user/tests/userlib_test.php
+++ b/user/tests/userlib_test.php
@@ -624,6 +624,40 @@ class core_userliblib_testcase extends advanced_testcase {
         foreach ($users as $user) {
             $this->assertTrue(user_can_view_profile($user));
         }
+
+        // Testing non-shared courses where capabilities are met, using system role overrides.
+        $CFG->forceloginforprofiles = $tempcfg;
+        $course4 = $this->getDataGenerator()->create_course();
+        $this->getDataGenerator()->enrol_user($user1->id, $course4->id);
+
+        // Assign a manager role at the system context.
+        $managerrole = $DB->get_record('role', array('shortname' => 'manager'));
+        $user9 = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->role_assign($managerrole->id, $user9->id);
+
+        // Make sure viewalldetails and viewdetails are overridden to 'prevent' (i.e. can be overridden at a lower context).
+        $systemcontext = context_system::instance();
+        assign_capability('moodle/user:viewdetails', CAP_PREVENT, $managerrole->id, $systemcontext, true);
+        assign_capability('moodle/user:viewalldetails', CAP_PREVENT, $managerrole->id, $systemcontext, true);
+        $systemcontext->mark_dirty();
+
+        // And override these to 'Allow' in a specific course.
+        $course4context = context_course::instance($course4->id);
+        assign_capability('moodle/user:viewalldetails', CAP_ALLOW, $managerrole->id, $course4context, true);
+        assign_capability('moodle/user:viewdetails', CAP_ALLOW, $managerrole->id, $course4context, true);
+        $course4context->mark_dirty();
+
+        // The manager now shouldn't have viewdetails in the system or user context.
+        $this->setUser($user9);
+        $user1context = context_user::instance($user1->id);
+        $this->assertFalse(has_capability('moodle/user:viewdetails', $systemcontext));
+        $this->assertFalse(has_capability('moodle/user:viewdetails', $user1context));
+
+        // Confirm that user_can_view_profile() returns true for $user1 when called without $course param. It should find $course1.
+        $this->assertTrue(user_can_view_profile($user1));
+
+        // Confirm this also works when restricting scope to just that course.
+        $this->assertTrue(user_can_view_profile($user1, $course4));
     }
 
     /**