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

[com_content] Redundant access check #21169

Merged
merged 3 commits into from
Aug 2, 2018
Merged

[com_content] Redundant access check #21169

merged 3 commits into from
Aug 2, 2018

Conversation

SharkyKZ
Copy link
Contributor

@SharkyKZ SharkyKZ commented Jul 18, 2018

Pull Request for Issue # .

Summary of Changes

This removes redundant access check from admin Articles model.

Testing Instructions

Code review. See that we already filter by access in the query:

// Filter by access level on categories.
if (!$user->authorise('core.admin'))
{
$groups = implode(',', $user->getAuthorisedViewLevels());
$query->where('a.access IN (' . $groups . ')');
$query->where('c.access IN (' . $groups . ')');
}

Alternatively, you can check that this snippet (placed somehwere in frontend/site, e.g. in Protostar index file) doesn't return unauthorized articles. Where $accessLevel is some access level ID:

JModelLegacy::addIncludePath(JPATH_ADMINISTRATOR . '/components/com_content/models', 'ContentModel');
$model = JModelLegacy::getInstance('Articles', 'ContentModel', array('ignore_request' => true));
$model->setState('params',  JFactory::getApplication()->getParams());
$model->setState('filter.access', $accessLevel);
$articles = $model->getItems();
var_dump($articles);

Documentation Changes Required

No.

@bene-we
Copy link
Contributor

bene-we commented Jul 23, 2018

I have tested this item ✅ successfully on 8d3d71e

What I tested:

On administrator site, create a new article with permission Super Users. After that I created a new user and added him to the administration group. I logged in with the newly created user and filtered for Super Users: No articles appear.

@icampus


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21169.

@Tchangue
Copy link

I have tested this item ✅ successfully on 8d3d71e

I created two articles in the same menu: one with permission public and the other one with permission registered. By navigating on this menu we can only see the article with public access and when a user log in the second articles with permission registered appears.

@icampus


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21169.

@ghost
Copy link

ghost commented Jul 24, 2018

Ready to Commit after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 24, 2018
@mbabker
Copy link
Contributor

mbabker commented Jul 25, 2018

Since the extended method now does nothing useful, it can be removed too (no, there are no B/C issues with that).

@roland-d
Copy link
Contributor

@SharkyKZ Can you update your PR as pointed out by @mbabker ?

@SharkyKZ
Copy link
Contributor Author

Updated.

@roland-d
Copy link
Contributor

@bene-we @Tchangue can you give this another test please?

@mbabker mbabker added this to the Joomla 3.8.12 milestone Aug 2, 2018
@mbabker mbabker merged commit c9e2fa9 into joomla:staging Aug 2, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 2, 2018
@SharkyKZ SharkyKZ deleted the articlesAccessCheck branch August 3, 2018 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants