[#12799] - [ACL] [com_content featured view] Correct invalid check
- Closed
- 11 Nov 2016
- Medium
- Build: staging
- # 12799
- Diff
- andrepereiradasilva:patch-28
- Success continuous-integration/drone the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Summary of Changes
There is an ACL check $user->authorise('core.edit.state', 'com_content.article') in this view.
This is an invalid ACL check. For ACL check articles we need to check with the article id (example: 'com_content.article.99'.)
So this PR removes this ACL check, since it is doing nothing and is bad code.
Testing Instructions
Mainly code review, but you can test if featured view continues to work.
1. Apply patch
2. Featured view still work.
Documentation Changes Required
None.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration Components Templates (admin) |
| Title |
|
||||||
There is a few views with $canOrder variable. What about others?
$ cd administrator/
$ grep -rnIG "canOrder"
components/com_plugins/views/plugins/tmpl/default.php:22:$canOrder = $user->authorise('core.edit.state', 'com_plugins');
components/com_content/views/featured/tmpl/default.php:22:$canOrder = $user->authorise('core.edit.state', 'com_content.article');
components/com_newsfeeds/views/newsfeeds/tmpl/default.php:22:$canOrder = $user->authorise('core.edit.state', 'com_newsfeeds.category');
components/com_banners/views/banners/tmpl/default.php:22:$canOrder = $user->authorise('core.edit.state', 'com_banners.category');
components/com_tags/views/tags/tmpl/default.php:26:$canOrder = $user->authorise('core.edit.state', 'com_tags');
components/com_users/views/levels/tmpl/default.php:22:$canOrder = $user->authorise('core.edit.state', 'com_users');
components/com_patchtester/backups/3a28a5b10eb0cc9e1e1302231a06d242.txt:20:$canOrder = $user->authorise('core.edit.state', 'com_content.article');
templates/hathor/html/com_plugins/plugins/default.php:20:$canOrder = $user->authorise('core.edit.state', 'com_plugins');
templates/hathor/html/com_plugins/plugins/default.php:85: <?php if ($canOrder && $saveOrder) :?>
templates/hathor/html/com_content/featured/default.php:20:$canOrder = $user->authorise('core.edit.state', 'com_content.article');
templates/hathor/html/com_content/featured/default.php:97: <?php if ($canOrder && $saveOrder) :?>
templates/hathor/html/com_newsfeeds/newsfeeds/default.php:22:$canOrder = $user->authorise('core.edit.state', 'com_newsfeeds.category');
templates/hathor/html/com_newsfeeds/newsfeeds/default.php:109: <?php if ($canOrder && $saveOrder) :?>
templates/hathor/html/com_banners/banners/default.php:20:$canOrder = $user->authorise('core.edit.state', 'com_banners.category');
templates/hathor/html/com_banners/banners/default.php:103: <?php if ($canOrder && $saveOrder) : ?>
templates/hathor/html/com_contact/contacts/default.php:21:$canOrder = $user->authorise('core.edit.state', 'com_contact.category');
templates/hathor/html/com_contact/contacts/default.php:113: <?php if ($canOrder && $saveOrder) :?>
templates/hathor/html/com_weblinks/weblinks/default.php:21:$canOrder = $user->authorise('core.edit.state', 'com_weblinks.category');
templates/hathor/html/com_weblinks/weblinks/default.php:107: <?php if ($canOrder && $saveOrder) :?>
templates/hathor/html/com_languages/languages/default.php:22:$canOrder = $user->authorise('core.edit.state', 'com_languages');
templates/hathor/html/com_languages/languages/default.php:84: <?php if ($canOrder && $saveOrder) :?>
templates/hathor/html/com_users/levels/default.php:20:$canOrder = $user->authorise('core.edit.state', 'com_users');
templates/hathor/html/com_users/levels/default.php:55: <?php if ($canOrder && $saveOrder) :?>
templates/hathor/html/com_modules/modules/default.php:22:$canOrder = $user->authorise('core.edit.state', 'com_modules');
templates/hathor/html/com_modules/modules/default.php:116: <?php if ($canOrder && $saveOrder) :?>
templates/hathor/html/com_menus/items/default.php:23:$canOrder = $user->authorise('core.edit.state', 'com_menus');
templates/hathor/html/com_menus/items/default.php:107: <?php if ($canOrder && $saveOrder) :?>
humm ... thinking more about it ... should we block the order because we don't allow core.edit.state in the component, ie, is order considered a state?
Maybe should exist a core.edit.order?
Do not allow to reorder if user can not change core.edit.state in component.
Do you want administrator who can not reorder but can change state (publish/upublish) or vice versa?
ok, so i reverted the changes and just corrected the asset name from com_content.article to com_content.
Will do the same in the other PR
| Title |
|
||||||
| Title |
|
||||||
Do not forget to use $canOrder in administrator/components/com_content/views/featured/tmpl/default.php :)
actually this one seems correct because it's really usign $canChange which is the core.edit.state of the article itself.
See
- https://github.com/andrepereiradasilva/joomla-cms/blob/patch-28/administrator/components/com_content/views/featured/tmpl/default.php#L131
- https://github.com/andrepereiradasilva/joomla-cms/blob/patch-28/administrator/components/com_content/views/featured/tmpl/default.php#L112
So i guess we need a complete review here ...
IMO should work like this:
- If the component allows advanced ACL (ie. component, category and item level) it would use the
$canChangewhich would be thecore.edit.stateof the item itself. - If the component allows medium ACL (ie. component, category level) it would use the
$canChangewhich would be thecore.edit.stateof the category of the item. - If the component allows simple ACL (only component level) it would use the
$canOrderwhich would be thecore.edit.stateof the component. - If component does not support
core.edit.statethen no check.
For points 2,3,4 OK.
I'm wondering whether articles re-ordering should use individual permission from each article ACL.
i think it should, actually it does that, because you can't edit the state of the article you shouldn't be able to change it's order. Since we don't have core.edit.order acl it would follow the same logic as the others, and actually it's what is applied in most views now.
Then please remove $canOrder from administrator/components/com_content/views/featured/tmpl/default.php when it is not used.
closed will be replace by one more integrated PR
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-11-11 15:21:13 |
| Closed_By | ⇒ | andrepereiradasilva |
| Category | Administration Components Templates (admin) | ⇒ | Administration com_content Templates (admin) Components |
I have tested this item✅ successfully on e14f923
There is an issue with hathor ordering buttons (in rows) disappearing after every reorder operation,
but this is not related to this PR
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12799.