fix(webservice) Do not let webservice request set disabled attributes

It seems deserializeValue method in Attribute was actually unused,
 setValue being called directly by simplePlugin.
Now it’s used and does about the same thing as setValue, just returns an
 error if the attribute is disabled.

issue #5811

include/simpleplugin/attributes/class_CompositeAttribute.inc

@@ -261,17 +261,6 @@ class CompositeAttribute extends Attribute
     }
   }
 
-  function deserializeValue($values)
-  {
-    if ($this->visible) {
-      foreach ($this->attributes as &$attribute) {
-        $attribute->setDisabled($this->disabled);
-        $attribute->deserializeValue($values);
-      }
-      unset($attribute);
-    }
-  }
-
   function renderFormInput()
   {
     $display = "";

include/simpleplugin/attributes/class_FileAttribute.inc

@@ -94,16 +94,17 @@ class FileAttribute extends Attribute
 
   /*! \brief Apply value from RPC requests
    *
-   *  \param array $values the values array
+   *  \param mixed $value the value
    */
-  function deserializeValue($values)
+  function deserializeValue($value)
   {
-    if (isset($values[$this->getLdapName()])) {
-      if ($this->binary) {
-        $this->setValue(base64_decode($values[$this->getLdapName()]));
-      } else {
-        $this->setValue($values[$this->getLdapName()]);
-      }
+    if ($this->disabled) {
+      return sprintf(_('Attribute %s is disabled, its value could not be set'), $this->getLdapName());
+    }
+    if ($this->binary) {
+      $this->setValue(base64_decode($value));
+    } else {
+      $this->setValue($value);
     }
   }
 }

include/simpleplugin/class_Attribute.inc

@@ -635,11 +635,12 @@ class Attribute
    *
    *  \param array $values the values array
    */
-  function deserializeValue($values)
+  function deserializeValue($value)
   {
-    if (isset($values[$this->getLdapName()])) {
-      $this->setValue($values[$this->getLdapName()]);
+    if ($this->disabled) {
+      return sprintf(_('Attribute %s is disabled, its value could not be set'), $this->getLdapName());
     }
+    $this->setValue($values[$this->getLdapName()]);
   }
 
   /*! \brief Add ACL information around display

include/simpleplugin/class_simplePlugin.inc

@@ -1914,7 +1914,10 @@ class simplePlugin
     foreach ($values as $name => $value) {
       if (isset($this->attributesAccess[$name])) {
         if (!$checkAcl || $this->attrIsWriteable($name)) {
-          $this->attributesAccess[$name]->setValue($value);
+          $error = $this->attributesAccess[$name]->deserializeValue($value);
+          if (!empty($error)) {
+            return $error;
+          }
         } else {
           return msgPool::permModify($this->dn, $name);
         }

[bmortier]

Remove the unused function parameter "$value".

By Ghost User on 2018-04-17T13:51:55 (imported from GitLab)

[bmortier]

Review the data-flow - use of uninitialized value.

By Ghost User on 2018-04-17T13:51:55 (imported from GitLab)

[bmortier]

SonarQube analysis reported 3 issues

• 3 major

Watch the comments in this conversation to review them.

1 extra issue

Note: The following issues were found on lines that were not modified in the commit. Because these issues can't be reported as line comments, they are summarized here:

1. Reduce the number of returns of this function 4, down to the maximum allowed 3.

By Ghost User on 2018-04-17T13:51:56 (imported from GitLab)