From 3b8876f5ef2b5cde1e9de2599efd03d02bdaf7d8 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Thu, 23 Oct 2014 11:05:57 +0200 Subject: [PATCH] MDL-47766 web services: get_grades exposes hidden grades to students --- lib/classes/grades_external.php | 6 +++- lib/db/services.php | 7 ++-- lib/tests/grades_externallib_test.php | 51 +++++++++++++++------------ 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/lib/classes/grades_external.php b/lib/classes/grades_external.php index 11ba4d3e313..57193649727 100644 --- a/lib/classes/grades_external.php +++ b/lib/classes/grades_external.php @@ -55,7 +55,9 @@ class core_grades_external extends external_api { } /** - * Retrieve grade items and, optionally, student grades + * Returns student course total grade and grades for activities. + * This function does not return category or manual items. + * This function is suitable for managers or teachers not students. * * @param int $courseid Course id * @param string $component Component name @@ -86,6 +88,8 @@ class core_grades_external extends external_api { throw new moodle_exception('errorcoursecontextnotvalid' , 'webservice', '', $exceptionparam); } + require_capability('moodle/grade:viewhidden', $coursecontext); + $course = $DB->get_record('course', array('id' => $params['courseid']), '*', MUST_EXIST); $access = false; diff --git a/lib/db/services.php b/lib/db/services.php index cc81df42d0c..c95271cfa23 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -104,9 +104,11 @@ $functions = array( 'core_grades_get_grades' => array( 'classname' => 'core_grades_external', 'methodname' => 'get_grades', - 'description' => 'Returns grade item details and optionally student grades.', + 'description' => 'Returns student course total grade and grades for activities. + This function does not return category or manual items. + This function is suitable for managers or teachers not students.', 'type' => 'read', - 'capabilities' => 'moodle/grade:view, moodle/grade:viewall', + 'capabilities' => 'moodle/grade:view, moodle/grade:viewall, moodle/grade:viewhidden', ), 'core_grades_update_grades' => array( @@ -946,7 +948,6 @@ $services = array( 'mod_assign_reveal_identities', 'message_airnotifier_is_system_configured', 'message_airnotifier_are_notification_preferences_configured', - 'core_grades_get_grades', 'core_grades_update_grades', 'mod_forum_get_forums_by_courses', 'mod_forum_get_forum_discussions', diff --git a/lib/tests/grades_externallib_test.php b/lib/tests/grades_externallib_test.php index 8a84f2e40f4..f5732923027 100644 --- a/lib/tests/grades_externallib_test.php +++ b/lib/tests/grades_externallib_test.php @@ -150,8 +150,8 @@ class core_grades_external_testcase extends externallib_advanced_testcase { $this->load_test_data($assignmentname, $student1rawgrade, $student2rawgrade); $assigmentcm = get_coursemodule_from_id('assign', $assignment->id, 0, false, MUST_EXIST); - // Student requesting their own grade for the assignment. - $this->setUser($student1); + // Teacher requesting a student grade for the assignment. + $this->setUser($teacher); $grades = core_grades_external::get_grades( $course->id, 'mod_assign', @@ -161,7 +161,7 @@ class core_grades_external_testcase extends externallib_advanced_testcase { $grades = external_api::clean_returnvalue(core_grades_external::get_grades_returns(), $grades); $this->assertEquals($student1rawgrade, $this->get_activity_student_grade($grades, $assigmentcm->id, $student1->id)); - // Student requesting all of their grades in a course. + // Teacher requesting all the grades of student1 in a course. $grades = core_grades_external::get_grades( $course->id, null, @@ -177,7 +177,20 @@ class core_grades_external_testcase extends externallib_advanced_testcase { $this->assertEquals($outcome['name'], 'Team work'); $this->assertEquals(0, $this->get_outcome_student_grade($grades, $assigmentcm->id, $student1->id)); + // Teacher requesting all the grades of all the students in a course. + $grades = core_grades_external::get_grades( + $course->id, + null, + null, + array($student1->id, $student2->id) + ); + $grades = external_api::clean_returnvalue(core_grades_external::get_grades_returns(), $grades); + $this->assertTrue(count($grades['items']) == 2); + $this->assertTrue(count($grades['items'][0]['grades']) == 2); + $this->assertTrue(count($grades['items'][1]['grades']) == 2); + // Student requesting another student's grade for the assignment (should fail). + $this->setUser($student1); try { $grades = core_grades_external::get_grades( $course->id, @@ -190,16 +203,19 @@ class core_grades_external_testcase extends externallib_advanced_testcase { $this->assertTrue(true); } - // Parent requesting their child's grade for the assignment. + // Parent requesting their child's grade for the assignment (should fail). $this->setUser($parent); - $grades = core_grades_external::get_grades( - $course->id, - 'mod_assign', - $assigmentcm->id, - array($student1->id) - ); - $grades = external_api::clean_returnvalue(core_grades_external::get_grades_returns(), $grades); - $this->assertEquals($student1rawgrade, $this->get_activity_student_grade($grades, $assigmentcm->id, $student1->id)); + try { + $grades = core_grades_external::get_grades( + $course->id, + 'mod_assign', + $assigmentcm->id, + array($student1->id) + ); + $this->fail('moodle_exception expected'); + } catch (moodle_exception $ex) { + $this->assertTrue(true); + } // Parent requesting another student's grade for the assignment(should fail). try { @@ -294,17 +310,6 @@ class core_grades_external_testcase extends externallib_advanced_testcase { $grades = grade_get_grades($course->id, 'mod', 'assign', $assignment->id); $this->assertEquals($grades->items[0]->hidden, 1); - // Student should now not be able to see it. - $this->setUser($student1); - $grades = core_grades_external::get_grades( - $course->id, - 'mod_assign', - $assigmentcm->id, - array($student1->id) - ); - $grades = external_api::clean_returnvalue(core_grades_external::get_grades_returns(), $grades); - $this->assertEquals(null, $this->get_activity($grades, $assigmentcm->id)); - // Teacher should still be able to see the hidden grades. $this->setUser($teacher); $grades = core_grades_external::get_grades(