diff --git a/sources/Users/ITopUserQuotaRepository.php b/sources/Users/ITopUserQuotaRepository.php index 3736f48bf..9094e2165 100644 --- a/sources/Users/ITopUserQuotaRepository.php +++ b/sources/Users/ITopUserQuotaRepository.php @@ -2,18 +2,20 @@ namespace Combodo\iTop\Users; +use ArchivedObjectException; use CoreException; use CoreUnexpectedValue; use DBObjectSearch; use DBObjectSet; -use DBSearch; use DBUnionSearch; use Dict; use DictExceptionMissingString; +use DictExceptionUnknownLanguage; use Exception; use IssueLog; use MetaModel; use MySQLException; +use OQLException; use User; use UserRights; @@ -28,10 +30,14 @@ class ITopUserQuotaRepository * @param bool $bAllData * @param string $sExcludedFinalClasses * - * @return DBObjectSearch|DBUnionSearch|null + * @return array + * @throws CoreException + * @throws CoreUnexpectedValue + * @throws DictExceptionMissingString + * @throws MySQLException * @throws Exception */ - public function GetConsoleUsers(string $sExcludedUsers = '', string $sExcludedProfiles = '', bool $bAllData = true, string $sExcludedFinalClasses = 'UserToken, UserRemoteSaaS'): null|DBObjectSearch|DBUnionSearch + public function GetConsoleUsers(string $sExcludedUsers = '', string $sExcludedProfiles = '', bool $bAllData = true, string $sExcludedFinalClasses = 'UserToken, UserRemoteSaaS'): array { $sOQLInQuotaUser = " SELECT User AS u @@ -52,14 +58,17 @@ class ITopUserQuotaRepository throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:ConsoleUsers'))); } - // TODO remove read only users - return $oFilter; + $aConsoleUsers = $this->GetUsersFromFilter($oFilter); + $aPortalUsers = $this->GetPortalUsers(); + $aReadOnlyUsers = $this->GetReadOnlyUsers(); + + return array_diff($aConsoleUsers, $aPortalUsers, $aReadOnlyUsers); } /** * @throws Exception */ - public function GetApplicationUsers(bool $bAllData = true): null|DBObjectSearch|DBUnionSearch + public function GetApplicationUsers(bool $bAllData = true): array { $sOQLApplicationUser = 'SELECT UserToken'; try { @@ -70,14 +79,13 @@ class ITopUserQuotaRepository throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:ApplicationUsers'))); } - return $oFilter; - + return $this->GetUsersFromFilter($oFilter); } /** * @throws Exception */ - public function GetDisabledUsers(bool $bAllData = true): null|DBObjectSearch|DBUnionSearch + public function GetDisabledUsers(bool $bAllData = true): array { $sOQLDisabledUser = " SELECT User AS u @@ -91,34 +99,46 @@ class ITopUserQuotaRepository throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:DisabledUsers'))); } - return $oFilter; + return $this->GetUsersFromFilter($oFilter); } -private function IsUserReadOnly(User $oUser, string $sClassCategory) + /** + * @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) { - $aClassStimuli = MetaModel::EnumStimuli($sClass); - if (count($aClassStimuli) > 0) { - $aStimuli = []; - foreach ($aClassStimuli as $sStimulusCode => $oStimulus) { - if (UserRights::IsStimulusAllowed($sClass, $sStimulusCode, null, $oUser)) { - $aStimuli[] = - $oStimulus->GetLabel(); - } - } - $sStimuli = implode(', ', $aStimuli); - } else { - $sStimuli = ''; - } - + // 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) || - $sStimuli != '' + UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, null, $oUser) ) { UserRights::Logoff(); return false; @@ -136,31 +156,32 @@ private function IsUserReadOnly(User $oUser, string $sClassCategory) public function GetReadOnlyUsers(): array { $aReadOnlyUsers = []; - $oAllUsersFilter = $this->GetAllUsers(); - $aAllUsers = $this->GetUsersFromFilter($oAllUsersFilter); + $aAllUsers = $this->GetAllUsers(); /** @var User $oUser */ foreach ($aAllUsers as $oUser) { $bIsReadOnlyUser = true; - if (!$this->IsUserReadOnly($oUser, 'bizmodel') || !$this->IsUserReadOnly($oUser, 'grant_by_profile')) { $bIsReadOnlyUser = false; } - if ($bIsReadOnlyUser) { $aReadOnlyUsers[] = $oUser; } - } - // TODO remove disabled users - return $aReadOnlyUsers; + // remove portal users + $aPortalUsers = $this->GetPortalUsers(); + $aReadOnlyUsers = array_diff($aReadOnlyUsers, $aPortalUsers); + // remove disabled users + $aDisabledUsers = $this->GetDisabledUsers(); + + return array_diff($aReadOnlyUsers, $aDisabledUsers); } /** * @throws Exception */ - public function getPortalUsers(bool $bAllData = true): null|DBObjectSearch|DBUnionSearch + public function GetPortalUsers(bool $bAllData = true): array { $sOQLPortalUser = ' SELECT User AS u @@ -177,7 +198,7 @@ private function IsUserReadOnly(User $oUser, string $sClassCategory) } // TODO remove read only users - return $oFilter; + return $this->GetUsersFromFilter($oFilter); } /** @@ -192,7 +213,7 @@ private function IsUserReadOnly(User $oUser, string $sClassCategory) return $aUsers; } $oSet = new DBObjectSet($oFilter, $aOrderBy, $aArgs); - while ($oUser = $oSet->fetch()) { + while ($oUser = $oSet->Fetch()) { $aUsers[] = $oUser; } @@ -202,7 +223,7 @@ private function IsUserReadOnly(User $oUser, string $sClassCategory) /** * @throws Exception */ - public function GetAllUsers(bool $bAllData = true): DBUnionSearch|DBObjectSearch|DBSearch|null + public function GetAllUsers(bool $bAllData = true): array { $sOqlUser = 'SELECT User'; @@ -213,9 +234,8 @@ private function IsUserReadOnly(User $oUser, string $sClassCategory) IssueLog::Error('combodo-users-quota-slave/GetUsersNotInQuota : '.$e->getMessage(), 'combodo-users-quota'); throw new Exception(Dict::S('CombodoUserQuota:Error')); } - - return $oFilter; - + + return $this->GetUsersFromFilter($oFilter); } } \ No newline at end of file diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php index 7be4d9d6b..42468ee94 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php @@ -87,8 +87,6 @@ abstract class ItopDataTestCase extends ItopTestCase */ public const DEFAULT_TEST_ENVIRONMENT = 'production'; public const USE_TRANSACTION = true; - public const CREATE_TEST_ORG = false; - protected static $aURP_Profiles = [ 'Administrator' => 1, 'Portal user' => 2, @@ -103,8 +101,13 @@ abstract class ItopDataTestCase extends ItopTestCase 'Document author' => 11, 'Portal power user' => 12, 'REST Services User' => 1024, + 'Configuration ReadOnly' => 5500, + 'Ticket ReadOnly' => 5501, + 'Service Catalog ReadOnly' => 5502, ]; + public const CREATE_TEST_ORG = false; + /** * This method is called before the first test of this test class is run (in the current process). */ diff --git a/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserQuotaRepositoryTest.php b/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserQuotaRepositoryTest.php index e9108abfa..a0f58ca8e 100644 --- a/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserQuotaRepositoryTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserQuotaRepositoryTest.php @@ -2,119 +2,53 @@ namespace Users; -use CMDBObjectSet; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; use Combodo\iTop\Users\ITopUserQuotaRepository; -use DBObjectSearch; -use MetaModel; use User; class ITopUserQuotaRepositoryTest extends ItopDataTestCase{ - - private static bool $bDatasetInitialized = false; - protected function setUp(): void { parent::setUp(); + $this->CreateReadOnlyUsers(); - if (self::$bDatasetInitialized) { - return; - } - - $this->createUsersQuotaDataset(); - self::$bDatasetInitialized = true; } - - /** * Creates a deterministic dataset for quota tests. * Users are created only once (idempotent on login). */ - private function createUsersQuotaDataset(): void + private function CreateReadOnlyUsers() { - // Keep names unique and easy to clean up later if needed. - $sPrefix = 'quota_test_'; - - // Create one user per quota "kind". - // NOTE: profile names can vary by iTop distribution; we try common ones. - $this->createUserIfMissing($sPrefix.'console', true, ['Administrator', 'Configuration Administrator']); - $this->createUserIfMissing($sPrefix.'portal', true, ['Portal user', 'Portal User']); - $this->createUserIfMissing($sPrefix.'readonly', true, ['ReadOnlyCI']); - $this->createUserIfMissing($sPrefix.'application', true, ['Service Desk Agent', 'Change Manager', 'Administrator']); - $this->createUserIfMissing($sPrefix.'disabled', false, ['Service Desk Agent', 'Administrator']); - $this->createUserIfMissing($sPrefix.'disabled', false, ['Service Desk Agent', 'Administrator']); - - - + $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Configuration ReadOnly']); + $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Ticket ReadOnly']); + $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Service Catalog ReadOnly']); + $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Configuration ReadOnly', 'Ticket ReadOnly']); + $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Ticket ReadOnly', 'Service Catalog ReadOnly']); + $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Configuration ReadOnly', 'Ticket ReadOnly', 'Service Catalog ReadOnly']); } - private function createUserIfMissing(string $sLogin, bool $bEnabled, array $aCandidateProfileNames): void - { - if ($this->findUserByLogin($sLogin) !== null) { - return; - } - - $iProfileId = $this->findFirstProfileIdByNames($aCandidateProfileNames); - $this->assertNotNull( - $iProfileId, - sprintf('Could not find any profile among: %s', implode(', ', $aCandidateProfileNames)) - ); - - $oOrg = MetaModel::NewObject('Organization'); - $oOrg->Set('name', 'Quota Test Org'); - $oOrg->DBInsert(); - - $oPerson = MetaModel::NewObject('Person'); - $oPerson->Set('name', strtoupper($sLogin)); - $oPerson->Set('first_name', 'Quota'); - $oPerson->Set('org_id', $oOrg->GetKey()); - $oPerson->Set('email', $sLogin.'@example.invalid'); - $oPerson->DBInsert(); - - $oUser = MetaModel::NewObject('UserLocal'); - $oUser->Set('login', $sLogin); - $oUser->Set('password', 'QuotaTest#123'); - $oUser->Set('contactid', $oPerson->GetKey()); - $oUser->Set('status', $bEnabled ? 'enabled' : 'disabled'); - - $oProfileList = $oUser->Get('profile_list'); - $oLink = MetaModel::NewObject('URP_UserProfile'); - $oLink->Set('profileid', $iProfileId); - $oProfileList->AddItem($oLink); - $oUser->Set('profile_list', $oProfileList); - - $oUser->DBInsert(); + private function CreateDisabledUser() { + $sUser = $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Configuration Manager']); + // get user by login + $oUser = \MetaModel::GetObjectByName('User', $sUser); + $oUser->Set('status', 'disabled'); + $oUser->DBUpdate(); } - private function findFirstProfileIdByNames(array $aProfileNames): ?int - { - foreach ($aProfileNames as $sProfileName) { - $oSearch = DBObjectSearch::FromOQL('SELECT URP_Profiles WHERE name = :name'); - $oSet = new CMDBObjectSet($oSearch, [], ['name' => $sProfileName]); - $oProfile = $oSet->Fetch(); - if ($oProfile !== false && $oProfile !== null) { - return (int) $oProfile->GetKey(); - } - } - return null; - } - - private function findUserByLogin(string $sLogin): ?User - { - $oSearch = DBObjectSearch::FromOQL('SELECT User WHERE login = :login'); - $oSet = new CMDBObjectSet($oSearch, [], ['login' => $sLogin]); - $oUser = $oSet->Fetch(); - return ($oUser instanceof User) ? $oUser : null; - } - - + /** + * @throws \CoreUnexpectedValue + * @throws \DictExceptionMissingString + * @throws \CoreException + * @throws \MySQLException + * @throws \Exception + */ public function testNotDuplicateInDifferentQuotas(): void { $oITopUserRepository = new ITopUserQuotaRepository(); $aQuotaUsers = [ - 'console' => $oITopUserRepository->GetUsersFromFilter($oITopUserRepository->GetConsoleUsers()), - 'portal' => $oITopUserRepository->GetUsersFromFilter($oITopUserRepository->GetPortalUsers()), - 'disabled' => $oITopUserRepository->GetUsersFromFilter($oITopUserRepository->GetDisabledUsers()), + 'console' => $oITopUserRepository->GetConsoleUsers(), + 'portal' => $oITopUserRepository->GetPortalUsers(), + 'disabled' => $oITopUserRepository->GetDisabledUsers(), 'readonly' => $oITopUserRepository->GetReadOnlyUsers(), - 'application' => $oITopUserRepository->GetUsersFromFilter($oITopUserRepository->GetApplicationUsers()), + 'application' => $oITopUserRepository->GetApplicationUsers(), ]; $aUserToQuotas = []; @@ -143,20 +77,15 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{ public function testAllUsersAreInQuota () { $oITopUserRepository = new ITopUserQuotaRepository(); - $oConsoleUsersFilter = $oITopUserRepository->GetConsoleUsers(); - $aConsoleUsers = $oITopUserRepository->GetUsersFromFilter($oConsoleUsersFilter); - $oPortalUsersFilter = $oITopUserRepository->GetPortalUsers(); - $aPortalUsers = $oITopUserRepository->GetUsersFromFilter($oPortalUsersFilter); - $oDisabledUsersFilter = $oITopUserRepository->GetDisabledUsers(); - $aDisabledUsers = $oITopUserRepository->GetUsersFromFilter($oDisabledUsersFilter); + $aConsoleUsers = $oITopUserRepository->GetConsoleUsers(); + $aPortalUsers = $oITopUserRepository->GetPortalUsers(); + $aDisabledUsers = $oITopUserRepository->GetDisabledUsers(); $aReadOnlyUsers = $oITopUserRepository->GetReadOnlyUsers(); - $oApplicationUsersFilter = $oITopUserRepository->GetApplicationUsers(); - $aApplicationUsers = $oITopUserRepository->GetUsersFromFilter($oApplicationUsersFilter); + $aApplicationUsers = $oITopUserRepository->GetApplicationUsers(); $aAllUsersFromQuota = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers); - $oAllUsersFilter = $oITopUserRepository->GetAllUsers(); - $aAllUsersFromOQL = $oITopUserRepository->GetUsersFromFilter($oAllUsersFilter); + $aAllUsersFromOQL = $oITopUserRepository->GetAllUsers(); $this->assertEmpty(array_merge(array_diff($aAllUsersFromQuota, $aAllUsersFromOQL), array_diff($aAllUsersFromOQL, $aAllUsersFromQuota))); } @@ -165,15 +94,11 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{ { $oITopUserRepository = new ITopUserQuotaRepository(); - $oConsoleUsersFilter = $oITopUserRepository->GetConsoleUsers(); - $aConsoleUsers = $oITopUserRepository->GetUsersFromFilter($oConsoleUsersFilter); - $oPortalUsersFilter = $oITopUserRepository->GetPortalUsers(); - $aPortalUsers = $oITopUserRepository->GetUsersFromFilter($oPortalUsersFilter); - $oDisabledUsersFilter = $oITopUserRepository->GetDisabledUsers(); - $aDisabledUsers = $oITopUserRepository->GetUsersFromFilter($oDisabledUsersFilter); + $aConsoleUsers = $oITopUserRepository->GetConsoleUsers(); + $aPortalUsers = $oITopUserRepository->GetPortalUsers(); + $aDisabledUsers = $oITopUserRepository->GetDisabledUsers(); $aReadOnlyUsers = $oITopUserRepository->GetReadOnlyUsers(); - $oApplicationUsersFilter = $oITopUserRepository->GetApplicationUsers(); - $aApplicationUsers = $oITopUserRepository->GetUsersFromFilter($oApplicationUsersFilter); + $aApplicationUsers = $oITopUserRepository->GetApplicationUsers(); $aAllQuotaUsers = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers);