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

[UX] Better Menu permissions #384

Open
ghost opened this issue Oct 22, 2014 · 12 comments
Open

[UX] Better Menu permissions #384

ghost opened this issue Oct 22, 2014 · 12 comments
Assignees
Milestone

Comments

@ghost
Copy link

ghost commented Oct 22, 2014

Similar to #382, I'd like to see better permissions for Menus. Drupal's Menu Admin per Menu module provides separate permissions for each menu, so something like this would be good.

@ghost
Copy link
Author

ghost commented Oct 24, 2014

Here's everything that MAPM does (based on this code):

  1. Adds a new 'Administer [MENU] menu items' permission for each menu.
  2. Changes access of:
    • 'admin/structure/menu' to anyone with any menu access
    • 'admin/structure/menu/manage/%', 'admin/structure/menu/manage/%/add', 'admin/structure/menu/item/%/edit', 'admin/structure/menu/item/%/reset' and 'admin/structure/menu/item/%/delete' to anyone with access to that menu
  3. Edits 'admin/structure/menu' page if user doesn't have 'Administer menus and menu items' permission:
    • Hides menus the user doesn't have access to
    • Hides the 'Edit menu' link for each menu
  4. Restricts 'Parent item' options (in Menu Settings on Node forms) to allowed menu items.
  5. Restricts 'Parent link' options (on Edit Menu Link forms) to allowed menu items.

@quicksketch quicksketch added this to the 1.x-future milestone Nov 23, 2014
@jenlampton jenlampton changed the title Better Menu permissions [UX] Better Menu permissions May 2, 2015
@jenlampton
Copy link
Member

Yes please! I also want better UX for these new permissions. Can I pretty please have a vertical tab on the menu config form that adds the permissions for menus there? See related: #900 and #38

@jenlampton jenlampton modified the milestones: 1.4.0, 1.x-future Jan 29, 2016
@jenlampton
Copy link
Member

I'm running into limitations today with the menu permissions not being granular enough on backdropcms.org - and I'm porting Menu Admin per Menu. I would love to see a solution for this get into 1.4. It would pair nicely with all the other permissions work we achieved in 1.3 :)

@jenlampton
Copy link
Member

I'm just gonna leave this right here...
https://github.com/backdrop-contrib/menu_admin_per_menu

@serundeputy
Copy link
Member

Looks like we will not have time for this moving to 1.5.

@serundeputy serundeputy modified the milestones: 1.5.0, 1.4.0 May 14, 2016
@jenlampton jenlampton modified the milestones: 1.x-future, 1.5.0 Sep 5, 2016
@jenlampton
Copy link
Member

Looks like we won't have time for this in 1.5 either, pushing back to 1.x-future.

@ghost
Copy link
Author

ghost commented May 12, 2019

It seems this issue is bigger than just wanting more granular control of menus. I just noticed that access to a node's "Menu Settings" vertical tab is controlled by the "Administer menus and menu items" permission. This means that to give an editor access to create/edit a node's menu item, they need access to the whole menu structure, including the Administration menu. Otherwise they simply can't create/edit menu items on the node form at all. That's not ideal, to the point I'm tempted to mark this as a bug report... Actually I think I will.

@ghost ghost added the type - bug report label May 12, 2019
@jenlampton
Copy link
Member

I do like the idea of adding a separation of permissions from "menus" and "menu items". But we will still need to give people who get access to manage menu items, access to each menu admin page (admin/structure/menu/manage/main-menu) so that they can adjust the order of menu items. So the Administer menus permission should maybe only apply to the menu settings page (admin/structure/menu/manage/main-menu/configure).

@olafgrabienski
Copy link

Yes, let's separate the permissions of menus and menu items! Quite important, in my opinion: Without Administer menus, a person shouldn't be able to delete a menu.

Do we want to leave more fine-grained control, e.g. permissions per menu, as described in #384 (comment) to the https://github.com/backdrop-contrib/menu_admin_per_menu module?

@jenlampton
Copy link
Member

Do we want to leave more fine-grained control, e.g. permissions per menu

Maybe that's best left to contrib if we can get the separation of menus + menu items in core.

@klonos
Copy link
Member

klonos commented Aug 26, 2020

This can be even more problematic with the addition of things like #1285 and #1610 in core, so I think that we should aim to fix things in 1.18.0 if not sooner. Vanilla Backdrop installation use case:

  1. Realise that editors should be able to edit the menu items of pages they create (for example they create a "Who we are" page, but want the menu item for it to be "About")
  2. Go to permissions -> only permission is the "Administer menus and menu items" one -> grant that to the "Editor" role.
  3. Now ALL content editors have access to edit/delete ALL menus and menu items; EVEN THE ADMIN MENU!! 😱

I think we should introduce a "Manage menu items in the %menu% menu" permission, that allows adding/deleting/editing/reordering menu items for that menu. What people with this permission will NOT be able to do is edit the menu name/description, nor delete the entire menu, nor export it as config, nor create new menus.

The "Administer menus and menu items" should become a "master" permission that allows any manipulation of any menu and its menu items (also future menus created). As such, the "Warning: Give to trusted roles only; this permission has security implications" description should be added to it.

PS: I've noticed that in D8/D9 this permission was moved under the System section 🤷

PPS:

Maybe that's best left to contrib if we can get the separation of menus + menu items in core.

I think what we should avoid over-complicating the permissions and the admin UI. So a thing that should be left for contrib is permissions to specific menu items, like the the following modules:

https://www.drupal.org/project/menu_item_visibility
https://www.drupal.org/project/menu_item_role_access
https://www.drupal.org/project/menu_per_role
https://www.drupal.org/project/custom_menu_perms
https://www.drupal.org/project/roles_for_menu

Also extremely fine-grained menu operation permissions like the following should be left to contrib:

https://www.drupal.org/project/simple_menu_permissions
https://www.drupal.org/project/custom_menu_perms

I do acknowledge though that there are quite a few contrib solutions out there trying to solve similar issues, so perhaps we should consolidate things as much as possible.

@klonos
Copy link
Member

klonos commented Jul 25, 2023

I had another look at the code of the 7.x branch of the MAPM module, and it's only about 180 lines of code. For the value that it provides (mainly the granular separation of permissions per menu), I think that it is worth incorporating its functionality in core.

I might take a crack at this.

@klonos klonos self-assigned this Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants