From 56d3ac668cd44d6cdde31aa9c25f541649bb3d7e Mon Sep 17 00:00:00 2001 From: v-dumas Date: Tue, 17 Dec 2024 16:31:59 +0100 Subject: [PATCH 1/2] =?UTF-8?q?N=C2=B08047=20-=20SuperUser=20profile=20mov?= =?UTF-8?q?ed=20under=20iTop=20Community=203.2.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../datamodel.itop-oauth-client.xml | 17 ++++ .../datamodel.itop-profiles-itil.xml | 95 +++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/datamodels/2.x/itop-oauth-client/datamodel.itop-oauth-client.xml b/datamodels/2.x/itop-oauth-client/datamodel.itop-oauth-client.xml index f2321aa2e..ddf664aba 100644 --- a/datamodels/2.x/itop-oauth-client/datamodel.itop-oauth-client.xml +++ b/datamodels/2.x/itop-oauth-client/datamodel.itop-oauth-client.xml @@ -952,5 +952,22 @@ HTML + + + + + + + allow + allow + allow + allow + allow + allow + + + + + diff --git a/datamodels/2.x/itop-profiles-itil/datamodel.itop-profiles-itil.xml b/datamodels/2.x/itop-profiles-itil/datamodel.itop-profiles-itil.xml index 47e7a47bb..7194df204 100755 --- a/datamodels/2.x/itop-profiles-itil/datamodel.itop-profiles-itil.xml +++ b/datamodels/2.x/itop-profiles-itil/datamodel.itop-profiles-itil.xml @@ -183,8 +183,103 @@ + + + + + + + + + + + + + + SuperUser + This profil allows all actions which are not Administrator restricted. + + + + allow + allow + allow + allow + allow + allow + + + + + allow + allow + + + + + allow + allow + + + + + allow + + + + + allow + allow + allow + allow + allow + allow + + + + + allow + allow + allow + allow + allow + allow + allow + allow + allow + + + + + allow + allow + allow + allow + allow + + + + + allow + allow + allow + allow + allow + allow + + + + + allow + allow + allow + allow + + + + Configuration Manager Person in charge of the documentation of the managed CIs From 5f8575763009410fee10831afd20463206ae99e8 Mon Sep 17 00:00:00 2001 From: odain-cbd <56586767+odain-cbd@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:27:49 +0100 Subject: [PATCH 2/2] =?UTF-8?q?N=C2=B07633=20-=20Reloads=20the=20same=20us?= =?UTF-8?q?er=20multiple=20times=20if=20it=20no=20longer=20exists=20(#692)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * N°7633 - Reloads the same user multiple times if it no longer exists * Update core/userrights.class.inc.php good catch Co-authored-by: Thomas Casteleyn * Update core/userrights.class.inc.php good catch (again) Co-authored-by: Thomas Casteleyn * Update core/userrights.class.inc.php Co-authored-by: Thomas Casteleyn * PR feedbacks from Romain * ci: rename user logins --------- Co-authored-by: Thomas Casteleyn --- core/userrights.class.inc.php | 75 +++++++++---------- .../unitary-tests/core/UserRightsTest.php | 51 +++++++++++++ 2 files changed, 86 insertions(+), 40 deletions(-) diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php index 3c4ad349a..9152efca5 100644 --- a/core/userrights.class.inc.php +++ b/core/userrights.class.inc.php @@ -1921,50 +1921,45 @@ class UserRights */ protected static function FindUser($sLogin, $sAuthentication = 'any', $bAllowDisabledUsers = false) { - if ($sAuthentication == 'any') - { - $oUser = self::FindUser($sLogin, 'internal'); - if ($oUser == null) - { - $oUser = self::FindUser($sLogin, 'external'); + if ($sAuthentication === 'any') { + $oUser = self::FindUser($sLogin, 'internal', $bAllowDisabledUsers); + if ($oUser !== null) { + return $oUser; } + + return self::FindUser($sLogin, 'external', $bAllowDisabledUsers); } - else - { - if (!isset(self::$m_aCacheUsers)) - { - self::$m_aCacheUsers = array('internal' => array(), 'external' => array()); - } - if (!isset(self::$m_aCacheUsers[$sAuthentication][$sLogin])) - { - switch($sAuthentication) - { - case 'external': - $sBaseClass = 'UserExternal'; - break; - - case 'internal': - $sBaseClass = 'UserInternal'; - break; - - default: - echo "

sAuthentication = $sAuthentication

\n"; - assert(false); // should never happen - } - $oSearch = DBObjectSearch::FromOQL("SELECT $sBaseClass WHERE login = :login"); - $oSearch->AllowAllData(); - if (!$bAllowDisabledUsers) - { - $oSearch->AddCondition('status', 'enabled'); - } - $oSet = new DBObjectSet($oSearch, array(), array('login' => $sLogin)); - $oUser = $oSet->fetch(); - self::$m_aCacheUsers[$sAuthentication][$sLogin] = $oUser; - } - $oUser = self::$m_aCacheUsers[$sAuthentication][$sLogin]; + if (!isset(self::$m_aCacheUsers)) { + self::$m_aCacheUsers = [ 'internal' => [], 'external' => [] ]; } - return $oUser; + + if (! isset(self::$m_aCacheUsers[$sAuthentication]) || ! array_key_exists($sLogin, self::$m_aCacheUsers[$sAuthentication])) { + switch($sAuthentication) { + case 'external': + $sBaseClass = 'UserExternal'; + break; + + case 'internal': + $sBaseClass = 'UserInternal'; + break; + + default: + echo "

sAuthentication = $sAuthentication

\n"; + assert(false); // should never happen + } + $oSearch = DBObjectSearch::FromOQL("SELECT $sBaseClass WHERE login = :login"); + $oSearch->AllowAllData(); + if (!$bAllowDisabledUsers) { + $oSearch->AddCondition('status', 'enabled'); + } + $oSet = new DBObjectSet($oSearch, array(), array('login' => $sLogin)); + $oUser = $oSet->fetch(); + + self::$m_aCacheUsers[$sAuthentication][$sLogin] = $oUser; + } + + return self::$m_aCacheUsers[$sAuthentication][$sLogin]; } /** diff --git a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php index 523fe0258..6a3d3d7a2 100644 --- a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php +++ b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php @@ -488,4 +488,55 @@ class UserRightsTest extends ItopDataTestCase 'with Admins hidden' => [true, 0], ]; } + + public function testFindUser_ExistingInternalUser() + { + $sLogin = 'UserRightsFindUser'.uniqid(); + $iKey = $this->CreateUser($sLogin, self::$aURP_Profiles['Administrator'])->GetKey(); + $oUser = $this->InvokeNonPublicStaticMethod(UserRights::class, "FindUser", [$sLogin]); + + $this->assertNotNull($oUser); + $this->assertEquals($iKey, $oUser->GetKey()); + $this->assertEquals(\UserLocal::class, get_class($oUser)); + + $this->assertDBQueryCount(0, function() use ($sLogin, $iKey){ + $oUser = $this->InvokeNonPublicStaticMethod(UserRights::class, "FindUser", [$sLogin]); + static::assertEquals($iKey, $oUser->GetKey()); + static::assertEquals(\UserLocal::class, get_class($oUser)); + }); + } + + public function testFindUser_ExistingExternalUser() + { + $sLogin = 'UserRightsFindUser'.uniqid(); + + $iKey = $this->GivenObjectInDB(\UserExternal::class, [ + 'login' => $sLogin, + 'language' => 'EN US', + ]); + + $oUser = $this->InvokeNonPublicStaticMethod(UserRights::class, "FindUser", [$sLogin]); + + $this->assertNotNull($oUser); + $this->assertEquals($iKey, $oUser->GetKey()); + $this->assertEquals(\UserExternal::class, get_class($oUser)); + + $this->assertDBQueryCount(0, function() use ($sLogin, $iKey){ + $oUser = $this->InvokeNonPublicStaticMethod(UserRights::class, "FindUser", [$sLogin]); + static::assertEquals($iKey, $oUser->GetKey()); + static::assertEquals(\UserExternal::class, get_class($oUser)); + }); + } + + public function testFindUser_UnknownLogin_AvoidSameSqlQueryTwice() + { + $sLogin = 'UserRightsFindUser'.uniqid(); + $oUser = $this->InvokeNonPublicStaticMethod(UserRights::class, "FindUser", [$sLogin]); + $this->assertNull($oUser); + + $this->assertDBQueryCount(0, function() use ($sLogin){ + $oUser = $this->InvokeNonPublicStaticMethod(UserRights::class, "FindUser", [$sLogin]); + $this->assertNull($oUser); + }); + } }