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

N°8532 - Apply filters on all DBSearch classes (#848)

Browse Source 
Co-authored-by: Molkobain <lajarige.guillaume@free.fr>
Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
This commit is contained in:
Stephen Abello
2026-03-31 15:41:28 +02:00
committed by GitHub
parent 7791585387
commit e467ca83cf
5 changed files with 290 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
core/config.class.inc.php
View File

@@ -1730,6 +1730,14 @@ class Config
'source_of_value' => '',
'show_in_conf_sample' => false,
],
'security.disable_joined_classes_filter' => [
'type' => 'bool',
'description' => 'If true, scope filters aren\'t applied to joined classes or union classes not directly listed in the SELECT clause.',
'default' => true,
'value' => true,
'source_of_value' => '',
'show_in_conf_sample' => false,
],
'security.hide_administrators' => [
'type' => 'bool',
'description' => 'If true, non-administrator users will not be able to see the administrator accounts, the Administrator profile and the links between the administrator accounts and their profiles.',

33
core/dbobjectsearch.class.php
View File

@@ -1925,4 +1925,37 @@ class DBObjectSearch extends DBSearch
{
return $this->GetCriteria()->ListParameters();
}
/**
* @inheritDoc
* @return DBObjectSearch
*/
protected function ApplyDataFilters(): DBObjectSearch
{
if ($this->IsAllDataAllowed() || $this->IsDataFiltered()) {
return $this;
}
$oSearch = $this;
$aClassesToFilter = $this->GetSelectedClasses();
// Opt-in for joined classes filtering, otherwise only filter the selected class(es)
if (MetaModel::GetConfig()->Get('security.disable_joined_classes_filter') === false) {
$aClassesToFilter = $this->GetJoinedClasses();
}
// Apply filter (this is similar to the one in DBSearch but the factorization could make it less readable)
foreach ($aClassesToFilter as $sClassAlias => $sClass) {
$oVisibleObjects = UserRights::GetSelectFilter($sClass, $this->GetModifierProperties('UserRightsGetSelectFilter'));
if ($oVisibleObjects === false) {
$oVisibleObjects = DBObjectSearch::FromEmptySet($sClass);
}
if (is_object($oVisibleObjects)) {
$oVisibleObjects->AllowAllData();
$oSearch = $oSearch->Filter($sClassAlias, $oVisibleObjects);
$oSearch->SetDataFiltered();
}
}
return $oSearch;
}
}

45
core/dbsearch.class.php
View File

@@ -1122,21 +1122,7 @@ abstract class DBSearch
*/
protected function GetSQLQuery($aOrderBy, $aArgs, $aAttToLoad, $aExtendedDataSpec, $iLimitCount, $iLimitStart, $bGetCount, $aGroupByExpr = null, $aSelectExpr = null)
{
$oSearch = $this;
if (!$this->IsAllDataAllowed() && !$this->IsDataFiltered()) {
foreach ($this->GetSelectedClasses() as $sClassAlias => $sClass) {
$oVisibleObjects = UserRights::GetSelectFilter($sClass, $this->GetModifierProperties('UserRightsGetSelectFilter'));
if ($oVisibleObjects === false) {
// Make sure this is a valid search object, saying NO for all
$oVisibleObjects = DBObjectSearch::FromEmptySet($sClass);
}
if (is_object($oVisibleObjects)) {
$oVisibleObjects->AllowAllData();
$oSearch = $oSearch->Filter($sClassAlias, $oVisibleObjects);
$oSearch->SetDataFiltered();
}
}
}
$oSearch = $this->ApplyDataFilters();
if (is_array($aGroupByExpr)) {
foreach ($aGroupByExpr as $sAlias => $oGroupByExp) {
@@ -1608,4 +1594,33 @@ abstract class DBSearch
* @return array{\VariableExpression}
*/
abstract public function GetExpectedArguments(): array;
/**
* Apply data filters to the search, if needed
*
* @return DBSearch
* @throws CoreException
*/
protected function ApplyDataFilters(): DBSearch
{
if ($this->IsAllDataAllowed() || $this->IsDataFiltered()) {
return $this;
}
$oSearch = $this;
$aClassesToFilter = $this->GetSelectedClasses();
foreach ($aClassesToFilter as $sClassAlias => $sClass) {
$oVisibleObjects = UserRights::GetSelectFilter($sClass, $this->GetModifierProperties('UserRightsGetSelectFilter'));
if ($oVisibleObjects === false) {
$oVisibleObjects = DBObjectSearch::FromEmptySet($sClass);
}
if (is_object($oVisibleObjects)) {
$oVisibleObjects->AllowAllData();
$oSearch = $oSearch->Filter($sClassAlias, $oVisibleObjects);
$oSearch->SetDataFiltered();
}
}
return $oSearch;
}
}

26
core/dbunionsearch.class.php
View File

@@ -673,4 +673,30 @@ class DBUnionSearch extends DBSearch
return $aVariableCriteria;
}
/**
* @inheritDoc
* @return DBUnionSearch
*/
protected function ApplyDataFilters(): DBUnionSearch
{
if ($this->IsAllDataAllowed() || $this->IsDataFiltered()) {
return $this;
}
// Opt-in for joined classes filtering, otherwise fallback on DBSearch filtering
if (MetaModel::GetConfig()->Get('security.disable_joined_classes_filter') === true) {
return parent::ApplyDataFilters();
}
// Apply filters per sub-search
$aFilteredSearches = [];
foreach ($this->GetSearches() as $oSubSearch) {
// Recursively call ApplyDataFilters on sub-searches
$aFilteredSearches[] = $oSubSearch->ApplyDataFilters();
}
$oSearch = new DBUnionSearch($aFilteredSearches);
return $oSearch;
}
}

193
tests/php-unit-tests/unitary-tests/core/DBSearchFilterJoinTest.php Normal file
View File

@@ -0,0 +1,193 @@
<?php
namespace Combodo\iTop\Test\UnitTest\Core;
use CMDBSource;
use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
use DBObjectSearch;
use DBObjectSet;
use DBSearch;
use lnkFunctionalCIToTicket;
use MetaModel;
use ormLinkSet;
use UserRequest;
use UserRights;
class DBSearchFilterJoinTest extends ItopDataTestCase
{
private const RESTRICTED_PROFILE = 'Configuration Manager';
private $aData = [];
protected function setUp(): void
{
parent::setUp();
$this->RequireOnceItopFile('application/startup.inc.php');
$this->aData = $this->CreateDBSearchFilterTestData();
DBSearch::EnableQueryCache(false, false);
$this->LoginRestrictedUser($this->aData['allowed_org_id'], self::RESTRICTED_PROFILE);
}
protected function tearDown(): void
{
parent::tearDown();
}
/**
* @dataProvider JoinedAndNestedOqlProvider
*/
public function testDBSearchFilterAppliedToJoinsWhenEnabled(string $sOql, int $iExpectedCount): void
{
$this->EnableJoinFilterConfig(true);
$oSearch = DBObjectSearch::FromOQL($sOql, ['denied_org' => $this->aData['denied_org_name'], 'allowed_org' => $this->aData['allowed_org_name']]);
$oSet = new \DBObjectSet($oSearch);
CMDBSource::TestQuery($oSearch->MakeSelectQuery());
$this->assertEquals($iExpectedCount, $oSet->Count());
}
/**
* @dataProvider JoinedAndNestedOqlProvider
*/
public function testDBSearchFilterAppliedToJoinsWhenDisabled(string $sOql, int $iExpectedCount, int $iExpectedDisabledCount): void
{
$this->EnableJoinFilterConfig(false);
$oSearch = DBObjectSearch::FromOQL($sOql, ['denied_org' => $this->aData['denied_org_name'], 'allowed_org' => $this->aData['allowed_org_name']]);
$oSet = new \DBObjectSet($oSearch);
CMDBSource::TestQuery($oSearch->MakeSelectQuery());
$this->assertEquals($iExpectedDisabledCount, $oSet->Count());
}
/**
* @dataProvider JoinedAndNestedOqlProvider
*/
public function testAllowAllDataBypassesDBSearchFilterWhenEnabled(string $sOql, int $iExpectedCount, int $iExpectedDisabledCount): void
{
$this->EnableJoinFilterConfig(true);
$oSearch = DBObjectSearch::FromOQL($sOql, ['denied_org' => $this->aData['denied_org_name'], 'allowed_org' => $this->aData['allowed_org_name']]);
$oSearch->AllowAllData();
$oSet = new \DBObjectSet($oSearch);
CMDBSource::TestQuery($oSearch->MakeSelectQuery());
$this->assertEquals($iExpectedDisabledCount, $oSet->Count());
}
/**
* @dataProvider JoinedAndNestedOqlProvider
*/
public function testAllowAllDataBypassesDBSearchFilterWhenDisabled(string $sOql, int $iExpectedCount, int $iExpectedDisabledCount): void
{
$this->EnableJoinFilterConfig(false);
$oSearch = DBObjectSearch::FromOQL($sOql, ['denied_org' => $this->aData['denied_org_name'], 'allowed_org' => $this->aData['allowed_org_name']]);
$oSearch->AllowAllData();
$oSet = new \DBObjectSet($oSearch);
CMDBSource::TestQuery($oSearch->MakeSelectQuery());
$this->assertEquals($iExpectedDisabledCount, $oSet->Count());
}
public function JoinedAndNestedOqlProvider(): array
{
return [
'join-filter-on-org' => [
'oql' => "SELECT OSF FROM OSFamily AS OSF JOIN VirtualMachine AS VM ON VM.osfamily_id = OSF.id JOIN Organization AS O ON VM.org_id = O.id WHERE O.name = :denied_org",
'expected_filtered_count' => 0,
'expected_unfiltered_count' => 1,
],
'nested-in-select' => [
'oql' => "SELECT OSF FROM OSFamily AS OSF WHERE OSF.id IN (SELECT OSF1 FROM OSFamily AS OSF1 JOIN VirtualMachine AS VM ON VM.osfamily_id = OSF1.id JOIN Organization AS O ON VM.org_id = O.id WHERE O.name = :denied_org)",
'expected_filtered_count' => 0,
'expected_unfiltered_count' => 1,
],
'userrequest-join-person-org' => [
'oql' => "SELECT OSF FROM OSFamily AS OSF JOIN VirtualMachine AS VM ON VM.osfamily_id = OSF.id JOIN lnkFunctionalCIToTicket AS L ON L.functionalci_id = VM.id JOIN UserRequest AS UR ON L.ticket_id = UR.id JOIN Person AS P ON UR.caller_id = P.id JOIN Organization AS O ON P.org_id = O.id WHERE O.name = :denied_org",
'expected_filtered_count' => 0,
'expected_unfiltered_count' => 1,
],
'union-join-filter-on-org' => [
'oql' => "SELECT OSF FROM OSFamily AS OSF JOIN VirtualMachine AS VM ON VM.osfamily_id = OSF.id JOIN Organization AS O ON VM.org_id = O.id WHERE O.name = :denied_org UNION SELECT OSF2 FROM OSFamily AS OSF2 JOIN VirtualMachine AS VM2 ON VM2.osfamily_id = OSF2.id JOIN Organization AS O2 ON VM2.org_id = O2.id WHERE O2.name = :allowed_org",
'expected_filtered_count' => 1,
'expected_unfiltered_count' => 2,
],
];
}
private function EnableJoinFilterConfig(bool $bEnabled): void
{
$oConfig = MetaModel::GetConfig();
$oConfig->Set('security.disable_joined_classes_filter', !$bEnabled);
}
private function CreateDBSearchFilterTestData(): array
{
$sSuffix = 'DBSearchFilterJoinTest';
$sAllowedOrgName = 'DBSearchFilterAllowedOrg-'.$sSuffix;
$iAllowedOrgId = $this->GivenObjectInDB('Organization', [
'name' => $sAllowedOrgName,
]);
$this->debug("Org allowed id: $iAllowedOrgId");
$sDeniedOrgName = 'DBSearchFilterDeniedOrg-'.$sSuffix;
$iDeniedOrgId = $this->GivenObjectInDB('Organization', [
'name' => $sDeniedOrgName,
]);
$this->debug("Org denied id: $iDeniedOrgId");
$iDeniedOsFamilyId = $this->GivenObjectInDB('OSFamily', [
'name' => 'DBSearchFilterOsFamilyDenied-'.$sSuffix,
]);
$iAllowedOsFamilyId = $this->GivenObjectInDB('OSFamily', [
'name' => 'DBSearchFilterOsFamilyAllowed-'.$sSuffix,
]);
$iDeniedVMId = $this->GivenObjectInDB('VirtualMachine', [
'name' => 'DBSearchFilterVmDenied-'.$sSuffix,
'org_id' => $iDeniedOrgId,
'osfamily_id' => $iDeniedOsFamilyId,
'virtualhost_id' => 1,
]);
$iVirtualHostId = $this->GivenObjectInDB('Hypervisor', [
'name' => 'DBSearchFilterVHost-'.$sSuffix,
'org_id' => $iAllowedOrgId,
]);
$this->GivenObjectInDB('VirtualMachine', [
'name' => 'DBSearchFilterVmAllowed-'.$sSuffix,
'org_id' => $iAllowedOrgId,
'osfamily_id' => $iAllowedOsFamilyId,
'virtualhost_id' => $iVirtualHostId,
]);
$oDeniedPerson = $this->CreatePerson('Denied-'.$sSuffix, $iDeniedOrgId);
$oUserRequest = $this->CreateUserRequest('Denied'.$sSuffix, [
'caller_id' => $oDeniedPerson->GetKey(),
'org_id' => $iDeniedOrgId,
]);
// Add Virtual Machine to UserRequest lnk
$oLinkSet = new ormLinkSet(UserRequest::class, 'functionalcis_list', DBObjectSet::FromScratch(lnkFunctionalCIToTicket::class));
$oLink = MetaModel::NewObject(lnkFunctionalCIToTicket::class, ['functionalci_id' => $iDeniedVMId]);
$oLinkSet->AddItem($oLink);
$oUserRequest->Set('functionalcis_list', $oLinkSet);
$oUserRequest->DBUpdate();
return [
'allowed_org_id' => $iAllowedOrgId,
'allowed_org_name' => $sAllowedOrgName,
'denied_org_name' => $sDeniedOrgName,
];
}
private function LoginRestrictedUser(int $iAllowedOrgId, string $sProfileName): void
{
$sLogin = $this->GivenUserRestrictedToAnOrganizationInDB($iAllowedOrgId, self::$aURP_Profiles[$sProfileName]);
UserRights::Login($sLogin);
}
}