Skip to content

Commit

Permalink
XWIKI-5027: Improve included content transformation
Browse files Browse the repository at this point in the history
  • Loading branch information
tmortagne committed Dec 7, 2022
1 parent c1fb144 commit b48116a
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ public DisplayMacro()
{
super("Display", DESCRIPTION, DisplayMacroParameters.class);

// The display macro must execute first since if it runs with the current context it needs to bring
// all the macros from the displayed page before the other macros are executed.
setPriority(10);
setDefaultCategories(Set.of(DEFAULT_CATEGORY_CONTENT));
}

Expand Down Expand Up @@ -97,7 +94,7 @@ public List<Block> execute(DisplayMacroParameters parameters, String content, Ma
}

// Step 3: Check right
if (!this.authorization.hasAccess(Right.VIEW, documentBridge.getDocumentReference())) {
if (!this.contextualAuthorization.hasAccess(Right.VIEW, documentBridge.getDocumentReference())) {
throw new MacroExecutionException(
String.format("Current user [%s] doesn't have view rights on document [%s]",
this.documentAccessBridge.getCurrentUserReference(), documentBridge.getDocumentReference()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@ public ContextMacro()
super("Context", DESCRIPTION, new DefaultContentDescriptor(CONTENT_DESCRIPTION, true, Block.LIST_BLOCK_TYPE),
ContextMacroParameters.class);

// The Context macro must execute early since it can contain include macros which can bring stuff like headings
// for other macros (TOC macro, etc). Make it the same priority as the Include macro.
setPriority(10);
setDefaultCategories(Set.of(DEFAULT_CATEGORY_DEVELOPMENT));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public abstract class AbstractIncludeMacro<P> extends AbstractMacro<P>
protected DocumentAccessBridge documentAccessBridge;

@Inject
protected ContextualAuthorizationManager authorization;
protected ContextualAuthorizationManager contextualAuthorization;

/**
* Used to serialize resolved document links into a string again since the Rendering API only manipulates Strings
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.Stack;

Expand All @@ -44,10 +43,12 @@
import org.xwiki.rendering.listener.MetaData;
import org.xwiki.rendering.macro.MacroExecutionException;
import org.xwiki.rendering.macro.include.IncludeMacroParameters;
import org.xwiki.rendering.macro.include.IncludeMacroParameters.Author;
import org.xwiki.rendering.macro.include.IncludeMacroParameters.Context;
import org.xwiki.rendering.transformation.MacroTransformationContext;
import org.xwiki.rendering.transformation.TransformationManager;
import org.xwiki.security.authorization.AuthorExecutor;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.Right;

/**
Expand All @@ -74,16 +75,16 @@ public class IncludeMacro extends AbstractIncludeMacro<IncludeMacroParameters>
@Inject
private AuthorExecutor authorExecutor;

@Inject
protected AuthorizationManager authorization;

/**
* Default constructor.
*/
public IncludeMacro()
{
super("Include", DESCRIPTION, IncludeMacroParameters.class);

// The include macro must execute first since if it runs with the current context it needs to bring
// all the macros from the included page before the other macros are executed.
setPriority(10);
setDefaultCategories(Set.of(DEFAULT_CATEGORY_CONTENT));
}

Expand Down Expand Up @@ -113,7 +114,7 @@ public List<Block> execute(IncludeMacroParameters parameters, String content, Ma
}

// Step 3: Check right
if (!this.authorization.hasAccess(Right.VIEW, documentBridge.getDocumentReference())) {
if (!this.contextualAuthorization.hasAccess(Right.VIEW, documentBridge.getDocumentReference())) {
throw new MacroExecutionException(
String.format("Current user [%s] doesn't have view rights on document [%s]",
this.documentAccessBridge.getCurrentUserReference(), documentBridge.getDocumentReference()));
Expand Down Expand Up @@ -177,17 +178,17 @@ public List<Block> execute(IncludeMacroParameters parameters, String content, Ma
metadata.getMetaData().addMetaData(MetaData.BASE, source);
}

// Step 7: If the included content author is different from the current author, we have to execute it right now
// so that it used the proper rights
// Get the translated version of the document to compare with the right author
// Step 7: The the include macro is explicitly configured to be executed with the included document content
// author of if that author does not have programming right, execute it right away
// Get the translated version of the document to get the content author
DocumentModelBridge translatedDocumentBridge;
try {
translatedDocumentBridge = this.documentAccessBridge.getTranslatedDocumentInstance(documentBridge);
} catch (Exception e) {
throw new MacroExecutionException("Failed to retrieve the translated version of the document", e);
}
if (!Objects.equals(translatedDocumentBridge.getContentAuthorReference(),
this.documentAccessBridge.getCurrentAuthorReference())) {
if (parameters.getAuthor() == Author.TARGET || parameters.getAuthor() == Author.AUTO && !this.authorization
.hasAccess(Right.PROGRAM, translatedDocumentBridge.getContentAuthorReference(), null)) {
// Merge the two XDOM before executing the included content so that it's as close as possible to the expect
// execution conditions
MacroBlock includeMacro = context.getCurrentMacroBlock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,30 @@ public enum Context
CURRENT
}

/**
* Control which author to execute the included content with.
*
* @since 15.0RC1
*/
public enum Author
{
/**
* Apply TARGET option unless the target author has programming right in which case it applies CURRENT (mainly
* for retro-compatibility reasons).
*/
AUTO,

/**
* The content is executed with the right of the current author which uses the include macro.
*/
CURRENT,

/**
* The content is executed with the right of the included content author.
*/
TARGET
}

/**
* @see #getReference()
*/
Expand All @@ -74,12 +98,14 @@ public enum Context
* @see #getSection()
*/
private String section;

/**
* @see #isExcludeFirstHeading()
*/
private boolean excludeFirstHeading;


private Author author;

/**
* @param reference the reference of the resource to include
* @since 3.4M1
Expand Down Expand Up @@ -122,9 +148,9 @@ public String getSection()
}

/**
* @param excludeFirstHeading {@code true} to remove the first heading found inside
* the document or the section, {@code false} to keep it
* @since 12.4RC1
* @param excludeFirstHeading {@code true} to remove the first heading found inside the document or the section,
* {@code false} to keep it
* @since 12.4RC1
*/
@PropertyName("Exclude First Heading")
@PropertyDescription("Exclude the first heading from the included document or section.")
Expand All @@ -133,10 +159,10 @@ public void setExcludeFirstHeading(boolean excludeFirstHeading)
{
this.excludeFirstHeading = excludeFirstHeading;
}

/**
* @return whether to exclude the first heading from the included document or section, or not.
* @since 12.4RC1
* @since 12.4RC1
*/
public boolean isExcludeFirstHeading()
{
Expand Down Expand Up @@ -206,4 +232,24 @@ public void setPage(String page)
this.reference = page;
this.type = EntityType.PAGE;
}

/**
* @return the author to use to execute the content
* @since 15.0RC1
*/
public Author getAuthor()
{
return this.author;
}

/**
* @param author the author to use to execute the content
* @since 15.0RC1
*/
@PropertyDescription("The author to use to execute the content")
@PropertyAdvanced
public void setAuthor(Author author)
{
this.author = author;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ public TemplateMacro()
{
super("Template", DESCRIPTION, TemplateMacroParameters.class);

// The template macro must execute first since if it runs with the current context it needs to bring
// all the macros from the template before the other macros are executed.
setPriority(10);
setDefaultCategories(Set.of(DEFAULT_CATEGORY_DEVELOPMENT));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ public UIExtensionMacro()
{
super("UI Extensions", DESCRIPTION, UIExtensionsMacroParameters.class);

// The ui extensions macro must execute first since if it runs with the current context it needs to bring
// all the macros from the extension before the other macros are executed.
setPriority(10);
setDefaultCategories(Set.of(DEFAULT_CATEGORY_DEVELOPMENT));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ public UIExtensionsMacro()
{
super("UI Extensions", DESCRIPTION, UIExtensionsMacroParameters.class);

// The ui extensions macro must execute first since if it runs with the current context it needs to bring
// all the macros from the extension before the other macros are executed.
setPriority(10);
setDefaultCategories(Set.of(DEFAULT_CATEGORY_DEVELOPMENT));
}

Expand Down

0 comments on commit b48116a

Please sign in to comment.