User tests: Successful: Unsuccessful:
See Issue #4328. "Save as Copy" and "Save & New" buttons are currently not available in the article edit form.
This is because PR #4198 broke the ACL for core.create.
Reverting PR #4198 since it broke "Save as Copy" and "Save & New" buttons in com_content.
Make sure ACL works in com_content and other extensions as expected.
Labels |
Added:
?
|
The thing is, that original PR may well have broken also ACL in 3rd party extensions. Or am I wrong with this assumption?
If so fixing com_content is not an option to solve this issue.
The deprecated method had some special case for com_content built in, which was of course crappy code.
For now I'm perfectly fine with reverting the PR according to our development strategy. Nothing really was broken before that PR was merged and now something is broken.
If someone comes up with a better fix which is tested good, then I'm fine with using that fix of course.
Imho, the code should actually also check category and component sections to see if there are additional ACL rules. But I'm not sure if that is easy to do and wouldn't break something else. I'm leaving that for ACL experts like @sanderpotjer to decide.
I have to go to a meeting very soon, so a quick sharing my thoughts on this issue without having looked in detail at it, will jump in later today again.
They expected Joomla ACL behavior is that all available actions for a component are listed in the "component" section of the access.xml file. This allows people to configure actions component wide instead of per item. So if an article has a specific action it should be included in the component section of the access.xml file.
So what happens here is that an article is checked for all actions in the article section (core.delete, core.edit, core.edit.state). On the page a core.create action is checked as well, but this is not a valid action for the article section, only available in the component and category section in case of com_content.
My first thought is that the actions should always be checked for the component section, so for all available actions in a component. If correctly the core.edit is still checked for the article first in case of an article, if missing for the category, if that is missing for the component and as last fallback for the global configuration.
As far as I know all Joomla core components have all available component actions included in the component section of the access.xml. But... most likely not all 3rd party components did this.
I will look into this more detailed in about 4 hours from now.
Hi Bakual, I would like to test but it's difficult without a new pull request that fixes the issue. I don't know how to revert the PR #4198, can you please make a new PR for testing?
This comment was created with the J!Tracker Application at http://issues.joomla.org/.
Hello @stellainformatica. This is the PR ;)
Category | ⇒ | ACL |
@test I have confirmed the issue as reported with the buttons and tabs. The patch successfully reverts to the original behaviour. Thanks
Waiting on Sander "acl guru" Potjer to comment
This comment was created with the J!Tracker Application at http://issues.joomla.org/.
@test
Was able to reproduce the issue then #4331 works as expected.
Thanks
This comment was created with the J!Tracker Application at http://issues.joomla.org/.
Just noticed that there are more things missing in article edition screen, and maybe they're related to this issue. Specifically I miss "Configure edit screen" and "Permissions" tabs, that should appear after "Options" tab (below title box). Is this related to this issue or a new unrelated one?
This comment was created with the J!Tracker Application at http://issues.joomla.org/.
@isidrobaq It seems to be related, as the patch also restores the missing tabs (from both Article and Module editors.)
@isidrobaq
but the patch seems to fix also the tabs?
I had a closer look at this issue. Executive summary: I agree with this pull request to revert PR #4198. Also @test fixes the reported issues of missing buttons and tabs.
Originally the pull request #4198 was not correct as it made some wrong assumptions. @berlanda wanted to perform access checks agains actions listed in the category sections of the access.xml
file.
By default the getActions
function always retrieves the available actions for a component from the component section of the access.xml
file:
$actions = JAccess::getActionsFromFile($path, "/access/section[@name='component']/");
This is because all available actions should be listed in the component section, otherwise it is not possible for users to configure permissions for an action component wide. This is the default Joomla approach, which you can see by looking at the com_content access.xml
file:
<access component="com_content">
<section name="component">
<action name="core.admin" title="JACTION_ADMIN" description="JACTION_ADMIN_COMPONENT_DESC" />
<action name="core.manage" title="JACTION_MANAGE" description="JACTION_MANAGE_COMPONENT_DESC" />
<action name="core.create" title="JACTION_CREATE" description="JACTION_CREATE_COMPONENT_DESC" />
<action name="core.delete" title="JACTION_DELETE" description="JACTION_DELETE_COMPONENT_DESC" />
<action name="core.edit" title="JACTION_EDIT" description="JACTION_EDIT_COMPONENT_DESC" />
<action name="core.edit.state" title="JACTION_EDITSTATE" description="JACTION_EDITSTATE_COMPONENT_DESC" />
<action name="core.edit.own" title="JACTION_EDITOWN" description="JACTION_EDITOWN_COMPONENT_DESC" />
</section>
<section name="category">
<action name="core.create" title="JACTION_CREATE" description="COM_CATEGORIES_ACCESS_CREATE_DESC" />
<action name="core.delete" title="JACTION_DELETE" description="COM_CATEGORIES_ACCESS_DELETE_DESC" />
<action name="core.edit" title="JACTION_EDIT" description="COM_CATEGORIES_ACCESS_EDIT_DESC" />
<action name="core.edit.state" title="JACTION_EDITSTATE" description="COM_CATEGORIES_ACCESS_EDITSTATE_DESC" />
<action name="core.edit.own" title="JACTION_EDITOWN" description="COM_CATEGORIES_ACCESS_EDITOWN_DESC" />
</section>
<section name="article">
<action name="core.delete" title="JACTION_DELETE" description="COM_CONTENT_ACCESS_DELETE_DESC" />
<action name="core.edit" title="JACTION_EDIT" description="COM_CONTENT_ACCESS_EDIT_DESC" />
<action name="core.edit.state" title="JACTION_EDITSTATE" description="COM_CONTENT_ACCESS_EDITSTATE_DESC" />
</section>
</access>
As you can see <section name="component">
contains all actions from the lower category & article 'levels'.
Getting back to @berlanda issue in #4198. Without changing the Joomla core code this can be solved by including the two actions in the example into the component section:
<section name="component">
<action name="core.admin" title="JACTION_ADMIN" description="JACTION_ADMIN_COMPONENT_DESC" />
<action name="core.manage" title="JACTION_MANAGE" description="JACTION_MANAGE_COMPONENT_DESC" />
<action name="roles.list" title="roles list" description="" />
<action name="roles.manage" title="roles manage" description="" />
</section>
<section name="category">
<action name="roles.list" title="roles list" description="" />
<action name="roles.manage" title="roles manage" description="" />
</section>
In this way $this->canDo = JHelperContent::getActions('com_foobar', 'category', $item->catid);
works properly.
In this issue of the missing buttons the core.create
action is checked for displaying the buttons or not. As you can see above the core.create
action is not available in the article section of the access.xml
file. This is correct as you can't create an article in an article itself. But that doesn't mean that the action can't be used. The same for core.admin
, which is only available in the component section but used for showing the permission tab or not. So if you only perform a check against the actions for the article section you are missing the core.create
, core.admin
, etc... actions. Resulting in this issue right now.
By using the component section to retrieve the actions doesn't mean that the actions are checked against the component permissions rather then the article/category ones. This is exactly where the second argument ($section
) in the getActions()
function is coming in. This is used to get the correct $assetName
for checking all available actions against (https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/helper/content.php#L128).
So $user->authorise($action->name, $assetName)
can be read as $user->authorise('core.create', 'com_content.article.12')
for example. Joomla will notice that the core.create
action is not available for the article level asset, and will check the parent asset (the category), if that is missing the component asset permissions and if that is missing the global configuration asset (the nested asset structure in Joomla).
Hopefully this explains why the component section should always be used for the getActionsFromFile()
check.
Thanks @sanderpotjer now it is clear RTC? ;)
Excellent explanation Sander!
RTC!
Labels |
Added:
?
|
Status | Pending | ⇒ | Ready to Commit |
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-09-24 22:37:51 |
Applied the patch to a clean 3.3.4. (no other patches)
if i click on Content and then Article Manager, a white page is returned with "Not Found"
Must i install other patches to get this working?
When reverting the Patch #4331 the message disappears and i can edit Articles and Categories again.
I'm sorry by the problems caused. I noticed the missing buttons too late, after the update.
May be the better fix will be to add missing rules to article's section in the
access.xml
ofcom_content
?Because this really does not make sense:
$actions = JAccess::getActionsFromFile($path, "/access/section[@name='component']/");
We always fallback to the
component
section. Note that deprecated_getActions()
method is supportingsection
.