Skip to content
Snippets Groups Projects
Commit fa1c0fb1 authored by Marius Dumitru Florea's avatar Marius Dumitru Florea
Browse files

XWIKI-21949: Restrict the execution of script macros during a realtime WYSIWYG editing session 

* Fix a bug in EntityChannelScriptAuthorTracker which didn't lower / update the script level when the target entity reference had initially a low script level (e.g. when two users edit a page whose last author didn't have script right, one of them could use the rights of the other to execute scripts)
* Assume that a request can submit data associated with multiple documents (or document translations) so don't try to determine the entity that is targeted by the request; simply compute the effective author by taking the most recent author with the least script rights
* Update since versions
parent 01542b3c
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 55 additions and 191 deletions
xwiki-platform-core/xwiki-platform-netflux/xwiki-platform-netflux-api/pom.xml
+ 1
− 6
View file @ fa1c0fb1
......@@ -34,7 +34,7 @@
<properties>
<!-- Name to display by the Extension Manager -->
<xwiki.extension.name>Netflux API</xwiki.extension.name>
<xwiki.jacoco.instructionRatio>0.89</xwiki.jacoco.instructionRatio>
<xwiki.jacoco.instructionRatio>0.91</xwiki.jacoco.instructionRatio>
</properties>
<dependencies>
<dependency>
......@@ -52,11 +52,6 @@
<artifactId>xwiki-platform-localization-api</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>xwiki-platform-bridge</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>xwiki-platform-container-api</artifactId>
......
xwiki-platform-core/xwiki-platform-netflux/xwiki-platform-netflux-api/src/main/java/org/xwiki/netflux/EntityChannelStore.java
+ 2
− 2
View file @ fa1c0fb1
......@@ -76,9 +76,9 @@ default Optional<EntityChannel> getChannel(EntityReference entityReference, List
*
* @param key the channel key
* @return the channel associated with the specified key, if any
* @since 15.10.11
* @since 15.10.12
* @since 16.4.1
* @since 16.5.0
* @since 16.6.0RC1
*/
@Unstable
default Optional<EntityChannel> getChannel(String key)
......
xwiki-platform-core/xwiki-platform-netflux/xwiki-platform-netflux-api/src/main/java/org/xwiki/netflux/internal/EffectiveAuthorSetterListener.java
+ 26
− 30
View file @ fa1c0fb1
......@@ -20,7 +20,6 @@
package org.xwiki.netflux.internal;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
......@@ -30,16 +29,11 @@
import javax.inject.Singleton;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.slf4j.Logger;
import org.xwiki.bridge.DocumentAccessBridge;
import org.xwiki.bridge.DocumentModelBridge;
import org.xwiki.bridge.event.ActionExecutingEvent;
import org.xwiki.component.annotation.Component;
import org.xwiki.container.Container;
import org.xwiki.container.Request;
import org.xwiki.localization.LocaleUtils;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.observation.event.AbstractLocalEventListener;
import org.xwiki.observation.event.Event;
import org.xwiki.user.UserReference;
......@@ -48,9 +42,9 @@
* Sets the effective author of the request to the effective author of the real-time session specified by the request.
*
* @version $Id$
* @since 15.10.11
* @since 15.10.12
* @since 16.4.1
* @since 16.5.0
* @since 16.6.0RC1
*/
@Component
@Singleton
......@@ -68,9 +62,6 @@ public class EffectiveAuthorSetterListener extends AbstractLocalEventListener
@Inject
private EntityChannelScriptAuthorTracker scriptAuthorTracker;
@Inject
private DocumentAccessBridge documentAccessBridge;
@Inject
private Container container;
......@@ -92,29 +83,34 @@ public void processLocalEvent(Event event, Object source, Object data)
});
}
/**
* If the request has content that was synchronized through Netflux channels (in a real-time session), then the
* author of that content is not necessarily the current user. For each Netflux channel, that is used to synchronize
* content that may contain scripts, we keep track on the last author with the least script rights. We consider the
* request effective author to be the script author with the least script rights among all script authors of the
* Netflux channels that have contributed content to this request.
* <p>
* Note that the Netflux channels specified on the request could be associated with different XWiki documents (or
* translations). We don't filter the channels that are associated with the current document (targeted by this
* request) because we want the effective author to be responsible for the entire content submitted by this request
* (not just the content of the current document).
*
* @param request the request for which to determine the effective author
* @return the user that is responsible in terms of access rights for the <strong>entire</strong> content submitted
* by this request and the effects it has on the server-side
*/
private Optional<UserReference> getEffectiveAuthor(Request request)
{
try {
DocumentReference documentReference = this.documentAccessBridge.getCurrentDocumentReference();
DocumentModelBridge translatedDocument =
this.documentAccessBridge.getTranslatedDocumentInstance(documentReference);
Locale realLocale = LocaleUtils.toLocale(translatedDocument.getRealLanguage());
DocumentReference documentReferenceWithRealLocale = new DocumentReference(documentReference, realLocale);
return getChannelsFromRequest(request).stream()
.map(channel -> this.scriptAuthorTracker.getScriptAuthor(channel)).filter(Optional::isPresent)
.map(Optional::get)
.filter(
entityChange -> Objects.equals(entityChange.getEntityReference(), documentReferenceWithRealLocale)
|| entityChange.getEntityReference().hasParent(documentReference))
.sorted().map(EntityChange::getAuthor).findFirst();
} catch (Exception e) {
this.logger.warn("Failed to determine the effective request author. Root cause is [{}].",
ExceptionUtils.getRootCauseMessage(e));
return Optional.empty();
}
return getChannelsFromRequest(request).stream()
.map(channel -> this.scriptAuthorTracker.getScriptAuthor(channel)).filter(Optional::isPresent)
.map(Optional::get).sorted().map(EntityChange::getAuthor).findFirst();
}
/**
* @param request the request from which to extract the Netflux channels
* @return the Netflux channels that have contributed content to the request, as indicated by the
* {@code netfluxChannel} request parameter
*/
private List<String> getChannelsFromRequest(Request request)
{
List<String> channels = request.getProperties("netfluxChannel").stream()
......
xwiki-platform-core/xwiki-platform-netflux/xwiki-platform-netflux-api/src/main/java/org/xwiki/netflux/internal/EntityChange.java
+ 2
− 2
View file @ fa1c0fb1
......@@ -31,9 +31,9 @@
* Represents a change to an entity.
*
* @version $Id$
* @since 15.10.11
* @since 15.10.12
* @since 16.4.1
* @since 16.5.0
* @since 16.6.0RC1
*/
public class EntityChange implements Comparable<EntityChange>
{
......
xwiki-platform-core/xwiki-platform-netflux/xwiki-platform-netflux-api/src/main/java/org/xwiki/netflux/internal/EntityChannelScriptAuthorBot.java
+ 2
− 2
View file @ fa1c0fb1
......@@ -41,9 +41,9 @@
* changes to the channel).
*
* @version $Id$
* @since 15.10.11
* @since 15.10.12
* @since 16.4.1
* @since 16.5.0
* @since 16.6.0RC1
*/
@Component
@Singleton
......
xwiki-platform-core/xwiki-platform-netflux/xwiki-platform-netflux-api/src/main/java/org/xwiki/netflux/internal/EntityChannelScriptAuthorTracker.java
+ 5
− 75
View file @ fa1c0fb1
......@@ -19,8 +19,6 @@
*/
package org.xwiki.netflux.internal;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
......@@ -29,17 +27,11 @@
import javax.inject.Named;
import javax.inject.Singleton;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.slf4j.Logger;
import org.xwiki.bridge.DocumentAccessBridge;
import org.xwiki.bridge.DocumentModelBridge;
import org.xwiki.component.annotation.Component;
import org.xwiki.localization.LocaleUtils;
import org.xwiki.model.EntityType;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.EntityReference;
import org.xwiki.model.reference.EntityReferenceResolver;
import org.xwiki.model.reference.ObjectPropertyReference;
import org.xwiki.netflux.EntityChannel;
import org.xwiki.netflux.EntityChannelStore;
import org.xwiki.netflux.internal.EntityChange.ScriptLevel;
......@@ -55,9 +47,9 @@
* contain scripts.
*
* @version $Id$
* @since 15.10.11
* @since 15.10.12
* @since 16.4.1
* @since 16.5.0
* @since 16.6.0RC1
*/
@Component(roles = EntityChannelScriptAuthorTracker.class)
@Singleton
......@@ -72,9 +64,6 @@ public class EntityChannelScriptAuthorTracker
@Inject
private AuthorizationManager authorizationManager;
@Inject
private DocumentAccessBridge documentAccessBridge;
@Inject
@Named("document")
private UserReferenceSerializer<DocumentReference> documentUserReferenceSerializer;
......@@ -123,48 +112,19 @@ public Optional<EntityChange> getScriptAuthor(String channelId)
void maybeUpdateScriptAuthor(EntityChannel entityChannel, UserReference scriptAuthor)
{
EntityReference targetEntityReference = getTargetEntityReference(entityChannel);
EntityChange channelScriptAuthor =
this.scriptAuthors.getOrDefault(entityChannel.getKey(), getEntityScriptAuthor(targetEntityReference));
EntityChange channelScriptAuthor = this.scriptAuthors.get(entityChannel.getKey());
ScriptLevel userScriptLevel = getUserScriptLevel(scriptAuthor, entityChannel.getEntityReference());
if (channelScriptAuthor == null || userScriptLevel.compareTo(channelScriptAuthor.getScriptLevel()) <= 0) {
// The user making the change has a lower or equal script level than the channel script level. We
// need to update the channel script level in order to prevent privilege escalation.
EntityChange scriptAuthorChange = new EntityChange(targetEntityReference, scriptAuthor, userScriptLevel);
EntityChange scriptAuthorChange =
new EntityChange(entityChannel.getEntityReference(), scriptAuthor, userScriptLevel);
this.scriptAuthors.put(entityChannel.getKey(), scriptAuthorChange);
this.logger.debug("Updated the script author associated with the entity channel [{}] to [{}].",
entityChannel, scriptAuthorChange);
}
}
private EntityChange getEntityScriptAuthor(EntityReference entityReference)
{
try {
if (entityReference instanceof DocumentReference) {
DocumentReference documentReference = (DocumentReference) entityReference;
DocumentModelBridge document =
this.documentAccessBridge.getTranslatedDocumentInstance(documentReference);
UserReference contentAuthor = document.getAuthors().getContentAuthor();
return new EntityChange(documentReference, contentAuthor,
getUserScriptLevel(contentAuthor, documentReference));
} else if (entityReference instanceof ObjectPropertyReference) {
ObjectPropertyReference objectPropertyReference = (ObjectPropertyReference) entityReference;
DocumentReference documentReference =
new DocumentReference(objectPropertyReference.extractReference(EntityType.DOCUMENT));
// Metadata is stored on the default document translation.
DocumentModelBridge document = this.documentAccessBridge.getDocumentInstance(documentReference);
UserReference metadataAuthor = document.getAuthors().getEffectiveMetadataAuthor();
return new EntityChange(documentReference, metadataAuthor,
getUserScriptLevel(metadataAuthor, documentReference));
}
} catch (Exception e) {
this.logger.warn("Failed to compute the script level for entity [{}]. Root cause is [{}].", entityReference,
ExceptionUtils.getRootCauseMessage(e));
}
return null;
}
private ScriptLevel getUserScriptLevel(UserReference userReference, EntityReference entityReference)
{
DocumentReference documentUserReference = this.documentUserReferenceSerializer.serialize(userReference);
......@@ -176,34 +136,4 @@ private ScriptLevel getUserScriptLevel(UserReference userReference, EntityRefere
return ScriptLevel.NO_SCRIPT;
}
}
private EntityReference getTargetEntityReference(EntityChannel entityChannel)
{
EntityReference targetEntityReference = entityChannel.getEntityReference();
List<String> path = entityChannel.getPath();
if (targetEntityReference.getType() == EntityType.DOCUMENT && !path.isEmpty()) {
try {
Locale locale = LocaleUtils.toLocale(path.get(0));
if (path.size() > 1) {
// The path indicates the document property that is synchronized.
String property = path.get(1);
if (!"content".equals(property)) {
// The entity channel is used to synchronize document metadata, which is shared by all document
// translations (stored on the default translation).
return new ObjectPropertyReference(this.explicitEntityReferenceResolver.resolve(property,
EntityType.OBJECT_PROPERTY, targetEntityReference));
}
}
// The entity channel is used to synchronize the value of a document property that is not shared between
// document translations. We need to take the script author from the specified document translation.
return new DocumentReference(targetEntityReference.extractReference(EntityType.DOCUMENT), locale);
} catch (Exception e) {
// The document reference is probably relative but we can't resolve it because there's no currrent
// document in the context where this method is called.
this.logger.warn("Failed to compute the target entity reference"
+ " associated with the entity channel [{}] with path [{}].", entityChannel.getKey(), path);
}
}
return targetEntityReference;
}
}
xwiki-platform-core/xwiki-platform-netflux/xwiki-platform-netflux-api/src/test/java/org/xwiki/netflux/internal/EffectiveAuthorSetterListenerTest.java
+ 16
− 20
View file @ fa1c0fb1
......@@ -30,8 +30,6 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.xwiki.bridge.DocumentAccessBridge;
import org.xwiki.bridge.DocumentModelBridge;
import org.xwiki.bridge.event.ActionExecutingEvent;
import org.xwiki.container.Container;
import org.xwiki.container.Request;
......@@ -57,9 +55,6 @@ class EffectiveAuthorSetterListenerTest
@MockComponent
private EntityChannelScriptAuthorTracker scriptAuthorTracker;
@MockComponent
private DocumentAccessBridge documentAccessBridge;
@MockComponent
private Container container;
......@@ -78,30 +73,31 @@ void beforeEach()
@Test
void onActionExecutingEvent() throws Exception
{
DocumentReference currentDocumentReference = new DocumentReference("test", "Some", "Page");
when(this.documentAccessBridge.getCurrentDocumentReference()).thenReturn(currentDocumentReference);
DocumentModelBridge tdoc = mock(DocumentModelBridge.class);
when(this.documentAccessBridge.getTranslatedDocumentInstance(currentDocumentReference)).thenReturn(tdoc);
when(tdoc.getRealLanguage()).thenReturn("fr");
DocumentReference documentReference = new DocumentReference("test", "Some", "Page");
UserReference otherUserReference = mock(UserReference.class);
// An entity change that targets a different document.
// An entity change that targets some document, with an older timestamp.
EntityChange entityChangeTwo =
new EntityChange(new DocumentReference("test", "Other", "Page"), otherUserReference, ScriptLevel.SCRIPT);
// An entity change that targets the current document translation, but with a higher script level.
EntityChange entityChangeThree =
new EntityChange(new DocumentReference(currentDocumentReference, Locale.FRENCH), otherUserReference,
ScriptLevel.PROGRAMMING);
// An entity change that targets the current document (access rights are checked at document level) with a lower
// script level. The lower script level should win.
EntityChange entityChangeFour = new EntityChange(new AttachmentReference("file.txt", currentDocumentReference),
// Wait a bit to be sure that the next entity changes have a newer timestamp.
Thread.sleep(10);
// An entity change that targets a document translation, with a higher script level.
EntityChange entityChangeThree = new EntityChange(new DocumentReference(documentReference, Locale.FRENCH),
otherUserReference, ScriptLevel.PROGRAMMING);
// An entity change that targets a document attachment, with a lower script level (same as the first change, but
// with a more recent timestamp). The lower script level with the recent timestamp should win.
EntityChange entityChangeFour = new EntityChange(new AttachmentReference("file.txt", documentReference),
this.effectiveAuthor, ScriptLevel.SCRIPT);
// Wait a bit to be sure that the next entity changes have a newer timestamp.
Thread.sleep(10);
EntityChange entityChangeFive = new EntityChange(new DocumentReference("test", "Other", "Page"),
otherUserReference, ScriptLevel.PROGRAMMING);
when(this.request.getProperties("netfluxChannel"))
.thenReturn(Arrays.asList("", "one", null, "two", "three", "four"));
.thenReturn(Arrays.asList("", "one", null, "two", "three", "four", "five"));
when(this.scriptAuthorTracker.getScriptAuthor("two")).thenReturn(Optional.of(entityChangeTwo));
when(this.scriptAuthorTracker.getScriptAuthor("three")).thenReturn(Optional.of(entityChangeThree));
when(this.scriptAuthorTracker.getScriptAuthor("four")).thenReturn(Optional.of(entityChangeFour));
when(this.scriptAuthorTracker.getScriptAuthor("five")).thenReturn(Optional.of(entityChangeFive));
this.listener.onEvent(new ActionExecutingEvent(), null, null);
......
xwiki-platform-core/xwiki-platform-netflux/xwiki-platform-netflux-api/src/test/java/org/xwiki/netflux/internal/EntityChannelScriptAuthorTrackerTest.java
+ 1
− 54
View file @ fa1c0fb1
......@@ -32,10 +32,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.xwiki.bridge.DocumentAccessBridge;
import org.xwiki.bridge.DocumentModelBridge;
import org.xwiki.model.EntityType;
import org.xwiki.model.document.DocumentAuthors;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.EntityReferenceResolver;
import org.xwiki.model.reference.ObjectPropertyReference;
......@@ -69,9 +66,6 @@ class EntityChannelScriptAuthorTrackerTest
@MockComponent
private AuthorizationManager authorizationManager;
@MockComponent
private DocumentAccessBridge documentAccessBridge;
@MockComponent
@Named("document")
private UserReferenceSerializer<DocumentReference> documentUserReferenceSerializer;
......@@ -99,19 +93,9 @@ class EntityChannelScriptAuthorTrackerTest
private ObjectPropertyReference objectPropertyReference =
new ObjectPropertyReference("text", new ObjectReference("Some.Class[0]", this.documentReference));
@Mock
private DocumentModelBridge document;
@Mock
DocumentAuthors documentAuthors;
@BeforeEach
void beforeEach()
{
when(this.document.getAuthors()).thenReturn(this.documentAuthors);
when(this.documentAuthors.getContentAuthor()).thenReturn(this.carol);
when(this.documentAuthors.getEffectiveMetadataAuthor()).thenReturn(this.carol);
// Alice has only script rights.
DocumentReference aliceReference = new DocumentReference("xwiki", "Users", "Alice");
when(this.documentUserReferenceSerializer.serialize(this.alice)).thenReturn(aliceReference);
......@@ -143,7 +127,7 @@ void beforeEach()
}
@Test
void getScriptAuthorForDocumentContent() throws Exception
void getScriptAuthor()
{
// No messages received yet on this channel.
assertFalse(this.tracker.getScriptAuthor("channelKey").isPresent());
......@@ -151,43 +135,6 @@ void getScriptAuthorForDocumentContent() throws Exception
EntityChannel entityChannel =
new EntityChannel(documentReference, List.of("fr", "content", "wiki"), "channelKey");
when(this.entityChannels.getChannel(entityChannel.getKey())).thenReturn(Optional.of(entityChannel));
when(this.documentAccessBridge.getTranslatedDocumentInstance(this.translationReference))
.thenReturn(this.document);
// Alice sends a message on the channel.
this.tracker.maybeUpdateScriptAuthor(entityChannel, alice);
assertEquals(alice, this.tracker.getScriptAuthor("channelKey").get().getAuthor());
// Bob sends a message on the channel. Bob has less script rights than Alice.
this.tracker.maybeUpdateScriptAuthor(entityChannel, bob);
assertEquals(bob, this.tracker.getScriptAuthor("channelKey").get().getAuthor());
// Alice sends another message on the channel, but it should not change the script author because Alice has more
// script rights than Bob (we can only lower the script level).
this.tracker.maybeUpdateScriptAuthor(entityChannel, alice);
// Close the entity channel.
when(this.entityChannels.getChannel(entityChannel.getKey())).thenReturn(Optional.empty());
// We can still get the script author once.
assertEquals(bob, this.tracker.getScriptAuthor("channelKey").get().getAuthor());
// The script author was reset.
assertFalse(this.tracker.getScriptAuthor("channelKey").isPresent());
}
@Test
void getScriptAuthorForDocumentMetaData() throws Exception
{
// No messages received yet on this channel.
assertFalse(this.tracker.getScriptAuthor("channelKey").isPresent());
EntityChannel entityChannel =
new EntityChannel(documentReference, List.of("en", "Some.Class[0].text", "wysiwyg"), "channelKey");
when(this.entityChannels.getChannel(entityChannel.getKey())).thenReturn(Optional.of(entityChannel));
when(this.documentAccessBridge.getDocumentInstance(this.documentReference)).thenReturn(this.document);
// Alice sends a message on the channel.
this.tracker.maybeUpdateScriptAuthor(entityChannel, alice);
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment