From bf4835eec0fac72afeb2b31b6ab83e306322c4ac Mon Sep 17 00:00:00 2001 From: "denis.flaven@combodo.com" Date: Wed, 6 Oct 2021 15:34:17 +0200 Subject: [PATCH] =?UTF-8?q?N=C2=B04354=20-=20=20Hide=20Administrator=20pro?= =?UTF-8?q?file=20to=20non-admins?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../userrightsprofile.class.inc.php | 116 +++++++++++-- core/config.class.inc.php | 8 + core/userrights.class.inc.php | 2 +- .../model.authent-external.php | 2 +- .../authent-ldap/datamodel.authent-ldap.xml | 2 +- .../2.x/authent-local/model.authent-local.php | 2 +- test/core/GetSelectFilterTest.php | 155 ++++++++++++++++++ 7 files changed, 266 insertions(+), 21 deletions(-) create mode 100644 test/core/GetSelectFilterTest.php diff --git a/addons/userrights/userrightsprofile.class.inc.php b/addons/userrights/userrightsprofile.class.inc.php index 181e35d73..60473f899 100644 --- a/addons/userrights/userrightsprofile.class.inc.php +++ b/addons/userrights/userrightsprofile.class.inc.php @@ -34,7 +34,7 @@ class URP_Profiles extends UserRightsBaseClassGUI { $aParams = array ( - "category" => "addon/userrights,grant_by_profile", + "category" => "addon/userrights,grant_by_profile,silo", "key_type" => "autoincrement", "name_attcode" => "name", "state_attcode" => "", @@ -219,7 +219,7 @@ class URP_UserProfile extends UserRightsBaseClassGUI { $aParams = array ( - "category" => "addon/userrights,grant_by_profile", + "category" => "addon/userrights,grant_by_profile,silo", "key_type" => "autoincrement", "name_attcode" => array("userlogin", "profile"), "state_attcode" => "", @@ -610,30 +610,112 @@ class UserRightsProfile extends UserRightsAddOnAPI { $this->LoadCache(); - $aObjectPermissions = $this->GetUserActionGrant($oUser, $sClass, UR_ACTION_READ); - if ($aObjectPermissions['permission'] == UR_ALLOWED_NO) + if (!static::IsAdministrator($oUser)) // Let us pass an administrator for testing without the need of setting up complex profile { - return false; + $aObjectPermissions = $this->GetUserActionGrant($oUser, $sClass, UR_ACTION_READ); + if ($aObjectPermissions['permission'] == UR_ALLOWED_NO) + { + return false; + } } - // Determine how to position the objects of this class - // + $oFilter = true; + $aConditions = array(); + + // Determine if this class is part of a silo and build the filter for it $sAttCode = self::GetOwnerOrganizationAttCode($sClass); - if (is_null($sAttCode)) + if (!is_null($sAttCode)) { - // No filtering for this object - return true; + $aUserOrgs = $this->GetUserOrgs($oUser, $sClass); + if (count($aUserOrgs) > 0) + { + $oFilter = $this->MakeSelectFilter($sClass, $aUserOrgs, $aSettings, $sAttCode); + } + // else: No org means 'any org' } - // Position the user - // - $aUserOrgs = $this->GetUserOrgs($oUser, $sClass); - if (count($aUserOrgs) == 0) + // else: No silo for this class + + // Specific conditions to hide, for non-administrators, the Administrator Users, the Administrator Profile and related links + // Note: when logged as an administrator, GetSelectFilter is completely bypassed. + if ($this->AdministratorsAreHidden()) { - // No org means 'any org' - return true; + if ($sClass == 'URP_Profiles') + { + $oExpression = new FieldExpression('id', $sClass); + $oScalarExpr = new ScalarExpression(1); + + $aConditions[] = new BinaryExpression($oExpression, '!=', $oScalarExpr); + } + else if (($sClass == 'URP_UserProfile') || ($sClass == 'User') || (is_subclass_of($sClass, 'User'))) + { + $aAdministrators = $this->GetAdministrators(); + if (count($aAdministrators) > 0) + { + $sAttCode = ($sClass == 'URP_UserProfile') ? 'userid' : 'id'; + $oExpression = new FieldExpression($sAttCode, $sClass); + $oListExpr = ListExpression::FromScalars($aAdministrators); + $aConditions[] = new BinaryExpression($oExpression, 'NOT IN', $oListExpr); + } + } } - return $this->MakeSelectFilter($sClass, $aUserOrgs, $aSettings, $sAttCode); + // Handling of the added conditions + if (count($aConditions) > 0) + { + if($oFilter === true) + { + // No 'silo' filter, let's build a clean one + $oFilter = new DBObjectSearch($sClass); + } + + // Add the conditions to the filter + foreach($aConditions as $oCondition) + { + $oFilter->AddConditionExpression($oCondition); + } + } + + return $oFilter; + } + + /** + * Retrieve (and memoize) the list of administrator accounts. + * Note that there should always be at least one administrator account + * @return number[] + */ + private function GetAdministrators() + { + static $aAdministrators = null; + + if ($aAdministrators === null) + { + // Find all administrators + $aAdministrators = array(); + $oAdministratorsFilter = new DBObjectSearch('User'); + $oLnkFilter = new DBObjectSearch('URP_UserProfile'); + $oExpression = new FieldExpression('profileid', 'URP_UserProfile'); + $oScalarExpr = new ScalarExpression(1); + $oCondition = new BinaryExpression($oExpression, '=', $oScalarExpr); + $oLnkFilter->AddConditionExpression($oCondition); + $oAdministratorsFilter->AddCondition_ReferencedBy($oLnkFilter, 'userid'); + $oAdministratorsFilter->AllowAllData(true); // Mandatory to prevent infinite recursion !! + $oSet = new DBObjectSet($oAdministratorsFilter); + $oSet->OptimizeColumnLoad(array('User' => array('login'))); + while($oUser = $oSet->Fetch()) + { + $aAdministrators[] = $oUser->GetKey(); + } + } + return $aAdministrators; + } + + /** + * Whether or not to hide the 'Administrator' profile and the administrator accounts + * @return boolean + */ + private function AdministratorsAreHidden() + { + return ((bool)MetaModel::GetConfig()->Get('security.hide_administrators')); } diff --git a/core/config.class.inc.php b/core/config.class.inc.php index b98f877b9..0361504ff 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -1503,6 +1503,14 @@ class Config '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.', + 'default' => false, + 'value' => false, + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ], ]; public function IsProperty($sPropCode) diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php index 4987fb2fe..cd1960561 100644 --- a/core/userrights.class.inc.php +++ b/core/userrights.class.inc.php @@ -638,7 +638,7 @@ abstract class UserInternal extends User { $aParams = array ( - "category" => "core,grant_by_profile", + "category" => "core,grant_by_profile,silo", "key_type" => "autoincrement", "name_attcode" => "login", "state_attcode" => "", diff --git a/datamodels/2.x/authent-external/model.authent-external.php b/datamodels/2.x/authent-external/model.authent-external.php index 139c5de25..c830ea30d 100755 --- a/datamodels/2.x/authent-external/model.authent-external.php +++ b/datamodels/2.x/authent-external/model.authent-external.php @@ -37,7 +37,7 @@ class UserExternal extends User { $aParams = array ( - "category" => "addon/authentication,grant_by_profile", + "category" => "addon/authentication,grant_by_profile,silo", "key_type" => "autoincrement", "name_attcode" => "login", "state_attcode" => "", diff --git a/datamodels/2.x/authent-ldap/datamodel.authent-ldap.xml b/datamodels/2.x/authent-ldap/datamodel.authent-ldap.xml index a6594147f..ea8021382 100644 --- a/datamodels/2.x/authent-ldap/datamodel.authent-ldap.xml +++ b/datamodels/2.x/authent-ldap/datamodel.authent-ldap.xml @@ -14,7 +14,7 @@ * @copyright Copyright (C) 2010-2020 Combodo SARL * @license http://opensource.org/licenses/AGPL-3.0 */ - addon/authentication,grant_by_profile + addon/authentication,grant_by_profile,silo false autoincrement priv_user_ldap diff --git a/datamodels/2.x/authent-local/model.authent-local.php b/datamodels/2.x/authent-local/model.authent-local.php index 23e8cf333..86b30120c 100755 --- a/datamodels/2.x/authent-local/model.authent-local.php +++ b/datamodels/2.x/authent-local/model.authent-local.php @@ -77,7 +77,7 @@ class UserLocal extends UserInternal { $aParams = array ( - "category" => "addon/authentication,grant_by_profile", + "category" => "addon/authentication,grant_by_profile,silo", "key_type" => "autoincrement", "name_attcode" => "login", "state_attcode" => "", diff --git a/test/core/GetSelectFilterTest.php b/test/core/GetSelectFilterTest.php new file mode 100644 index 000000000..36e5e7988 --- /dev/null +++ b/test/core/GetSelectFilterTest.php @@ -0,0 +1,155 @@ + 'REST Services User'), true); + $oAdminProfile = MetaModel::GetObjectFromOQL("SELECT URP_Profiles WHERE name = :name", array('name' => 'Administrator'), true); + + $this->sLogin = "getselectfilter-user-" . date('dmYHis'); + + // Ensure that we have at least one administrator account + if (is_object($oRestProfile) && is_object($oAdminProfile)) + { + $this->oUser = $this->CreateUser($this->sLogin, $oRestProfile->GetKey(), $this->sPassword); + $this->AddProfileToUser($this->oUser, $oAdminProfile->GetKey()); + } + } + + public function testGetSelectFilter() + { + $oUserRights = new UserRightsProfile(); + $aClasses = get_declared_classes(); + $aUserClasses = ['User']; + $aUserLocalAncestors = ['User', 'UserInternal', 'UserLocal']; + foreach($aClasses as $sClass) + { + if (is_subclass_of($sClass, 'User')) + { + $aUserClasses[] = $sClass; + } + } + + $oConfig = MetaModel::GetConfig(); + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // Default behavior: Administrators, Administrator profile and URP_UserProfile related to administrators are visible + // via GetSelectFilter + + $oConfig->Set('security.hide_administrators', false); + + $oFilterProfiles = $oUserRights->GetSelectFilter($this->oUser, 'URP_Profiles'); + if ($oFilterProfiles === true) + { + $oFilterProfiles = new DBObjectSearch('URP_Profiles'); + } + $oSet = new DBObjectSet($oFilterProfiles); + $bAdminProfileFound = false; + while($oProfile = $oSet->Fetch()) + { + if ($oProfile->GetKey() == 1) + { + $bAdminProfileFound = true; + break; + } + } + $this->assertEquals($bAdminProfileFound, true); + + foreach($aUserLocalAncestors as $sUserClass) + { + $bAdminUserFound = false; + $oFilterUser = $oUserRights->GetSelectFilter($this->oUser,$sUserClass); + if ($oFilterUser === true) + { + $oFilterUser = new DBObjectSearch($sUserClass); + } + $oSet = new DBObjectSet($oFilterUser); + while($oUser = $oSet->Fetch()) + { + if($oUser->GetKey() == $this->oUser->GetKey()) + { + $bAdminUserFound = true; + break; + } + } + $this->assertEquals($bAdminUserFound, true); + } + + $oFilterLnkProfiles = $oUserRights->GetSelectFilter($this->oUser, 'URP_UserProfile'); + if ($oFilterLnkProfiles === true) + { + $oFilterLnkProfiles = new DBObjectSearch('URP_UserProfile'); + } + $oSet = new DBObjectSet($oFilterLnkProfiles); + // There should some lnk referencing either our administrator account or the Administrator profile + $bUserFound = false; + $bProfileFound = false; + while($oLnk = $oSet->Fetch()) + { + if($oLnk->Get('userid') == $this->oUser->GetKey()) + { + $bUserFound = true; + } + if($oLnk->Get('profileid') == 1) + { + $bProfileFound = true; + } + } + $this->assertEquals($bUserFound, true); + $this->assertEquals($bProfileFound, true); + + + ////////////////////////////////////////////////////////////////////////////////////////////////////// + // Administrator account, Administrator profile and URP_UserProfile related to administrators are now hidden + // via GetSelectFilter + $oConfig->Set('security.hide_administrators', true); + + $oFilterProfiles = $oUserRights->GetSelectFilter($this->oUser, 'URP_Profiles'); + $this->assertNotEquals($oFilterProfiles,true); // This class must be filtered + $oSet = new DBObjectSet($oFilterProfiles); + while($oProfile = $oSet->Fetch()) + { + $this->assertNotEquals($oProfile->GetKey(), 1); // No profile should have id = 1 (Administrator) + } + foreach($aUserClasses as $sUserClass) + { + $oFilterUser = $oUserRights->GetSelectFilter($this->oUser, $sUserClass); + $this->assertNotEquals($oFilterUser,true); // This class must be filtered + $oSet = new DBObjectSet($oFilterUser); + while($oUser = $oSet->Fetch()) + { + $this->assertNotEquals($oUser->GetKey(), $this->oUser->GetKey()); // Our administrator account should not be visible + } + } + + $oFilterLnkProfiles = $oUserRights->GetSelectFilter($this->oUser, 'URP_UserProfile'); + $this->assertNotEquals($oFilterLnkProfiles,true); // This class must be filtered + $oSet = new DBObjectSet($oFilterLnkProfiles); + // There should be no lnk referencing either our administrator account or the profile Administrator + while($oLnk = $oSet->Fetch()) + { + $this->assertNotEquals($oLnk->Get('userid'), $this->oUser->GetKey()); + $this->assertNotEquals($oLnk->Get('profileid'), 1); + } + + } +} \ No newline at end of file