Mails

https://forum.xwiki.org/t/security-script-programming-right-escalation-by-edition-by-privileged-user/11805
https://forum.xwiki.org/t/document-rights-handling-different-rights-non-existent-documents-and-document-right-changes/13984
https://forum.xwiki.org/t/storage-and-flexibility-of-document-rights/14108
https://forum.xwiki.org/t/ux-for-changing-document-rights/14003

 Other URLs

https://cryptpad.fr/code/#/2/code/view/+TPXssg+m1xU0HLaoX27oGSc3D3KpPbVzZI2cWqmhi0/

Description

• Identified Issues
• Proposed Solution
• Known Constrains
• Scenarios
• Architecture
  • Database
  • Rights Check
• APIs Use
  • Programmatically create and edit pages
• User Experience
  • Page edition
  • Page Creation
  • Identifying pages with missing required rights
  • Changing Required Rights
• Auto-detection of RRs
• Migration Strategy
• Decreasing the rights
• TODO

Identified Issues

• CKEditor executes the content with the rights of the current user:
  • It is dangerous to use CKEditor for any user with PR/Script rights
• The rights of the current user are always granted to the page on save
  • It is dangerous to edit
  • This is in particular true for collaborative editing
  • It is easy for users to break pages. The required rights application improves this situation, but the automatic analysis cannot always determine the actually required rights, leading to annoying warnings, e.g., for users with script but without programming right.

Proposed Solution

Introduce the notion of required rights of a document to indicate which rights its content needs. These rights need to be actively set by a user that has them and then limit the available rights to those rights that are set. For example, if a document doesn't have any special rights set, no script macro will be executed. At the same time, editing is limited to users having the set rights. This protects both against accidental breakages and malicious edits that introduce, e.g., hidden scripts that are later executed.

Known Constrains

• Take into account cases like Change Request (CR) where the rights might be customized
• Take into account sheets (and include macro) → every case where we still want the rights of the sheet to be used
• Don't forget that the executed rights must always be the min of initial author x current editor
• Most pages does not have Required Rights defined, we need to a) provide a migration and b) support having pages without defined required rights and let the admin configure what should happen to them (default for now: current behavior)

Scenarios

1. simple user creates a new page with "simple" content
2. simple user creates a new page and introduces a macro that requires programming rights
3. simple user edit an existing page with no Required Rights
4. simple user edit an existing page with a Required Rights set to programming
5. admin user creates a new page with "simple" content
6. admin user creates a new page and introduces a macro requiring PR rights
7. admin user edit an existing page with no Required Rights
8. admin user edit an existing page with no Required Rights and introduce a macro requiring programming rights
9. admin user edit an existing page with a Required Rights set to programming

Architecture

Database

• Introduce a new requiredRights field on com.xpn.xwiki.doc.XWikiDocument. This field is required to distinguish between a document with zero required rights, or a document with an empty list of required rights
  • Note: an alternative is to use the presence of the EDIT right in the list of required rights, as this right is required to edit anyway.
• Introduce a new org.xwiki.doc.rights.XWikiDocumentRequiredRights with two fields: a document ID, and a right enum value. This new class represent a many-to-many mapping between rights and documents.
• TO DECIDE (see this proposal on storage and flexibility): XWikiDocumentRequiredRights includes potentially one of the following:
  • an EntityType enum to denote the level of the right (to be able to specify that, e.g., admin right is required on the wiki level)
  • an entity reference that denotes the entity on which the user needs to have the right. This could be empty to denote that the right should be on the document itself.

Rights Check

There are two cases to consider:

• Edit right: users should only be able to edit a page when they have all rights that are requested by the required rights.
• Rights of the page content, in particular script and programming right: check both the rights of the last author and the required rights set on the document

For edit right, the default AuthorizationManager will check for a user U on document D:

• if the user U has edit right on D (as before)
• if required rights are enabled, also check if the user has all the listed rights

Some notes on caching for this:

• The idea here is not to store this result in the security cache. This is because the security cache has an optimization to check if the security rules are empty on a document and if yes, it asks for access on the parent reference until it finds one with non-empty security rules. Storing required rights in the security cache would mean that suddenly all documents have non-empty rules (we would need to ensure they're stored as rules).
• A separate cache should be created to cache the required rights per document such that not the full document needs to be in the cache to check rights.

A new DocumentAuthorizationManager will be introduced to check rights taking required rights into account. When checking a right R for an author U of a document D and an entity E, it

• checks if U has right R on E, if not it returns false
• checks if requiredRights is set to true, it checks if R is listed in the required rights, if not, it returns false
• checks if requiredRights is set to false, and strict mode is enabled, it returns false
• if all the previous checks passed, it returns true

These calls accept either a document or a document reference. In the first case, the required rights are directly read from the passed document. In the second case, the required rights are read from the aforementioned cache.

The DocumentAuthorizationManager always loads the stored required rights of the passed document reference from the aforementioned cache/the stored document to avoid using unsaved changes.

The ContextualAuthorizationManager uses the DocumentAuthorizationManager when checking script or programming right. This should cover most cases of script and programming right. All other right checks, in particular those that involve admin right on the wiki level, need to be adapted to use the new DocumentAuthorizationManager.

APIs Use

Programmatically create and edit pages

The current API expects the create page to be defined with requiredRights=false

For instance, a page created using AWM will require some rights that would now be part of the RRs.

• It should be possible to force requiredRights to true in the API (note: at some point in the (far) future, we could decide to always set requiredRights to true)
• If no requiredRights value is explicitly set, we have two options:
  • do nothing and let use the legacy behavior
  • use some heuristics to set requiredRights to true and add the RRs based on the content of the page (e.g., the presence of some XObject on create and edit)

From a script in a page with required rights enabled without programming right, it shouldn't be possible to save a page without required rights, and it shouldn't be possible to save a page with more required rights than the current page.

User Experience

Page edition

A user can be disabled editing a page for two reasons:

1. The user does not have edit rights
2. The user does is missing some RRs

An explanation of the reasons for which is user is not allowed to edit a given page should be proposed to the user for the second reason. In this case the edit button should still be displayed to the user but:

• disabled
• a text explaining the restriction should be displayed on hover

This is difficult to implement as it requires a way to check edit rights without required rights, which could be abused to disable required rights.

Alternatively, a new "Rights" tab is added to the bottom of the document that displays:

• What rights the current user has on this document and the possibility to display the same for another user/group.
• Information about required rights set on the current document
• Information about the rights set on the current document/current space
• An expandable section with information about rights set on the parent spaces/wiki(s)

This should be similar to the Rights Improvements 9.x (2017) proposal except that a) it is extended to cover document rights b) it is displayed in a tab next to the document information.

Page Creation

Pages are initially created with required rights enabled but no required rights defined. Required rights will only be available after saving at least once. The UI to grant required rights will prompt the user to (optionally) save immediately to enable the granted required rights. (Proposal)

Identifying pages with missing required rights

A dashboard with a list of pages with requiredRights=false is proposed in the administration.
This dashboard would also propose to analyze documents to automatically set the required rights (based on the auto-detection described below).

We can consider different strategies:

• let users manually request for the analysis of a document
• introduce a new document analysis task that would apply the auto-detection feature on documents with requiredRights=false when they are created or updated. The result of the analysis would be stored and displayed in the dashboard. Admins would just have to click an auto-apply button to update the RRs of a page.

Changing Required Rights

The required rights API is used to analyze the document. Based on the analysis result, we can suggest one of three options:

1. Suggest setting a required right for documents that are using the legacy mode.
2. Suggest adding rights when a document contains a macro or XObject that needs more rights than the document has.
3. Suggest removing rights when a document no longer contains any macro or XObject that needs them. For example, after removing the last script macro, script right should be removed to allow editing by users without script right.

What we should consider here is that the first operation is basically migration support and not strictly necessary. We could also just provide a tool for admins to perform this migration as explained in the section before.

The second operation is crucial to make it easy to use script macros, but it is potentially dangerous as the rights will be granted to all and not just new content.

The third operation is not crucial, but having the least possible rights is desirable also to allow editing by as many users as possible, so we should support users with that. It could, however, also happen that we’re missing an analyzer and a right is required even though we cannot detect it. This would be a bug but still means that we shouldn’t push users too aggressively to remove rights.

Based on this proposal, we could do the following:

1. Display hints about all three operations in the page information tab with buttons to perform them if we’re certain and display a less strong hint when manual review would be required. In all cases, if the analysis identified any elements requiring rights, the analysis result will be shown in a dialog.
2. When performing operation 2, require the user to see the full analysis result that includes all script contents, for example, by placing the confirmation button below the content. Also, enable the confirm button only after a timeout of a few seconds.
3. When saving, suggest operation 2 when new rights are required. So, for example, when saving after adding the first Velocity macro, a dialog would show all script macros and suggest enabling either script or programming right. However, when the user select script right, there won’t be any request to enable programming right when saving again unless the user added, e.g., a Groovy macro that always needs programming right.
4. In error messages about missing script right, display a button to enable script right when the user has it - this would again show the dialog with the analysis result.

Hints about operation 1 should only be shown to advanced users to not confuse simple users with operations they don't understand. Further, this operation shouldn't be suggested on protected extension pages. For operation 2 and 3 to be available, the user would need at least script right, anyway, so they could be displayed to all users with the respective rights. Suggesting adding rights could also be more prominent, e.g., above the page content for users with the respective rights as this indicates that the page might actually be broken.

While we could already suggest enabling a right when a macro or XObject is added, in many cases the rights depend on the macro parameters, macro content or XObject fields. Therefore, this could be too early and could result in several, annoying prompts when the user changes values several times.

Similar to rights, required rights can be saved together with a document change but are only applied after the document has been saved.

Auto-detection of RRs

Auto-detection of RRs is based on the required rights API.

Migration Strategy

• all documents are initialized with requiredRights equals false
• in a first time, the strategy is set to legacy
• all documents of XS are move to requiredRights and missing required rights are added
• For extensions:
  • option 1: migrate to 15.x+ and define the requiredRights
  • option 2: propose a way to define requiredRights on older version?
  • option 3: auto-detection feature: each imported file would be executed with minimal rights and scanned for XObject requiring PR/Script rights (e.g., stylesheets). If no error is raised, the document is saved with requiredRights set to true and an empty list of RRs, otherwise the user is asked to fix the missing RRs

Decreasing the rights

An interesting situation is the one where a user with not RRs but with edit rights wishes to edit a page nonetheless.

Should it be possible:

• All the time. In this case the RRs would be removed according to the rights of the editor
• A new checkbox is added to the RRs configuration form, allowing users to decrease the rights only if allowed
• never

TODO

• xar import/export