[#21661] Article access check is only using the current category, not…

… checking further up the branch

git-svn-id: http://joomlacode.org/svn/joomla/development/trunk@19112 6f6e1ebd-4c2b-0410-823f-f34bde69bce9

components/com_content/models/article.php

@@ -80,7 +80,17 @@ public function &getItem($pk = null)
 				$db = $this->getDbo();
 				$query = $db->getQuery(true);
 
-				$query->select($this->getState('item.select', 'a.*'));
+				$query->select($this->getState(
+					'item.select', 'a.id, a.asset_id, a.title, a.alias, a.title_alias, a.introtext, a.fulltext, ' .
+					// If badcats is not null, this means that the article is inside an unpublished category
+					// In this case, the state is set to 0 to indicate Unpublished (even if the article state is Published)
+					'CASE WHEN badcats.id is null THEN a.state ELSE 0 END AS state, ' .  
+					'a.mask, a.catid, a.created, a.created_by, a.created_by_alias, ' .
+					'a.modified, a.modified_by, a.checked_out, a.checked_out_time, a.publish_up, a.publish_down, ' . 
+					'a.images, a.urls, a.attribs, a.version, a.parentid, a.ordering, ' .
+					'a.metakey, a.metadesc, a.access, a.hits, a.metadata, a.featured, a.language, a.xreference'
+					)
+				);
 				$query->from('#__content AS a');
 
 				// Join on category table.
@@ -107,6 +117,14 @@ public function &getItem($pk = null)
 
 				$query->where('(a.publish_up = ' . $nullDate . ' OR a.publish_up <= ' . $nowDate . ')');
 				$query->where('(a.publish_down = ' . $nullDate . ' OR a.publish_down >= ' . $nowDate . ')');
+
+				// Join to check for category published state in parent categories up the tree
+				// If all categories are published, badcats.id will be null, and we just use the article state
+				$subquery = ' (SELECT cat.id as id FROM #__categories AS cat JOIN #__categories AS parent ';
+				$subquery .= 'ON cat.lft BETWEEN parent.lft AND parent.rgt ';
+				$subquery .= 'WHERE parent.extension = ' . $db->quote('com_content');
+				$subquery .= ' AND parent.published <= 0 GROUP BY cat.id)';
+				$query->join('LEFT OUTER', $subquery . ' AS badcats ON badcats.id = c.id');
 
 				// Filter by published state.
 				$published = $this->getState('filter.published');
@@ -131,7 +149,7 @@ public function &getItem($pk = null)
 				// Check for published state if filter set.
 				if (((is_numeric($published)) || (is_numeric($archived))) && (($data->state != $published) && ($data->state != $archived)))
 				{
-					JError::raiseError(404, JText::_('COM_CONTENT_ERROR_ARTICLE_NOT_FOUND'));
+					throw new JException(JText::_('COM_CONTENT_ERROR_ARTICLE_NOT_FOUND'), 404);
 				}
 
 				// Convert parameter fields to objects.

components/com_content/models/articles.php

@@ -116,16 +116,33 @@ function getListQuery()
 		$query->select(
 			$this->getState(
 				'list.select',
-				'a.id, a.title, a.alias, a.title_alias, a.introtext, a.state, a.catid, a.created, a.created_by, a.created_by_alias,' .
+				'a.id, a.title, a.alias, a.title_alias, a.introtext, ' . 
+				'a.catid, a.created, a.created_by, a.created_by_alias, ' .
 				// use created if modified is 0
-				'CASE WHEN a.modified = 0 THEN a.created ELSE a.modified END as modified,' .
+				'CASE WHEN a.modified = 0 THEN a.created ELSE a.modified END as modified, ' .
 					'a.modified_by, uam.name as modified_by_name,' .
 				// use created if publish_up is 0
-				'CASE WHEN a.publish_up = 0 THEN a.created ELSE a.publish_up END as publish_up,' .
-					'a.publish_down, a.attribs, a.metadata, a.metakey, a.metadesc, a.access,'.
-					'a.hits, a.xreference, a.featured,'.' LENGTH(a.fulltext) AS readmore'
+				'CASE WHEN a.publish_up = 0 THEN a.created ELSE a.publish_up END as publish_up, ' .
+					'a.publish_down, a.attribs, a.metadata, a.metakey, a.metadesc, a.access, '.
+					'a.hits, a.xreference, a.featured,'.' LENGTH(a.fulltext) AS readmore '
 			)
 		);
+		// Process an Archived Article layout
+		if ($this->getState('filter.published') == 2)
+		{
+			// If badcats is not null, this means that the article is inside an unpublished category
+			// In this case, the state is set to 0 to indicate Unpublished (even if the article state is Published)
+			$query->select($this->getState('list.select','CASE WHEN badcats.id is null THEN a.state ELSE 2 END AS state'));
+		}
+		else
+		{
+			// Process non-archived layout
+			// If badcats is not null, this means that the article is inside an unpublished category
+			// In this case, the state is set to 0 to indicate Unpublished (even if the article state is Published)
+			$query->select($this->getState('list.select','CASE WHEN badcats.id is not null THEN 0 ELSE a.state END AS state'));			
+		}
+
+
 		$query->from('#__content AS a');
 
 		// Join over the frontpage articles.
@@ -149,6 +166,29 @@ function getListQuery()
 		// Join on voting table
 		$query->select('ROUND( v.rating_sum / v.rating_count ) AS rating, v.rating_count as rating_count');
 		$query->join('LEFT', '#__content_rating AS v ON a.id = v.content_id');
+
+		// Join to check for category published state in parent categories up the tree
+		$query->select('c.published, CASE WHEN badcats.id is null THEN c.published ELSE 0 END AS parents_published');
+		$subquery = 'SELECT cat.id as id FROM #__categories AS cat JOIN #__categories AS parent ';
+		$subquery .= 'ON cat.lft BETWEEN parent.lft AND parent.rgt ';
+		$subquery .= 'WHERE parent.extension = ' . $db->quote('com_content');
+		if ($this->getState('filter.published') == 2)
+		{
+			// Find any up-path categories that are archived
+			// If any up-path categories are archived, include all children in archived layout
+			$subquery .= ' AND parent.published = 2 GROUP BY cat.id ';
+			// Set effective state to archived if up-path category is archived
+			$publishedWhere = 'CASE WHEN badcats.id is null THEN a.state ELSE 2 END';
+		} 
+		else 
+		{
+			// Find any up-path categories that are not published
+			// If all categories are published, badcats.id will be null, and we just use the article state
+			$subquery .= ' AND parent.published != 1 GROUP BY cat.id ';
+			// Select state to unpublished if up-path category is unpublished
+			$publishedWhere = 'CASE WHEN badcats.id is null THEN a.state ELSE 0 END';
+		}
+		$query->join('LEFT OUTER', '(' . $subquery . ') AS badcats ON badcats.id = c.id');
 
 		// Filter by access level.
 		if ($access = $this->getState('filter.access')) {
@@ -160,12 +200,17 @@ function getListQuery()
 		// Filter by published state
 		$published = $this->getState('filter.published');
 
-		if (is_numeric($published)) {
-			$query->where('a.state = '.(int) $published);
-		} else if (is_array($published)) {
+		if (is_numeric($published)) 
+		{
+			// Use article state if badcats.id is null, otherwise, force 0 for unpublished
+			$query->where($publishedWhere . ' = ' . (int) $published);
+		} 
+		else if (is_array($published)) 
+		{
 			JArrayHelper::toInteger($published);
 			$published = implode(',', $published);
-			$query->where('a.state IN ('.$published.')');
+			// Use article state if badcats.id is null, otherwise, force 0 for unpublished
+			$query->where($publishedWhere . ' IN ('.$published.')');
 		}
 
 		// Filter by featured state

installation/CHANGELOG

@@ -27,6 +27,9 @@ $ -> Language fix or change
 - -> Removed
 ! -> Note
 
+14-Oct-2010 Mark Dexter
+ # [#21661] Article access check is only using the current category, not checking further up the branch
+
 14-Oct-2010 Christophe Demko
  ^ Fix loading .ini files instead of .sys.ini files for #22710
  # [#21780] JHtmlTabs not compatible with JHtmlRules

libraries/joomla/application/categories.php

@@ -30,6 +30,13 @@ class JCategories
 	 * @var mixed
 	 */
 	protected $_nodes;
+
+	/**
+	 * Array of checked categories -- used to save values when _nodes are null
+	 * 
+	 * @var array
+	 */
+	protected $_checkedCategories;
 
 	/**
 	 * Name of the extension the categories belong to
@@ -139,15 +146,22 @@ public function get($id = 'root', $forceload = false)
 				$id = 'root';
 			}
 		}
-		if (!isset($this->_nodes[$id]) || $forceload)
+		// If this $id has not been processed yet, execute the _load method
+		if ((!isset($this->_nodes[$id]) && !isset($this->_checkedCategories[$id])) || $forceload)
 		{
 			$this->_load($id);
 		}
 
+		// If we already have a value in _nodes for this $id, then use it
 		if(isset($this->_nodes[$id]))
 		{
 			return $this->_nodes[$id];
 		}
+		// If we processed this $id already and it was not valid, then return null
+		elseif (isset($this->_checkedCategories[$id]))
+		{
+			return null;
+		}
 		return false;
 	}
 
@@ -157,6 +171,8 @@ protected function _load($id)
 		$app = JFactory::getApplication();
 		$user = JFactory::getUser();
 		$extension = $this->_extension;
+		// Record that this $id has been checked
+		$this->_checkedCategories[$id] = true;
 
 		$query = new JDatabaseQuery;
 
@@ -183,6 +199,12 @@ protected function _load($id)
 			$query->leftJoin('#__categories AS s ON (s.lft <= c.lft AND s.rgt >= c.rgt) OR (s.lft > c.lft AND s.rgt < c.rgt)');
 			$query->where('s.id='.(int)$id);
 		}
+
+		$subQuery = ' (SELECT cat.id as id FROM #__categories AS cat JOIN #__categories AS parent ' . 
+					'ON cat.lft BETWEEN parent.lft AND parent.rgt WHERE parent.extension = ' . $db->quote($extension) .
+					' AND parent.published != 1 GROUP BY cat.id) ';
+		$query->leftJoin($subQuery . 'AS badcats ON badcats.id = c.id');
+		$query->where('badcats.id is null');
 
 		// i for item
 		if(isset($this->_options['countItems']) && $this->_options['countItems'] == 1)
@@ -226,13 +248,18 @@ protected function _load($id)
 				// Create the node
 				if (!isset($this->_nodes[$result->id]))
 				{
-					// Create the JCategoryNode
+					// Create the JCategoryNode and add to _nodes
 					$this->_nodes[$result->id] = new JCategoryNode($result, $this);
+
+					// If this is not root, and if the current nodes parent is in the list or the current node parent is 0
 					if($result->id != 'root' && (isset($this->_nodes[$result->parent_id]) || $result->parent_id == 0))
 					{
-						// Compute relationship between node and its parent
+						// Compute relationship between node and its parent - set the parent in the _nodes field
 						$this->_nodes[$result->id]->setParent($this->_nodes[$result->parent_id]);
 					}
+
+					// if the node's parent id is not in the _nodes list and the node is not root (doesn't have parent_id == 0),
+					// then remove this nodes from the list 
 					if(!(isset($this->_nodes[$result->parent_id]) || $result->parent_id == 0))
 					{
 						unset($this->_nodes[$result->id]);
@@ -244,7 +271,9 @@ protected function _load($id)
 						$this->_nodes[$result->id]->setAllLoaded();
 						$childrenLoaded = true;
 					}
-				} elseif($result->id == $id || $childrenLoaded) {
+				}
+				elseif($result->id == $id || $childrenLoaded) 
+				{
 					// Create the JCategoryNode
 					$this->_nodes[$result->id] = new JCategoryNode($result, $this);
 					if($result->id != 'root' && (isset($this->_nodes[$result->parent_id]) || $result->parent_id))

tests/CHANGELOG

@@ -27,6 +27,9 @@ $ -> Language fix or change
 - -> Removed
 ! -> Note
 
+14-Oct-2010 Mark Dexter
++ article0003Test added to test effect of up-path category state on article menu items
+
 12-Oct-2010 Mark Dexter
 # Fix multiple system tests for ACL UI changes
 # Fix problem in language0001test.php from Category Manager UI change

tests/system/SeleniumJoomlaTestCase.php

@@ -704,8 +704,47 @@ function togglePublished($articleTitle)
 		$this->click("//table[@class='adminlist']/tbody//tr//td/a[contains(text(), '" .	$articleTitle . "')]/../../td[3]/a");
 		$this->waitForPageToLoad("30000");
 	}
+
+	function toggleCheckBox($itemTitle)
+	{
+		echo "Toggling check box selection of article " . $itemTitle . "\n";
+		$this->click("//table[@class='adminlist']/tbody//tr//td/a[contains(text(), '" .	$itemTitle . "')]/../../td[1]/input");
+	}
+
+	/**
+	 * 
+	 * Sets state of component category or item
+	 * @param string 	Title or Category or Item 
+	 * @param string	Name of Menu: Article Manager, Banners, Contacts, Newsfeeds, Weblinks
+	 * @param string	Category or Item
+	 * @param string	last part of toolbar name: publish, unpublish, archive, trash
+	 */
 
-
+	function changeState($title = null, $menu = 'Article Manager', $type = 'Category', $newState = 'publish')
+	{
+		$this->gotoAdmin();
+		echo "Changing state of " . $type . " " . $title . " in " . $menu . " to " . $newState .  "\n";
+		$this->click("link=" . $menu);
+		$this->waitForPageToLoad("30000");
+		if ($type == 'Category')
+		{
+			$this->click("//ul[@id='submenu']/li[2]/a");
+			$this->waitForPageToLoad("30000");
+		}
+		$filter = ($menu == 'Banners') ? 'filter_state' : 'filter_published';
+		$this->select($filter, "label=All");
+		$this->waitForPageToLoad("30000");
+		$this->type("filter_search", $title);
+		$this->click("//button[@type='submit']");
+		$this->waitForPageToLoad("30000");
+		$this->toggleCheckBox($title);
+		$this->click("//li[@id='toolbar-" . $newState . "']/a/span");
+		$this->waitForPageToLoad("30000");
+		$this->click("//button[@type='button']");
+		$this->waitForPageToLoad("30000");
+		$this->gotoAdmin();
+	}
+
 	function checkNotices()
 	{
 		try {

tests/system/suite/TestSuite.php

@@ -23,6 +23,7 @@
 require_once 'menus/menu0002Test.php';
 require_once 'articles/article0001Test.php';
 require_once 'articles/article0002Test.php';
+require_once 'articles/article0003Test.php';
 require_once 'articles/featured0001Test.php';
 require_once 'articles/featured0002Test.php';
 require_once 'com_users/user0001Test.php';
@@ -60,6 +61,7 @@ public static function suite()
 		$suite->addTestSuite('Menu0002');
 		$suite->addTestSuite('Article0001');
 		$suite->addTestSuite('Article0002');
+		$suite->addTestSuite('Article0003');
 		$suite->addTestSuite('Featured0001Test');
 		$suite->addTestSuite('Featured0002Test');
 		$suite->addTestSuite('User0001Test');