From 153ca2837412e918ab1fbce89509bb1fc262987f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20R=C3=BCegg?= Date: Tue, 19 Dec 2023 10:47:13 +0100 Subject: [PATCH] Fix non-unique key used for permission caching (#6753) * Fix non-unique key used for caching * Update CHANGELOG.md --- CHANGELOG.md | 1 + protected/humhub/libs/BasePermission.php | 32 ++++++++++++++--- .../user/components/PermissionManager.php | 35 +++++++++---------- 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c3af3781e..26b478ead1 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/protected/humhub/libs/BasePermission.php b/protected/humhub/libs/BasePermission.php index a887220d9b..f9ad7420e5 100644 --- a/protected/humhub/libs/BasePermission.php +++ b/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); } diff --git a/protected/humhub/modules/user/components/PermissionManager.php b/protected/humhub/modules/user/components/PermissionManager.php index e097709944..f39f5c916d 100644 --- a/protected/humhub/modules/user/components/PermissionManager.php +++ b/protected/humhub/modules/user/components/PermissionManager.php @@ -1,4 +1,5 @@ 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]); } - }