From 808ad0df737e4979aadc276ebb8b4f8f4eaea71d Mon Sep 17 00:00:00 2001 From: Juan Segarra Montesinos Date: Tue, 27 Oct 2020 19:06:49 +0100 Subject: [PATCH] MDL-69797 gradingform_guide: Implement authorization into fetch method --- .../grader/gradingpanel/external/fetch.php | 9 ++- ...gradingpanel_guide_external_fetch_test.php | 68 ++++++++++++++++--- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/grade/grading/form/guide/classes/grades/grader/gradingpanel/external/fetch.php b/grade/grading/form/guide/classes/grades/grader/gradingpanel/external/fetch.php index f1f0f5d52f3..36823725920 100644 --- a/grade/grading/form/guide/classes/grades/grader/gradingpanel/external/fetch.php +++ b/grade/grading/form/guide/classes/grades/grader/gradingpanel/external/fetch.php @@ -100,7 +100,7 @@ class fetch extends external_api { * @since Moodle 3.8 */ public static function execute(string $component, int $contextid, string $itemname, int $gradeduserid): array { - global $CFG; + global $CFG, $USER; require_once("{$CFG->libdir}/gradelib.php"); [ 'component' => $component, @@ -133,7 +133,12 @@ class fetch extends external_api { } // Fetch the actual data. - $gradeduser = core_user::get_user($gradeduserid); + $gradeduser = core_user::get_user($gradeduserid, '*', MUST_EXIST); + + // One can access its own grades. Others just if they're graders. + if ($gradeduserid != $USER->id) { + $gradeitem->require_user_can_grade($gradeduser, $USER); + } return self::get_fetch_data($gradeitem, $gradeduser); } diff --git a/grade/grading/form/guide/tests/grades_grader_gradingpanel_guide_external_fetch_test.php b/grade/grading/form/guide/tests/grades_grader_gradingpanel_guide_external_fetch_test.php index 50ec39ba4fe..77c05972866 100644 --- a/grade/grading/form/guide/tests/grades_grader_gradingpanel_guide_external_fetch_test.php +++ b/grade/grading/form/guide/tests/grades_grader_gradingpanel_guide_external_fetch_test.php @@ -178,8 +178,6 @@ class fetch_test extends advanced_testcase { */ public function test_execute_fetch_graded(): void { $this->resetAfterTest(); - $generator = \testing_util::get_data_generator(); - $guidegenerator = $generator->get_plugin_generator('gradingform_guide'); [ 'forum' => $forum, @@ -189,23 +187,74 @@ class fetch_test extends advanced_testcase { 'teacher' => $teacher, ] = $this->get_test_data(); - $this->setUser($teacher); + $this->execute_and_assert_fetch($forum, $controller, $definition, $teacher, $teacher, $student); + } + + /** + * Class mates should not get other's grades. + */ + public function test_execute_fetch_does_not_return_data_to_other_students(): void { + $this->resetAfterTest(); + + [ + 'forum' => $forum, + 'controller' => $controller, + 'definition' => $definition, + 'student' => $student, + 'teacher' => $teacher, + 'course' => $course, + ] = $this->get_test_data(); + + $evilstudent = $this->getDataGenerator()->create_and_enrol($course, 'student'); + + $this->expectException(\required_capability_exception::class); + $this->execute_and_assert_fetch($forum, $controller, $definition, $evilstudent, $teacher, $student); + } + + /** + * Grades can be returned to graded user. + */ + public function test_execute_fetch_return_data_to_graded_user(): void { + $this->resetAfterTest(); + + [ + 'forum' => $forum, + 'controller' => $controller, + 'definition' => $definition, + 'student' => $student, + 'teacher' => $teacher, + ] = $this->get_test_data(); + + $this->execute_and_assert_fetch($forum, $controller, $definition, $student, $teacher, $student); + } + + /** + * Executes and performs all the assertions of the fetch method with the given parameters. + */ + private function execute_and_assert_fetch ($forum, $controller, $definition, $fetcheruser, $grader, $gradeduser) { + $generator = \testing_util::get_data_generator(); + $guidegenerator = $generator->get_plugin_generator('gradingform_guide'); + + $this->setUser($grader); $gradeitem = component_gradeitem::instance('mod_forum', $forum->get_context(), 'forum'); - $grade = $gradeitem->get_grade_for_user($student, $teacher); - $instance = $gradeitem->get_advanced_grading_instance($teacher, $grade); + $grade = $gradeitem->get_grade_for_user($gradeduser, $grader); + $instance = $gradeitem->get_advanced_grading_instance($grader, $grade); - $submissiondata = $guidegenerator->get_test_form_data($controller, (int) $student->id, + $submissiondata = $guidegenerator->get_test_form_data($controller, (int) $gradeduser->id, 10, 'Propper good speling', 0, 'ASCII art is not a picture' ); - $gradeitem->store_grade_from_formdata($student, $teacher, (object) [ + $gradeitem->store_grade_from_formdata($gradeduser, $grader, (object) [ 'instanceid' => $instance->get_id(), 'advancedgrading' => $submissiondata, ]); + + $this->setUser($fetcheruser); + // Set up some items we need to return on other interfaces. - $result = fetch::execute('mod_forum', (int) $forum->get_context()->id, 'forum', (int) $student->id); + $result = fetch::execute('mod_forum', (int) $forum->get_context()->id, 'forum', (int) $gradeduser->id); $result = external_api::clean_returnvalue(fetch::execute_returns(), $result); $this->assertIsArray($result); @@ -233,7 +282,7 @@ class fetch_test extends advanced_testcase { $this->assertEquals(100, $result['grade']['maxgrade']); $this->assertArrayHasKey('gradedby', $result['grade']); - $this->assertEquals(fullname($teacher), $result['grade']['gradedby']); + $this->assertEquals(fullname($grader), $result['grade']['gradedby']); $this->assertArrayHasKey('criterion', $result['grade']); $criteria = $result['grade']['criterion']; @@ -326,6 +375,7 @@ class fetch_test extends advanced_testcase { 'definition' => $definition, 'student' => $student, 'teacher' => $teacher, + 'course' => $course, ]; }