XWiki Platform remote code execution from account through UIExtension parameters
Description
XWiki Platform is a generic wiki platform. Prior to versions 4.10.19, 15.5.4, and 15.10-rc-1, parameters of UI extensions are always interpreted as Velocity code and executed with programming rights. Any user with edit right on any document like the user's own profile can create UI extensions. This allows remote code execution and thereby impacts the confidentiality, integrity and availability of the whole XWiki installation. This vulnerability has been patched in XWiki 14.10.19, 15.5.4 and 15.9-RC1. No known workarounds are available.
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
org.xwiki.platform:xwiki-platform-uiextension-apiMaven | < 14.10.19 | 14.10.19 |
org.xwiki.platform:xwiki-platform-uiextension-apiMaven | >= 15.0-rc-1, < 15.5.4 | 15.5.4 |
org.xwiki.platform:xwiki-platform-uiextension-apiMaven | >= 15.6-rc-1, < 15.9-rc-1 | 15.9-rc-1 |
Affected products
1- Range: < 14.10.19
Patches
356748e154a90XWIKI-21335: Execute UI Extension parameters with the rights of their author
4 files changed · +170 −52
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/main/java/org/xwiki/uiextension/internal/WikiUIExtensionComponentBuilder.java+1 −4 modified@@ -143,12 +143,9 @@ public List<WikiComponent> buildComponents(BaseObject baseObject) throws WikiCom String.format("Failed to initialize Panel UI extension [%s]", baseObject.getReference()), e); } - String rawParameters = baseObject.getStringValue(PARAMETERS_PROPERTY); - // It would be nice to have PER_LOOKUP components for UIX parameters but without constructor injection it's // safer to use a POJO and pass the Component Manager to it. - WikiUIExtensionParameters parameters = - new WikiUIExtensionParameters(id, rawParameters, this.wikiComponentManager); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(baseObject, this.wikiComponentManager); extension.setParameters(parameters); extension.setScope(scope);
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/main/java/org/xwiki/uiextension/internal/WikiUIExtensionParameters.java+55 −26 modified@@ -22,9 +22,10 @@ import java.io.IOException; import java.io.StringReader; import java.io.StringWriter; -import java.util.HashMap; import java.util.Map; import java.util.Properties; +import java.util.function.Function; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.exception.ExceptionUtils; @@ -38,11 +39,20 @@ import org.xwiki.logging.LoggerConfiguration; import org.xwiki.model.EntityType; import org.xwiki.model.ModelContext; +import org.xwiki.model.reference.DocumentReference; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; import org.xwiki.velocity.VelocityEngine; import org.xwiki.velocity.VelocityManager; import org.xwiki.velocity.XWikiVelocityContext; import org.xwiki.velocity.XWikiVelocityException; +import com.xpn.xwiki.objects.BaseObject; + +import static org.xwiki.uiextension.internal.WikiUIExtensionConstants.ID_PROPERTY; +import static org.xwiki.uiextension.internal.WikiUIExtensionConstants.PARAMETERS_PROPERTY; + /** * Wiki UI Extension parameter manager. * @@ -97,25 +107,37 @@ public class WikiUIExtensionParameters */ private Execution execution; + private AuthorExecutor authorExecutor; + + private final DocumentReference documentReference; + + private final DocumentReference authorReference; + + private final AuthorizationManager authorizationManager; + /** * Default constructor. * - * @param id the unique identifier of this set of parameters, mostly used to isolate parameters value execution - * @param rawParameters raw parameters, their values can contain velocity directives + * @param baseObject the object from which the parameters shall be loaded * @param cm the XWiki component manager * @throws WikiComponentException if some required components can't be found in the Component Manager */ - public WikiUIExtensionParameters(String id, String rawParameters, ComponentManager cm) + public WikiUIExtensionParameters(BaseObject baseObject, ComponentManager cm) throws WikiComponentException { - this.id = id; - this.parameters = parseParameters(rawParameters); + this.id = baseObject.getStringValue(ID_PROPERTY); + this.parameters = parseParameters(baseObject.getStringValue(PARAMETERS_PROPERTY)); + + this.documentReference = baseObject.getDocumentReference(); + this.authorReference = baseObject.getOwnerDocument().getAuthorReference(); try { this.execution = cm.getInstance(Execution.class); this.velocityManager = cm.getInstance(VelocityManager.class); this.modelContext = cm.getInstance(ModelContext.class); this.loggerConfiguration = cm.getInstance(LoggerConfiguration.class); + this.authorExecutor = cm.getInstance(AuthorExecutor.class); + this.authorizationManager = cm.getInstance(AuthorizationManager.class); } catch (ComponentLookupException e) { throw new WikiComponentException( "Failed to get an instance for a component role required by Wiki Components.", e); @@ -148,7 +170,7 @@ private Properties parseParameters(String rawParameters) */ public Map<String, String> get() { - boolean isCacheValid = false; + Map<String, String> result; // Even though the parameters are dynamic, we cache a rendered version of them in order to improve performance. // This cache has a short lifespan, it gets discarded for each new request, or if the database has been switched @@ -158,43 +180,50 @@ public Map<String, String> get() if (currentContextId == this.previousContextId && currentWiki.equals(previousWiki) && this.evaluatedParameters != null) { - isCacheValid = true; - } - - if (!isCacheValid) { - this.evaluatedParameters = new HashMap<>(); - - if (this.parameters.size() > 0) { + result = this.evaluatedParameters; + } else { + result = this.parameters.stringPropertyNames().stream() + .filter(StringUtils::isNotBlank) + .collect(Collectors.toMap(Function.identity(), this.parameters::getProperty)); + + if (!this.parameters.isEmpty() + && this.authorizationManager.hasAccess(Right.SCRIPT, this.authorReference, this.documentReference)) + { try { - VelocityEngine velocityEngine = this.velocityManager.getVelocityEngine(); - VelocityContext velocityContext = this.velocityManager.getVelocityContext(); + this.authorExecutor.call(() -> { + VelocityEngine velocityEngine = this.velocityManager.getVelocityEngine(); + VelocityContext velocityContext = this.velocityManager.getVelocityContext(); - for (String propertyKey : this.parameters.stringPropertyNames()) { - if (!StringUtils.isBlank(propertyKey)) { - String propertyValue = this.parameters.getProperty(propertyKey); + result.replaceAll((propertyKey, propertyValue) -> { StringWriter writer = new StringWriter(); try { String namespace = this.id + ':' + propertyKey; velocityEngine.evaluate( new XWikiVelocityContext(velocityContext, this.loggerConfiguration.isDeprecatedLogEnabled()), writer, namespace, propertyValue); - this.evaluatedParameters.put(propertyKey, writer.toString()); + return writer.toString(); } catch (XWikiVelocityException e) { LOGGER.warn(String.format( "Failed to evaluate UI extension data value, key [%s], value [%s]. Reason: [%s]", propertyKey, propertyValue, e.getMessage())); } - } - } - } catch (XWikiVelocityException ex) { + + return propertyValue; + }); + + return null; + }, this.authorReference, this.documentReference); + } catch (Exception ex) { LOGGER.warn(String.format("Failed to get velocity engine. Reason: [%s]", ex.getMessage())); } - this.previousContextId = currentContextId; - this.previousWiki = currentWiki; } + + this.evaluatedParameters = result; + this.previousContextId = currentContextId; + this.previousWiki = currentWiki; } - return this.evaluatedParameters; + return result; } }
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/test/java/org/xwiki/uiextension/WikiUIExtensionComponentBuilderTest.java+18 −0 modified@@ -23,6 +23,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.concurrent.Callable; import org.apache.velocity.VelocityContext; import org.junit.jupiter.api.Test; @@ -43,6 +44,9 @@ import org.xwiki.rendering.transformation.RenderingContext; import org.xwiki.rendering.transformation.Transformation; import org.xwiki.rendering.util.ErrorBlockGenerator; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; import org.xwiki.test.annotation.BeforeComponent; import org.xwiki.test.junit5.mockito.InjectMockComponents; import org.xwiki.test.junit5.mockito.MockComponent; @@ -90,6 +94,12 @@ public class WikiUIExtensionComponentBuilderTest implements WikiUIExtensionConst @MockComponent private LoggerConfiguration loggerConfiguration; + @MockComponent + private AuthorExecutor authorExecutor; + + @MockComponent + private AuthorizationManager authorizationManager; + @InjectMockComponents private WikiUIExtensionComponentBuilder builder; @@ -125,6 +135,13 @@ public void configure(MockitoComponentManager componentManager, MockitoOldcore o componentManager.registerMockComponent(Transformation.class, "macro"); componentManager.registerMockComponent(ContentParser.class); + when(this.authorExecutor.call(any(), any(), any())).thenAnswer(invocation -> { + Callable<?> callable = invocation.getArgument(0); + return callable.call(); + }); + + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOC_REF)).thenReturn(true); + // The document holding the UI extension object. this.componentDoc = mock(XWikiDocument.class, "xwiki:XWiki.MyUIExtension"); when(this.componentDoc.getDocumentReference()).thenReturn(DOC_REF); @@ -194,6 +211,7 @@ private BaseObject createExtensionObject(String id, String extensionPointId, Str BaseObjectReference objectReference = new BaseObjectReference(new ObjectReference("XWiki.UIExtensionClass[0]", DOC_REF)); when(extensionObject.getReference()).thenReturn(objectReference); + when(extensionObject.getDocumentReference()).thenReturn(DOC_REF); when(extensionObject.getOwnerDocument()).thenReturn(this.componentDoc);
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/test/java/org/xwiki/uiextension/WikiUIExtensionParametersTest.java+96 −22 modified@@ -20,6 +20,7 @@ package org.xwiki.uiextension; import java.io.StringWriter; +import java.util.concurrent.Callable; import org.apache.commons.collections.MapUtils; import org.apache.velocity.VelocityContext; @@ -31,25 +32,35 @@ import org.xwiki.context.ExecutionContext; import org.xwiki.logging.LoggerConfiguration; import org.xwiki.model.ModelContext; +import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.WikiReference; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; import org.xwiki.test.LogLevel; import org.xwiki.test.junit5.LogCaptureExtension; import org.xwiki.test.junit5.mockito.ComponentTest; import org.xwiki.test.junit5.mockito.InjectComponentManager; import org.xwiki.test.junit5.mockito.MockComponent; import org.xwiki.test.mockito.MockitoComponentManager; +import org.xwiki.uiextension.internal.WikiUIExtensionConstants; import org.xwiki.uiextension.internal.WikiUIExtensionParameters; import org.xwiki.velocity.VelocityEngine; import org.xwiki.velocity.VelocityManager; import org.xwiki.velocity.XWikiVelocityException; +import com.xpn.xwiki.doc.XWikiDocument; +import com.xpn.xwiki.objects.BaseObject; + import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; /** @@ -61,6 +72,10 @@ @ComponentTest class WikiUIExtensionParametersTest { + private static final DocumentReference AUTHOR_REFERENCE = new DocumentReference("xwiki", "XWiki", "XWikiAdmin"); + + private static final DocumentReference DOCUMENT_REFERENCE = new DocumentReference("xwiki", "Space", "UIX"); + @Mock private VelocityEngine velocityEngine; @@ -76,6 +91,12 @@ class WikiUIExtensionParametersTest @MockComponent private LoggerConfiguration loggerConfiguration; + @MockComponent + private AuthorExecutor authorExecutor; + + @MockComponent + private AuthorizationManager authorizationManager; + @RegisterExtension private final LogCaptureExtension logCapture = new LogCaptureExtension(LogLevel.WARN); @@ -91,24 +112,33 @@ public void setUp() throws Exception when(this.execution.getContext()).thenReturn(executionContext); when(this.velocityManager.getVelocityContext()).thenReturn(velocityContext); when(this.velocityManager.getVelocityEngine()).thenReturn(this.velocityEngine); + + when(this.authorExecutor.call(any(), any(), any())).thenAnswer(invocation -> { + Callable<?> callable = invocation.getArgument(0); + return callable.call(); + }); + + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE)).thenReturn(true); + + when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); } @Test void getParametersWithAnEmptyParametersProperty() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "", this.componentManager); + BaseObject mockUIX = constructMockUIXObject(""); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); assertEquals(MapUtils.EMPTY_MAP, parameters.get()); } @Test void getParametersWithAnEqualSignInAValue() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); // Since the StringWriter is created within the method, the value is "" and not "value". assertEquals("", parameters.get().get("key")); @@ -117,15 +147,13 @@ void getParametersWithAnEqualSignInAValue() throws Exception @Test void getParametersWithCommentAloneOnLine() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); - String paramsStr = "# a = 1\n" + "x=1\n" + "y=2\n" + "# ...\n" + "z=3"; - WikiUIExtensionParameters parameters = - new WikiUIExtensionParameters("id", paramsStr, this.componentManager); + BaseObject mockUIX = constructMockUIXObject(paramsStr); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); parameters.get(); verify(this.velocityEngine).evaluate(any(), any(), eq("id:x"), eq("1")); @@ -136,13 +164,11 @@ void getParametersWithCommentAloneOnLine() throws Exception @Test void getParametersWithCommentEndOfLine() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); - String paramsStr = "x=1##b\n" + "y=2####x\n" + "z=3 ## xyz\n"; - WikiUIExtensionParameters parameters = - new WikiUIExtensionParameters("id", paramsStr, this.componentManager); + BaseObject mockUIX = constructMockUIXObject(paramsStr); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); parameters.get(); verify(this.velocityEngine).evaluate(any(), any(), eq("id:x"), eq("1##b")); @@ -153,26 +179,26 @@ void getParametersWithCommentEndOfLine() throws Exception @Test void getParametersWhenVelocityFails() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenThrow(new XWikiVelocityException("")); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); - // It should fail and put a warn in the logs - assertNull(parameters.get().get("key")); + // It put a warning in the logs and return the not-evaluated value. + assertEquals("value", parameters.get().get("key")); assertEquals("Failed to evaluate UI extension data value, key [key], value [value]. Reason: []", this.logCapture.getMessage(0)); } @Test void getParametersFromTheSameRequestAndForTheSameWiki() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); // It should fail silently assertEquals("", parameters.get().get("key")); @@ -191,7 +217,8 @@ void getParametersFromTheSameRequestButForDifferentWikis() throws Exception when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); // It should fail silently assertEquals("", parameters.get().get("key")); @@ -205,11 +232,11 @@ void getParametersFromTheSameRequestButForDifferentWikis() throws Exception @Test void getParametersFromDifferentRequests() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("wiki1")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); ExecutionContext ec1 = mock(ExecutionContext.class, "ec1"); ExecutionContext ec2 = mock(ExecutionContext.class, "ec2"); @@ -223,4 +250,51 @@ void getParametersFromDifferentRequests() throws Exception verify(this.velocityEngine, times(2)) .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value")); } + + @Test + void getParametersWithoutScriptRight() throws Exception + { + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE)).thenReturn(false); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); + + assertEquals("value", parameters.get().get("key")); + verifyNoInteractions(this.velocityEngine); + } + + @Test + void getParametersWithAuthorExecutor() throws Exception + { + // Do not call the callable to check that the call to the Velocity engine is inside the author executor. + doReturn(null).when(this.authorExecutor).call(any(), any(), any()); + + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); + + assertEquals("value", parameters.get().get("key")); + verify(this.authorExecutor).call(any(), eq(AUTHOR_REFERENCE), eq(DOCUMENT_REFERENCE)); + verifyNoInteractions(this.velocityEngine); + } + + @Test + void getParametersWithEmptyKey() throws Exception + { + BaseObject mockUIX = constructMockUIXObject("=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); + + assertTrue(parameters.get().isEmpty()); + } + + private BaseObject constructMockUIXObject(String parameters) + { + BaseObject result = mock(); + + when(result.getStringValue(WikiUIExtensionConstants.ID_PROPERTY)).thenReturn("id"); + when(result.getStringValue(WikiUIExtensionConstants.PARAMETERS_PROPERTY)).thenReturn(parameters); + when(result.getOwnerDocument()).thenReturn(mock(XWikiDocument.class)); + when(result.getOwnerDocument().getAuthorReference()).thenReturn(WikiUIExtensionParametersTest.AUTHOR_REFERENCE); + when(result.getDocumentReference()).thenReturn(WikiUIExtensionParametersTest.DOCUMENT_REFERENCE); + + return result; + } }
171e7c7d0e56XWIKI-21335: Execute UI Extension parameters with the rights of their author
4 files changed · +170 −52
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/main/java/org/xwiki/uiextension/internal/WikiUIExtensionComponentBuilder.java+1 −4 modified@@ -136,12 +136,9 @@ public List<WikiComponent> buildComponents(BaseObject baseObject) throws WikiCom String.format("Failed to initialize Panel UI extension [%s]", baseObject.getReference()), e); } - String rawParameters = baseObject.getStringValue(PARAMETERS_PROPERTY); - // It would be nice to have PER_LOOKUP components for UIX parameters but without constructor injection it's // safer to use a POJO and pass the Component Manager to it. - WikiUIExtensionParameters parameters = - new WikiUIExtensionParameters(id, rawParameters, this.wikiComponentManager); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(baseObject, this.wikiComponentManager); extension.setParameters(parameters); extension.setScope(scope);
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/main/java/org/xwiki/uiextension/internal/WikiUIExtensionParameters.java+55 −26 modified@@ -22,9 +22,10 @@ import java.io.IOException; import java.io.StringReader; import java.io.StringWriter; -import java.util.HashMap; import java.util.Map; import java.util.Properties; +import java.util.function.Function; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.exception.ExceptionUtils; @@ -38,11 +39,20 @@ import org.xwiki.logging.LoggerConfiguration; import org.xwiki.model.EntityType; import org.xwiki.model.ModelContext; +import org.xwiki.model.reference.DocumentReference; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; import org.xwiki.velocity.VelocityEngine; import org.xwiki.velocity.VelocityManager; import org.xwiki.velocity.XWikiVelocityContext; import org.xwiki.velocity.XWikiVelocityException; +import com.xpn.xwiki.objects.BaseObject; + +import static org.xwiki.uiextension.internal.WikiUIExtensionConstants.ID_PROPERTY; +import static org.xwiki.uiextension.internal.WikiUIExtensionConstants.PARAMETERS_PROPERTY; + /** * Wiki UI Extension parameter manager. * @@ -97,25 +107,37 @@ public class WikiUIExtensionParameters */ private Execution execution; + private AuthorExecutor authorExecutor; + + private final DocumentReference documentReference; + + private final DocumentReference authorReference; + + private final AuthorizationManager authorizationManager; + /** * Default constructor. * - * @param id the unique identifier of this set of parameters, mostly used to isolate parameters value execution - * @param rawParameters raw parameters, their values can contain velocity directives + * @param baseObject the object from which the parameters shall be loaded * @param cm the XWiki component manager * @throws WikiComponentException if some required components can't be found in the Component Manager */ - public WikiUIExtensionParameters(String id, String rawParameters, ComponentManager cm) + public WikiUIExtensionParameters(BaseObject baseObject, ComponentManager cm) throws WikiComponentException { - this.id = id; - this.parameters = parseParameters(rawParameters); + this.id = baseObject.getStringValue(ID_PROPERTY); + this.parameters = parseParameters(baseObject.getStringValue(PARAMETERS_PROPERTY)); + + this.documentReference = baseObject.getDocumentReference(); + this.authorReference = baseObject.getOwnerDocument().getAuthorReference(); try { this.execution = cm.getInstance(Execution.class); this.velocityManager = cm.getInstance(VelocityManager.class); this.modelContext = cm.getInstance(ModelContext.class); this.loggerConfiguration = cm.getInstance(LoggerConfiguration.class); + this.authorExecutor = cm.getInstance(AuthorExecutor.class); + this.authorizationManager = cm.getInstance(AuthorizationManager.class); } catch (ComponentLookupException e) { throw new WikiComponentException( "Failed to get an instance for a component role required by Wiki Components.", e); @@ -148,7 +170,7 @@ private Properties parseParameters(String rawParameters) */ public Map<String, String> get() { - boolean isCacheValid = false; + Map<String, String> result; // Even though the parameters are dynamic, we cache a rendered version of them in order to improve performance. // This cache has a short lifespan, it gets discarded for each new request, or if the database has been switched @@ -158,43 +180,50 @@ public Map<String, String> get() if (currentContextId == this.previousContextId && currentWiki.equals(previousWiki) && this.evaluatedParameters != null) { - isCacheValid = true; - } - - if (!isCacheValid) { - this.evaluatedParameters = new HashMap<>(); - - if (this.parameters.size() > 0) { + result = this.evaluatedParameters; + } else { + result = this.parameters.stringPropertyNames().stream() + .filter(StringUtils::isNotBlank) + .collect(Collectors.toMap(Function.identity(), this.parameters::getProperty)); + + if (!this.parameters.isEmpty() + && this.authorizationManager.hasAccess(Right.SCRIPT, this.authorReference, this.documentReference)) + { try { - VelocityEngine velocityEngine = this.velocityManager.getVelocityEngine(); - VelocityContext velocityContext = this.velocityManager.getVelocityContext(); + this.authorExecutor.call(() -> { + VelocityEngine velocityEngine = this.velocityManager.getVelocityEngine(); + VelocityContext velocityContext = this.velocityManager.getVelocityContext(); - for (String propertyKey : this.parameters.stringPropertyNames()) { - if (!StringUtils.isBlank(propertyKey)) { - String propertyValue = this.parameters.getProperty(propertyKey); + result.replaceAll((propertyKey, propertyValue) -> { StringWriter writer = new StringWriter(); try { String namespace = this.id + ':' + propertyKey; velocityEngine.evaluate( new XWikiVelocityContext(velocityContext, this.loggerConfiguration.isDeprecatedLogEnabled()), writer, namespace, propertyValue); - this.evaluatedParameters.put(propertyKey, writer.toString()); + return writer.toString(); } catch (XWikiVelocityException e) { LOGGER.warn(String.format( "Failed to evaluate UI extension data value, key [%s], value [%s]. Reason: [%s]", propertyKey, propertyValue, e.getMessage())); } - } - } - } catch (XWikiVelocityException ex) { + + return propertyValue; + }); + + return null; + }, this.authorReference, this.documentReference); + } catch (Exception ex) { LOGGER.warn(String.format("Failed to get velocity engine. Reason: [%s]", ex.getMessage())); } - this.previousContextId = currentContextId; - this.previousWiki = currentWiki; } + + this.evaluatedParameters = result; + this.previousContextId = currentContextId; + this.previousWiki = currentWiki; } - return this.evaluatedParameters; + return result; } }
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/test/java/org/xwiki/uiextension/WikiUIExtensionComponentBuilderTest.java+18 −0 modified@@ -23,6 +23,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.concurrent.Callable; import org.apache.velocity.VelocityContext; import org.junit.jupiter.api.Test; @@ -43,6 +44,9 @@ import org.xwiki.rendering.transformation.RenderingContext; import org.xwiki.rendering.transformation.Transformation; import org.xwiki.rendering.util.ErrorBlockGenerator; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; import org.xwiki.test.annotation.BeforeComponent; import org.xwiki.test.annotation.ComponentList; import org.xwiki.test.junit5.mockito.InjectMockComponents; @@ -93,6 +97,12 @@ class WikiUIExtensionComponentBuilderTest implements WikiUIExtensionConstants @MockComponent private LoggerConfiguration loggerConfiguration; + @MockComponent + private AuthorExecutor authorExecutor; + + @MockComponent + private AuthorizationManager authorizationManager; + @InjectMockComponents private WikiUIExtensionComponentBuilder builder; @@ -128,6 +138,13 @@ public void configure(MockitoComponentManager componentManager, MockitoOldcore o componentManager.registerMockComponent(Transformation.class, "macro"); componentManager.registerMockComponent(ContentParser.class); + when(this.authorExecutor.call(any(), any(), any())).thenAnswer(invocation -> { + Callable<?> callable = invocation.getArgument(0); + return callable.call(); + }); + + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOC_REF)).thenReturn(true); + // The document holding the UI extension object. this.componentDoc = mock(XWikiDocument.class, "xwiki:XWiki.MyUIExtension"); when(this.componentDoc.getDocumentReference()).thenReturn(DOC_REF); @@ -197,6 +214,7 @@ private BaseObject createExtensionObject(String id, String extensionPointId, Str BaseObjectReference objectReference = new BaseObjectReference(new ObjectReference("XWiki.UIExtensionClass[0]", DOC_REF)); when(extensionObject.getReference()).thenReturn(objectReference); + when(extensionObject.getDocumentReference()).thenReturn(DOC_REF); when(extensionObject.getOwnerDocument()).thenReturn(this.componentDoc);
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/test/java/org/xwiki/uiextension/WikiUIExtensionParametersTest.java+96 −22 modified@@ -20,6 +20,7 @@ package org.xwiki.uiextension; import java.io.StringWriter; +import java.util.concurrent.Callable; import org.apache.commons.collections.MapUtils; import org.apache.velocity.VelocityContext; @@ -31,25 +32,35 @@ import org.xwiki.context.ExecutionContext; import org.xwiki.logging.LoggerConfiguration; import org.xwiki.model.ModelContext; +import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.WikiReference; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; import org.xwiki.test.LogLevel; import org.xwiki.test.junit5.LogCaptureExtension; import org.xwiki.test.junit5.mockito.ComponentTest; import org.xwiki.test.junit5.mockito.InjectComponentManager; import org.xwiki.test.junit5.mockito.MockComponent; import org.xwiki.test.mockito.MockitoComponentManager; +import org.xwiki.uiextension.internal.WikiUIExtensionConstants; import org.xwiki.uiextension.internal.WikiUIExtensionParameters; import org.xwiki.velocity.VelocityEngine; import org.xwiki.velocity.VelocityManager; import org.xwiki.velocity.XWikiVelocityException; +import com.xpn.xwiki.doc.XWikiDocument; +import com.xpn.xwiki.objects.BaseObject; + import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; /** @@ -61,6 +72,10 @@ @ComponentTest class WikiUIExtensionParametersTest { + private static final DocumentReference AUTHOR_REFERENCE = new DocumentReference("xwiki", "XWiki", "XWikiAdmin"); + + private static final DocumentReference DOCUMENT_REFERENCE = new DocumentReference("xwiki", "Space", "UIX"); + @Mock private VelocityEngine velocityEngine; @@ -76,6 +91,12 @@ class WikiUIExtensionParametersTest @MockComponent private LoggerConfiguration loggerConfiguration; + @MockComponent + private AuthorExecutor authorExecutor; + + @MockComponent + private AuthorizationManager authorizationManager; + @RegisterExtension private final LogCaptureExtension logCapture = new LogCaptureExtension(LogLevel.WARN); @@ -91,24 +112,33 @@ public void setUp() throws Exception when(this.execution.getContext()).thenReturn(executionContext); when(this.velocityManager.getVelocityContext()).thenReturn(velocityContext); when(this.velocityManager.getVelocityEngine()).thenReturn(this.velocityEngine); + + when(this.authorExecutor.call(any(), any(), any())).thenAnswer(invocation -> { + Callable<?> callable = invocation.getArgument(0); + return callable.call(); + }); + + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE)).thenReturn(true); + + when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); } @Test void getParametersWithAnEmptyParametersProperty() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "", this.componentManager); + BaseObject mockUIX = constructMockUIXObject(""); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); assertEquals(MapUtils.EMPTY_MAP, parameters.get()); } @Test void getParametersWithAnEqualSignInAValue() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); // Since the StringWriter is created within the method, the value is "" and not "value". assertEquals("", parameters.get().get("key")); @@ -117,15 +147,13 @@ void getParametersWithAnEqualSignInAValue() throws Exception @Test void getParametersWithCommentAloneOnLine() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); - String paramsStr = "# a = 1\n" + "x=1\n" + "y=2\n" + "# ...\n" + "z=3"; - WikiUIExtensionParameters parameters = - new WikiUIExtensionParameters("id", paramsStr, this.componentManager); + BaseObject mockUIX = constructMockUIXObject(paramsStr); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); parameters.get(); verify(this.velocityEngine).evaluate(any(), any(), eq("id:x"), eq("1")); @@ -136,13 +164,11 @@ void getParametersWithCommentAloneOnLine() throws Exception @Test void getParametersWithCommentEndOfLine() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); - String paramsStr = "x=1##b\n" + "y=2####x\n" + "z=3 ## xyz\n"; - WikiUIExtensionParameters parameters = - new WikiUIExtensionParameters("id", paramsStr, this.componentManager); + BaseObject mockUIX = constructMockUIXObject(paramsStr); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); parameters.get(); verify(this.velocityEngine).evaluate(any(), any(), eq("id:x"), eq("1##b")); @@ -153,26 +179,26 @@ void getParametersWithCommentEndOfLine() throws Exception @Test void getParametersWhenVelocityFails() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenThrow(new XWikiVelocityException("")); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); - // It should fail and put a warn in the logs - assertNull(parameters.get().get("key")); + // It put a warning in the logs and return the not-evaluated value. + assertEquals("value", parameters.get().get("key")); assertEquals("Failed to evaluate UI extension data value, key [key], value [value]. Reason: []", this.logCapture.getMessage(0)); } @Test void getParametersFromTheSameRequestAndForTheSameWiki() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); // It should fail silently assertEquals("", parameters.get().get("key")); @@ -191,7 +217,8 @@ void getParametersFromTheSameRequestButForDifferentWikis() throws Exception when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); // It should fail silently assertEquals("", parameters.get().get("key")); @@ -205,11 +232,11 @@ void getParametersFromTheSameRequestButForDifferentWikis() throws Exception @Test void getParametersFromDifferentRequests() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("wiki1")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); ExecutionContext ec1 = mock(ExecutionContext.class, "ec1"); ExecutionContext ec2 = mock(ExecutionContext.class, "ec2"); @@ -223,4 +250,51 @@ void getParametersFromDifferentRequests() throws Exception verify(this.velocityEngine, times(2)) .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value")); } + + @Test + void getParametersWithoutScriptRight() throws Exception + { + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE)).thenReturn(false); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); + + assertEquals("value", parameters.get().get("key")); + verifyNoInteractions(this.velocityEngine); + } + + @Test + void getParametersWithAuthorExecutor() throws Exception + { + // Do not call the callable to check that the call to the Velocity engine is inside the author executor. + doReturn(null).when(this.authorExecutor).call(any(), any(), any()); + + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); + + assertEquals("value", parameters.get().get("key")); + verify(this.authorExecutor).call(any(), eq(AUTHOR_REFERENCE), eq(DOCUMENT_REFERENCE)); + verifyNoInteractions(this.velocityEngine); + } + + @Test + void getParametersWithEmptyKey() throws Exception + { + BaseObject mockUIX = constructMockUIXObject("=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); + + assertTrue(parameters.get().isEmpty()); + } + + private BaseObject constructMockUIXObject(String parameters) + { + BaseObject result = mock(); + + when(result.getStringValue(WikiUIExtensionConstants.ID_PROPERTY)).thenReturn("id"); + when(result.getStringValue(WikiUIExtensionConstants.PARAMETERS_PROPERTY)).thenReturn(parameters); + when(result.getOwnerDocument()).thenReturn(mock(XWikiDocument.class)); + when(result.getOwnerDocument().getAuthorReference()).thenReturn(WikiUIExtensionParametersTest.AUTHOR_REFERENCE); + when(result.getDocumentReference()).thenReturn(WikiUIExtensionParametersTest.DOCUMENT_REFERENCE); + + return result; + } }
1b2574eb9664XWIKI-21335: Execute UI Extension parameters with the rights of their author
4 files changed · +170 −52
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/main/java/org/xwiki/uiextension/internal/WikiUIExtensionComponentBuilder.java+1 −4 modified@@ -143,12 +143,9 @@ public List<WikiComponent> buildComponents(BaseObject baseObject) throws WikiCom String.format("Failed to initialize Panel UI extension [%s]", baseObject.getReference()), e); } - String rawParameters = baseObject.getStringValue(PARAMETERS_PROPERTY); - // It would be nice to have PER_LOOKUP components for UIX parameters but without constructor injection it's // safer to use a POJO and pass the Component Manager to it. - WikiUIExtensionParameters parameters = - new WikiUIExtensionParameters(id, rawParameters, this.wikiComponentManager); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(baseObject, this.wikiComponentManager); extension.setParameters(parameters); extension.setScope(scope);
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/main/java/org/xwiki/uiextension/internal/WikiUIExtensionParameters.java+55 −26 modified@@ -22,9 +22,10 @@ import java.io.IOException; import java.io.StringReader; import java.io.StringWriter; -import java.util.HashMap; import java.util.Map; import java.util.Properties; +import java.util.function.Function; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.exception.ExceptionUtils; @@ -38,11 +39,20 @@ import org.xwiki.logging.LoggerConfiguration; import org.xwiki.model.EntityType; import org.xwiki.model.ModelContext; +import org.xwiki.model.reference.DocumentReference; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; import org.xwiki.velocity.VelocityEngine; import org.xwiki.velocity.VelocityManager; import org.xwiki.velocity.XWikiVelocityContext; import org.xwiki.velocity.XWikiVelocityException; +import com.xpn.xwiki.objects.BaseObject; + +import static org.xwiki.uiextension.internal.WikiUIExtensionConstants.ID_PROPERTY; +import static org.xwiki.uiextension.internal.WikiUIExtensionConstants.PARAMETERS_PROPERTY; + /** * Wiki UI Extension parameter manager. * @@ -97,25 +107,37 @@ public class WikiUIExtensionParameters */ private Execution execution; + private AuthorExecutor authorExecutor; + + private final DocumentReference documentReference; + + private final DocumentReference authorReference; + + private final AuthorizationManager authorizationManager; + /** * Default constructor. * - * @param id the unique identifier of this set of parameters, mostly used to isolate parameters value execution - * @param rawParameters raw parameters, their values can contain velocity directives + * @param baseObject the object from which the parameters shall be loaded * @param cm the XWiki component manager * @throws WikiComponentException if some required components can't be found in the Component Manager */ - public WikiUIExtensionParameters(String id, String rawParameters, ComponentManager cm) + public WikiUIExtensionParameters(BaseObject baseObject, ComponentManager cm) throws WikiComponentException { - this.id = id; - this.parameters = parseParameters(rawParameters); + this.id = baseObject.getStringValue(ID_PROPERTY); + this.parameters = parseParameters(baseObject.getStringValue(PARAMETERS_PROPERTY)); + + this.documentReference = baseObject.getDocumentReference(); + this.authorReference = baseObject.getOwnerDocument().getAuthorReference(); try { this.execution = cm.getInstance(Execution.class); this.velocityManager = cm.getInstance(VelocityManager.class); this.modelContext = cm.getInstance(ModelContext.class); this.loggerConfiguration = cm.getInstance(LoggerConfiguration.class); + this.authorExecutor = cm.getInstance(AuthorExecutor.class); + this.authorizationManager = cm.getInstance(AuthorizationManager.class); } catch (ComponentLookupException e) { throw new WikiComponentException( "Failed to get an instance for a component role required by Wiki Components.", e); @@ -148,7 +170,7 @@ private Properties parseParameters(String rawParameters) */ public Map<String, String> get() { - boolean isCacheValid = false; + Map<String, String> result; // Even though the parameters are dynamic, we cache a rendered version of them in order to improve performance. // This cache has a short lifespan, it gets discarded for each new request, or if the database has been switched @@ -158,43 +180,50 @@ public Map<String, String> get() if (currentContextId == this.previousContextId && currentWiki.equals(previousWiki) && this.evaluatedParameters != null) { - isCacheValid = true; - } - - if (!isCacheValid) { - this.evaluatedParameters = new HashMap<>(); - - if (this.parameters.size() > 0) { + result = this.evaluatedParameters; + } else { + result = this.parameters.stringPropertyNames().stream() + .filter(StringUtils::isNotBlank) + .collect(Collectors.toMap(Function.identity(), this.parameters::getProperty)); + + if (!this.parameters.isEmpty() + && this.authorizationManager.hasAccess(Right.SCRIPT, this.authorReference, this.documentReference)) + { try { - VelocityEngine velocityEngine = this.velocityManager.getVelocityEngine(); - VelocityContext velocityContext = this.velocityManager.getVelocityContext(); + this.authorExecutor.call(() -> { + VelocityEngine velocityEngine = this.velocityManager.getVelocityEngine(); + VelocityContext velocityContext = this.velocityManager.getVelocityContext(); - for (String propertyKey : this.parameters.stringPropertyNames()) { - if (!StringUtils.isBlank(propertyKey)) { - String propertyValue = this.parameters.getProperty(propertyKey); + result.replaceAll((propertyKey, propertyValue) -> { StringWriter writer = new StringWriter(); try { String namespace = this.id + ':' + propertyKey; velocityEngine.evaluate( new XWikiVelocityContext(velocityContext, this.loggerConfiguration.isDeprecatedLogEnabled()), writer, namespace, propertyValue); - this.evaluatedParameters.put(propertyKey, writer.toString()); + return writer.toString(); } catch (XWikiVelocityException e) { LOGGER.warn(String.format( "Failed to evaluate UI extension data value, key [%s], value [%s]. Reason: [%s]", propertyKey, propertyValue, e.getMessage())); } - } - } - } catch (XWikiVelocityException ex) { + + return propertyValue; + }); + + return null; + }, this.authorReference, this.documentReference); + } catch (Exception ex) { LOGGER.warn(String.format("Failed to get velocity engine. Reason: [%s]", ex.getMessage())); } - this.previousContextId = currentContextId; - this.previousWiki = currentWiki; } + + this.evaluatedParameters = result; + this.previousContextId = currentContextId; + this.previousWiki = currentWiki; } - return this.evaluatedParameters; + return result; } }
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/test/java/org/xwiki/uiextension/WikiUIExtensionComponentBuilderTest.java+18 −0 modified@@ -23,6 +23,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.concurrent.Callable; import org.apache.velocity.VelocityContext; import org.junit.jupiter.api.Test; @@ -43,6 +44,9 @@ import org.xwiki.rendering.transformation.RenderingContext; import org.xwiki.rendering.transformation.Transformation; import org.xwiki.rendering.util.ErrorBlockGenerator; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; import org.xwiki.test.annotation.BeforeComponent; import org.xwiki.test.junit5.mockito.InjectMockComponents; import org.xwiki.test.junit5.mockito.MockComponent; @@ -90,6 +94,12 @@ public class WikiUIExtensionComponentBuilderTest implements WikiUIExtensionConst @MockComponent private LoggerConfiguration loggerConfiguration; + @MockComponent + private AuthorExecutor authorExecutor; + + @MockComponent + private AuthorizationManager authorizationManager; + @InjectMockComponents private WikiUIExtensionComponentBuilder builder; @@ -125,6 +135,13 @@ public void configure(MockitoComponentManager componentManager, MockitoOldcore o componentManager.registerMockComponent(Transformation.class, "macro"); componentManager.registerMockComponent(ContentParser.class); + when(this.authorExecutor.call(any(), any(), any())).thenAnswer(invocation -> { + Callable<?> callable = invocation.getArgument(0); + return callable.call(); + }); + + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOC_REF)).thenReturn(true); + // The document holding the UI extension object. this.componentDoc = mock(XWikiDocument.class, "xwiki:XWiki.MyUIExtension"); when(this.componentDoc.getDocumentReference()).thenReturn(DOC_REF); @@ -194,6 +211,7 @@ private BaseObject createExtensionObject(String id, String extensionPointId, Str BaseObjectReference objectReference = new BaseObjectReference(new ObjectReference("XWiki.UIExtensionClass[0]", DOC_REF)); when(extensionObject.getReference()).thenReturn(objectReference); + when(extensionObject.getDocumentReference()).thenReturn(DOC_REF); when(extensionObject.getOwnerDocument()).thenReturn(this.componentDoc);
xwiki-platform-core/xwiki-platform-uiextension/xwiki-platform-uiextension-api/src/test/java/org/xwiki/uiextension/WikiUIExtensionParametersTest.java+96 −22 modified@@ -20,6 +20,7 @@ package org.xwiki.uiextension; import java.io.StringWriter; +import java.util.concurrent.Callable; import org.apache.commons.collections.MapUtils; import org.apache.velocity.VelocityContext; @@ -31,25 +32,35 @@ import org.xwiki.context.ExecutionContext; import org.xwiki.logging.LoggerConfiguration; import org.xwiki.model.ModelContext; +import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.WikiReference; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; import org.xwiki.test.LogLevel; import org.xwiki.test.junit5.LogCaptureExtension; import org.xwiki.test.junit5.mockito.ComponentTest; import org.xwiki.test.junit5.mockito.InjectComponentManager; import org.xwiki.test.junit5.mockito.MockComponent; import org.xwiki.test.mockito.MockitoComponentManager; +import org.xwiki.uiextension.internal.WikiUIExtensionConstants; import org.xwiki.uiextension.internal.WikiUIExtensionParameters; import org.xwiki.velocity.VelocityEngine; import org.xwiki.velocity.VelocityManager; import org.xwiki.velocity.XWikiVelocityException; +import com.xpn.xwiki.doc.XWikiDocument; +import com.xpn.xwiki.objects.BaseObject; + import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; /** @@ -61,6 +72,10 @@ @ComponentTest class WikiUIExtensionParametersTest { + private static final DocumentReference AUTHOR_REFERENCE = new DocumentReference("xwiki", "XWiki", "XWikiAdmin"); + + private static final DocumentReference DOCUMENT_REFERENCE = new DocumentReference("xwiki", "Space", "UIX"); + @Mock private VelocityEngine velocityEngine; @@ -76,6 +91,12 @@ class WikiUIExtensionParametersTest @MockComponent private LoggerConfiguration loggerConfiguration; + @MockComponent + private AuthorExecutor authorExecutor; + + @MockComponent + private AuthorizationManager authorizationManager; + @RegisterExtension private final LogCaptureExtension logCapture = new LogCaptureExtension(LogLevel.WARN); @@ -91,24 +112,33 @@ public void setUp() throws Exception when(this.execution.getContext()).thenReturn(executionContext); when(this.velocityManager.getVelocityContext()).thenReturn(velocityContext); when(this.velocityManager.getVelocityEngine()).thenReturn(this.velocityEngine); + + when(this.authorExecutor.call(any(), any(), any())).thenAnswer(invocation -> { + Callable<?> callable = invocation.getArgument(0); + return callable.call(); + }); + + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE)).thenReturn(true); + + when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); } @Test void getParametersWithAnEmptyParametersProperty() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "", this.componentManager); + BaseObject mockUIX = constructMockUIXObject(""); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); assertEquals(MapUtils.EMPTY_MAP, parameters.get()); } @Test void getParametersWithAnEqualSignInAValue() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); // Since the StringWriter is created within the method, the value is "" and not "value". assertEquals("", parameters.get().get("key")); @@ -117,15 +147,13 @@ void getParametersWithAnEqualSignInAValue() throws Exception @Test void getParametersWithCommentAloneOnLine() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); - String paramsStr = "# a = 1\n" + "x=1\n" + "y=2\n" + "# ...\n" + "z=3"; - WikiUIExtensionParameters parameters = - new WikiUIExtensionParameters("id", paramsStr, this.componentManager); + BaseObject mockUIX = constructMockUIXObject(paramsStr); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); parameters.get(); verify(this.velocityEngine).evaluate(any(), any(), eq("id:x"), eq("1")); @@ -136,13 +164,11 @@ void getParametersWithCommentAloneOnLine() throws Exception @Test void getParametersWithCommentEndOfLine() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); - String paramsStr = "x=1##b\n" + "y=2####x\n" + "z=3 ## xyz\n"; - WikiUIExtensionParameters parameters = - new WikiUIExtensionParameters("id", paramsStr, this.componentManager); + BaseObject mockUIX = constructMockUIXObject(paramsStr); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); parameters.get(); verify(this.velocityEngine).evaluate(any(), any(), eq("id:x"), eq("1##b")); @@ -153,26 +179,26 @@ void getParametersWithCommentEndOfLine() throws Exception @Test void getParametersWhenVelocityFails() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenThrow(new XWikiVelocityException("")); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); - // It should fail and put a warn in the logs - assertNull(parameters.get().get("key")); + // It put a warning in the logs and return the not-evaluated value. + assertEquals("value", parameters.get().get("key")); assertEquals("Failed to evaluate UI extension data value, key [key], value [value]. Reason: []", this.logCapture.getMessage(0)); } @Test void getParametersFromTheSameRequestAndForTheSameWiki() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("xwiki")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); // It should fail silently assertEquals("", parameters.get().get("key")); @@ -191,7 +217,8 @@ void getParametersFromTheSameRequestButForDifferentWikis() throws Exception when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); // It should fail silently assertEquals("", parameters.get().get("key")); @@ -205,11 +232,11 @@ void getParametersFromTheSameRequestButForDifferentWikis() throws Exception @Test void getParametersFromDifferentRequests() throws Exception { - when(this.modelContext.getCurrentEntityReference()).thenReturn(new WikiReference("wiki1")); when(this.velocityEngine .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value"))) .thenReturn(true); - WikiUIExtensionParameters parameters = new WikiUIExtensionParameters("id", "key=value", this.componentManager); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); ExecutionContext ec1 = mock(ExecutionContext.class, "ec1"); ExecutionContext ec2 = mock(ExecutionContext.class, "ec2"); @@ -223,4 +250,51 @@ void getParametersFromDifferentRequests() throws Exception verify(this.velocityEngine, times(2)) .evaluate(any(VelocityContext.class), any(StringWriter.class), eq("id:key"), eq("value")); } + + @Test + void getParametersWithoutScriptRight() throws Exception + { + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE)).thenReturn(false); + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); + + assertEquals("value", parameters.get().get("key")); + verifyNoInteractions(this.velocityEngine); + } + + @Test + void getParametersWithAuthorExecutor() throws Exception + { + // Do not call the callable to check that the call to the Velocity engine is inside the author executor. + doReturn(null).when(this.authorExecutor).call(any(), any(), any()); + + BaseObject mockUIX = constructMockUIXObject("key=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); + + assertEquals("value", parameters.get().get("key")); + verify(this.authorExecutor).call(any(), eq(AUTHOR_REFERENCE), eq(DOCUMENT_REFERENCE)); + verifyNoInteractions(this.velocityEngine); + } + + @Test + void getParametersWithEmptyKey() throws Exception + { + BaseObject mockUIX = constructMockUIXObject("=value"); + WikiUIExtensionParameters parameters = new WikiUIExtensionParameters(mockUIX, this.componentManager); + + assertTrue(parameters.get().isEmpty()); + } + + private BaseObject constructMockUIXObject(String parameters) + { + BaseObject result = mock(); + + when(result.getStringValue(WikiUIExtensionConstants.ID_PROPERTY)).thenReturn("id"); + when(result.getStringValue(WikiUIExtensionConstants.PARAMETERS_PROPERTY)).thenReturn(parameters); + when(result.getOwnerDocument()).thenReturn(mock(XWikiDocument.class)); + when(result.getOwnerDocument().getAuthorReference()).thenReturn(WikiUIExtensionParametersTest.AUTHOR_REFERENCE); + when(result.getDocumentReference()).thenReturn(WikiUIExtensionParametersTest.DOCUMENT_REFERENCE); + + return result; + } }
Vulnerability mechanics
Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
7- github.com/advisories/GHSA-c2gg-4gq4-jv5jghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2024-31997ghsaADVISORY
- github.com/xwiki/xwiki-platform/commit/171e7c7d0e56deaa7b3678657ae26ef95379b1eaghsax_refsource_MISCWEB
- github.com/xwiki/xwiki-platform/commit/1b2574eb966457ca4ef34e557376b8751d1be90dghsax_refsource_MISCWEB
- github.com/xwiki/xwiki-platform/commit/56748e154a9011f0d6239bec0823eaaeab6ec3f7ghsax_refsource_MISCWEB
- github.com/xwiki/xwiki-platform/security/advisories/GHSA-c2gg-4gq4-jv5jghsax_refsource_CONFIRMWEB
- jira.xwiki.org/browse/XWIKI-21335ghsax_refsource_MISCWEB
News mentions
0No linked articles in our index yet.