diff --git a/addons/userrights/userrightsprofile.class.inc.php b/addons/userrights/userrightsprofile.class.inc.php
index 80f9acc0c..3c3725f13 100644
--- a/addons/userrights/userrightsprofile.class.inc.php
+++ b/addons/userrights/userrightsprofile.class.inc.php
@@ -933,8 +933,9 @@ class UserRightsProfile extends UserRightsAddOnAPI
}
/**
- * Find out which attribute is corresponding the the dimension 'owner org'
- * returns null if no such attribute has been found (no filtering should occur)
+ * @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)
*/
public static function GetOwnerOrganizationAttCode($sClass)
{
diff --git a/addons/userrights/userrightsprofile.db.class.inc.php b/addons/userrights/userrightsprofile.db.class.inc.php
index 12a8874c9..1c890baad 100644
--- a/addons/userrights/userrightsprofile.db.class.inc.php
+++ b/addons/userrights/userrightsprofile.db.class.inc.php
@@ -580,10 +580,10 @@ class UserRightsProfile extends UserRightsAddOnAPI
/**
* Read and cache organizations allowed to the given user
*
- * @param $oUser
- * @param $sClass (not used here but can be used in overloads)
+ * @param User $oUser
+ * @param string $sClass (not used here but can be used in overloads)
*
- * @return array
+ * @return array keys of the User allowed org
* @throws \CoreException
* @throws \Exception
*/
diff --git a/application/applicationextension.inc.php b/application/applicationextension.inc.php
index 14f271258..13a9ecdf7 100644
--- a/application/applicationextension.inc.php
+++ b/application/applicationextension.inc.php
@@ -2015,6 +2015,8 @@ class RestUtils
*
* @return DBObject The object found
* @throws Exception If the input structure is not valid or it could not find exactly one object
+ *
+ * @see DBObject::CheckChangedExtKeysValues() generic method to check that we can access the linked object isn't used in that use case because values can be literal, OQL, friendlyname
*/
public static function FindObjectFromKey($sClass, $key, $bAllowNullValue = false)
{
diff --git a/application/cmdbabstract.class.inc.php b/application/cmdbabstract.class.inc.php
index 3277edc9d..3c8d9e2be 100644
--- a/application/cmdbabstract.class.inc.php
+++ b/application/cmdbabstract.class.inc.php
@@ -5365,6 +5365,11 @@ EOF
'errors' => ''.($bResult ? '' : implode('
', $aErrorsToDisplay)).'
',
);
if ($bResult && (!$bPreview)) {
+ // doing the check will load multiple times same objects :/
+ // but it shouldn't cost too much on execution time
+ // user can mitigate by selecting less extkeys/lnk to set and/or less objects to update 🤷‍♂️
+ $oObj->CheckChangedExtKeysValues();
+
$oObj->DBUpdate();
}
}
diff --git a/application/exceptions/InvalidExternalKeyValueException.php b/application/exceptions/InvalidExternalKeyValueException.php
new file mode 100644
index 000000000..cd8a08990
--- /dev/null
+++ b/application/exceptions/InvalidExternalKeyValueException.php
@@ -0,0 +1,36 @@
+GetKey();
+ $aContextData[self::ENUM_PARAMS_ATTCODE] = $sAttCode;
+ $aContextData[self::ENUM_PARAMS_ATTVALUE] = $oObject->Get($sAttCode);
+
+ $oCurrentUser = UserRights::GetUserObject();
+ if (false === is_null($oCurrentUser)) {
+ $aContextData[self::ENUM_PARAMS_USER] = get_class($oCurrentUser) . '::' . $oCurrentUser->GetKey();
+ }
+
+ parent::__construct('Attribute pointing to an object that is either non existing or not readable by the current user', $aContextData, '', $oPrevious);
+ }
+
+ public function GetAttCode(): string
+ {
+ return $this->getContextData()[self::ENUM_PARAMS_ATTCODE];
+ }
+
+ public function GetAttValue(): string
+ {
+ return $this->getContextData()[self::ENUM_PARAMS_ATTVALUE];
+ }
+}
diff --git a/application/utils.inc.php b/application/utils.inc.php
index 6b8269104..66c08e51d 100644
--- a/application/utils.inc.php
+++ b/application/utils.inc.php
@@ -3029,6 +3029,7 @@ HTML;
*
* @return bool if string null or empty
* @since 3.0.2 N°5302
+ * @since 2.7.10 N°6458 add method in the 2.7 branch
*/
public static function IsNullOrEmptyString(?string $sString): bool
{
@@ -3044,6 +3045,7 @@ HTML;
*
* @return bool if string is not null and not empty
* @since 3.0.2 N°5302
+ * @since 2.7.10 N°6458 add method in the 2.7 branch
*/
public static function IsNotNullOrEmptyString(?string $sString): bool
{
diff --git a/core/dbobject.class.php b/core/dbobject.class.php
index 49cda69ee..2770566c3 100644
--- a/core/dbobject.class.php
+++ b/core/dbobject.class.php
@@ -2512,6 +2512,83 @@ abstract class DBObject implements iDisplay
return array($this->m_bCheckStatus, $this->m_aCheckIssues, $this->m_bSecurityIssue);
}
+ /**
+ * Checks for extkey attributes values. This will throw exception on non-existing as well as non-accessible objects (silo, scopes).
+ * That's why the test is done for all users including Administrators
+ *
+ * Note that due to perf issues, this isn't called directly by the ORM, but has to be called by consumers when possible.
+ *
+ * @param callable(string, string):bool|null $oIsObjectLoadableCallback Override to check if object is accessible.
+ * Parameters are object class and key
+ * Return value should be false if cannot access object, true otherwise
+ * @return void
+ *
+ * @throws ArchivedObjectException
+ * @throws CoreException if cannot get object attdef list
+ * @throws CoreUnexpectedValue
+ * @throws InvalidExternalKeyValueException
+ * @throws MySQLException
+ * @throws SecurityException if one extkey is pointing to an invalid value
+ *
+ * @link https://github.com/Combodo/iTop/security/advisories/GHSA-245j-66p9-pwmh
+ * @since 2.7.10 3.0.4 3.1.1 3.2.0 N°6458
+ *
+ * @see \RestUtils::FindObjectFromKey for the same check in the REST endpoint
+ */
+ final public function CheckChangedExtKeysValues(callable $oIsObjectLoadableCallback = null)
+ {
+ if (is_null($oIsObjectLoadableCallback)) {
+ $oIsObjectLoadableCallback = function ($sClass, $sId) {
+ $oRemoteObject = MetaModel::GetObject($sClass, $sId, false);
+ if (is_null($oRemoteObject)) {
+ return false;
+ }
+ return true;
+ };
+ }
+
+ $aChanges = $this->ListChanges();
+ $aAttCodesChanged = array_keys($aChanges);
+ foreach ($aAttCodesChanged as $sAttDefCode) {
+ $oAttDef = MetaModel::GetAttributeDef(get_class($this), $sAttDefCode);
+
+ if ($oAttDef instanceof AttributeLinkedSetIndirect) {
+ /** @var ormLinkSet $oOrmSet */
+ $oOrmSet = $this->Get($sAttDefCode);
+ while ($oLnk = $oOrmSet->Fetch()) {
+ $oLnk->CheckChangedExtKeysValues($oIsObjectLoadableCallback);
+ }
+ continue;
+ }
+
+ /** @noinspection PhpConditionCheckedByNextConditionInspection */
+ /** @noinspection NotOptimalIfConditionsInspection */
+ if (($oAttDef instanceof AttributeHierarchicalKey) || ($oAttDef instanceof AttributeExternalKey)) {
+ $sRemoteObjectClass = $oAttDef->GetTargetClass();
+ $sRemoteObjectKey = $this->Get($sAttDefCode);
+ } else if ($oAttDef instanceof AttributeObjectKey) {
+ $sRemoteObjectClassAttCode = $oAttDef->Get('class_attcode');
+ $sRemoteObjectClass = $this->Get($sRemoteObjectClassAttCode);
+ $sRemoteObjectKey = $this->Get($sAttDefCode);
+ } else {
+ continue;
+ }
+
+ /** @noinspection NotOptimalIfConditionsInspection */
+ /** @noinspection TypeUnsafeComparisonInspection */
+ if (utils::IsNullOrEmptyString($sRemoteObjectClass)
+ || utils::IsNullOrEmptyString($sRemoteObjectKey)
+ || ($sRemoteObjectKey == 0) // non-strict comparison as we might have bad surprises
+ ) {
+ continue;
+ }
+
+ if (false === $oIsObjectLoadableCallback($sRemoteObjectClass, $sRemoteObjectKey)) {
+ throw new InvalidExternalKeyValueException($this, $sAttDefCode);
+ }
+ }
+ }
+
/**
* Check if it is allowed to delete the existing object from the database
*
@@ -2655,11 +2732,11 @@ abstract class DBObject implements iDisplay
* @api
* @api-advanced
*
- * @see \DBObject::ListPreviousValuesForUpdatedAttributes() to get previous values anywhere in the CRUD stack
- * @see https://www.itophub.io/wiki/page?id=latest%3Acustomization%3Asequence_crud iTop CRUD stack documentation
- * @return array attname => currentvalue List the attributes that have been changed using {@see DBObject::Set()}.
+ * @return array attcode => currentvalue List the attributes that have been changed using {@see DBObject::Set()}.
* Reset during {@see DBObject::DBUpdate()}
* @throws Exception
+ * @see \DBObject::ListPreviousValuesForUpdatedAttributes() to get previous values anywhere in the CRUD stack
+ * @see https://www.itophub.io/wiki/page?id=latest%3Acustomization%3Asequence_crud iTop CRUD stack documentation
* @uses m_aCurrValues
*/
public function ListChanges()
@@ -3098,6 +3175,8 @@ abstract class DBObject implements iDisplay
* @throws \CoreWarning
* @throws \MySQLException
* @throws \OQLException
+ *
+ * @see DBWrite
*/
public function DBInsertNoReload()
{
@@ -3326,13 +3405,13 @@ abstract class DBObject implements iDisplay
* Update an object in DB
*
* @api
- * @see DBObject::DBWrite()
- *
* @return int object key
*
* @throws \CoreException
* @throws \CoreCannotSaveObjectException if CheckToWrite() returns issues
* @throws \Exception
+ *
+ * @see DBObject::DBWrite()
*/
public function DBUpdate()
{
@@ -3784,13 +3863,18 @@ abstract class DBObject implements iDisplay
/**
* Make the current changes persistent - clever wrapper for Insert or Update
- *
- * @api
+ *
+ * @api
*
* @return int
- *
- * @throws \CoreCannotSaveObjectException
- * @throws \CoreException
+ *
+ * @throws ArchivedObjectException
+ * @throws CoreCannotSaveObjectException
+ * @throws CoreException
+ * @throws CoreUnexpectedValue
+ * @throws CoreWarning
+ * @throws MySQLException
+ * @throws OQLException
*/
public function DBWrite()
{
diff --git a/core/metamodel.class.php b/core/metamodel.class.php
index 3cca4dfe7..9af99c36f 100644
--- a/core/metamodel.class.php
+++ b/core/metamodel.class.php
@@ -1445,8 +1445,10 @@ abstract class MetaModel
*
* @return AttributeDefinition[]
* @throws \CoreException
+ *
+ * @see GetAttributesList for attcode list
*/
- final static public function ListAttributeDefs($sClass)
+ final public static function ListAttributeDefs($sClass)
{
self::_check_subclass($sClass);
return self::$m_aAttribDefs[$sClass];
@@ -1459,8 +1461,10 @@ abstract class MetaModel
* @param string[] $aDesiredAttTypes Array of AttributeDefinition classes to filter the list on
* @param string|null $sListCode If provided, attributes will be limited to those in this zlist
*
- * @return array
+ * @return string[] list of attcodes
* @throws \CoreException
+ *
+ * @see ListAttributeDefs to get AttributeDefinition array instead
*/
final public static function GetAttributesList(string $sClass, array $aDesiredAttTypes = [], ?string $sListCode = null)
{
diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php
index 160e4c93e..aea8658e4 100644
--- a/core/userrights.class.inc.php
+++ b/core/userrights.class.inc.php
@@ -1121,9 +1121,7 @@ class UserRights
}
/**
- * Return the current user login or an empty string if nobody connected.
- *
- * @return string
+ * @return string connected {@see User} login field value, otherwise empty string
*/
public static function GetUser()
{
@@ -1571,9 +1569,9 @@ class UserRights
/**
* @param string $sClass
- * @param int $iActionCode
- * @param \DBObjectSet $oInstanceSet
- * @param \User $oUser
+ * @param int $iActionCode see UR_ACTION_* constants
+ * @param DBObjectSet $oInstanceSet
+ * @param User $oUser
*
* @return int (UR_ALLOWED_YES|UR_ALLOWED_NO|UR_ALLOWED_DEPENDS)
* @throws \CoreException
diff --git a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php
index d32822ac1..21516082a 100644
--- a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php
+++ b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php
@@ -32,6 +32,7 @@ use Combodo\iTop\Form\Form;
use Combodo\iTop\Form\FormManager;
use Combodo\iTop\Portal\Helper\ApplicationHelper;
use Combodo\iTop\Portal\Helper\ObjectFormHandlerHelper;
+use Combodo\iTop\Portal\Helper\SecurityHelper;
use CoreCannotSaveObjectException;
use DBObject;
use DBObjectSearch;
@@ -43,6 +44,7 @@ use DOMXPath;
use Exception;
use ExceptionLog;
use InlineImage;
+use InvalidExternalKeyValueException;
use IssueLog;
use LogChannels;
use MetaModel;
@@ -50,6 +52,7 @@ use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\HttpException;
use UserRights;
use utils;
+use const UR_ACTION_READ;
/**
* Description of ObjectFormManager
@@ -1133,6 +1136,9 @@ class ObjectFormManager extends FormManager
$bWasModified = $this->oObject->IsModified();
$bActivateTriggers = (!$bIsNew && $bWasModified);
+ /** @var SecurityHelper $oSecurityHelper */
+ $oSecurityHelper = $this->oContainer->get('security_helper');
+
// Forcing allowed writing on the object if necessary. This is used in some particular cases.
$bAllowWrite = $this->oFormHandlerHelper->getSecurityHelper()->IsActionAllowed($bIsNew ? UR_ACTION_CREATE : UR_ACTION_MODIFY, $sObjectClass, $this->oObject->GetKey());
if ($bAllowWrite) {
@@ -1142,12 +1148,15 @@ class ObjectFormManager extends FormManager
// Writing object to DB
try
{
+ $this->oObject->CheckChangedExtKeysValues(function ($sClass, $sId) use ($oSecurityHelper): bool {
+ return $oSecurityHelper->IsActionAllowed(UR_ACTION_READ, $sClass, $sId);
+ });
$this->oObject->DBWrite();
- }
- catch (CoreCannotSaveObjectException $e) {
+ } catch (CoreCannotSaveObjectException $e) {
throw new Exception($e->getHtmlMessage());
- }
- catch (Exception $e) {
+ } catch (InvalidExternalKeyValueException $e) {
+ throw new Exception($e->getIssue());
+ } catch (Exception $e) {
$aContext = [
'origin' => __CLASS__.'::'.__METHOD__,
'obj_class' => get_class($this->oObject),
diff --git a/lib/composer/autoload_classmap.php b/lib/composer/autoload_classmap.php
index b8abe1782..5cdf3879c 100644
--- a/lib/composer/autoload_classmap.php
+++ b/lib/composer/autoload_classmap.php
@@ -700,6 +700,7 @@ return array(
'IntervalOqlExpression' => $baseDir . '/core/oql/oqlquery.class.inc.php',
'Introspection' => $baseDir . '/core/introspection.class.inc.php',
'InvalidConfigParamException' => $baseDir . '/application/exceptions/InvalidConfigParamException.php',
+ 'InvalidExternalKeyValueException' => $baseDir . '/application/exceptions/InvalidExternalKeyValueException.php',
'InvalidPasswordAttributeOneWayPassword' => $baseDir . '/application/exceptions/InvalidPasswordAttributeOneWayPassword.php',
'IssueLog' => $baseDir . '/core/log.class.inc.php',
'ItopCounter' => $baseDir . '/core/counter.class.inc.php',
diff --git a/lib/composer/autoload_static.php b/lib/composer/autoload_static.php
index 97194965a..0487e482d 100644
--- a/lib/composer/autoload_static.php
+++ b/lib/composer/autoload_static.php
@@ -1064,6 +1064,7 @@ class ComposerStaticInit7f81b4a2a468a061c306af5e447a9a9f
'IntervalOqlExpression' => __DIR__ . '/../..' . '/core/oql/oqlquery.class.inc.php',
'Introspection' => __DIR__ . '/../..' . '/core/introspection.class.inc.php',
'InvalidConfigParamException' => __DIR__ . '/../..' . '/application/exceptions/InvalidConfigParamException.php',
+ 'InvalidExternalKeyValueException' => __DIR__ . '/../..' . '/application/exceptions/InvalidExternalKeyValueException.php',
'InvalidPasswordAttributeOneWayPassword' => __DIR__ . '/../..' . '/application/exceptions/InvalidPasswordAttributeOneWayPassword.php',
'IssueLog' => __DIR__ . '/../..' . '/core/log.class.inc.php',
'ItopCounter' => __DIR__ . '/../..' . '/core/counter.class.inc.php',
diff --git a/sources/Controller/Base/Layout/ObjectController.php b/sources/Controller/Base/Layout/ObjectController.php
index ccd5cfe51..83770fcd5 100644
--- a/sources/Controller/Base/Layout/ObjectController.php
+++ b/sources/Controller/Base/Layout/ObjectController.php
@@ -18,8 +18,8 @@ use Combodo\iTop\Application\UI\Base\Component\QuickCreate\QuickCreateHelper;
use Combodo\iTop\Application\UI\Base\Layout\Object\ObjectSummary;
use Combodo\iTop\Application\UI\Base\Layout\PageContent\PageContentFactory;
use Combodo\iTop\Controller\AbstractController;
-use Combodo\iTop\Service\Router\Router;
use Combodo\iTop\Service\Base\ObjectRepository;
+use Combodo\iTop\Service\Router\Router;
use CoreCannotSaveObjectException;
use DeleteException;
use Dict;
@@ -428,6 +428,8 @@ JS;
'transaction_id' => $sTransactionId,
],
]);
+
+ $oObj->CheckChangedExtKeysValues();
$oObj->DBInsertNoReload();
@@ -632,6 +634,8 @@ JS;
throw new CoreCannotSaveObjectException(array('id' => $oObj->GetKey(), 'class' => $sClass, 'issues' => $aErrors));
}
+ $oObj->CheckChangedExtKeysValues();
+
// Transactions are now handled in DBUpdate
$oObj->SetContextSection('temporary_objects', [
'finalize' => [
diff --git a/tests/php-unit-tests/README.md b/tests/php-unit-tests/README.md
index 0b5d06b3d..3f556426f 100644
--- a/tests/php-unit-tests/README.md
+++ b/tests/php-unit-tests/README.md
@@ -59,6 +59,16 @@ Fix that in the XML configuration in the PHP section
```
+
+### Measure the time spent in a test
+
+Simply cut'n paste the following line at several places within the test function:
+
+```php
+if (isset($fStarted)) {echo 'L'.__LINE__.': '.round(microtime(true) - $fStarted, 3)."\n";} $fStarted = microtime(true);
+```
+
+
### Understand tests interactions
With PHPStorm, select two tests, right click to get the context menu, then `run`.
@@ -119,4 +129,3 @@ This won't work because the comment MUST start with `/**` (two stars) to be cons
Therefore, if the tests are isolated, then `setupBeforeClass` will be called as often as `setUp`.
-This has been proven with [`runClassInSeparateProcessTest.php`](experiments/runClassInSeparateProcessTest.php)
\ 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 422dd357e..893456630 100644
--- a/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php
+++ b/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php
@@ -749,7 +749,7 @@ abstract class ItopDataTestCase extends ItopTestCase
* @return array
* @throws Exception
*/
- protected function AddCIToTicket($oCI, $oTicket, $sImpactCode)
+ protected function AddCIToTicket($oCI, $oTicket, $sImpactCode = 'manual')
{
$oNewLink = new lnkFunctionalCIToTicket();
$oNewLink->Set('functionalci_id', $oCI->GetKey());
diff --git a/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php b/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php
index 51b93a416..984a3f570 100644
--- a/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php
+++ b/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php
@@ -17,22 +17,23 @@
// along with iTop. If not, see
//
-/**
- * Created by PhpStorm.
- * User: Eric
- * Date: 02/10/2017
- * Time: 13:58
- */
-
namespace Combodo\iTop\Test\UnitTest\Core;
+use Attachment;
use Combodo\iTop\Service\Events\EventData;
use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
use CoreException;
use DBObject;
+use InvalidExternalKeyValueException;
use lnkContactToFunctionalCI;
use lnkPersonToTeam;
use MetaModel;
+use Organization;
+use Person;
+use Team;
+use User;
+use UserRights;
+use utils;
/**
@@ -41,6 +42,7 @@ use MetaModel;
class DBObjectTest extends ItopDataTestCase
{
const CREATE_TEST_ORG = true;
+ const INVALID_OBJECT_KEY = 123456789;
// Counts
public $aReloadCount = [];
@@ -433,6 +435,267 @@ class DBObjectTest extends ItopDataTestCase
}
}
+ private function GetAlwaysTrueCallback(): callable
+ {
+ return static function () {
+ return true;
+ };
+ }
+
+ private function GetAlwaysFalseCallback(): callable
+ {
+ return static function () {
+ return false;
+ };
+ }
+
+ /**
+ * @covers DBObject::CheckChangedExtKeysValues()
+ * @runInSeparateProcess MetaModel::GetObject returning wrong values :(
+ */
+ public function testCheckExtKeysSiloOnAttributeExternalKey()
+ {
+ //--- Preparing data...
+ $oAlwaysTrueCallback = $this->GetAlwaysTrueCallback();
+ $oAlwaysFalseCallback = $this->GetAlwaysFalseCallback();
+
+ /** @var Organization $oDemoOrg */
+ $oDemoOrg = MetaModel::GetObjectByName(Organization::class, 'Demo');
+ /** @var Organization $oMyCompanyOrg */
+ $oMyCompanyOrg = MetaModel::GetObjectByName(Organization::class, 'My Company/Department');
+
+ /** @var Person $oPersonOfDemoOrg */
+ $oPersonOfDemoOrg = MetaModel::GetObjectByName(Person::class, 'Agatha Christie');
+ /** @var Person $oPersonOfMyCompanyOrg */
+ $oPersonOfMyCompanyOrg = MetaModel::GetObjectByName(Person::class, 'My first name My last name');
+
+ $sConfigurationManagerProfileId = 3; // Access to Person objects
+ $oUserWithAllowedOrgs = $this->CreateDemoOrgUser($oDemoOrg, $sConfigurationManagerProfileId);
+
+ $oAdminUser = MetaModel::GetObjectByName(User::class, 'admin', false);
+ if (is_null($oAdminUser)) {
+ $oAdminUser = $this->CreateUser('admin', 1);
+ }
+
+ /** @var Person $oPersonObject */
+ $oPersonObject = $this->CreatePerson(0, $oMyCompanyOrg->GetKey());
+
+ //--- Now we can do some tests !
+ UserRights::Login($oUserWithAllowedOrgs->Get('login'));
+ $this->ResetMetaModelQueyCacheGetObject();
+
+ try {
+ $oPersonObject->CheckChangedExtKeysValues();
+ } catch (InvalidExternalKeyValueException $eCannotSave) {
+ $this->fail('Should skip external keys already written in Database');
+ }
+
+ $oPersonObject->Set('manager_id', $oPersonOfDemoOrg->GetKey());
+ try {
+ $oPersonObject->CheckChangedExtKeysValues();
+ } catch (InvalidExternalKeyValueException $eCannotSave) {
+ $this->fail('Should allow objects in the same org as the current user');
+ }
+
+ try {
+ $oPersonObject->CheckChangedExtKeysValues($oAlwaysFalseCallback);
+ $this->fail('Should consider the callback returning "false"');
+ } catch (InvalidExternalKeyValueException $eCannotSave) {
+ // Ok, the exception was expected
+ }
+
+ $oPersonObject->Set('manager_id', $oPersonOfMyCompanyOrg->GetKey());
+ try {
+ $oPersonObject->CheckChangedExtKeysValues();
+ $this->fail('Should not allow objects not being in the allowed orgs of the current user');
+ } catch (InvalidExternalKeyValueException $eCannotSave) {
+ $this->assertEquals('manager_id', $eCannotSave->GetAttCode(), 'Should report the wrong external key attcode');
+ $this->assertEquals($oMyCompanyOrg->GetKey(), $eCannotSave->GetAttValue(), 'Should report the unauthorized external key value');
+ }
+
+ try {
+ $oPersonObject->CheckChangedExtKeysValues($oAlwaysTrueCallback);
+ } catch (InvalidExternalKeyValueException $eCannotSave) {
+ $this->fail('Should consider the callback returning "true"');
+ }
+
+ UserRights::Logoff();
+ $this->ResetMetaModelQueyCacheGetObject();
+
+ UserRights::Login($oAdminUser->Get('login'));
+ $oPersonObject->CheckChangedExtKeysValues();
+ $this->assertTrue(true, 'Admin user can create objects in any org');
+ }
+
+ /**
+ * @covers DBObject::CheckChangedExtKeysValues()
+ * @runInSeparateProcess MetaModel::GetObject returning wrong values :(
+ */
+ public function testCheckExtKeysOnAttributeLinkedSetIndirect()
+ {
+ //--- Preparing data...
+ /** @var Organization $oDemoOrg */
+ $oDemoOrg = MetaModel::GetObjectByName(Organization::class, 'Demo');
+ /** @var Person $oPersonOnItDepartmentOrg */
+ $oPersonOnItDepartmentOrg = MetaModel::GetObjectByName(Person::class, 'Anna Gavalda');
+ /** @var Person $oPersonOnDemoOrg */
+ $oPersonOnDemoOrg = MetaModel::GetObjectByName(Person::class, 'Claude Monet');
+
+ $sConfigManagerProfileId = 3; // access to Team and Contact objects
+ $oUserWithAllowedOrgs = $this->CreateDemoOrgUser($oDemoOrg, $sConfigManagerProfileId);
+
+ //--- Now we can do some tests !
+ UserRights::Login($oUserWithAllowedOrgs->Get('login'));
+ $this->ResetMetaModelQueyCacheGetObject();
+
+ $oTeam = MetaModel::NewObject(Team::class, [
+ 'name' => 'The A Team',
+ 'org_id' => $oDemoOrg->GetKey()
+ ]);
+
+ // Part 1 - Test with an invalid id (non-existing object)
+ //
+ $oPersonLinks = \DBObjectSet::FromScratch(lnkPersonToTeam::class);
+ $oPersonLinks->AddObject(MetaModel::NewObject(lnkPersonToTeam::class, [
+ 'person_id' => self::INVALID_OBJECT_KEY,
+ ]));
+ $oTeam->Set('persons_list', $oPersonLinks);
+
+ try {
+ $oTeam->CheckChangedExtKeysValues();
+ $this->fail('An unknown object should be detected as invalid');
+ } catch (InvalidExternalKeyValueException $e) {
+ // we are getting the exception on the lnk class
+ // In consequence attcode is `lnkPersonToTeam.person_id` instead of `Team.persons_list`
+ $this->assertEquals('person_id', $e->GetAttCode(), 'The reported attcode should be the external key on the link');
+ $this->assertEquals(self::INVALID_OBJECT_KEY, $e->GetAttValue(), 'The reported value should be the external key on the link');
+ }
+
+ try {
+ $oTeam->CheckChangedExtKeysValues($this->GetAlwaysTrueCallback());
+ } catch (InvalidExternalKeyValueException $e) {
+ $this->fail('Should have no error when callback returns true');
+ }
+
+ // Part 2 - Test with an allowed object
+ //
+ $oPersonLinks = \DBObjectSet::FromScratch(lnkPersonToTeam::class);
+ $oPersonLinks->AddObject(MetaModel::NewObject(lnkPersonToTeam::class, [
+ 'person_id' => $oPersonOnDemoOrg->GetKey(),
+ ]));
+ $oTeam->Set('persons_list', $oPersonLinks);
+
+ try {
+ $oTeam->CheckChangedExtKeysValues();
+ } catch (InvalidExternalKeyValueException $e) {
+ $this->fail('An authorized object should be detected as valid');
+ }
+
+ try {
+ $oTeam->CheckChangedExtKeysValues($this->GetAlwaysFalseCallback());
+ $this->fail('Should cascade the callback result when it is "false"');
+ } catch (InvalidExternalKeyValueException $e) {
+ // Ok, the exception was expected
+ }
+
+ // Part 3 - Test with a not allowed object
+ //
+ $oPersonLinks = \DBObjectSet::FromScratch(lnkPersonToTeam::class);
+ $oPersonLinks->AddObject(MetaModel::NewObject(lnkPersonToTeam::class, [
+ 'person_id' => $oPersonOnItDepartmentOrg->GetKey(),
+ ]));
+ $oTeam->Set('persons_list', $oPersonLinks);
+
+ try {
+ $oTeam->CheckChangedExtKeysValues();
+ $this->fail('An unauthorized object should be detected as invalid');
+ }
+ catch (InvalidExternalKeyValueException $e) {
+ // Ok, the exception was expected
+ }
+
+ try {
+ $oTeam->CheckChangedExtKeysValues($this->GetAlwaysTrueCallback());
+ } catch (InvalidExternalKeyValueException $e) {
+ $this->fail('Should cascade the callback result when it is "true"');
+ }
+
+ $oTeam->DBInsert(); // persisting invalid value and resets the object changed values
+ try {
+ $oTeam->CheckChangedExtKeysValues();
+ }
+ catch (InvalidExternalKeyValueException $e) {
+ $this->fail('An unauthorized value should be ignored when it is not being modified');
+ }
+ }
+
+ /**
+ * @covers DBObject::CheckChangedExtKeysValues()
+ * @runInSeparateProcess MetaModel::GetObject returning wrong values :(
+ */
+ public function testCheckExtKeysSiloOnAttributeObjectKey()
+ {
+ //--- Preparing data...
+ /** @var Organization $oDemoOrg */
+ $oDemoOrg = MetaModel::GetObjectByName(Organization::class, 'Demo');
+ /** @var Person $oPersonOnItDepartmentOrg */
+ $oPersonOnItDepartmentOrg = MetaModel::GetObjectByName(Person::class, 'Anna Gavalda');
+ /** @var Person $oPersonOnDemoOrg */
+ $oPersonOnDemoOrg = MetaModel::GetObjectByName(Person::class, 'Claude Monet');
+
+ $sConfigManagerProfileId = 3; // access to Team and Contact objects
+ $oUserWithAllowedOrgs = $this->CreateDemoOrgUser($oDemoOrg, $sConfigManagerProfileId);
+
+ //--- Now we can do some tests !
+ UserRights::Login($oUserWithAllowedOrgs->Get('login'));
+ $this->ResetMetaModelQueyCacheGetObject();
+
+ $oAttachment = MetaModel::NewObject(Attachment::class, [
+ 'item_class' => Person::class,
+ 'item_id' => $oPersonOnDemoOrg->GetKey(),
+ ]);
+ try {
+ $oAttachment->CheckChangedExtKeysValues();
+ } catch (InvalidExternalKeyValueException $e) {
+ $this->fail('Should be allowed to create an attachment pointing to a ticket in the allowed org list');
+ }
+
+ $oAttachment = MetaModel::NewObject(Attachment::class, [
+ 'item_class' => Person::class,
+ 'item_id' => $oPersonOnItDepartmentOrg->GetKey(),
+ ]);
+ $this->ResetMetaModelQueyCacheGetObject();
+ try {
+ $oAttachment->CheckChangedExtKeysValues();
+ $this->fail('There should be an error on attachment pointing to a non allowed org object');
+ } catch (InvalidExternalKeyValueException $e) {
+ $this->assertEquals('item_id', $e->GetAttCode(), 'Should report the object key attribute');
+ $this->assertEquals($oPersonOnItDepartmentOrg->GetKey(), $e->GetAttValue(), 'Should report the object key value');
+ }
+ }
+
+ /**
+ * Helper to reset the metamodel cache
+ * We might need to create something generic and add it to {@see UserRights::Logoff()} ?
+ */
+ private function ResetMetaModelQueyCacheGetObject() {
+ $this->SetNonPublicStaticProperty(MetaModel::class, 'aQueryCacheGetObject', []);
+ }
+
+ private function CreateDemoOrgUser(Organization $oDemoOrg, string $sProfileId): User
+ {
+ utils::GetConfig()->SetModuleSetting('authent-local', 'password_validation.pattern', '');
+ $oUserWithAllowedOrgs = $this->CreateContactlessUser('demo_test_' . __CLASS__, $sProfileId);
+ /** @var \URP_UserOrg $oUserOrg */
+ $oUserOrg = \MetaModel::NewObject('URP_UserOrg', ['allowed_org_id' => $oDemoOrg->GetKey(),]);
+ $oAllowedOrgList = $oUserWithAllowedOrgs->Get('allowed_org_list');
+ $oAllowedOrgList->AddItem($oUserOrg);
+ $oUserWithAllowedOrgs->Set('allowed_org_list', $oAllowedOrgList);
+ $oUserWithAllowedOrgs->DBWrite();
+
+ return $oUserWithAllowedOrgs;
+ }
+
/**
* Test attribute integer incrementation.
*