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');