OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.37k stars 1.12k forks source link

Removing View Content Permission prevent viewing Widgets too #8230

Open HermesSbicego-Laser opened 5 years ago

HermesSbicego-Laser commented 5 years ago

Repro

The BuildDisplay of widgets should test a custom permission: I think we should add a specific permission "View Widgets" and Adjust to it when testing the ViewContent; The new permission will be overridden if a specific widget is set as Securable, so we keep the ability to be more specific if necessary.

NB: I tried to make every widget "Securable" but is not suitable beacuase of too much configuration work.

sebastienros commented 5 years ago

Are you authenticated when you access the homepage with this repro?

HermesSbicego-Laser commented 5 years ago

No, anonymous

sebastienros commented 5 years ago

How can you see anything if "Remove to Anonymous role the "View all content" and "View own content""

HermesSbicego-Laser commented 5 years ago

I will try to explain me better. This is my case. 5 contenttypes, securable

Anonymous should be able to view Pages BlogPosts Projections And all widgets But they should not be able to view Companies Insertions

In order to give permissions to view only those contents I need to remove "view all contents" and "view own" for anonymous... Removing It, anonymous users can't view widgets no more (menu, HTML, recent blog posts, ...all...) I think this is an issue.

To make every widget "Securable" could be a workaround solution but It is not suitable because of too much configuration work and introduces redundant edit/publish/delete permission for all widgets, imo.

sebastienros commented 5 years ago

The content item controller should still check the permission for the widget, but the layer should not. Did you find where this is done right now?

MatteoPiovanelli-Laser commented 5 years ago

Easy repro: Brand new tenant. By default, anonymous users see the main menu on top of the home page. Edit the Anonymous role. Remove "View all content" but add "View Page by others". Now anonymous users won't see the main menu on top of the home page.

MatteoPiovanelli-Laser commented 5 years ago

I think permissions for the widgets are being checked here: https://github.com/OrchardCMS/Orchard/blob/bdf97f570d3e88895cdae8887208fabda1981438/src/Orchard.Web/Modules/Orchard.Widgets/Filters/WidgetFilter.cs#L84

MatteoPiovanelli-Laser commented 5 years ago

But, I would think that if I place a Widget on a Layer whose rule evaluates to true, you'd be automatically authorized to view that widget.

MatteoPiovanelli-Laser commented 5 years ago

I would look to remove that condition from that filter, but that may break someone's tenants. Maybe we can figure out a way to add a ViewWidgets permission that we adjust into, and that is implied by VIewAllContent.

MatteoPiovanelli-Laser commented 5 years ago

A quick test doing just that with a ViewWidgets permission: Not everything shows. For example, from a menu: The menuWidget itself is shown, but not a CustomLink menuItem. At the same time, a ContentMenuItem that links to an item that the user can view will show in the menu. image Off that menu, an anonymous user with the ViewWidget and the ViewPage permission would see the second menu item but not the first

sebastienros commented 5 years ago

Maybe a ViewWidget permission that inherits from ViewContent. So we could disable ViewContent and still add ViewWidgets?