N°1436 - Access control updated for grant_by_profile categories of classes -

Fix access to internal classes form the core engine

SVN:trunk[5903]

addons/userrights/userrightsprofile.class.inc.php

@@ -203,6 +203,12 @@ class URP_Profiles extends UserRightsBaseClassGUI
 	// preserve DB integrity by deleting links to users
 	protected function OnDelete()
 	{
+		// Don't remove admin profile
+		if ($this->Get('name') === ADMIN_PROFILE_NAME)
+		{
+			throw new SecurityException(Dict::Format('UI:Login:Error:AccessAdmin'));
+		}
+
 		// Note: this may break the rule that says: "a user must have at least ONE profile" !
 		$oLnkSet = $this->Get('user_list');
 		while($oLnk = $oLnkSet->Fetch())
@@ -300,13 +306,38 @@ class URP_UserProfile extends UserRightsBaseClassGUI
 		$this->CheckIfProfileIsAllowed(UR_ACTION_DELETE);
 	}
 
+	/**
+	 * @param $iActionCode
+	 *
+	 * @throws \ArchivedObjectException
+	 * @throws \CoreException
+	 * @throws \SecurityException
+	 */
 	protected function CheckIfProfileIsAllowed($iActionCode)
 	{
+		// When initializing or admin, we need to let everything pass trough
+		if (!UserRights::IsLoggedIn() || UserRights::IsAdministrator()) { return; }
+
+		// Only administrators can manage administrators
+		$iOrigUserId = $this->GetOriginal('userid');
+		if (!empty($iOrigUserId))
+		{
+			$oUser = MetaModel::GetObject('User', $iOrigUserId, true, true);
+			if (UserRights::IsAdministrator($oUser) && !UserRights::IsAdministrator())
+			{
+				throw new SecurityException(Dict::Format('UI:Login:Error:AccessRestricted'));
+			}
+		}
+		$oUser = MetaModel::GetObject('User', $this->Get('userid'), true, true);
+		if (UserRights::IsAdministrator($oUser) && !UserRights::IsAdministrator())
+		{
+			throw new SecurityException(Dict::Format('UI:Login:Error:AccessRestricted'));
+		}
 		if (!UserRights::IsActionAllowed(get_class($this), $iActionCode, DBObjectSet::FromObject($this)))
 		{
 			throw new SecurityException(Dict::Format('UI:Error:ObjectCannotBeUpdated'));
 		}
-		if (UserRights::IsLoggedIn() && !UserRights::IsAdministrator() && ($this->Get('profile') === ADMIN_PROFILE_NAME))
+		if (!UserRights::IsAdministrator() && ($this->Get('profile') === ADMIN_PROFILE_NAME))
 		{
 			throw new SecurityException(Dict::Format('UI:Login:Error:AccessAdmin'));
 		}
@@ -352,6 +383,42 @@ class URP_UserOrg extends UserRightsBaseClassGUI
 	{
 		return Dict::Format('UI:UserManagement:LinkBetween_User_And_Org', $this->Get('userlogin'), $this->Get('allowed_org_name'));
 	}
+
+
+	protected function OnInsert()
+	{
+		$this->CheckIfOrgIsAllowed();
+	}
+
+	protected function OnUpdate()
+	{
+		$this->CheckIfOrgIsAllowed();
+	}
+
+	protected function OnDelete()
+	{
+		$this->CheckIfOrgIsAllowed();
+	}
+
+	/**
+	 * @throws \CoreException
+	 */
+	protected function CheckIfOrgIsAllowed()
+	{
+		if (UserRights::IsAdministrator()) { return; }
+
+		$oUser = UserRights::GetUserObject();
+		$oAddon = UserRights::GetModuleInstance();
+		$aOrgs = $oAddon->GetUserOrgs($oUser, '');
+		if (count($aOrgs) > 0)
+		{
+			$iOrigOrgId = $this->GetOriginal('allowed_org_id');
+			if ((!empty($iOrigOrgId) && !in_array($iOrigOrgId, $aOrgs)) || !in_array($this->Get('allowed_org_id'), $aOrgs))
+			{
+				throw new SecurityException(Dict::Format('Class:User/Error:OrganizationNotAllowed'));
+			}
+		}
+	}
 }
 
 

core/ormlinkset.class.inc.php

@@ -539,6 +539,27 @@ class ormLinkSet implements iDBObjectSetIterator, Iterator, SeekableIterator
 		}
 	}
 
+	/**
+	 * Get the list of all modified (added, modified and removed) links
+	 *
+	 * @return array of link objects
+	 * @throws \Exception
+	 */
+	public function ListModifiedLinks()
+	{
+		$aAdded = $this->aAdded;
+		$aModified = $this->aModified;
+		$aRemoved = array();
+		if (count($this->aRemoved) > 0)
+		{
+			$oSearch = new DBObjectSearch($this->sClass);
+			$oSearch->AddCondition('id', $this->aRemoved, 'IN');
+			$oSet = new DBObjectSet($oSearch);
+			$aRemoved = $oSet->ToArray();
+		}
+		return array_merge($aAdded, $aModified, $aRemoved);
+	}
+
 	/**
 	 * @param DBObject $oHostObject
 	 */

core/userrights.class.inc.php

@@ -283,18 +283,21 @@ abstract class User extends cmdbAbstractObject
 				}
 			}
 		}
-		// Check that this user has at least one profile assigned
+		// Check that this user has at least one profile assigned when profiles have changed
-		$oSet = $this->Get('profile_list');
+		if (array_key_exists('profile_list', $aChanges))
-		if ($oSet->Count() == 0)
 		{
-			$this->m_aCheckIssues[] = Dict::Format('Class:User/Error:AtLeastOneProfileIsNeeded');
+			$oSet = $this->Get('profile_list');
+			if ($oSet->Count() == 0)
+			{
+				$this->m_aCheckIssues[] = Dict::Format('Class:User/Error:AtLeastOneProfileIsNeeded');
+			}
 		}
 		// Only administrators can manage administrators
 		if (UserRights::IsAdministrator($this) && !UserRights::IsAdministrator())
 		{
 			$this->m_aCheckIssues[] = Dict::Format('UI:Login:Error:AccessRestricted');
 		}
-		// Check users with restricted organizations
+
 		if (!UserRights::IsAdministrator())
 		{
 			$oUser = UserRights::GetUserObject();
@@ -304,19 +307,28 @@ abstract class User extends cmdbAbstractObject
 				$aOrgs = $oAddon->GetUserOrgs($oUser, '');
 				if (count($aOrgs) > 0)
 				{
-					/** @var ORMLinkset $oSet */
+					// Check that the modified User belongs to one of our organization
-					$oSet = $this->Get('allowed_org_list');
+					if (!in_array($this->GetOriginal('org_id'), $aOrgs) || !in_array($this->Get('org_id'), $aOrgs))
-					if ($oSet->Count() == 0)
 					{
-						$this->m_aCheckIssues[] = Dict::Format('Class:User/Error:AtLeastOneOrganizationIsNeeded');
+						$this->m_aCheckIssues[] = Dict::Format('Class:User/Error:UserOrganizationNotAllowed');
 					}
-					else
+					// Check users with restricted organizations when allowed organizations have changed
+					if ($this->IsNew() || array_key_exists('allowed_org_list', $aChanges))
 					{
-						while ($oUserOrg = $oSet->Fetch())
+						$oSet = $this->get('allowed_org_list');
+						if ($oSet->Count() == 0)
 						{
-							if (!in_array($oUserOrg->Get('allowed_org_id'), $aOrgs))
+							$this->m_aCheckIssues[] = Dict::Format('Class:User/Error:AtLeastOneOrganizationIsNeeded');
+						}
+						else
+						{
+							$aModifiedLinks = $oSet->ListModifiedLinks();
+							foreach($aModifiedLinks as $oLink)
 							{
-								$this->m_aCheckIssues[] = Dict::Format('Class:User/Error:OrganizationNotAllowed');
+								if (!in_array($oLink->Get('allowed_org_id'), $aOrgs))
+								{
+									$this->m_aCheckIssues[] = Dict::Format('Class:User/Error:OrganizationNotAllowed');
+								}
 							}
 						}
 					}
@@ -961,10 +973,12 @@ class UserRights
 	}
 
 	/**
+	 * Add additional filter for organization silos to all the requests.
+	 *
 	 * @param $sClass
 	 * @param array $aSettings
 	 *
-	 * @return bool
+	 * @return bool|\Expression
 	 */
 	public static function GetSelectFilter($sClass, $aSettings = array())
 	{
@@ -975,8 +989,8 @@ class UserRights
 
 		try
 		{
-			// In case of pb, use AllowAllData internally
+			// Check Bug 1436 for details
-			if (MetaModel::HasCategory($sClass, 'bizmodel') || MetaModel::HasCategory($sClass, 'grant_by_profile'))
+			if (MetaModel::HasCategory($sClass, 'bizmodel'))
 			{
 				return self::$m_oAddOn->GetSelectFilter(self::$m_oUser, $sClass, $aSettings);
 			}

dictionaries/en.dictionary.itop.ui.php

@@ -140,6 +140,7 @@ Dict::Add('EN US', 'English', 'English', array(
 	'Class:User/Error:AtLeastOneProfileIsNeeded' => 'At least one profile must be assigned to this user.',
 	'Class:User/Error:AtLeastOneOrganizationIsNeeded' => 'At least one organization must be assigned to this user.',
 	'Class:User/Error:OrganizationNotAllowed' => 'Organization not allowed.',
+	'Class:User/Error:UserOrganizationNotAllowed' => 'The user account does not belong to your allowed organizations.',
 	'Class:UserInternal' => 'User Internal',
 	'Class:UserInternal+' => 'User defined within iTop',
 ));

dictionaries/fr.dictionary.itop.ui.php

@@ -164,6 +164,7 @@ Dict::Add('FR FR', 'French', 'Français', array(
 	'Class:User/Error:AtLeastOneProfileIsNeeded' => 'L\'utilisateur doit avoir au moins un profil.',
 	'Class:User/Error:AtLeastOneOrganizationIsNeeded' => 'L\'utilisateur doit avoir au moins une organisation.',
 	'Class:User/Error:OrganizationNotAllowed' => 'Organisation non autorisée.',
+	'Class:User/Error:UserOrganizationNotAllowed' => 'L\'utilisateur n\'appartient pas à vos organisations.',
 	'Class:UserInternal' => 'Utilisateur interne',
 	'Class:UserInternal+' => 'Utilisateur défini dans iTop',
 	'Class:URP_Dimensions' => 'Dimension',