Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unauthorised links and article authorisation fix #2545

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Unauthorised links and article authorisation fix #2545

wants to merge 5 commits into from

Conversation

Tazzios
Copy link

@Tazzios Tazzios commented Sep 20, 2019

#2542
Good Changes

  • When retrieving articles the c.access won`t be checked( the article access is leading)
  • unauthorised link setting is now taken into account.

questional change:

  • Removed category acces level check in categoryfinder.php

Instead of removing the acces level check for category There should be a way to retrieve articles from given category id which doesn't take the category acces level in to account.
The change that i made is a problem if there are particles that want to show category`s with different access levels

article behavior 'Show Unauthorised Links' #2542
Removed the autorisation check on category level
Added check on show_noauth.
article behavior 'Show Unauthorised Links' #2542
Removed the autorisation check on category level so that the check will be only on category level.
#1 added is null or empty and JSON_UNQUOTE
Forgot a table prefix
@mahagr
Copy link
Member

mahagr commented Oct 1, 2019

@Tazzios I will need to look at how the latest Joomla deals with this. You likely used Joomla code to implement this change, can you share it to me? :)

@Tazzios
Copy link
Author

Tazzios commented Oct 1, 2019

For the fix i didn`t check the joomla code, so this is al reversed:

1. Inherited authorization
What i have found is components/com_content/models/category.php getitems() on line 226.
It is a function where the articles from a category are retrieved. In the code you can see that there is not filtered on the category acces level. So the acces level check is done later on the article level

public function getItems()
	{
		$limit = $this->getState('list.limit');

		if ($this->_articles === null && $category = $this->getCategory())
		{
			$model = JModelLegacy::getInstance('Articles', 'ContentModel', array('ignore_request' => true));
			$model->setState('params', JFactory::getApplication()->getParams());
			$model->setState('filter.category_id', $category->id);
			$model->setState('filter.published', $this->getState('filter.published'));
			$model->setState('filter.access', $this->getState('filter.access'));
			$model->setState('filter.language', $this->getState('filter.language'));
			$model->setState('filter.featured', $this->getState('filter.featured'));
			$model->setState('list.ordering', $this->_buildContentOrderBy());
			$model->setState('list.start', $this->getState('list.start'));
			$model->setState('list.limit', $limit);
			$model->setState('list.direction', $this->getState('list.direction'));
			$model->setState('list.filter', $this->getState('list.filter'));
			$model->setState('filter.tag', $this->getState('filter.tag'));

			// Filter.subcategories indicates whether to include articles from subcategories in the list or blog
			$model->setState('filter.subcategories', $this->getState('filter.subcategories'));
			$model->setState('filter.max_category_levels', $this->getState('filter.max_category_levels'));
			$model->setState('list.links', $this->getState('list.links'));

			if ($limit >= 0)
			{
				$this->_articles = $model->getItems();

				if ($this->_articles === false)
				{
					$this->setError($model->getError());
				}
			}
			else
			{
				$this->_articles = array();
			}

			$this->_pagination = $model->getPagination();
		}

		return $this->_articles;
	}

How it works In joomla: a new article get the same authorization as the category by default but it is possible to give the article a totally different acces level. That's why, if you want to get the articles, you shouldn't apply the category acces level.
If you want to show a list of categories then off course you want to apply the category acces level.
A better solution then what i have made Would be Making the same function as above in Gantry which would leave the current function the same only. Particle makers need to call the new function to fixed it.

Source: https://stackoverflow.com/questions/14066698/code-to-display-articles-from-category-id

2. Show intro to unauthorized users
I found a few times 'show_noauth' in the files in 'com_content'. The best one is i think 'components\com_content\models\articles.php line 123


 		// Process show_noauth parameter
 		if ((!$params->get('show_noauth')) || (!JComponentHelper::getParams('com_content')->get('show_noauth')))
 		{
 			$this->setState('filter.access', true);
 		}
 		else
 		{
 			$this->setState('filter.access', false);
 		}

I think this code says something like if show_noauth=true don`t apply the access filter for the article. ( but i could be wrong about that ;) )
When implementing my code i noticed that not all hosting providers databases likes the 'JSON_EXTRACT' command that i use in the sql code.

https://docs.joomla.org/Restricting_access_to_%22read_more%22

@mahagr
Copy link
Member

mahagr commented Oct 2, 2019

The method was added to MariaDB 10.2.3, MySQL 5.7, both released around late 2016. Unfortunately Joomla up to 3.9 requires only MySQL 5.1, so we would need to delay this change for Gantry 5.5 where we raise the requirements.

@Tazzios
Copy link
Author

Tazzios commented Oct 2, 2019

Instead of using JSON_EXTRACT it is possible to extract that information with PHP but that require extra code. I`m not that good that i can make that.

At joomla the also noticed the JSON support, as a lot of settings are saved as JSON file in the database.

5.6, 5.7 and 8.0 are the currently supported versions of the MySQL database. Version 5.5 reached its end of life in December 2018, while MySQL 5.6 will be supported until February 2021.
Note that the minimum MySQL version can be subject to change during the lifetime of Joomla 4, if necessary, as in the future we might look to use MySQL 5.7 which would allow the use of the JSON database type in MySQL columns.

https://developer.joomla.org/news/788-joomla-4-on-the-move.html

Part 1 is more imported as that situation occurs al lot more. It is also easier to fixed by:

  • Removing the c.access check in ContentFinder.php line 109 (shouldn't have been there)
  • Create a function where all the articles from a category (or multiple) are retrieved without checking c.access and only a.access.

@mahagr
Copy link
Member

mahagr commented Oct 3, 2019

I think there's still use for category ACL as you may have a particle listing all the categories (and it can be used for non-article categories). But for locating the articles we may want to look at updated Joomla code (yes, the current code was implemented by reading what Joomla used to do back in 2016). I think the queries were mostly based on Joomla content modules.

I am looking into Joomla 4 now, maybe I can take a look on Joomla 3.9/4.0 articles and try to update the code as those are going to be the "supported" versions of Gantry 5.5. In quotes as Joomla 4 isn't out yet and there's still a lot of work to do in supporting Bootstrap 4.

@Tazzios
Copy link
Author

Tazzios commented Oct 4, 2019

I think there's still use for category ACL as you may have a particle listing all the categories (and it can be used for non-article categories).

Yes I don't say other wise, but currently there is also a category check in the contentfinder.php which is wrong in any situation.
In the categoryfinder.php the check should be present . That`s why i opt for creating an new function. My branch where i also removed the check in categoryfinder.php is because i don't have much experience and i don't show a category list in my site generated by grantry.

But for locating the articles we may want to look at updated Joomla code (yes, the current code was implemented by reading what Joomla used to do back in 2016). I think the queries were mostly based on Joomla content modules.

I am looking into Joomla 4 now, maybe I can take a look on Joomla 3.9/4.0 articles and try to update the code as those are going to be the "supported" versions of Gantry 5.5. In quotes as Joomla 4 isn't out yet and there's still a lot of work to do in supporting Bootstrap 4.

Great. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants