MDL-66730 core_course: Improve permission check for category moving

Co-authored-by: Erica Bithell <egb10@cam.ac.uk>
moodle · Oct 4, 2023 · 86d104b · 86d104b
1 parent a75bb40
commit 86d104b
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 0 deletions.
diff --git a/course/externallib.php b/course/externallib.php
@@ -2140,6 +2140,19 @@ public static function update_categories($categories) {
             self::validate_context($categorycontext);
             require_capability('moodle/category:manage', $categorycontext);
 
+            // If the category parent is being changed, check for capability in the new parent category
+            if (isset($cat['parent']) && ($cat['parent'] !== $category->parent)) {
+                if ($cat['parent'] == 0) {
+                    // Creating a top level category requires capability in the system context
+                    $parentcontext = context_system::instance();
+                } else {
+                    // Category context
+                    $parentcontext = context_coursecat::instance($cat['parent']);
+                }
+                self::validate_context($parentcontext);
+                require_capability('moodle/category:manage', $parentcontext);
+            }
+
             // this will throw an exception if descriptionformat is not valid
             util::validate_format($cat['descriptionformat']);
 

diff --git a/course/tests/externallib_test.php b/course/tests/externallib_test.php
@@ -365,6 +365,44 @@ public function test_update_categories() {
         core_course_external::update_categories($categories);
     }
 
+    /**
+     * Test update_categories method for moving categories
+     */
+    public function test_update_categories_moving() {
+        $this->resetAfterTest();
+
+        // Create data.
+        $categorya  = self::getDataGenerator()->create_category([
+            'name' => 'CAT_A',
+        ]);
+        $categoryasub = self::getDataGenerator()->create_category([
+            'name' => 'SUBCAT_A',
+            'parent' => $categorya->id
+        ]);
+        $categoryb  = self::getDataGenerator()->create_category([
+            'name' => 'CAT_B',
+        ]);
+
+        // Create a new test user.
+        $testuser = self::getDataGenerator()->create_user();
+        $this->setUser($testuser);
+
+        // Set the capability for CAT_A only.
+        $contextcata = context_coursecat::instance($categorya->id);
+        $roleid = $this->assignUserCapability('moodle/category:manage', $contextcata->id);
+
+        // Then we move SUBCAT_A parent: CAT_A => CAT_B.
+        $categories = [
+            [
+                'id' => $categoryasub->id,
+                'parent' => $categoryb->id
+            ]
+        ];
+
+        $this->expectException('required_capability_exception');
+        core_course_external::update_categories($categories);
+    }
+
     /**
      * Test create_courses numsections
      */