Skip to content

XWIKI-22656: Add a UI for setting required rights on a document #4158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
May 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
caa90c8
XWIKI-20907: Introduce the notion of required rights
michitux Nov 13, 2024
68ff5b8
XWIKI-20907: Introduce the notion of required rights
michitux Nov 13, 2024
74b8343
XWIKI-20907: Introduce the notion of required rights
michitux Nov 13, 2024
f148b7d
XWIKI-20907: Introduce the notion of required rights
michitux Jan 21, 2025
1d2b73e
XWIKI-22656: Add a UI for setting required rights on a document
michitux Feb 13, 2025
266aefd
XWIKI-22656: Add a UI for setting required rights on a document
michitux Feb 13, 2025
6a6ca11
XWIKI-22656: Add a UI for setting required rights on a document
michitux Feb 13, 2025
f74ef58
XWIKI-22656: Add a UI for setting required rights on a document
michitux Mar 13, 2025
34f6cfa
XWIKI-22656: Add a UI for setting required rights on a document
michitux Apr 8, 2025
f9ac64d
XWIKI-22656: Add a UI for setting required rights on a document
michitux Apr 10, 2025
b1a1c85
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 6, 2025
93442d0
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 6, 2025
13a2175
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 7, 2025
6859771
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 7, 2025
6898691
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 7, 2025
b3adcbd
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 12, 2025
305a764
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 13, 2025
0a84852
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 13, 2025
dbf4dea
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 14, 2025
4d41f39
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 15, 2025
b5f13dc
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 15, 2025
b0aed69
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 15, 2025
a8e8d75
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 15, 2025
b7bd889
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 15, 2025
bde9ff6
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 15, 2025
3abe08a
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 15, 2025
391b450
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 15, 2025
1ffc809
XWIKI-22656: Add a UI for setting required rights on a document
michitux May 15, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
(function () {
'use strict';
const $ = jQuery;

// Reload the content of the CKEditor instances when the document rights are changed to reflect the new rights
// in the executed macros.
$(document).on('xwiki:document:requiredRightsUpdated.ckeditor', function (event, data) {
$.each(CKEDITOR.instances, function(key, editor) {
if (matchesRequiredRightsChangeEvent(editor, event, data)) {
maybeReload(editor);
}
});
});

const matchesRequiredRightsChangeEvent = function (editor, event, data) {
// Check if the syntax change event targets the edited document (the source document).
return editor.config.sourceDocument.documentReference.equals(data.documentReference) &&
// Check if the syntax plugin is enabled for this editor instance.
editor.plugins['xwiki-rights'];
};

const maybeReload = function (editor) {
// Only reload in WYSIWYG mode. In source mode, rights aren't influencing anything.
// TODO: it would be nice if we could mark something in source mode to ensure that on the next switch to
// wysiwyg mode, the content would be reloaded regardless if it has been modified or not.
if (editor.mode === 'wysiwyg') {
editor.execCommand('xwiki-refresh');
}
};

// An empty plugin that can be used to enable / disable the reloading on required rights changes on a particular
// CKEditor instance.
CKEDITOR.plugins.add('xwiki-rights', {
requires: 'xwiki-macro,xwiki-source'
});
})();
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ CKEDITOR.editorConfig = function(config) {
'xwiki-maximize',
'xwiki-office',
'xwiki-realtime',
'xwiki-rights',
'xwiki-save',
'xwiki-selection',
'xwiki-slash',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@
<version>${project.version}</version>
<type>xar</type>
</dependency>
<!-- Used to test the integration between required rights and the editor. -->
<dependency>
<groupId>org.xwiki.platform</groupId>
<artifactId>xwiki-platform-security-requiredrights-ui</artifactId>
<version>${project.version}</version>
<scope>runtime</scope>
</dependency>
<!-- ================================
Test only dependencies
================================ -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@
import org.xwiki.test.docker.junit5.TestReference;
import org.xwiki.test.docker.junit5.UITest;
import org.xwiki.test.ui.TestUtils;
import org.xwiki.test.ui.po.InformationPane;
import org.xwiki.test.ui.po.RequiredRightsModal;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
Expand Down Expand Up @@ -439,4 +443,59 @@ void selectionRestoreOnSwitchToSource(TestUtils setup, TestReference testReferen

viewPage.cancel();
}

@Test
@Order(8)
void refreshOnRequiredRightsChange(TestUtils setup, TestReference testReference)
{
// Test that updating required rights refreshes the content.

// Grant alice script right on this page to allow using the Velocity macro.
setup.loginAsSuperAdmin();
setup.setRights(testReference, null, "XWiki.alice", "script", true);
setup.loginAndGotoPage("alice", "pa$$word", setup.getURL(testReference));

// Enter in-place edit mode.
InplaceEditablePage viewPage = new InplaceEditablePage().editInplace();
CKEditor ckeditor = new CKEditor("content");
RichTextAreaElement richTextArea = ckeditor.getRichTextArea();
richTextArea.clear();

// Insert the Velocity macro. The macro placeholder should be displayed.
richTextArea.sendKeys(Keys.ENTER, "/velocity");
AutocompleteDropdown qa = new AutocompleteDropdown();
qa.waitForItemSelected("/velocity", "Velocity");
richTextArea.sendKeys(Keys.ENTER);
qa.waitForItemSubmitted();

richTextArea.waitForContentRefresh();

assertEquals("macro:velocity", richTextArea.getText());

InformationPane informationPane = viewPage.openInformationDocExtraPane();

RequiredRightsModal requiredRightsModal = informationPane.openRequiredRightsModal();
requiredRightsModal.setEnforceRequiredRights(true);
requiredRightsModal.clickSave(true);

richTextArea.waitForContentRefresh();

assertThat(richTextArea.getText(), startsWith("Failed to execute the [velocity] macro."));

viewPage.save();

assertTrue(viewPage.hasRequiredRightsWarning(true));

requiredRightsModal = viewPage.openRequiredRightsModal();
requiredRightsModal.setEnforcedRequiredRight("script");
requiredRightsModal.clickSave(true);

richTextArea.waitForContentRefresh();

assertEquals("macro:velocity", richTextArea.getText());

setup.getDriver().waitUntilCondition(driver -> !viewPage.hasRequiredRightsWarning(false));

viewPage.cancel();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.xwiki.model.internal.document.SafeDocumentAuthors;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.DocumentReferenceResolver;
import org.xwiki.model.reference.EntityReference;
import org.xwiki.model.reference.EntityReferenceSerializer;
import org.xwiki.model.reference.ObjectReference;
import org.xwiki.model.reference.PageReference;
Expand Down Expand Up @@ -1169,6 +1170,24 @@ public Object newObject(String classname) throws XWikiException
return getObject(classname, nb);
}

/**
* Creates a new XObject from the given class reference.
*
* @param classReference the reference to the class of the XObject to be created
* @return the object created
* @throws XWikiException if an error occurs while creating the XObject
* @since 17.4.0RC1
*/
@Unstable
public Object newObject(EntityReference classReference) throws XWikiException
{
int index = getDoc().createXObject(classReference, getXWikiContext());

updateAuthor();

return getObject(classReference, index);
}

/**
* @return true of the document has been loaded from cache
*/
Expand Down Expand Up @@ -1229,6 +1248,21 @@ public Vector<Object> getObjects(String className)
return getXObjects(objects);
}

/**
* Retrieves and returns all objects corresponding to the class reference corresponding to the resolution of the
* given entity reference, or an empty list if there are none.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would specify in the javadoc that the list might contain null...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually cannot contain null elements as they are filtered out by getXObjects(objects). It's the same as for the method taking a String. If we want to change this, I think we should give the new method a new name in order to avoid that is accidentally called (some scripts might already pass an entity reference that is currently implicitly converted to string).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no it's good then, I missed the filter and I was being extra cautious...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess in the future we should deprecate the API you replaced that are taking a String instead of a reference. But maybe not now...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But maybe not now...

Yeah, as usually with this kind of API, there is a lot of work before we can but the @Dreprecated annotation (if we don't want to explode the log). Now what we do sometimes is put at least the javadoc @deprecated (which does not have any log impact).

*
* @param classReference the reference that is resolved to an XClass for retrieving the corresponding xobjects
* @return a list of xobjects corresponding to the given XClass or an empty list.
* @since 17.4.0RC1
*/
@Unstable
public List<Object> getObjects(EntityReference classReference)
{
List<BaseObject> objects = this.getDoc().getXObjects(classReference);
return getXObjects(objects);
}

/**
* Get the first object that contains the given fieldname
*
Expand Down Expand Up @@ -1387,6 +1421,29 @@ public Object getObject(String classname, int nb)
}
}

/**
* Gets the object matching the given class reference and given object number.
*
* @param classReference the reference of the class of the object
* @param nb the number of the object
* @return the XWiki Object
* @since 17.4.0RC1
*/
@Unstable
public Object getObject(EntityReference classReference, int nb)
{
try {
BaseObject obj = this.getDoc().getXObject(classReference, nb);
if (obj == null) {
return null;
} else {
return newObjectApi(obj, getXWikiContext());
}
} catch (Exception e) {
return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no log or anything?

Copy link
Contributor Author

@michitux michitux May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this behavior from the other methods that take a String instead of an EntityReference. Now I noticed that actually none of the called methods declare any exception, so we could also just remove it. It might be a breaking change, though, in case a script already calls this method with an EntityReference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well if no called method declare it I'm not sure how it could be a breaking change? I think I would clean up the code to remove the catch, and maybe add a log error to the original method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could catch non-declared exceptions, that would afaik stop the Velocity script when not catched anymore.

Copy link
Member

@tmortagne tmortagne May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it would be much cleaner to remove the catch. Unfortunately, it indeed might not look that much like a new method from Velocity code point of view, so better keep the same behavior.

An alternative would be to name all those new methods getXObject(s), which is closer to XWikiDocument naming. But it's also probably less obvious, and I like the idea that your version will improve performances a bit of any Velocity code which happen to call those methods with EntityReference already.

}
}

/**
* @param objectReference the object reference
* @return the XWiki object from this document that matches the specified object reference
Expand Down Expand Up @@ -1882,7 +1939,7 @@ public Attachment getAttachment(String filename)

/**
* @param filename the name of the attachment
* @return the attachment with the given filename or null if the attachment does not exist
* @return the attachment with the given filename or null if the attachment doesn’t exist
* @since 17.2.0RC1
*/
public Attachment removeAttachment(String filename)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.slf4j.Logger;
import org.xwiki.component.annotation.Component;
import org.xwiki.model.EntityType;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.EntityReference;
import org.xwiki.model.reference.LocalDocumentReference;
import org.xwiki.security.authorization.Right;
Expand Down Expand Up @@ -115,17 +116,24 @@ public DocumentRequiredRights readRequiredRights(XWikiDocument document)
return new DocumentRequiredRights(enforce, rights);
}

private DocumentRequiredRight readRequiredRight(BaseObject object)
/**
* Read the required right from an XObject.
*
* @param object the XObject to read the required right from. Must not be {@code null}
* @return the required right. Can be an illegal right if the value of the property is not a valid right
* @since 17.4.0RC1
*/
public DocumentRequiredRight readRequiredRight(BaseObject object)
{
String value = object.getStringValue(PROPERTY_NAME);
EntityType entityType = EntityType.DOCUMENT;
EntityType initialEntityType = EntityType.DOCUMENT;
Right right = Right.toRight(value);
if (right.equals(Right.ILLEGAL)) {
String[] levelRight = StringUtils.split(value, "_", 2);
if (levelRight.length == 2) {
right = Right.toRight(levelRight[1]);
try {
entityType = EntityType.valueOf(levelRight[0].toUpperCase());
initialEntityType = EntityType.valueOf(levelRight[0].toUpperCase());
} catch (IllegalArgumentException e) {
// Ensure that we return an illegal right even if the right part of the value could be parsed.
right = Right.ILLEGAL;
Expand All @@ -134,15 +142,35 @@ private DocumentRequiredRight readRequiredRight(BaseObject object)
}
}

EntityType entityType = getEffectiveEntityType(right, initialEntityType, object.getDocumentReference());

return new DocumentRequiredRight(right, entityType);
}

/**
* Determines the most specific effective {@link EntityType} based on the specified parameters. It adjusts
* the entity type according to the rights' targeted entity types and the provided base document reference.
*
* @param right the {@link Right} being analyzed; it defines the targeted entity types to consider
* @param initialEntityType the initial {@link EntityType} to be evaluated
* @param baseDocumentReference the base {@link DocumentReference} used to adjust and validate the entity type
* @return the most specific {@link EntityType} that is targeted by the {@link Right} and the same level or above
* the given initial entity type in the hierarchy of the document reference. If no suitable entity type is found,
* it defaults to {@link EntityType#DOCUMENT}, or null if the right targets only the farm level
*/
public EntityType getEffectiveEntityType(Right right, EntityType initialEntityType,
DocumentReference baseDocumentReference)
{
EntityType entityType = initialEntityType;
Set<EntityType> targetedEntityTypes = right.getTargetedEntityType();
if (targetedEntityTypes == null) {
// This means the right targets only the farm level, which is null.
entityType = null;
} else {
EntityReference entityReference = object.getDocumentReference().extractReference(entityType);
EntityReference entityReference = baseDocumentReference.extractReference(entityType);
// The specified entity type seems to be below the document level. Fall back to document level instead.
if (entityReference == null) {
entityReference = object.getDocumentReference();
entityReference = baseDocumentReference;
}
// Try to get the lowest level where this right can be assigned. This is done to ensure that, e.g.,
// programming right can imply admin right on the wiki level even if programming right is only specified on
Expand All @@ -157,7 +185,6 @@ private DocumentRequiredRight readRequiredRight(BaseObject object)
entityType = EntityType.DOCUMENT;
}
}

return new DocumentRequiredRight(right, entityType);
return entityType;
}
}
1 change: 1 addition & 0 deletions xwiki-platform-core/xwiki-platform-security/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,6 @@
<module>xwiki-platform-security-authentication</module>
<module>xwiki-platform-security-authorization</module>
<module>xwiki-platform-security-requiredrights</module>
<module>xwiki-platform-security-test</module>
</modules>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
<module>xwiki-platform-security-requiredrights-api</module>
<module>xwiki-platform-security-requiredrights-default</module>
<module>xwiki-platform-security-requiredrights-macro</module>
<module>xwiki-platform-security-requiredrights-rest</module>
<module>xwiki-platform-security-requiredrights-ui</module>
</modules>
<profiles>
<profile>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.xwiki.platform.security.requiredrights.internal;

import org.xwiki.security.authorization.requiredrights.DocumentRequiredRight;

/**
* A suggestion for an operation that removes or adds rights to a document.
*
* @param increasesRights if more rights are granted due to this change
* @param rightToRemove the right to replace
* @param rightToAdd the right to add
* @param requiresManualReview if the analysis is certain that the right is needed/not needed or the user needs to
* @param hasPermission if the current user has the permission to perform the proposed change manually review the
* analysis result to determine if the right is actually needed/not needed
*
* @version $Id$
* @since 17.4.0RC1
*/
public record RequiredRightChangeSuggestion(boolean increasesRights, DocumentRequiredRight rightToRemove,
DocumentRequiredRight rightToAdd, boolean requiresManualReview,
boolean hasPermission)
{
}
Loading
Loading