we got recently a bug report (https://jira.xwiki.org/browse/XWIKI-17754) about using the
attachment.download.blacklist configuration to force downloading an attachment, in case of an attachment added by a PR user. Following this issue, I proposed a first PR with a new behaviour but after discussing it with @tmortagne (discussion started there: https://matrix.to/#/!ikPtGZaGWtyblizzlR:matrix.xwiki.com/$16012960212711ZcDTy:matrix.xwiki.com?via=matrix.xwiki.com&via=matrix.org&via=t2bot.io) it appeared that:
- the reported bug is right now an expected behaviour
- we need to decide if it’s a behaviour we still want
- we need to decide how to improve it to fix the “bug”
So, right now the behaviour of downloading attachment is the following:
- if the attachment has been added by a user with PR (Programming Rights), the attachment is always displayed inline.
- if the attachment has been added by someone without PR, and a configuration is given for
attachment.download.blacklistand no configuration is given for
attachment.download.whitelistthen the mimetype of the attachment is checked against the mimetype list of this blacklist to define if it should be downloaded or displayed inline (if present in blacklist, it won’t be displayed inline)
- in other cases, the mimetype of the attachment is checked against the mimetype list defined in
attachment.download.whitelist: this one might be the default whitelist or a custom one.
Note that this behaviour is currently partially explained in https://www.xwiki.org/xwiki/bin/view/FAQ/How%20to%20open%20PDF%20attachments%20from%20the%20wiki%20directly%20in%20the%20browser (partially because it doesn’t explain in which condition the blacklist should be used).
Note 2: It’s always possible to use a
force-download request parameter to override the mentioned behaviour and force the download, I won’t mention it anymore here since this proposal is about the default behaviour without specific parameters.
So the problem with the current behaviour, and the bug reported in XWIKI-17754 is that right now the attachments can never be forced to be downloaded by default if they have been added by a PR user. My first proposal to overcome this issue was to change the way we resolve this, by checking first the blacklist, before checking if the attachment was added with PR or not.
As indicated in my first PR, the resolution was then the following:
The problem with that solution is that it could break compatibility of some configurations: we couldn’t handle usecase where a wiki is configured to allow only PR users to display images attachments inline. If you have blacklisted some mimetypes thinking “it’s only for normal users” you’ll be hit by the change.
A way to overcome this, would be to introduce a new boolean configuration, to basically say “use the old mechanism”.
Now @tmortagne proposed another solution, which would be to introduce a new list of mimetypes, for attachments “forced to be downloaded”: this list would be taken into account before the actual mechanism and would allow to force downloading attachments based on mimetype, even if added by PR users.
AFAICS there is no scenario (based on mimetype) that we couldn’t support. The only problem I can think of with this solution is that it makes the configuration for mimetypes of attachment a bit more difficult to understand with 3 different lists. Now Thomas mentioned that we could move the existing configuration lists to a “Security” section since those are about protecting against XSS, and put the new configuration in a “Attachment” section, since it’s only about usability.