From 7fca6424abdccd910dbdd35fc1a477db504f4f9a Mon Sep 17 00:00:00 2001 From: jf-cbd <121934370+jf-cbd@users.noreply.github.com> Date: Thu, 25 Jun 2026 17:45:43 +0200 Subject: [PATCH] =?UTF-8?q?=20N=C2=B09723=20-=20IsActionAllowed=20should?= =?UTF-8?q?=20work=20with=20non=20instantiated=20users=20objects=20(#947)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Replace references to user objects keys since objects can be non-instantiated (so no key available) and replace them by equivalent code * Rename test + use self variable for profile --------- Co-authored-by: jf-cbd --- .../userrightsprofile.class.inc.php | 82 ++++++++++++------- .../unitary-tests/core/UserRightsTest.php | 11 +++ 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/addons/userrights/userrightsprofile.class.inc.php b/addons/userrights/userrightsprofile.class.inc.php index 5952ece3e5..1fc947be49 100644 --- a/addons/userrights/userrightsprofile.class.inc.php +++ b/addons/userrights/userrightsprofile.class.inc.php @@ -582,16 +582,26 @@ class UserRightsProfile extends UserRightsAddOnAPI */ public function ListProfiles($oUser) { - $aRet = []; - $oSearch = new DBObjectSearch('URP_UserProfile'); - $oSearch->AllowAllData(); - $oSearch->NoContextParameters(); - $oSearch->Addcondition('userid', $oUser->GetKey(), '='); - $oProfiles = new DBObjectSet($oSearch); - while ($oUserProfile = $oProfiles->Fetch()) { - $aRet[$oUserProfile->Get('profileid')] = $oUserProfile->Get('profileid_friendlyname'); + if (count($oUser->ListChanges()) === 0) { // backward compatibility + $aRet = []; + $oSearch = new DBObjectSearch('URP_UserProfile'); + $oSearch->AllowAllData(); + $oSearch->NoContextParameters(); + $oSearch->Addcondition('userid', $oUser->GetKey(), '='); + $oProfiles = new DBObjectSet($oSearch); + while ($oUserProfile = $oProfiles->Fetch()) { + $aRet[$oUserProfile->Get('profileid')] = $oUserProfile->Get('profileid_friendlyname'); + } + + return $aRet; + } else { + $aRet = []; + $oProfilesSet = $oUser->Get('profile_list'); + foreach ($oProfilesSet as $oUserProfile) { + $aRet[$oUserProfile->Get('profileid')] = $oUserProfile->Get('profileid_friendlyname'); + } + return $aRet; } - return $aRet; } public function GetSelectFilter($oUser, $sClass, $aSettings = []) @@ -705,26 +715,23 @@ class UserRightsProfile extends UserRightsAddOnAPI protected function GetUserActionGrant($oUser, $sClass, $iActionCode) { $this->LoadCache(); - - // load and cache permissions for the current user on the given class - // - $iUser = $oUser->GetKey(); - if (isset($this->m_aObjectActionGrants[$iUser][$sClass][$iActionCode])) { - $aTest = $this->m_aObjectActionGrants[$iUser][$sClass][$iActionCode]; - if (is_array($aTest)) { - return $aTest; + if (count($oUser->ListChanges()) === 0) { + // load and cache permissions for the current user on the given class + if (isset($this->m_aObjectActionGrants[$oUser->GetKey()][$sClass][$iActionCode])) { + $aTest = $this->m_aObjectActionGrants[$oUser->GetKey()][$sClass][$iActionCode]; + if (is_array($aTest)) { + return $aTest; + } } } $sAction = self::$m_aActionCodes[$iActionCode]; $bStatus = null; - // Cache user's profiles - if (false === array_key_exists($iUser, $this->aUsersProfilesList)) { - $this->aUsersProfilesList[$iUser] = UserRights::ListProfiles($oUser); - } + + $aProfileList = $this->GetProfileList($oUser); // Call the API of UserRights because it caches the list for us - foreach ($this->aUsersProfilesList[$iUser] as $iProfile => $oProfile) { + foreach ($aProfileList as $iProfile => $oProfile) { $bGrant = $this->GetProfileActionGrant($iProfile, $sClass, $sAction); if (!is_null($bGrant)) { if ($bGrant) { @@ -742,7 +749,9 @@ class UserRightsProfile extends UserRightsAddOnAPI $aRes = [ 'permission' => $iPermission, ]; - $this->m_aObjectActionGrants[$iUser][$sClass][$iActionCode] = $aRes; + if (count($oUser->ListChanges()) === 0) { + $this->m_aObjectActionGrants[$oUser->GetKey()][$sClass][$iActionCode] = $aRes; + } return $aRes; } @@ -824,18 +833,14 @@ class UserRightsProfile extends UserRightsAddOnAPI { $this->LoadCache(); // Note: this code is VERY close to the code of IsActionAllowed() - $iUser = $oUser->GetKey(); - // Cache user's profiles - if (false === array_key_exists($iUser, $this->aUsersProfilesList)) { - $this->aUsersProfilesList[$iUser] = UserRights::ListProfiles($oUser); - } + $aProfileList = $this->GetProfileList($oUser); // Note: The object set is ignored because it was interesting to optimize for huge data sets // and acceptable to consider only the root class of the object set $bStatus = null; // Call the API of UserRights because it caches the list for us - foreach ($this->aUsersProfilesList[$iUser] as $iProfile => $oProfile) { + foreach ($aProfileList as $iProfile => $oProfile) { $bGrant = $this->GetClassStimulusGrant($iProfile, $sClass, $sStimulusCode); if (!is_null($bGrant)) { if ($bGrant) { @@ -893,6 +898,25 @@ class UserRightsProfile extends UserRightsAddOnAPI } return $bHasSharing; } + + /** + * @param \User $oUser + * + * @return array + * @throws \Exception + */ + public function GetProfileList(User $oUser): array + { + if (count($oUser->ListChanges()) === 0) { // if user is already in db and not changed + $iUser = $oUser->GetKey(); + if (false === array_key_exists($iUser, $this->aUsersProfilesList)) { + $aProfiles = UserRights::ListProfiles($oUser); + $this->aUsersProfilesList[$iUser] = $aProfiles; + } + return $this->aUsersProfilesList[$iUser]; + } + return UserRights::ListProfiles($oUser); + } } UserRights::SelectModule('UserRightsProfile'); diff --git a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php index 5c286b91a5..d84c2e9621 100644 --- a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php +++ b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php @@ -81,6 +81,17 @@ class UserRightsTest extends ItopDataTestCase return $oUser; } + public function testIsActionAllowedWithNonInstantiatedUserObject() + { + $oUser = $this->GivenUserWithProfiles('test1', [self::$aURP_Profiles['Configuration Manager']]); // not a readonly profile + $oAdminUser = $this->GivenUserWithProfiles('test2', [self::$aURP_Profiles['Administrator']]); + $oAdminUser->DBInsert(); + $_SESSION = []; + UserRights::Login($oAdminUser->Get('login')); + + self::assertTrue(UserRights::IsActionAllowed('Server', UR_ACTION_MODIFY, null, $oUser) === UR_ALLOWED_YES); + } + /** * @param array $aProfileIds * @param array $aShouldBeAllowedToSeeClass