From 07b904ee1b78690212352e8224ada7ecdbe0a75d Mon Sep 17 00:00:00 2001 From: "denis.flaven@combodo.com" Date: Thu, 27 Feb 2025 12:17:52 +0100 Subject: [PATCH 1/5] =?UTF-8?q?N=C2=B08231=20-=20making=20rest=20api=20log?= =?UTF-8?q?s=20more=20readable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- application/applicationextension.inc.php | 13 ++ core/attributedef.class.inc.php | 8 +- core/restservices.class.inc.php | 106 ++++++++++- .../core/Delta/delta_test_sanitize_output.xml | 116 +++++++++++++ .../core/RestServicesSanitizeOutputTest.php | 164 ++++++++++++++++++ .../unitary-tests/core/RestServicesTest.php | 125 +++++++++++++ webservices/rest.php | 13 +- 7 files changed, 536 insertions(+), 9 deletions(-) create mode 100644 tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml create mode 100644 tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php create mode 100644 tests/php-unit-tests/unitary-tests/core/RestServicesTest.php diff --git a/application/applicationextension.inc.php b/application/applicationextension.inc.php index aeed05e8b..8003e2266 100644 --- a/application/applicationextension.inc.php +++ b/application/applicationextension.inc.php @@ -1316,6 +1316,11 @@ interface iRestServiceProvider public function ExecOperation($sVersion, $sVerb, $aParams); } +interface iRestInputSanitizer +{ + public function SanitizeJsonInput(string $sJsonInput): string; +} + /** * Minimal REST response structure. Derive this structure to add response data and error codes. * @@ -1405,6 +1410,14 @@ class RestResult * @api */ public $message; + + /** + * Sanitize the content of this result to hide sensitive information + */ + public function SanitizeContent() + { + // The default implementation does nothing + } } /** diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index fa61300da..238a6183e 100644 --- a/core/attributedef.class.inc.php +++ b/core/attributedef.class.inc.php @@ -138,7 +138,7 @@ abstract class AttributeDefinition protected $aCSSClasses; - public function GetType() + public function GetType() { return Dict::S('Core:'.get_class($this)); } @@ -3775,7 +3775,7 @@ class AttributeFinalClass extends AttributeString */ class AttributePassword extends AttributeString implements iAttributeNoGroupBy { - const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW; + const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW; /** * Useless constructor, but if not present PHP 7.4.0/7.4.1 is crashing :( (N°2329) @@ -3851,7 +3851,7 @@ class AttributePassword extends AttributeString implements iAttributeNoGroupBy */ class AttributeEncryptedString extends AttributeString implements iAttributeNoGroupBy { - const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW; + const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW; static $sKey = null; // Encryption key used for all encrypted fields static $sLibrary = null; // Encryption library used for all encrypted fields @@ -9243,7 +9243,7 @@ class AttributeSubItem extends AttributeDefinition */ class AttributeOneWayPassword extends AttributeDefinition implements iAttributeNoGroupBy { - const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW; + const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW; /** * Useless constructor, but if not present PHP 7.4.0/7.4.1 is crashing :( (N°2329) diff --git a/core/restservices.class.inc.php b/core/restservices.class.inc.php index dbcc29e6e..05d8942aa 100644 --- a/core/restservices.class.inc.php +++ b/core/restservices.class.inc.php @@ -34,7 +34,9 @@ */ class ObjectResult { - public $code; + use SanitizeTrait; + + public $code; public $message; public $class; public $key; @@ -122,6 +124,19 @@ class ObjectResult { $this->fields[$sAttCode] = $this->MakeResultValue($oObject, $sAttCode, $bExtendedOutput); } + +public function SanitizeContent() + { + foreach($this->fields as $sFieldAttCode => $fieldValue) + { + try { + $oAttDef = MetaModel::GetAttributeDef($this->class, $sFieldAttCode); + } catch (Exception $e) { // for special cases like ID + continue; + } + $this->SanitizeFieldIfSensitive($this->fields, $sFieldAttCode, $fieldValue, $oAttDef); + } + } } @@ -181,6 +196,16 @@ class RestResultWithObjects extends RestResult $sObjKey = get_class($oObject).'::'.$oObject->GetKey(); $this->objects[$sObjKey] = $oObjRes; } + +public function SanitizeContent() + { + parent::SanitizeContent(); + + foreach($this->objects as $sObjKey => $oObjRes) + { + $oObjRes->SanitizeContent(); + } + } } class RestResultWithRelations extends RestResultWithObjects @@ -247,9 +272,10 @@ class RestDelete * * @package Core */ -class CoreServices implements iRestServiceProvider +class CoreServices implements iRestServiceProvider, iRestInputSanitizer { - /** + use SanitizeTrait; + /** * Enumerate services delivered by this class * * @param string $sVersion The version (e.g. 1.0) supported by the services @@ -663,6 +689,33 @@ class CoreServices implements iRestServiceProvider } return $oResult; } + + public function SanitizeJsonInput(string $sJsonInput): string + { + $sSanitizedJsonInput = $sJsonInput; + $aJsonData = json_decode($sSanitizedJsonInput, true); + $sOperation = $aJsonData['operation']; + + switch ($sOperation) { + case 'core/check_credentials': + if (isset($aJsonData['password'])) { + $aJsonData['password'] = '*****'; + } + break; + case 'core/update': + case 'core/create': + default : + $sClass = $aJsonData['class']; + if (isset($aJsonData['fields'])) { + foreach ($aJsonData['fields'] as $sFieldAttCode => $fieldValue) { + $oAttDef = MetaModel::GetAttributeDef($sClass, $sFieldAttCode); + $this->SanitizeFieldIfSensitive($aJsonData['fields'], $sFieldAttCode, $fieldValue, $oAttDef); + } + } + break; + } + return json_encode($aJsonData, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); + } /** * Helper for object deletion @@ -802,3 +855,50 @@ class CoreServices implements iRestServiceProvider return $iLimit * max(0, $iPage - 1); } } + +trait SanitizeTrait +{ + /** + * Sanitize a field if it is sensitive. + * + * @param array $fields The fields array + * @param string $sFieldAttCode The attribute code + * @param mixed $oAttDef The attribute definition + * @throws Exception + */ + private function SanitizeFieldIfSensitive(array &$fields, string $sFieldAttCode, $fieldValue, $oAttDef): void + { + // for simple attribute + if ($oAttDef instanceof iAttributeNoGroupBy) // iAttributeNoGroupBy is equivalent to sensitive attribute + { + $fields[$sFieldAttCode] = '*****'; + return; + } + // for 1-n / n-n relation + if ($oAttDef instanceof AttributeLinkedSet) { + foreach ($fieldValue as $i => $aLnkValues) { + foreach ($aLnkValues as $sLnkAttCode => $sLnkValue) { + $oLnkAttDef = MetaModel::GetAttributeDef($oAttDef->GetLinkedClass(), $sLnkAttCode); + if ($oLnkAttDef instanceof iAttributeNoGroupBy) { // 1-n relation + $fields[$sFieldAttCode][$i][$sLnkAttCode] = '*****'; + } + elseif ($oAttDef instanceof AttributeLinkedSetIndirect && $oLnkAttDef instanceof AttributeExternalField) { // for n-n relation + $oExtKeyAttDef = MetaModel::GetAttributeDef($oLnkAttDef->GetTargetClass(), $oLnkAttDef->GetExtAttCode()); + if ($oExtKeyAttDef instanceof iAttributeNoGroupBy) { + $fields[$sFieldAttCode][$i][$sLnkAttCode] = '*****'; + } + } + } + } + return; + } + + // for external attribute + if ($oAttDef instanceof AttributeExternalField) { + $oExtKeyAttDef = MetaModel::GetAttributeDef($oAttDef->GetTargetClass(), $oAttDef->GetExtAttCode()); + if ($oExtKeyAttDef instanceof iAttributeNoGroupBy) { + $fields[$sFieldAttCode] = '*****'; + } + } + } +} \ No newline at end of file diff --git a/tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml b/tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml new file mode 100644 index 000000000..798f5f8a0 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml @@ -0,0 +1,116 @@ + + + + + cmdbAbstractObject + + bizmodel + false + autoincrement + test_server + id + + + + + + lnkContactTestToServer + test_server_id + contact_test_id + true + + + PasswordTest + server_test_id + true + + + name + + false + + + + + + + cmdbAbstractObject + + bizmodel + false + autoincrement + contact_test + id + + + + + + password + + + lnkContactTestToServer + contact_test_id + test_server_id + true + + + + + + + cmdbAbstractObject + + bizmodel + false + autoincrement + lnk_contact_server_test + id + + + + + + contact_test_id + password + + + TestServer + DEL_MANUAL + test_server + false + + + + ContactTest + DEL_MANUAL + contact_test + false + + + + + + cmdbAbstractObject + + bizmodel + false + autoincrement + password_test + id + + + + + + TestServer + server_test_id + DEL_MANUAL + + + password + + + + + \ No newline at end of file diff --git a/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php b/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php new file mode 100644 index 000000000..806ad234f --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php @@ -0,0 +1,164 @@ + self::SIMPLE_PASSWORD] + ); + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oContactTest, ['ContactTest' => ['password']]); + $oRestResultWithObject->SanitizeContent(); + static::assertEquals( + '{"objects":{"ContactTest::-1":{"code":0,"message":"ok","class":"ContactTest","key":-1,"fields":{"password":"*****"}}},"code":0,"message":null}', + json_encode($oRestResultWithObject)); + } + + /** + * @return void + * @throws Exception + */ + public function testSanitizeAttributeExternalFieldOnLink() + { + $oContactTest = $this->createObject('ContactTest', [ + 'password' => self::SIMPLE_PASSWORD] + ); + + $oTestServer = $this->createObject('TestServer', [ + 'name' => 'test_server', + ]); + + + // create lnkContactTestToServer + $oLnkContactTestToServer = $this->createObject('lnkContactTestToServer', [ + 'contact_test_id' => $oContactTest->GetKey(), + 'test_server_id' => $oTestServer->GetKey() + ]); + + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oLnkContactTestToServer, + ['lnkContactTestToServer' => ['contact_test_password']]); + + $oRestResultWithObject->SanitizeContent(); + + static::assertContains( + '*****', + json_encode($oRestResultWithObject)); + + static::assertNotContains( + self::SIMPLE_PASSWORD, + json_encode($oRestResultWithObject)); + } + + /** + * @throws Exception + */ + public function testSanitizeAttributeOnObjectRelatedThroughNNRelation() + { + $oContactTest = $this->createObject('ContactTest', [ + 'password' => self::SIMPLE_PASSWORD]); + + $oTestServer = $this->createObject('TestServer', [ + 'name' => 'test_server', + ]); + + // create lnkContactTestToServer + $this->createObject('lnkContactTestToServer', [ + 'contact_test_id' => $oContactTest->GetKey(), + 'test_server_id' => $oTestServer->GetKey() + ]); + + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oTestServer, + ['TestServer' => ['contact_list']]); + + $oRestResultWithObject->SanitizeContent(); + static::assertContains( + '*****', + json_encode($oRestResultWithObject)); + + static::assertNotContains( + self::SIMPLE_PASSWORD, + json_encode($oRestResultWithObject)); + } + + + /** + * @throws CoreException + * @throws CoreUnexpectedValue + * @throws ArchivedObjectException + * @throws Exception + */ + public function testSanitizeOnObjectRelatedThrough1NRelation() + { + $oTestServer = $this->createObject('TestServer', [ + 'name' => 'my_server', + ]); + + $oPassword = new PasswordTest(); + $oPassword->Set('password', self::SIMPLE_PASSWORD); + $oPassword->Set('server_test_id', $oTestServer->GetKey()); + + /** @var ormLinkSet $oContactList */ + $oContactList = $oTestServer->Get('password_list'); + $oContactList->AddItem($oPassword); + $oTestServer->Set('password_list', $oContactList); + + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oTestServer, ['TestServer' => ['id', 'password_list']]); + $oRestResultWithObject->SanitizeContent(); + + static::assertContains( + '*****', + json_encode($oRestResultWithObject)); + + static::assertNotContains( + self::SIMPLE_PASSWORD, + json_encode($oRestResultWithObject)); + + } + + /** + * @return string Abs path to the XML delta to use for the tests of that class + */ + public function GetDatamodelDeltaAbsPath(): string + { + return __DIR__ . '/Delta/delta_test_sanitize_output.xml'; + } +} diff --git a/tests/php-unit-tests/unitary-tests/core/RestServicesTest.php b/tests/php-unit-tests/unitary-tests/core/RestServicesTest.php new file mode 100644 index 000000000..eb3a1f1c0 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/core/RestServicesTest.php @@ -0,0 +1,125 @@ +SanitizeJsonInput($sJsonData); + static::assertEquals($sExpectedJsonDataSanitized, $sOutputJson); + } + + /** + * @return array[] + */ + public function providerTestSanitizeJsonInput(): array + { + return [ + 'core/check_credentials' => [ + '{"operation": "core/check_credentials", "user": "admin", "password": "admin"}', + '{ + "operation": "core/check_credentials", + "user": "admin", + "password": "*****" +}' + ], + 'core/update' => [ + '{"operation": "core/update", "comment": "Update user", "class": "UserLocal", "key": {"id":1}, "output_fields": "first_name, password", "fields": {"password" : "123456"}}', + '{ + "operation": "core/update", + "comment": "Update user", + "class": "UserLocal", + "key": { + "id": 1 + }, + "output_fields": "first_name, password", + "fields": { + "password": "*****" + } +}' + ], + 'core/create' => [ + '{"operation": "core/create", "comment": "Create user", "class": "UserLocal", "fields": {"first_name": "John", "last_name": "Doe", "email": "jd@example/com", "password" : "123456"}}', + '{ + "operation": "core/create", + "comment": "Create user", + "class": "UserLocal", + "fields": { + "first_name": "John", + "last_name": "Doe", + "email": "jd@example/com", + "password": "*****" + } +}' + ], + ]; + } + + /** + * @param $sOperation + * @param $aJsonData + * @param $sExpectedJsonDataSanitized + * @return void + * @throws CoreException + * @throws CoreUnexpectedValue + * @dataProvider providerTestSanitizeJsonOutput + */ + public function testSanitizeJsonOutput($sOperation, $aJsonData, $sExpectedJsonDataSanitized) + { + $oUser = new UserLocal(); + $oUser->Set('password', '123456'); + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oUser, ['UserLocal' => ['login', 'password']]); + $oRestResultWithObject->SanitizeContent(); + static::assertEquals($sExpectedJsonDataSanitized, json_encode($oRestResultWithObject)); + } + + /** + * @return array[] + */ + public function providerTestSanitizeJsonOutput(): array + { + return [ + + 'core/update' => [ + 'core/update', + ['comment' => 'Update user', 'class' => 'UserLocal', 'key' => ['login' => 'my_example'], 'output_fields' => 'password', 'fields' => ['password' => 'opkB!req57']], + '{"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"*****"}}},"code":0,"message":null}' + ], + 'core/create' => [ + 'core/create', + ['comment' => 'Create user', 'class' => 'UserLocal', 'fields' => ['password' => 'Azertyuiiop*12', 'login' => 'toto', 'profile_list' => [1]]], + '{"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"*****"}}},"code":0,"message":null}' + ], + 'core/get' => [ + 'core/get', + ['comment' => 'Get user', 'class' => 'UserLocal', 'key' => ['login' => 'my_example'], 'output_fields' => 'first_name, password'], + '{"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"*****"}}},"code":0,"message":null}' + ], + 'core/check_credentials' => [ + 'core/check_credentials', + ['user' => 'admin', 'password' => 'admin'], + '{"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"*****"}}},"code":0,"message":null}' + ], + ]; + } +} diff --git a/webservices/rest.php b/webservices/rest.php index d68c95e84..36f1ce82f 100644 --- a/webservices/rest.php +++ b/webservices/rest.php @@ -209,6 +209,13 @@ try /** @var iRestServiceProvider $oRS */ $oRS = $aOpToRestService[$sOperation]['service_provider']; $sProvider = get_class($oRS); + + if ($oRS instanceof iRestInputSanitizer) { + $sSanitizedJsonInput = $oRS->SanitizeJsonInput($sJsonString); + } + else { + $sSanitizedJsonInput = $sJsonString; + } CMDBObject::SetTrackOrigin('webservice-rest'); $oResult = $oRS->ExecOperation($sVersion, $sOperation, $aJsonData); @@ -234,6 +241,7 @@ catch(Exception $e) // $sResponse = json_encode($oResult); + if ($sResponse === false) { $oJsonIssue = new RestResult(); @@ -267,7 +275,7 @@ if (MetaModel::GetConfig()->Get('log_rest_service')) $oLog->SetTrim('userinfo', UserRights::GetUser()); $oLog->Set('version', $sVersion); $oLog->Set('operation', $sOperation); - $oLog->SetTrim('json_input', $sJsonString); + $oLog->SetTrim('json_input', $sSanitizedJsonInput); $oLog->Set('provider', $sProvider); $sMessage = $oResult->message; @@ -277,7 +285,8 @@ if (MetaModel::GetConfig()->Get('log_rest_service')) } $oLog->SetTrim('message', $sMessage); $oLog->Set('code', $oResult->code); - $oLog->SetTrim('json_output', $sResponse); + $oResult->SanitizeContent(); + $oLog->SetTrim('json_output', json_encode($oResult, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE)); $oLog->DBInsertNoReload(); $oKPI->ComputeAndReport('Log inserted'); From 8f8ac46f556c5e1dbaed7ab0fe4e6be3a7159948 Mon Sep 17 00:00:00 2001 From: jf-cbd Date: Fri, 21 Feb 2025 13:16:07 +0100 Subject: [PATCH 2/5] =?UTF-8?q?N=C2=B08215=20-=20When=20PHP=20warning=20ar?= =?UTF-8?q?e=20enabled,=20Global=20Request=20doesn't=20work?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../portal/src/Form/ObjectFormManager.php | 95 ++++++++++--------- 1 file changed, 48 insertions(+), 47 deletions(-) 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 a7613887e..99a0f5b35 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 @@ -1230,55 +1230,56 @@ class ObjectFormManager extends FormManager $this->aFieldsAtts = array(); $this->aExtraData = array(); $aFieldsDMOnlyAttCodes = array(); - switch ($this->aFormProperties['type']) { - case 'custom_list': - case 'static': - foreach ($this->aFormProperties['fields'] as $sAttCode => $aOptions) { - // When in a transition and no flags are specified for the field, we will retrieve its flags from DM later - if ($this->IsTransitionForm() && empty($aOptions)) { - $aFieldsDMOnlyAttCodes[] = $sAttCode; - continue; - } + if (array_key_exists('type', $this->aFormProperties)) { + switch ($this->aFormProperties['type']) { + case 'custom_list': + case 'static': + foreach ($this->aFormProperties['fields'] as $sAttCode => $aOptions) { + // When in a transition and no flags are specified for the field, we will retrieve its flags from DM later + if ($this->IsTransitionForm() && empty($aOptions)) { + $aFieldsDMOnlyAttCodes[] = $sAttCode; + continue; + } - // Otherwise we proceed as usual - $iFieldFlags = OPT_ATT_NORMAL; - // Checking if field should be slave - if (isset($aOptions['slave']) && ($aOptions['slave'] === true)) { - $iFieldFlags = $iFieldFlags | OPT_ATT_SLAVE; + // Otherwise we proceed as usual + $iFieldFlags = OPT_ATT_NORMAL; + // Checking if field should be slave + if (isset($aOptions['slave']) && ($aOptions['slave'] === true)) { + $iFieldFlags = $iFieldFlags | OPT_ATT_SLAVE; + } + // Checking if field should be must_change + if (isset($aOptions['must_change']) && ($aOptions['must_change'] === true)) { + $iFieldFlags = $iFieldFlags | OPT_ATT_MUSTCHANGE; + } + // Checking if field should be must prompt + if (isset($aOptions['must_prompt']) && ($aOptions['must_prompt'] === true)) { + $iFieldFlags = $iFieldFlags | OPT_ATT_MUSTPROMPT; + } + // Checking if field should be hidden + if (isset($aOptions['hidden']) && ($aOptions['hidden'] === true)) { + $iFieldFlags = $iFieldFlags | OPT_ATT_HIDDEN; + } + // Checking if field should be readonly + if (isset($aOptions['read_only']) && ($aOptions['read_only'] === true)) { + $iFieldFlags = $iFieldFlags | OPT_ATT_READONLY; + } + // Checking if field should be mandatory + if (isset($aOptions['mandatory']) && ($aOptions['mandatory'] === true)) { + $iFieldFlags = $iFieldFlags | OPT_ATT_MANDATORY; + } + // Finally, adding the attribute and its flags + $this->aFieldsAtts[$sAttCode] = $iFieldFlags; } - // Checking if field should be must_change - if (isset($aOptions['must_change']) && ($aOptions['must_change'] === true)) { - $iFieldFlags = $iFieldFlags | OPT_ATT_MUSTCHANGE; - } - // Checking if field should be must prompt - if (isset($aOptions['must_prompt']) && ($aOptions['must_prompt'] === true)) { - $iFieldFlags = $iFieldFlags | OPT_ATT_MUSTPROMPT; - } - // Checking if field should be hidden - if (isset($aOptions['hidden']) && ($aOptions['hidden'] === true)) { - $iFieldFlags = $iFieldFlags | OPT_ATT_HIDDEN; - } - // Checking if field should be readonly - if (isset($aOptions['read_only']) && ($aOptions['read_only'] === true)) { - $iFieldFlags = $iFieldFlags | OPT_ATT_READONLY; - } - // Checking if field should be mandatory - if (isset($aOptions['mandatory']) && ($aOptions['mandatory'] === true)) { - $iFieldFlags = $iFieldFlags | OPT_ATT_MANDATORY; - } - // Finally, adding the attribute and its flags - $this->aFieldsAtts[$sAttCode] = $iFieldFlags; - } - break; + break; - case 'zlist': - foreach (MetaModel::FlattenZList(MetaModel::GetZListItems($sObjectClass, $this->aFormProperties['fields'])) as $sAttCode) { - $this->aFieldsAtts[$sAttCode] = OPT_ATT_NORMAL; - } - break; + case 'zlist': + foreach (MetaModel::FlattenZList(MetaModel::GetZListItems($sObjectClass, $this->aFormProperties['fields'])) as $sAttCode) { + $this->aFieldsAtts[$sAttCode] = OPT_ATT_NORMAL; + } + break; + } } - - if ($this->aFormProperties['layout'] !== null) { + if (isset($this->aFormProperties['layout'])) { $oXPath = new DOMXPath($this->oHtmlDocument); /** @var \DOMElement $oFieldNode */ foreach ($oXPath->query('//div[contains(@class, "form_field")][@data-field-id]') as $oFieldNode) { @@ -1337,7 +1338,7 @@ class ObjectFormManager extends FormManager // Also, retrieving mandatory attributes from metamodel to be able to complete the form with them if necessary // // Note: When in a transition, we don't do this for fields that should be set from DM - if ($this->aFormProperties['type'] !== 'static') { + if (array_key_exists('type', $this->aFormProperties) && $this->aFormProperties['type'] !== 'static') { if ($this->IsTransitionForm()) { $aDatamodelAttCodes = $this->oObject->GetTransitionAttributes($this->aFormProperties['stimulus_code']); } @@ -1448,7 +1449,7 @@ class ObjectFormManager extends FormManager // - The layout $this->oHtmlDocument = new DOMDocument(); - if ($this->aFormProperties['layout'] !== null) { + if (isset($this->aFormProperties['layout'])) { // Checking if we need to render the template from twig to html in order to parse the fields if ($this->aFormProperties['layout']['type'] === 'twig') { if ($this->oContainer !== null) { From 77ba0b398f04a248f551a4f94b2cb745807d68af Mon Sep 17 00:00:00 2001 From: jf-cbd Date: Thu, 6 Mar 2025 12:11:20 +0100 Subject: [PATCH 3/5] Fix merge conflict --- core/restservices.class.inc.php | 1 - 1 file changed, 1 deletion(-) diff --git a/core/restservices.class.inc.php b/core/restservices.class.inc.php index ac43d8e7f..812b18a1c 100644 --- a/core/restservices.class.inc.php +++ b/core/restservices.class.inc.php @@ -46,7 +46,6 @@ class ObjectResult */ use SanitizeTrait; - public $code; public $message; /** * @var mixed|null From 278496eaf6f889b241d7acab1862b32780d6af02 Mon Sep 17 00:00:00 2001 From: jf-cbd Date: Thu, 6 Mar 2025 14:40:39 +0100 Subject: [PATCH 4/5] Update tests for 3.1 --- .../core/Delta/delta_test_sanitize_output.xml | 214 ++++++++-------- .../core/RestServicesSanitizeOutputTest.php | 235 +++++++++--------- 2 files changed, 227 insertions(+), 222 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml b/tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml index 798f5f8a0..4c475c9e7 100644 --- a/tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml +++ b/tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml @@ -1,116 +1,116 @@ - - - cmdbAbstractObject - - bizmodel - false - autoincrement - test_server - id - - - - - - lnkContactTestToServer - test_server_id - contact_test_id - true - - - PasswordTest - server_test_id - true - - - name - - false - - - + + + cmdbAbstractObject + + bizmodel + false + autoincrement + test_server + id + + + + + + lnkContactTestToServer + test_server_id + contact_test_id + true + + + PasswordTest + server_test_id + true + + + name + + false + + + - - cmdbAbstractObject - - bizmodel - false - autoincrement - contact_test - id - - - - - - password - - - lnkContactTestToServer - contact_test_id - test_server_id - true - - - + + cmdbAbstractObject + + bizmodel + false + autoincrement + contact_test + id + + + + + + password + + + lnkContactTestToServer + contact_test_id + test_server_id + true + + + - - cmdbAbstractObject - - bizmodel - false - autoincrement - lnk_contact_server_test - id - - - - - - contact_test_id - password - - - TestServer - DEL_MANUAL - test_server - false + + cmdbAbstractObject + + bizmodel + false + autoincrement + lnk_contact_server_test + id + + + + + + contact_test_id + password + + + TestServer + DEL_MANUAL + test_server + false - - - ContactTest - DEL_MANUAL - contact_test - false + + + ContactTest + DEL_MANUAL + contact_test + false - - - - - cmdbAbstractObject - - bizmodel - false - autoincrement - password_test - id - - - - - - TestServer - server_test_id - DEL_MANUAL - - - password - - - - + + + + + cmdbAbstractObject + + bizmodel + false + autoincrement + password_test + id + + + + + + TestServer + server_test_id + DEL_MANUAL + + + password + + + + \ No newline at end of file diff --git a/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php b/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php index 806ad234f..199eeb4bf 100644 --- a/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php +++ b/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php @@ -21,144 +21,149 @@ use RestResultWithObjects; */ class RestServicesSanitizeOutputTest extends ItopCustomDatamodelTestCase { - private const SIMPLE_PASSWORD = '123456'; + private const SIMPLE_PASSWORD = '123456'; - /** - * @throws Exception - */ - protected function setUp(): void - { - parent::setUp(); - // Workaround to cope with inconsistent settings in itop-config files from the CI - AttributeEncryptedString::$sKey = '6eb9d9afa3ee0fbcebe622a33bf57aaeafb7c37998fd24c403c2522c2d60117f'; - } + /** + * @throws Exception + */ + protected function setUp(): void + { + parent::setUp(); + // Workaround to cope with inconsistent settings in itop-config files from the CI + AttributeEncryptedString::$sKey = '6eb9d9afa3ee0fbcebe622a33bf57aaeafb7c37998fd24c403c2522c2d60117f'; + } - /** - * @return void - * @throws CoreException - */ - public function testSanitizeAttributeOnRequestedObject() - { - $oContactTest = MetaModel::NewObject('ContactTest', [ - 'password' => self::SIMPLE_PASSWORD] - ); - $oRestResultWithObject = new RestResultWithObjects(); - $oRestResultWithObject->AddObject(0, 'ok', $oContactTest, ['ContactTest' => ['password']]); - $oRestResultWithObject->SanitizeContent(); - static::assertEquals( - '{"objects":{"ContactTest::-1":{"code":0,"message":"ok","class":"ContactTest","key":-1,"fields":{"password":"*****"}}},"code":0,"message":null}', - json_encode($oRestResultWithObject)); - } + /** + * @return void + * @throws CoreException + */ + public function testSanitizeAttributeOnRequestedObject() + { + $oContactTest = MetaModel::NewObject('ContactTest', [ + 'password' => self::SIMPLE_PASSWORD + ] + ); + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oContactTest, ['ContactTest' => ['password']]); + $oRestResultWithObject->SanitizeContent(); + static::assertEquals( + '{"objects":{"ContactTest::-1":{"code":0,"message":"ok","class":"ContactTest","key":-1,"fields":{"password":"*****"}}},"code":0,"message":null}', + json_encode($oRestResultWithObject)); + } - /** - * @return void - * @throws Exception - */ - public function testSanitizeAttributeExternalFieldOnLink() - { - $oContactTest = $this->createObject('ContactTest', [ - 'password' => self::SIMPLE_PASSWORD] - ); + /** + * @return void + * @throws Exception + */ + public function testSanitizeAttributeExternalFieldOnLink() + { + $oContactTest = $this->createObject('ContactTest', [ + 'password' => self::SIMPLE_PASSWORD + ] + ); - $oTestServer = $this->createObject('TestServer', [ - 'name' => 'test_server', - ]); + $oTestServer = $this->createObject('TestServer', [ + 'name' => 'test_server', + ]); - // create lnkContactTestToServer - $oLnkContactTestToServer = $this->createObject('lnkContactTestToServer', [ - 'contact_test_id' => $oContactTest->GetKey(), - 'test_server_id' => $oTestServer->GetKey() - ]); + // create lnkContactTestToServer + $oLnkContactTestToServer = $this->createObject('lnkContactTestToServer', [ + 'contact_test_id' => $oContactTest->GetKey(), + 'test_server_id' => $oTestServer->GetKey() + ]); - $oRestResultWithObject = new RestResultWithObjects(); - $oRestResultWithObject->AddObject(0, 'ok', $oLnkContactTestToServer, - ['lnkContactTestToServer' => ['contact_test_password']]); + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oLnkContactTestToServer, + ['lnkContactTestToServer' => ['contact_test_password']]); - $oRestResultWithObject->SanitizeContent(); + $oRestResultWithObject->SanitizeContent(); - static::assertContains( - '*****', - json_encode($oRestResultWithObject)); + static::assertStringContainsString( + '*****', + json_encode($oRestResultWithObject)); - static::assertNotContains( - self::SIMPLE_PASSWORD, - json_encode($oRestResultWithObject)); - } + static::assertStringNotContainsString( + self::SIMPLE_PASSWORD, + json_encode($oRestResultWithObject)); + } - /** - * @throws Exception - */ - public function testSanitizeAttributeOnObjectRelatedThroughNNRelation() - { - $oContactTest = $this->createObject('ContactTest', [ - 'password' => self::SIMPLE_PASSWORD]); + /** + * @throws Exception + */ + public function testSanitizeAttributeOnObjectRelatedThroughNNRelation() + { + $oContactTest = $this->createObject('ContactTest', [ + 'password' => self::SIMPLE_PASSWORD + ]); - $oTestServer = $this->createObject('TestServer', [ - 'name' => 'test_server', - ]); + $oTestServer = $this->createObject('TestServer', [ + 'name' => 'test_server', + ]); - // create lnkContactTestToServer - $this->createObject('lnkContactTestToServer', [ - 'contact_test_id' => $oContactTest->GetKey(), - 'test_server_id' => $oTestServer->GetKey() - ]); + // create lnkContactTestToServer + $this->createObject('lnkContactTestToServer', [ + 'contact_test_id' => $oContactTest->GetKey(), + 'test_server_id' => $oTestServer->GetKey() + ]); - $oRestResultWithObject = new RestResultWithObjects(); - $oRestResultWithObject->AddObject(0, 'ok', $oTestServer, - ['TestServer' => ['contact_list']]); + $oTestServer->Reload(); - $oRestResultWithObject->SanitizeContent(); - static::assertContains( - '*****', - json_encode($oRestResultWithObject)); + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oTestServer, + ['TestServer' => ['contact_list']]); - static::assertNotContains( - self::SIMPLE_PASSWORD, - json_encode($oRestResultWithObject)); - } + $oRestResultWithObject->SanitizeContent(); + static::assertStringContainsString( + '*****', + json_encode($oRestResultWithObject)); + + static::assertStringNotContainsString( + self::SIMPLE_PASSWORD, + json_encode($oRestResultWithObject)); + } - /** - * @throws CoreException - * @throws CoreUnexpectedValue - * @throws ArchivedObjectException - * @throws Exception - */ - public function testSanitizeOnObjectRelatedThrough1NRelation() - { - $oTestServer = $this->createObject('TestServer', [ - 'name' => 'my_server', - ]); + /** + * @throws CoreException + * @throws CoreUnexpectedValue + * @throws ArchivedObjectException + * @throws Exception + */ + public function testSanitizeOnObjectRelatedThrough1NRelation() + { + $oTestServer = $this->createObject('TestServer', [ + 'name' => 'my_server', + ]); - $oPassword = new PasswordTest(); - $oPassword->Set('password', self::SIMPLE_PASSWORD); - $oPassword->Set('server_test_id', $oTestServer->GetKey()); + $oPassword = new PasswordTest(); + $oPassword->Set('password', self::SIMPLE_PASSWORD); + $oPassword->Set('server_test_id', $oTestServer->GetKey()); - /** @var ormLinkSet $oContactList */ - $oContactList = $oTestServer->Get('password_list'); - $oContactList->AddItem($oPassword); - $oTestServer->Set('password_list', $oContactList); + /** @var ormLinkSet $oContactList */ + $oContactList = $oTestServer->Get('password_list'); + $oContactList->AddItem($oPassword); + $oTestServer->Set('password_list', $oContactList); - $oRestResultWithObject = new RestResultWithObjects(); - $oRestResultWithObject->AddObject(0, 'ok', $oTestServer, ['TestServer' => ['id', 'password_list']]); - $oRestResultWithObject->SanitizeContent(); + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oTestServer, ['TestServer' => ['id', 'password_list']]); + $oRestResultWithObject->SanitizeContent(); - static::assertContains( - '*****', - json_encode($oRestResultWithObject)); + static::assertStringContainsString( + '*****', + json_encode($oRestResultWithObject)); - static::assertNotContains( - self::SIMPLE_PASSWORD, - json_encode($oRestResultWithObject)); + static::assertStringNotContainsString( + self::SIMPLE_PASSWORD, + json_encode($oRestResultWithObject)); - } + } - /** - * @return string Abs path to the XML delta to use for the tests of that class - */ - public function GetDatamodelDeltaAbsPath(): string - { - return __DIR__ . '/Delta/delta_test_sanitize_output.xml'; - } + /** + * @return string Abs path to the XML delta to use for the tests of that class + */ + public function GetDatamodelDeltaAbsPath(): string + { + return __DIR__.'/Delta/delta_test_sanitize_output.xml'; + } } From 1142bf327c62983b4923e9fc847e80610f21c1c0 Mon Sep 17 00:00:00 2001 From: jf-cbd Date: Thu, 6 Mar 2025 14:54:18 +0100 Subject: [PATCH 5/5] Fix tests --- .../unitary-tests/core/RestServicesSanitizeOutputTest.php | 2 +- tests/php-unit-tests/unitary-tests/core/RestServicesTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php b/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php index 199eeb4bf..e6c7fb21c 100644 --- a/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php +++ b/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php @@ -46,7 +46,7 @@ class RestServicesSanitizeOutputTest extends ItopCustomDatamodelTestCase $oRestResultWithObject = new RestResultWithObjects(); $oRestResultWithObject->AddObject(0, 'ok', $oContactTest, ['ContactTest' => ['password']]); $oRestResultWithObject->SanitizeContent(); - static::assertEquals( + static::assertJsonStringEqualsJsonString( '{"objects":{"ContactTest::-1":{"code":0,"message":"ok","class":"ContactTest","key":-1,"fields":{"password":"*****"}}},"code":0,"message":null}', json_encode($oRestResultWithObject)); } diff --git a/tests/php-unit-tests/unitary-tests/core/RestServicesTest.php b/tests/php-unit-tests/unitary-tests/core/RestServicesTest.php index c5c42bcd3..f549ae10c 100644 --- a/tests/php-unit-tests/unitary-tests/core/RestServicesTest.php +++ b/tests/php-unit-tests/unitary-tests/core/RestServicesTest.php @@ -25,7 +25,7 @@ class RestServicesTest extends ItopDataTestCase { $oRS = new CoreServices(); $sOutputJson = $oRS->SanitizeJsonInput($sJsonData); - static::assertEquals($sExpectedJsonDataSanitized, $sOutputJson); + static::assertJsonStringEqualsJsonString($sExpectedJsonDataSanitized, $sOutputJson); } /** @@ -90,7 +90,7 @@ class RestServicesTest extends ItopDataTestCase $oRestResultWithObject = new RestResultWithObjects(); $oRestResultWithObject->AddObject(0, 'ok', $oUser, ['UserLocal' => ['login', 'password']]); $oRestResultWithObject->SanitizeContent(); - static::assertEquals($sExpectedJsonDataSanitized, json_encode($oRestResultWithObject)); + static::assertJsonStringEqualsJsonString($sExpectedJsonDataSanitized, json_encode($oRestResultWithObject)); } /**