MDL-47766 web services: get_grades exposes hidden grades to students

diff --git a/lib/classes/grades_external.php b/lib/classes/grades_external.php
index 11ba4d3..5719364 100644 (file)
--- 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 f71f55f..c9318ff 100644 (file)
--- 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(
@@ -962,7 +964,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_paginated',

diff --git a/lib/tests/grades_externallib_test.php b/lib/tests/grades_externallib_test.php
index d5d7336..d0afb5e 100644 (file)
--- 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->cmid, 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(