MDL-48391 tool_uploadcourse: access checks for configuring enrolments.
authorPaul Holden <paulh@moodle.com>
Thu, 14 May 2020 22:39:09 +0000 (23:39 +0100)
committerPaul Holden <paulh@moodle.com>
Wed, 22 Jul 2020 16:52:24 +0000 (17:52 +0100)
admin/tool/uploadcourse/classes/course.php
admin/tool/uploadcourse/classes/helper.php
admin/tool/uploadcourse/lang/en/tool_uploadcourse.php
admin/tool/uploadcourse/tests/course_test.php

index 88cd888..fd7569e 100644 (file)
@@ -770,7 +770,20 @@ class tool_uploadcourse_course {
 
         // Saving data.
         $this->data = $coursedata;
+
+        // Get enrolment data. Where the course already exists, we can also perform validation.
         $this->enrolmentdata = tool_uploadcourse_helper::get_enrolment_data($this->rawdata);
+        if ($exists) {
+            $errors = $this->validate_enrolment_data($coursedata['id'], $this->enrolmentdata);
+
+            if (!empty($errors)) {
+                foreach ($errors as $key => $message) {
+                    $this->error($key, $message);
+                }
+
+                return false;
+            }
+        }
 
         if (isset($this->rawdata['tags']) && strval($this->rawdata['tags']) !== '') {
             $this->data['tags'] = preg_split('/\s*,\s*/', trim($this->rawdata['tags']), -1, PREG_SPLIT_NO_EMPTY);
@@ -871,6 +884,71 @@ class tool_uploadcourse_course {
         $context->mark_dirty();
     }
 
+    /**
+     * Validate passed enrolment data against an existing course
+     *
+     * @param int $courseid
+     * @param array[] $enrolmentdata
+     * @return lang_string[] Errors keyed on error code
+     */
+    protected function validate_enrolment_data(int $courseid, array $enrolmentdata): array {
+        // Nothing to validate.
+        if (empty($enrolmentdata)) {
+            return [];
+        }
+
+        $errors = [];
+
+        $enrolmentplugins = tool_uploadcourse_helper::get_enrolment_plugins();
+        $instances = enrol_get_instances($courseid, false);
+
+        foreach ($enrolmentdata as $method => $options) {
+            $plugin = $enrolmentplugins[$method];
+
+            // Find matching instances by enrolment method.
+            $methodinstances = array_filter($instances, static function(stdClass $instance) use ($method) {
+                return (strcmp($instance->enrol, $method) == 0);
+            });
+
+            if (!empty($options['delete'])) {
+                // Ensure user is able to delete the instances.
+                foreach ($methodinstances as $methodinstance) {
+                    if (!$plugin->can_delete_instance($methodinstance)) {
+                        $errors['errorcannotdeleteenrolment'] = new lang_string('errorcannotdeleteenrolment', 'tool_uploadcourse',
+                            $plugin->get_instance_name($methodinstance));
+
+                        break;
+                    }
+                }
+            } else if (!empty($options['disable'])) {
+                // Ensure user is able to toggle instance statuses.
+                foreach ($methodinstances as $methodinstance) {
+                    if (!$plugin->can_hide_show_instance($methodinstance)) {
+                        $errors['errorcannotdisableenrolment'] =
+                            new lang_string('errorcannotdisableenrolment', 'tool_uploadcourse',
+                                $plugin->get_instance_name($methodinstance));
+
+                        break;
+                    }
+                }
+            } else {
+                // Ensure user is able to create/update instance.
+                $methodinstance = empty($methodinstances) ? null : reset($methodinstances);
+                if ((empty($methodinstance) && !$plugin->can_add_instance($courseid)) ||
+                        (!empty($methodinstance) && !$plugin->can_edit_instance($methodinstance))) {
+
+                    $errors['errorcannotcreateorupdateenrolment'] =
+                        new lang_string('errorcannotcreateorupdateenrolment', 'tool_uploadcourse',
+                            $plugin->get_instance_name($methodinstance));
+
+                    break;
+                }
+            }
+        }
+
+        return $errors;
+    }
+
     /**
      * Add the enrolment data for the course.
      *
@@ -907,7 +985,16 @@ class tool_uploadcourse_course {
                 foreach ($instances as $instance) {
                     if ($instance->enrol == $enrolmethod) {
                         $plugin = $enrolmentplugins[$instance->enrol];
-                        $plugin->delete_instance($instance);
+
+                        // Ensure user is able to delete the instance.
+                        if ($plugin->can_delete_instance($instance)) {
+                            $plugin->delete_instance($instance);
+                        } else {
+                            $this->error('errorcannotdeleteenrolment',
+                                new lang_string('errorcannotdeleteenrolment', 'tool_uploadcourse',
+                                    $plugin->get_instance_name($instance)));
+                        }
+
                         break;
                     }
                 }
@@ -916,22 +1003,37 @@ class tool_uploadcourse_course {
                 foreach ($instances as $instance) {
                     if ($instance->enrol == $enrolmethod) {
                         $plugin = $enrolmentplugins[$instance->enrol];
-                        $plugin->update_status($instance, ENROL_INSTANCE_DISABLED);
-                        $enrol_updated = true;
+
+                        // Ensure user is able to toggle instance status.
+                        if ($plugin->can_hide_show_instance($instance)) {
+                            $plugin->update_status($instance, ENROL_INSTANCE_DISABLED);
+                        } else {
+                            $this->error('errorcannotdisableenrolment',
+                                new lang_string('errorcannotdisableenrolment', 'tool_uploadcourse',
+                                    $plugin->get_instance_name($instance)));
+                        }
+
                         break;
                     }
                 }
             } else {
-                $plugin = null;
-                if (empty($instance)) {
-                    $plugin = $enrolmentplugins[$enrolmethod];
+                // Create/update enrolment.
+                $plugin = $enrolmentplugins[$enrolmethod];
+
+                // Ensure user is able to create/update instance.
+                if (empty($instance) && $plugin->can_add_instance($course->id)) {
                     $instance = new stdClass();
                     $instance->id = $plugin->add_default_instance($course);
                     $instance->roleid = $plugin->get_config('roleid');
                     $instance->status = ENROL_INSTANCE_ENABLED;
-                } else {
-                    $plugin = $enrolmentplugins[$instance->enrol];
+                } else if (!empty($instance) && $plugin->can_edit_instance($instance)) {
                     $plugin->update_status($instance, ENROL_INSTANCE_ENABLED);
+                } else {
+                    $this->error('errorcannotcreateorupdateenrolment',
+                        new lang_string('errorcannotcreateorupdateenrolment', 'tool_uploadcourse',
+                            $plugin->get_instance_name($instance)));
+
+                    break;
                 }
 
                 // Now update values.
index 1011c37..a91e790 100644 (file)
@@ -175,7 +175,7 @@ class tool_uploadcourse_helper {
      *
      * The result is cached for faster execution.
      *
-     * @return array
+     * @return enrol_plugin[]
      */
     public static function get_enrolment_plugins() {
         $cache = cache::make('tool_uploadcourse', 'helper');
index b44b4f3..9a96d10 100644 (file)
@@ -78,6 +78,9 @@ $string['defaultvalues'] = 'Default course values';
 $string['defaultvaluescustomfieldcategory'] = 'Default values for \'{$a}\'';
 $string['encoding'] = 'Encoding';
 $string['encoding_help'] = 'Encoding of the CSV file.';
+$string['errorcannotcreateorupdateenrolment'] = 'Cannot create or update enrolment method \'{$a}\'';
+$string['errorcannotdeleteenrolment'] = 'Cannot delete enrolment method \'{$a}\'';
+$string['errorcannotdisableenrolment'] = 'Cannot disable enrolment method \'{$a}\'';
 $string['errorwhilerestoringcourse'] = 'Error while restoring the course';
 $string['errorwhiledeletingcourse'] = 'Error while deleting the course';
 $string['generatedshortnameinvalid'] = 'The generated shortname is invalid';
index 7d811cb..9bd1f4e 100644 (file)
@@ -1055,6 +1055,9 @@ class tool_uploadcourse_course_testcase extends advanced_testcase {
     public function test_enrolment_data() {
         $this->resetAfterTest(true);
 
+        // We need to set the current user as one with the capability to edit manual enrolment instances in the new course.
+        $this->setAdminUser();
+
         $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
         $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
         $data = array('shortname' => 'c1', 'summary' => 'S', 'fullname' => 'FN', 'category' => '1');
@@ -1081,6 +1084,123 @@ class tool_uploadcourse_course_testcase extends advanced_testcase {
         $this->assertEquals(strtotime('12th July 2013'), $enroldata['manual']->enrolenddate);
     }
 
+    /**
+     * Data provider for testing enrolment errors
+     *
+     * @return array
+     */
+    public function enrolment_uploaddata_error_provider(): array {
+        return [
+            ['errorcannotcreateorupdateenrolment', [
+                'shortname' => 'C1',
+                'enrolment_1' => 'manual',
+            ]],
+            ['errorcannotdeleteenrolment', [
+                'shortname' => 'C1',
+                'enrolment_1' => 'manual',
+                'enrolment_1_delete' => '1',
+            ]],
+            ['errorcannotdisableenrolment', [
+                'shortname' => 'C1',
+                'enrolment_1' => 'manual',
+                'enrolment_1_disable' => '1',
+            ]],
+        ];
+    }
+
+    /**
+     * Test that user without permission, cannot modify enrolment instances when creating courses
+     *
+     * @param string $expectederror
+     * @param array $uploaddata
+     *
+     * @dataProvider enrolment_uploaddata_error_provider
+     */
+    public function test_enrolment_error_create_course(string $expectederror, array $uploaddata): void {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // Create category in which to create the new course.
+        $category = $this->getDataGenerator()->create_category();
+        $categorycontext = context_coursecat::instance($category->id);
+
+        $user = $this->getDataGenerator()->create_user();
+        $this->setUser($user);
+
+        // Assign the user as a manager of the category, disable ability to configure manual enrolment instances.
+        $roleid = $DB->get_field('role', 'id', ['shortname' => 'manager']);
+        role_assign($roleid, $user->id, $categorycontext);
+        role_change_permission($roleid, $categorycontext, 'enrol/manual:config', CAP_PROHIBIT);
+
+        $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
+        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
+
+        $upload = new tool_uploadcourse_course($mode, $updatemode, array_merge($uploaddata, [
+            'category' => $category->id,
+            'fullname' => 'My course',
+        ]));
+
+        // Enrolment validation isn't performed during 'prepare' for new courses.
+        $this->assertTrue($upload->prepare());
+        $upload->proceed();
+
+        $errors = $upload->get_errors();
+        $this->assertArrayHasKey($expectederror, $errors);
+
+        $this->assertEquals(get_string($expectederror, 'tool_uploadcourse', 'Manual enrolments'),
+            (string) $errors[$expectederror]);
+    }
+
+    /**
+     * Test that user without permission, cannot modify enrolment instances when updating courses
+     *
+     * @param string $expectederror
+     * @param array $uploaddata
+     *
+     * @dataProvider enrolment_uploaddata_error_provider
+     */
+    public function test_enrolment_error_update_course(string $expectederror, array $uploaddata): void {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // Create category in which to create the new course.
+        $category = $this->getDataGenerator()->create_category();
+        $categorycontext = context_coursecat::instance($category->id);
+
+        $course = $this->getDataGenerator()->create_course([
+            'category' => $category->id,
+            'shortname' => $uploaddata['shortname'],
+        ]);
+
+        $user = $this->getDataGenerator()->create_user();
+        $this->setUser($user);
+
+        // Assign the user as a manager of the category, disable ability to configure manual enrolment instances.
+        $roleid = $DB->get_field('role', 'id', ['shortname' => 'manager']);
+        role_assign($roleid, $user->id, $categorycontext);
+        role_change_permission($roleid, $categorycontext, 'enrol/manual:config', CAP_PROHIBIT);
+
+        // Sanity check.
+        $instances = enrol_get_instances($course->id, true);
+        $this->assertCount(1, $instances);
+        $this->assertEquals('manual', reset($instances)->enrol);
+
+        $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY;
+        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
+
+        $upload = new tool_uploadcourse_course($mode, $updatemode, $uploaddata);
+
+        $this->assertFalse($upload->prepare());
+
+        $errors = $upload->get_errors();
+        $this->assertArrayHasKey($expectederror, $errors);
+
+        $this->assertEquals(get_string($expectederror, 'tool_uploadcourse', 'Manual enrolments'),
+            (string) $errors[$expectederror]);
+    }
+
     /**
      * Test upload processing of course custom fields
      */