PR-staging

Success

User tests: Successful: Unsuccessful:

avatar Bakual
Bakual
24 Sep 2014

Issue

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.

Solution

Reverting PR #4198 since it broke "Save as Copy" and "Save & New" buttons in com_content.

Testing

Make sure ACL works in com_content and other extensions as expected.

avatar Bakual Bakual - open - 24 Sep 2014
avatar jissues-bot jissues-bot - change - 24 Sep 2014
Labels Added: PR-staging
avatar b2z
b2z - comment - 24 Sep 2014

May be the better fix will be to add missing rules to article's section in the access.xml of com_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 supporting section.

avatar Bakual
Bakual - comment - 24 Sep 2014

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.

avatar sanderpotjer
sanderpotjer - comment - 24 Sep 2014

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.

avatar stellainformatica
stellainformatica - comment - 24 Sep 2014

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/.

avatar b2z
b2z - comment - 24 Sep 2014

Hello @stellainformatica. This is the PR ;)

avatar stellainformatica
stellainformatica - comment - 24 Sep 2014

Opssssssss sorry
Now I've tested, the issue reported in #4328 is solved

avatar brianteeman brianteeman - change - 24 Sep 2014
Category ACL
avatar brianteeman brianteeman - test_item - 24 Sep 2014 - Tested successfully
avatar brianteeman brianteeman - alter_testresult - 24 Sep 2014 - stellainformatica: Tested successfully
avatar brianteeman
brianteeman - comment - 24 Sep 2014

@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/.

avatar MAT978 MAT978 - test_item - 24 Sep 2014 - Tested successfully
avatar MAT978
MAT978 - comment - 24 Sep 2014

@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/.

avatar ralain
ralain - comment - 24 Sep 2014

@test
Patch works, and also restores "Save as Copy" and "Save & New" buttons in the Module editor.

avatar isidrobaq
isidrobaq - comment - 24 Sep 2014

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/.

avatar ralain
ralain - comment - 24 Sep 2014

@isidrobaq It seems to be related, as the patch also restores the missing tabs (from both Article and Module editors.)

avatar MAT978
MAT978 - comment - 24 Sep 2014

@isidrobaq
but the patch seems to fix also the tabs?

avatar sanderpotjer
sanderpotjer - comment - 24 Sep 2014

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.

Let me provide some background

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.

avatar b2z
b2z - comment - 24 Sep 2014

Thanks @sanderpotjer now it is clear :+1: RTC? ;)

avatar Bakual
Bakual - comment - 24 Sep 2014

Excellent explanation Sander!

RTC!

avatar Bakual Bakual - change - 24 Sep 2014
Labels Added: RTC
avatar Kubik-Rubik Kubik-Rubik - test_item - 24 Sep 2014 - Tested successfully
avatar Kubik-Rubik Kubik-Rubik - change - 24 Sep 2014
Status Pending Ready to Commit
avatar phproberto phproberto - close - 24 Sep 2014
avatar phproberto phproberto - change - 24 Sep 2014
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2014-09-24 22:37:51
avatar blueforce blueforce - test_item - 25 Sep 2014 - Tested successfully
avatar DonPedro34
DonPedro34 - comment - 26 Sep 2014

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.

avatar berlanda
berlanda - comment - 6 Dec 2014

I'm sorry by the problems caused. I noticed the missing buttons too late, after the update.

Add a Comment

Login with GitHub to post a comment