MDL-12886 refactored function return full external function info and improved reporting of access control problems

lang/en_utf8/webservice.php

@@ -1,5 +1,6 @@
 <?php
 
+$string['accessexception'] = 'Access control exception';
 $string['addfunction'] = 'Add function';
 $string['addfunctionhelp'] = 'Select the function to add to the service.';
 $string['addrequiredcapability'] = 'Assign/Unassign the required capability';

lib/externallib.php

@@ -24,6 +24,74 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
+/**
+ * Returns detailed functio information
+ * @param string|object $function name of external function or record from external_function
+ * @param int $strictness IGNORE_MISSING means compatible mode, false returned if record not found, debug message if more found;
+ *                        MUST_EXIST means throw exception if no record or multiple records found
+ * @return object description or false if not found or exception thrown
+ */
+function external_function_info($function, $strictness=MUST_EXIST) {
+    global $DB, $CFG;
+
+    if (!is_object($function)) {
+        if (!$function = $DB->get_record('external_functions', array('name'=>$function), '*', $strictness)) {
+            return false;
+        }
+    }
+
+    //first find and include the ext implementation class
+    $function->classpath = empty($function->classpath) ? get_component_directory($function->component).'/externallib.php' : $CFG->dirroot.'/'.$function->classpath;
+    if (!file_exists($function->classpath)) {
+        throw new coding_exception('Can not find file with external function implementation');
+    }
+    require_once($function->classpath);
+
+    $function->parameters_method = $function->methodname.'_parameters';
+    $function->returns_method    = $function->methodname.'_returns';
+
+    // make sure the implementaion class is ok
+    if (!method_exists($function->classname, $function->methodname)) {
+        throw new coding_exception('Missing implementation method');
+    }
+    if (!method_exists($function->classname, $function->parameters_method)) {
+        throw new coding_exception('Missing parameters description');
+    }
+    if (!method_exists($function->classname, $function->returns_method)) {
+        throw new coding_exception('Missing returned values description');
+    }
+
+    // fetch the parameters description
+    $function->parameters_desc = call_user_func(array($function->classname, $function->parameters_method));
+    if (!($function->parameters_desc instanceof external_function_parameters)) {
+        throw new coding_exception('Invalid parameters description');
+    }
+
+    // fetch the return values description
+    $function->returns_desc = call_user_func(array($function->classname, $function->returns_method));
+    // null means void result or result is ignored
+    if (!is_null($function->returns_desc) and !($function->returns_desc instanceof external_description)) {
+        throw new coding_exception('Invalid return description');
+    }
+
+    //now get the function description
+    //TODO: use localised lang pack descriptions, it would be nice to have
+    //      easy to understand descriptiosn in admin UI,
+    //      on the other hand this is still a bit in a flux and we need to find some new naming
+    //      conventions for these descriptions in lang packs
+    $function->description = null;
+    $servicesfile = get_component_directory($function->component).'/db/services.php';
+    if (file_exists($servicesfile)) {
+        $functions = null;
+        include($servicesfile);
+        if (isset($functions[$function->name]['description'])) {
+            $function->description = $functions[$function->name]['description'];
+        }
+    }
+
+    return $function;
+}
+
 /**
  * Exception indicating user is not allowed to use external function in
  * the current context.

webservice/lib.php

@@ -25,6 +25,18 @@
 
 require_once($CFG->libdir.'/externallib.php');
 
+/**
+ * Exception indicating access control problem in web service call
+ */
+class webservice_access_exception extends moodle_exception {
+    /**
+     * Constructor
+     */
+    function __construct($debuginfo) {
+        parent::__construct('accessexception', 'exception', '', null, $debuginfo);
+    }
+}
+
 function webservice_protocol_is_enabled($protocol) {
     global $CFG;
 
@@ -118,10 +130,11 @@ abstract class webservice_zend_server implements webservice_server {
         // start the server
         $this->zend_server->setClass($this->service_class);
         $response = $this->zend_server->handle();
-
+/*
-        //$grrr = ob_get_clean();
+        $grrr = ob_get_clean();
-        //error_log($grrr);
+        error_log($grrr);
-
+        error_log($response);
+*/
         // session cleanup
         $this->session_cleanup();
 
@@ -204,6 +217,7 @@ abstract class webservice_zend_server implements webservice_server {
             $methods .= $this->get_virtual_method_code($function);
         }
 
+        // let's use unique class name, there might be problem in unit tests
         $classname = 'webservices_virtual_class_000000';
         while(class_exists($classname)) {
             $classname++;
@@ -230,39 +244,7 @@ class '.$classname.' {
     protected function get_virtual_method_code($function) {
         global $CFG;
 
-        //first find and include the ext implementation class
+        $function = external_function_info($function);
-        $function->classpath = empty($function->classpath) ? get_component_directory($function->component).'/externallib.php' : $CFG->dirroot.'/'.$function->classpath;
-        if (!file_exists($function->classpath)) {
-            throw new coding_exception('Can not find file with external function implementation');
-        }
-        require_once($function->classpath);
-
-        $function->parameters_method = $function->methodname.'_parameters';
-        $function->returns_method    = $function->methodname.'_returns';
-
-        // make sure the implementaion class is ok
-        if (!method_exists($function->classname, $function->methodname)) {
-            throw new coding_exception('Missing implementation method');
-        }
-        if (!method_exists($function->classname, $function->parameters_method)) {
-            throw new coding_exception('Missing parameters description');
-        }
-        if (!method_exists($function->classname, $function->returns_method)) {
-            throw new coding_exception('Missing returned values description');
-        }
-
-        // fetch the parameters description
-        $function->parameters_desc = call_user_func(array($function->classname, $function->parameters_method));
-        if (!($function->parameters_desc instanceof external_function_parameters)) {
-            throw new coding_exception('Invalid parameters description');
-        }
-
-        // fetch the return values description
-        $function->returns_desc = call_user_func(array($function->classname, $function->returns_method));
-        // null means void result or result is ignored
-        if (!is_null($function->returns_desc) and !($function->returns_desc instanceof external_description)) {
-            throw new coding_exception('Invalid return description');
-        }
 
         $params      = array();
         $params_desc = array();
@@ -316,8 +298,8 @@ class '.$classname.' {
 
         $code = '
     /**
-     * External function: '.$function->name.'
+     * '.$function->description.'
-     * TODO: add function description
+     *
 '.$params_desc.'
 '.$return.'
      */
@@ -370,27 +352,25 @@ class '.$classname.' {
             $this->restricted_context = get_context_instance(CONTEXT_SYSTEM);
 
             if (!is_enabled_auth('webservice')) {
-                error_log('WS auth not enabled');
+                throw new webservice_access_exception('WS auth not enabled');
-                die('WS auth not enabled');
             }
 
             if (!$auth = get_auth_plugin('webservice')) {
-                error_log('WS auth missing');
+                throw new webservice_access_exception('WS auth missing');
-                die('WS auth missing');
             }
 
             // the username is hardcoded as URL parameter because we can not easily parse the request data :-(
             if (!$username = optional_param('wsusername', '', PARAM_RAW)) {
-                throw new invalid_parameter_exception('Missing username');
+                throw new webservice_access_exception('Missing username');
             }
 
             // the password is hardcoded as URL parameter because we can not easily parse the request data :-(
             if (!$password = optional_param('wspassword', '', PARAM_RAW)) {
-                throw new invalid_parameter_exception('Missing password');
+                throw new webservice_access_exception('Missing password');
             }
 
             if (!$auth->user_login_webservice($username, $password)) {
-                throw new invalid_parameter_exception('Wrong username or password');
+                throw new webservice_access_exception('Wrong username or password');
             }
 
             $user = $DB->get_record('user', array('username'=>$username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0), '*', MUST_EXIST);
@@ -406,7 +386,7 @@ class '.$classname.' {
         }
 
         if (!has_capability("webservice/$this->wsname:use", $this->restricted_context)) {
-            throw new invalid_parameter_exception('Access to web service not allowed');
+            throw new webservice_access_exception('Access to web service not allowed');
         }
 
         external_api::set_context_restriction($this->restricted_context);
@@ -630,23 +610,23 @@ abstract class webservice_base_server implements webservice_server {
             $this->restricted_context = get_context_instance(CONTEXT_SYSTEM);
 
             if (!is_enabled_auth('webservice')) {
-                die('WS auth not enabled');
+                throw new webservice_access_exception('WS auth not enabled');
             }
 
             if (!$auth = get_auth_plugin('webservice')) {
-                die('WS auth missing');
+                throw new webservice_access_exception('WS auth missing');
             }
 
             if (!$this->username) {
-                throw new invalid_parameter_exception('Missing username');
+                throw new webservice_access_exception('Missing username');
             }
 
             if (!$this->password) {
-                throw new invalid_parameter_exception('Missing password');
+                throw new webservice_access_exception('Missing password');
             }
 
             if (!$auth->user_login_webservice($this->username, $this->password)) {
-                throw new invalid_parameter_exception('Wrong username or password');
+                throw new webservice_access_exception('Wrong username or password');
             }
 
             $user = $DB->get_record('user', array('username'=>$this->username, 'mnethostid'=>$CFG->mnet_localhost_id, 'deleted'=>0), '*', MUST_EXIST);
@@ -661,7 +641,7 @@ abstract class webservice_base_server implements webservice_server {
         }
 
         if (!has_capability("webservice/$this->wsname:use", $this->restricted_context)) {
-            throw new invalid_parameter_exception('Access to web service not allowed');
+            throw new webservice_access_exception('Access to web service not allowed');
         }
 
         external_api::set_context_restriction($this->restricted_context);
@@ -681,8 +661,7 @@ abstract class webservice_base_server implements webservice_server {
         }
 
         // function must exist
-        $function = $DB->get_record('external_functions', array('name'=>$this->functionname), '*', MUST_EXIST);
+        $function = external_function_info($this->functionname);
-
 
         // now let's verify access control
         if ($this->simple) {
@@ -730,46 +709,6 @@ abstract class webservice_base_server implements webservice_server {
         if (!$allowed) {
             throw new invalid_parameter_exception('Access to external function not allowed');
         }
-        // now we finally know the user may execute this function,
-        // the last step is to set context restriction - in this simple case
-        // we use system context because each external system has different user account
-        // and we can manage everything through normal permissions.
-
-        // get the params and return descriptions of the function
-        unset($function->id); // we want to prevent any accidental db updates ;-)
-
-        $function->classpath = empty($function->classpath) ? get_component_directory($function->component).'/externallib.php' : $CFG->dirroot.'/'.$function->classpath;
-        if (!file_exists($function->classpath)) {
-            throw new coding_exception('Can not find file with external function implementation');
-        }
-        require_once($function->classpath);
-
-        $function->parameters_method = $function->methodname.'_parameters';
-        $function->returns_method    = $function->methodname.'_returns';
-
-        // make sure the implementaion class is ok
-        if (!method_exists($function->classname, $function->methodname)) {
-            throw new coding_exception('Missing implementation method');
-        }
-        if (!method_exists($function->classname, $function->parameters_method)) {
-            throw new coding_exception('Missing parameters description');
-        }
-        if (!method_exists($function->classname, $function->returns_method)) {
-            throw new coding_exception('Missing returned values description');
-        }
-
-        // fetch the parameters description
-        $function->parameters_desc = call_user_func(array($function->classname, $function->parameters_method));
-        if (!($function->parameters_desc instanceof external_function_parameters)) {
-            throw new coding_exception('Invalid parameters description');
-        }
-
-        // fetch the return values description
-        $function->returns_desc = call_user_func(array($function->classname, $function->returns_method));
-        // null means void result or result is ignored
-        if (!is_null($function->returns_desc) and !($function->returns_desc instanceof external_description)) {
-            throw new coding_exception('Invalid return description');
-        }
 
         // we have all we need now
         $this->function = $function;

webservice/xmlrpc/locallib.php

@@ -37,7 +37,5 @@ class webservice_xmlrpc_server extends webservice_zend_server {
         parent::__construct('Zend_XmlRpc_Server');
         $this->wsname = 'xmlrpc';
     }
-
-
 }
 

webservice/xmlrpc/simpleserver.php

@@ -33,12 +33,8 @@ require_once("$CFG->dirroot/webservice/xmlrpc/locallib.php");
 //TODO: for now disable all mess in xml
 ini_set('display_errors', '0');
 ini_set('log_errors', '1');
-$CFG->debug = 0;
 $CFG->debugdisplay = false;
 
-//error_log('yy');
-//error_log(var_export($_SERVER, true));
-
 if (!webservice_protocol_is_enabled('xmlrpc')) {
     die;
 }