Skip to content
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

XWIKI-22817: Dashboards of users without script right have translation calls in gadget titles #3839

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
XWIKI-22817: Dashboards of users without script right have translatio…
…n calls in gadget titles

* Catch titles with just a translation call and load the translation
  directly without evaluating any script.
* Adapt the required rights analyzer.
* Change the gadget wizard to produce a translation macro instead of a
  Velocity call.
michitux committed Jan 24, 2025
commit 3540c2721c5607fac0009d2b9eb80a9f64b30604
Original file line number Diff line number Diff line change
@@ -23,6 +23,8 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javax.inject.Inject;
import javax.inject.Named;
@@ -33,6 +35,7 @@
import org.xwiki.component.annotation.Component;
import org.xwiki.context.Execution;
import org.xwiki.job.event.status.JobProgressManager;
import org.xwiki.localization.ContextualLocalizationManager;
import org.xwiki.model.EntityType;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.DocumentReferenceResolver;
@@ -79,6 +82,9 @@ public class DefaultGadgetSource implements GadgetSource
protected static final EntityReference GADGET_CLASS =
new EntityReference("GadgetClass", EntityType.DOCUMENT, new EntityReference("XWiki", EntityType.SPACE));

static final Pattern TRANSLATION_SCRIPT_PATTERN =
Pattern.compile("\\s*\\$services\\.localization\\.render\\(\\s*'([a-zA-Z0-9.]+)'\\s*\\)\\s*");

/**
* The execution context, to grab XWiki context and access to documents.
*/
@@ -122,6 +128,9 @@ public class DefaultGadgetSource implements GadgetSource
@Inject
private DocumentAuthorizationManager authorizationManager;

@Inject
private ContextualLocalizationManager localizationManager;

/**
* Prepare the parser to parse the title and content of the gadget into blocks.
*/
@@ -196,17 +205,8 @@ private List<Gadget> prepareGadgets(List<BaseObject> objects, Syntax sourceSynta
String position = xObject.getStringValue("position");
String id = xObject.getNumber() + "";

String gadgetTitle;

XWikiDocument ownerDocument = xObject.getOwnerDocument();
if (!ownerDocument.isRestricted() && this.authorizationManager.hasAccess(Right.SCRIPT,
EntityType.DOCUMENT, ownerDocument.getAuthorReference(), ownerDocument.getDocumentReference()))
{
gadgetTitle =
this.evaluateVelocityTitle(velocityContext, velocityEngine, key, title, ownerDocument);
} else {
gadgetTitle = title;
}
String gadgetTitle = evaluateTitle(title, ownerDocument, velocityContext, velocityEngine, key);

// parse both the title and content in the syntax of the transformation context
List<Block> titleBlocks =
@@ -230,6 +230,35 @@ private List<Gadget> prepareGadgets(List<BaseObject> objects, Syntax sourceSynta
return gadgets;
}

private String evaluateTitle(String title, XWikiDocument ownerDocument, VelocityContext velocityContext,
VelocityEngine velocityEngine, String key) throws Exception
{
String gadgetTitle;
if (StringUtils.isBlank(title)) {
gadgetTitle = title;
} else {
// Gadgets are inserted with a localization script service call as title by default. To not break
// backwards compatibility with existing dashboards, just handle those translation calls directly here so
// they work even without scripting right.
Matcher matcher = TRANSLATION_SCRIPT_PATTERN.matcher(title);
if (matcher.matches()) {
String translationKey = matcher.group(1);
gadgetTitle = this.localizationManager.getTranslationPlain(translationKey);
if (gadgetTitle == null) {
gadgetTitle = translationKey;
}
} else if (!ownerDocument.isRestricted() && this.authorizationManager.hasAccess(Right.SCRIPT,
EntityType.DOCUMENT, ownerDocument.getAuthorReference(), ownerDocument.getDocumentReference()))
{
gadgetTitle =
this.evaluateVelocityTitle(velocityContext, velocityEngine, key, title, ownerDocument);
} else {
gadgetTitle = title;
}
}
return gadgetTitle;
}

private String evaluateVelocityTitle(VelocityContext velocityContext, VelocityEngine velocityEngine, String key,
String title, XWikiDocument ownerDocument) throws Exception
{
Original file line number Diff line number Diff line change
@@ -26,6 +26,7 @@
import javax.inject.Named;
import javax.inject.Singleton;

import org.apache.commons.lang3.StringUtils;
import org.xwiki.component.annotation.Component;
import org.xwiki.platform.security.requiredrights.RequiredRight;
import org.xwiki.platform.security.requiredrights.RequiredRightAnalysisResult;
@@ -71,29 +72,43 @@ public List<RequiredRightAnalysisResult> analyze(BaseObject object) throws Requi

// Analyze the title
String titleString = object.getStringValue("title");
if (titleString != null && this.velocityDetector.containsVelocityScript(titleString)) {
result.add(new RequiredRightAnalysisResult(
object.getReference(),
this.translationMessageSupplierProvider.get("dashboard.requiredrights.gadget.title"),
this.translationMessageSupplierProvider.get(
"dashboard.requiredrights.gadget.title.description", titleString),
List.of(RequiredRight.MAYBE_SCRIPT, RequiredRight.MAYBE_PROGRAM)
));
// The gadget source handles localization script service calls separately to support them for users without
// script right.
if (StringUtils.isNotBlank(titleString)
&& !DefaultGadgetSource.TRANSLATION_SCRIPT_PATTERN.matcher(titleString).matches())
{
if (this.velocityDetector.containsVelocityScript(titleString)) {
result.add(new RequiredRightAnalysisResult(
object.getReference(),
this.translationMessageSupplierProvider.get("dashboard.requiredrights.gadget.title"),
this.translationMessageSupplierProvider.get(
"dashboard.requiredrights.gadget.title.description", titleString),
List.of(RequiredRight.MAYBE_SCRIPT, RequiredRight.MAYBE_PROGRAM)
));
} else {
result.addAll(analyzeWikiContent(object, titleString));
}
}

// Analyze the content
String contentString = object.getStringValue("content");
if (contentString != null) {
try {
XDOM parsedContent = this.contentParser.parse(contentString,
object.getOwnerDocument().getSyntax(), object.getDocumentReference());
parsedContent.getMetaData().addMetaData("entityReference", object.getReference());
result.addAll(this.xdomRequiredRightAnalyzer.analyze(parsedContent));
} catch (MissingParserException | ParseException e) {
throw new RequiredRightsException("Failed to parse value of 'content' property.", e);
}
if (StringUtils.isNotBlank(contentString)) {
result.addAll(analyzeWikiContent(object, contentString));
}

return result;
}

private List<RequiredRightAnalysisResult> analyzeWikiContent(BaseObject object, String contentString)
throws RequiredRightsException
{
try {
XDOM parsedContent = this.contentParser.parse(contentString,
object.getOwnerDocument().getSyntax(), object.getDocumentReference());
parsedContent.getMetaData().addMetaData("entityReference", object.getReference());
return this.xdomRequiredRightAnalyzer.analyze(parsedContent);
} catch (MissingParserException | ParseException e) {
throw new RequiredRightsException("Failed to parse value of 'content' property.", e);
}
}
}
Original file line number Diff line number Diff line change
@@ -31,6 +31,7 @@
import org.mockito.stubbing.Answer;
import org.xwiki.context.Execution;
import org.xwiki.context.ExecutionContext;
import org.xwiki.localization.ContextualLocalizationManager;
import org.xwiki.model.EntityType;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.DocumentReferenceResolver;
@@ -79,6 +80,9 @@ class DefaultGadgetSourceTest
@MockComponent
private DocumentAuthorizationManager authorizationManager;

@MockComponent
private ContextualLocalizationManager localizationManager;

@Mock
private DocumentReference documentReference;

@@ -193,17 +197,28 @@ void getGadgetWithoutScriptRight() throws Exception
assertEquals(new ArrayList<>(), this.defaultGadgetSource.getGadgets(testSource, macroTransformationContext));

BaseObject gadgetObject1 = mock(BaseObject.class);
when(xWikiDocument.getXObjects(gadgetClassReference)).thenReturn(Collections.singletonList(gadgetObject1));
when(gadgetObject1.getOwnerDocument()).thenReturn(ownerDocument);
when(gadgetObject1.getStringValue("title")).thenReturn("Gadget 2");
when(gadgetObject1.getLargeStringValue("content")).thenReturn("Some other content");
when(gadgetObject1.getStringValue("position")).thenReturn("2");
when(gadgetObject1.getNumber()).thenReturn(12);

BaseObject gadgetObject2 = mock();
when(gadgetObject2.getOwnerDocument()).thenReturn(ownerDocument);
when(gadgetObject2.getStringValue("title")).thenReturn("$services.localization.render('xwiki.gadget2')");
when(gadgetObject2.getLargeStringValue("content")).thenReturn("Localized content");
when(gadgetObject2.getStringValue("position")).thenReturn("3");
when(gadgetObject2.getNumber()).thenReturn(13);

when(this.localizationManager.getTranslationPlain("xwiki.gadget2")).thenReturn("Translated Title");

when(xWikiDocument.getXObjects(gadgetClassReference)).thenReturn(List.of(gadgetObject1, gadgetObject2));

when(this.authorizationManager.hasAccess(Right.SCRIPT, EntityType.DOCUMENT, ownerAuthorReference,
ownerSourceReference)).thenReturn(false);

List<Gadget> gadgets = this.defaultGadgetSource.getGadgets(testSource, macroTransformationContext);
assertEquals(1, gadgets.size());
assertEquals(2, gadgets.size());
Gadget gadget = gadgets.get(0);
assertEquals("Gadget 2", gadget.getTitle().get(0).toString());
assertEquals("Some other content", gadget.getContent().get(0).toString());
@@ -212,5 +227,14 @@ void getGadgetWithoutScriptRight() throws Exception
.execute(eq("Gadget 2"), any(), any(), any());
verify(this.contentExecutor)
.execute(eq("Some other content"), any(), any(), any());

Gadget gadget2 = gadgets.get(1);
assertEquals("Translated Title", gadget2.getTitle().get(0).toString());
assertEquals("Localized content", gadget2.getContent().get(0).toString());
assertEquals("13", gadget2.getId());
verify(this.contentExecutor)
.execute(eq("Translated Title"), any(), any(), any());
verify(this.contentExecutor)
.execute(eq("Localized content"), any(), any(), any());
}
}
Original file line number Diff line number Diff line change
@@ -33,9 +33,10 @@
import org.xwiki.platform.security.requiredrights.RequiredRightAnalysisResult;
import org.xwiki.platform.security.requiredrights.RequiredRightAnalyzer;
import org.xwiki.platform.security.requiredrights.RequiredRightsException;
import org.xwiki.platform.security.requiredrights.internal.analyzer.XDOMRequiredRightAnalyzer;
import org.xwiki.platform.security.requiredrights.display.BlockSupplierProvider;
import org.xwiki.platform.security.requiredrights.internal.analyzer.XDOMRequiredRightAnalyzer;
import org.xwiki.rendering.block.MacroBlock;
import org.xwiki.rendering.block.WordBlock;
import org.xwiki.rendering.block.XDOM;
import org.xwiki.rendering.parser.ContentParser;
import org.xwiki.rendering.parser.MissingParserException;
@@ -106,7 +107,7 @@ void setup() throws Exception
@Test
void checkTitleWithVelocity() throws RequiredRightsException
{
String title = "$services.localization.render('gadget')";
String title = "$evil";
when(this.object.getStringValue("title")).thenReturn(title);

List<RequiredRightAnalysisResult> analysisResults = this.analyzer.analyze(this.object);
@@ -122,6 +123,17 @@ void checkTitleWithVelocity() throws RequiredRightsException
List.of(RequiredRight.MAYBE_PROGRAM, RequiredRight.MAYBE_SCRIPT)));
}

@Test
void checkTitleWithTranslation() throws RequiredRightsException
{
String title = "$services.localization.render('gadget')";
when(this.object.getStringValue("title")).thenReturn(title);

List<RequiredRightAnalysisResult> analysisResults = this.analyzer.analyze(this.object);

assertEquals(List.of(), analysisResults);
}

@Test
void checkContentWithXDOMAnalyzer() throws RequiredRightsException, MissingParserException, ParseException
{
@@ -143,4 +155,25 @@ void checkContentWithXDOMAnalyzer() throws RequiredRightsException, MissingParse
assertEquals(1, analysisResults.size());
assertEquals(wikiResult, analysisResults.get(0));
}

@Test
void checkTitleWithXDOMAnalyzer() throws Exception
{
String title = "{{velocity}}Title code{{/velocity}}";
when(this.object.getStringValue("title")).thenReturn(title);
XDOM xdom = new XDOM(List.of(new WordBlock("title")));
when(this.contentParser.parse(title, Syntax.XWIKI_2_1, this.documentReference)).thenReturn(xdom);

RequiredRightAnalysisResult wikiResult = mock();
when(this.xdomRequiredRightAnalyzer.analyze(xdom)).thenReturn(List.of(wikiResult));

List<RequiredRightAnalysisResult> analysisResults = this.analyzer.analyze(this.object);

verify(this.xdomRequiredRightAnalyzer).analyze(xdom);
assertEquals(this.object.getReference(),
xdom.getMetaData().getMetaData().get(XDOMRequiredRightAnalyzer.ENTITY_REFERENCE_METADATA));

assertEquals(1, analysisResults.size());
assertEquals(wikiResult, analysisResults.get(0));
}
}
Original file line number Diff line number Diff line change
@@ -68,8 +68,8 @@ define(['jquery', 'xwiki-ckeditor'], function($, ckeditorPromise) {
};

var getDefaultGadgetTitle = function(macroEditor) {
var gadgetName = macroEditor.attr('data-macroid').split('/')[0];
return "$services.localization.render('rendering.macro." + gadgetName + ".name')";
const gadgetName = macroEditor.attr('data-macroid').split('/')[0];
return '{{translation key="rendering.macro.' + gadgetName.replace('~', '~~').replace('"', '~"') + '.name"/}}';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if we shouldn't also escape }}.

};

var currentGadget;