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/datamodels/2.x/itop-oauth-client/datamodel.itop-oauth-client.xml b/datamodels/2.x/itop-oauth-client/datamodel.itop-oauth-client.xml index 486b9e311..d90a6f954 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 2c495639f..74c3bcd9d 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 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); + }); + } }