MDL-58762 report: Check group permissions in course user reports
authorJuan Leyva <juanleyvadelgado@gmail.com>
Tue, 2 May 2017 08:04:55 +0000 (10:04 +0200)
committerDavid Monllao <davidm@moodle.com>
Thu, 7 Sep 2017 08:53:32 +0000 (10:53 +0200)
Teachers were able to see any student report even with forced separated
groups and capability moodle/course:accessallgroups off.

report/completion/lib.php
report/completion/user.php
report/log/lang/en/report_log.php
report/log/lib.php
report/log/user.php
report/outline/lang/en/report_outline.php
report/outline/lib.php
report/outline/user.php
report/stats/lib.php
report/stats/user.php

index a98d2ce..d60a5cc 100644 (file)
@@ -80,25 +80,30 @@ function report_completion_can_access_user_report($user, $course) {
     }
 
     if ($course->id != SITEID and !$course->enablecompletion) {
-        return;
+        return false;
     }
 
     $coursecontext = context_course::instance($course->id);
     $personalcontext = context_user::instance($user->id);
 
-    if (has_capability('report/completion:view', $coursecontext)) {
-        return true;
-    }
-
-    if (has_capability('moodle/user:viewuseractivitiesreport', $personalcontext)) {
-        if ($course->showreports and (is_viewing($coursecontext, $user) or is_enrolled($coursecontext, $user))) {
+    if ($user->id == $USER->id) {
+        if ($course->showreports and (is_viewing($coursecontext, $USER) or is_enrolled($coursecontext, $USER))) {
             return true;
         }
-
-    } else if ($user->id == $USER->id) {
-        if ($course->showreports and (is_viewing($coursecontext, $USER) or is_enrolled($coursecontext, $USER))) {
+    } else if (has_capability('moodle/user:viewuseractivitiesreport', $personalcontext)) {
+        if ($course->showreports and (is_viewing($coursecontext, $user) or is_enrolled($coursecontext, $user))) {
             return true;
         }
+
+    }
+
+    // Check if $USER shares group with $user (in case separated groups are enabled and 'moodle/site:accessallgroups' is disabled).
+    if (!groups_user_groups_visible($course, $user->id)) {
+        return false;
+    }
+
+    if (has_capability('report/completion:view', $coursecontext)) {
+        return true;
     }
 
     return false;
index b73c9be..47af842 100644 (file)
@@ -45,7 +45,7 @@ if ($USER->id != $user->id and has_capability('moodle/user:viewuseractivitiesrep
     require_login($course);
 }
 
-if (!report_completion_can_access_user_report($user, $course, true)) {
+if (!report_completion_can_access_user_report($user, $course)) {
     // this should never happen
     print_error('nocapability', 'report_completion');
 }
index f37d48f..82d6f23 100644 (file)
@@ -37,6 +37,7 @@ $string['log:view'] = 'View course logs';
 $string['log:viewtoday'] = 'View today\'s logs';
 $string['page'] = 'Page {$a}';
 $string['logsformat'] = 'Logs format';
+$string['nocapability'] = 'Can not access user log report';
 $string['nologreaderenabled'] = 'No log reader enabled';
 $string['origin'] = 'Source';
 $string['other'] = 'Other';
index 6a53182..5c4d38e 100644 (file)
@@ -89,6 +89,21 @@ function report_log_can_access_user_report($user, $course) {
     $coursecontext = context_course::instance($course->id);
     $personalcontext = context_user::instance($user->id);
 
+    if ($user->id == $USER->id) {
+        if ($course->showreports and (is_viewing($coursecontext, $USER) or is_enrolled($coursecontext, $USER))) {
+            return array(true, true);
+        }
+    } else if (has_capability('moodle/user:viewuseractivitiesreport', $personalcontext)) {
+        if ($course->showreports and (is_viewing($coursecontext, $user) or is_enrolled($coursecontext, $user))) {
+            return array(true, true);
+        }
+    }
+
+    // Check if $USER shares group with $user (in case separated groups are enabled and 'moodle/site:accessallgroups' is disabled).
+    if (!groups_user_groups_visible($course, $user->id)) {
+        return array(false, false);
+    }
+
     $today = false;
     $all = false;
 
@@ -99,21 +114,6 @@ function report_log_can_access_user_report($user, $course) {
         $all = true;
     }
 
-    if ($today and $all) {
-        return array(true, true);
-    }
-
-    if (has_capability('moodle/user:viewuseractivitiesreport', $personalcontext)) {
-        if ($course->showreports and (is_viewing($coursecontext, $user) or is_enrolled($coursecontext, $user))) {
-            return array(true, true);
-        }
-
-    } else if ($user->id == $USER->id) {
-        if ($course->showreports and (is_viewing($coursecontext, $USER) or is_enrolled($coursecontext, $USER))) {
-            return array(true, true);
-        }
-    }
-
     return array($all, $today);
 }
 
index 3fdd4b3..f8426e7 100644 (file)
@@ -58,6 +58,10 @@ if ($USER->id != $user->id and has_capability('moodle/user:viewuseractivitiesrep
 
 list($all, $today) = report_log_can_access_user_report($user, $course);
 
+if (!$today && !$all) {
+    print_error('nocapability', 'report_log');
+}
+
 if ($mode === 'today') {
     if (!$today) {
         require_capability('report/log:viewtoday', $coursecontext);
index 36e44fd..245838f 100644 (file)
@@ -26,6 +26,7 @@
 $string['eventactivityreportviewed'] = 'Activity report viewed';
 $string['eventoutlinereportviewed'] = 'Outline report viewed';
 $string['neverseen'] = 'Never seen';
+$string['nocapability'] = 'Can not access user outline report';
 $string['nologreaderenabled'] = 'No log reader enabled';
 $string['numviews'] = '{$a->numviews} by {$a->distinctusers} users';
 $string['outline:view'] = 'View activity report';
index 876e349..996e10a 100644 (file)
@@ -70,19 +70,24 @@ function report_outline_can_access_user_report($user, $course) {
     $coursecontext = context_course::instance($course->id);
     $personalcontext = context_user::instance($user->id);
 
-    if (has_capability('report/outline:view', $coursecontext)) {
-        return true;
-    }
-
-    if (has_capability('moodle/user:viewuseractivitiesreport', $personalcontext)) {
-        if ($course->showreports and (is_viewing($coursecontext, $user) or is_enrolled($coursecontext, $user))) {
+    if ($user->id == $USER->id) {
+        if ($course->showreports and (is_viewing($coursecontext, $USER) or is_enrolled($coursecontext, $USER))) {
             return true;
         }
-
-    } else if ($user->id == $USER->id) {
-        if ($course->showreports and (is_viewing($coursecontext, $USER) or is_enrolled($coursecontext, $USER))) {
+    } else if (has_capability('moodle/user:viewuseractivitiesreport', $personalcontext)) {
+        if ($course->showreports and (is_viewing($coursecontext, $user) or is_enrolled($coursecontext, $user))) {
             return true;
         }
+
+    }
+
+    // Check if $USER shares group with $user (in case separated groups are enabled and 'moodle/site:accessallgroups' is disabled).
+    if (!groups_user_groups_visible($course, $user->id)) {
+        return false;
+    }
+
+    if (has_capability('report/outline:view', $coursecontext)) {
+        return true;
     }
 
     return false;
index 162453e..a1e6515 100644 (file)
@@ -55,8 +55,8 @@ if ($USER->id != $user->id and has_capability('moodle/user:viewuseractivitiesrep
 }
 $PAGE->set_url('/report/outline/user.php', array('id'=>$userid, 'course'=>$courseid, 'mode'=>$mode));
 
-if (!report_outline_can_access_user_report($user, $course, true)) {
-    require_capability('report/outline:view', $coursecontext);
+if (!report_outline_can_access_user_report($user, $course)) {
+    print_error('nocapability', 'report_outline');
 }
 
 $stractivityreport = get_string('activityreport');
index f0a069b..aec2a9d 100644 (file)
@@ -78,21 +78,25 @@ function report_stats_can_access_user_report($user, $course) {
     $coursecontext = context_course::instance($course->id);
     $personalcontext = context_user::instance($user->id);
 
-    if (has_capability('report/stats:view', $coursecontext)) {
-        return true;
-    }
-
-    if (has_capability('moodle/user:viewuseractivitiesreport', $personalcontext)) {
-        if ($course->showreports and (is_viewing($coursecontext, $user) or is_enrolled($coursecontext, $user))) {
+    if ($user->id == $USER->id) {
+        if ($course->showreports and (is_viewing($coursecontext, $USER) or is_enrolled($coursecontext, $USER))) {
             return true;
         }
-
-    } else if ($user->id == $USER->id) {
-        if ($course->showreports and (is_viewing($coursecontext, $USER) or is_enrolled($coursecontext, $USER))) {
+    } else if (has_capability('moodle/user:viewuseractivitiesreport', $personalcontext)) {
+        if ($course->showreports and (is_viewing($coursecontext, $user) or is_enrolled($coursecontext, $user))) {
             return true;
         }
     }
 
+    // Check if $USER shares group with $user (in case separated groups are enabled and 'moodle/site:accessallgroups' is disabled).
+    if (!groups_user_groups_visible($course, $user->id)) {
+        return false;
+    }
+
+    if (has_capability('report/stats:view', $coursecontext)) {
+        return true;
+    }
+
     return false;
 }
 
index 2d5a5c8..5477142 100644 (file)
@@ -51,7 +51,7 @@ if ($USER->id != $user->id and has_capability('moodle/user:viewuseractivitiesrep
     require_login($course);
 }
 
-if (!report_stats_can_access_user_report($user, $course, true)) {
+if (!report_stats_can_access_user_report($user, $course)) {
     // this should never happen
     print_error('nocapability', 'report_stats');
 }