diff --git a/core/restservices.class.inc.php b/core/restservices.class.inc.php index 3e86f6759..ac43d8e7f 100644 --- a/core/restservices.class.inc.php +++ b/core/restservices.class.inc.php @@ -44,6 +44,9 @@ class ObjectResult * @var string * @api */ + use SanitizeTrait; + + public $code; public $message; /** * @var mixed|null @@ -156,20 +159,17 @@ class ObjectResult { $this->fields[$sAttCode] = $this->MakeResultValue($oObject, $sAttCode, $bExtendedOutput); } - + public function SanitizeContent() { - foreach($this->fields as $sAttCode => $value) + foreach($this->fields as $sFieldAttCode => $fieldValue) { - try{ - $oAttDef = MetaModel::GetAttributeDef($this->class, $sAttCode); + try { + $oAttDef = MetaModel::GetAttributeDef($this->class, $sFieldAttCode); } catch (Exception $e) { // for special cases like ID continue; } - if ($oAttDef instanceof iAttributeNoGroupBy) // iAttributeNoGroupBy is equivalent to sensitive attribute - { - $this->fields[$sAttCode] = '******'; - } + $this->SanitizeFieldIfSensitive($this->fields, $sFieldAttCode, $fieldValue, $oAttDef); } } } @@ -237,11 +237,11 @@ 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(); @@ -336,7 +336,8 @@ class RestDelete */ 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 @@ -554,18 +555,18 @@ class CoreServices implements iRestServiceProvider, iRestInputSanitizer } else { - if (!$bExtendedOutput && RestUtils::GetOptionalParam($aParams, 'output_fields', '*') != '*') + if (!$bExtendedOutput && RestUtils::GetOptionalParam($aParams, 'output_fields', '*') != '*') { $aFields = $aShowFields[$sClass]; //Id is not a valid attribute to optimize - if (in_array('id', $aFields)) + if (in_array('id', $aFields)) { unset($aFields[array_search('id', $aFields)]); } $aAttToLoad = array($oObjectSet->GetClassAlias() => $aFields); $oObjectSet->OptimizeColumnLoad($aAttToLoad); } - + while ($oObject = $oObjectSet->Fetch()) { $oResult->AddObject(0, '', $oObject, $aShowFields, $bExtendedOutput); @@ -762,7 +763,7 @@ class CoreServices implements iRestServiceProvider, iRestInputSanitizer } return $oResult; } - + public function SanitizeJsonInput(string $sJsonInput): string { $sSanitizedJsonInput = $sJsonInput; @@ -780,17 +781,14 @@ class CoreServices implements iRestServiceProvider, iRestInputSanitizer default : $sClass = $aJsonData['class']; if (isset($aJsonData['fields'])) { - foreach ($aJsonData['fields'] as $sAttCode => $value) { - $oAttDef = MetaModel::GetAttributeDef($sClass, $sAttCode); - if ($oAttDef instanceof iAttributeNoGroupBy) // iAttributeNoGroupBy is equivalent to sensitive attribute - { - $aJsonData['fields'][$sAttCode] = '*****'; - } + 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); + return json_encode($aJsonData, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); } /** @@ -931,3 +929,50 @@ class CoreServices implements iRestServiceProvider, iRestInputSanitizer 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 index 2465f3548..798f5f8a0 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,10 +1,114 @@ - + + cmdbAbstractObject + + bizmodel + false + autoincrement + test_server + id + + + - - encrypted_string + + 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 diff --git a/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php b/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php index f73207de3..806ad234f 100644 --- a/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php +++ b/tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php @@ -1,54 +1,164 @@ -// +declare(strict_types=1); namespace Combodo\iTop\Test\UnitTest\Core; +use ArchivedObjectException; +use AttributeEncryptedString; use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase; -use Group; +use CoreException; +use CoreUnexpectedValue; +use Exception; +use MetaModel; +use ormLinkSet; +use PasswordTest; +use RestResultWithObjects; /** * @runTestsInSeparateProcesses * @preserveGlobalState disabled * @backupGlobals disabled */ -class RestServicesSanitizeOutputTest extends iTopCustomDatamodelTestCase +class RestServicesSanitizeOutputTest extends ItopCustomDatamodelTestCase { - public function setUp(): void + 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'; } - - public function testSanitizeJsonOutput() + /** + * @return void + * @throws CoreException + */ + public function testSanitizeAttributeOnRequestedObject() { - $oGroup = new Group(); - $oGroup->Set('encrypted_string', "123456"); - $oRestResultWithObject = new \RestResultWithObjects(); - $oRestResultWithObject->AddObject(0, "ok", $oGroup, ['Group' => ['encrypted_string']]); + $oContactTest = MetaModel::NewObject('ContactTest', [ + 'password' => self::SIMPLE_PASSWORD] + ); + $oRestResultWithObject = new RestResultWithObjects(); + $oRestResultWithObject->AddObject(0, 'ok', $oContactTest, ['ContactTest' => ['password']]); $oRestResultWithObject->SanitizeContent(); - $this->assertEquals('{"code":0,"message":null,"objects":{"Group::-1":{"code":0,"message":"ok","class":"Group","key":-1,"fields":{"encrypted_string":"******"}}}}', json_encode($oRestResultWithObject)); + 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"; + 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 index 12eebe6dd..c5c42bcd3 100644 --- a/tests/php-unit-tests/unitary-tests/core/RestServicesTest.php +++ b/tests/php-unit-tests/unitary-tests/core/RestServicesTest.php @@ -1,21 +1,5 @@ -// +declare(strict_types=1); namespace Combodo\iTop\Test\UnitTest\Core; @@ -23,7 +7,7 @@ use Combodo\iTop\Test\UnitTest\ItopDataTestCase; use CoreException; use CoreServices; use CoreUnexpectedValue; -use SimpleGraphException; +use RestResultWithObjects; use UserLocal; /** @@ -33,36 +17,34 @@ use UserLocal; */ class RestServicesTest extends ItopDataTestCase { - public function setUp(): void - { - parent::setUp(); - } + /** + * @return void + * @dataProvider providerTestSanitizeJsonInput + */ + public function testSanitizeJsonInput($sJsonData, $sExpectedJsonDataSanitized) + { + $oRS = new CoreServices(); + $sOutputJson = $oRS->SanitizeJsonInput($sJsonData); + static::assertEquals($sExpectedJsonDataSanitized, $sOutputJson); + } - /** - * @return void - * @dataProvider providerTestSanitizeJsonInput - */ - public function testSanitizeJsonInput($sJsonData, $sExpectedJsonDataSanitized) - { - $oRS = new CoreServices(); - $sOutputJson = $oRS->SanitizeJsonInput($sJsonData); - $this->assertEquals($sExpectedJsonDataSanitized, $sOutputJson); - } - - public function providerTestSanitizeJsonInput() - { - return [ - 'core/check_credentials' => [ - '{"operation": "core/check_credentials", "user": "admin", "password": "admin"}', - '{ + /** + * @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"}}', - '{ + ], + '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", @@ -74,10 +56,10 @@ class RestServicesTest extends ItopDataTestCase "password": "*****" } }' - ], - 'core/create' => [ - '{"operation": "core/create", "comment": "Create user", "class": "UserLocal", "fields": {"first_name": "John", "last_name": "Doe", "email": "jd@example/com", "password" : "123456"}}', - '{ + ], + '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", @@ -88,53 +70,56 @@ class RestServicesTest extends ItopDataTestCase "password": "*****" } }' - ], - ]; - } + ], + ]; + } - /** - * @param $sOperation - * @param $aJsonData - * @param $sExpectedJsonDataSanitized - * @return void - * @throws CoreException - * @throws CoreUnexpectedValue - * @throws SimpleGraphException - * @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(); - $this->assertEquals($sExpectedJsonDataSanitized, json_encode($oRestResultWithObject)); - } + /** + * @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)); + } - public function providerTestSanitizeJsonOutput() - { - return [ + /** + * @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']], - '{"code":0,"message":null,"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"******"}}}}' - ], - 'core/create' => [ - 'core/create', - ['comment' => 'Create user', 'class' => 'UserLocal', 'fields' => ['password' => 'Azertyuiiop*12', 'login' => 'toto', 'profile_list' => [1]]], - '{"code":0,"message":null,"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"******"}}}}' - ], - 'core/get' => [ - 'core/get', - ['comment' => 'Get user', 'class' => 'UserLocal', 'key' => ['login' => 'my_example'], 'output_fields' => 'first_name, password'], - '{"code":0,"message":null,"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"******"}}}}' - ], - 'core/check_credentials' => [ - 'core/check_credentials', - ['user' => 'admin', 'password' => 'admin'], - '{"code":0,"message":null,"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"******"}}}}' ], - ]; - } + '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 280e35707..6029cff03 100644 --- a/webservices/rest.php +++ b/webservices/rest.php @@ -224,14 +224,14 @@ 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); } @@ -299,7 +299,7 @@ if (MetaModel::GetConfig()->Get('log_rest_service')) $oLog->SetTrim('message', $sMessage); $oLog->Set('code', $oResult->code); $oResult->SanitizeContent(); - $oLog->SetTrim('json_output', json_encode($oResult, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT)); + $oLog->SetTrim('json_output', json_encode($oResult, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE)); $oLog->DBInsertNoReload(); } \ No newline at end of file