From 1c2bdf454a99dec06147437f1fd5aeaa7ada48e7 Mon Sep 17 00:00:00 2001
From: Huong Nguyen <huongnv13@gmail.com>
Date: Mon, 18 Sep 2023 16:30:46 +0700
Subject: [PATCH] MDL-66730 core_course: Improve permission check for category
 moving

Co-authored-by: Erica Bithell <egb10@cam.ac.uk>
---
 course/externallib.php            | 13 +++++++++++
 course/tests/externallib_test.php | 38 +++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/course/externallib.php b/course/externallib.php
index 1a737653f96..679262aff4c 100644
--- a/course/externallib.php
+++ b/course/externallib.php
@@ -2131,6 +2131,19 @@ class core_course_external extends external_api {
             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
             external_validate_format($cat['descriptionformat']);
 
diff --git a/course/tests/externallib_test.php b/course/tests/externallib_test.php
index 231c6fba399..2e46cf8197d 100644
--- a/course/tests/externallib_test.php
+++ b/course/tests/externallib_test.php
@@ -362,6 +362,44 @@ class core_course_externallib_testcase extends externallib_advanced_testcase {
         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
      */