diff --git a/addons/userrights/userrightsprofile.class.inc.php b/addons/userrights/userrightsprofile.class.inc.php index e9ead975b6..6d159ecf70 100644 --- a/addons/userrights/userrightsprofile.class.inc.php +++ b/addons/userrights/userrightsprofile.class.inc.php @@ -648,7 +648,7 @@ public function GetSelectFilter($oUser, $sClass, $aSettings = array()) $aConditions = array(); // Determine if this class is part of a silo and build the filter for it - $sAttCode = self::GetOwnerOrganizationAttCode($sClass); + $sAttCode = UserRights::GetOwnerOrganizationAttCode($sClass); if (!is_null($sAttCode)) { $aUserOrgs = $this->GetUserOrgs($oUser, $sClass); @@ -834,7 +834,7 @@ public function IsActionAllowed($oUser, $sClass, $iActionCode, $oInstanceSet = n // But currently we are checking wether the objects might be written... // Let's exclude the objects based on the relevant criteria - $sOrgAttCode = self::GetOwnerOrganizationAttCode($sClass); + $sOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sClass); if (!is_null($sOrgAttCode)) { $aUserOrgs = $this->GetUserOrgs($oUser, $sClass); @@ -938,31 +938,11 @@ public function FlushPrivileges() * @param string $sClass * @return string|null Find out which attribute is corresponding the dimension 'owner org' * returns null if no such attribute has been found (no filtering should occur) + * @deprecated 3.3.0 use @UserRights::GetOwnerOrganizationAttCode instead */ public static function GetOwnerOrganizationAttCode($sClass) { - $sAttCode = null; - - $aCallSpec = array($sClass, 'MapContextParam'); - if (($sClass == 'Organization') || is_subclass_of($sClass, 'Organization')) - { - $sAttCode = 'id'; - } - elseif (is_callable($aCallSpec)) - { - $sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter - if (!MetaModel::IsValidAttCode($sClass, $sAttCode)) - { - // Skip silently. The data model checker will tell you something about this... - $sAttCode = null; - } - } - elseif(MetaModel::IsValidAttCode($sClass, 'org_id')) - { - $sAttCode = 'org_id'; - } - - return $sAttCode; + return UserRights::GetOwnerOrganizationAttCode($sClass); } /** diff --git a/addons/userrights/userrightsprofile.db.class.inc.php b/addons/userrights/userrightsprofile.db.class.inc.php index 7ae7214b02..ab22fe0f1d 100644 --- a/addons/userrights/userrightsprofile.db.class.inc.php +++ b/addons/userrights/userrightsprofile.db.class.inc.php @@ -778,7 +778,7 @@ public function GetSelectFilter($oUser, $sClass, $aSettings = array()) // Determine how to position the objects of this class // - $sAttCode = self::GetOwnerOrganizationAttCode($sClass); + $sAttCode = UserRights::GetOwnerOrganizationAttCode($sClass); if (is_null($sAttCode)) { // No filtering for this object @@ -909,7 +909,7 @@ public function IsActionAllowed($oUser, $sClass, $iActionCode, $oInstanceSet = n // But currently we are checking wether the objects might be written... // Let's exclude the objects based on the relevant criteria - $sOrgAttCode = self::GetOwnerOrganizationAttCode($sClass); + $sOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sClass); if (!is_null($sOrgAttCode)) { $aUserOrgs = $this->GetUserOrgs($oUser, $sClass); @@ -1015,28 +1015,7 @@ public function FlushPrivileges() */ public static function GetOwnerOrganizationAttCode($sClass) { - $sAttCode = null; - - $aCallSpec = array($sClass, 'MapContextParam'); - if (($sClass == 'Organization') || is_subclass_of($sClass, 'Organization')) - { - $sAttCode = 'id'; - } - elseif (is_callable($aCallSpec)) - { - $sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter - if (!MetaModel::IsValidAttCode($sClass, $sAttCode)) - { - // Skip silently. The data model checker will tell you something about this... - $sAttCode = null; - } - } - elseif(MetaModel::IsValidAttCode($sClass, 'org_id')) - { - $sAttCode = 'org_id'; - } - - return $sAttCode; + return UserRights::GetOwnerOrganizationAttCode($sClass);; } /** diff --git a/core/inlineimage.class.inc.php b/core/inlineimage.class.inc.php index ce49078ea2..a0ec221c52 100644 --- a/core/inlineimage.class.inc.php +++ b/core/inlineimage.class.inc.php @@ -140,36 +140,21 @@ public function SetItem(DBObject $oItem, $bUpdateOnChange = false) */ public function SetDefaultOrgId() { - // First check that the organization CAN be fetched from the target class - // - $sClass = $this->Get('item_class'); - $aCallSpec = array($sClass, 'MapContextParam'); - if (is_callable($aCallSpec)) - { - $sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter - if (MetaModel::IsValidAttCode($sClass, $sAttCode)) - { - // Second: check that the organization CAN be fetched from the current user - // - if (MetaModel::IsValidClass('Person')) - { - $aCallSpec = array($sClass, 'MapContextParam'); - if (is_callable($aCallSpec)) - { - $sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter - if (MetaModel::IsValidAttCode($sClass, $sAttCode)) - { - // OK - try it - // - $oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false); - if ($oCurrentPerson) - { - $this->Set('item_org_id', $oCurrentPerson->Get($sAttCode)); - } - } - } - } - } + // If the item class has no organization attribute, then no need to set the organization id + if (is_null(UserRights::GetOwnerOrganizationAttCode( $this->Get('item_class')))) { + // No need for silos + return; + } + // get organization attribute code for the person class + $sOrgAttrCodeForPerson = UserRights::GetOwnerOrganizationAttCode('Person'); + if (is_null($sOrgAttrCodeForPerson)) { + // No need for silos + return; + } + + $oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false); + if ($oCurrentPerson) { + $this->Set('item_org_id', $oCurrentPerson->Get($sOrgAttrCodeForPerson)); } } diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php index 3c4ad349a5..7eb331a040 100644 --- a/core/userrights.class.inc.php +++ b/core/userrights.class.inc.php @@ -2033,6 +2033,36 @@ public static function GetLastLoginStatus() { return self::$m_sLastLoginStatus; } + + + /** + * @param string $sClass + * @return string|null Find out which attribute is corresponding the dimension 'owner org' + * returns null if no such attribute has been found (no filtering should occur) + * @since 3.3.0 + */ + public static function GetOwnerOrganizationAttCode($sClass) + { + $sAttCode = null; + + $aCallSpec = array($sClass, 'MapContextParam'); + if (($sClass == 'Organization') || is_subclass_of($sClass, 'Organization')) { + $sAttCode = 'id'; + } + elseif (is_callable($aCallSpec)) { + $sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter + if (!MetaModel::IsValidAttCode($sClass, $sAttCode)) { + // Skip silently. The data model checker will tell you something about this... + $sAttCode = null; + } + } + elseif(MetaModel::IsValidAttCode($sClass, 'org_id')) { + $sAttCode = 'org_id'; + } + + return $sAttCode; + } + } /** diff --git a/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml b/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml index c661eb97f2..bc54d4d52b 100755 --- a/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml +++ b/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml @@ -160,26 +160,20 @@ $this->Set('item_class', $sClass); $this->Set('item_id', $iItemId); - $aCallSpec = array($sClass, 'MapContextParam'); - if (is_callable($aCallSpec)) - { - $sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter - if (MetaModel::IsValidAttCode($sClass, $sAttCode)) - { - $iOrgId = $oItem->Get($sAttCode); - if ($iOrgId > 0) - { - if ($iOrgId != $this->Get('item_org_id')) - { - $this->Set('item_org_id', $iOrgId); - if ($bUpdateOnChange) - { - $this->DBUpdate(); - } - } - } - } + $sAttCode = UserRights::GetOwnerOrganizationAttCode( $sClass); + if (is_null($sAttCode)) { + // No need for silos + return; } + $iOrgId = $oItem->Get($sAttCode); + if ($iOrgId > 0) { + if ($iOrgId != $this->Get('item_org_id')) { + $this->Set('item_org_id', $iOrgId); + if ($bUpdateOnChange) { + $this->DBUpdate(); + } + } + } }]]> @@ -193,24 +187,15 @@ Set('item_org_id', $oCurrentPerson->Get($sAttCode)); - } - } - } + $sOrgAttrCodeForPerson = UserRights::GetOwnerOrganizationAttCode('Person'); + if (is_null($sOrgAttrCodeForPerson)) { + // No need for silos + return; + } + + $oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false); + if ($oCurrentPerson) { + $this->Set('item_org_id', $oCurrentPerson->Get($sOrgAttrCodeForPerson)); } }]]> diff --git a/datamodels/2.x/itop-structure/module.itop-structure.php b/datamodels/2.x/itop-structure/module.itop-structure.php index f2d72272e6..aa3e2aa978 100644 --- a/datamodels/2.x/itop-structure/module.itop-structure.php +++ b/datamodels/2.x/itop-structure/module.itop-structure.php @@ -142,7 +142,7 @@ public static function AfterDatabaseCreation(Config $oConfiguration, $sPreviousV $sPersonClass = 'Person'; $sPersonStateAttCode = MetaModel::GetStateAttributeCode($sPersonClass); - $sPersonOwnerOrgAttCode = UserRightsProfile::GetOwnerOrganizationAttCode($sPersonClass); + $sPersonOwnerOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sPersonClass); $iClassesWithLogCount = 0; $aCreatedTriggerIds = []; @@ -177,7 +177,7 @@ public static function AfterDatabaseCreation(Config $oConfiguration, $sPreviousV ); // Filter on class owner org. if any - $sClassOwnerOrgAttCode = UserRightsProfile::GetOwnerOrganizationAttCode($sClass); + $sClassOwnerOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sClass); $oOwnerOrgExpr = empty($sClassOwnerOrgAttCode) ? null : new BinaryExpression( new FieldExpression($sPersonOwnerOrgAttCode), '=', diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php index 30a94d7431..1d0825ed4d 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php @@ -1087,10 +1087,10 @@ protected function GivenObject(string $sClass, array $aParams): DBObject * @param string $sClass * @param array $aValues * - * @return DBObject + * @return int * @throws Exception */ - protected function GivenObjectInDB($sClass, $aValues) + protected function GivenObjectInDB($sClass, $aValues):int { // Check and complete the values foreach ($aValues as $sAttCode => $oValue) { @@ -1406,4 +1406,37 @@ protected function SkipIfModuleNotPresent(string $sModule): void self::markTestSkipped("Test skipped: module '$sModule' is not present"); } } + + protected function GivenUserLoggedInWithContact(int $iContactOrgId) + { + $iContactId = $this->GivenObjectInDB('Person', [ + 'first_name' => 'TestContact', + 'name' => 'TestContact', + 'org_id' => $iContactOrgId]); + $sLogin = 'demo_test_'.uniqid(__CLASS__, true); + $iUser = $this->GivenObjectInDB('UserLocal', [ + 'login' => $sLogin, + 'password' => 'tagada-Secret,007', + 'language' => 'EN US', + 'contactid' => $iContactId, + 'profile_list' => [ + 'profileid:'.self::$aURP_Profiles['Configuration Manager'] + ] + ]); + \UserRights::Login($sLogin); + } + + protected function GivenUserLoggedInWithoutContact() + { + $sLogin = 'demo_test_'.uniqid(__CLASS__, true); + $iUser = $this->GivenObjectInDB('UserLocal', [ + 'login' => $sLogin, + 'password' => 'tagada-Secret,007', + 'language' => 'EN US', + 'profile_list' => [ + 'profileid:'.self::$aURP_Profiles['Configuration Manager'] + ] + ]); + \UserRights::Login($sLogin); + } } diff --git a/tests/php-unit-tests/unitary-tests/core/InlineImageTest.php b/tests/php-unit-tests/unitary-tests/core/InlineImageTest.php index d71055b137..84189deaac 100644 --- a/tests/php-unit-tests/unitary-tests/core/InlineImageTest.php +++ b/tests/php-unit-tests/unitary-tests/core/InlineImageTest.php @@ -59,4 +59,33 @@ public function OnFormCancelInvalidTempIdProvider() ], ]; } + + public function testSetDefaultOrgIdWhenLoggedInWithContact() + { + $iContactOrgId = $this->GivenObjectInDB('Organization', ['name' => 'TestOrg']); + $this->GivenUserLoggedInWithContact($iContactOrgId); + + $oInlineImage = \MetaModel::NewObject('InlineImage',['item_class' => 'UserRequest']); + $oInlineImage->SetDefaultOrgId(); + $this->assertEquals($iContactOrgId, $oInlineImage->Get('item_org_id'),'The org_id should be the one of the contact'); + + $oInlineImage = \MetaModel::NewObject('InlineImage',['item_class' => 'TriggerOnObjectCreate']); + $oInlineImage->SetDefaultOrgId(); + $this->assertEquals(0, $oInlineImage->Get('item_org_id'),'The org_id should be left undefined'); + } + + + public function testSetDefaultOrgIdWhenLoggedInWithoutContact() + { + $this->GivenUserLoggedInWithoutContact(); + + $oInlineImage = \MetaModel::NewObject('InlineImage',['item_class' => 'UserRequest']); + $oInlineImage->SetDefaultOrgId(); + $this->assertEquals(0, $oInlineImage->Get('item_org_id'),'The org_id should be left undefined'); + + $oInlineImage = \MetaModel::NewObject('InlineImage',['item_class' => 'TriggerOnObjectCreate']); + $oInlineImage->SetDefaultOrgId(); + $this->assertEquals(0, $oInlineImage->Get('item_org_id'),'The org_id should be left undefined'); + } + } diff --git a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php index 523fe0258f..9c812b319b 100644 --- a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php +++ b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php @@ -488,4 +488,18 @@ public function NonAdminCannotListAdminProfilesProvider(): array 'with Admins hidden' => [true, 0], ]; } + + public function testGetOwnerOrganizationAttCode() + { + $this->assertEquals('id', UserRights::GetOwnerOrganizationAttCode('Organization')); + + $this->assertEquals('org_id', UserRights::GetOwnerOrganizationAttCode('Server')); + $this->assertEquals('org_id', UserRights::GetOwnerOrganizationAttCode('UserRequest')); + + $this->assertEquals('item_org_id', UserRights::GetOwnerOrganizationAttCode('InlineImage')); + $this->assertEquals('item_org_id', UserRights::GetOwnerOrganizationAttCode('Attachment')); + + $this->assertNull(UserRights::GetOwnerOrganizationAttCode('TriggerOnObjectCreation')); + $this->assertNull(UserRights::GetOwnerOrganizationAttCode('lnkPersonToTeam')); + } } diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/TestAttachment.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/TestAttachment.php index 8485c956f0..9745b6d682 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/TestAttachment.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/TestAttachment.php @@ -69,4 +69,46 @@ public function OnRemoveAttachment(EventData $oData) $oDocument = $oAttachment->Get('contents'); $this->sRemoveAttachmentName = $oDocument->GetFileName(); } + + + public function testSetItemOnObjectWithDefinedOrganization() + { + $iOrgId = $this->GivenObjectInDB('Organization', ['name' => 'TestOrg']); + $oUserRequest = $this->GivenObject('UserRequest', ['title' => 'TestUserRequest', 'org_id'=>$iOrgId]); + + $oAttachment = new \Attachment(); + $oAttachment->SetItem($oUserRequest); + $this->assertEquals($iOrgId, $oAttachment->Get('item_org_id'),'The org_id should be the one of the contact'); + } + + + public function testSetItemOnObjectWithoutDefinedOrganization() + { + $oUserRequest = $this->GivenObject('TriggerOnObjectCreate', ['target_class' => 'UserRequest','description'=>'TestUserRequest']); + + $oAttachment = new \Attachment(); + $oAttachment->SetItem($oUserRequest); + $this->assertEquals(0, $oAttachment->Get('item_org_id'),'The org_id should be the one of the contact'); + } + + + public function testSetDefaultOrgIdWhenLoggedInWithContact() + { + $iContactOrgId = $this->GivenObjectInDB('Organization', ['name' => 'TestOrg']); + $this->GivenUserLoggedInWithContact($iContactOrgId); + + $oAttachment = new \Attachment(); + $oAttachment->SetDefaultOrgId(); + $this->assertEquals($iContactOrgId, $oAttachment->Get('item_org_id'),'The org_id should be the one of the contact'); + } + + + public function testSetDefaultOrgIdWhenLoggedInWithoutContact() + { + $this->GivenUserLoggedInWithoutContact(); + + $oAttachment = new \Attachment(); + $oAttachment->SetDefaultOrgId(); + $this->assertEquals(0, $oAttachment->Get('item_org_id'),'The org_id should be left undefined'); + } }