optional route param for content-item display #16355
Conversation
| @@ -37,7 +37,7 @@ public async Task<IActionResult> Display(string contentItemId, string jsonPath) | |||
| return this.ChallengeOrForbid(); | |||
| } | |||
|
|
|||
| var model = await _contentItemDisplayManager.BuildDisplayAsync(contentItem, this); | |||
There was a problem hiding this comment.
I don't think we want to allow this. It can be a security issue that a user can specify arbitrary display types, even like SummaryAdmin, and thus look at the display of an item that normally they should never see.
There was a problem hiding this comment.
while i understand your point, I think of the display type as a visual layout of the content item so any sensitive information should not be related to the displaytype. we already use permissions for this. I mean you could easilly leak sensitive info in the detail display type too.
There was a problem hiding this comment.
The current assumption is though that the display type is something that's explicitly always set either by the developer or the administrator, and the developer can assume the context of each of them. This would break these assumptions.
If you search for "SummaryAdmin" in the codebase, you'll see that there are plenty of examples where the generates shapes display something meant for the admin without further authorization (e.g. the ArchiveLater, Localization, Lists modules). Regardless of these, or the other examples in OC being security issues or not, these changes would open up potentially unintended disclose of information displayed in such shapes.
We could audit Orchard Core to add authorizations everywhere, but then third-party code will still be open for abuse. And this is only "SummaryAdmin", but now you could generate displays any other, custom display type as well, with consequences that we can't determine.
allows optional route displaytype option for content item display