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.

Sign up for GitHub

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

Jump to bottom

Add optional permission check for menu items when adding new menu items #12748

Closed
MikeAlhayek opened this issue Nov 1, 2022 · 6 comments · Fixed by #17263
Closed

Add optional permission check for menu items when adding new menu items #12748

MikeAlhayek opened this issue Nov 1, 2022 · 6 comments · Fixed by #17263
Labels
enhancement
Milestone
2.x

Comments

@MikeAlhayek
Copy link
Member

MikeAlhayek commented Nov 1, 2022 •
edited
Loading

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

I need to add menu item to a specific menu to a the Dashboard. However, this menu item should only show up if the user is authenticated and has permission to AccessAdminPanel.

Describe the solution you'd like

Adding a permission selector when adding a menu item will help is a case like this. Also, it may be nice to trigger a custom event when the menu is being displayed for someone to hook into it from code.
with something like this

MenuHandlers.InvokeAsync((handler) => handler.Displaying(MenuDisplayingContext() 
{
	Menu = menu,
	MenuItems = menuItems 
	ContentItem = menuContentItem,
	Level = level,
	Differentiator = differentiator
}))

on the line

OrchardCore/src/OrchardCore.Modules/OrchardCore.Menu/MenuShapes.cs

Line 102 in 339d74b

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.
@hishamco
Copy link
Member

hishamco commented Nov 2, 2022

I'm wondering why you didn't use NavigationBuilder for that? You can add a certain menu item(s) when permission has been applied

@MikeAlhayek
Copy link
Member Author

@hishamco I don't think there is a way to use NavigationBuilder along with the menu. The menu shape is build using the created menu content item. If I want to use the builder, I would have to create my own menu via code instead of using the menu UI.

@hishamco
Copy link
Member

hishamco commented Nov 3, 2022

We already adding menus from code with defined permissions in many places. For menu UI there's no option for that

@sebastienros
Copy link
Member

  • new feature, with a driver that add the management ui to MenuItemPart
  • new event in the main Menu feature, that the new feature would use to not accept menu items that don't have the requirements

@sebastienros sebastienros added this to the 1.x milestone Nov 10, 2022
@MikeAlhayek
Copy link
Member Author

@sebastienros this issue is coming up again and need a way to add permission check of admin menu items.

I am okay with adding permission picker to MenuItemPart, however how would be handle cache with the permission check. We would have to cache the menu per user using the user context. Can you think of a better way to solve this problem?

Why don't suggest using a new feature for this? Why can't we just add the permission picker to the menu items as part of the existing feature. The only thing, developers will need to add user context to the admin menu in they want to use permissions. So maybe we would need to evaluate the items and determine if we should cache at a user level or not.

@sebastienros
Copy link
Member

I assume we could add a list of required permissions for all menu items that are edited. It's not adding a security layer, only hide/show a menu item based on permissions.

Add a new driver for this. Better if the permissions are stored in a custom part.

The cache should be indexed per role combination, and with some expiration. (note this should already be the case).

@MikeAlhayek MikeAlhayek mentioned this issue Dec 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
2.x
Development

Successfully merging a pull request may close this issue.

Add permission support for menu items OrchardCMS/OrchardCore
3 participants
@sebastienros @hishamco @MikeAlhayek