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.
This commit is contained in:
Jake Dallimore
2017-08-18 11:37:10 +08:00
parent 1c6106e8a8
commit 01d058a037
2 changed files with 42 additions and 6 deletions
+8 -6
View File
@@ -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;
}
+34
View File
@@ -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));
}
/**