N°8231 - making rest api log more readable

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
+	}
 }
 
 /**

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)

core/restservices.class.inc.php

@@ -122,6 +122,25 @@ class ObjectResult
 	{
 		$this->fields[$sAttCode] = $this->MakeResultValue($oObject, $sAttCode, $bExtendedOutput);
 	}
+	
+public function SanitizeContent()
+	{
+		foreach($this->fields as $sAttCode => $value)
+		{
+            try{
+			$oAttDef = MetaModel::GetAttributeDef($this->class, $sAttCode);
+            } catch (Exception $e) { // for special cases like ID
+                continue;
+            }
+			if ($oAttDef instanceof iAttributeNoGroupBy) // iAttributeNoGroupBy is equivalent to sensitive attribute
+            {
+                $this->fields[$sAttCode] = '******';
+            }
+			{
+				$this->fields[$sAttCode] = '******';
+			}
+		}
+	}
 }
 
 
@@ -181,6 +200,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,7 +276,7 @@ class RestDelete
  *
  * @package     Core
  */
-class CoreServices implements iRestServiceProvider
+class CoreServices implements iRestServiceProvider, iRestInputSanitizer
 {
 	/**
 	 * Enumerate services delivered by this class
@@ -663,6 +692,34 @@ 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'];
+            foreach ($aJsonData['fields'] as $sAttCode => $value) {
+                $oAttDef = MetaModel::GetAttributeDef($sClass, $sAttCode);
+                if ($oAttDef instanceof iAttributeNoGroupBy) // iAttributeNoGroupBy is equivalent to sensitive attribute
+                {
+                    $aJsonData['fields'][$sAttCode] = '*****';
+                }
+            }
+            break;
+        }
+		return json_encode($aJsonData, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT);
+	}
 
 	/**
 	 * Helper for object deletion	

tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.7">
+    <classes>
+        <class id="Group">
+            <fields>
+                <field id="encrypted_string" xsi:type="AttributeEncryptedString" _delta="define">
+                    <sql>encrypted_string</sql>
+                </field>
+            </fields>
+        </class>
+    </classes>
+</itop_design>

tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php

@@ -0,0 +1,54 @@
+<?php
+// Copyright (c) 2010-2018 Combodo SARL
+//
+//   This file is part of iTop.
+//
+//   iTop is free software; you can redistribute it and/or modify
+//   it under the terms of the GNU Affero General Public License as published by
+//   the Free Software Foundation, either version 3 of the License, or
+//   (at your option) any later version.
+//
+//   iTop is distributed in the hope that it will be useful,
+//   but WITHOUT ANY WARRANTY; without even the implied warranty of
+//   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//   GNU Affero General Public License for more details.
+//
+//   You should have received a copy of the GNU Affero General Public License
+//   along with iTop. If not, see <http://www.gnu.org/licenses/>
+//
+
+namespace Combodo\iTop\Test\UnitTest\Core;
+
+use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase;
+use Group;
+
+/**
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ * @backupGlobals disabled
+ */
+class RestServicesSanitizeOutputTest extends iTopCustomDatamodelTestCase
+{
+    public function setUp(): void
+    {
+        parent::setUp();
+    }
+
+
+    public function testSanitizeJsonOutput()
+    {
+        $oGroup = new Group();
+        $oGroup->Set('encrypted_string', "123456");
+        $oRestResultWithObject = new \RestResultWithObjects();
+        $oRestResultWithObject->AddObject(0, "ok", $oGroup, ['Group' => ['encrypted_string']]);
+        $oRestResultWithObject->SanitizeContent();
+        $this->assertEquals('{"objects":{"Group::-1":{"code":0,"message":"ok","class":"Group","key":-1,"fields":{"encrypted_string":"******"}}},"code":0,"message":null}', json_encode($oRestResultWithObject));
+    }
+
+
+
+    public function GetDatamodelDeltaAbsPath(): string
+    {
+        return  __DIR__ . "/Delta/delta_test_sanitize_output.xml";
+    }
+}

tests/php-unit-tests/unitary-tests/core/RestServicesTest.php

@@ -0,0 +1,140 @@
+<?php
+// Copyright (c) 2010-2018 Combodo SARL
+//
+//   This file is part of iTop.
+//
+//   iTop is free software; you can redistribute it and/or modify
+//   it under the terms of the GNU Affero General Public License as published by
+//   the Free Software Foundation, either version 3 of the License, or
+//   (at your option) any later version.
+//
+//   iTop is distributed in the hope that it will be useful,
+//   but WITHOUT ANY WARRANTY; without even the implied warranty of
+//   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//   GNU Affero General Public License for more details.
+//
+//   You should have received a copy of the GNU Affero General Public License
+//   along with iTop. If not, see <http://www.gnu.org/licenses/>
+//
+
+namespace Combodo\iTop\Test\UnitTest\Core;
+
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+use CoreException;
+use CoreServices;
+use CoreUnexpectedValue;
+use SimpleGraphException;
+use UserLocal;
+
+/**
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ * @backupGlobals disabled
+ */
+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);
+        $this->assertEquals($sExpectedJsonDataSanitized, $sOutputJson);
+    }
+
+    public function providerTestSanitizeJsonInput()
+    {
+        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
+     * @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));
+    }
+
+    public function providerTestSanitizeJsonOutput()
+    {
+        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}'                ],
+        ];
+    }
+}

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));
 
 	$oLog->DBInsertNoReload();
 	$oKPI->ComputeAndReport('Log inserted');