MDL-52072 enrol: Fix course visibility checks in external functions

moodle · Jan 5, 2016 · c14e2d6 · c14e2d6
1 parent 25cc38f
commit c14e2d6
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 8 deletions.
diff --git a/enrol/externallib.php b/enrol/externallib.php
@@ -603,14 +603,21 @@ public static function get_course_enrolment_methods_parameters() {
      *
      * @param int $courseid
      * @return array of course enrolment methods
+     * @throws moodle_exception
      */
     public static function get_course_enrolment_methods($courseid) {
+        global $DB;
 
         $params = self::validate_parameters(self::get_course_enrolment_methods_parameters(), array('courseid' => $courseid));
 
-        $coursecontext = context_course::instance($params['courseid']);
-        $categorycontext = $coursecontext->get_parent_context();
-        self::validate_context($categorycontext);
+        // Note that we can't use validate_context because the user is not enrolled in the course.
+        require_login(null, false, null, false, true);
+
+        $course = $DB->get_record('course', array('id' => $params['courseid']), '*', MUST_EXIST);
+        $context = context_course::instance($course->id);
+        if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $context)) {
+            throw new moodle_exception('coursehidden');
+        }
 
         $result = array();
         $enrolinstances = enrol_get_instances($params['courseid'], true);

diff --git a/enrol/self/externallib.php b/enrol/self/externallib.php
@@ -52,6 +52,7 @@ public static function get_instance_info_parameters() {
      *
      * @param int $instanceid instance id of self enrolment plugin.
      * @return array instance information.
+     * @throws moodle_exception
      */
     public static function get_instance_info($instanceid) {
         global $DB, $CFG;
@@ -66,10 +67,15 @@ public static function get_instance_info($instanceid) {
             throw new moodle_exception('invaliddata', 'error');
         }
 
+        // Note that we can't use validate_context because the user is not enrolled in the course.
+        require_login(null, false, null, false, true);
+
         $enrolinstance = $DB->get_record('enrol', array('id' => $params['instanceid']), '*', MUST_EXIST);
-        $coursecontext = context_course::instance($enrolinstance->courseid);
-        $categorycontext = $coursecontext->get_parent_context();
-        self::validate_context($categorycontext);
+        $course = $DB->get_record('course', array('id' => $enrolinstance->courseid), '*', MUST_EXIST);
+        $context = context_course::instance($course->id);
+        if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $context)) {
+            throw new moodle_exception('coursehidden');
+        }
 
         $instanceinfo = (array) $enrolplugin->get_enrol_info($enrolinstance);
         if (isset($instanceinfo['requiredparam']->enrolpassword)) {

diff --git a/enrol/self/tests/externallib_test.php b/enrol/self/tests/externallib_test.php
@@ -47,7 +47,9 @@ public function test_get_instance_info() {
         $studentrole = $DB->get_record('role', array('shortname'=>'student'));
         $this->assertNotEmpty($studentrole);
 
-        $course = self::getDataGenerator()->create_course();
+        $coursedata = new stdClass();
+        $coursedata->visible = 0;
+        $course = self::getDataGenerator()->create_course($coursedata);
 
         // Add enrolment methods for course.
         $instanceid1 = $selfplugin->add_instance($course, array('status' => ENROL_INSTANCE_ENABLED,
@@ -68,6 +70,7 @@ public function test_get_instance_info() {
         $enrolmentmethods = $DB->get_records('enrol', array('courseid' => $course->id, 'status' => ENROL_INSTANCE_ENABLED));
         $this->assertCount(3, $enrolmentmethods);
 
+        $this->setAdminUser();
         $instanceinfo1 = enrol_self_external::get_instance_info($instanceid1);
         $instanceinfo1 = external_api::clean_returnvalue(enrol_self_external::get_instance_info_returns(), $instanceinfo1);
 
@@ -95,5 +98,14 @@ public function test_get_instance_info() {
         $this->assertEquals('Test instance 3', $instanceinfo3['name']);
         $this->assertTrue($instanceinfo3['status']);
         $this->assertEquals(get_string('password', 'enrol_self'), $instanceinfo3['enrolpassword']);
+
+        // Try to retrieve information using a normal user for a hidden course.
+        $user = self::getDataGenerator()->create_user();
+        $this->setUser($user);
+        try {
+            enrol_self_external::get_instance_info($instanceid3);
+        } catch (moodle_exception $e) {
+            $this->assertEquals('coursehidden', $e->errorcode);
+        }
     }
 }
diff --git a/enrol/tests/externallib_test.php b/enrol/tests/externallib_test.php
@@ -552,7 +552,9 @@ public function test_get_course_enrolment_methods() {
         $this->assertNotEmpty($studentrole);
 
         $course1 = self::getDataGenerator()->create_course();
-        $course2 = self::getDataGenerator()->create_course();
+        $coursedata = new stdClass();
+        $coursedata->visible = 0;
+        $course2 = self::getDataGenerator()->create_course($coursedata);
 
         // Add enrolment methods for course.
         $instanceid1 = $selfplugin->add_instance($course1, array('status' => ENROL_INSTANCE_ENABLED,
@@ -569,6 +571,8 @@ public function test_get_course_enrolment_methods() {
         $enrolmentmethods = $DB->get_records('enrol', array('courseid' => $course1->id, 'status' => ENROL_INSTANCE_ENABLED));
         $this->assertCount(2, $enrolmentmethods);
 
+        $this->setAdminUser();
+
         // Check if information is returned.
         $enrolmentmethods = core_enrol_external::get_course_enrolment_methods($course1->id);
         $enrolmentmethods = external_api::clean_returnvalue(core_enrol_external::get_course_enrolment_methods_returns(),
@@ -598,5 +602,14 @@ public function test_get_course_enrolment_methods() {
         $this->assertEquals('self', $enrolmentmethod['type']);
         $this->assertTrue($enrolmentmethod['status']);
         $this->assertEquals('enrol_self_get_instance_info', $enrolmentmethod['wsfunction']);
+
+        // Try to retrieve information using a normal user for a hidden course.
+        $user = self::getDataGenerator()->create_user();
+        $this->setUser($user);
+        try {
+            core_enrol_external::get_course_enrolment_methods($course2->id);
+        } catch (moodle_exception $e) {
+            $this->assertEquals('coursehidden', $e->errorcode);
+        }
     }
 }