Tag Search Page not observing permissions

Topics: General
Jun 26, 2013 at 11:53 PM
I noticed that Orchard.Tags search result page does not observe custom content permissions for a content item. When viewing the result page, anything associated with the tag is displayed in summary view, even if the user does not have permissions to the item.

Digging deeper, this extends beyond just Orchard.ContentPermissions.

If a user's roles only grant ViewOwnContent at a global level, Tag Search still displays summaries for all content associated with the tag.

I opened a ticket #19836 for this.
Jun 26, 2013 at 11:57 PM
Edited Jun 27, 2013 at 12:06 AM
Resolved in Pull Request - #19836 - TAG SEARCH & VIEWCONTENT
Coordinator
Jun 27, 2013 at 12:29 AM
We knew about this issue but fixing it means that it's breaking the pagination.

Though, not having it fixed is actually worse ...
Jun 27, 2013 at 1:44 AM
Broken pagination is only that less than a page size of items might display. I'm not sure of how to get at the Authorizer against IContent within the IContentQuery .Where (Only the record is available). If I knew how, I would happily fix this in other places, too, such as the Blog list and in Taxonomy.
Coordinator
Jun 27, 2013 at 1:49 AM
You can't. The only way to do that would be to preindex the visibility of content items for each role, whenever permissions or content items are changed.
Jun 27, 2013 at 1:53 AM
Should I modify the content query for tags to instead just get all matching items from NHibernate, check permissions once they are retrieved, and slice the result set? yield return should still kill permission processing on the collection once we have hit the page size.

It's a bigger hit to the database, but at least we will have paging AND proper permissions.

I can update my pull request if that's the desired direction.
Jun 27, 2013 at 1:59 AM
Hmm... That's going to be a big hit if there are a lot of items.
What can we do?
Coordinator
Jun 27, 2013 at 2:29 AM
That's exactly why we did nothing so far. There is no ideal solution. It's a case by case basis, one can always apply checking in their template and display a placeholder instead.
Developer
Jun 27, 2013 at 2:52 AM
And it's not only about this page but all other places where pagination is used (containers, index search and projections iirc). There's no silver bullet. Theoretically we could eg. store role permissions in the database (and keep those in sync) for query purposes but it could open up a whole lot of new issues...

I did that more or less as Sebastien previously described - by keeping an index of content item ids available for each role - and used that to filter items based on ids (Contains) when performing a query.
Jun 27, 2013 at 3:35 AM
Then should that be added in? Put in a placeholder as the default? Perhaps wrap the content item through Tags Search with a placeholder that checks ViewContent, and the developer can customize the theme to change the placeholder (or to display the content item) if they wish. Err on caution, rather than displaying the unauthorized item.
Coordinator
Jun 27, 2013 at 6:12 PM
This is impossible. Security is implemented in a dynamic way in Orchard, using the current user for instance, or could also use the current date, whatever value. So the value you compute for a specific set of variables will be wrong in the next request.

The only safe thing that can be done to be correct is to hide items dynamically and thus breaking the pagination.
Jun 27, 2013 at 6:29 PM
I'm referring to creating a shape wrapper under the Tags SearchController--perhaps "TaggedContent_Wrapper"--to wrap all ContentItems displayed on the Tag Results. Then, in the shape wrapper's code, it can check the Authorizer to see if the current user has ViewContent for the ContentItem, and only render the contained ContentItem if ViewContent is present.

Paging will be messed up, but I think that is much less of a concern that displaying content items that the user shouldn't see. In choosing between two evils, I would prefer a page of 2 items when "Displaying 0-20 of 400", with the other 18 not rendered, then have the user see something they are not authorized for.

It's at least a stop-gap until a real solution can be implemented.