From 480186f9d2fca880513da8bc5a609674d106cbd3 Mon Sep 17 00:00:00 2001 From: pjeanjean Date: Thu, 19 Oct 2023 17:35:07 +0200 Subject: [PATCH] XWIKI-21337: Apply PDF templates with the rights of their authors (cherry picked from commit d28e21a670c69880b951e415dd2ddd69d273eae9) --- .../com/xpn/xwiki/pdf/impl/PdfExportImpl.java | 80 +++++--- .../xpn/xwiki/pdf/impl/PdfExportImplTest.java | 181 +++++++++++++++--- 2 files changed, 206 insertions(+), 55 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/pdf/impl/PdfExportImpl.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/pdf/impl/PdfExportImpl.java index 9f471406bc61..b4d86b793d54 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/pdf/impl/PdfExportImpl.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/pdf/impl/PdfExportImpl.java @@ -29,6 +29,7 @@ import java.io.StringWriter; import java.lang.reflect.Type; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -37,9 +38,9 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang.exception.ExceptionUtils; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.velocity.VelocityContext; import org.dom4j.Element; import org.dom4j.io.OutputFormat; @@ -54,8 +55,11 @@ import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.DocumentReferenceResolver; import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; +import org.xwiki.user.UserReferenceSerializer; import org.xwiki.velocity.VelocityManager; -import org.xwiki.velocity.XWikiVelocityException; import org.xwiki.xml.EntityResolver; import org.xwiki.xml.XMLReaderFactory; import org.xwiki.xml.XMLUtils; @@ -95,30 +99,37 @@ public class PdfExportImpl implements PdfExport private static final Logger LOGGER = LoggerFactory.getLogger(PdfExportImpl.class); /** Document name resolver. */ - private static DocumentReferenceResolver referenceResolver = + private final DocumentReferenceResolver referenceResolver = Utils.getComponent(DocumentReferenceResolver.TYPE_STRING, "currentmixed"); /** Document name serializer. */ - private static EntityReferenceSerializer referenceSerializer = + private final EntityReferenceSerializer referenceSerializer = Utils.getComponent(EntityReferenceSerializer.TYPE_STRING); /** Provides access to document properties. */ - private static DocumentAccessBridge dab = Utils.getComponent(DocumentAccessBridge.class); + private final DocumentAccessBridge dab = Utils.getComponent(DocumentAccessBridge.class); /** Velocity engine manager, used for interpreting velocity. */ - private static VelocityManager velocityManager = Utils.getComponent(VelocityManager.class); + private final VelocityManager velocityManager = Utils.getComponent(VelocityManager.class); - private static XMLReaderFactory xmlReaderFactory = Utils.getComponent(XMLReaderFactory.class); + private final XMLReaderFactory xmlReaderFactory = Utils.getComponent(XMLReaderFactory.class); + + private final AuthorizationManager authorizationManager = Utils.getComponent(AuthorizationManager.class); + + private final AuthorExecutor authorExecutor = Utils.getComponent(AuthorExecutor.class); + + private final UserReferenceSerializer userReferenceSerializer = + Utils.getComponent(UserReferenceSerializer.TYPE_DOCUMENT_REFERENCE, "document"); /** * Used to get the temporary directory. */ - private Environment environment = Utils.getComponent((Type) Environment.class); + private final Environment environment = Utils.getComponent((Type) Environment.class); /** * Used to render XSL-FO to PDF. */ - private XSLFORenderer xslFORenderer = Utils.getComponent(XSLFORenderer.class, "fop"); + private final XSLFORenderer xslFORenderer = Utils.getComponent(XSLFORenderer.class, "fop"); @Override public void exportToPDF(XWikiDocument doc, OutputStream out, XWikiContext context) throws XWikiException @@ -184,7 +195,7 @@ private String convertToStrictXHtml(String input) HTMLCleaner cleaner = Utils.getComponent(HTMLCleaner.class); HTMLCleanerConfiguration config = cleaner.getDefaultConfiguration(); - List filters = new ArrayList(config.getFilters()); + List filters = new ArrayList<>(config.getFilters()); filters.add(Utils.getComponent(HTMLFilter.class, "uniqueId")); config.setFilters(filters); String result = HTMLUtils.toString(cleaner.clean(new StringReader(input), config)); @@ -261,7 +272,7 @@ private void renderXSLFO(String xmlfo, OutputStream out, ExportType type, final throws XWikiException { try { - this.xslFORenderer.render(new ByteArrayInputStream(xmlfo.getBytes("UTF-8")), out, type.getMimeType()); + this.xslFORenderer.render(new ByteArrayInputStream(xmlfo.getBytes(StandardCharsets.UTF_8)), out, type.getMimeType()); } catch (IllegalStateException e) { throw createException(e, type, XWikiException.ERROR_XWIKI_APP_SEND_RESPONSE_EXCEPTION); } catch (Exception e) { @@ -338,7 +349,7 @@ String applyCSS(String html, String css, XWikiContext context) // Dom4J 2.1.1 disables external DTDs by default, so we set our own XMLReader. // See https://github.com/dom4j/dom4j/issues/51 - XMLReader xmlReader = xmlReaderFactory.createXMLReader(); + XMLReader xmlReader = this.xmlReaderFactory.createXMLReader(); reader.setXMLReader(xmlReader); reader.setEntityResolver(new DefaultEntityResolver()); @@ -465,28 +476,47 @@ private String getPDFTemplateProperty(String propertyName, XWikiContext context) DocumentReference templateReference; DocumentReference classReference; if (StringUtils.isNotEmpty(pdftemplate)) { - templateReference = referenceResolver.resolve(pdftemplate); + templateReference = this.referenceResolver.resolve(pdftemplate); classReference = new DocumentReference(templateReference.getWikiReference().getName(), "XWiki", "PDFClass"); } else { - templateReference = dab.getCurrentDocumentReference(); - String currentWiki = dab.getCurrentDocumentReference().getRoot().getName(); + templateReference = this.dab.getCurrentDocumentReference(); + String currentWiki = this.dab.getCurrentDocumentReference().getRoot().getName(); classReference = new DocumentReference(currentWiki, "XWiki", "PDFClass"); } - String result = (String) dab.getProperty(templateReference, classReference, propertyName); - if (StringUtils.isBlank(result)) { + String templateContent = (String) this.dab.getProperty(templateReference, classReference, propertyName); + if (StringUtils.isBlank(templateContent)) { return ""; } - String templateName = referenceSerializer.serialize(templateReference); + + String templateName = this.referenceSerializer.serialize(templateReference); + DocumentReference templateAuthorReference; + String result = templateContent; try { - StringWriter writer = new StringWriter(); - VelocityContext vcontext = velocityManager.getVelocityContext(); - velocityManager.getVelocityEngine().evaluate(vcontext, writer, templateName, result); - result = writer.toString(); - } catch (XWikiVelocityException e) { - LOGGER.warn("Error applying Velocity to the [{}] property of the [{}] document. Using the property's value " - + "without applying Velocity.", propertyName, templateName, ExceptionUtils.getRootCauseMessage(e)); + templateAuthorReference = this.userReferenceSerializer.serialize( + this.dab.getDocumentInstance(templateReference).getAuthors().getEffectiveMetadataAuthor()); + } catch (Exception e) { + LOGGER.warn("Error fetching the author of template [{}] during PDF conversion. Using the [{}] property of " + + "the document's value without applying Velocity.", templateName, propertyName); + return result; } + + if (this.authorizationManager.hasAccess(Right.SCRIPT, templateAuthorReference, templateReference)) { + try { + result = this.authorExecutor.call(() -> { + StringWriter writer = new StringWriter(); + VelocityContext vcontext = this.velocityManager.getVelocityContext(); + this.velocityManager.getVelocityEngine().evaluate(vcontext, writer, templateName, + templateContent); + return writer.toString(); + }, templateAuthorReference, templateReference); + } catch (Exception e) { + LOGGER.warn("Failed to run Velocity engine in author executor. Using the [{}] property of the [{}] " + + "document's value without applying Velocity. Reason: [{}]", + propertyName, templateName, ExceptionUtils.getRootCauseMessage(e)); + } + } + return result; } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/pdf/impl/PdfExportImplTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/pdf/impl/PdfExportImplTest.java index 7532ee3167a9..b273f8544617 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/pdf/impl/PdfExportImplTest.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/pdf/impl/PdfExportImplTest.java @@ -19,23 +19,55 @@ */ package com.xpn.xwiki.pdf.impl; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.StringReader; + +import javax.xml.parsers.DocumentBuilderFactory; + +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.w3c.dom.Document; import org.xwiki.bridge.DocumentAccessBridge; import org.xwiki.environment.Environment; -import org.xwiki.model.reference.DocumentReferenceResolver; -import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.job.event.status.JobProgressManager; +import org.xwiki.model.reference.DocumentReference; +import org.xwiki.observation.ObservationManager; +import org.xwiki.security.authorization.AuthorExecutor; +import org.xwiki.security.authorization.AuthorizationManager; +import org.xwiki.security.authorization.Right; +import org.xwiki.template.TemplateManager; import org.xwiki.test.annotation.ComponentList; +import org.xwiki.test.junit5.mockito.MockComponent; +import org.xwiki.user.UserReference; +import org.xwiki.user.UserReferenceResolver; +import org.xwiki.velocity.VelocityEngine; import org.xwiki.velocity.VelocityManager; +import org.xwiki.xml.EntityResolver; +import org.xwiki.xml.html.HTMLCleaner; +import org.xwiki.xml.html.HTMLCleanerConfiguration; +import org.xwiki.xml.html.filter.HTMLFilter; +import org.xwiki.xml.internal.XMLReaderFactoryComponent; +import org.xwiki.xml.internal.html.DefaultHTMLCleanerConfiguration; import com.xpn.xwiki.XWikiContext; import com.xpn.xwiki.doc.XWikiDocument; import com.xpn.xwiki.internal.pdf.XSLFORenderer; +import com.xpn.xwiki.pdf.api.PdfExport; import com.xpn.xwiki.test.MockitoOldcore; import com.xpn.xwiki.test.junit5.mockito.InjectMockitoOldcore; import com.xpn.xwiki.test.junit5.mockito.OldcoreTest; +import com.xpn.xwiki.test.reference.ReferenceComponentList; +import com.xpn.xwiki.web.XWikiServletRequestStub; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; /** @@ -44,40 +76,59 @@ * @version $Id$ */ @ComponentList({ - org.xwiki.xml.internal.XMLReaderFactoryComponent.class, + XMLReaderFactoryComponent.class, }) +@ReferenceComponentList @OldcoreTest -public class PdfExportImplTest +class PdfExportImplTest { + private static final DocumentReference AUTHOR_REFERENCE = new DocumentReference("xwiki", "XWiki", "XWikiAdmin"); + + private static final DocumentReference DOCUMENT_REFERENCE = new DocumentReference("xwiki", "XWiki", "PDFClass"); + @InjectMockitoOldcore private MockitoOldcore oldcore; - /** - * Verify that PDF Export can apply some CSS on the XHTML when that XHTML already has some style defined and in - * shorthand notation. - */ - @Test - public void applyCSSWhenExistingStyleDefinedUsingShorthandNotation() throws Exception + @Mock + private VelocityEngine velocityEngine; + + @MockComponent + private AuthorizationManager authorizationManager; + + @MockComponent + private HTMLCleaner htmlCleaner; + + @MockComponent + private AuthorExecutor authorExecutor; + + private String htmlContent; + + private String cssProperties; + + private XWikiContext context; + + private PdfExportImpl pdfExport; + + @BeforeEach + void setUp() throws Exception { - this.oldcore.getMocker().registerMockComponent(DocumentReferenceResolver.TYPE_STRING, "currentmixed"); - this.oldcore.getMocker().registerMockComponent(EntityReferenceSerializer.TYPE_STRING); - this.oldcore.getMocker().registerMockComponent(DocumentAccessBridge.class); - this.oldcore.getMocker().registerMockComponent(DocumentAccessBridge.class); this.oldcore.getMocker().registerMockComponent(PDFResourceResolver.class); this.oldcore.getMocker().registerMockComponent(Environment.class); - this.oldcore.getMocker().registerMockComponent(VelocityManager.class); + this.oldcore.getMocker().registerMockComponent(TemplateManager.class); + this.oldcore.getMocker().registerMockComponent(ObservationManager.class); + this.oldcore.getMocker().registerMockComponent(JobProgressManager.class); + this.oldcore.getMocker().registerMockComponent(EntityResolver.class); this.oldcore.getMocker().registerMockComponent(XSLFORenderer.class, "fop"); - - PdfExportImpl pdfExport = new PdfExportImpl(); + this.oldcore.getMocker().registerMockComponent(HTMLFilter.class, "uniqueId"); // The content below allows us to test several points: // 1) The SPAN below already has some style defined in shorthand notation( "background" is shorthand, // see https://www.w3schools.com/css/css_background.asp). That's important for the test since that's what was // failing in the past and why this test was written. // 2) We also test that HTML entities are correctly kept since we had issues with this at one point. - String html = "\n" + this.htmlContent = "\n" + "\n" + + "\"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n" + "\n" + "\n" + " Main.ttt - ttt\n" @@ -98,13 +149,51 @@ public void applyCSSWhenExistingStyleDefinedUsingShorthandNotation() throws Exce + "\n" + "</body></html>"; - String css = "span { color:red; }"; + this.cssProperties = "span { color:red; }"; - XWikiContext xcontext = this.oldcore.getXWikiContext(); + // Set up HTML cleaner. + Document htmlDocument = DocumentBuilderFactory.newInstance().newDocumentBuilder() + .parse(new ByteArrayInputStream(this.htmlContent.getBytes())); + HTMLCleanerConfiguration cleanerConfiguration = new DefaultHTMLCleanerConfiguration(); + when(this.htmlCleaner.getDefaultConfiguration()).thenReturn(cleanerConfiguration); + when(this.htmlCleaner.clean(any(StringReader.class), eq(cleanerConfiguration))).thenReturn(htmlDocument); + + // Get a mocked Velocity Engine. + VelocityManager velocityManager = this.oldcore.getMocker().registerMockComponent(VelocityManager.class); + when(velocityManager.getVelocityEngine()).thenReturn(this.velocityEngine); + + // Prepare a document reference and author reference for the template. + XWikiDocument template = new XWikiDocument(DOCUMENT_REFERENCE); + UserReferenceResolver<DocumentReference> userReferenceResolver = + this.oldcore.getMocker().getInstance(UserReferenceResolver.TYPE_DOCUMENT_REFERENCE, "document"); + UserReference userReference = userReferenceResolver.resolve(AUTHOR_REFERENCE); + template.getAuthors().setEffectiveMetadataAuthor(userReference); + + // Return a non-empty template property. + DocumentAccessBridge dab = this.oldcore.getDocumentAccessBridge(); + when(dab.getProperty(template.getDocumentReference(), template.getDocumentReference(), "style")) + .thenReturn(this.cssProperties); + when(dab.getDocumentInstance(DOCUMENT_REFERENCE)).thenReturn(template); + + // Set necessary parameters in the request. + this.context = this.oldcore.getXWikiContext(); + XWikiServletRequestStub request = new XWikiServletRequestStub(); + request.put("pdftemplate", "XWiki.PDFClass"); + this.context.setRequest(request); XWikiDocument doc = mock(XWikiDocument.class); - when(doc.getExternalURL("view", xcontext)).thenReturn("http://localhost:8080/export"); - xcontext.setDoc(doc); + when(doc.getExternalURL("view", this.context)).thenReturn("http://localhost:8080/export"); + this.context.setDoc(doc); + this.pdfExport = new PdfExportImpl(); + } + + /** + * Verify that PDF Export can apply some CSS on the XHTML when that XHTML already has some style defined and in + * shorthand notation. + */ + @Test + void applyCSSWhenExistingStyleDefinedUsingShorthandNotation() + { // - Verify that element's style attributes are normalized and that the SPAN's color is set to red. // - Verify that the accent in the content is still there. // TODO: right now we output the DOM with DOM4J and use the default of converting entities when using the @@ -113,8 +202,8 @@ public void applyCSSWhenExistingStyleDefinedUsingShorthandNotation() throws Exce String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" + "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" " - + "\"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">" - + "<html xmlns=\"http://www.w3.org/1999/xhtml\"><head>\n" + + "\"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">" + + "<html xmlns=\"http://www.w3.org/1999/xhtml\"><head>\n" + "<title>\n" + " Main.ttt - ttt\n" + "\n" @@ -124,15 +213,47 @@ public void applyCSSWhenExistingStyleDefinedUsingShorthandNotation() throws Exce + "
\n" + "
\n\n" + "
\n" - + "

Hello Clément

\n" + + "

Hello Clément

\n" + "
\n" + "
\n" + "
\n\n" + ""; - assertEquals(expected, pdfExport.applyCSS(html, css, xcontext)); + assertEquals(expected, this.pdfExport.applyCSS(this.htmlContent, this.cssProperties, this.context)); + } + + /** + * Verify that the Velocity Engine is never accessed if the user does not have script rights. + */ + @Test + void applyPDFTemplateWithoutScriptRights() throws Exception + { + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE)).thenReturn(false); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + this.pdfExport.exportHtml(this.htmlContent, baos, PdfExport.ExportType.PDF, this.context); + verify(this.authorizationManager).hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE); + verifyNoInteractions(this.authorExecutor); + verifyNoInteractions(this.velocityEngine); + } + + /** + * Verify that the Velocity Engine is not accessed outside an Author Executor. + */ + @Test + void applyPDFTemplateWithAuthorExecutor() throws Exception + { + when(this.authorizationManager.hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE)).thenReturn(true); + + // Do not call the callable to check that the call to the Velocity engine is inside the author executor. + doReturn("").when(this.authorExecutor).call(any(), any(), any()); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + this.pdfExport.exportHtml(this.htmlContent, baos, PdfExport.ExportType.PDF, this.context); + verify(this.authorizationManager).hasAccess(Right.SCRIPT, AUTHOR_REFERENCE, DOCUMENT_REFERENCE); + verify(this.authorExecutor).call(any(), eq(AUTHOR_REFERENCE), eq(DOCUMENT_REFERENCE)); + verifyNoInteractions(this.velocityEngine); } }