Bugfixes in access control and tweaked user_tops to properly handle permissions on one category while visiblity is only granted on subcategories of it.

classes/courseservice.php

@@ -31,6 +31,7 @@ use \local_treestudyplan\local\helpers\webservicehelper;
 use \local_treestudyplan\completionscanner;
 use \local_treestudyplan\gradingscanner;
 use \core_course_category;
+use moodle_exception;
 
 /**
  * Webservice related to courses
@@ -61,6 +62,31 @@ class courseservice extends \external_api {
         }
         $tops = [];
 
+        if (has_capability($capability,\context_system::instance(),$userid)) {
+            if ($capability == 'moodle/category:viewcourselist') {
+                // We are now just looking for the visible main level categories.
+                // Add all categories of depth = 1;
+                $rs = $DB->get_recordset("course_categories",["depth" => 1],'sortorder');
+                foreach( $rs as $rcat) {
+                    // Get the category, and double check if the category is visible to the current user.
+                    // Just in case it is a hidden category and the user does not have the viewhidden permission.
+                    $cat = \core_course_category::get($rcat->id, \IGNORE_MISSING, false, $userid);
+                    if ($cat !== null) {
+                        // Register the category.
+                        array_push($tops,$cat);
+                    }
+                }
+                $rs->close();
+
+            } else {
+                // We were primarily searching for a 
+                // Return the top visible categories for this user.
+                // Recurses only once.
+                return self::user_tops($userid);
+            }
+        } else {
+            // We need to search for the permissions on an individial context level.
+            // This part finds all top categories with a certain permission that are also visible for the user.
             /*
             SELECT UNIQUE ctx.* FROM mdl_context AS ctx
             INNER JOIN mdl_role_assignments AS ra ON ra.contextid = ctx.id
@@ -70,19 +96,19 @@ class courseservice extends \external_api {
             AND ra.userid = 58 AND rc.capability = 'moodle/category:viewcourselist'
             ORDER BY  ctx.depth ASC, cat.sortorder ASC;
             */
-        $capability = 'moodle/category:viewcourselist';
+            //$capability = 'moodle/category:viewcourselist';
 
             $sql = "SELECT UNIQUE ctx.* FROM {context} AS ctx
                     INNER JOIN {role_assignments} AS ra ON ra.contextid = ctx.id
                     INNER JOIN {role_capabilities} AS rc ON ra.roleid = rc.roleid
                     LEFT JOIN {course_categories} AS cat ON ctx.instanceid = cat.id
-                WHERE ( ctx.contextlevel = :ctxl_coursecat OR ctx.contextlevel = :ctxl_system )
+                    WHERE ( ctx.contextlevel = :ctxl_coursecat )
                     AND ra.userid = :userid AND rc.capability = :capability
                     ORDER BY ctx.depth ASC, cat.sortorder ASC";
 
             // Use recordset to handle the eventuality of a really big and complex moodle setup.
             $recordset = $DB->get_recordset_sql($sql,  ["userid" => $userid, "capability" => $capability,
-                                                "ctxl_coursecat" => \CONTEXT_COURSECAT, "ctxl_system" => \CONTEXT_SYSTEM,]);
+                                                    "ctxl_coursecat" => \CONTEXT_COURSECAT,]);
 
             $contextids = [];
             foreach ($recordset as $r) {
@@ -98,23 +124,73 @@ class courseservice extends \external_api {
                     // Double check permissions according to the moodle capability system.
                     $ctx = \context::instance_by_id($r->id);
                     if (has_capability($capability,$ctx,$userid)) {
-                    // Get the actual category object.
-                    if ($r->contextlevel == \CONTEXT_SYSTEM) {
-                        // The user can view all (non-hidden) categories, so add all categories of depth = 1;
-                        $tops = []; // Reset the array, just in case.
-                        $rs = $DB->get_recordset("course_categories",["depth" => 1],'sortorder');
-                        foreach( $rs as $rcat) {
                         // Get the category, and double check if the category is visible to the current user.
                         // Just in case it is a hidden category and the user does not have the viewhidden permission.
-                            $cat = \core_course_category::get($rcat->id, \IGNORE_MISSING, false, $userid);
+                        $cat = \core_course_category::get($r->instanceid, \IGNORE_MISSING, false, $userid);
                         if ($cat !== null) {
+                            // Register the context id in the list now, since we know the category is really visible.
+                            array_push($contextids,$r->id);
                             // Register the category.
                             array_push($tops,$cat);
+                        } else {
+                            // The category is not visible. Add the first known visible subcategories.
+                            $children = self::get_first_visible_children($r->id,$userid);
+                            foreach ($children as $cat) {
+                                array_push($tops,$cat);
                             }
                         }
-                        $rs->close();
+                    } 
-                        break; // Stop the loop immediately so the list of visible depth 2 categories is returned.
+                }
-                    } else { // Can only be \CONTEXT_COURSECAT according to the SQL query.
+            }
+            $recordset->close(); 
+        }
+
+        return $tops;
+    }
+
+    /**
+     * Find the top-most child categories for a given category that are visible.
+     * 
+     * @param int $parentid The category to search for
+     * @return array of \core_course_category
+     */
+    private static function get_first_visible_children($parentid, $userid) {
+        global $DB;
+        $capability = 'moodle/category:viewcourselist';
+
+        $tops = [];
+        $path_like = $DB->sql_like('ctx.path',':pathsearch');
+
+        $sql = "SELECT UNIQUE ctx.* FROM {context} AS ctx
+                INNER JOIN {role_assignments} AS ra ON ra.contextid = ctx.id
+                INNER JOIN {role_capabilities} AS rc ON ra.roleid = rc.roleid
+                LEFT JOIN {course_categories} AS cat ON ctx.instanceid = cat.id
+                WHERE ( ctx.contextlevel = :ctxl_coursecat )
+                AND ra.userid = :userid AND rc.capability = :capability
+                AND {$path_like}
+                ORDER BY ctx.depth ASC, cat.sortorder ASC";
+
+        // Use recordset to handle the eventuality of a really big and complex moodle setup.
+        $recordset = $DB->get_recordset_sql($sql,  ["userid" => $userid, 
+                                                    "capability" => $capability,
+                                                    "ctxl_coursecat" => \CONTEXT_COURSECAT, 
+                                                    "pathsearch" => "%/{$parentid}/%",
+                                                   ]);
+
+        $contextids = [];
+        foreach ($recordset as $r) {
+            // Get the paths as an array.
+            $parents = explode("/",$r->path);
+            // Strip the first item, since it is an empty one.
+            array_shift($parents);
+            // Strip the last item, since it refers to self.
+            array_pop($parents);
+            // Figure out if any of the remaining parent contexts are already contexts with permission.
+            $intersect = array_intersect($contextids,$parents);
+            if (count($intersect) == 0) {
+                // Double check permissions according to the moodle capability system.
+                $ctx = \context::instance_by_id($r->id);
+                if (has_capability($capability,$ctx,$userid)) {
                     // Get the category, and double check if the category is visible to the current user.
                     // Just in case it is a hidden category and the user does not have the viewhidden permission.
                     $cat = \core_course_category::get($r->instanceid, \IGNORE_MISSING, false, $userid);
@@ -127,11 +203,11 @@ class courseservice extends \external_api {
                 } 
             }
         }
-        }
         $recordset->close(); 
         return $tops;
     }
 
+
     /**
      * Return value description for map_categories function
      */
@@ -179,18 +255,26 @@ class courseservice extends \external_api {
     public static function map_categories($rootid = 0) {
         global $USER;
 
-        $root = \core_course_category::get($rootid,\MUST_EXIST,true);
 
         // Determine top categories from provided context.
 
-        if ($root->id == 0) {
+        if ($rootid == 0) {
             // On the system level, determine the user's topmost allowed catecories.
             // This uses a custom function, since moodle's "core_course_category::user_top()" is somewhat deficient.
-            $children = self::user_tops($USER->id);
+            $children = self::user_tops();
-        } else if ($root->is_uservisible()) {
+            if (count($children) == 0) {
+                throw new moodle_exception("error:nocategoriesvisible","local_treestudyplan");
+            }
+        } else {
+            
+            $root = \core_course_category::get($rootid,\MUST_EXIST,true);
+            if ($root->is_uservisible()) {
                 $children = [$root];
             } else {
-            return []; // Category not user visible.
+                $ci = new contextinfo($root->get_context());
+                $contextname = $ci->pathstr();
+                throw new moodle_exception("error:cannotviewcategory","local_treestudyplan",'',$contextname);
+            }
         }
 
         $list = [];
@@ -493,36 +577,59 @@ class courseservice extends \external_api {
         } else { // Operation == "view" || default.
             $capability = self::CAP_VIEW;
         }
-        // Retrieve context ids used.
+        
-        $contextcounts = [];
+        // Get the context ids of all categories the user has access to view and wich have the given permission.
         $contextids = [];
+        $tops = self::user_tops(null,$capability);
+        foreach ($tops as $cat) {
+            $ctx = \context_coursecat::instance($cat->id);
+            $contextids[] = $ctx->id;
+        }
+
+        // Now get an overview of the number of study plans in a given context.
+        $contextcounts = [];
+        $insertctxs = [];
         $rs = $DB->get_recordset_sql("SELECT DISTINCT context_id, COUNT(*) as num FROM {local_treestudyplan}
                                          GROUP BY context_id");
         foreach ($rs as $r) {
+            // Build the counts.
             $contextcounts[$r->context_id] = $r->num;
-            $contextids[] = $r->context_id;
+            // Add any of the categories containing studyplans to the list.
+            $ctx = \context::instance_by_id($r->context_id);
+            if (has_capability($capability,$ctx) && !in_array($r->context_id,$contextids)) {
+                $insertctxs[] = $ctx;
             }
+        }
+
         $rs->close();
 
-        // Add system context to list if needed.
-        if (!in_array(1,$contextids)) {
-            array_unshift($contextids,1);
-        }
-
         $cats = [];
-        // If the reference context id is not in the list, push it there
+
+        // If the reference context id is not in the list, push it there if the user has proper permissions in that context
         if ($refctxid > 1 && !in_array($refctxid, $contextids)) {
             try { 
+                // Get the context.
                 $refctx = \context::instance_by_id($refctxid);
-                $refpath = $refctx->get_parent_context_ids(true);
+                // Double check permissions.
+                if (has_capability($capability,$refctx)) {
+                    $insertctxs[] = $refctx;
+                }
+            } catch(\dml_missing_record_exception $x) {
+                // ignore context
+            }
+        }
+
+        foreach ($insertctxs as $ictx) {
+            // Place this context and all relevant direct parents in the correct spots.
+            $ipath = $ictx->get_parent_context_ids(true);
             $found = false;
-                foreach ($refpath as $i => $pid) {
+            foreach ($ipath as $i => $pid) {
                 $idx = array_search($pid,$contextids);
                 if($idx !== false) {
 
                     $contextids = array_merge(
                         array_slice($contextids, 0, $idx+1),
-                            array_reverse(array_slice($refpath,0,$i)),
+                        array_reverse(array_slice($ipath,0,$i)),
                         array_slice($contextids, $idx+1, count($contextids) - 1)
                         ) ;
 
@@ -531,26 +638,24 @@ class courseservice extends \external_api {
                 }
             }
             if(!$found) {
-                    array_unshift($contextids,$refctxid);
+                array_unshift($contextids,$ictx->id);
-                }
-            } catch(\dml_missing_record_exception $x) {
-                // ignore context
             }
         }
 
-        // we only have to check these contexts for access permissions
+
+        // Now translate this to the list of categories.
         foreach ($contextids as $ctxid ) {
             try {
                 $ctx = \context::instance_by_id($ctxid);
-                if (has_capability($capability, $ctx)) {
                 if ($ctx->contextlevel == CONTEXT_SYSTEM) {
                     $cat = \core_course_category::top();
                 } else if ($ctx->contextlevel == CONTEXT_COURSECAT) {
-                        $cat = \core_course_category::get($ctx->instanceid,\MUST_EXIST,true);
+                    $cat = \core_course_category::get($ctx->instanceid,\MUST_EXIST,false);
                 }
                 $cats[] = $cat;
-                    if ($operation == "view" && $ctxid == $refctxid) {
+                // In edit mode, also include direct children of the currently selected context.
-                        // Include direct children for navigation purposes
+                if ($operation == "edit" && $ctxid == $refctxid) {
+                    // Include direct children for navigation purposes.
                     foreach ($cat->get_children() as $ccat) {
                         $ccatctx = \context_coursecat::instance($ccat->id);
                         if (!in_array($ccatctx->id,$contextids)) {
@@ -558,7 +663,6 @@ class courseservice extends \external_api {
                         }
                     }
                 }
-                }
             } catch (\dml_missing_record_exception $x) {
                 // ignore context
             }

edit-plan.php

@@ -58,7 +58,6 @@ if ($categoryid > 0) {
     exit;
 }
 
-require_capability('local/treestudyplan:editstudyplan', $studyplancontext);
 $ci = new contextinfo($studyplancontext);
 $contextname = $ci->pathstr();
 
@@ -70,6 +69,21 @@ $PAGE->set_heading($contextname);
 if ($studyplancontext->id > 1) {
     navigation_node::override_active_url(new moodle_url('/course/index.php', ['categoryid' => $categoryid ]));
     $PAGE->navbar->add(get_string('cfg_plans', 'local_treestudyplan'));
+
+    // Coursecat context
+    $cat = \core_course_category::get($studyplancontext->instanceid,IGNORE_MISSING,true); // We checck visibility later
+} else { 
+    // System context
+    $cat = \core_course_category::top();
+}
+
+if (!$cat->is_uservisible()) {
+
+    throw new \moodle_exception("error:cannotviewcategory","local_treestudyplan","/local/treestudyplan/edit_plan.php",$contextname,print_r($cat,true));
+}
+
+if (!has_capability('local/treestudyplan:editstudyplan', $studyplancontext)) {
+    throw new \moodle_exception("error:nostudyplaneditaccess","local_treestudyplan","/local/treestudyplan/edit_plan.php",$contextname);
 }
 
 // Load javascripts and specific css.

lang/en/local_treestudyplan.php

@@ -429,3 +429,8 @@ $string["warning_incomplete_nograderq"] = 'Because the grade is not marked as a
 
 $string["error:nosuchcompetency"] = 'Warning: This competency no longer exists';
 $string["individuals"] = 'Individuals';
+
+$string["error:cannotviewcategory"] = 'Error: You do not have access to view this category or context: {$a}';
+$string["error:nostudyplanviewaccess"] = 'Error: You do not have access to view study plans in this category or context: {$a}';
+$string["error:nostudyplaneditaccess"] = 'Error: You do not have access to manage study plans in this category or context: {$a}';
+$string["error:nocategoriesvisible"] = 'Error: You have no viewing permissions in any category. Therefore the course list remains empty.';

lang/nl/local_treestudyplan.php

@@ -430,3 +430,7 @@ $string["warning_incomplete_nograderq"] = 'Omdat het behalen van een cijfer niet
 
 $string["error:nosuchcompetency"] = 'Waarschuwing: deze competentie is niet langer beschikbaar. ';
 $string["individuals"] = 'Individueel';
+$string["error:cannotviewcategory"] = 'Fout: Je hebt geen rechten om deze category of context te bekijken: {$a}';
+$string["error:nostudyplanviewaccess"] = 'Fout: Je hebt geen rechten om studieplannen in deze categorie of context te bekijken: {$a}';
+$string["error:nostudyplaneditaccess"] = 'Fout: Je hebt geen rechten om studieplannen in deze categorie of context te beheren: {$a}';
+$string["error:nocategoriesvisible"] = 'Fout: Je kunt geen cursussen in een categorie bekijken. Daarom blijft de cursuslijst leeg';

version.php

@@ -22,7 +22,7 @@
 defined('MOODLE_INTERNAL') || die();
 
 $plugin->component = 'local_treestudyplan';  // Recommended since 2.0.2 (MDL-26035). Required since 3.0 (MDL-48494).
-$plugin->version = 2024020400;  // YYYYMMDDHH (year, month, day, iteration).
+$plugin->version = 2024020502;  // YYYYMMDDHH (year, month, day, iteration).
 $plugin->requires = 2021051700; // YYYYMMDDHH (This is the release version for Moodle 3.11).
 
 $plugin->release = "1.1.0";

view-plan.php

@@ -22,6 +22,7 @@
 
 require_once("../../config.php");
 
+use local_treestudyplan\contextinfo;
 use \local_treestudyplan\courseservice;
 
 require_once($CFG->libdir.'/weblib.php');
@@ -60,8 +61,11 @@ if ($categoryid > 0) {
     exit;
 }
 
-require_capability('local/treestudyplan:viewuserreports', $studyplancontext);
+$ci = new contextinfo($studyplancontext);
-$contextname = $studyplancontext->get_context_name(false, false);
+$contextname = $ci->pathstr();
+
+
+
 
 $PAGE->set_pagelayout('base');
 //$PAGE->set_context($studyplancontext);
@@ -71,6 +75,20 @@ $PAGE->set_heading(get_string('view_plan', 'local_treestudyplan')." - ".$context
 if ($studyplancontext->id > 1) {
     navigation_node::override_active_url(new moodle_url('/course/index.php', ['categoryid' => $categoryid ]));
     $PAGE->navbar->add(get_string('view_plan', 'local_treestudyplan'));
+
+    // Coursecat context
+    $cat = \core_course_category::get($studyplancontext->instanceid,IGNORE_MISSING,true); // We checck visibility later
+} else { 
+    // System context
+    $cat = \core_course_category::top();
+}
+
+if (!$cat->is_uservisible()) {
+    throw new \moodle_exception("error:cannotviewcategory","local_treestudyplan","/local/treestudyplan/view_plan.php",$contextname);
+}
+
+if(!has_capability('local/treestudyplan:viewuserreports', $studyplancontext)) {
+    throw new \moodle_exception("error:nostudyplanviewaccess","local_treestudyplan","/local/treestudyplan/view_plan.php",$contextname);
 }
 
 // Load javascripts and specific css.