MDL-20293 strict param validation support - fixed group external lib methods

diff --git a/group/externallib.php b/group/externallib.php
index 2b20f42c73ffd0405a4b26fcf3a2a0cbbac2d873..ced9041c809202c36e07358ba287828e7095a5de 100644 (file)
--- a/group/externallib.php
+++ b/group/externallib.php
@@ -41,14 +41,23 @@ class moodle_group_external extends external_api {
 
         foreach ($params as $groupparam) {
             $group = new object();
-            // clean params
-            $group->courseid  = clean_param($groupparam['courseid'], PARAM_INTEGER);
-            $group->name      = clean_param($groupparam['groupname'], PARAM_MULTILANG);
+
+            // validate params
+            $group->courseid    = validate_param($groupparam['courseid'], PARAM_INTEGER);
+            $group->name        = validate_param($groupparam['groupname'], PARAM_MULTILANG); // must be course unique!
+            $group->description = validate_param($groupparam['description'], PARAM_RAW);
             if (array_key_exists('enrolmentkey', $groupparam)) {
-                $group->enrolmentkey = $groupparam['enrolmentkey'];
+                $group->enrolmentkey = validate_param($groupparam['enrolmentkey'], PARAM_RAW);
             } else {
                 $group->enrolmentkey = '';
             }
+            if (empty($group->name)) {
+                throw new invalid_parameter_exception('Invalid group name');
+            }
+            if ($DB->get_record('groups', array('courseid'=>$group->courseid, 'name'=>$group->name))) {
+                throw new invalid_parameter_exception('Group with the same name already exists in the course');
+            }
+            
             // now security checks
             $context = get_context_instance(CONTEXT_COURSE, $group->courseid);
             self::validate_context($context);
@@ -74,8 +83,10 @@ class moodle_group_external extends external_api {
         //      fetching by id is not enough!
 
         foreach ($params as $groupid) {
-            $groupid = clean_param($groupid, PARAM_INTEGER);
-            $group = groups_get_group($groupid, 'id, courseid, name, enrolmentkey', MUST_EXIST);
+            // validate params
+            $groupid = validate_param($groupid, PARAM_INTEGER);
+            $group = groups_get_group($groupid, 'id, courseid, name, description, enrolmentkey', MUST_EXIST);
+
             // now security checks
             $context = get_context_instance(CONTEXT_COURSE, $group->courseid);
             self::validate_context($context);
@@ -99,11 +110,13 @@ class moodle_group_external extends external_api {
         $groups = array();
 
         foreach ($params as $groupid) {
-            $groupid = clean_param($groupid, PARAM_INTEGER);
+            // validate params
+            $groupid = validate_param($groupid, PARAM_INTEGER);
             if (!$group = groups_get_group($groupid, 'id, courseid', IGNORE_MISSING)) {
                 // silently ignore attempts to delete nonexisting groups
                 continue;
             }
+
             // now security checks
             $context = get_context_instance(CONTEXT_COURSE, $group->courseid);
             self::validate_context($context);
@@ -123,7 +136,8 @@ class moodle_group_external extends external_api {
         $groups = array();
 
         foreach ($params as $groupid) {
-            $groupid = clean_param($groupid, PARAM_INTEGER);
+            // validate params
+            $groupid = validate_param($groupid, PARAM_INTEGER);
             $group = groups_get_group($groupid, 'id, courseid, name, enrolmentkey', MUST_EXIST);
             // now security checks
             $context = get_context_instance(CONTEXT_COURSE, $group->courseid);
@@ -151,10 +165,11 @@ class moodle_group_external extends external_api {
         $groups = array();
 
         foreach ($params as $member) {
-            $groupid = clean_param($member['groupid'], PARAM_INTEGER);
-            $userid = clean_param($member['userid'], PARAM_INTEGER);
+            // validate params
+            $groupid = validate_param($member['groupid'], PARAM_INTEGER);
+            $userid = validate_param($member['userid'], PARAM_INTEGER);
             $group = groups_get_group($groupid, 'id, courseid', MUST_EXIST);
-            $user = $DB->get_record('user', array('id'=>$userid, 'deleted'=>0, 'mnethostid'=>$CFG->mnet_localhost_id));
+            $user = $DB->get_record('user', array('id'=>$userid, 'deleted'=>0, 'mnethostid'=>$CFG->mnet_localhost_id), '*', MUST_EXIST);
 
             // now security checks
             $context = get_context_instance(CONTEXT_COURSE, $group->courseid);
@@ -179,10 +194,11 @@ class moodle_group_external extends external_api {
         $groups = array();
 
         foreach ($params as $member) {
-            $groupid = clean_param($member['groupid'], PARAM_INTEGER);
-            $userid = clean_param($member['userid'], PARAM_INTEGER);
-            $group = groups_get_group($groupid, 'id, courseid');
-            $user = $DB->get_record('user', array('id'=>$userid, 'deleted'=>0, 'mnethostid'=>$CFG->mnet_localhost_id));
+            // validate params
+            $groupid = validate_param($member['groupid'], PARAM_INTEGER);
+            $userid = validate_param($member['userid'], PARAM_INTEGER);
+            $group = groups_get_group($groupid, 'id, courseid', MUST_EXIST);
+            $user = $DB->get_record('user', array('id'=>$userid, 'deleted'=>0, 'mnethostid'=>$CFG->mnet_localhost_id), '*', MUST_EXIST);
 
             // now security checks
             $context = get_context_instance(CONTEXT_COURSE, $group->courseid);

diff --git a/lib/externallib.php b/lib/externallib.php
index b041805196917ea451089e0f4c1e2b4b8086d98d..9d086d710221d7aed3dfffeba96072c2de7f62f2 100644 (file)
--- a/lib/externallib.php
+++ b/lib/externallib.php
@@ -55,6 +55,9 @@ class external_api {
      * @return void
      */
     protected static function validate_context($context) {
+        if (empty($context)) {
+            throw new invalid_parameter_exception('Context does not exist');
+        }
         if (empty(self::$contextrestriction)) {
             self::$contextrestriction = get_context_instance(CONTEXT_SYSTEM);
         }