mirror of
https://github.com/Combodo/iTop.git
synced 2026-04-23 18:48:51 +02:00
N°7633 - Reloads the same user multiple times if it no longer exists (#692)
* 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 <thomas.casteleyn@super-visions.com> * Update core/userrights.class.inc.php good catch (again) Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com> * Update core/userrights.class.inc.php Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com> * PR feedbacks from Romain * ci: rename user logins --------- Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
This commit is contained in:
@@ -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 "<p>sAuthentication = $sAuthentication</p>\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 "<p>sAuthentication = $sAuthentication</p>\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];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user