From b2329939b689bfac70cce5bc787c83b1f7abfb7a Mon Sep 17 00:00:00 2001 From: jf-cbd Date: Mon, 8 Jun 2026 18:41:01 +0200 Subject: [PATCH] WIP --- dictionaries/en.dictionary.itop.core.php | 2 +- lib/composer/autoload_classmap.php | 2 +- lib/composer/autoload_static.php | 2 +- ...ory.php => ITopUserCountingRepository.php} | 283 +++++++++--------- ...php => ITopUserCountingRepositoryTest.php} | 52 ++-- 5 files changed, 177 insertions(+), 164 deletions(-) rename sources/Users/{ITopUserQuotaRepository.php => ITopUserCountingRepository.php} (62%) rename tests/php-unit-tests/unitary-tests/sources/Users/{ITopUserQuotaRepositoryTest.php => ITopUserCountingRepositoryTest.php} (62%) diff --git a/dictionaries/en.dictionary.itop.core.php b/dictionaries/en.dictionary.itop.core.php index 802247a632..f439d017ad 100644 --- a/dictionaries/en.dictionary.itop.core.php +++ b/dictionaries/en.dictionary.itop.core.php @@ -217,7 +217,7 @@ Operators:
'Core:Context=CRON' => 'cron', 'Core:Context=GUI:Portal' => 'Portal', - 'Core:GetQuota:Error' => 'Error while getting %1$s quota', + 'Core:GetCountingUsers:Error' => 'Error while getting %1$s quota', 'Core:ConsoleUsers' => 'console users', 'Core:DisabledUsers' => 'disabled users', 'Core:PortalUsers' => 'portal users', diff --git a/lib/composer/autoload_classmap.php b/lib/composer/autoload_classmap.php index 323cd9b46d..4be9f41e7e 100644 --- a/lib/composer/autoload_classmap.php +++ b/lib/composer/autoload_classmap.php @@ -651,7 +651,7 @@ return array( 'Combodo\\iTop\\SessionTracker\\SessionGC' => $baseDir . '/sources/SessionTracker/SessionGC.php', 'Combodo\\iTop\\SessionTracker\\SessionHandler' => $baseDir . '/sources/SessionTracker/SessionHandler.php', 'Combodo\\iTop\\SessionTracker\\iSessionHandlerExtension' => $baseDir . '/sources/SessionTracker/iSessionHandlerExtension.php', - 'Combodo\\iTop\\Users\\ITopUserQuotaRepository' => $baseDir . '/sources/Users/ITopUserQuotaRepository.php', + 'Combodo\\iTop\\Users\\ITopUserCountingRepository' => $baseDir . '/sources/Users/ITopUserCountingRepository.php', 'CompileCSSService' => $baseDir . '/application/compilecssservice.class.inc.php', 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', 'Config' => $baseDir . '/core/config.class.inc.php', diff --git a/lib/composer/autoload_static.php b/lib/composer/autoload_static.php index b1b5060675..896ade20b6 100644 --- a/lib/composer/autoload_static.php +++ b/lib/composer/autoload_static.php @@ -1052,7 +1052,7 @@ class ComposerStaticInitfc0e9e9dea11dcbb6272414776c30685 'Combodo\\iTop\\SessionTracker\\SessionGC' => __DIR__ . '/../..' . '/sources/SessionTracker/SessionGC.php', 'Combodo\\iTop\\SessionTracker\\SessionHandler' => __DIR__ . '/../..' . '/sources/SessionTracker/SessionHandler.php', 'Combodo\\iTop\\SessionTracker\\iSessionHandlerExtension' => __DIR__ . '/../..' . '/sources/SessionTracker/iSessionHandlerExtension.php', - 'Combodo\\iTop\\Users\\ITopUserQuotaRepository' => __DIR__ . '/../..' . '/sources/Users/ITopUserQuotaRepository.php', + 'Combodo\\iTop\\Users\\ITopUserCountingRepository' => __DIR__ . '/../..' . '/sources/Users/ITopUserCountingRepository.php', 'CompileCSSService' => __DIR__ . '/../..' . '/application/compilecssservice.class.inc.php', 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', 'Config' => __DIR__ . '/../..' . '/core/config.class.inc.php', diff --git a/sources/Users/ITopUserQuotaRepository.php b/sources/Users/ITopUserCountingRepository.php similarity index 62% rename from sources/Users/ITopUserQuotaRepository.php rename to sources/Users/ITopUserCountingRepository.php index 9094e2165a..75bb20c680 100644 --- a/sources/Users/ITopUserQuotaRepository.php +++ b/sources/Users/ITopUserCountingRepository.php @@ -22,13 +22,13 @@ use UserRights; /** * */ -class ITopUserQuotaRepository +class ITopUserCountingRepository { /** - * @param string $sExcludedUsers - * @param string $sExcludedProfiles + * @param array $aExcludedUsers + * @param array $aExcludedProfiles * @param bool $bAllData - * @param string $sExcludedFinalClasses + * @param array $aExcludedFinalClasses * * @return array * @throws CoreException @@ -37,23 +37,26 @@ class ITopUserQuotaRepository * @throws MySQLException * @throws Exception */ - public function GetConsoleUsers(string $sExcludedUsers = '', string $sExcludedProfiles = '', bool $bAllData = true, string $sExcludedFinalClasses = 'UserToken, UserRemoteSaaS'): array + public function GetConsoleUsers(array $aExcludedUsers = [], array $aExcludedProfiles = [], bool $bAllData = true, array $aExcludedFinalClasses = ['UserToken', 'UserRemoteSaaS']): array { - $sOQLInQuotaUser = " - SELECT User AS u - WHERE u.status != 'disabled' - AND u.login NOT IN ('$sExcludedUsers') - AND u.finalclass != ' $sExcludedFinalClasses ' - AND id NOT IN ( - SELECT User AS uex - JOIN URP_UserProfile AS uup ON uup.userid = uex.id - JOIN URP_Profiles AS up ON uup.profileid = up.id - WHERE up.name IN ('$sExcludedProfiles')) - "; + $sExcludedUsers = $this->ArrayToOQLStringParameter($aExcludedUsers); + $sExcludedProfiles = $this->ArrayToOQLStringParameter($aExcludedProfiles); + $sExcludedFinalClasses = $this->ArrayToOQLStringParameter($aExcludedFinalClasses); + + $sOQLUserConsole = " + SELECT User AS u + WHERE u.status != 'disabled' + AND u.login NOT IN ($sExcludedUsers) + AND u.finalclass NOT IN ($sExcludedFinalClasses) + AND id NOT IN ( + SELECT User AS uex + JOIN URP_UserProfile AS uup ON uup.userid = uex.id + JOIN URP_Profiles AS up ON uup.profileid = up.id + WHERE up.name IN ($sExcludedProfiles)) + "; try { - $oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLInQuotaUser) : DBObjectSearch::FromOQL($sOQLInQuotaUser); - } - catch (Exception $e) { + $oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLUserConsole) : DBObjectSearch::FromOQL($sOQLUserConsole); + } catch (Exception $e) { IssueLog::Error('Core:GetConsoleUsersQuota:Error : '.$e->getMessage()); throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:ConsoleUsers'))); } @@ -65,16 +68,53 @@ class ITopUserQuotaRepository return array_diff($aConsoleUsers, $aPortalUsers, $aReadOnlyUsers); } + private function ArrayToOQLStringParameter(array $aValues): string + { + $aQuotedValues = []; + foreach ($aValues as $value) { + $value = trim((string) $value); + if ($value === '') { + continue; + } + $aQuotedValues[] = "'".addslashes($value)."'"; + } + + return empty($aQuotedValues) ? "''" : implode(', ', $aQuotedValues); + } + + /** + * @throws CoreUnexpectedValue + * @throws CoreException + * @throws MySQLException + */ + public function GetUsersFromFilter(DBObjectSearch|DBUnionSearch|null $oFilter, array $aOrderBy = [], array $aArgs = []): array + { + $aUsers = []; + if (is_null($oFilter)) { + return $aUsers; + } + $oSet = new DBObjectSet($oFilter, $aOrderBy, $aArgs); + while ($oUser = $oSet->Fetch()) { + $aUsers[] = $oUser; + } + + return $aUsers; + } + /** * @throws Exception */ - public function GetApplicationUsers(bool $bAllData = true): array + public function GetPortalUsers(bool $bAllData = true): array { - $sOQLApplicationUser = 'SELECT UserToken'; + $sOQLPortalUser = " + SELECT User AS u + JOIN URP_UserProfile AS uup ON uup.userid = u.id + JOIN URP_Profiles AS up ON uup.profileid = up.id + WHERE up.id = '2' AND u.status != 'disabled' "; + try { - $oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLApplicationUser) : DBObjectSearch::FromOQL($sOQLApplicationUser); - } - catch (Exception $e) { + $oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLPortalUser) : DBObjectSearch::FromOQL($sOQLPortalUser); + } catch (Exception $e) { IssueLog::Error('Core:GetConsoleUsersQuota:Error : '.$e->getMessage()); throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:ApplicationUsers'))); } @@ -82,72 +122,6 @@ class ITopUserQuotaRepository return $this->GetUsersFromFilter($oFilter); } - /** - * @throws Exception - */ - public function GetDisabledUsers(bool $bAllData = true): array - { - $sOQLDisabledUser = " - SELECT User AS u - WHERE u.status = 'disabled' - "; - try { - $oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLDisabledUser) : DBObjectSearch::FromOQL($sOQLDisabledUser); - } - catch (Exception $e) { - IssueLog::Error('Core:GetDisabledUsersQuota:Error : '.$e->getMessage()); - throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:DisabledUsers'))); - } - - return $this->GetUsersFromFilter($oFilter); - } - - /** - * @throws CoreException - * @throws MySQLException - * @throws CoreUnexpectedValue - * @throws OQLException - * @throws ArchivedObjectException - * @throws DictExceptionUnknownLanguage - */ - private function IsUserReadOnly(User $oUser, string $sClassCategory): bool -{ - if ($oUser->Get('status') == 'disabled') { - return false; - } - - // check if user is a portal user - $oProfileLinks = $oUser->Get('profile_list'); - while ($oLink = $oProfileLinks->Fetch()) { - $iProfileId = $oLink->Get('profileid'); - if (!$iProfileId) { - continue; - } - $oProfile = MetaModel::GetObject('URP_Profiles', $iProfileId, false); - if ($oProfile && $oProfile->Get('name') === PORTAL_PROFILE_NAME) { - return false; - } - } - - // login (mandatory to compute rights) - UserRights::Login($oUser->GetName()); - - foreach (MetaModel::GetClasses($sClassCategory) as $sClass) { - // no need to check stimulis for now since users can't execute stimulus without UR_ACTION_MODIFY - if ( - UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, null, $oUser) || - UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_MODIFY, null, $oUser) || - UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, null, $oUser) || - UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, null, $oUser) - ) { - UserRights::Logoff(); - return false; - } - } - UserRights::Logoff(); - return true; -} - /** * @throws DictExceptionMissingString * @throws CoreException @@ -178,48 +152,6 @@ class ITopUserQuotaRepository return array_diff($aReadOnlyUsers, $aDisabledUsers); } - /** - * @throws Exception - */ - public function GetPortalUsers(bool $bAllData = true): array - { - $sOQLPortalUser = ' - SELECT User AS u - JOIN URP_UserProfile AS uup ON uup.userid = u.id - JOIN URP_Profiles AS up ON uup.profileid = up.id - WHERE up.name = \' '.PORTAL_PROFILE_NAME.'\''; - - try { - $oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLPortalUser) : DBObjectSearch::FromOQL($sOQLPortalUser); - } - catch (Exception $e) { - IssueLog::Error('combodo-users-quota-slave/GetUsersInQuota : '.$e->getMessage(), 'combodo-users-quota'); - throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:PortalUsers'))); - } - - // TODO remove read only users - return $this->GetUsersFromFilter($oFilter); - } - - /** - * @throws CoreUnexpectedValue - * @throws CoreException - * @throws MySQLException - */ - public function GetUsersFromFilter(DBObjectSearch|DBUnionSearch|null $oFilter, array $aOrderBy = [], array $aArgs = []): array - { - $aUsers = []; - if (is_null($oFilter)) { - return $aUsers; - } - $oSet = new DBObjectSet($oFilter, $aOrderBy, $aArgs); - while ($oUser = $oSet->Fetch()) { - $aUsers[] = $oUser; - } - - return $aUsers; - } - /** * @throws Exception */ @@ -229,13 +161,92 @@ class ITopUserQuotaRepository try { $oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOqlUser) : DBObjectSearch::FromOQL($sOqlUser); - } - catch (Exception $e) { + } catch (Exception $e) { IssueLog::Error('combodo-users-quota-slave/GetUsersNotInQuota : '.$e->getMessage(), 'combodo-users-quota'); throw new Exception(Dict::S('CombodoUserQuota:Error')); } - + return $this->GetUsersFromFilter($oFilter); } -} \ No newline at end of file + /** + * @throws CoreException + * @throws MySQLException + * @throws CoreUnexpectedValue + * @throws OQLException + * @throws ArchivedObjectException + * @throws DictExceptionUnknownLanguage + */ + private function IsUserReadOnly(User $oUser, string $sClassCategory): bool + { + if ($oUser->Get('status') == 'disabled') { + return false; + } + + // check if user is a portal user + $oProfileLinks = $oUser->Get('profile_list'); + while ($oLink = $oProfileLinks->Fetch()) { + $iProfileId = $oLink->Get('profileid'); + if (!$iProfileId) { + continue; + } + $oProfile = MetaModel::GetObject('URP_Profiles', $iProfileId, false); + if ($oProfile && $oProfile->Get('name') === PORTAL_PROFILE_NAME) { + return false; + } + } + + // login (mandatory to compute rights) + UserRights::Login($oUser->GetName()); + + foreach (MetaModel::GetClasses($sClassCategory) as $sClass) { + // no need to check stimuli for now since users can't execute stimulus without UR_ACTION_MODIFY + if ( + UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, null, $oUser) || + UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_MODIFY, null, $oUser) || + UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, null, $oUser) || + UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, null, $oUser) + ) { + UserRights::Logoff(); + return false; + } + } + UserRights::Logoff(); + return true; + } + + /** + * @throws Exception + */ + public function GetDisabledUsers(bool $bAllData = true): array + { + $sOQLDisabledUser = " + SELECT User AS u + WHERE u.status = 'disabled' + "; + try { + $oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLDisabledUser) : DBObjectSearch::FromOQL($sOQLDisabledUser); + } catch (Exception $e) { + IssueLog::Error('Core:GetDisabledUsersQuota:Error : '.$e->getMessage()); + throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:DisabledUsers'))); + } + + return $this->GetUsersFromFilter($oFilter); + } + + /** + * @throws Exception + */ + public function GetApplicationUsers(bool $bAllData = true): array + { + $sOQLApplicationUser = 'SELECT UserToken'; + try { + $oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLApplicationUser) : DBObjectSearch::FromOQL($sOQLApplicationUser); + } catch (Exception $e) { + IssueLog::Error('Core:GetConsoleUsersQuota:Error : '.$e->getMessage()); + throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:ApplicationUsers'))); + } + + return $this->GetUsersFromFilter($oFilter); + } +} diff --git a/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserQuotaRepositoryTest.php b/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserCountingRepositoryTest.php similarity index 62% rename from tests/php-unit-tests/unitary-tests/sources/Users/ITopUserQuotaRepositoryTest.php rename to tests/php-unit-tests/unitary-tests/sources/Users/ITopUserCountingRepositoryTest.php index a0f58ca8e9..8ea985757c 100644 --- a/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserQuotaRepositoryTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserCountingRepositoryTest.php @@ -3,10 +3,11 @@ namespace Users; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; -use Combodo\iTop\Users\ITopUserQuotaRepository; +use Combodo\iTop\Users\ITopUserCountingRepository; use User; -class ITopUserQuotaRepositoryTest extends ItopDataTestCase{ +class ITopUserCountingRepositoryTest extends ItopDataTestCase +{ protected function setUp(): void { parent::setUp(); @@ -24,7 +25,8 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{ $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Configuration ReadOnly', 'Ticket ReadOnly', 'Service Catalog ReadOnly']); } - private function CreateDisabledUser() { + private function CreateDisabledUser() + { $sUser = $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Configuration Manager']); // get user by login $oUser = \MetaModel::GetObjectByName('User', $sUser); @@ -39,11 +41,11 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{ * @throws \MySQLException * @throws \Exception */ - public function testNotDuplicateInDifferentQuotas(): void + public function testNotDuplicateInDifferentCountsCategories(): void { - $oITopUserRepository = new ITopUserQuotaRepository(); + $oITopUserRepository = new ITopUserCountingRepository(); - $aQuotaUsers = [ + $aCountedUsers = [ 'console' => $oITopUserRepository->GetConsoleUsers(), 'portal' => $oITopUserRepository->GetPortalUsers(), 'disabled' => $oITopUserRepository->GetDisabledUsers(), @@ -51,31 +53,32 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{ 'application' => $oITopUserRepository->GetApplicationUsers(), ]; - $aUserToQuotas = []; - foreach ($aQuotaUsers as $sQuota => $aUsers) { + $aCountedUserFormated = []; + foreach ($aCountedUsers as $sCountedCategory => $aUsers) { foreach ($aUsers as $oUser) { $sUserId = (string) $oUser->GetKey(); - $aUserToQuotas[$sUserId][$sQuota] = true; + $aCountedUserFormated[$sUserId][$sCountedCategory] = true; } } $aDuplicates = []; - foreach ($aUserToQuotas as $sUserId => $aQuotas) { - $aQuotaNames = array_keys($aQuotas); - if (count($aQuotaNames) > 1) { - sort($aQuotaNames); - $aDuplicates[] = sprintf('User #%s appears in: %s', $sUserId, implode(', ', $aQuotaNames)); + foreach ($aCountedUserFormated as $sUserId => $aCountedCategory) { + $aCountedCategoryName = array_keys($aCountedCategory); + if (count($aCountedCategoryName) > 1) { + sort($aCountedCategoryName); + $aDuplicates[] = sprintf('User #%s appears in: %s', $sUserId, implode(', ', $aCountedCategoryName)); } } $this->assertEmpty( $aDuplicates, - "Some users are counted in multiple quotas:\n- ".implode("\n- ", $aDuplicates) + "Some users are counted in multiple categories:\n- ".implode("\n- ", $aDuplicates) ); } - public function testAllUsersAreInQuota () { - $oITopUserRepository = new ITopUserQuotaRepository(); + public function testAllUsersAreCounted() + { + $oITopUserRepository = new ITopUserCountingRepository(); $aConsoleUsers = $oITopUserRepository->GetConsoleUsers(); $aPortalUsers = $oITopUserRepository->GetPortalUsers(); @@ -83,16 +86,16 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{ $aReadOnlyUsers = $oITopUserRepository->GetReadOnlyUsers(); $aApplicationUsers = $oITopUserRepository->GetApplicationUsers(); - $aAllUsersFromQuota = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers); + $aAllUsersFromMergedCounts = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers); $aAllUsersFromOQL = $oITopUserRepository->GetAllUsers(); - $this->assertEmpty(array_merge(array_diff($aAllUsersFromQuota, $aAllUsersFromOQL), array_diff($aAllUsersFromOQL, $aAllUsersFromQuota))); + $this->assertEmpty(array_merge(array_diff($aAllUsersFromMergedCounts, $aAllUsersFromOQL), array_diff($aAllUsersFromOQL, $aAllUsersFromMergedCounts))); } - public function testAllUsersInQuotaAreUsersObjects () + public function testAllCountedUsersAreUsersObjects() { - $oITopUserRepository = new ITopUserQuotaRepository(); + $oITopUserRepository = new ITopUserCountingRepository(); $aConsoleUsers = $oITopUserRepository->GetConsoleUsers(); $aPortalUsers = $oITopUserRepository->GetPortalUsers(); @@ -100,13 +103,12 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{ $aReadOnlyUsers = $oITopUserRepository->GetReadOnlyUsers(); $aApplicationUsers = $oITopUserRepository->GetApplicationUsers(); - $aAllQuotaUsers = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers); + $aCountedUsers = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers); - foreach ($aAllQuotaUsers as $oUser) { + foreach ($aCountedUsers as $oUser) { $this->assertInstanceOf(User::class, $oUser); } - } -} \ No newline at end of file +}