Fix non-unique key used for permission caching (#6753)

* Fix non-unique key used for caching

* Update CHANGELOG.md

CHANGELOG.md

@@ -3,6 +3,7 @@ HumHub Changelog
 
 1.15.2 (Unreleased)
 -------------------------
+- Fix #6753: Non-unique key used for permission caching
 - Fix #6741: Fix no pretty url of password recovery link
 - Enh #6734: Trim Base URL on save
 - Fix #6708: Fix displaying of alert when RichText is changed on refresh a page

protected/humhub/libs/BasePermission.php

@@ -9,6 +9,7 @@
 namespace humhub\libs;
 
 use humhub\modules\space\models\Space;
+use humhub\modules\user\components\PermissionManager;
 use humhub\modules\user\models\User;
 use Yii;
 use yii\base\BaseObject;
@@ -42,7 +43,7 @@ class BasePermission extends BaseObject
     /**
      * @var string title of the permission
      */
-    protected $title ='';
+    protected $title = '';
 
     /**
      * @var string description of the permission
@@ -144,6 +145,24 @@ class BasePermission extends BaseObject
         return $this->moduleId;
     }
 
+    /**
+     * This function must return a unique que.
+     * It must either start with the module id, or be a valid UUID.
+     * The function may return null to avoid caching in the PermissionManager.
+     *
+     * @param array $parameters Parameters passed to PermissionManager::can() which allows a permission to evaluate if caching should be disabled
+     *
+     * @return string|null A unique string (see method description) or null, if caching should be disabled
+     * @since 1.15.2
+     * @see PermissionManager::can()
+     * @see UUID::v4()
+     * @noinspection PhpUnusedParameterInspection
+     */
+    public function getCacheKey(array $parameters = []): ?string
+    {
+        return $this->getModuleId() . $this->getId();
+    }
+
     /**
      * Returns the default state of the permission.
      * The defaultState is either defined by setting $defaultState attribute
@@ -179,7 +198,7 @@ class BasePermission extends BaseObject
      */
     protected function getConfiguredState($groupId)
     {
-        if(!isset(Yii::$app->params['defaultPermissions'][static::class])) {
+        if (!isset(Yii::$app->params['defaultPermissions'][static::class])) {
             return null;
         }
 
@@ -188,8 +207,10 @@ class BasePermission extends BaseObject
         }
 
         // Allow asterisk to overwrite all groups excluding guest groups
-        if (isset(Yii::$app->params['defaultPermissions'][static::class]['*'])
-            && !in_array($groupId, [Space::USERGROUP_GUEST, User::USERGROUP_GUEST], true)) {
+        if (
+            isset(Yii::$app->params['defaultPermissions'][static::class]['*'])
+            && !in_array($groupId, [Space::USERGROUP_GUEST, User::USERGROUP_GUEST], true)
+        ) {
             return Yii::$app->params['defaultPermissions'][static::class]['*'];
         }
 
@@ -238,7 +259,8 @@ class BasePermission extends BaseObject
     /**
      * @param array Ids of additional fixed groups
      */
-    public function addFixedGroups($groupIds) {
+    public function addFixedGroups($groupIds)
+    {
         if (is_array($groupIds) && !empty($groupIds)) {
             $this->fixedGroups = array_merge($this->fixedGroups, $groupIds);
         }

protected/humhub/modules/user/components/PermissionManager.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * @link https://www.humhub.org/
  * @copyright Copyright (c) 2018 HumHub GmbH & Co. KG
@@ -24,7 +25,6 @@ use yii\db\ActiveRecord;
  */
 class PermissionManager extends Component
 {
-
     /**
      * User identity.
      * @var \humhub\modules\user\models\User
@@ -76,20 +76,18 @@ class PermissionManager extends Component
                     return false;
                 }
             }
+
             return $verifyAll;
-        } elseif ($allowCaching) {
-            $permission = ($permission instanceof BasePermission) ? $permission : Yii::createObject($permission);
-            $key = $permission->getId();
-
-            if (!isset($this->_access[$key])) {
-                $this->_access[$key] = $this->verify($permission);
-            }
-
-            return $this->_access[$key];
-        } else {
-            $permission = ($permission instanceof BasePermission) ? $permission : Yii::createObject($permission);
-            return $this->verify($permission);
         }
+
+        $permission = ($permission instanceof BasePermission) ? $permission : Yii::createObject($permission);
+
+        /** @var BasePermission $permission */
+        if ($allowCaching && $key = $permission->getCacheKey()) {
+            return $this->_access[$key] ??= $this->verify($permission);
+        }
+
+        return $this->verify($permission);
     }
 
     /**
@@ -283,8 +281,10 @@ class PermissionManager extends Component
 
         foreach ($this->_groupPermissions[$groupId] as $groupPermission) {
             /** @var $groupPermission GroupPermission */
-            if ($groupPermission->permission_id == $permission->getId()
-                && $groupPermission->module_id == $permission->getModuleId()) {
+            if (
+                $groupPermission->permission_id == $permission->getId()
+                && $groupPermission->module_id == $permission->getModuleId()
+            ) {
                 return $groupPermission;
             }
         }
@@ -422,7 +422,7 @@ class PermissionManager extends Component
      */
     protected function createPermissionRecord()
     {
-        return new GroupPermission;
+        return new GroupPermission();
     }
 
     /**
@@ -486,7 +486,7 @@ class PermissionManager extends Component
      */
     public static function findUsersByPermission($permission)
     {
-        $pm = new static;
+        $pm = new static();
 
         $allowedGroupIds = [];
         foreach (Group::find()->all() as $group) {
@@ -497,5 +497,4 @@ class PermissionManager extends Component
 
         return UserModel::find()->joinWith('groupUsers')->andWhere(['IN', 'group_user.group_id', $allowedGroupIds]);
     }
-
 }