Fixed issue #17075: User roles : all rights are enabled

Merge pull request #1755 from Shnoulle/master_17075

application/models/Permission.php

@@ -524,7 +524,7 @@ public function copySurveyPermissions($iSurveyIDSource, $iSurveyIDTarget)
      * @param string $sEntityName string The entity name
      * @param $sPermission string Name of the permission
      * @param $sCRUD string The permission detail you want to check on: 'create','read','update','delete','import' or 'export'
-     * @param $iUserID integer User ID - if not given the one of the current user is used, 0 is used for role permission (not related to user)
+     * @param $iUserID integer User ID - if empty : use the current user
      * @return bool True if user has the permission
      */
     public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD = 'read', $iUserID = null)
@@ -555,7 +555,7 @@ public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD = '
 
         /* Always return true for CConsoleApplication (before or after plugin ? All other seems better after plugin) */
         // TODO: see above about entry script and superadmin
-        if (is_null($iUserID) && Yii::app() instanceof CConsoleApplication) {
+        if (empty($iUserID) && Yii::app() instanceof CConsoleApplication) {
             return true;
         }
 
@@ -566,15 +566,14 @@ public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD = '
         }
         $sCRUD = $sCRUD . '_p';
 
-        /* Always return false for guests */
-        // TODO: should not be necessary
+        /* Be sure to have an user id */
         $iUserID = $this->getUserId($iUserID);
-        if (!$iUserID && $iUserID !== 0) {
+        /* Always return false for guests */
+        if (empty($iUserID)) {
             return false;
         }
 
-        /* Always return true if you are the owner : this can be done in core plugin ? */
-        // TODO: give the rights to owner adding line in permissions table, so it will return true with the normal way
+        /* Always return true if user are the owner of entity*/
         if ($iUserID == $this->getEntityOwnerId($iEntityID, $sEntityName)) {
             return true;
         }
@@ -611,7 +610,7 @@ public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD = '
             if (safecount($aRoles) > 0) {
                 $allowed = false;
                 foreach ($aRoles as $role) {
-                    $allowed = $allowed || $this->hasRolePermission($role['ptid'], $sPermission, substr($sCRUD, 0, -2));
+                    $allowed = $allowed || $this->roleHasPermission($role['ptid'], $sPermission, substr($sCRUD, 0, -2));
                 }
                 /* Can return false ? Even if user have the specific right … */
                 return $allowed;
@@ -677,16 +676,26 @@ public function hasSurveyPermission($iSurveyID, $sPermission, $sCRUD = 'read', $
     }
 
     /**
-     * Returns true if a role has permission to read/create/update a certain template
-     * @param string $roleId
-     * @param $sCRUD string The permission detailsyou want to check on: 'create','read','update','delete','import' or 'export'
-     * @param integer $iUserID integer User ID - if not given the one of the current user is used
-     * @return bool True if user has the permission
+     * Returns true if a role has permission for crud
+     * @param integer $roleId
+     * @param string $sPermission 
+     * @param string $sCRUD The permission detailsyou want to check on: 'create','read','update','delete','import' or 'export'
+     * @return bool allowed permssion
      */
-    public function hasRolePermission($iRoleId, $sPermission, $sCRUD = 'read')
+    public function roleHasPermission($iRoleId, $sPermission, $sCRUD = 'read')
     {
-        /* Get permission foor user id = 0 */
-        return $this->hasPermission($iRoleId, 'role', $sPermission, $sCRUD, 0);
+        if (!in_array($sCRUD, array('create', 'read', 'update', 'delete', 'import', 'export'))) {
+            return false;
+        }
+        $rolePermission = $this->findByAttributes(array(
+            "entity_id" => $iRoleId,
+            "entity" => "role",
+            "permission" => $sPermission
+        ));
+        if (empty($rolePermission)) {
+            return false;
+        }
+        return (bool) $rolePermission->getAttribute("{$sCRUD}_p");
     }
 
     /**
@@ -714,14 +723,15 @@ private static function comparePermissionTitle($aApermission, $aBpermission)
 
     /**
      * get the default/fixed $iUserID
-     * @param integer $iUserID optional user id
+     * @param integer|null $iUserID optional user id
      * @return int user id
      * @throws Exception
      */
     public function getUserId($iUserID = null)
     {
-        if (is_null($iUserID)) {
+        if (empty($iUserID)) {
             if (Yii::app() instanceof CConsoleApplication) {
+                /* Alt : return 1st forcedAdmin ? */
                 throw new Exception('Permission must not be tested with console application.');
             }
             /* See TestBaseClass tearDownAfterClass */

application/models/Permissiontemplates.php

@@ -291,9 +291,16 @@ public function removeCdataFormat($node)
         return trim(preg_replace($regex_patterns, $regex_replace, $nodeText));
     }
 
+    /**
+     * return if this role GIVE a permission
+     * Used in self::compileExportXML only
+     * @param string $sPermission 
+     * @param string $sCRUD
+     * @return boolean
+     */
     public function getHasPermission($sPermission, $sCRUD)
     {
-        return Permission::model()->hasRolePermission($this->ptid, $sPermission, $sCRUD);
+        return Permission::model()->roleHasPermission($this->ptid, $sPermission, $sCRUD);
     }
 
     /**

application/views/admin/permissiontemplates/partials/_permissions.php

@@ -57,7 +57,7 @@
 
                                         <!-- checkbox -->
                                         <input type="checkbox"  class="specific-permission-selector" name='Permission[<?php echo $sPermissionKey.']['.$sCRUDKey;?>]' id='perm_<?php echo $sPermissionKey.'_'.$sCRUDKey;?>'
-                                            <?php if(Permission::model()->hasRolePermission( $oModel->ptid, $sPermissionKey, $sCRUDKey)):?>
+                                            <?php if(Permission::model()->roleHasPermission($oModel->ptid, $sPermissionKey, $sCRUDKey)):?>
                                                 checked="checked"
                                             <?php endif; ?>
                                             <?php if(substr($sPermissionKey,0,5) === 'auth_' && $sCRUDKey === 'read'): ?>