Fixes #5152 Forbid modification of groups we have no rights on

include/class_msgPool.inc

@@ -137,23 +137,28 @@ class msgPool {
   /*!
    * \brief Display that we have no permission to modify an object
    *
-   * \param string $name Name of the object which will be modified
+   * \param string $name Name of the object which cannot be modified (or array of objects names)
+   * \param string $fields Name of the field of the object which cannot be modified
    */
-  public static function permModify($name = "")
+  public static function permModify($name = '', $field = '')
   {
-    if ($name == "") {
-      return _("You have no permission to modify this object!");
+    if ($name == '') {
+      return _('You have no permission to modify this object!');
     }
 
     if (!is_array($name)) {
-      return _("You have no permission to modify the object:")."<br><br><i>$name</i>";
+      if ($field != '') {
+        return sprintf(_('You have no permission to modify the field "%s" of object "%s"'), $field, $name);
+      } else {
+        return sprintf(_('You have no permission to modify the object:<br/>%s'), '<br/><i>'.$name.'</i>');
+      }
     }
 
     if (count($name) == 1) {
-      return _("You have no permission to modify the object:")."<br>".msgPool::buildList($name);
+      return sprintf(_('You have no permission to modify the object:<br/>%s'), msgPool::buildList($name));
     }
 
-    return _("You have no permission to modify these objects:")."<br>".msgPool::buildList($name);
+    return sprintf(_('You have no permission to modify these objects:<br/>%s'), msgPool::buildList($name));
   }
 
   /*!

include/simpleplugin/class_simplePlugin.inc

@@ -432,8 +432,15 @@ class simplePlugin extends plugin
     return $this->header.$smarty->fetch($this->templatePath);
   }
 
-  function attr_is_writeable($attr)
+  /*! \brief Check if logged in user have enough right to write this attribute value
+   *
+   * \param mixed $attr Attribute object or name (in this case it will be fetched from attributesAccess)
+   */
+  function attrIsWriteable($attr)
   {
+    if (!is_object($attr)) {
+      $attr = $this->attributesAccess[$attr];
+    }
     if ($attr->getLdapName() == 'base') {
       if (!$this->acl_skip_write() && (!$this->initially_was_account || $this->acl_is_moveable() || $this->acl_is_removeable())) {
         return TRUE;
@@ -455,7 +462,7 @@ class simplePlugin extends plugin
 
     /* Handle rights to modify the base */
     if (isset($this->attributesAccess['base'])) {
-      if ($this->attr_is_writeable($this->attributesAccess['base'])) {
+      if ($this->attrIsWriteable('base')) {
         $smarty->assign('baseACL', 'rw');
       } else {
         $smarty->assign('baseACL', 'r');
@@ -612,7 +619,7 @@ class simplePlugin extends plugin
       // A first pass that loads the post values
       foreach ($this->attributesInfo as $section => &$sectionInfo) {
         foreach ($sectionInfo['attrs'] as &$attr) {
-          if ($this->attr_is_writeable($attr)) {
+          if ($this->attrIsWriteable($attr)) {
             // Each attribute know how to read its value from POST
             $attr->loadPostValue();
           }
@@ -623,7 +630,7 @@ class simplePlugin extends plugin
       // A second one that applies them. That allow complex stuff such as attribute disabling
       foreach ($this->attributesInfo as $section => &$sectionInfo) {
         foreach ($sectionInfo['attrs'] as &$attr) {
-          if ($this->attr_is_writeable($attr)) {
+          if ($this->attrIsWriteable($attr)) {
             // Each attribute know how to read its value from POST
             $attr->applyPostValue();
           }
@@ -1025,7 +1032,7 @@ class simplePlugin extends plugin
         if (!$checkAcl || $this->acl_is_writeable($this->attributesAccess[$name]->getAcl())) {
           $this->attributesAccess[$name]->setValue($value);
         } else {
-          return sprintf(_('You don\'t have sufficient rights to edit field "%s"'), $name);
+          return msgPool::permModify($this->dn, $name);
         }
       } else {
         return sprintf(_('Unknown field "%s"'), $name);

plugins/personal/roles/class_userRoles.inc

@@ -76,9 +76,9 @@ class userRoles extends simplePlugin
     /* Groups handling */
     $groups      = array();
     $groupsattrs = objects::ls('ogroup', array('cn' => 1,  'description' => 1));
-    foreach($groupsattrs as $dn => $groupattr) {
+    foreach ($groupsattrs as $dn => $groupattr) {
       $groupDisplay = $groupattr['cn'];
-      if(isset($groupattr['description'])) {
+      if (isset($groupattr['description'])) {
         if (strlen($groupattr['description']) > 50) {
           $groupattr['description'] = substr($groupattr['description'], 0, 50).'…';
         }
@@ -105,9 +105,9 @@ class userRoles extends simplePlugin
     /* Roles handling */
     $roles        = array();
     $rolesattrs  = objects::ls('role', array('cn' => 1,  'description' => 1));
-    foreach($rolesattrs as $dn => $roleattr) {
+    foreach ($rolesattrs as $dn => $roleattr) {
       $roleDisplay = $roleattr['cn'];
-      if(isset($roleattr['description'])) {
+      if (isset($roleattr['description'])) {
         if (strlen($roleattr['description']) > 50) {
           $roleattr['description'] = substr($roleattr['description'], 0, 50).'…';
         }
@@ -177,8 +177,8 @@ class userRoles extends simplePlugin
   {
     parent::prepare_save();
     if ($this->is_template) {
-      $this->attrs['userGroups'] = $this->groupsMembership;
-      $this->attrs['userRoles'] = $this->rolesMembership;
+      $this->attrs['userGroups']  = $this->groupsMembership;
+      $this->attrs['userRoles']   = $this->rolesMembership;
     }
   }
 
@@ -194,11 +194,17 @@ class userRoles extends simplePlugin
         'dn'          => $this->dn
       );
 
+      $errors = array();
+
       /* Take care about groupsMembership values: add to groups */
       $groupsMembership = $this->attributesAccess['groupsMembership']->getValue();
       foreach ($groupsMembership as $ogroupdn) {
         if (!in_array($ogroupdn, $this->savedGroupsMembership)) {
           $g = objects::open($ogroupdn, 'ogroup');
+          if (!$g->getBaseObject()->attrIsWriteable('member')) {
+            $errors[] = msgPool::permModify($ogroupdn, 'member');
+            continue;
+          }
           $g->getBaseObject()->attributesAccess['member']->addValue($this->dn, $fake_attrs);
           $g->save();
         }
@@ -208,6 +214,10 @@ class userRoles extends simplePlugin
       foreach ($this->savedGroupsMembership as $ogroupdn) {
         if (!in_array($ogroupdn, $groupsMembership)) {
           $g = objects::open($ogroupdn, 'ogroup');
+          if (!$g->getBaseObject()->attrIsWriteable('member')) {
+            $errors[] = msgPool::permModify($ogroupdn, 'member');
+            continue;
+          }
           $g->getBaseObject()->attributesAccess['member']->searchAndRemove($this->dn);
           $g->save();
         }
@@ -218,6 +228,10 @@ class userRoles extends simplePlugin
       foreach ($rolesMembership as $roledn) {
         if (!in_array($roledn, $this->savedRolesMembership)) {
           $r = objects::open($roledn, 'role');
+          if (!$r->getBaseObject()->attrIsWriteable('roleOccupant')) {
+            $errors[] = msgPool::permModify($roledn, 'roleOccupant');
+            continue;
+          }
           $r->getBaseObject()->attributesAccess['roleOccupant']->addValue($this->dn, $fake_attrs);
           $r->save();
         }
@@ -227,10 +241,17 @@ class userRoles extends simplePlugin
       foreach ($this->savedRolesMembership as $roledn) {
         if (!in_array($roledn, $rolesMembership)) {
           $r = objects::open($roledn, 'role');
+          if (!$r->getBaseObject()->attrIsWriteable('roleOccupant')) {
+            $errors[] = msgPool::permModify($roledn, 'roleOccupant');
+            continue;
+          }
           $r->getBaseObject()->attributesAccess['roleOccupant']->searchAndRemove($this->dn);
           $r->save();
         }
       }
+
+      /* Display errors if any */
+      msg_dialog::displayChecks($errors);
     }
   }