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

[ACL] [com_menus item edit view] Correct check on new with "All Menu Items" selected #12825

Merged
merged 1 commit into from
Nov 13, 2016

Conversation

andrepereiradasilva
Copy link
Contributor

@andrepereiradasilva andrepereiradasilva commented Nov 8, 2016

Summary of Changes

When we create a menu item without selecting a menu first, ie, we are in "All Menus Item" view and press New to create a new menu item, the core.edit.state ACL check is not correct because we don't have a menutype id.

So this PR makes it use com_menus asset in this case.

Testing Instructions

Mainly code review. But you can also:

  1. Use latest staging and apply patch
  2. Create a user in Administrator group
  3. Go to com_menus permissions and disable core.edit.state for Administrator
  4. Login with Administrator user
  5. Go to "All Menu items" view
  6. Click New for creating a new menu item and check the published field is disabled

Documentation Changes Required

None.

@andrepereiradasilva andrepereiradasilva changed the title [ACL] [com_menus item edit view] Correct check when "All Menus" is selected [ACL] [com_menus item edit view] Correct check when "All Menu Items" is selected Nov 8, 2016
@andrepereiradasilva andrepereiradasilva changed the title [ACL] [com_menus item edit view] Correct check when "All Menu Items" is selected [ACL] [com_menus item edit view] Correct check on new with "All Menu Items" selected Nov 8, 2016
@jeckodevelopment
Copy link
Member

But, even if the state is on "published" you have to specify:

  • Menu title
  • Menu item type
  • Menu

@andrepereiradasilva
Copy link
Contributor Author

don't understand your comment

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Nov 8, 2016

The thing is in this case the core is making an ACL check like this:

JFactory::getUser()->authorise('core.edit.state', 'com_menus.menu.0');

This is obvious wrong because you don't have never a menutype with id 0, so you don't have asset with that name.

@jeckodevelopment
Copy link
Member

ok, thanks!

@ggppdk
Copy link
Contributor

ggppdk commented Nov 8, 2016

I have tested this item ✅ successfully on d0cbc51

The correct asset of the correct menu is checked for existing menu items, and for new menu items when a menu is selected
e.g. 'com_menus.menu.6'

  • so this is working correctly, as it was working before

For new menu item while we are at "All menus" (what this PR fixes)
the asset that is checked (for core.edit.state) is now 'com_menus'

@infograf768
Copy link
Member

I have tested this item ✅ successfully on d0cbc51


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

@jeckodevelopment
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 8, 2016
@roland-d roland-d added this to the Joomla 3.7.0 milestone Nov 13, 2016
@roland-d roland-d merged commit 631501d into joomla:staging Nov 13, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 13, 2016
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.

None yet

6 participants