composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-04-23 18:48:51 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'support/2.7' into feature/OAuthMail

Browse Source
This commit is contained in:
Eric Espie
2022-05-23 10:56:32 +02:00
parent a3f122184c da6621f2ff
commit a6580e3cd8
12 changed files with 141 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
application/utils.inc.php
View File

@@ -283,7 +283,7 @@ class utils
*
* @since 2.5.2 2.6.0 new 'transaction_id' filter
* @since 2.7.0 new 'element_identifier' filter
* @since 2.7.7, 3.0.2, 3.1.0 new 'url' N°4899
* @since 2.7.7, 3.0.2, 3.1.0 N°4899 - new 'url' filter
*/
protected static function Sanitize_Internal($value, $sSanitizationFilter)
{

62
core/htmlsanitizer.class.inc.php
View File

@@ -116,6 +116,18 @@ abstract class DOMSanitizer extends HTMLSanitizer
{
/** @var DOMDocument */
protected $oDoc;
/**
* @var string Class to use for InlineImage static method calls
* @used-by \Combodo\iTop\Test\UnitTest\Core\Sanitizer\HTMLDOMSanitizerTest::testDoSanitizeCallInlineImageProcessImageTag
*/
protected $sInlineImageClassName;
public function __construct($sInlineImageClassName = InlineImage::class)
{
parent::__construct();
$this->sInlineImageClassName = $sInlineImageClassName;
}
abstract public function GetTagsWhiteList();
@@ -211,7 +223,7 @@ abstract class DOMSanitizer extends HTMLSanitizer
// Recurse
$this->CleanNode($oNode);
if (($oNode instanceof DOMElement) && (strtolower($oNode->tagName) == 'img')) {
InlineImage::ProcessImageTag($oNode);
$this->sInlineImageClassName::ProcessImageTag($oNode);
}
}
}
@@ -347,6 +359,30 @@ class HTMLDOMSanitizer extends DOMSanitizer
'white-space',
);
public function __construct($sInlineImageClassName = InlineImage::class)
{
parent::__construct($sInlineImageClassName);
// Building href validation pattern from url and email validation patterns as the patterns are not used the same way in HTML content than in standard attributes value.
// eg. "foo@bar.com" vs "mailto:foo@bar.com?subject=Title&body=Hello%20world"
if (!array_key_exists('href', self::$aAttrsWhiteList)) {
// Regular urls
$sUrlPattern = utils::GetConfig()->Get('url_validation_pattern');
// Mailto urls
$sMailtoPattern = '(mailto:('.utils::GetConfig()->Get('email_validation_pattern').')(?:\?(?:subject|body)=([a-zA-Z0-9+\$_.-]*)(?:&(?:subject|body)=([a-zA-Z0-9+\$_.-]*))?)?)';
// Notification placeholders
// eg. $this->caller_id$, $this->hyperlink()$, $this->hyperlink(portal)$, $APP_URL$, $MODULES_URL$, ...
// Note: Authorize both $xxx$ and %24xxx%24 as the latter one is encoded when used in HTML attributes (eg. a[href])
$sPlaceholderPattern = '(\$|%24)[\w-]*(->[\w]*(\([\w-]*?\))?)?(\$|%24)';
$sPattern = $sUrlPattern.'|'.$sMailtoPattern.'|'.$sPlaceholderPattern;
$sPattern = '/'.str_replace('/', '\/', $sPattern).'/i';
self::$aAttrsWhiteList['href'] = $sPattern;
}
}
public function GetTagsWhiteList()
{
return static::$aTagsWhiteList;
@@ -372,30 +408,6 @@ class HTMLDOMSanitizer extends DOMSanitizer
return static::$aStylesWhiteList;
}
public function __construct()
{
parent::__construct();
// Building href validation pattern from url and email validation patterns as the patterns are not used the same way in HTML content than in standard attributes value.
// eg. "foo@bar.com" vs "mailto:foo@bar.com?subject=Title&body=Hello%20world"
if (!array_key_exists('href', self::$aAttrsWhiteList)) {
// Regular urls
$sUrlPattern = utils::GetConfig()->Get('url_validation_pattern');
// Mailto urls
$sMailtoPattern = '(mailto:('.utils::GetConfig()->Get('email_validation_pattern').')(?:\?(?:subject|body)=([a-zA-Z0-9+\$_.-]*)(?:&(?:subject|body)=([a-zA-Z0-9+\$_.-]*))?)?)';
// Notification placeholders
// eg. $this->caller_id$, $this->hyperlink()$, $this->hyperlink(portal)$, $APP_URL$, $MODULES_URL$, ...
// Note: Authorize both $xxx$ and %24xxx%24 as the latter one is encoded when used in HTML attributes (eg. a[href])
$sPlaceholderPattern = '(\$|%24)[\w-]*(->[\w]*(\([\w-]*?\))?)?(\$|%24)';
$sPattern = $sUrlPattern.'|'.$sMailtoPattern.'|'.$sPlaceholderPattern;
$sPattern = '/'.str_replace('/', '\/', $sPattern).'/i';
self::$aAttrsWhiteList['href'] = $sPattern;
}
}
public function LoadDoc($sHTML)
{
@$this->oDoc->loadHTML('<?xml encoding="UTF-8"?>'.$sHTML); // For loading HTML chunks where the character set is not specified

21
datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php
View File

@@ -120,17 +120,6 @@ class ObjectFormManager extends FormManager
{
$aJson = static::DecodeFormManagerData($sJson);
$oConfig = utils::GetConfig();
$bIsContentCheckEnabled = $oConfig->GetModuleSetting(PORTAL_ID, 'enable_formmanager_content_check', true);
if ($bIsContentCheckEnabled && (false === $bTrustContent)) {
/** @noinspection NestedPositiveIfStatementsInspection */
if (isset($aJson['formproperties']['layout']['type']) && ($aJson['formproperties']['layout']['type'] === 'twig')) {
// There will be an IssueLog above in the hierarchy due to the exception, but we are logging here so that we can output the JSON data !
IssueLog::Error('Portal received a query with forbidden twig content!', \LogChannels::PORTAL, ['formmanager_data' => $aJson]);
throw new \SecurityException('Twig content not allowed in this context!');
}
}
/** @var \Combodo\iTop\Portal\Form\ObjectFormManager $oFormManager */
$oFormManager = parent::FromJSON($sJson);
@@ -1172,16 +1161,18 @@ class ObjectFormManager extends FormManager
$sObjectClass = get_class($this->oObject);
try {
// modification flags
$bIsNew = $this->oObject->IsNew();
$bWasModified = $this->oObject->IsModified();
$bActivateTriggers = (!$bIsNew && $bWasModified);
// Forcing allowed writing on the object if necessary. This is used in some particular cases.
$bAllowWrite = ($sObjectClass === 'Person' && $this->oObject->GetKey() == UserRights::GetContactId());
$bAllowWrite = $this->oContainer->get('security_helper')->IsActionAllowed($bIsNew ? UR_ACTION_CREATE : UR_ACTION_MODIFY, $sObjectClass, $this->oObject->GetKey());
if ($bAllowWrite) {
$this->oObject->AllowWrite(true);
}
// Writing object to DB
$bIsNew = $this->oObject->IsNew();
$bWasModified = $this->oObject->IsModified();
$bActivateTriggers = (!$bIsNew && $bWasModified);
try
{
$this->oObject->DBWrite();

6
datamodels/2.x/itop-portal-base/portal/src/Helper/SecurityHelper.php
View File

@@ -103,6 +103,12 @@ class SecurityHelper
return false;
}
// Forcing allowed writing on the object if necessary. This is used in some particular cases.
$bObjectIsCurrentUser = ($sObjectClass === 'Person' && $this->oObject->GetKey() == UserRights::GetContactId());
if(in_array($sAction , array(UR_ACTION_MODIFY, UR_ACTION_READ)) && $bObjectIsCurrentUser){
return true;
}
// Checking the scopes layer
// - Transforming scope action as there is only 2 values
$sScopeAction = ($sAction === UR_ACTION_READ) ? UR_ACTION_READ : UR_ACTION_MODIFY;

8
datamodels/2.x/itop-portal-base/portal/src/Twig/AppExtension.php
View File

@@ -99,7 +99,13 @@ class AppExtension extends AbstractExtension
return $sUrl;
});
//$filters[] = new TwigFilter('filter', 'twig_array_filter');
$filters[] = new Twig_SimpleFilter('filter', function ($array, $arrow) {
if ($arrow == 'system'){
return json_encode($array);
}
return twig_array_filter($array, $arrow);
});
return $filters;
}

84
js/forms-json-utils.js
View File

@@ -300,60 +300,62 @@ function ValidateField(sFieldId, sPattern, bMandatory, sFormId, nullValue, origi
function ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue)
{
if ($('#'+sFieldId).size() > 0) {
var bValid;
var sExplain = '';
var sTextContent;
if ($('#'+sFieldId).length === 0) {
return;
}
if ($('#'+sFieldId).prop('disabled')) {
bValid = true; // disabled fields are not checked
var bValid;
var sExplain = '';
var sTextContent;
if ($('#'+sFieldId).prop('disabled')) {
bValid = true; // disabled fields are not checked
} else {
// Get the contents without the tags
var oFormattedContents = $("#cke_"+sFieldId+" iframe");
if (oFormattedContents.length == 0) {
var oSourceContents = $("#cke_"+sFieldId+" textarea.cke_source");
sTextContent = oSourceContents.val();
} else {
// Get the contents without the tags
var oFormattedContents = $("#cke_"+sFieldId+" iframe");
if (oFormattedContents.length == 0) {
var oSourceContents = $("#cke_"+sFieldId+" textarea.cke_source");
sTextContent = oSourceContents.val();
} else {
sTextContent = oFormattedContents.contents().find("body").text();
sTextContent = oFormattedContents.contents().find("body").text();
if (sTextContent == '') {
// No plain text, maybe there is just an image...
var oImg = oFormattedContents.contents().find("body img");
if (oImg.length != 0) {
sTextContent = 'image';
}
if (sTextContent == '') {
// No plain text, maybe there is just an image...
var oImg = oFormattedContents.contents().find("body img");
if (oImg.length != 0) {
sTextContent = 'image';
}
}
}
// Get the original value without the tags
var oFormattedOriginalContents = (originalValue !== undefined) ? $('<div></div>').html(originalValue) : undefined;
var sTextOriginalContents = (oFormattedOriginalContents !== undefined) ? oFormattedOriginalContents.text() : undefined;
// Get the original value without the tags
var oFormattedOriginalContents = (originalValue !== undefined) ? $('<div></div>').html(originalValue) : undefined;
var sTextOriginalContents = (oFormattedOriginalContents !== undefined) ? oFormattedOriginalContents.text() : undefined;
if (bMandatory && (sTextContent == nullValue)) {
bValid = false;
if (bMandatory && (sTextContent == nullValue)) {
bValid = false;
sExplain = Dict.S('UI:ValueMustBeSet');
} else if ((sTextOriginalContents != undefined) && (sTextContent == sTextOriginalContents)) {
bValid = false;
if (sTextOriginalContents == nullValue) {
sExplain = Dict.S('UI:ValueMustBeSet');
} else if ((sTextOriginalContents != undefined) && (sTextContent == sTextOriginalContents)) {
bValid = false;
if (sTextOriginalContents == nullValue) {
sExplain = Dict.S('UI:ValueMustBeSet');
} else {
// Note: value change check is not working well yet as the HTML to Text conversion is not exactly the same when done from the PHP value or the CKEditor value.
sExplain = Dict.S('UI:ValueMustBeChanged');
}
} else {
bValid = true;
// Note: value change check is not working well yet as the HTML to Text conversion is not exactly the same when done from the PHP value or the CKEditor value.
sExplain = Dict.S('UI:ValueMustBeChanged');
}
} else {
bValid = true;
}
}
ReportFieldValidationStatus(sFieldId, sFormId, bValid, sExplain);
ReportFieldValidationStatus(sFieldId, sFormId, bValid, sExplain);
if ($('#'+sFieldId).data('timeout_validate') == undefined) {
// We need to check periodically as CKEditor doesn't trigger our events. More details in UIHTMLEditorWidget::Display() @ line 92
var iTimeoutValidate = setInterval(function () {
ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue);
}, 500);
$('#'+sFieldId).data('timeout_validate', iTimeoutValidate);
}
if ($('#'+sFieldId).data('timeout_validate') == undefined) {
// We need to check periodically as CKEditor doesn't trigger our events. More details in UIHTMLEditorWidget::Display() @ line 92
var iTimeoutValidate = setInterval(function () {
ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue);
}, 500);
$('#'+sFieldId).data('timeout_validate', iTimeoutValidate);
}
}

2
pages/UI.php
View File

@@ -1306,7 +1306,7 @@ HTML
$sWarnings = implode(', ', $aWarnings);
$oP->AddHeaderMessage($sWarnings, 'message_info');
}
cmdbAbstractObject::DisplayCreationForm($oP, $sClass, $oObj);
cmdbAbstractObject::DisplayCreationForm($oP, $sClass, $oObj, [], ['transaction_id' => $sTransactionId]);
$oP->add(<<<HTML
</div><!-- End of wizContainer -->
</div><!-- End of object-details -->

39
test/application/UtilsTest.php
View File

@@ -19,10 +19,12 @@
*
*/
use Combodo\iTop\Test\UnitTest\ItopTestCase;
/**
* @covers utils
*/
class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopDataTestCase
class UtilsTest extends ItopTestCase
{
public function testEndsWith()
{
@@ -466,20 +468,27 @@ class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopDataTestCase
public function sanitizerDataProvider()
{
return [
'good integer' => ['integer', '2565', '2565'],
'bad integer' => ['integer', 'a2656', '2656'],
'good class' => ['class', 'UserRequest', 'UserRequest'],
'bad class' => ['class', 'MyUserRequest',null],
'good string' => ['string', 'Is Peter smart and funny?', 'Is Peter smart and funny?'],
'bad string' => ['string', 'Is Peter <smart> & funny?', 'Is Peter &#60;smart&#62; &#38; funny?'],
'good transaction_id' => ['transaction_id', '8965.-dd', '8965.-dd'],
'bad transaction_id' => ['transaction_id', '8965.-dd+', null],
'good parameter' => ['parameter', 'JU8965-dd=_', 'JU8965-dd=_'],
'bad parameter' => ['parameter', '8965.-dd+', null],
'good field_name' => ['field_name', 'Name->bUzz38', 'Name->bUzz38'],
'bad field_name' => ['field_name', 'name-buzz', null],
'good context_param' => ['context_param', '%dssD25_=%:+-', '%dssD25_=%:+-'],
'bad context_param' => ['context_param', '%dssD,25_=%:+-', null],
'good integer' => ['integer', '2565', '2565'],
'bad integer' => ['integer', 'a2656', '2656'],
/**
* 'class' filter needs a loaded datamodel... and is only an indirection to \MetaModel::IsValidClass so might very important to test !
* If we switch this class to ItopDataTestCase then we are seeing :
* - the class now takes 18s to process instead of... 459ms when using ItopTestCase !!!
* - multiple errors are thrown in testGetAbsoluteUrlAppRootPersistency :(
* We decided it wasn't worse the effort to test the 'class' filter !
*/
// 'good class' => ['class', 'UserRequest', 'UserRequest'],
// 'bad class' => ['class', 'MyUserRequest',null],
'good string' => ['string', 'Is Peter smart and funny?', 'Is Peter smart and funny?'],
'bad string' => ['string', 'Is Peter <smart> & funny?', 'Is Peter &#60;smart&#62; &#38; funny?'],
'good transaction_id' => ['transaction_id', '8965.-dd', '8965.-dd'],
'bad transaction_id' => ['transaction_id', '8965.-dd+', null],
'good parameter' => ['parameter', 'JU8965-dd=_', 'JU8965-dd=_'],
'bad parameter' => ['parameter', '8965.-dd+', null],
'good field_name' => ['field_name', 'Name->bUzz38', 'Name->bUzz38'],
'bad field_name' => ['field_name', 'name-buzz', null],
'good context_param' => ['context_param', '%dssD25_=%:+-', '%dssD25_=%:+-'],
'bad context_param' => ['context_param', '%dssD,25_=%:+-', null],
'good element_identifier' => ['element_identifier', 'AD05nb', 'AD05nb'],
'bad element_identifier' => ['element_identifier', 'AD05nb+', 'AD05nb'],
'good url' => ['url', 'https://www.w3schools.com', 'https://www.w3schools.com'],

1
test/core/sanitizer/AbstractDOMSanitizerTest.php
View File

@@ -14,7 +14,6 @@ abstract class AbstractDOMSanitizerTest extends ItopTestCase
parent::setUp();
require_once(APPROOT.'application/utils.inc.php');
require_once(APPROOT.'core/htmlsanitizer.class.inc.php');
require_once(APPROOT.'test/core/sanitizer/InlineImageMock.php');
}
protected function ReadTestFile($sFileToTest, $sFolderName)

12
test/core/sanitizer/HTMLDOMSanitizerTest.php
View File

@@ -2,16 +2,12 @@
namespace Combodo\iTop\Test\UnitTest\Core\Sanitizer;
use HTMLDOMSanitizer;
use InlineImageMock;
require_once __DIR__.'/AbstractDOMSanitizerTest.php';
/**
* @runTestsInSeparateProcesses
* @preserveGlobalState disabled
* @backupGlobals disabled
*/
class HTMLDOMSanitizerTest extends AbstractDOMSanitizerTest
{
/**
@@ -222,15 +218,17 @@ class HTMLDOMSanitizerTest extends AbstractDOMSanitizerTest
/**
* @dataProvider CallInlineImageProcessImageTagProvider
* @uses \InlineImageMock
*/
public function testDoSanitizeCallInlineImageProcessImageTag($sHtml, $iExpectedCount)
{
require_once APPROOT.'test/core/sanitizer/InlineImageMock.php';
InlineImageMock::ResetCallCounter();
$oSanitizer = new HTMLDOMSanitizer();
$oSanitizer = new HTMLDOMSanitizer(InlineImageMock::class);
$oSanitizer->DoSanitize($sHtml);
$iCalledCount = \InlineImage::GetCallCounter();
$iCalledCount = \InlineImageMock::GetCallCounter();
$this->assertEquals($iExpectedCount, $iCalledCount);
}

12
test/core/sanitizer/InlineImageMock.php
View File

@@ -1,4 +1,6 @@
<?php
/** @noinspection PhpUnused */
/** @noinspection PhpIllegalPsrClassPathInspection */
/**
* Copyright (C) 2010-2021 Combodo SARL
*
@@ -20,10 +22,11 @@
*/
/**
* Mock class used by @see \Combodo\iTop\Test\UnitTest\Core\HTMLDOMSanitizerTest
* Mock class used to count number of calls for the ProcessImage static method
*
* @used-by \Combodo\iTop\Test\UnitTest\Core\Sanitizer\HTMLDOMSanitizerTest::testDoSanitizeCallInlineImageProcessImageTag()
*/
class InlineImage
class InlineImageMock
{
private static $iCallCounter = 0;
@@ -32,6 +35,11 @@ class InlineImage
self::$iCallCounter++;
}
public static function ResetCallCounter()
{
self::$iCallCounter = 0;
}
public static function GetCallCounter()
{
return self::$iCallCounter;

2
test/unittestautoload.php
View File

@@ -1,5 +1,5 @@
<?php
include('./vendor/autoload.php');
include('vendor/autoload.php');
include('ItopTestCase.php');
include('ItopDataTestCase.php');