Hi everybody,
while working on the Document Rights Management Improvement, @mleduc and I noticed that such a fundamental new concept is very hard to get right, in particular when taking backwards-compatibility into account. Further, a lot more work is required to analyze the content to suggest appropriate required rights both for a migration and for improving the usability when users add macros or XObjects to pages that should have certain rights. This makes it currently unrealistic to introduce all of this in one go, in particular with good usability.
What I would like to therefore propose instead is to basically reverse the order of these two steps and to first introduce the analysis code to find which rights are required on a page. This analysis would both do a static analysis on macros in page content (recursively analyzing the content of macros with wiki content similar to the link extraction) and an analysis of the XObjects on a page, including their fields that could again contain syntax that would be analyzed for macros.
The analysis would yield detailed results for every place where it determined that a right is missing including the location like content, title or object (property) and a message telling what exactly is wrong which includes the content that would have required the right (e.g., the content of a Velocity macro if script right is missing). The message should also tell which right is most likely required.
The proposal is to use these analyzers in one or several of the following ways, some of them might be configurable:
- For users with edit rights, display a warning in view mode above every document whose analysis determines that its last author(s) don’t have sufficient rights, and thus, e.g., a JSX cannot be executed. The warning should be short with a button to display detailed information either by expanding it or in a popup.
- When a user wants to edit a document or a property of it, the analysis would be executed both with the current author(s) and with the current user. If changing the author to the current user would change the analysis in any way, the user would get a warning similar to how the user is warned when editing extension pages. This warning has two purposes:
- Making a user aware that editing might remove errors and thereby lead to the execution of scripts with the possibility to review the exact contents of the scripts. For this case, we could also allow continuing to edit in restricted mode to avoid executing code in the WYSIWYG editor.
- Making a user aware that editing might break the page with the possibility to review the code that would be broken.
- (Optional, probably not in first version) When saving, we could either warn the user if content is broken by saving or even deny saving in this case. A really cool option would also be to allow creating a change request here such that a user with appropriate rights can then review and merge the change request.
The idea of this is to make users more aware of what impact editing a page could have on the scripts and other active content in it and thus prevent security issues from exploiting admin rights. For this reason, when implementing (2), we need to make sure that the checks are also executed during rollback and when editing single properties.
In addition to this immediate need for for security there are other features that could leverage the same APIs:
- An admin tool to find all pages that are currently missing rights, e.g., due to right or user changes or due to users with insufficient rights editing pages.
- Hide macros that require more rights than what the current user has in the macro insertion dialog, i.e., implementing Add concept of required permissions for Macros
- Directly warn users when editing objects that an object might not work as intended by having a similar concept of required permissions for objects to hide objects that the user cannot use at all and by running the analysis even before saving (the analysis would take, e.g., parameter values into account).
- Ultimately, to suggest required rights and offer ways to mass-migrate large parts of the wiki to required rights when implementing the Document Rights Management Improvement.
While thinking about some analysis code, we noticed that in some cases it is hard to analyze if some code really requires a right or not. For example, consider a title like Hashtag #XWiki - #XWiki could be a velocity macro but at the same time, for users without script right, this is the only way to write this title. We developed the idea of having a distinction between “error” where it is clear that there is an error and “warning” where content just might be different with more rights. The idea would be to display the warning in view mode only when there are errors in the analysis result to not annoy users but to include warnings in the screen before editing.
The HTML macro is also a bit special in the sense that in some modes it is basically impossible to determine if some content could contain JavaScript or not. This in particular the case with wiki=true. We came up with the following table:
| Wiki parameter | Clean parameter | Analysis result |
|---|---|---|
| any | false | Needs script right |
| false | true | Depends on static analysis of the HTML using the sanitizer filter. |
| true | true | Might need script right - classify as warning as analysis is impossible (HTML depends on macro execution). |
We haven’t looked into this yet, but the idea would also be to analyze the XClass that might be defined in a document to understand which rights its properties like DB lists require.
Analyzers would be implemented as components with hints depending on macro or XClass name such that extensions can contribute their own analyzers. Further, there would be generic fallback analyzers that e.g., just analyze the content of a macro if it indicates support for syntax or XObject properties based on property types.
We propose to implement the analysis based on the authors set in the document and not as a generic analysis which rights are required independent of the author as this allows leveraging some existing code like MacroPermissionPolicy and it gives the code more freedom to take additional conditions into account that cannot be expressed as rights, like a macro indicating that it is only available to a single user. Still, the analysis results should indicate which right would be required (both in text form and as a Right object with an EntityType to indicate that a right could be required on space or wiki level) such that an analysis run, e.g., with guest as author could determine the minimum rights that are required.
Do you have any opinions on this?
@mleduc and I started already some first work on this, you can see it here. Unfortunately, it seems unrealistic to get this fully finished into XWiki 15.8 but we would at least try to have an almost finished version in the 15.8 timeframe that we could either include disabled/without UI in 15.8 or include for 15.9.