?
avatar ggppdk
ggppdk
5 Aug 2016

Steps to reproduce the issue

at global configuration permissions
and
at Article component

do not assign Edit and Edit.own privilege

  • neither to the usergroup Manager directly
  • nor via their parents groups

(set everything to not-set or "inherited")

  • this is soft-deny so that we allow further down

Then give edit privelege for a specific article to the manager usergroup via

  • either directly (edit the article as super admin and give edit to usergroup Manager)
  • or do same for the category of the article (edit the article as super admin and give edit to usergroup Manager)

Expected result

If a "manager" user logins to backend and tries to edit it, then he will be able to do so

Actual result

The article manager indeed lists it as editable, but clicking on it , edit is denied

System information (as much as possible)

J3.6.1 / J3.6.2

Additional comments

Problem is with new code at allowEdit() method of (backend) article controler
i had made a comment directly on the code about the problem,

  • but i should have opened and issue ?

Here is my comment
fd72a9f#commitcomment-18521573

Fix is obvious,
and code seemed obviously wrong,

  • just i was hesitant to insist that code is bad, without testing, so i think i opened this issue a little too late as J3.6.2 has been released

at least fix is easy

Votes

# of Users Experiencing Issue
2/2
Average Importance Score
4.50

avatar ggppdk ggppdk - open - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
Title
Soft deny ACL is broken in J3.6.2 for Articles component (only for this component)
Soft deny ACL is broken in J3.6.1 / J3.6.2 for Articles component (only for this component)
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
The description was changed
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
The description was changed
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
The description was changed
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
The description was changed
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
The description was changed
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
The description was changed
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
The description was changed
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
Title
Soft deny ACL is broken in J3.6.1 / J3.6.2 for Articles component (only for this component)
Soft deny ACL is broken in J3.6.1 / J3.6.2 for Articles component (backend article controller)
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
Title
Soft deny ACL is broken in J3.6.1 / J3.6.2 for Articles component (only for this component)
Allow Edit after Soft deny is impossible in J3.6.1 / J3.6.2 for Articles component (backend article controller)
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk ggppdk - change - 5 Aug 2016
The description was changed
avatar ggppdk ggppdk - edited - 5 Aug 2016
avatar ggppdk
ggppdk - comment - 5 Aug 2016

At least this is not a security issue,

Instead it is a usability issue for sites (1% ? or less since we speak about backend) that

  • use ACL with soft deny and then allow editing in specific categories

Editing for these authors / editors is no longer possible, the new code here needs to be removed,
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/controllers/article.php#L89-L94

avatar infograf768
infograf768 - comment - 5 Aug 2016
avatar infograf768 infograf768 - change - 5 Aug 2016
Category ACL Administration Components
avatar sanderpotjer
sanderpotjer - comment - 5 Aug 2016

I can confirm the same issue, reported by several users of my extension meanwhile.

The code change in fd72a9f#diff-64d86da3f57caa0ed222b746c070879eR89 caused this issue.

Basically it is checking for JFactory::getUser()->authorise('core.edit', 'com_content); and directly returning false, without taking the category or article level permissions into account.

Not sure why return parent::allowEdit($data, $key); was moved from the bottom of the function to the current position.

A simple solution would be to revert to the previous situation with the check at the bottom of the function after checking the article level. But that only works if @wilsonge didn't had a specific reason to move the check?


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

avatar brianteeman brianteeman - change - 5 Aug 2016
Labels Added: ?
avatar mbabker
mbabker - comment - 5 Aug 2016

The change was in response to a reported and verified security issue so please take into consideration that even though the fix may not be 100% there is not an option to just revert it in full.

avatar ggppdk
ggppdk - comment - 5 Aug 2016

Yes tried to guess what exactly they were trying to fix

whatever it was (i have an idea)
it was certainly done the wrong way, since it cancels the way ACL heritage is supposed to work

avatar sanderpotjer
sanderpotjer - comment - 5 Aug 2016

My guess it was related to https://developer.joomla.org/security-centre/652-20160801-core-core-acl-violations.html

Not sure if there is any other information that can be shared about the security issue? So a proper solution can be found for both the security issue as the bug that is introduced now.


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

avatar ggppdk
ggppdk - comment - 5 Aug 2016

I deleted my comments, i think better focus on a proper fix for next release,
and make a PR that can be tested

avatar ggppdk
ggppdk - comment - 6 Aug 2016

I will make a PR soon,
after i testing code thoroughly to fix this B/C break

avatar wilsonge
wilsonge - comment - 6 Aug 2016

Hey Both, grab me on my community email (george.wilson (a) community.joomla.org and I'll explain to you the issue that was fixed

avatar ggppdk ggppdk - edited - 6 Aug 2016
avatar ggppdk ggppdk - change - 7 Aug 2016
Title
Allow Edit after Soft deny is impossible in J3.6.1 / J3.6.2 for Articles component (backend article controller)
Allow Edit after Soft deny is no longer possible (always denied) for Articles manager
Status New Closed
Closed_Date 0000-00-00 00:00:00 2016-08-07 19:59:59
Closed_By ggppdk
avatar ggppdk ggppdk - close - 7 Aug 2016
avatar ggppdk ggppdk - change - 7 Aug 2016
Status Closed New
Closed_Date 2016-08-07 20:00:05
Closed_By ggppdk
avatar ggppdk ggppdk - reopen - 7 Aug 2016
avatar ggppdk ggppdk - edited - 7 Aug 2016
avatar ggppdk ggppdk - change - 8 Aug 2016
Status New Closed
Closed_Date 0000-00-00 00:00:00 2016-08-08 00:46:00
Closed_By ggppdk
avatar ggppdk ggppdk - close - 8 Aug 2016
avatar ggppdk
ggppdk - comment - 8 Aug 2016

Please test my PR #11511

Add a Comment

Login with GitHub to post a comment