Open orchardbot opened 9 years ago
jayharris commented:
Pull Request 7814
@sebastienros commented:
I need more time to check why the change was done in the first place and that your recommendation would not break the previous fix.
jayharris commented:
If there's any way I can help, or if you would like for me to jump on a call to discuss, I'm more than happy to do so.
jayharris commented:
The issue was introduced with rev 7366af95ee7a
jayharris created: https://orchard.codeplex.com/workitem/21110
The Content Items page is broken for Moderators. Even though they do not have EditContent permissions, the page is displaying all Content Items, including ones that do not have the Creatable flag set, including all layers, widgets, and menus.
Reproduction Steps
Log in as Moderator with default Moderator permissions. Go to Admin Dashboard.
Go to Content page.
Problem Code src/Orchard.Web/Core/Contents/Controllers/AdminController.cs
The issue stems from filtering the list of Content Types used to render the page based on permissions (implemented in Revision 7366af95ee7aa1cfb9bb815199051aff43eefb1a). This filtering logic can be found within the GetCreatableTypes method. The page checks for EditContent permissions for each Creatable content type using the GetCreatableTypes method, and uses this list to get applicable content items and to build filter lists using ContentManager.Query. However, this breaks the page because Moderator doesn't have EditContent permissions for any type, creating an empty list of applicable Content Types; when an empty or null list of ContentTypeDefinitions is passed to ContentManager.Query, the filter is not used, and instead the query is executed without respect to ContentType.
This, Content Items are returned that are not just Creatable.
Why I think this is a bad idea This assumes that Dashboard users should only be able to see content if they have EditContent permissions on the entire type. However, there are cases where this isn't going to be applicable.
View Permissions: A moderator will presumably have View permissions on all content. They may want to see a full list of all content on the site, even if they cannot edit it. Being able to see the list here is much more convenient than having to browse through everything, individually. Under the implemented scenario, they cannot see anything because they do not have Edit permissions.
EditContent through Content Permissions: A user might have Edit permissions on a ContentItem without having edit permissions on the ContentType. An example would be a Blog Author that has an Author page. Through Content Permissions they may have PublishContent permissions on their Author page without having EditContent permissions to the entire Page ContentType. In the implemented scenario, the Author would not be able to navigate to edit their page because they do not have EditContent permissions on the ContentType.
Recommendation My recommendation is that the GetCreatableTypes method go back to the way it was, allowing all Creatable content types to appear in the list, regardless of if the user has EditContent permissions, and relying on the permission system to control visibility of the View | Edit | Publish links within the SummaryAdmin shape.