OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.25k stars 2.34k forks source link

OrchardCore.Menu : need Manage Menu permission to add/edit a menu item #8385

Open PiemP opened 3 years ago

PiemP commented 3 years ago

Is your feature request related to a problem? Please describe.

To edit/add a menu item is necessary have the Manage Menu permission. This permission cause the visualization of the Menus link in the admin menu, that is not good if you want to limit users about see/edit only a specific menu.

Describe the solution you'd like

For the my specific problem (hide the Menus label in the admin menu) is enough to add something to render the menu item able to be removed by NavigationBuilder but I believe is better to fix the Menus interface because it ignore the user permissions (in my case Menu is not securable and I see the button to add a Menu) and manage some permissions dedicated to the Menu Items (If I have see right, now the controller use only the Manage Menu permission).

Describe alternatives you've considered

builder.Add(S["Content"], design => design
                    .AddClass("menus").Id("menus")
                    .Add(S["Menus"], S["Menus"].PrefixPosition(), menus => menus
                        .Permission(Permissions.ManageMenu)
                        .Action("List", "Admin", rvd)
                        .LocalNav()
                        ));

instead of

builder.Add(S["Content"], design => design
                    .Add(S["Menus"], S["Menus"].PrefixPosition(), menus => menus
                        .Permission(Permissions.ManageMenu)
                        .Action("List", "Admin", rvd)
                        .LocalNav()
                        ));

PS. I'm working on dev branch, but I don't update it by a couple on month.

Skrypt commented 3 years ago

So, your goal is to allow only some users to be able to modify a specific "frontend" menu. Right now we allow all users that have the ManageMenu permission and the issue is that it will allow then to edit all the Menus without exception.

Now, you proposed change seems to only allow to hide these Menu items from the UI by assigning a specific class to each of them. This is a good workaround but it looks hacky to me.

So, right now we don't have per user permissions in OC. So, to repro, I would create a new role named "SpecificMenuNameEditor". Then I'd go assign this role to all the users I want to be editors for this menu. Then, the last step would be to pick that role as the specific editors for that menu. Then, the controller or handler would need to validate if the current user has either ManageMenu or SpecificMenuNameEditor permissions to allow modifying any menu.

So, in this case we would just need to add a "list of roles" allowed to menus.

PiemP commented 3 years ago

Sorry @Skrypt,

I believe to not explain me well.

So, your goal is to allow only some users to be able to modify a specific "frontend" menu.

Yes .

Right now we allow all users that have the ManageMenu permission and the issue is that it will allow then to edit all the Menus without exception.

No, an user with ManageMenu permission can access to the Menus page but he can't add a new menu (or better, he see the button to add a new Menu but, if he click on it, the system show "unauthorized" page). But, a user that want to edit a "Menu Item" need to have this permission.

Now, you proposed change seems to only allow to hide these Menu items from the UI by assigning a specific class to each of them. This is a good workaround but it looks hacky to me.

For my needs, in this moment, could be enough to have an identifier that allow me to hide this menu item because I don't need this (we already do this with the Media Library link).

So, right now we don't have per user permissions in OC. So, to repro, I would create a new role named "SpecificMenuNameEditor". Then I'd go assign this role to all the users I want to be editors for this menu. Then, the last step would be to pick that role as the specific editors for that menu. Then, the controller or handler would need to validate if the current user has either ManageMenu or SpecificMenuNameEditor permissions to allow modifying any menu.

No. My use case is: I have defined a new content item called Menu Custom (with a Menu part, MenuItemsList part and an Alias part), create an Admin Menu with a link for an instance of Menu Custom, create a role with the permission to interact with this content item (Menu Custom). Now, the user can see the instance of Menu custom from the backend and frontend but, if he want to edit this instance of Menu Custom, I have to give him the ManageMenu permission and other permission to allow user to edit a specific Menu Item. ManageMenu permission cause the show of the Menus link in the admin menu (not desiderable).

I don't want a specific permission for a menu because the feature that allows me to create a new admin menu can accomplish this requirement well (I eventually create an admin menu for each Menu Custom instance etc etc...).

Sorry, my written english is not good.

Skrypt commented 3 years ago

Ok, it needs some testing because I just found that there are some more issues with permission and Menus. I created a User which I assigned to the Editor role. And then went to the Content Elements page. Now, I can publish/unpublish the Menu but I can't modify it's elements which seems really strange.

And now just looked and I don't even have the Manage Menu permission assigned to that role.

vip924 commented 5 months ago

Do you know how to show or hide menu items based on the roles in Orchard Core 1.6.0?

I have 2 content items linked to 2 menu items in an Orchard Core menu. I added a new role, set permission to only a specific content item, and removed the view permission of the other content item. Still, both the content items are shown in the menu.

I tried enabling the securable option in the menu content type definition as per another post in StackOverflow.

PiemP commented 5 months ago

Admin menu right?

You can create 2 admin menus (one for one content item and one for the other content item) and use permissions related to each admin menu to give the grant to see one or both admin menu to the user.

Or you can use Placeholder Admin Node that allow you to add a menu item that need a special permission to see it.

Hope to have understand your request.

vip924 commented 5 months ago

Hi @PiemP, thanks so much for the reply, I am not using the admin menu.

I have created 2 content types and linked them to 2 menu items in the Orchard Core menu content type. (Please check the screenshot)

I added a new role, set permission to only a specific content item (Highlighted red in the screenshot), and removed the view permission of the other content item (Highlighted blue in the screenshot).

Still, both the content items are shown in the menu. But the 2nd content item (blue) is now showing "You do not have access to this resource." in the view page when I click the menu.

image

PiemP commented 5 months ago

This issue is related to the admin menus (edit menu in backend not show menu item in frontend).

I'm not sure, but I believe that doesn't exist something like the permission related to one menu link like happen for the admin menus. In admin menu if you use Placeholder Admin Node you have the possibility to add one or more permission to see one menu item.

Probably the simplest solution is about the creation of two menu and show one menu only if you have some permission. Or evaluate user permission in the frontend shape and show some Menu item only if the user have a specific permission. I don't know in liquid how you could check user permission, but in razor you can inject IAuthorizationService and use AuthorizeAsync function to check if user have this permission.