* (Fixes issue 1245) Optimized the implementation of checkAccess of C…

…PhpAuthManager and CDbAuthManager
yiisoft · Jun 18, 2010 · 723994a · 723994a
1 parent b0eedd6
commit 723994a
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 112 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -51,6 +51,7 @@ Version 1.1.3 to be released
 - Enh #1225: Added 'firstError' option to CHtml::errorSummary() to support displaying only the first error message of each model attribute (Qiang)
 - Enh #1232: Added CAuthManager::showErrors. When value is true Yii will turn on error_reporting for RBAC bizRules. False by default (Sam Dark)
 - Enh #1239: CBreadcrumbs should have the 'Home' label translated (Qiang)
+- Enh #1245: Optimized the implementation of checkAccess of CPhpAuthManager and CDbAuthManager (Qiang)
 - Enh #1261: Added magicFile parameter to CFileHelper::getMimeType() and getMimeTypeByExtension() (Qiang)
 - Enh #1268: Added isset and unset support to behavior properties in a component context (Qiang)
 - Enh #1271: Added CWebUser::getFlashes() (Qiang)

diff --git a/framework/web/auth/CDbAuthManager.php b/framework/web/auth/CDbAuthManager.php
@@ -81,80 +81,43 @@ public function init()
 	 */
 	public function checkAccess($itemName,$userId,$params=array())
 	{
-		if(!empty($this->defaultRoles) && $this->checkDefaultRoles($itemName,$params))
-			return true;
-
-		$sql="SELECT name, type, description, t1.bizrule, t1.data, t2.bizrule AS bizrule2, t2.data AS data2 FROM {$this->itemTable} t1, {$this->assignmentTable} t2 WHERE name=itemname AND userid=:userid";
-		$command=$this->db->createCommand($sql);
-		$command->bindValue(':userid',$userId);
-
-		// check directly assigned items
-		$names=array();
-		foreach($command->queryAll() as $row)
-		{
-			Yii::trace('Checking permission "'.$row['name'].'"','system.web.auth.CDbAuthManager');
-			if($this->executeBizRule($row['bizrule2'],$params,unserialize($row['data2']))
-				&& $this->executeBizRule($row['bizrule'],$params,unserialize($row['data'])))
-			{
-				if($row['name']===$itemName)
-					return true;
-				$names[]=$row['name'];
-			}
-		}
-
-		// check all descendant items
-		while($names!==array())
-		{
-			$items=$this->getItemChildren($names);
-			$names=array();
-			foreach($items as $item)
-			{
-				Yii::trace('Checking permission "'.$item->getName().'"','system.web.auth.CDbAuthManager');
-				if($this->executeBizRule($item->getBizRule(),$params,$item->getData()))
-				{
-					if($item->getName()===$itemName)
-						return true;
-					$names[]=$item->getName();
-				}
-			}
-		}
-
-		return false;
+		$assignments=$this->getAuthAssignments($userId);
+		return $this->checkAccessRecursive($itemName,$userId,$params,$assignments);
 	}
 
-
 	/**
-	 * Checks the access based on the default roles as declared in {@link defaultRoles}.
+	 * Performs access check for the specified user.
+	 * This method is internally called by {@link checkAccess}.
 	 * @param string the name of the operation that need access check
+	 * @param mixed the user ID. This should can be either an integer and a string representing
+	 * the unique identifier of a user. See {@link IWebUser::getId}.
 	 * @param array name-value pairs that would be passed to biz rules associated
 	 * with the tasks and roles assigned to the user.
-	 * @return boolean whether the operations can be performed by the user according to the default roles.
-	 * @since 1.0.3
+	 * @param array the assignments to the specified user
+	 * @return boolean whether the operations can be performed by the user.
+	 * @since 1.1.3
 	 */
-	protected function checkDefaultRoles($itemName,$params)
+	protected function checkAccessRecursive($itemName,$userId,$params,$assignments)
 	{
-		$names=array();
-		foreach($this->defaultRoles as $role)
-		{
-			if(is_string($role))
-				$names[]=$this->db->quoteValue($role);
-			else
-				$names[]=$role;
-		}
-		if(count($names)<4)
-			$condition='name='.implode(' OR name=',$names);
-		else
-			$condition='name IN ('.implode(', ',$names).')';
-		$sql="SELECT name, type, description, bizrule, data FROM {$this->itemTable} WHERE $condition";
-		$command=$this->db->createCommand($sql);
-		$rows=$command->queryAll();
-
-		foreach($rows as $row)
+		if(($item=$this->getAuthItem($itemName))===null)
+			return false;
+		Yii::trace('Checking permission "'.$item->getName().'"','system.web.auth.CDbAuthManager');
+		if($this->executeBizRule($item->getBizRule(),$params,$item->getData()))
 		{
-			Yii::trace('Checking default role "'.$row['name'].'"','system.web.auth.CDbAuthManager');
-			$item=new CAuthItem($this,$row['name'],$row['type'],$row['description'],$row['bizrule'],unserialize($row['data']));
-			if($item->checkAccess($itemName,$params))
+			if(in_array($itemName,$this->defaultRoles))
 				return true;
+			if(isset($assignments[$itemName]))
+			{
+				$assignment=$assignments[$itemName];
+				if($this->executeBizRule($assignment->getBizRule(),$params,$assignment->getData()))
+					return true;
+			}
+			$sql="SELECT parent FROM {$this->itemChildTable} WHERE child=:name";
+			foreach($this->db->createCommand($sql)->bindValue(':name',$itemName)->queryColumn() as $parent)
+			{
+				if($this->checkAccessRecursive($parent,$userId,$params,$assignments))
+					return true;
+			}
 		}
 		return false;
 	}

diff --git a/framework/web/auth/CPhpAuthManager.php b/framework/web/auth/CPhpAuthManager.php
@@ -63,61 +63,26 @@ public function init()
 	 */
 	public function checkAccess($itemName,$userId,$params=array())
 	{
-		if(!empty($this->defaultRoles) && $this->checkDefaultRoles($itemName,$params))
-			return true;
-
-		// check directly assigned items
-		$names=array();
-		foreach($this->getAuthAssignments($userId) as $assignment)
+		if(!isset($this->_items[$itemName]))
+			return false;
+		$item=$this->_items[$itemName];
+		Yii::trace('Checking permission "'.$item->getName().'"','system.web.auth.CPhpAuthManager');
+		if($this->executeBizRule($item->getBizRule(),$params,$item->getData()))
 		{
-			$item=$this->getAuthItem($assignment->getItemName());
-			Yii::trace('Checking permission "'.($item ? $item->getName() : 'unknown').'"','system.web.auth.CPhpAuthManager');
-			if($item!==null && $this->executeBizRule($assignment->getBizRule(),$params,$assignment->getData())
-				&& $this->executeBizRule($item->getBizRule(),$params,$item->getData()))
+			if(in_array($itemName,$this->defaultRoles))
+				return true;
+			if(isset($this->_assignments[$userId][$itemName]))
 			{
-				if($item->getName()===$itemName)
+				$assignment=$this->_assignments[$userId][$itemName];
+				if($this->executeBizRule($assignment->getBizRule(),$params,$assignment->getData()))
 					return true;
-				$names[]=$item->getName();
 			}
-		}
-
-		// check all descendant items
-		while($names!==array())
-		{
-			$items=$this->getItemChildren($names);
-			$names=array();
-			foreach($items as $item)
+			foreach($this->_children as $parentName=>$children)
 			{
-				Yii::trace('Checking permission "'.$item->getName().'"','system.web.auth.CPhpAuthManager');
-				if($this->executeBizRule($item->getBizRule(),$params,$item->getData()))
-				{
-					if($item->getName()===$itemName)
-						return true;
-					$names[]=$item->getName();
-				}
+				if(isset($children[$itemName]) && $this->checkAccess($parentName,$userId,$params))
+					return true;
 			}
 		}
-
-		return false;
-	}
-
-	/**
-	 * Checks the access based on the default roles as declared in {@link defaultRoles}.
-	 * @param string the name of the operation that need access check
-	 * @param array name-value pairs that would be passed to biz rules associated
-	 * with the tasks and roles assigned to the user.
-	 * @return boolean whether the operations can be performed by the user according to the default roles.
-	 * @since 1.0.3
-	 */
-	protected function checkDefaultRoles($itemName,$params)
-	{
-		foreach($this->defaultRoles as $role)
-		{
-			Yii::trace('Checking default role "'.$role.'"','system.web.auth.CPhpAuthManager');			
-			$item=$this->getAuthItem($role);
-			if($item!==null && $item->checkAccess($itemName,$params))
-				return true;
-		}
 		return false;
 	}